drivers/ide/ide-probe.c, kernel 2.6.23.1
PATCH 1/1 drivers/ide/ide-probe.c, kernel 2.6.23.1
Hi.
This week I ran into a strange hardware problem. During boot I got a 35 second delay while waiting for IDE-disks that weren't there to report that they were not in a BSY state. The problem was most likely in the hardware but this patch enables you to ignore waiting for disks by setting hdX=noprobe (and not setting the geometry by hand) as a kernel option.
If no noprobe-option is set the code will work (more or less) as the original but if set the code will skip the ide_wait_not_busy() for that drive. Even if there would be a drive there and it is still BSY afterwards it should not matter since it isn't probed for later.
There are other ways to get around the "35-seconds-of-waiting-in-vain", like actually fix the hardware, insert a second drive that works or recompile the kernel with edd-support builtin (atleast I've seen that solution on a forum) and possibly others. But this patch would allow people to get Linux to boot quickly on wonky hardware without having to recompile anything.
(The code also honors the MAX_DRIVES variable instead of assuming that ther will be 2 drives on the bus.)
I will be happy for all the comments I can get. :) But be gentle, this is my first patch...
Best regards
Jonas Stare
--
drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +18 -13
@@ -643,6 +643,7 @@ static int wait_hwif_ready(ide_hwif_t *hwif) { int rc; + int unit; printk(KERN_DEBUG "Probing IDE interface %s...\n", hwif->name);
@@ -659,20 +660,24 @@ return rc; /* Now make sure both master & slave are ready */ - SELECT_DRIVE(&hwif->drives[0]); - hwif->OUTB(8, hwif->io_ports[IDE_CONTROL_OFFSET]); - mdelay(2); - rc = ide_wait_not_busy(hwif, 35000); - if (rc) - return rc; - SELECT_DRIVE(&hwif->drives[1]); - hwif->OUTB(8, hwif->io_ports[IDE_CONTROL_OFFSET]); - mdelay(2); - rc = ide_wait_not_busy(hwif, 35000); + for (unit = 0; unit < MAX_DRIVES; ++unit) { + /* Ignore disks that we will not probe for later. */ + if (!hwif->drives[unit].noprobe || + hwif->drives[unit].forced_geom) { + SELECT_DRIVE(&hwif->drives[unit]); + hwif->OUTB(8, hwif->io_ports[IDE_CONTROL_OFFSET]); + mdelay(2); + rc = ide_wait_not_busy(hwif, 35000); + /* Exit function with master selected (let's be sane) */ + SELECT_DRIVE(&hwif->drives[0]); + if (rc) + return rc; + } else { + printk(KERN_DEBUG "Skip ide_wait_not_busy for %s:%d\n", + hwif->name, unit); + } + } - /* Exit function with master reselected (let's be sane) */ - SELECT_DRIVE(&hwif->drives[0]); - return rc; }
Quoted 21 lines · On Fri, 09 Nov 2007 11:22:41 +0100 Jonas Stare <[email protected]>…
Hi.
This week I ran into a strange hardware problem. During boot I got a 35 second delay while waiting for IDE-disks that weren't there to report that they were not in a BSY state. The problem was most likely in the hardware but this patch enables you to ignore waiting for disks by setting hdX=noprobe (and not setting the geometry by hand) as a kernel option.
If no noprobe-option is set the code will work (more or less) as the original but if set the code will skip the
ide_wait_not_busy()for that drive. Even if there would be a drive there and it is still BSY afterwards it should not matter since it isn't probed for later.There are other ways to get around the "35-seconds-of-waiting-in-vain", like actually fix the hardware, insert a second drive that works or recompile the kernel with edd-support builtin (atleast I've seen that solution on a forum) and possibly others. But this patch would allow people to get Linux to boot quickly on wonky hardware without having to recompile anything.
Quoted 2 lines
(The code also honors the
MAX_DRIVESvariable instead of assuming that ther will be 2 drives on the bus.)
I keep on hearing about problems with boot-time IDE probing. It's public enemy #1 with the embedded guys.
It does seem that operator intervention is needed in some fashion.
Quoted 52 lines
I will be happy for all the comments I can get. :) But be gentle, this is my first patch...
Best regards
Jonas StareSigned-off-by Jonas Stare <[email protected]>--
diff -ulinux-2.6.23.1-orig/drivers/ide/ide-probe.clinux-2.6.23.1/drivers/ide/ide-probe.c
---linux-2.6.23.1-orig/drivers/ide/ide-probe.c2007-10-12 18:43:44.000000000 +0200
+++linux-2.6.23.1/drivers/ide/ide-probe.c2007-11-09
10:43:16.000000000 +0100drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +10
-10@@ -643,6 +643,7 @@ static int wait_hwif_ready(ide_hwif_t *hwif) { int rc; + int unit; printk(KERN_DEBUG "Probing IDE interface %s...\n", hwif->name);@@ -659,20 +660,24 @@ return rc; /* Now make sure both master & slave are ready */ - SELECT_DRIVE(&hwif->drives[0]); - hwif->OUTB(8, hwif->io_ports[IDE_CONTROL_OFFSET]); - mdelay(2); - rc = ide_wait_not_busy(hwif, 35000); - if (rc) - return rc; - SELECT_DRIVE(&hwif->drives[1]); - hwif->OUTB(8, hwif->io_ports[IDE_CONTROL_OFFSET]); - mdelay(2); - rc = ide_wait_not_busy(hwif, 35000); + for (unit = 0; unit < MAX_DRIVES; ++unit) { + /* Ignore disks that we will not probe for later. */ + if (!hwif->drives[unit].noprobe || + hwif->drives[unit].forced_geom) { + SELECT_DRIVE(&hwif->drives[unit]); + hwif->OUTB(8, hwif->io_ports[IDE_CONTROL_OFFSET]); + mdelay(2); + rc = ide_wait_not_busy(hwif, 35000); + /* Exit function with master selected (let's besane) */
drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +5
-0+ SELECT_DRIVE(&hwif->drives[0]); + if (rc) + return rc; + } else { + printk(KERN_DEBUG "Skip ide_wait_not_busy for%s:%d\n",
drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 line 682
Quoted 9 lines
drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +3
-3+ hwif->name, unit); + } + } - /* Exit function with master reselected (let's be sane) */ - SELECT_DRIVE(&hwif->drives[0]); - return rc; }
Maybe that's the fix, maybe not - I'll defer to others on that (please).
Your email client is wordwrapping the text, and replaces tabs with spaces. Most of them seem to do this nowadays. For thunderbird, please see
http://mbligh.org/linuxdocs/Email/Clients/Thunderbird
Quoted 4 lines
Quoted 4 lines
that they were not in a BSY state. The problem was most likely in the hardware but this patch enables you to ignore waiting for disks by setting hdX=noprobe (and not setting the geometry by hand) as a kernel option.
Probably someone saving a billionth of a cent on a resistor.
Quoted 4 lines
drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 line 671
Quoted 4 lines
drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +0
-4- SELECT_DRIVE(&hwif->drives[1]); - hwif->OUTB(8, hwif->io_ports[IDE_CONTROL_OFFSET]); - mdelay(2); - rc = ide_wait_not_busy(hwif, 35000);
Please first verify if you see the same problem with the newer ATA drivers. I suspect you will and if so we should be fixing those not the legacy drivers (or as well as ..)
Alan
Hi,
Quoted 6 lines · Andrew Morton
Quoted 4 lines · On Fri, 09 Nov 2007 11:22:41 +0100 Jonas Stare <[email protected]>…
Hi.
This week I ran into a strange hardware problem. During boot I got a 35 second delay while waiting for IDE-disks that weren't there to report
With what chipset and host driver does this happen?
Quoted 17 lines
Quoted 16 lines
that they were not in a BSY state. The problem was most likely in the hardware but this patch enables you to ignore waiting for disks by setting hdX=noprobe (and not setting the geometry by hand) as a kernel option.
If no noprobe-option is set the code will work (more or less) as the original but if set the code will skip the
ide_wait_not_busy()for that drive. Even if there would be a drive there and it is still BSY afterwards it should not matter since it isn't probed for later.There are other ways to get around the "35-seconds-of-waiting-in-vain", like actually fix the hardware, insert a second drive that works or recompile the kernel with edd-support builtin (atleast I've seen that solution on a forum) and possibly others. But this patch would allow people to get Linux to boot quickly on wonky hardware without having to recompile anything.
Quoted 5 lines
Quoted 2 lines
(The code also honors the
MAX_DRIVESvariable instead of assuming that ther will be 2 drives on the bus.)I keep on hearing about problems with boot-time IDE probing. It's public enemy #1 with the embedded guys.
The problem is that we are not hearing about them.
Please forward the reports to [email protected].
Quoted 4 lines
It does seem that operator intervention is needed in some fashion.
Quoted 2 lines
I will be happy for all the comments I can get. :) But be gentle, this is my first patch...
Jonas, could you also put printk() dumping content of 'stat' in ide-iops.c::ide_wait_not_busy() so we can verify that it is not some problem with ide_wait_not_busy() itself.
Quoted 17 lines
Quoted 17 lines
Best regards
Jonas StareSigned-off-by Jonas Stare <[email protected]>--
diff -ulinux-2.6.23.1-orig/drivers/ide/ide-probe.clinux-2.6.23.1/drivers/ide/ide-probe.c
---linux-2.6.23.1-orig/drivers/ide/ide-probe.c2007-10-12 18:43:44.000000000 +0200
+++linux-2.6.23.1/drivers/ide/ide-probe.c2007-11-09
10:43:16.000000000 +0100drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +1
-0@@ -643,6 +643,7 @@ static int wait_hwif_ready(ide_hwif_t *hwif) { int rc; + int unit;
Quoted 5 lines
drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 line 661
Quoted 5 lines
drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +0
-0printk(KERN_DEBUG "Probing IDE interface %s...\n", hwif->name);@@ -659,20 +660,24 @@ return rc;
Hmm, so the first ide_wait_not_busy() (for the currently selected device) is OK and doesn't stall?
Quoted 7 lines
Quoted 7 lines
drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +0
-6/* Now make sure both master & slave are ready */ - SELECT_DRIVE(&hwif->drives[0]); - hwif->OUTB(8, hwif->io_ports[IDE_CONTROL_OFFSET]); - mdelay(2); - rc = ide_wait_not_busy(hwif, 35000); - if (rc) - return rc;
Quoted 8 lines
drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 line 666
Quoted 8 lines
drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +4
-4- SELECT_DRIVE(&hwif->drives[1]); - hwif->OUTB(8, hwif->io_ports[IDE_CONTROL_OFFSET]); - mdelay(2); - rc = ide_wait_not_busy(hwif, 35000); + for (unit = 0; unit < MAX_DRIVES; ++unit) { + /* Ignore disks that we will not probe for later. */ + if (!hwif->drives[unit].noprobe || + hwif->drives[unit].forced_geom) {
It is better to check for ->present
(->forced_geom implies that ->present is set).
Quoted 7 lines
Quoted 7 lines
drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +5
-0+ SELECT_DRIVE(&hwif->drives[unit]); + hwif->OUTB(8, hwif->io_ports[IDE_CONTROL_OFFSET]); + mdelay(2); + rc = ide_wait_not_busy(hwif, 35000); + /* Exit function with master selected (let's besane) */
+SELECT_DRIVE(&hwif->drives[0]);
This changes the previous behavior adding an extra SELECT_DRIVE() before trying the slave drive.
Quoted 20 lines
Quoted 5 lines
drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +4
-0+ if (rc) + return rc; + } else { + printk(KERN_DEBUG "Skip ide_wait_not_busy for%s:%d\n",
drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 line 682
Quoted 9 lines
drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +3
-3+ hwif->name, unit); + } + } - /* Exit function with master reselected (let's be sane) */ - SELECT_DRIVE(&hwif->drives[0]); - return rc; }Maybe that's the fix, maybe not - I'll defer to others on that (please).
Your email client is wordwrapping the text, and replaces tabs with spaces. Most of them seem to do this nowadays. For thunderbird, please see
http://mbligh.org/linuxdocs/Email/Clients/Thunderbird
Hi, thanks for the reply. :)
Quoted 10 lines · Bartlomiej Zolnierkiewicz
Hi,
Quoted 5 lines · Andrew Morton
Quoted 3 lines · On Fri, 09 Nov 2007 11:22:41 +0100 Jonas Stare <[email protected]>…
Hi.
This week I ran into a strange hardware problem. During boot I got a 35 second delay while waiting for IDE-disks that weren't there to report
With what chipset and host driver does this happen?
I am not sure about the chip-set but I think it was vt82c686b. It used the via82cxxx-driver, but only _after_ using the generic ide-code to probe/wait (a long time) for the disks. (This was in Suse 10.1 SP 1.)
Quoted 13 lines
Quoted 8 lines
Quoted 6 lines
that they were not in a BSY state. The problem was most likely in the hardware but this patch enables you to ignore waiting for disks by setting hdX=noprobe (and not setting the geometry by hand) as a kernel option.
If no noprobe-option is set the code will work (more or less) as the original but if set the code will skip the
ide_wait_not_busy()for that drive. Even if there would be a drive there and it is still BSY afterwards it should not matter since it isn't probed for later.There are other ways to get around the "35-seconds-of-waiting-in-vain", like actually fix the hardware, insert a second drive that works or recompile the kernel with edd-support builtin (atleast I've seen that solution on a forum) and possibly others. But this patch would allow people to get Linux to boot quickly on wonky hardware without having to recompile anything. (The code also honors the
MAX_DRIVESvariable instead of assuming that ther will be 2 drives on the bus.)I keep on hearing about problems with boot-time IDE probing. It's public enemy #1 with the embedded guys.
The problem is that we are not hearing about them.
Please forward the reports to [email protected].
Quoted 8 lines
Quoted 3 lines
It does seem that operator intervention is needed in some fashion.
Quoted 1 line
I will be happy for all the comments I can get. :) But be gentle, this is my first patch...
Jonas, could you also put
printk()dumping content of 'stat' in ide-iops.c::ide_wait_not_busy() so we can verify that it is not some problem withide_wait_not_busy()itself.
Sorry. :( I don't have access to the hardware anymore (which is a "home-made" embedded machine). But from what I could get from poking around was that the BSY-bit on the slave (that never has or ever will exists) was set, probably because those who built the thing wanted to save money and/or space on that "billionth of a cent"-resistor that Alan Cox talked about.
Quoted 16 lines
Quoted 16 lines
Quoted 16 lines
Best regards
Jonas StareSigned-off-by Jonas Stare <[email protected]>--
diff -ulinux-2.6.23.1-orig/drivers/ide/ide-probe.clinux-2.6.23.1/drivers/ide/ide-probe.c---linux-2.6.23.1-orig/drivers/ide/ide-probe.c2007-10-12 18:43:44.000000000 +0200
+++linux-2.6.23.1/drivers/ide/ide-probe.c2007-11-09
10:43:16.000000000 +0100drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +1
-0@@ -643,6 +643,7 @@ static int wait_hwif_ready(ide_hwif_t *hwif) { int rc; + int unit;
Quoted 9 lines
Quoted 5 lines
drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 line 661
Quoted 5 lines
drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +0
-0printk(KERN_DEBUG "Probing IDE interface %s...\n", hwif->name);@@ -659,20 +660,24 @@ return rc;Hmm, so the first
ide_wait_not_busy()(for the currently selected device) is OK and doesn't stall?
It didn't stall for me... But even if it had, probe_hwif() will ignore the entire controller if you set "idex=noprobe".
(From drivers/ide/ide-probe.c)
static void probe_hwif(ide_hwif_t *hwif, void (*fixup)(ide_hwif_t *hwif))
{
unsigned int unit;
unsigned long flags;
unsigned int irqd;
if (hwif->noprobe)
return;Quoted 16 lines
Quoted 7 lines
Quoted 7 lines
drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +0
-6/* Now make sure both master & slave are ready */ - SELECT_DRIVE(&hwif->drives[0]); - hwif->OUTB(8, hwif->io_ports[IDE_CONTROL_OFFSET]); - mdelay(2); - rc = ide_wait_not_busy(hwif, 35000); - if (rc) - return rc;Quoted 8 lines
drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 line 666
Quoted 8 lines
drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +4
-4- SELECT_DRIVE(&hwif->drives[1]); - hwif->OUTB(8, hwif->io_ports[IDE_CONTROL_OFFSET]); - mdelay(2); - rc = ide_wait_not_busy(hwif, 35000); + for (unit = 0; unit < MAX_DRIVES; ++unit) { + /* Ignore disks that we will not probe for later. */ + if (!hwif->drives[unit].noprobe || + hwif->drives[unit].forced_geom) {
Quoted 3 lines
It is better to check for ->present
(->forced_geom implies that ->present is set).
Great comment. :) I'll change that right away...
Quoted 11 lines
Quoted 7 lines
Quoted 7 lines
drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +5
-0+ SELECT_DRIVE(&hwif->drives[unit]); + hwif->OUTB(8, hwif->io_ports[IDE_CONTROL_OFFSET]); + mdelay(2); + rc = ide_wait_not_busy(hwif, 35000); + /* Exit function with master selected (let's besane) */
+SELECT_DRIVE(&hwif->drives[0]);This changes the previous behavior adding an extra
SELECT_DRIVE()before trying the slave drive.
Mmmm, yes, I know. But I couldn't come up with a clean and nice way to be sure that the first drive is selected. Maybe I could move it inside the if-statement below?
Quoted 23 lines
Quoted 14 lines
Quoted 5 lines
drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +4
-0+ if (rc) + return rc; + } else { + printk(KERN_DEBUG "Skip ide_wait_not_busy for%s:%d\n",
drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 line 682
Quoted 9 lines
drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +3
-3+ hwif->name, unit); + } + } - /* Exit function with master reselected (let's be sane) */ - SELECT_DRIVE(&hwif->drives[0]); - return rc; }Quoted 4 lines
Maybe that's the fix, maybe not - I'll defer to others on that (please).
Your email client is wordwrapping the text, and replaces tabs with spaces. Most of them seem to do this nowadays. For thunderbird, please see
http://mbligh.org/linuxdocs/Email/Clients/Thunderbird-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Hi,
Quoted 10 lines · Jonas Stare
Hi, thanks for the reply. :)
Quoted 7 lines · Bartlomiej Zolnierkiewicz
Hi,
Quoted 4 lines · Andrew Morton
Quoted 2 lines · On Fri, 09 Nov 2007 11:22:41 +0100 Jonas Stare <[email protected]>…
Hi.
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
Quoted 2 lines
This week I ran into a strange hardware problem. During boot I got a 35 second delay while waiting for IDE-disks that weren't there to report
With what chipset and host driver does this happen?
I am not sure about the chip-set but I think it was vt82c686b. It used the via82cxxx-driver, but only _after_ using the generic ide-code to probe/wait (a long time) for the disks. (This was in Suse 10.1 SP 1.)
AFAIK this means kernel version 2.6.16.xx.
If this is the case maybe the underlying problem has been already fixed (but it doesn't mean that your patch is not worth applying, quite the contrary).
Quoted 114 lines
Quoted 25 lines
Quoted 18 lines
Quoted 10 lines
that they were not in a BSY state. The problem was most likely in the hardware but this patch enables you to ignore waiting for disks by setting hdX=noprobe (and not setting the geometry by hand) as a kernel option.
If no noprobe-option is set the code will work (more or less) as the original but if set the code will skip the
ide_wait_not_busy()for that drive. Even if there would be a drive there and it is still BSY afterwards it should not matter since it isn't probed for later.Quoted 8 lines
There are other ways to get around the "35-seconds-of-waiting-in-vain", like actually fix the hardware, insert a second drive that works or recompile the kernel with edd-support builtin (atleast I've seen that solution on a forum) and possibly others. But this patch would allow people to get Linux to boot quickly on wonky hardware without having to recompile anything.
(The code also honors theMAX_DRIVESvariable instead of assuming that ther will be 2 drives on the bus.)Quoted 2 lines
I keep on hearing about problems with boot-time IDE probing. It's public enemy #1 with the embedded guys.
The problem is that we are not hearing about them.
Please forward the reports to [email protected].
Quoted 9 lines
Quoted 4 lines
It does seem that operator intervention is needed in some fashion.
Quoted 2 lines
I will be happy for all the comments I can get. :) But be gentle, this is my first patch...
Jonas, could you also put
printk()dumping content of 'stat' in ide-iops.c::ide_wait_not_busy() so we can verify that it is not some problem withide_wait_not_busy()itself.Sorry. :( I don't have access to the hardware anymore (which is a "home-made" embedded machine). But from what I could get from poking around was that the BSY-bit on the slave (that never has or ever will exists) was set, probably because those who built the thing wanted to save money and/or space on that "billionth of a cent"-resistor that Alan Cox talked about.
Quoted 17 lines
Quoted 17 lines
Quoted 17 lines
Best regards
Jonas StareSigned-off-by Jonas Stare <[email protected]>--
diff -ulinux-2.6.23.1-orig/drivers/ide/ide-probe.clinux-2.6.23.1/drivers/ide/ide-probe.c
---linux-2.6.23.1-orig/drivers/ide/ide-probe.c2007-10-12 18:43:44.000000000 +0200
+++linux-2.6.23.1/drivers/ide/ide-probe.c2007-11-09
10:43:16.000000000 +0100drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +1
-0@@ -643,6 +643,7 @@ static int wait_hwif_ready(ide_hwif_t *hwif) { int rc; + int unit;Quoted 9 lines
Quoted 5 lines
drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 line 661
Quoted 5 lines
drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +0
-0printk(KERN_DEBUG "Probing IDE interface %s...\n", hwif->name);@@ -659,20 +660,24 @@ return rc;Hmm, so the first
ide_wait_not_busy()(for the currently selected device) is OK and doesn't stall?It didn't stall for me... But even if it had,
probe_hwif()will ignore the entire controller if you set "idex=noprobe".(From
drivers/ide/ide-probe.c)static void
probe_hwif(ide_hwif_t *hwif, void (*fixup)(ide_hwif_t *hwif)){ unsigned int unit; unsigned long flags; unsigned int irqd; if (hwif->noprobe) return;Quoted 16 lines
Quoted 7 lines
Quoted 7 lines
drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +0
-6/* Now make sure both master & slave are ready */ - SELECT_DRIVE(&hwif->drives[0]); - hwif->OUTB(8, hwif->io_ports[IDE_CONTROL_OFFSET]); - mdelay(2); - rc = ide_wait_not_busy(hwif, 35000); - if (rc) - return rc;Quoted 8 lines
drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 line 666
Quoted 8 lines
drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +4
-4- SELECT_DRIVE(&hwif->drives[1]); - hwif->OUTB(8, hwif->io_ports[IDE_CONTROL_OFFSET]); - mdelay(2); - rc = ide_wait_not_busy(hwif, 35000); + for (unit = 0; unit < MAX_DRIVES; ++unit) { + /* Ignore disks that we will not probe for later. */ + if (!hwif->drives[unit].noprobe || + hwif->drives[unit].forced_geom) {Quoted 3 lines
It is better to check for ->present
(->forced_geom implies that ->present is set).Great comment. :) I'll change that right away...
Quoted 8 lines
Quoted 7 lines
Quoted 7 lines
drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +5
-0+ SELECT_DRIVE(&hwif->drives[unit]); + hwif->OUTB(8, hwif->io_ports[IDE_CONTROL_OFFSET]); + mdelay(2); + rc = ide_wait_not_busy(hwif, 35000); + /* Exit function with master selected (let's besane) */
+SELECT_DRIVE(&hwif->drives[0]);
Quoted 7 lines
Quoted 3 lines
This changes the previous behavior adding an extra
SELECT_DRIVE()before trying the slave drive.Mmmm, yes, I know. But I couldn't come up with a clean and nice way to be sure that the first drive is selected. Maybe I could move it inside the if-statement below?
Then the code will miss re-selecting master drive after waiting for slave one if there were no problems (rc == 0).
What about adding "if (unit)" statement before SELECT_DRIVE() call?
Quoted 14 lines
Quoted 14 lines
Quoted 14 lines
Quoted 14 lines
drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +4
-0+ if (rc) + return rc; + } else { + printk(KERN_DEBUG "Skip ide_wait_not_busy for%s:%d\n",
drivers/ide/ide-probe.c 2007-11-09 10:43:16.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +3
-3+ hwif->name, unit); + } + } - /* Exit function with master reselected (let's be sane) */ - SELECT_DRIVE(&hwif->drives[0]); - return rc; }
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Maybe that's the fix, maybe not - I'll defer to others on that (please).
Your email client is wordwrapping the text, and replaces tabs with spaces. Most of them seem to do this nowadays. For thunderbird, please see
http://mbligh.org/linuxdocs/Email/Clients/Thunderbird
Please fix the issues + wordwrapping and resubmit.
Thanks,
Bart
Resubmit of IDE-probing-patch. It now checks hwif->drives[unit].present instead of forced_geom and there is also an if-statement so it doesn't change back to the master unless it have to. I choose to put it inside the if-statement that checks the return code from ide_wait_not_busy, since it shouldn't revert back to the master drive unless something is wrong and we have to leave the function. (I also left the old SELECT_DRIVE after the for-loop so that we will select the master if everything is ok.) Inside the if-statement or not makes not big difference until you get more than two drives on the ide-bus...but this code should be ready for it. :)
drivers/ide/ide-probe.c 2007-11-16 10:26:23.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +20 -10
@@ -643,6 +643,7 @@ static int wait_hwif_ready(ide_hwif_t *hwif) { int rc; + int unit; printk(KERN_DEBUG "Probing IDE interface %s...\n", hwif->name);
@@ -659,16 +660,25 @@ return rc; /* Now make sure both master & slave are ready */ - SELECT_DRIVE(&hwif->drives[0]); - hwif->OUTB(8, hwif->io_ports[IDE_CONTROL_OFFSET]); - mdelay(2); - rc = ide_wait_not_busy(hwif, 35000); - if (rc) - return rc; - SELECT_DRIVE(&hwif->drives[1]); - hwif->OUTB(8, hwif->io_ports[IDE_CONTROL_OFFSET]); - mdelay(2); - rc = ide_wait_not_busy(hwif, 35000); + for (unit = 0; unit < MAX_DRIVES; ++unit) { + /* Ignore disks that we will not probe for later. */ + if (!hwif->drives[unit].noprobe || + hwif->drives[unit].present) { + SELECT_DRIVE(&hwif->drives[unit]); + hwif->OUTB(8, hwif->io_ports[IDE_CONTROL_OFFSET]); + mdelay(2); + rc = ide_wait_not_busy(hwif, 35000); + if (rc) { + /* Exit function with master reselected */ + if (unit != 0) + SELECT_DRIVE(&hwif->drives[0]); + return rc; + } + } else { + printk(KERN_DEBUG "Skip ide_wait_not_busy for %s:%d\n", + hwif->name, unit); + } + } /* Exit function with master reselected (let's be sane) */ SELECT_DRIVE(&hwif->drives[0]);
Quoted 19 lines · Jonas Stare
diff -u
linux-2.6.23.1-orig/drivers/ide/ide-probe.clinux-2.6.23.1/drivers/ide/ide-probe.cdrivers/ide/ide-probe.c 2007-11-16 10:26:23.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +1
-4@@ -643,6 +643,7 @@ static int wait_hwif_ready(ide_hwif_t *hwif) { int rc; + int unit; printk(KERN_DEBUG "Probing IDE interface %s...\n", hwif->name);@@ -659,16 +660,25 @@ return rc; /* Now make sure both master & slave are ready */ - SELECT_DRIVE(&hwif->drives[0]); - hwif->OUTB(8, hwif->io_ports[IDE_CONTROL_OFFSET]); - mdelay(2); - rc = ide_wait_not_busy(hwif, 35000);
drivers/ide/ide-probe.c 2007-11-16 10:26:23.000000000 +0100 line 666
Quoted 10 lines
drivers/ide/ide-probe.c 2007-11-16 10:26:23.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +4
-6- if (rc) - return rc; - SELECT_DRIVE(&hwif->drives[1]); - hwif->OUTB(8, hwif->io_ports[IDE_CONTROL_OFFSET]); - mdelay(2); - rc = ide_wait_not_busy(hwif, 35000); + for (unit = 0; unit < MAX_DRIVES; ++unit) { + /* Ignore disks that we will not probe for later. */ + if (!hwif->drives[unit].noprobe || + hwif->drives[unit].present) {
Why not:
if (hwif->drives[unit].noprobe || !hwif->drives[unit].present) {
printk(KERN_DEBUG "Skip ide_wait_not_busy for %s:%d\n",
hwif->name, unit);
continue;
}
SELECT_DRIVE(&hwif->drives[unit]);
hwif->OUTB(8, hwif->io_ports[IDE_CONTROL_OFFSET]);
mdelay(2);
rc = ide_wait_not_busy(hwif, 35000);
if (rc) {
/* Exit function with master reselected */
if (unit != 0)
SELECT_DRIVE(&hwif->drives[0]);
return rc;
}
}drivers/ide/ide-probe.c 2007-11-16 10:26:23.000000000 +0100 line 684
Quoted 3 lines
drivers/ide/ide-probe.c 2007-11-16 10:26:23.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +0
-0/* Exit function with master reselected (let's be sane) */ SELECT_DRIVE(&hwif->drives[0]);
MBR, Sergei
Quoted 45 lines · Sergei Shtylyov
Quoted 22 lines · Jonas Stare
diff -u
linux-2.6.23.1-orig/drivers/ide/ide-probe.clinux-2.6.23.1/drivers/ide/ide-probe.c---linux-2.6.23.1-orig/drivers/ide/ide-probe.c2007-10-12 18:43:44.000000000 +0200
+++linux-2.6.23.1/drivers/ide/ide-probe.c2007-11-16
10:26:23.000000000 +0100drivers/ide/ide-probe.c 2007-11-16 10:26:23.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +1
-4@@ -643,6 +643,7 @@ static int wait_hwif_ready(ide_hwif_t *hwif) { int rc; + int unit; printk(KERN_DEBUG "Probing IDE interface %s...\n", hwif->name);@@ -659,16 +660,25 @@ return rc; /* Now make sure both master & slave are ready */ - SELECT_DRIVE(&hwif->drives[0]); - hwif->OUTB(8, hwif->io_ports[IDE_CONTROL_OFFSET]); - mdelay(2); - rc = ide_wait_not_busy(hwif, 35000);drivers/ide/ide-probe.c 2007-11-16 10:26:23.000000000 +0100 line 666
Quoted 10 lines
drivers/ide/ide-probe.c 2007-11-16 10:26:23.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +4
-6- if (rc) - return rc; - SELECT_DRIVE(&hwif->drives[1]); - hwif->OUTB(8, hwif->io_ports[IDE_CONTROL_OFFSET]); - mdelay(2); - rc = ide_wait_not_busy(hwif, 35000); + for (unit = 0; unit < MAX_DRIVES; ++unit) { + /* Ignore disks that we will not probe for later. */ + if (!hwif->drives[unit].noprobe || + hwif->drives[unit].present) {Why not: if (hwif->drives[unit].noprobe || !hwif->drives[unit].present) { printk(KERN_DEBUG "Skip ide_wait_not_busy for %s:%d\n", hwif->name, unit); continue; } SELECT_DRIVE(&hwif->drives[unit]); hwif->OUTB(8, hwif->io_ports[IDE_CONTROL_OFFSET]); mdelay(2);
Quoted 8 lines
rc = ide_wait_not_busy(hwif, 35000);
if (rc) {
/* Exit function with master reselected */
if (unit != 0)
SELECT_DRIVE(&hwif->drives[0]);
return rc;
}
}Hmmm, I was rather fond of my "wait for the disk unless we are specifically told not to" rather than "don't check unless there might be a disk there". ...on the other hand thats not what my comment said. :)
Quoted 5 lines
drivers/ide/ide-probe.c 2007-11-16 10:26:23.000000000 +0100 line 684
Quoted 3 lines
drivers/ide/ide-probe.c 2007-11-16 10:26:23.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +0
-0/* Exit function with master reselected (let's be sane) */ SELECT_DRIVE(&hwif->drives[0]);MBR, Sergei
Quoted 10 lines · Jonas Stare
Resubmit of IDE-probing-patch. It now checks hwif->drives[unit].present instead of
forced_geomand there is also an if-statement so it doesn't change back to the master unless it have to. I choose to put it inside the if-statement that checks the return code fromide_wait_not_busy, since it shouldn't revert back to the master drive unless something is wrong and we have to leave the function. (I also left the oldSELECT_DRIVEafter the for-loop so that we will select the master if everything is ok.) Inside the if-statement or not makes not big difference until you get more than two drives on the ide-bus...but this code should be ready for it. :)Signed-off-by Jonas Stare <[email protected]>
Patch looks fine (thanks for fixing it) but please replace the changelog above with the proper patch description (more like in your first mail which contained the description of the problem encountered and of the solution, also please try to keep it within 80-columns limit) - for git history patch changelog is not really that interesting/valuable.
PS patch summary ("Subject:") may also be improved while at it - it should describe the main goal of the patch, also since patch will be applied to the current git tree giving the kernel version is not necessary.
Bart
PATCH 1/1 drivers/ide/ide-probe.c Skip ide_wait_not_busy on noprobe-disks. was: [PATCH][RESUBMIT] drivers/ide/ide-probe.c, kernel 2.6.23.1
There is a problem in some hardware where the kernel will stall for 35 seconds waiting for disks that dont exist. This patch will skip waiting for the BSY-bit on ide-drives to go away if you set hdx=noprobe as a kernel option and the disk is not marked as 'present' (like when you set the geometry by hand).
If no noprobe-option is set the code will work (more or less) as the original but if set the code will skip the ide_wait_not_busy() for that drive. Even if there would be a drive there and it is still BSY afterwards it should not matter since it isn't probed for later.
(The code also honors the MAX_DRIVES variable instead of assuming that there will be two harddrives on the bus.)
drivers/ide/ide-probe.c 2007-11-16 10:26:23.000000000 +0100 renamed from drivers/ide/ide-probe.c 2007-10-12 18:43:44.000000000 +0200 +20 -10
@@ -643,6 +643,7 @@ static int wait_hwif_ready(ide_hwif_t *hwif) { int rc; + int unit; printk(KERN_DEBUG "Probing IDE interface %s...\n", hwif->name);
@@ -659,16 +660,25 @@ return rc; /* Now make sure both master & slave are ready */ - SELECT_DRIVE(&hwif->drives[0]); - hwif->OUTB(8, hwif->io_ports[IDE_CONTROL_OFFSET]); - mdelay(2); - rc = ide_wait_not_busy(hwif, 35000); - if (rc) - return rc; - SELECT_DRIVE(&hwif->drives[1]); - hwif->OUTB(8, hwif->io_ports[IDE_CONTROL_OFFSET]); - mdelay(2); - rc = ide_wait_not_busy(hwif, 35000); + for (unit = 0; unit < MAX_DRIVES; ++unit) { + /* Ignore disks that we will not probe for later. */ + if (!hwif->drives[unit].noprobe || + hwif->drives[unit].present) { + SELECT_DRIVE(&hwif->drives[unit]); + hwif->OUTB(8, hwif->io_ports[IDE_CONTROL_OFFSET]); + mdelay(2); + rc = ide_wait_not_busy(hwif, 35000); + if (rc) { + /* Exit function with master reselected */ + if (unit != 0) + SELECT_DRIVE(&hwif->drives[0]); + return rc; + } + } else { + printk(KERN_DEBUG "Skip ide_wait_not_busy for %s:%d\n", + hwif->name, unit); + } + } /* Exit function with master reselected (let's be sane) */ SELECT_DRIVE(&hwif->drives[0]);
Re: [PATCH] drivers/ide/ide-probe.c Skip ide_wait_not_busy on noprobe-disks. was: [PATCH][RESUBMIT] drivers/ide/ide-probe.c, kernel 2.6.23.1
Quoted 11 lines · Jonas Stare
There is a problem in some hardware where the kernel will stall for 35 seconds waiting for disks that dont exist. This patch will skip waiting for the BSY-bit on ide-drives to go away if you set hdx=noprobe as a kernel option and the disk is not marked as 'present' (like when you set the geometry by hand).
If no noprobe-option is set the code will work (more or less) as the original but if set the code will skip the
ide_wait_not_busy()for that drive. Even if there would be a drive there and it is still BSY afterwards it should not matter since it isn't probed for later.
Quoted 5 lines
(The code also honors the
MAX_DRIVESvariable instead of assuming that there will be two harddrives on the bus.)Signed-off-by Jonas Stare <[email protected]>
Thanks!
PS There were still some whitespace problems (all tabs were replaced by spaces) so I just applied it by hand (and made few minor cleanups while at it).
Bart