* 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.