[PATCH] [FS] Print error message for unknown device type

File system commands like "ls" etc. require a device type parameter. If an unknown type is specified, they return an error code but no visible feedback to the user:
-> ls FOOBAR 1:1 / ->
Add an error message to make clear what happens, and why.
Signed-off-by: Wolfgang Denk wd@denx.de --- disk/part.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/disk/part.c b/disk/part.c index 8982ef3bae..14000835c8 100644 --- a/disk/part.c +++ b/disk/part.c @@ -512,8 +512,10 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str,
/* Look up the device */ dev = blk_get_device_by_str(ifname, dev_str, dev_desc); - if (dev < 0) + if (dev < 0) { + printf("** Unknown device type %s **\n", ifname); goto cleanup; + }
/* Convert partition ID string to number */ if (!part_str || !*part_str) {

Hello Wolfgang,
Am 22.01.2020 um 12:08 schrieb Wolfgang Denk:
File system commands like "ls" etc. require a device type parameter. If an unknown type is specified, they return an error code but no visible feedback to the user:
-> ls FOOBAR 1:1 / ->
Add an error message to make clear what happens, and why.
Signed-off-by: Wolfgang Denk wd@denx.de
disk/part.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
Tested on wandboard.
Tested-by: Heiko Schocher hs@denx.de
diff --git a/disk/part.c b/disk/part.c index 8982ef3bae..14000835c8 100644 --- a/disk/part.c +++ b/disk/part.c @@ -512,8 +512,10 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str,
/* Look up the device */ dev = blk_get_device_by_str(ifname, dev_str, dev_desc);
- if (dev < 0)
- if (dev < 0) {
goto cleanup;printf("** Unknown device type %s **\n", ifname);
- }
It would be nice to have here a list of supported devices, so a user can see what are valid arguments for ifname.
/* Convert partition ID string to number */ if (!part_str || !*part_str) {
bye, Heiko

Dear Heiko,
In message 3546d28c-f638-5357-a20f-5d03db762be0@denx.de you wrote:
File system commands like "ls" etc. require a device type parameter. If an unknown type is specified, they return an error code but no visible feedback to the user:
-> ls FOOBAR 1:1 / ->
Add an error message to make clear what happens, and why.
Signed-off-by: Wolfgang Denk wd@denx.de
disk/part.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
Tested on wandboard.
Tested-by: Heiko Schocher hs@denx.de
Thanks for testing.
- if (dev < 0)
- if (dev < 0) {
goto cleanup;printf("** Unknown device type %s **\n", ifname);
- }
It would be nice to have here a list of supported devices, so a user can see what are valid arguments for ifname.
Yes, you are absolutely right. I aready thought about this, but I have to admit that I got stuck in the code; there are several complexities - code for example for blk_driver_lookup_typename() is duplicated both in drivers/block/blk_legacy.c and in drivers/block/blk-uclass.c; I was not able to find any exported interface that actually allows to get a list of supported device drivers, and the things I tried all looked really ugly to me.
Adding Simon to Cc: - he has designed and written all this code and should know better.
Simon, what would be a clean and elegant approach to get such a list of supported drivers ?
In any case I recommend to accept this patch as is; this other thing is additional information that can /should get added later in a spearate patch.
Best regards,
Wolfgang Denk

Hello Simon,
ping...
In message 20200122121253.7B0B3240638@gemini.denx.de I wrote:
Dear Heiko,
In message 3546d28c-f638-5357-a20f-5d03db762be0@denx.de you wrote:
File system commands like "ls" etc. require a device type parameter. If an unknown type is specified, they return an error code but no visible feedback to the user:
-> ls FOOBAR 1:1 / ->
Add an error message to make clear what happens, and why.
Signed-off-by: Wolfgang Denk wd@denx.de
disk/part.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
Tested on wandboard.
Tested-by: Heiko Schocher hs@denx.de
Thanks for testing.
- if (dev < 0)
- if (dev < 0) {
goto cleanup;printf("** Unknown device type %s **\n", ifname);
- }
It would be nice to have here a list of supported devices, so a user can see what are valid arguments for ifname.
Yes, you are absolutely right. I aready thought about this, but I have to admit that I got stuck in the code; there are several complexities - code for example for blk_driver_lookup_typename() is duplicated both in drivers/block/blk_legacy.c and in drivers/block/blk-uclass.c; I was not able to find any exported interface that actually allows to get a list of supported device drivers, and the things I tried all looked really ugly to me.
Adding Simon to Cc: - he has designed and written all this code and should know better.
Simon, what would be a clean and elegant approach to get such a list of supported drivers ?
In any case I recommend to accept this patch as is; this other thing is additional information that can /should get added later in a spearate patch.
Best regards,
Wolfgang Denk
Best regards,
Wolfgang Denk

Hi,
On Fri, 24 Jan 2020 at 06:21, Wolfgang Denk wd@denx.de wrote:
Hello Simon,
ping...
In message 20200122121253.7B0B3240638@gemini.denx.de I wrote:
Dear Heiko,
In message 3546d28c-f638-5357-a20f-5d03db762be0@denx.de you wrote:
File system commands like "ls" etc. require a device type parameter. If an unknown type is specified, they return an error code but no visible feedback to the user:
-> ls FOOBAR 1:1 / ->
Add an error message to make clear what happens, and why.
Signed-off-by: Wolfgang Denk wd@denx.de
disk/part.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
Tested on wandboard.
Tested-by: Heiko Schocher hs@denx.de
Thanks for testing.
- if (dev < 0)
- if (dev < 0) {
printf("** Unknown device type %s **\n", ifname); goto cleanup;
- }
Reviewed-by: Simon Glass sjg@chromium.org
It would be nice to have here a list of supported devices, so a user can see what are valid arguments for ifname.
There are two functions involved here, since we still support non-DM block devices. The function called is blk_get_devnum_by_typename(), with two implementations.
So we would need two different implementations to read the list of devices:
- blk_get_devnum_by_typename() has a uclass_foreach_dev() loop - blk_driver_lookup_typename() shows how to iterate through the legacy block devices
Yes, you are absolutely right. I aready thought about this, but I have to admit that I got stuck in the code; there are several complexities - code for example for blk_driver_lookup_typename() is duplicated both in drivers/block/blk_legacy.c and in drivers/block/blk-uclass.c; I was not able to find any exported interface that actually allows to get a list of supported device drivers, and the things I tried all looked really ugly to me.
Adding Simon to Cc: - he has designed and written all this code and should know better.
Simon, what would be a clean and elegant approach to get such a list of supported drivers ?
In any case I recommend to accept this patch as is; this other thing is additional information that can /should get added later in a spearate patch.
Regards, Simon

On Wed, 22 Jan 2020 at 14:09, Wolfgang Denk wd@denx.de wrote:
File system commands like "ls" etc. require a device type parameter. If an unknown type is specified, they return an error code but no visible feedback to the user:
-> ls FOOBAR 1:1 / ->
Add an error message to make clear what happens, and why.
Signed-off-by: Wolfgang Denk wd@denx.de
disk/part.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/disk/part.c b/disk/part.c index 8982ef3bae..14000835c8 100644 --- a/disk/part.c +++ b/disk/part.c @@ -512,8 +512,10 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str,
/* Look up the device */ dev = blk_get_device_by_str(ifname, dev_str, dev_desc);
if (dev < 0)
if (dev < 0) {
printf("** Unknown device type %s **\n", ifname); goto cleanup;
} /* Convert partition ID string to number */ if (!part_str || !*part_str) {
-- 2.23.0
Oww, I've submitted the similar patch to this mailing list recently but it is waiting for moderator approval :)
participants (4)
-
Heiko Schocher
-
Nikita Ermakov
-
Simon Glass
-
Wolfgang Denk