[PATCH 02/31] mtd: spi-nor: Tidy up error handling / debug code

[Simon Glass]

Hi Vignesh,

On Mon, 20 Jul 2020 at 00:26, Vignesh Raghavendra <vigneshr at ti.com> wrote:
>
> Hi Simon,
>
> On 19/07/20 9:45 pm, Simon Glass wrote:
> > The -ENODEV error value in spi_nor_read_id() is incorrect since there
> > clearly is a device - it just cannot be supported.
>
> Description 's not entirely accurate... If there is no flash on the SPI
> bus, then read ID would return all 0s or 0xFFs depending on the pullups
> on the board which would fail to match any flash in the spi-nor-ids table.
>
> So its not just the case of device IDs not recognized or supported by
> U-Boot, it maybe that flash is absent altogether.

But struct udevice * exists, so there is a device. Whether it is
connected to something is a separate issue.

The problem is that -ENODEV means that the device should not be
created or should not exist.

What happens if an invalid ID is returned? Does it unbind the device?

Regards,
Simon


>
> Regards
> Vignesh
>
> > Use -ENOMEDIUM instead
> > which has the virtue of being less common.
> >
> > Fix the return value in spi_nor_scan().
> >
> > Also there are a few printf() statements which should be debug() since
> > they bloat the code with unused strings at present. Fix those while here.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> >  drivers/mtd/spi/sf_probe.c     | 2 +-
> >  drivers/mtd/spi/spi-nor-core.c | 2 +-
> >  drivers/mtd/spi/spi-nor-tiny.c | 4 ++--
> >  3 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
> > index 475f6c31db..b959e3453a 100644
> > --- a/drivers/mtd/spi/sf_probe.c
> > +++ b/drivers/mtd/spi/sf_probe.c
> > @@ -119,7 +119,7 @@ static int spi_flash_std_erase(struct udevice *dev, u32 offset, size_t len)
> >       struct erase_info instr;
> >
> >       if (offset % mtd->erasesize || len % mtd->erasesize) {
> > -             printf("SF: Erase offset/length not multiple of erase size\n");
> > +             debug("SF: Erase offset/length not multiple of erase size\n");
> >               return -EINVAL;
> >       }
> >
> > diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> > index fdcd830ce4..0113e70037 100644
> > --- a/drivers/mtd/spi/spi-nor-core.c
> > +++ b/drivers/mtd/spi/spi-nor-core.c
> > @@ -2470,7 +2470,7 @@ static int spi_nor_init(struct spi_nor *nor)
> >                * designer) that this is bad.
> >                */
> >               if (nor->flags & SNOR_F_BROKEN_RESET)
> > -                     printf("enabling reset hack; may not recover from unexpected reboots\n");
> > +                     debug("enabling reset hack; may not recover from unexpected reboots\n");
> >               set_4byte(nor, nor->info, 1);
> >       }
> >
> > diff --git a/drivers/mtd/spi/spi-nor-tiny.c b/drivers/mtd/spi/spi-nor-tiny.c
> > index 9f676c649d..fa26ea33c8 100644
> > --- a/drivers/mtd/spi/spi-nor-tiny.c
> > +++ b/drivers/mtd/spi/spi-nor-tiny.c
> > @@ -377,7 +377,7 @@ static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
> >       }
> >       dev_dbg(nor->dev, "unrecognized JEDEC id bytes: %02x, %02x, %02x\n",
> >               id[0], id[1], id[2]);
> > -     return ERR_PTR(-ENODEV);
> > +     return ERR_PTR(-EMEDIUMTYPE);
> >  }
> >
> >  static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
> > @@ -733,7 +733,7 @@ int spi_nor_scan(struct spi_nor *nor)
> >
> >       info = spi_nor_read_id(nor);
> >       if (IS_ERR_OR_NULL(info))
> > -             return -ENOENT;
> > +             return PTR_ERR(info);
> >       /* Parse the Serial Flash Discoverable Parameters table. */
> >       ret = spi_nor_init_params(nor, info, &params);
> >       if (ret)
> >