| Message ID | 20231108-fix-mips-nand-v1-2-5fc5586d04de@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Fix polarity and bindings for GPIO-based NAND drivers | expand |
Hello Linus, linus.walleij@linaro.org wrote on Wed, 08 Nov 2023 15:33:50 +0100: > This creates a schema for GPIO controlled NAND. The txt > schema was old and wrong. > > Mark the old way of passing GPIOs in a long array as > deprecated and encourage per-pin GPIO assignments with > the named nnn-gpios phandles. > > I was unable to re-use raw-nand-chip.yaml or even > nand-chip.yaml: the reason is that they both assume > that we have potentially several NAND chips with chip > selects and thus enforce a node name "nand@0" etc, > which doesn't quite work for this device. But what about nand-controller.yaml? This driver is just about emulating what a NAND controller would do with GPIOs, any NAND chip can be wired, no? > Since the GPIO controlled NAND is both a NAND controller > and a NAND chip jitted together, Not really, it's just the controller part? I know for years NAND controllers, ECC engines and NAND chips have been considered a single hardware entity, but I believe this one is just about emulating the host controller part. > I have to modify the > mtd.yaml nodename requirement to include nand-controller@ > as this is the nodename that this device should use. > > Deprecate the custom "band-width" property in favor of > "nand-bus-width". > > Reported-by: Howard Harte <hharte@magicandroidapps.com> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > Check the required section especially. Since there is no > hardware default for anything when using GPIOs for this, > I think all these parameters are compulsory. > --- > .../devicetree/bindings/mtd/gpio-control-nand.txt | 47 ------ > .../devicetree/bindings/mtd/gpio-control-nand.yaml | 168 +++++++++++++++++++++ > Documentation/devicetree/bindings/mtd/mtd.yaml | 2 +- > 3 files changed, 169 insertions(+), 48 deletions(-) > > diff --git a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt > deleted file mode 100644 > index 486a17d533d7..000000000000 > --- a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt > +++ /dev/null > @@ -1,47 +0,0 @@ > -GPIO assisted NAND flash > - > -The GPIO assisted NAND flash uses a memory mapped interface to > -read/write the NAND commands and data and GPIO pins for the control > -signals. > - > -Required properties: > -- compatible : "gpio-control-nand" > -- reg : should specify localbus chip select and size used for the chip. The > - resource describes the data bus connected to the NAND flash and all accesses > - are made in native endianness. > -- #address-cells, #size-cells : Must be present if the device has sub-nodes > - representing partitions. > -- gpios : Specifies the GPIO pins to control the NAND device. The order of > - GPIO references is: RDY, nCE, ALE, CLE, and nWP. nCE and nWP are optional. > - > -Optional properties: > -- bank-width : Width (in bytes) of the device. If not present, the width > - defaults to 1 byte. > -- chip-delay : chip dependent delay for transferring data from array to > - read registers (tR). If not present then a default of 20us is used. > -- gpio-control-nand,io-sync-reg : A 64-bit physical address for a read > - location used to guard against bus reordering with regards to accesses to > - the GPIO's and the NAND flash data bus. If present, then after changing > - GPIO state and before and after command byte writes, this register will be > - read to ensure that the GPIO accesses have completed. > - > -The device tree may optionally contain sub-nodes describing partitions of the > -address space. See partition.txt for more detail. > - > -Examples: > - > -gpio-nand@1,0 { > - compatible = "gpio-control-nand"; > - reg = <1 0x0000 0x2>; > - #address-cells = <1>; > - #size-cells = <1>; > - gpios = <&banka 1 0>, /* RDY */ > - <0>, /* nCE */ > - <&banka 3 0>, /* ALE */ > - <&banka 4 0>, /* CLE */ > - <0>; /* nWP */ > - > - partition@0 { > - ... > - }; > -}; > diff --git a/Documentation/devicetree/bindings/mtd/gpio-control-nand.yaml b/Documentation/devicetree/bindings/mtd/gpio-control-nand.yaml > new file mode 100644 > index 000000000000..5b30ee7ad4a5 > --- /dev/null > +++ b/Documentation/devicetree/bindings/mtd/gpio-control-nand.yaml > @@ -0,0 +1,168 @@ > +# SPDX-License-Identifier: GPL-2.0 > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mtd/gpio-control-nand.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: NAND memory exclusively connected to GPIO lines > + > +maintainers: > + - Linus Walleij <linus.walleij@linaro.org> > + > +description: | > + It is possible to connect a NAND flash memory without any > + dedicated NAND controller hardware, using just general purpose > + I/O (GPIO) pins. This will not be fast, but it will work. > + The address and data lines of the chip will still need to be > + connected so that the contents of a NAND page can be > + memory-mapped and accessed after the special lines are toggled > + by GPIO. > + > +# The GPIO controlled NAND has wires going directly to one single > +# NAND chip, so it is both a nand controller and a nand chip at > +# the same time, but it does not have things such as chip select > +# since the use is hammered down to one single chip only. > +# There is no point for the chip itself to be a subnode of the > +# controller so the raw NAND chip properties are added right into > +# the controller node like this. I kind of disagree here, this "piece of software" only replaces a NAND controller. You always need a NAND chip in front of it, and that's a specific piece of hardware anyway. Or maybe I don't understand the hardware behind? (truly not impossible) > + > +allOf: > + - $ref: mtd.yaml# > + > +properties: > + $nodename: > + pattern: "^(nand|nand-controller)@[a-f0-9]+$" > + > + compatible: > + const: gpio-control-nand > + > + reg: > + description: | > + This should specify the address where the NAND page currently > + accessed gets memory-mapped, and the size of the page. Usually > + this will be the same as the page size of the NAND. This is definitely a host controller parameter. Even if the hardware only supports a single NAND chip, I believe it should be described as a subnode with a dummy "reg = <0>;" property. Thanks, Miquèl
On Wed, Nov 08, 2023 at 03:33:50PM +0100, Linus Walleij wrote: > This creates a schema for GPIO controlled NAND. The txt > schema was old and wrong. > > Mark the old way of passing GPIOs in a long array as > deprecated and encourage per-pin GPIO assignments with > the named nnn-gpios phandles. We have 1 user upstream with only a single commit adding it in 2017. This doesn't seem like something that's going to gain new users either. Is it really worth modernizing this binding? Rob
On Wed, Nov 8, 2023 at 8:11 PM Rob Herring <robh@kernel.org> wrote: > On Wed, Nov 08, 2023 at 03:33:50PM +0100, Linus Walleij wrote: > > This creates a schema for GPIO controlled NAND. The txt > > schema was old and wrong. > > > > Mark the old way of passing GPIOs in a long array as > > deprecated and encourage per-pin GPIO assignments with > > the named nnn-gpios phandles. > > We have 1 user upstream with only a single commit adding it in 2017. > This doesn't seem like something that's going to gain new users either. > Is it really worth modernizing this binding? The whole ordeal was actually prompted by the emergence of a new user who wants to add a device tree for a new device. So I don't want to add new users for the old bindings. Yours, Linus Walleij
diff --git a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt deleted file mode 100644 index 486a17d533d7..000000000000 --- a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt +++ /dev/null @@ -1,47 +0,0 @@ -GPIO assisted NAND flash - -The GPIO assisted NAND flash uses a memory mapped interface to -read/write the NAND commands and data and GPIO pins for the control -signals. - -Required properties: -- compatible : "gpio-control-nand" -- reg : should specify localbus chip select and size used for the chip. The - resource describes the data bus connected to the NAND flash and all accesses - are made in native endianness. -- #address-cells, #size-cells : Must be present if the device has sub-nodes - representing partitions. -- gpios : Specifies the GPIO pins to control the NAND device. The order of - GPIO references is: RDY, nCE, ALE, CLE, and nWP. nCE and nWP are optional. - -Optional properties: -- bank-width : Width (in bytes) of the device. If not present, the width - defaults to 1 byte. -- chip-delay : chip dependent delay for transferring data from array to - read registers (tR). If not present then a default of 20us is used. -- gpio-control-nand,io-sync-reg : A 64-bit physical address for a read - location used to guard against bus reordering with regards to accesses to - the GPIO's and the NAND flash data bus. If present, then after changing - GPIO state and before and after command byte writes, this register will be - read to ensure that the GPIO accesses have completed. - -The device tree may optionally contain sub-nodes describing partitions of the -address space. See partition.txt for more detail. - -Examples: - -gpio-nand@1,0 { - compatible = "gpio-control-nand"; - reg = <1 0x0000 0x2>; - #address-cells = <1>; - #size-cells = <1>; - gpios = <&banka 1 0>, /* RDY */ - <0>, /* nCE */ - <&banka 3 0>, /* ALE */ - <&banka 4 0>, /* CLE */ - <0>; /* nWP */ - - partition@0 { - ... - }; -}; diff --git a/Documentation/devicetree/bindings/mtd/gpio-control-nand.yaml b/Documentation/devicetree/bindings/mtd/gpio-control-nand.yaml new file mode 100644 index 000000000000..5b30ee7ad4a5 --- /dev/null +++ b/Documentation/devicetree/bindings/mtd/gpio-control-nand.yaml @@ -0,0 +1,168 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mtd/gpio-control-nand.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: NAND memory exclusively connected to GPIO lines + +maintainers: + - Linus Walleij <linus.walleij@linaro.org> + +description: | + It is possible to connect a NAND flash memory without any + dedicated NAND controller hardware, using just general purpose + I/O (GPIO) pins. This will not be fast, but it will work. + The address and data lines of the chip will still need to be + connected so that the contents of a NAND page can be + memory-mapped and accessed after the special lines are toggled + by GPIO. + +# The GPIO controlled NAND has wires going directly to one single +# NAND chip, so it is both a nand controller and a nand chip at +# the same time, but it does not have things such as chip select +# since the use is hammered down to one single chip only. +# There is no point for the chip itself to be a subnode of the +# controller so the raw NAND chip properties are added right into +# the controller node like this. + +allOf: + - $ref: mtd.yaml# + +properties: + $nodename: + pattern: "^(nand|nand-controller)@[a-f0-9]+$" + + compatible: + const: gpio-control-nand + + reg: + description: | + This should specify the address where the NAND page currently + accessed gets memory-mapped, and the size of the page. Usually + this will be the same as the page size of the NAND. + + label: true + + partitions: true + + nand-ecc-algo: true + + nand-ecc-step-size: true + + nand-ecc-strength: true + + nand-use-soft-ecc-engine: true + + gpio-control-nand,io-sync-reg: + description: | + A 64-bit physical address for a read location used to guard + against bus reordering with regards to accesses to the GPIOs and + the NAND flash data bus. If present, then after changing GPIO state + and before and after command byte writes, this register will be + read to ensure that the GPIO accesses have completed. + $ref: /schemas/types.yaml#/definitions/uint64 + + gpios: + description: + Legacy GPIO array for the NAND chip lines, order RDY, + NCE, ALE, CLE, NWP. + deprecated: true + maxItems: 5 + + rdy-gpios: + description: + GPIO for the NAND chip RDY line + maxItems: 1 + + ce-gpios: + description: + GPIO for the NAND chip CE chip enable line, usually + this is active low, so it should be tagged with the GPIO + flag GPIO_ACTIVE_LOW. + maxItems: 1 + + ale-gpios: + description: + GPIO for the NAND chip ALE line + maxItems: 1 + + cle-gpios: + description: + GPIO for the NAND chip CLE line + maxItems: 1 + + wp-gpios: + description: + GPIO for the NAND chip WP line, usually this is + active low, so it should be tagged with the GPIO + flag GPIO_ACTIVE_LOW. + maxItems: 1 + + bank-width: + description: + Width (in bytes) of the device. If not present, the + width defaults to 1 byte. This is deprecated, use + nand-bus-width instead. + deprecated: true + enum: [ 1, 2 ] + default: 1 + + nand-bus-width: + description: + Bus width to the NAND chip + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [8, 16] + default: 8 + + chip-delay: + description: + chip dependent delay for transferring data from array to + read registers (tR). If not present then a default of 20us + is used. + $ref: /schemas/types.yaml#/definitions/uint32 + +required: + - compatible + - reg + - ale-gpios + - cle-gpios + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + nand@20200000 { + compatible = "gpio-control-nand"; + /* 512 bytes memory window at 0x20200000 */ + reg = <0x20200000 0x200>; + rdy-gpios = <&gpio0 7 GPIO_ACTIVE_HIGH>; + ce-gpios = <&gpio0 12 GPIO_ACTIVE_LOW>; + ale-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>; + cle-gpios = <&gpio0 8 GPIO_ACTIVE_HIGH>; + wp-gpios = <&gpio0 13 GPIO_ACTIVE_LOW>; + + label = "ixp400 NAND"; + + nand-use-soft-ecc-engine; + nand-ecc-algo = "bch"; + nand-ecc-step-size = <512>; + nand-ecc-strength = <4>; + + partitions { + compatible = "fixed-partitions"; + #address-cells = <1>; + #size-cells = <1>; + + fs@0 { + label = "SysA Kernel"; + reg = <0x0 0x400000>; + }; + + fs@400000 { + label = "SysA Code"; + reg = <0x400000 0x7C00000>; + }; + }; + }; diff --git a/Documentation/devicetree/bindings/mtd/mtd.yaml b/Documentation/devicetree/bindings/mtd/mtd.yaml index f322290ee516..e6fd82cbc35d 100644 --- a/Documentation/devicetree/bindings/mtd/mtd.yaml +++ b/Documentation/devicetree/bindings/mtd/mtd.yaml @@ -12,7 +12,7 @@ maintainers: properties: $nodename: - pattern: "^(flash|.*sram|nand)(@.*)?$" + pattern: "^(flash|.*sram|nand|nand-controller)(@.*)?$" label: description:
This creates a schema for GPIO controlled NAND. The txt schema was old and wrong. Mark the old way of passing GPIOs in a long array as deprecated and encourage per-pin GPIO assignments with the named nnn-gpios phandles. I was unable to re-use raw-nand-chip.yaml or even nand-chip.yaml: the reason is that they both assume that we have potentially several NAND chips with chip selects and thus enforce a node name "nand@0" etc, which doesn't quite work for this device. Since the GPIO controlled NAND is both a NAND controller and a NAND chip jitted together, I have to modify the mtd.yaml nodename requirement to include nand-controller@ as this is the nodename that this device should use. Deprecate the custom "band-width" property in favor of "nand-bus-width". Reported-by: Howard Harte <hharte@magicandroidapps.com> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- Check the required section especially. Since there is no hardware default for anything when using GPIOs for this, I think all these parameters are compulsory. --- .../devicetree/bindings/mtd/gpio-control-nand.txt | 47 ------ .../devicetree/bindings/mtd/gpio-control-nand.yaml | 168 +++++++++++++++++++++ Documentation/devicetree/bindings/mtd/mtd.yaml | 2 +- 3 files changed, 169 insertions(+), 48 deletions(-)