From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vignesh Raghavendra Date: Thu, 30 Jul 2020 14:49:55 +0530 Subject: [PATCH 02/31] mtd: spi-nor: Tidy up error handling / debug code In-Reply-To: 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, On 26/07/20 8:24 pm, Simon Glass wrote: > 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? There are two cases: 1. spi flash node is present in the DT but there is no device on the board. In this case device is not unbound from DT node. 2. There is no flash device described as child of SPI bus but user tries sf probe (Or some U-boot code calls spi_flash_probe_bus_cs()). In this case a device is created on the fly and bound to sf driver and probe is attempted (as part of which attempt is make to read Device ID). If there is no flash found, then the device is unbound and removed (see drivers/mtd/spi/sf-uclass.c::spi_flash_probe_bus_cs and drivers/spi/spi-uclass.c::spi_get_bus_and_cs). So at least in case 2 its U-boot which is creating device and try to look for it which would fail if not present. IMO, -ENODEV makes sense here since the intention is to dynamically find the device w/o an actual DT node present. [...] Regards Vignesh