Re: [PATCH 2/2] mtd: nand: raw: atmel: Add error handling when rb-gpios missing
Hi
Il mer 20 set 2023, 08:13 Alexander Dahl ha scritto:
> Hello Eugen, hello Michael,
>
> Am Wed, Aug 23, 2023 at 01:59:58PM +0300 schrieb Eugen Hristev:
> > On 8/23/23 09:54, Michael Nazzareno Trimarchi wrote:
> > > Hi
> > >
> > > On Wed, Aug 23, 2023 at 8:28 AM Eugen Hristev
> > > wrote:
> > > >
> > > > Hi,
> > > >
> > > > On 8/8/23 18:03, Alexander Dahl wrote:
> > > > > Hello Michael,
> > > > >
> > > > > Am Tue, Aug 08, 2023 at 03:49:45PM +0200 schrieb Michael Nazzareno
> Trimarchi:
> > > > > > Hi
> > > > > >
> > > > > > On Tue, Aug 8, 2023 at 3:03 PM Alexander Dahl
> wrote:
> > > > > > >
> > > > > > > Adapt behaviour to Linux kernel driver.
> > > > > > >
> > > > > > > The return value of gpio_request_by_name_nodev() was not
> checked before,
> > > > > > > and thus in case 'rb-gpios' was missing in DT, rb.type was set
> to
> > > > > > > ATMEL_NAND_GPIO_RB nevertheless, leading to output like this
> for
> > > > > > > example (on sam9x60-curiosity with the line removed from dts):
> > > > > > >
> > > > > > > NAND: Could not find valid ONFI parameter page; aborting
> > > > > > > device found, Manufacturer ID: 0xc2, Chip ID: 0xdc
> > > > > > > Macronix NAND 512MiB 3,3V 8-bit
> > > > > > > 512 MiB, SLC, erase size: 256 KiB, page size: 4096, OOB
> size: 64
> > > > > > > atmel-nand-controller nand-controller: NAND scan failed:
> -22
> > > > > > > Failed to probe nand driver (err = -22)
> > > > > > > Failed to initialize NAND controller. (error -22)
> > > > > > > 0 MiB
> > > > > > >
> > > > > > > Note: not having that gpio assigned in dts is fine, the driver
> does not
> > > > > > > override nand_chip->dev_ready() then and a generic solution is
> used.
> > > > > > >
> > > > > > > Fixes: 6a8dfd57220d ("nand: atmel: Add DM based NAND driver")
> > > > > > > Signed-off-by: Alexander Dahl
> > > > > > > ---
> > > > > > >drivers/mtd/nand/raw/atmel/nand-controller.c | 11
> +++
> > > > > > >1 file changed, 7 insertions(+), 4 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c
> b/drivers/mtd/nand/raw/atmel/nand-controller.c
> > > > > > > index 2b29c8def6..8e745a5111 100644
> > > > > > > --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> > > > > > > +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> > > > > > > @@ -1600,10 +1600,13 @@ static struct atmel_nand
> *atmel_nand_create(struct atmel_nand_controller *nc,
> > > > > > > nand->cs[i].rb.type =
> ATMEL_NAND_NATIVE_RB;
> > > > > > > nand->cs[i].rb.id = val;
> > > > > > > } else {
> > > > > > > - gpio_request_by_name_nodev(np,
> "rb-gpios", 0,
> > > > > > > -
> &nand->cs[i].rb.gpio,
> > > > > > > -
> GPIOD_IS_IN);
> > > > > > > - nand->cs[i].rb.type =
> ATMEL_NAND_GPIO_RB;
> > > > > > > + ret = gpio_request_by_name_nodev(np,
> "rb-gpios", 0,
> > > > > > > +
> &nand->cs[i].rb.gpio,
> > > > > > > +
> GPIOD_IS_IN);
> > > > > > > + if (ret)
> > > > > > > + dev_err(nc->dev, "Failed to
> get R/B gpio (err = %d)\n", ret);
> > > > > >
> > > > > > Should not then an error here
> > > > >
> > > > > Different log level or no message at all?
> > > > >
> > > > > Note: Linux prints the same message with error level in that case.
> > > > >
> > > > > Greets
> > > > > Alex
> > > > >
> > > >
> > > > Since the rb-gpios is optional, we can continue probing without it.
> > > > Throwing an error message is optional and pure informative. So I am
> fine
> > > > with it
> > > >
> > >
> > > Yes ok, but I'm not sure linux give an error if the gpio is get as
> > > optional and condition
> > > is IS_ERR. Am I right?
> >
> >
> > if (IS_ERR(gpio) && PTR_ERR(gpio) != -ENOENT) {
> > dev_err(nc->dev,
> > "Failed to get R/B gpio (err =
> %ld)\n",
> > PTR_ERR(gpio));
> > return ERR_CAST(gpio);
> > }
> >
> > So Linux throws the message if IS_ERR . If the property is missing
> (ENOENT)
> > it moves on.
> >
> > Can we replicate the same behavior or this behavior does not suit us in
> > U-boot ?
> >
> > Basically I think it should be :
> >
> > if (ret && ret != -ENOENT)
> > dev_err(...)
> > if (!ret)
> > rb.type = ATMEL_NAND_GPIO_RB;
> >
> > Is this what you had in mind Michael ?
>
> The discussion between you guys somehow got stuck. I still have this
> patch on my TODO list, but I'm not sure how to proceed. Please advice
> in which direction to go forward, there's still an error condition
> (described in the initial commit message) here, which needs to be
> handled, otherwise the NAND flash might not be usable.
Yes the suggestion above is what is the rig
Re: [PATCH 2/2] mtd: nand: raw: atmel: Add error handling when rb-gpios missing
Hello Eugen, hello Michael,
Am Wed, Aug 23, 2023 at 01:59:58PM +0300 schrieb Eugen Hristev:
> On 8/23/23 09:54, Michael Nazzareno Trimarchi wrote:
> > Hi
> >
> > On Wed, Aug 23, 2023 at 8:28 AM Eugen Hristev
> > wrote:
> > >
> > > Hi,
> > >
> > > On 8/8/23 18:03, Alexander Dahl wrote:
> > > > Hello Michael,
> > > >
> > > > Am Tue, Aug 08, 2023 at 03:49:45PM +0200 schrieb Michael Nazzareno
> > > > Trimarchi:
> > > > > Hi
> > > > >
> > > > > On Tue, Aug 8, 2023 at 3:03 PM Alexander Dahl
> > > > > wrote:
> > > > > >
> > > > > > Adapt behaviour to Linux kernel driver.
> > > > > >
> > > > > > The return value of gpio_request_by_name_nodev() was not checked
> > > > > > before,
> > > > > > and thus in case 'rb-gpios' was missing in DT, rb.type was set to
> > > > > > ATMEL_NAND_GPIO_RB nevertheless, leading to output like this for
> > > > > > example (on sam9x60-curiosity with the line removed from dts):
> > > > > >
> > > > > > NAND: Could not find valid ONFI parameter page; aborting
> > > > > > device found, Manufacturer ID: 0xc2, Chip ID: 0xdc
> > > > > > Macronix NAND 512MiB 3,3V 8-bit
> > > > > > 512 MiB, SLC, erase size: 256 KiB, page size: 4096, OOB size:
> > > > > > 64
> > > > > > atmel-nand-controller nand-controller: NAND scan failed: -22
> > > > > > Failed to probe nand driver (err = -22)
> > > > > > Failed to initialize NAND controller. (error -22)
> > > > > > 0 MiB
> > > > > >
> > > > > > Note: not having that gpio assigned in dts is fine, the driver does
> > > > > > not
> > > > > > override nand_chip->dev_ready() then and a generic solution is used.
> > > > > >
> > > > > > Fixes: 6a8dfd57220d ("nand: atmel: Add DM based NAND driver")
> > > > > > Signed-off-by: Alexander Dahl
> > > > > > ---
> > > > > >drivers/mtd/nand/raw/atmel/nand-controller.c | 11 +++
> > > > > >1 file changed, 7 insertions(+), 4 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c
> > > > > > b/drivers/mtd/nand/raw/atmel/nand-controller.c
> > > > > > index 2b29c8def6..8e745a5111 100644
> > > > > > --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> > > > > > +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> > > > > > @@ -1600,10 +1600,13 @@ static struct atmel_nand
> > > > > > *atmel_nand_create(struct atmel_nand_controller *nc,
> > > > > > nand->cs[i].rb.type =
> > > > > > ATMEL_NAND_NATIVE_RB;
> > > > > > nand->cs[i].rb.id = val;
> > > > > > } else {
> > > > > > - gpio_request_by_name_nodev(np, "rb-gpios",
> > > > > > 0,
> > > > > > -
> > > > > > &nand->cs[i].rb.gpio,
> > > > > > - GPIOD_IS_IN);
> > > > > > - nand->cs[i].rb.type = ATMEL_NAND_GPIO_RB;
> > > > > > + ret = gpio_request_by_name_nodev(np,
> > > > > > "rb-gpios", 0,
> > > > > > +
> > > > > > &nand->cs[i].rb.gpio,
> > > > > > +
> > > > > > GPIOD_IS_IN);
> > > > > > + if (ret)
> > > > > > + dev_err(nc->dev, "Failed to get R/B
> > > > > > gpio (err = %d)\n", ret);
> > > > >
> > > > > Should not then an error here
> > > >
> > > > Different log level or no message at all?
> > > >
> > > > Note: Linux prints the same message with error level in that case.
> > > >
> > > > Greets
> > > > Alex
> > > >
> > >
> > > Since the rb-gpios is optional, we can continue probing without it.
> > > Throwing an error message is optional and pure informative. So I am fine
> > > with it
> > >
> >
> > Yes ok, but I'm not sure linux give an error if the gpio is get as
> > optional and condition
> > is IS_ERR. Am I right?
>
>
> if (IS_ERR(gpio) && PTR_ERR(gpio) != -ENOENT) {
> dev_err(nc->dev,
> "Failed to get R/B gpio (err = %ld)\n",
> PTR_ERR(gpio));
> return ERR_CAST(gpio);
> }
>
> So Linux throws the message if IS_ERR . If the property is missing (ENOENT)
> it moves on.
>
> Can we replicate the same behavior or this behavior does not suit us in
> U-boot ?
>
> Basically I think it should be :
>
> if (ret && ret != -ENOENT)
> dev_err(...)
> if (!ret)
> rb.type = ATMEL_NAND_GPIO_RB;
>
> Is this what you had in mind Michael ?
The discussion between you guys somehow got stuck. I still have this
patch on my TODO list, but I'm not sure how to proceed. Please advice
in which direction to go forward, there's still an error condition
(described in the initial commit message) here, which needs to be
Re: [PATCH 2/2] mtd: nand: raw: atmel: Add error handling when rb-gpios missing
On 8/23/23 09:54, Michael Nazzareno Trimarchi wrote:
Hi
On Wed, Aug 23, 2023 at 8:28 AM Eugen Hristev
wrote:
Hi,
On 8/8/23 18:03, Alexander Dahl wrote:
Hello Michael,
Am Tue, Aug 08, 2023 at 03:49:45PM +0200 schrieb Michael Nazzareno Trimarchi:
Hi
On Tue, Aug 8, 2023 at 3:03 PM Alexander Dahl wrote:
Adapt behaviour to Linux kernel driver.
The return value of gpio_request_by_name_nodev() was not checked before,
and thus in case 'rb-gpios' was missing in DT, rb.type was set to
ATMEL_NAND_GPIO_RB nevertheless, leading to output like this for
example (on sam9x60-curiosity with the line removed from dts):
NAND: Could not find valid ONFI parameter page; aborting
device found, Manufacturer ID: 0xc2, Chip ID: 0xdc
Macronix NAND 512MiB 3,3V 8-bit
512 MiB, SLC, erase size: 256 KiB, page size: 4096, OOB size: 64
atmel-nand-controller nand-controller: NAND scan failed: -22
Failed to probe nand driver (err = -22)
Failed to initialize NAND controller. (error -22)
0 MiB
Note: not having that gpio assigned in dts is fine, the driver does not
override nand_chip->dev_ready() then and a generic solution is used.
Fixes: 6a8dfd57220d ("nand: atmel: Add DM based NAND driver")
Signed-off-by: Alexander Dahl
---
drivers/mtd/nand/raw/atmel/nand-controller.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c
b/drivers/mtd/nand/raw/atmel/nand-controller.c
index 2b29c8def6..8e745a5111 100644
--- a/drivers/mtd/nand/raw/atmel/nand-controller.c
+++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
@@ -1600,10 +1600,13 @@ static struct atmel_nand *atmel_nand_create(struct
atmel_nand_controller *nc,
nand->cs[i].rb.type = ATMEL_NAND_NATIVE_RB;
nand->cs[i].rb.id = val;
} else {
- gpio_request_by_name_nodev(np, "rb-gpios", 0,
- &nand->cs[i].rb.gpio,
- GPIOD_IS_IN);
- nand->cs[i].rb.type = ATMEL_NAND_GPIO_RB;
+ ret = gpio_request_by_name_nodev(np, "rb-gpios", 0,
+&nand->cs[i].rb.gpio,
+GPIOD_IS_IN);
+ if (ret)
+ dev_err(nc->dev, "Failed to get R/B gpio (err =
%d)\n", ret);
Should not then an error here
Different log level or no message at all?
Note: Linux prints the same message with error level in that case.
Greets
Alex
Since the rb-gpios is optional, we can continue probing without it.
Throwing an error message is optional and pure informative. So I am fine
with it
Yes ok, but I'm not sure linux give an error if the gpio is get as
optional and condition
is IS_ERR. Am I right?
if (IS_ERR(gpio) && PTR_ERR(gpio) != -ENOENT) {
dev_err(nc->dev,
"Failed to get R/B gpio (err = %ld)\n",
PTR_ERR(gpio));
return ERR_CAST(gpio);
}
So Linux throws the message if IS_ERR . If the property is missing
(ENOENT) it moves on.
Can we replicate the same behavior or this behavior does not suit us in
U-boot ?
Basically I think it should be :
if (ret && ret != -ENOENT)
dev_err(...)
if (!ret)
rb.type = ATMEL_NAND_GPIO_RB;
Is this what you had in mind Michael ?
Eugen
For the rest is fine
Michael
What I wanted to ask is what happens with nand->cs[i].rb.type , is it 0
by default ?
Other than that, I can apply this patch, Michael, do you have any more
comments on it ?
Thanks,
Eugen
Michael
+ else
+ nand->cs[i].rb.type = ATMEL_NAND_GPIO_RB;
}
gpio_request_by_name_nodev(np, "cs-gpios", 0,
--
2.30.2
--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__
Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com
Re: [PATCH 2/2] mtd: nand: raw: atmel: Add error handling when rb-gpios missing
Hi
On Wed, Aug 23, 2023 at 8:28 AM Eugen Hristev
wrote:
>
> Hi,
>
> On 8/8/23 18:03, Alexander Dahl wrote:
> > Hello Michael,
> >
> > Am Tue, Aug 08, 2023 at 03:49:45PM +0200 schrieb Michael Nazzareno
> > Trimarchi:
> >> Hi
> >>
> >> On Tue, Aug 8, 2023 at 3:03 PM Alexander Dahl wrote:
> >>>
> >>> Adapt behaviour to Linux kernel driver.
> >>>
> >>> The return value of gpio_request_by_name_nodev() was not checked before,
> >>> and thus in case 'rb-gpios' was missing in DT, rb.type was set to
> >>> ATMEL_NAND_GPIO_RB nevertheless, leading to output like this for
> >>> example (on sam9x60-curiosity with the line removed from dts):
> >>>
> >>> NAND: Could not find valid ONFI parameter page; aborting
> >>> device found, Manufacturer ID: 0xc2, Chip ID: 0xdc
> >>> Macronix NAND 512MiB 3,3V 8-bit
> >>> 512 MiB, SLC, erase size: 256 KiB, page size: 4096, OOB size: 64
> >>> atmel-nand-controller nand-controller: NAND scan failed: -22
> >>> Failed to probe nand driver (err = -22)
> >>> Failed to initialize NAND controller. (error -22)
> >>> 0 MiB
> >>>
> >>> Note: not having that gpio assigned in dts is fine, the driver does not
> >>> override nand_chip->dev_ready() then and a generic solution is used.
> >>>
> >>> Fixes: 6a8dfd57220d ("nand: atmel: Add DM based NAND driver")
> >>> Signed-off-by: Alexander Dahl
> >>> ---
> >>> drivers/mtd/nand/raw/atmel/nand-controller.c | 11 +++
> >>> 1 file changed, 7 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>> b/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>> index 2b29c8def6..8e745a5111 100644
> >>> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> >>> @@ -1600,10 +1600,13 @@ static struct atmel_nand
> >>> *atmel_nand_create(struct atmel_nand_controller *nc,
> >>> nand->cs[i].rb.type = ATMEL_NAND_NATIVE_RB;
> >>> nand->cs[i].rb.id = val;
> >>> } else {
> >>> - gpio_request_by_name_nodev(np, "rb-gpios", 0,
> >>> - &nand->cs[i].rb.gpio,
> >>> - GPIOD_IS_IN);
> >>> - nand->cs[i].rb.type = ATMEL_NAND_GPIO_RB;
> >>> + ret = gpio_request_by_name_nodev(np, "rb-gpios",
> >>> 0,
> >>> +
> >>> &nand->cs[i].rb.gpio,
> >>> +GPIOD_IS_IN);
> >>> + if (ret)
> >>> + dev_err(nc->dev, "Failed to get R/B gpio
> >>> (err = %d)\n", ret);
> >>
> >> Should not then an error here
> >
> > Different log level or no message at all?
> >
> > Note: Linux prints the same message with error level in that case.
> >
> > Greets
> > Alex
> >
>
> Since the rb-gpios is optional, we can continue probing without it.
> Throwing an error message is optional and pure informative. So I am fine
> with it
>
Yes ok, but I'm not sure linux give an error if the gpio is get as
optional and condition
is IS_ERR. Am I right?
For the rest is fine
Michael
> What I wanted to ask is what happens with nand->cs[i].rb.type , is it 0
> by default ?
>
> Other than that, I can apply this patch, Michael, do you have any more
> comments on it ?
>
> Thanks,
> Eugen
> >>
> >> Michael
> >>
> >>> + else
> >>> + nand->cs[i].rb.type = ATMEL_NAND_GPIO_RB;
> >>> }
> >>>
> >>> gpio_request_by_name_nodev(np, "cs-gpios", 0,
> >>> --
> >>> 2.30.2
> >>>
> >>
> >>
> >> --
> >> Michael Nazzareno Trimarchi
> >> Co-Founder & Chief Executive Officer
> >> M. +39 347 913 2170
> >> mich...@amarulasolutions.com
> >> __
> >>
> >> Amarula Solutions BV
> >> Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> >> T. +31 (0)85 111 9172
> >> i...@amarulasolutions.com
> >> www.amarulasolutions.com
>
--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__
Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com
Re: [PATCH 2/2] mtd: nand: raw: atmel: Add error handling when rb-gpios missing
Hi,
On 8/8/23 18:03, Alexander Dahl wrote:
Hello Michael,
Am Tue, Aug 08, 2023 at 03:49:45PM +0200 schrieb Michael Nazzareno Trimarchi:
Hi
On Tue, Aug 8, 2023 at 3:03 PM Alexander Dahl wrote:
Adapt behaviour to Linux kernel driver.
The return value of gpio_request_by_name_nodev() was not checked before,
and thus in case 'rb-gpios' was missing in DT, rb.type was set to
ATMEL_NAND_GPIO_RB nevertheless, leading to output like this for
example (on sam9x60-curiosity with the line removed from dts):
NAND: Could not find valid ONFI parameter page; aborting
device found, Manufacturer ID: 0xc2, Chip ID: 0xdc
Macronix NAND 512MiB 3,3V 8-bit
512 MiB, SLC, erase size: 256 KiB, page size: 4096, OOB size: 64
atmel-nand-controller nand-controller: NAND scan failed: -22
Failed to probe nand driver (err = -22)
Failed to initialize NAND controller. (error -22)
0 MiB
Note: not having that gpio assigned in dts is fine, the driver does not
override nand_chip->dev_ready() then and a generic solution is used.
Fixes: 6a8dfd57220d ("nand: atmel: Add DM based NAND driver")
Signed-off-by: Alexander Dahl
---
drivers/mtd/nand/raw/atmel/nand-controller.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c
b/drivers/mtd/nand/raw/atmel/nand-controller.c
index 2b29c8def6..8e745a5111 100644
--- a/drivers/mtd/nand/raw/atmel/nand-controller.c
+++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
@@ -1600,10 +1600,13 @@ static struct atmel_nand *atmel_nand_create(struct
atmel_nand_controller *nc,
nand->cs[i].rb.type = ATMEL_NAND_NATIVE_RB;
nand->cs[i].rb.id = val;
} else {
- gpio_request_by_name_nodev(np, "rb-gpios", 0,
- &nand->cs[i].rb.gpio,
- GPIOD_IS_IN);
- nand->cs[i].rb.type = ATMEL_NAND_GPIO_RB;
+ ret = gpio_request_by_name_nodev(np, "rb-gpios", 0,
+&nand->cs[i].rb.gpio,
+GPIOD_IS_IN);
+ if (ret)
+ dev_err(nc->dev, "Failed to get R/B gpio (err =
%d)\n", ret);
Should not then an error here
Different log level or no message at all?
Note: Linux prints the same message with error level in that case.
Greets
Alex
Since the rb-gpios is optional, we can continue probing without it.
Throwing an error message is optional and pure informative. So I am fine
with it
What I wanted to ask is what happens with nand->cs[i].rb.type , is it 0
by default ?
Other than that, I can apply this patch, Michael, do you have any more
comments on it ?
Thanks,
Eugen
Michael
+ else
+ nand->cs[i].rb.type = ATMEL_NAND_GPIO_RB;
}
gpio_request_by_name_nodev(np, "cs-gpios", 0,
--
2.30.2
--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__
Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com
Re: [PATCH 2/2] mtd: nand: raw: atmel: Add error handling when rb-gpios missing
Hello Michael,
Am Tue, Aug 08, 2023 at 03:49:45PM +0200 schrieb Michael Nazzareno Trimarchi:
> Hi
>
> On Tue, Aug 8, 2023 at 3:03 PM Alexander Dahl wrote:
> >
> > Adapt behaviour to Linux kernel driver.
> >
> > The return value of gpio_request_by_name_nodev() was not checked before,
> > and thus in case 'rb-gpios' was missing in DT, rb.type was set to
> > ATMEL_NAND_GPIO_RB nevertheless, leading to output like this for
> > example (on sam9x60-curiosity with the line removed from dts):
> >
> > NAND: Could not find valid ONFI parameter page; aborting
> > device found, Manufacturer ID: 0xc2, Chip ID: 0xdc
> > Macronix NAND 512MiB 3,3V 8-bit
> > 512 MiB, SLC, erase size: 256 KiB, page size: 4096, OOB size: 64
> > atmel-nand-controller nand-controller: NAND scan failed: -22
> > Failed to probe nand driver (err = -22)
> > Failed to initialize NAND controller. (error -22)
> > 0 MiB
> >
> > Note: not having that gpio assigned in dts is fine, the driver does not
> > override nand_chip->dev_ready() then and a generic solution is used.
> >
> > Fixes: 6a8dfd57220d ("nand: atmel: Add DM based NAND driver")
> > Signed-off-by: Alexander Dahl
> > ---
> > drivers/mtd/nand/raw/atmel/nand-controller.c | 11 +++
> > 1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c
> > b/drivers/mtd/nand/raw/atmel/nand-controller.c
> > index 2b29c8def6..8e745a5111 100644
> > --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> > +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> > @@ -1600,10 +1600,13 @@ static struct atmel_nand *atmel_nand_create(struct
> > atmel_nand_controller *nc,
> > nand->cs[i].rb.type = ATMEL_NAND_NATIVE_RB;
> > nand->cs[i].rb.id = val;
> > } else {
> > - gpio_request_by_name_nodev(np, "rb-gpios", 0,
> > - &nand->cs[i].rb.gpio,
> > - GPIOD_IS_IN);
> > - nand->cs[i].rb.type = ATMEL_NAND_GPIO_RB;
> > + ret = gpio_request_by_name_nodev(np, "rb-gpios", 0,
> > +
> > &nand->cs[i].rb.gpio,
> > +GPIOD_IS_IN);
> > + if (ret)
> > + dev_err(nc->dev, "Failed to get R/B gpio
> > (err = %d)\n", ret);
>
> Should not then an error here
Different log level or no message at all?
Note: Linux prints the same message with error level in that case.
Greets
Alex
>
> Michael
>
> > + else
> > + nand->cs[i].rb.type = ATMEL_NAND_GPIO_RB;
> > }
> >
> > gpio_request_by_name_nodev(np, "cs-gpios", 0,
> > --
> > 2.30.2
> >
>
>
> --
> Michael Nazzareno Trimarchi
> Co-Founder & Chief Executive Officer
> M. +39 347 913 2170
> mich...@amarulasolutions.com
> __
>
> Amarula Solutions BV
> Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> T. +31 (0)85 111 9172
> i...@amarulasolutions.com
> www.amarulasolutions.com
Re: [PATCH 2/2] mtd: nand: raw: atmel: Add error handling when rb-gpios missing
Hi
On Tue, Aug 8, 2023 at 3:03 PM Alexander Dahl wrote:
>
> Adapt behaviour to Linux kernel driver.
>
> The return value of gpio_request_by_name_nodev() was not checked before,
> and thus in case 'rb-gpios' was missing in DT, rb.type was set to
> ATMEL_NAND_GPIO_RB nevertheless, leading to output like this for
> example (on sam9x60-curiosity with the line removed from dts):
>
> NAND: Could not find valid ONFI parameter page; aborting
> device found, Manufacturer ID: 0xc2, Chip ID: 0xdc
> Macronix NAND 512MiB 3,3V 8-bit
> 512 MiB, SLC, erase size: 256 KiB, page size: 4096, OOB size: 64
> atmel-nand-controller nand-controller: NAND scan failed: -22
> Failed to probe nand driver (err = -22)
> Failed to initialize NAND controller. (error -22)
> 0 MiB
>
> Note: not having that gpio assigned in dts is fine, the driver does not
> override nand_chip->dev_ready() then and a generic solution is used.
>
> Fixes: 6a8dfd57220d ("nand: atmel: Add DM based NAND driver")
> Signed-off-by: Alexander Dahl
> ---
> drivers/mtd/nand/raw/atmel/nand-controller.c | 11 +++
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c
> b/drivers/mtd/nand/raw/atmel/nand-controller.c
> index 2b29c8def6..8e745a5111 100644
> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> @@ -1600,10 +1600,13 @@ static struct atmel_nand *atmel_nand_create(struct
> atmel_nand_controller *nc,
> nand->cs[i].rb.type = ATMEL_NAND_NATIVE_RB;
> nand->cs[i].rb.id = val;
> } else {
> - gpio_request_by_name_nodev(np, "rb-gpios", 0,
> - &nand->cs[i].rb.gpio,
> - GPIOD_IS_IN);
> - nand->cs[i].rb.type = ATMEL_NAND_GPIO_RB;
> + ret = gpio_request_by_name_nodev(np, "rb-gpios", 0,
> +&nand->cs[i].rb.gpio,
> +GPIOD_IS_IN);
> + if (ret)
> + dev_err(nc->dev, "Failed to get R/B gpio (err
> = %d)\n", ret);
Should not then an error here
Michael
> + else
> + nand->cs[i].rb.type = ATMEL_NAND_GPIO_RB;
> }
>
> gpio_request_by_name_nodev(np, "cs-gpios", 0,
> --
> 2.30.2
>
--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__
Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com
[PATCH 2/2] mtd: nand: raw: atmel: Add error handling when rb-gpios missing
Adapt behaviour to Linux kernel driver.
The return value of gpio_request_by_name_nodev() was not checked before,
and thus in case 'rb-gpios' was missing in DT, rb.type was set to
ATMEL_NAND_GPIO_RB nevertheless, leading to output like this for
example (on sam9x60-curiosity with the line removed from dts):
NAND: Could not find valid ONFI parameter page; aborting
device found, Manufacturer ID: 0xc2, Chip ID: 0xdc
Macronix NAND 512MiB 3,3V 8-bit
512 MiB, SLC, erase size: 256 KiB, page size: 4096, OOB size: 64
atmel-nand-controller nand-controller: NAND scan failed: -22
Failed to probe nand driver (err = -22)
Failed to initialize NAND controller. (error -22)
0 MiB
Note: not having that gpio assigned in dts is fine, the driver does not
override nand_chip->dev_ready() then and a generic solution is used.
Fixes: 6a8dfd57220d ("nand: atmel: Add DM based NAND driver")
Signed-off-by: Alexander Dahl
---
drivers/mtd/nand/raw/atmel/nand-controller.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c
b/drivers/mtd/nand/raw/atmel/nand-controller.c
index 2b29c8def6..8e745a5111 100644
--- a/drivers/mtd/nand/raw/atmel/nand-controller.c
+++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
@@ -1600,10 +1600,13 @@ static struct atmel_nand *atmel_nand_create(struct
atmel_nand_controller *nc,
nand->cs[i].rb.type = ATMEL_NAND_NATIVE_RB;
nand->cs[i].rb.id = val;
} else {
- gpio_request_by_name_nodev(np, "rb-gpios", 0,
- &nand->cs[i].rb.gpio,
- GPIOD_IS_IN);
- nand->cs[i].rb.type = ATMEL_NAND_GPIO_RB;
+ ret = gpio_request_by_name_nodev(np, "rb-gpios", 0,
+&nand->cs[i].rb.gpio,
+GPIOD_IS_IN);
+ if (ret)
+ dev_err(nc->dev, "Failed to get R/B gpio (err =
%d)\n", ret);
+ else
+ nand->cs[i].rb.type = ATMEL_NAND_GPIO_RB;
}
gpio_request_by_name_nodev(np, "cs-gpios", 0,
--
2.30.2
