* atmel-nand-controller: NAND chip selects? @ 2018-06-11 11:04 Ladislav Michl 2018-06-11 11:21 ` Boris Brezillon 0 siblings, 1 reply; 6+ messages in thread From: Ladislav Michl @ 2018-06-11 11:04 UTC (permalink / raw) To: linux-mtd; +Cc: Alexandre Belloni, Nicolas Ferre, Boris Brezillon Consider there are more NAND chips connected to the same lines and only nCE (connected to GPIO line one per each chip) is used to select them. How is driver supposed to work in such situation? Common memory region cannot be requested multiple times as well as the same gpio for R/B cannot be requested. Something as davinci_nand for memory region? Use 'ranges' property? How should one express in DT gpio is shared between child nodes? Any pointers appreciated. Thank you, ladis ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: atmel-nand-controller: NAND chip selects? 2018-06-11 11:04 atmel-nand-controller: NAND chip selects? Ladislav Michl @ 2018-06-11 11:21 ` Boris Brezillon 2018-06-11 14:42 ` Ladislav Michl 0 siblings, 1 reply; 6+ messages in thread From: Boris Brezillon @ 2018-06-11 11:21 UTC (permalink / raw) To: Ladislav Michl; +Cc: linux-mtd, Alexandre Belloni, Nicolas Ferre On Mon, 11 Jun 2018 13:04:25 +0200 Ladislav Michl <ladis@linux-mips.org> wrote: > Consider there are more NAND chips connected to the same lines and only nCE > (connected to GPIO line one per each chip) is used to select them. > How is driver supposed to work in such situation? > Common memory region cannot be requested multiple times as well as the > same gpio for R/B cannot be requested. I thought you could request several times the same GPIO if it's in input mode, but maybe I'm wrong. > Something as davinci_nand for > memory region? Use 'ranges' property? How should one express in DT gpio > is shared between child nodes? For both problems, the solution is to reserve the resources at the NAND controller level instead of the NAND level. It's pretty easy for the CS memory range, because the driver can easily detect when the same CS is used by different NAND chips. It's a bit more complicated for the R/B pins. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: atmel-nand-controller: NAND chip selects? 2018-06-11 11:21 ` Boris Brezillon @ 2018-06-11 14:42 ` Ladislav Michl 2018-06-11 18:22 ` Boris Brezillon 2018-06-11 20:30 ` Boris Brezillon 0 siblings, 2 replies; 6+ messages in thread From: Ladislav Michl @ 2018-06-11 14:42 UTC (permalink / raw) To: Boris Brezillon; +Cc: linux-mtd, Alexandre Belloni, Nicolas Ferre On Mon, Jun 11, 2018 at 01:21:31PM +0200, Boris Brezillon wrote: > On Mon, 11 Jun 2018 13:04:25 +0200 > Ladislav Michl <ladis@linux-mips.org> wrote: > > > Consider there are more NAND chips connected to the same lines and only nCE > > (connected to GPIO line one per each chip) is used to select them. > > How is driver supposed to work in such situation? > > Common memory region cannot be requested multiple times as well as the > > same gpio for R/B cannot be requested. > > I thought you could request several times the same GPIO if it's in input > mode, but maybe I'm wrong. Well, it does not seem to work: atmel-nand-controller 10000000.ebi:nand-controller: Failed to get R/B gpio (err = -16) > > Something as davinci_nand for > > memory region? Use 'ranges' property? How should one express in DT gpio > > is shared between child nodes? > > For both problems, the solution is to reserve the resources at the NAND > controller level instead of the NAND level. It's pretty easy for the CS > memory range, because the driver can easily detect when the same CS is > used by different NAND chips. It's a bit more complicated for the R/B > pins. Do you mean something like this? ebi: ebi@10000000 { status = "okay"; pinctrl-0 = <&pinctrl_nand_cs &pinctrl_nand_cs1 &pinctrl_nand_rb>; pinctrl-names = "default"; nand_controller: nand-controller { status = "okay"; /* reg = <0x3 0x0 0x800000>; */ nand@3,0 { reg = <0x3 0x0 0x800000>; rb-gpios = <&pioC 13 GPIO_ACTIVE_HIGH>; cs-gpios = <&pioC 8 GPIO_ACTIVE_HIGH>; nand-bus-width = <8>; nand-ecc-mode = "soft"; nand-ecc-algo = "bch"; nand-ecc-strength = <8>; nand-on-flash-bbt; label = "atmel_nand"; }; nand@3,1 { reg = <0x3 0x0 0x800000>; rb-gpios = <&pioC 13 GPIO_ACTIVE_HIGH>; cs-gpios = <&pioC 14 GPIO_ACTIVE_HIGH>; nand-bus-width = <8>; nand-ecc-mode = "soft"; nand-ecc-algo = "bch"; nand-ecc-strength = <8>; nand-on-flash-bbt; label = "atmel_nand"; }; }; }; Note the commented out 'reg' property. I'm not sure if you meant to actively compare 'reg's of nand child nodes or put 'reg' property on 'nand-controller' level (latter does not currently fly): atmel-ebi 10000000.ebi: missing or invalid timings definition in /ahb/ebi@10000000/nand-controller atmel-ebi 10000000.ebi: failed to configure EBI bus for /ahb/ebi@10000000/nand-controller, disabling the device Thank you, ladis ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: atmel-nand-controller: NAND chip selects? 2018-06-11 14:42 ` Ladislav Michl @ 2018-06-11 18:22 ` Boris Brezillon 2018-06-11 20:30 ` Boris Brezillon 1 sibling, 0 replies; 6+ messages in thread From: Boris Brezillon @ 2018-06-11 18:22 UTC (permalink / raw) To: Ladislav Michl; +Cc: linux-mtd, Alexandre Belloni, Nicolas Ferre On Mon, 11 Jun 2018 16:42:14 +0200 Ladislav Michl <ladis@linux-mips.org> wrote: > On Mon, Jun 11, 2018 at 01:21:31PM +0200, Boris Brezillon wrote: > > On Mon, 11 Jun 2018 13:04:25 +0200 > > Ladislav Michl <ladis@linux-mips.org> wrote: > > > > > Consider there are more NAND chips connected to the same lines and only nCE > > > (connected to GPIO line one per each chip) is used to select them. > > > How is driver supposed to work in such situation? > > > Common memory region cannot be requested multiple times as well as the > > > same gpio for R/B cannot be requested. > > > > I thought you could request several times the same GPIO if it's in input > > mode, but maybe I'm wrong. > > Well, it does not seem to work: > atmel-nand-controller 10000000.ebi:nand-controller: Failed to get R/B gpio (err = -16) Okay, that's a problem. > > > > Something as davinci_nand for > > > memory region? Use 'ranges' property? How should one express in DT gpio > > > is shared between child nodes? > > > > For both problems, the solution is to reserve the resources at the NAND > > controller level instead of the NAND level. It's pretty easy for the CS > > memory range, because the driver can easily detect when the same CS is > > used by different NAND chips. It's a bit more complicated for the R/B > > pins. > > Do you mean something like this? > > ebi: ebi@10000000 { > status = "okay"; > pinctrl-0 = <&pinctrl_nand_cs &pinctrl_nand_cs1 &pinctrl_nand_rb>; > pinctrl-names = "default"; > > nand_controller: nand-controller { > status = "okay"; > /* reg = <0x3 0x0 0x800000>; */ > nand@3,0 { > reg = <0x3 0x0 0x800000>; > rb-gpios = <&pioC 13 GPIO_ACTIVE_HIGH>; > cs-gpios = <&pioC 8 GPIO_ACTIVE_HIGH>; > nand-bus-width = <8>; > nand-ecc-mode = "soft"; > nand-ecc-algo = "bch"; > nand-ecc-strength = <8>; > nand-on-flash-bbt; > label = "atmel_nand"; > }; > nand@3,1 { > reg = <0x3 0x0 0x800000>; > rb-gpios = <&pioC 13 GPIO_ACTIVE_HIGH>; > cs-gpios = <&pioC 14 GPIO_ACTIVE_HIGH>; > nand-bus-width = <8>; > nand-ecc-mode = "soft"; > nand-ecc-algo = "bch"; > nand-ecc-strength = <8>; > nand-on-flash-bbt; > label = "atmel_nand"; Two devices with the same label, that probably won't work correctly. > }; > }; > }; > > Note the commented out 'reg' property. I'm not sure if you meant to actively > compare 'reg's of nand child nodes or put 'reg' property on 'nand-controller' I thought about doing the former. The first cell in reg is encoding the EBI CS, so it's pretty easy to have a table of already requested CS ids in struct atmel_nand_controller, and when you see the same CS, you just have to get the already requested iomem region. > level (latter does not currently fly): > atmel-ebi 10000000.ebi: missing or invalid timings definition in /ahb/ebi@10000000/nand-controller > atmel-ebi 10000000.ebi: failed to configure EBI bus for /ahb/ebi@10000000/nand-controller, disabling the device It's caused by this check [1] because I assumed the NAND controller would never have a reg prop defined. If we need to, we can extend the check to make sure the child not does not have an "atmel,xxx-nand-controller" compat. [1]https://elixir.bootlin.com/linux/v4.17/source/drivers/memory/atmel-ebi.c#L573 [2]https://elixir.bootlin.com/linux/v4.17/source/drivers/mtd/nand/raw/atmel/nand-controller.c#L2435 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: atmel-nand-controller: NAND chip selects? 2018-06-11 14:42 ` Ladislav Michl 2018-06-11 18:22 ` Boris Brezillon @ 2018-06-11 20:30 ` Boris Brezillon 2018-06-11 21:31 ` Ladislav Michl 1 sibling, 1 reply; 6+ messages in thread From: Boris Brezillon @ 2018-06-11 20:30 UTC (permalink / raw) To: Ladislav Michl; +Cc: linux-mtd, Alexandre Belloni, Nicolas Ferre On Mon, 11 Jun 2018 16:42:14 +0200 Ladislav Michl <ladis@linux-mips.org> wrote: > On Mon, Jun 11, 2018 at 01:21:31PM +0200, Boris Brezillon wrote: > > On Mon, 11 Jun 2018 13:04:25 +0200 > > Ladislav Michl <ladis@linux-mips.org> wrote: > > > > > Consider there are more NAND chips connected to the same lines and only nCE > > > (connected to GPIO line one per each chip) is used to select them. > > > How is driver supposed to work in such situation? > > > Common memory region cannot be requested multiple times as well as the > > > same gpio for R/B cannot be requested. > > > > I thought you could request several times the same GPIO if it's in input > > mode, but maybe I'm wrong. > > Well, it does not seem to work: > atmel-nand-controller 10000000.ebi:nand-controller: Failed to get R/B gpio (err = -16) > > > > Something as davinci_nand for > > > memory region? Use 'ranges' property? How should one express in DT gpio > > > is shared between child nodes? > > > > For both problems, the solution is to reserve the resources at the NAND > > controller level instead of the NAND level. It's pretty easy for the CS > > memory range, because the driver can easily detect when the same CS is > > used by different NAND chips. It's a bit more complicated for the R/B > > pins. > > Do you mean something like this? > > ebi: ebi@10000000 { > status = "okay"; > pinctrl-0 = <&pinctrl_nand_cs &pinctrl_nand_cs1 &pinctrl_nand_rb>; > pinctrl-names = "default"; > > nand_controller: nand-controller { > status = "okay"; > /* reg = <0x3 0x0 0x800000>; */ > nand@3,0 { > reg = <0x3 0x0 0x800000>; > rb-gpios = <&pioC 13 GPIO_ACTIVE_HIGH>; > cs-gpios = <&pioC 8 GPIO_ACTIVE_HIGH>; > nand-bus-width = <8>; > nand-ecc-mode = "soft"; > nand-ecc-algo = "bch"; > nand-ecc-strength = <8>; > nand-on-flash-bbt; > label = "atmel_nand"; > }; > nand@3,1 { > reg = <0x3 0x0 0x800000>; > rb-gpios = <&pioC 13 GPIO_ACTIVE_HIGH>; > cs-gpios = <&pioC 14 GPIO_ACTIVE_HIGH>; > nand-bus-width = <8>; > nand-ecc-mode = "soft"; > nand-ecc-algo = "bch"; > nand-ecc-strength = <8>; > nand-on-flash-bbt; > label = "atmel_nand"; > }; > }; > }; I thought a bit more about your problem, and maybe we should have this kind of representation: ebi: ebi@10000000 { status = "okay"; pinctrl-0 = <&pinctrl_nand_cs &pinctrl_nand_cs1 &pinctrl_nand_rb>; pinctrl-names = "default"; nand_controller: nand-controller { status = "okay"; reg = <0x3 0x0 0x800000>; rb-gpios = <&pioC 13 GPIO_ACTIVE_HIGH>; cs-gpios = <&pioC 8 GPIO_ACTIVE_HIGH>, <&pioC 14 GPIO_ACTIVE_HIGH>; #address-cells = <1>; #size-cells = <0>; cs0-ebi-id = <3>; cs0-cs-gpio = <0>; cs0-rb-gpio = <0>; cs1-ebi-id = <3>; cs1-cs-gpio = <1>; cs1-rb-gpio = <0>; nand@0 { reg = <0>; nand-bus-width = <8>; nand-ecc-mode = "soft"; nand-ecc-algo = "bch"; nand-ecc-strength = <8>; nand-on-flash-bbt; label = "mynand0"; }; nand@1 { reg = <1>; nand-bus-width = <8>; nand-ecc-mode = "soft"; nand-ecc-algo = "bch"; nand-ecc-strength = <8>; nand-on-flash-bbt; label = "mynand1"; }; }; }; This way we have all resources attached to the NAND controller, and a translation from virutal CS-ids to physical resources done through the csY-xxx props. Of course, that means patching the DT bindings doc and the EBI + NAND controller drivers to support this new representation. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: atmel-nand-controller: NAND chip selects? 2018-06-11 20:30 ` Boris Brezillon @ 2018-06-11 21:31 ` Ladislav Michl 0 siblings, 0 replies; 6+ messages in thread From: Ladislav Michl @ 2018-06-11 21:31 UTC (permalink / raw) To: Boris Brezillon; +Cc: linux-mtd, Alexandre Belloni, Nicolas Ferre On Mon, Jun 11, 2018 at 10:30:51PM +0200, Boris Brezillon wrote: > On Mon, 11 Jun 2018 16:42:14 +0200 > Ladislav Michl <ladis@linux-mips.org> wrote: > > > On Mon, Jun 11, 2018 at 01:21:31PM +0200, Boris Brezillon wrote: > > > On Mon, 11 Jun 2018 13:04:25 +0200 > > > Ladislav Michl <ladis@linux-mips.org> wrote: > > > > > > > Consider there are more NAND chips connected to the same lines and only nCE > > > > (connected to GPIO line one per each chip) is used to select them. > > > > How is driver supposed to work in such situation? > > > > Common memory region cannot be requested multiple times as well as the > > > > same gpio for R/B cannot be requested. > > > > > > I thought you could request several times the same GPIO if it's in input > > > mode, but maybe I'm wrong. > > > > Well, it does not seem to work: > > atmel-nand-controller 10000000.ebi:nand-controller: Failed to get R/B gpio (err = -16) > > > > > > Something as davinci_nand for > > > > memory region? Use 'ranges' property? How should one express in DT gpio > > > > is shared between child nodes? > > > > > > For both problems, the solution is to reserve the resources at the NAND > > > controller level instead of the NAND level. It's pretty easy for the CS > > > memory range, because the driver can easily detect when the same CS is > > > used by different NAND chips. It's a bit more complicated for the R/B > > > pins. > > > > Do you mean something like this? > > > > ebi: ebi@10000000 { > > status = "okay"; > > pinctrl-0 = <&pinctrl_nand_cs &pinctrl_nand_cs1 &pinctrl_nand_rb>; > > pinctrl-names = "default"; > > > > nand_controller: nand-controller { > > status = "okay"; > > /* reg = <0x3 0x0 0x800000>; */ > > nand@3,0 { > > reg = <0x3 0x0 0x800000>; > > rb-gpios = <&pioC 13 GPIO_ACTIVE_HIGH>; > > cs-gpios = <&pioC 8 GPIO_ACTIVE_HIGH>; > > nand-bus-width = <8>; > > nand-ecc-mode = "soft"; > > nand-ecc-algo = "bch"; > > nand-ecc-strength = <8>; > > nand-on-flash-bbt; > > label = "atmel_nand"; > > }; > > nand@3,1 { > > reg = <0x3 0x0 0x800000>; > > rb-gpios = <&pioC 13 GPIO_ACTIVE_HIGH>; > > cs-gpios = <&pioC 14 GPIO_ACTIVE_HIGH>; > > nand-bus-width = <8>; > > nand-ecc-mode = "soft"; > > nand-ecc-algo = "bch"; > > nand-ecc-strength = <8>; > > nand-on-flash-bbt; > > label = "atmel_nand"; > > }; > > }; > > }; > > I thought a bit more about your problem, and maybe we should have this > kind of representation: > > ebi: ebi@10000000 { > status = "okay"; > pinctrl-0 = <&pinctrl_nand_cs &pinctrl_nand_cs1 &pinctrl_nand_rb>; > pinctrl-names = "default"; > > nand_controller: nand-controller { > status = "okay"; > reg = <0x3 0x0 0x800000>; > rb-gpios = <&pioC 13 GPIO_ACTIVE_HIGH>; > cs-gpios = <&pioC 8 GPIO_ACTIVE_HIGH>, > <&pioC 14 GPIO_ACTIVE_HIGH>; > > #address-cells = <1>; > #size-cells = <0>; > > cs0-ebi-id = <3>; > cs0-cs-gpio = <0>; > cs0-rb-gpio = <0>; > cs1-ebi-id = <3>; > cs1-cs-gpio = <1>; > cs1-rb-gpio = <0>; > > nand@0 { > reg = <0>; > nand-bus-width = <8>; > nand-ecc-mode = "soft"; > nand-ecc-algo = "bch"; > nand-ecc-strength = <8>; > nand-on-flash-bbt; > label = "mynand0"; > }; > > nand@1 { > reg = <1>; > nand-bus-width = <8>; > nand-ecc-mode = "soft"; > nand-ecc-algo = "bch"; > nand-ecc-strength = <8>; > nand-on-flash-bbt; > label = "mynand1"; > }; > }; > }; > > This way we have all resources attached to the NAND controller, and a > translation from virutal CS-ids to physical resources done through the > csY-xxx props. > > Of course, that means patching the DT bindings doc and the EBI + NAND > controller drivers to support this new representation. Ugh... Tried to put this idea into code and so far it looks horribly (I mean implementation supporting both present and suggested bindings looks ungly, that said I liked your previous suggestion more, but let me think about it again tomorrow) Otherwise driver can be hacked down to work with this hardware configuration: nand: device found, Manufacturer ID: 0x98, Chip ID: 0xda nand: Toshiba NAND 256MiB 3,3V 8-bit nand: 256 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 128 nand: WARNING: atmel_nand: the ECC used on your system is too weak compared to the one required by the NAND chip Bad block table found at page 131008, version 0x01 Bad block table found at page 130944, version 0x01 nand: device found, Manufacturer ID: 0xc2, Chip ID: 0xdc nand: Macronix MX30LF4G18AC nand: 512 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64 nand: timing mode 3 not acknowledged by the NAND chip Bad block table found at page 262080, version 0x01 Bad block table found at page 262016, version 0x01 nand_read_bbt: bad block at 0x000014b40000 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-06-11 21:31 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-06-11 11:04 atmel-nand-controller: NAND chip selects? Ladislav Michl 2018-06-11 11:21 ` Boris Brezillon 2018-06-11 14:42 ` Ladislav Michl 2018-06-11 18:22 ` Boris Brezillon 2018-06-11 20:30 ` Boris Brezillon 2018-06-11 21:31 ` Ladislav Michl
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).