Messages in this thread | | | Date | Thu, 11 May 2023 11:59:07 +0300 | Subject | Re: [PATCH v3 6/6] mtd: rawnand: meson: rename node for chip select | From | Arseniy Krasnov <> |
| |
On 10.05.2023 23:53, Miquel Raynal wrote:
Hello Martin, Miquel
> Hi Martin & Arseniy, > > martin.blumenstingl@googlemail.com wrote on Wed, 10 May 2023 22:40:37 > +0200: > >> Hello Arseniy, >> >> On Wed, May 10, 2023 at 1:13 PM Arseniy Krasnov >> <AVKrasnov@sberdevices.ru> wrote: >>> >>> This renames node with values for chip select from "reg" to "cs". It is >>> needed because when OTP access is enabled on the attached storage, MTD >>> subsystem registers this storage in the NVMEM subsystem. NVMEM in turn >>> tries to use "reg" node in its own manner, supposes that it has another >>> layout. All of this leads to device initialization failure. >> In general: if we change the device-tree interface (in this case: >> replacing a "reg" with a "cs" property) the dt-bindings have to be >> updated as well. > > True, and I would add, bindings should not be broken.
I see, that's true. That is bad way to change bindings.
> >> Documentation/devicetree/bindings/mtd/nand-controller.yaml and >> Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml show >> that the chip select of a NAND chip is specified with a "reg" >> property. > > All NAND controller binding expect the chip-select to be in the > 'reg' property, very much like a spi device would use reg to store the > cs as well: the reg property tells you how you address the device. > > I also fully agree with Martin's comments below. Changing reg is likely > a wrong approach :) > >> Also the code has to be backwards compatible with old .dtbs. >> >>> Example: >>> >>> [...] nvmem mtd0-user-otp: nvmem: invalid reg on /soc/bus@ffe00000/... >>> [...] mtd mtd0: Failed to register OTP NVMEM device >>> [...] meson-nand ffe07800.nfc: failed to register MTD device: -22 >>> [...] meson-nand ffe07800.nfc: failed to init NAND chips >>> [...] meson-nand: probe of ffe07800.nfc failed with error -22 >> This is odd - can you please share your definition of the &nfc node?
Sure, here it is:
mtd_nand: nfc@7800 { compatible = "amlogic,meson-axg-nfc"; ... nand@0 { reg = <0>; }; }
I checked, that 'nand_set_flash_node()' is called with 'nand@0' and i suppose that it is correct (as You mentioned below). But, 'nvmem_add_cells_from_of()' is called with parent: 'nfc@7800', then it iterates over its childs, e.g. 'nand@0' and thus i get such situation. I guess, that 'nvmem_add_cells_from_of()' must be called with 'nand@0' ?
>> >> &nfc { >> nand_chip0: nand@0 { >> reg = <0>; >> }; >> }; >> >> This should result in nand_set_flash_node() being called with >> &nand_chip0 (if it's called with &nfc then something is buggy in our >> driver). >> If there's no child nodes within &nand_chip0 then why would the >> MTD-to-NVMEM code think that it has to parse something? >> If you do have child nodes and those are partitions, then make sure >> that the structure is correct (see the extra "partitions" node inside >> which all partitions are nested): >> &nand_chip0 { >> partitions { >> compatible = "fixed-partitions"; >> #address-cells = <1>; >> #size-cells = <1>; >> >> partition@0 { >> label = "u-boot"; >> reg = <0x0000000 0x4000>; >> read-only; >> }; >> }; >> };
No, partition nodes are disabled in this case.
Thanks, Arseniy
>> >> >> Best regards,, >> Martin > > > Thanks, > Miquèl
| |