[PATCH 2/2] mtd: nand: raw: atmel: Add error handling when rb-gpios missing

[Michael Nazzareno Trimarchi]

Hi

On Wed, Aug 23, 2023 at 8:28 AM Eugen Hristev
<eugen.hristev at collabora.com> 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 <ada at thorsis.com> 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 <ada at thorsis.com>
> >>> ---
> >>>   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
> >> michael at amarulasolutions.com
> >> __________________________________
> >>
> >> Amarula Solutions BV
> >> Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> >> T. +31 (0)85 111 9172
> >> info at amarulasolutions.com
> >> www.amarulasolutions.com
>


-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael at amarulasolutions.com
__________________________________

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info at amarulasolutions.com
www.amarulasolutions.com