From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Sun, 26 Jul 2020 08:54:31 -0600 Subject: [PATCH 02/31] mtd: spi-nor: Tidy up error handling / debug code In-Reply-To: <462d9b71-ad6b-b597-87e6-e4c473b18b85@ti.com> References: <20200719161601.495421-1-sjg@chromium.org> <20200719161601.495421-3-sjg@chromium.org> <462d9b71-ad6b-b597-87e6-e4c473b18b85@ti.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Vignesh, On Mon, 20 Jul 2020 at 00:26, Vignesh Raghavendra 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 > > --- > > > > 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, ¶ms); > > if (ret) > >