[U-Boot] [PATCH] cmd_ide: Fix detection problem with missing devices
Currently only IDE busses are probed and all possible available devices are listed in the IDE bootup log. Even when devices on the bus are not available. This leads to the following output on the CPCI750:
IDE: Bus 0: OK Bus 1: OK Device 0: Model: HITACHI_DK23FA-20J Firm: 00M7A0A0 Ser#: 42D182 Type: Hard Disk Capacity: 19077.1 MB = 18.6 GB (39070080 x 512) Device 1: Model: Firm: Ser#: Type: # 1F # Capacity: not available Device 2: Model: SanDisk SDCFB-128 Firm: vde 1.10 Ser#: gmo5i311404 Type: Removable Hard Disk Capacity: 122.5 MB = 0.1 GB (250880 x 512) Device 3: Model: Firm: Ser#: Type: # 1F # Capacity: not available
Here devices 1 + 3 are listed which are not availble.
This patch now fixes this problem by skipping the device output for non-available devices. This is the resulting log:
IDE: Bus 0: OK Bus 1: OK Device 0: Model: HITACHI_DK23FA-20J Firm: 00M7A0A0 Ser#: 42D182 Type: Hard Disk Capacity: 19077.1 MB = 18.6 GB (39070080 x 512) Device 1: not available Device 2: Model: SanDisk SDCFB-128 Firm: vde 1.10 Ser#: gmo5i311404 Type: Removable Hard Disk Capacity: 122.5 MB = 0.1 GB (250880 x 512) Device 3: not available
Signed-off-by: Stefan Roese sr@denx.de --- common/cmd_ide.c | 21 ++++++++++++++------- 1 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/common/cmd_ide.c b/common/cmd_ide.c index 71db933..7f9217d 100644 --- a/common/cmd_ide.c +++ b/common/cmd_ide.c @@ -154,7 +154,7 @@ static void ide_reset(void); #define ide_reset() /* dummy */ #endif
-static void ide_ident(block_dev_desc_t *dev_desc); +static int ide_ident(block_dev_desc_t *dev_desc); static uchar ide_wait(int dev, ulong t);
#define IDE_TIME_OUT 2000 /* 2 sec timeout */ @@ -714,6 +714,7 @@ skip_bus:
curr_device = -1; for (i = 0; i < CONFIG_SYS_IDE_MAXDEVICE; ++i) { + int ret; #ifdef CONFIG_IDE_LED int led = (IDE_BUS(i) == 0) ? LED_IDE1 : LED_IDE2; #endif @@ -727,8 +728,12 @@ skip_bus: if (!ide_bus_ok[IDE_BUS(i)]) continue; ide_led(led, 1); /* LED on */ - ide_ident(&ide_dev_desc[i]); + ret = ide_ident(&ide_dev_desc[i]); ide_led(led, 0); /* LED off */ + if (ret < 0) { + puts("not available\n"); + continue; + } dev_print(&ide_dev_desc[i]); if ((ide_dev_desc[i].lba > 0) && (ide_dev_desc[i].blksz > 0)) { init_part(&ide_dev_desc[i]); /* initialize partition type */ @@ -987,7 +992,7 @@ static void input_data(int dev, ulong *sect_buf, int words)
/* ------------------------------------------------------------------------- */ -static void ide_ident(block_dev_desc_t *dev_desc) +static int ide_ident(block_dev_desc_t *dev_desc) { ulong iobuf[ATA_SECTORWORDS]; unsigned char c; @@ -1018,7 +1023,7 @@ static void ide_ident(block_dev_desc_t *dev_desc) max_bus_scan = CONFIG_SYS_IDE_MAXBUS; if (device >= max_bus_scan * 2) { dev_desc->type = DEV_TYPE_UNKNOWN; - return; + return -1; } #endif
@@ -1087,7 +1092,7 @@ static void ide_ident(block_dev_desc_t *dev_desc) ATA_LBA | ATA_DEVICE(device)); retries++; #else - return; + return -1; #endif } #ifdef CONFIG_ATAPI @@ -1096,7 +1101,7 @@ static void ide_ident(block_dev_desc_t *dev_desc) } /* see above - ugly to read */
if (retries == 2) /* Not found */ - return; + return -1; #endif
input_swap_data(device, iobuf, ATA_SECTORWORDS); @@ -1161,7 +1166,7 @@ static void ide_ident(block_dev_desc_t *dev_desc) #ifdef CONFIG_ATAPI if (dev_desc->if_type == IF_TYPE_ATAPI) { atapi_inquiry(dev_desc); - return; + return 0; } #endif /* CONFIG_ATAPI */
@@ -1193,6 +1198,8 @@ static void ide_ident(block_dev_desc_t *dev_desc) dev_desc->type = DEV_TYPE_HARDDISK; dev_desc->blksz = ATA_BLOCKSIZE; dev_desc->lun = 0; /* just to fill something in... */ + + return 0; }
/* ------------------------------------------------------------------------- */
Dear Stefan Roese,
In message 1242278687-23685-1-git-send-email-sr@denx.de you wrote:
Currently only IDE busses are probed and all possible available devices are listed in the IDE bootup log. Even when devices on the bus are not available. This leads to the following output on the CPCI750:
IDE: Bus 0: OK Bus 1: OK Device 0: Model: HITACHI_DK23FA-20J Firm: 00M7A0A0 Ser#: 42D182 Type: Hard Disk Capacity: 19077.1 MB = 18.6 GB (39070080 x 512) Device 1: Model: Firm: Ser#: Type: # 1F # Capacity: not available Device 2: Model: SanDisk SDCFB-128 Firm: vde 1.10 Ser#: gmo5i311404 Type: Removable Hard Disk Capacity: 122.5 MB = 0.1 GB (250880 x 512) Device 3: Model: Firm: Ser#: Type: # 1F # Capacity: not available
If my understanding is correct, then this is a bug on your hardware.
#endif @@ -727,8 +728,12 @@ skip_bus: if (!ide_bus_ok[IDE_BUS(i)]) continue; ide_led(led, 1); /* LED on */
ide_ident(&ide_dev_desc[i]);
ret = ide_ident(&ide_dev_desc[i]);if (ret < 0) {puts("not available\n");continue;}
IIRC this should not be needed; normally, ide_bus_ok() should catch this.
Best regards,
Wolfgang Denk
Hi Wolfgang,
On Thursday 14 May 2009 15:44:01 Wolfgang Denk wrote:
Currently only IDE busses are probed and all possible available devices are listed in the IDE bootup log. Even when devices on the bus are not available. This leads to the following output on the CPCI750:
IDE: Bus 0: OK Bus 1: OK Device 0: Model: HITACHI_DK23FA-20J Firm: 00M7A0A0 Ser#: 42D182 Type: Hard Disk Capacity: 19077.1 MB = 18.6 GB (39070080 x 512) Device 1: Model: Firm: Ser#: Type: # 1F # Capacity: not available Device 2: Model: SanDisk SDCFB-128 Firm: vde 1.10 Ser#: gmo5i311404 Type: Removable Hard Disk Capacity: 122.5 MB = 0.1 GB (250880 x 512) Device 3: Model: Firm: Ser#: Type: # 1F # Capacity: not available
If my understanding is correct, then this is a bug on your hardware.
I don't think so.
#endif @@ -727,8 +728,12 @@ skip_bus: if (!ide_bus_ok[IDE_BUS(i)]) continue; ide_led(led, 1); /* LED on */
ide_ident(&ide_dev_desc[i]);
ret = ide_ident(&ide_dev_desc[i]);if (ret < 0) {puts("not available\n");continue;}IIRC this should not be needed; normally, ide_bus_ok() should catch this.
No. ide_bus_ok() only checks the first drive/device on a bus. If this device is ok, then all drives from this bus are printed.
Best regards, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================
Dear Stefan Roese,
In message 200905141555.33901.sr@denx.de you wrote:
IDE: Bus 0: OK Bus 1: OK Device 0: Model: HITACHI_DK23FA-20J Firm: 00M7A0A0 Ser#: 42D182 Type: Hard Disk Capacity: 19077.1 MB = 18.6 GB (39070080 x 512) Device 1: Model: Firm: Ser#: Type: # 1F # Capacity: not available Device 2: Model: SanDisk SDCFB-128 Firm: vde 1.10 Ser#: gmo5i311404 Type: Removable Hard Disk Capacity: 122.5 MB = 0.1 GB (250880 x 512) Device 3: Model: Firm: Ser#: Type: # 1F # Capacity: not available
If my understanding is correct, then this is a bug on your hardware.
I don't think so.
I do, because what we see on other hardware looks different:
For example:
U-Boot 1.3.2-rc2-g02e4b4e3 (Feb 28 2008 - 16:46:26) ... IDE: Bus 0: ..OK Device 0: Model: HITACHI_DK23DA-20 Firm: 00J2A0A1 Ser#: 12Y0MN Type: Hard Disk Capacity: 19077.1 MB = 18.6 GB (39070080 x 512) Device 1: not available SRAM: 512 kB ...
If your board probes for a non-existing Device 1 and tries to print Model and Firmware versions, then there must be something wrong...
Best regards,
Wolfgang Denk
On Thursday 14 May 2009 16:59:51 Wolfgang Denk wrote:
If my understanding is correct, then this is a bug on your hardware.
I don't think so.
I do, because what we see on other hardware looks different:
For example:
U-Boot 1.3.2-rc2-g02e4b4e3 (Feb 28 2008 - 16:46:26)
^^^^^^^^^^^^^
That's very old.
... IDE: Bus 0: ..OK Device 0: Model: HITACHI_DK23DA-20 Firm: 00J2A0A1 Ser#: 12Y0MN Type: Hard Disk Capacity: 19077.1 MB = 18.6 GB (39070080 x 512) Device 1: not available SRAM: 512 kB ...
If your board probes for a non-existing Device 1 and tries to print Model and Firmware versions, then there must be something wrong...
From my understanding this worked at some point on the CPCI750 as well. Perhaps you could give the TOT U-Boot version a try on your board used above. My guess is that this will fail as well.
I tried to locate a problem that was introduced at some time but couldn't locate it (no, I didn't use git bisect). But looking at the current code this just can't work as is for the 2nd drive. And I still think my fix is ok.
Thanks.
Best regards, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================
Dear Stefan Roese,
In message 200905141714.34154.sr@denx.de you wrote:
U-Boot 1.3.2-rc2-g02e4b4e3 (Feb 28 2008 - 16:46:26)
^^^^^^^^^^^^^ That's very old.
Indeed. That's why I included this information as data point.
From my understanding this worked at some point on the CPCI750 as well.
Ah! You didn't mention this before. That's bad.
Perhaps you could give the TOT U-Boot version a try on your board used above. My guess is that this will fail as well.
Correct.
I tried to locate a problem that was introduced at some time but couldn't locate it (no, I didn't use git bisect). But looking at the current code this
Well, I did bisect it, and found that it's an "interesting" issue, as inbettween there obviously were some versions where it didn't work at all; i. e.:
old:
IDE: Bus 0: ..OK Device 0: Model: HITACHI_DK23DA-20 Firm: 00J2A0A1 Ser#: 12Y0MN Type: Hard Disk Capacity: 19077.1 MB = 18.6 GB (39070080 x 512) Device 1: not available
then:
IDE: Bus 0: OK Device 0: Model: HITACHI_DK23DA-20 Firm: 00J2A0A1 Ser#: 12Y0MN Vendor: HITACHI_DK23DA-20 Prod.: 12Y0MN Rev: 00J2A0A1 Type: Hard Disk Capacity: 19077.1 MB = 18.6 GB (39070080 x 512) Device 1: not available
then:
IDE: Bus 0: OK Device 0: not available Device 1: not available
This was caused by:
574b319512b13e10800f0045e39b993f4ca25e42 is first bad commit commit 574b319512b13e10800f0045e39b993f4ca25e42 Author: Detlev Zundel dzu@denx.de Date: Mon May 5 16:11:21 2008 +0200
Fix disk type output in disk/part.c Signed-off-by: Detlev Zundel dzu@denx.de
The bug is here:
- if (dev_desc->type==DEV_TYPE_UNKNOWN) { - puts ("not available\n"); - return; - } - if (dev_desc->if_type==IF_TYPE_SCSI) { - printf ("(%d:%d) ", dev_desc->target,dev_desc->lun); - } - if (dev_desc->if_type==IF_TYPE_IDE) { - printf ("Model: %s Firm: %s Ser#: %s\n", + switch (dev_desc->type) { + case IF_TYPE_SCSI: ... + case DEV_TYPE_UNKNOWN: + default: + puts ("not available\n"); + return;
The initial test was on "dev_desc->type", but this was "optimized" into the switch - not noticing that the switch tests for "dev_desc->if_type" which can only take "IF_*" values, but no "DEV_*" values.
Later, the switch() was fixed into "switch (dev_desc->if_type)", but then the DEV_TYPE_UNKNOWN was converted into IF_TYPE_UNKNOWN which might have seemed logical at that time, but by that we lost all trace of the original test; see
commit 8ec6e332eace0ee78c71ee5f645d12b06813b86f Author: Tor Krill tor@excito.com Date: Thu May 29 11:10:30 2008 +0200
Fix incorrect switch for IF_TYPE in part.c
and finally:
IDE: Bus 0: OK Device 0: Model: HITACHI_DK23DA-20 Firm: 00J2A0A1 Ser#: 12Y0MN Type: Hard Disk Capacity: 19077.1 MB = 18.6 GB (39070080 x 512) Device 1: Model: Firm: Ser#: Type: # 1F # Capacity: not available
Hey, that's fun, isn't it :-(
just can't work as is for the 2nd drive. And I still think my fix is ok.
I don't think so. Could you please try the patch in the following mail? Thanks in advance.
Best regards,
Wolfgang Denk
Commit 574b319512 introduced a subtle bug by mixing a list of tests for "dev_desc->type" and "dev_desc->if_type" into one switch(), which then mostly did not work because "dev_desc->type" cannot take any "IF_*" type values. A later fix in commit 8ec6e332ea changed the switch() into testing "dev_desc->if_type", but at this point the initial test for unknown device types was completely lost, which resulted in output like that for IDE ports without device attached:
Device 1: Model: Firm: Ser#: Type: # 1F # Capacity: not available
This patch re-introduces the missing test for unknown device types.
Signed-off-by: Wolfgang Denk wd@denx.de Cc: Stefan Roese sr@denx.de Cc: Detlev Zundel dzu@denx.de --- disk/part.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/disk/part.c b/disk/part.c index c777493..39c1b42 100644 --- a/disk/part.c +++ b/disk/part.c @@ -114,6 +114,11 @@ void dev_print (block_dev_desc_t *dev_desc) lbaint_t lba512; #endif
+ if (dev_desc->type==DEV_TYPE_UNKNOWN) { + puts ("not available\n"); + return; + } + switch (dev_desc->if_type) { case IF_TYPE_SCSI: printf ("(%d:%d) Vendor: %s Prod.: %s Rev: %s\n",
Commit 574b319512 introduced a subtle bug by mixing a list of tests for "dev_desc->type" and "dev_desc->if_type" into one switch(), which then mostly did not work because "dev_desc->type" cannot take any "IF_*" type values. A later fix in commit 8ec6e332ea changed the switch() into testing "dev_desc->if_type", but at this point the initial test for unknown device types was completely lost, which resulted in output like that for IDE ports without device attached:
Device 1: Model: Firm: Ser#: Type: # 1F # Capacity: not available
This patch re-introduces the missing test for unknown device types.
Signed-off-by: Wolfgang Denk wd@denx.de Cc: Stefan Roese sr@denx.de Cc: Detlev Zundel dzu@denx.de --- Oh those **** typos!
disk/part.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/disk/part.c b/disk/part.c index c777493..39c1b42 100644 --- a/disk/part.c +++ b/disk/part.c @@ -114,6 +114,11 @@ void dev_print (block_dev_desc_t *dev_desc) lbaint_t lba512; #endif
+ if (dev_desc->type==DEV_TYPE_UNKNOWN) { + puts ("not available\n"); + return; + } + switch (dev_desc->if_type) { case IF_TYPE_SCSI: printf ("(%d:%d) Vendor: %s Prod.: %s Rev: %s\n",
Hi Wolfgang,
On Thursday 14 May 2009 22:59:49 Wolfgang Denk wrote:
Commit 574b319512 introduced a subtle bug by mixing a list of tests for "dev_desc->type" and "dev_desc->if_type" into one switch(), which then mostly did not work because "dev_desc->type" cannot take any "IF_*" type values. A later fix in commit 8ec6e332ea changed the switch() into testing "dev_desc->if_type", but at this point the initial test for unknown device types was completely lost, which resulted in output like that for IDE ports without device attached:
Device 1: Model: Firm: Ser#: Type: # 1F # Capacity: not available
This patch re-introduces the missing test for unknown device types.
Signed-off-by: Wolfgang Denk wd@denx.de Cc: Stefan Roese sr@denx.de Cc: Detlev Zundel dzu@denx.de
Tested-by: Stefan Roese sr@denx.de
One small nitpicking comment though below.
Oh those **** typos!
disk/part.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/disk/part.c b/disk/part.c index c777493..39c1b42 100644 --- a/disk/part.c +++ b/disk/part.c @@ -114,6 +114,11 @@ void dev_print (block_dev_desc_t *dev_desc) lbaint_t lba512; #endif
- if (dev_desc->type==DEV_TYPE_UNKNOWN) {
if (dev_desc->type == DEV_TYPE_UNKNOWN) {
Thanks.
Best regards, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================
Dear Stefan Roese,
In message 200905150727.09965.sr@denx.de you wrote:
This patch re-introduces the missing test for unknown device types.
Signed-off-by: Wolfgang Denk wd@denx.de Cc: Stefan Roese sr@denx.de Cc: Detlev Zundel dzu@denx.de
Tested-by: Stefan Roese sr@denx.de
So it's working for you, too? Good :-)
One small nitpicking comment though below.
...
- if (dev_desc->type==DEV_TYPE_UNKNOWN) {
if (dev_desc->type == DEV_TYPE_UNKNOWN) {
OK, will fix.
Best regards,
Wolfgang Denk
On Friday 15 May 2009 09:27:50 Wolfgang Denk wrote:
This patch re-introduces the missing test for unknown device types.
Signed-off-by: Wolfgang Denk wd@denx.de Cc: Stefan Roese sr@denx.de Cc: Detlev Zundel dzu@denx.de
Tested-by: Stefan Roese sr@denx.de
So it's working for you, too? Good :-)
Yes. Thanks.
Best regards, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================
Commit 574b319512 introduced a subtle bug by mixing a list of tests for "dev_desc->type" and "dev_desc->if_type" into one switch(), which then mostly did not work because "dev_desc->type" cannot take any "IF_*" type values. A later fix in commit 8ec6e332ea changed the switch() into testing "dev_desc->if_type", but at this point the initial test for unknown device types was completely lost, which resulted in output like that for IDE ports without device attached:
Device 1: Model: Firm: Ser#: Type: # 1F # Capacity: not available
This patch re-introduces the missing test for unknown device types.
Signed-off-by: Wolfgang Denk wd@denx.de Cc: Stefan Roese sr@denx.de Cc: Detlev Zundel dzu@denx.de --- v2: fix typo v3: fix white space
disk/part.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/disk/part.c b/disk/part.c index c777493..b92fb45 100644 --- a/disk/part.c +++ b/disk/part.c @@ -114,6 +114,11 @@ void dev_print (block_dev_desc_t *dev_desc) lbaint_t lba512; #endif
+ if (dev_desc->type == DEV_TYPE_UNKNOWN) { + puts ("not available\n"); + return; + } + switch (dev_desc->if_type) { case IF_TYPE_SCSI: printf ("(%d:%d) Vendor: %s Prod.: %s Rev: %s\n",
In message 1242372478-8446-1-git-send-email-wd@denx.de you wrote:
Commit 574b319512 introduced a subtle bug by mixing a list of tests for "dev_desc->type" and "dev_desc->if_type" into one switch(), which then mostly did not work because "dev_desc->type" cannot take any "IF_*" type values. A later fix in commit 8ec6e332ea changed the switch() into testing "dev_desc->if_type", but at this point the initial test for unknown device types was completely lost, which resulted in output like that for IDE ports without device attached:
Device 1: Model: Firm: Ser#: Type: # 1F # Capacity: not available
This patch re-introduces the missing test for unknown device types.
Signed-off-by: Wolfgang Denk wd@denx.de Cc: Stefan Roese sr@denx.de Cc: Detlev Zundel dzu@denx.de
v2: fix typo v3: fix white space
disk/part.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
Applied.
Wolfgang Denk
participants (2)
-
Stefan Roese -
Wolfgang Denk