[U-Boot] [PATCH v3 0/2] While converting SATA on Apalis iMX6 to use driver model I noticed it

crashing in case no SATA driver is actually there:
Apalis iMX6 # sata init data abort pc : [<8ff78e1a>] lr : [<8ff78e1b>] reloc pc : [<1781be1a>] lr : [<1781be1b>] sp : 8df53068 ip : 00000001 fp : 00000002 r10: 8df612e8 r9 : 8df5ceb0 r8 : 8ffc2b28 r7 : 8ff746ed r6 : 00000000 r5 : 68abf82a r4 : 00000000 r3 : 8ff73d39 r2 : c0000000 r1 : 00000000 r0 : 0000000c Flags: nzcv IRQs off FIQs off Mode SVC_32 Code: 47700025 6803b570 6bdd4604 f0254805 (68abf82a) Resetting CPU ...
resetting ...
Investigating this I identified 3 places where no proper checking is done.
This patch set adds such checks leading to a graceful error instead:
Apalis iMX6 # sata init Cannot probe SATA device 0 (err=-19)
Changes in v3: - Added Simon's reviewed-by.
Changes in v2: - Update dm_test_uclass_devices_find() to test this behaviour as suggested by Simon. - Dropped "[PATCH 2/3] dm: sata: add null pointer check for dev" as suggested by Simon. - Added Simon's reviewed-by.
Marcel Ziswiler (2): dm: device: fail uclass_find_first_device() if list_empty cmd: sata: add null pointer check for dev
cmd/sata.c | 4 ++++ drivers/core/uclass.c | 2 +- test/dm/core.c | 4 ++++ 3 files changed, 9 insertions(+), 1 deletion(-)

From: Marcel Ziswiler marcel.ziswiler@toradex.com
While uclass_find_device() fails with -ENODEV in case of list_empty strangely uclass_find_first_device() returns 0.
Fix uclass_find_first_device() to also fail with -ENODEV instead.
Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com Reviewed-by: Simon Glass sjg@chromium.org
---
Changes in v3: - Added Simon's reviewed-by.
Changes in v2: - Update dm_test_uclass_devices_find() to test this behaviour as suggested by Simon.
drivers/core/uclass.c | 2 +- test/dm/core.c | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c index a622f07941..fc3157de39 100644 --- a/drivers/core/uclass.c +++ b/drivers/core/uclass.c @@ -225,7 +225,7 @@ int uclass_find_first_device(enum uclass_id id, struct udevice **devp) if (ret) return ret; if (list_empty(&uc->dev_head)) - return 0; + return -ENODEV;
*devp = list_first_entry(&uc->dev_head, struct udevice, uclass_node);
diff --git a/test/dm/core.c b/test/dm/core.c index 260f6494a2..edd55b05d6 100644 --- a/test/dm/core.c +++ b/test/dm/core.c @@ -749,6 +749,10 @@ static int dm_test_uclass_devices_find(struct unit_test_state *uts) ut_assert(dev); }
+ ret = uclass_find_first_device(UCLASS_TEST_DUMMY, &dev); + ut_assert(ret == -ENODEV); + ut_assert(!dev); + return 0; } DM_TEST(dm_test_uclass_devices_find, DM_TESTF_SCAN_PDATA);

On Fri, Feb 01, 2019 at 04:01:07PM +0100, Marcel Ziswiler wrote:
From: Marcel Ziswiler marcel.ziswiler@toradex.com
While uclass_find_device() fails with -ENODEV in case of list_empty strangely uclass_find_first_device() returns 0.
Fix uclass_find_first_device() to also fail with -ENODEV instead.
Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

From: Marcel Ziswiler marcel.ziswiler@toradex.com
Calling sata_scan() with a null pointer probably won't make much sense.
Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com Reviewed-by: Simon Glass sjg@chromium.org
---
Changes in v3: None Changes in v2: - Dropped "[PATCH 2/3] dm: sata: add null pointer check for dev" as suggested by Simon. - Added Simon's reviewed-by.
cmd/sata.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/cmd/sata.c b/cmd/sata.c index 6d62ba8f74..a73cc54bd3 100644 --- a/cmd/sata.c +++ b/cmd/sata.c @@ -60,6 +60,10 @@ int sata_probe(int devnum) printf("Cannot probe SATA device %d (err=%d)\n", devnum, rc); return CMD_RET_FAILURE; } + if (!dev) { + printf("No SATA device found!\n"); + return CMD_RET_FAILURE; + } rc = sata_scan(dev); if (rc) { printf("Cannot scan SATA device %d (err=%d)\n", devnum, rc);

On Fri, Feb 01, 2019 at 04:01:08PM +0100, Marcel Ziswiler wrote:
From: Marcel Ziswiler marcel.ziswiler@toradex.com
Calling sata_scan() with a null pointer probably won't make much sense.
Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
participants (2)
-
Marcel Ziswiler
-
Tom Rini