libata: warn if speed limited due to 40-wire cable (v2)
PATCH 1/1 libata: warn if speed limited due to 40-wire cable (v2)
Here's a revised version of my previous patch to warn the user if a drive's transfer rate is limited because of a 40-wire cable detection. This one hopefully addresses Alan's previous comments - we now do this at the very end of the function, and the ugly if condition has been cleaned up somewhat. Also, it's been inadvertently tested (it seems that pata_amd Nvidia cable detection is broken in current -git..)
drivers/ata/libata-core.c +0 -0
@@ -3305,19 +3305,7 @@ static void ata_dev_xfermask(struct ata_
xfer_mask = ata_pack_xfermask(ap->pio_mask, ap->mwdma_mask, ap->udma_mask);
- /* Apply cable rule here. Don't apply it early because when
- * we handle hot plug the cable type can itself change.
- */
- if (ap->cbl == ATA_CBL_PATA40)
- xfer_mask &= ~(0xF8 << ATA_SHIFT_UDMA);
- /* Apply drive side cable rule. Unknown or 80 pin cables reported
- * host side are checked drive side as well. Cases where we know a
- * 40wire cable is used safely for 80 are not checked here.
- */
- if (ata_drive_40wire(dev->id) && (ap->cbl == ATA_CBL_PATA_UNK || ap->cbl == ATA_CBL_PATA80))
- xfer_mask &= ~(0xF8 << ATA_SHIFT_UDMA);
-
-
+ /* drive modes available */
xfer_mask &= ata_pack_xfermask(dev->pio_mask, dev->mwdma_mask, dev->udma_mask); xfer_mask &= ata_id_xfermask(dev->id); @@ -3348,6 +3336,25 @@ static void ata_dev_xfermask(struct ata_ if (ap->ops->mode_filter) xfer_mask = ap->ops->mode_filter(ap, dev, xfer_mask);
+ /* Apply cable rule here. Don't apply it early because when
+ * we handle hot plug the cable type can itself change.
+ * Check this last so that we know if the transfer rate was
+ * solely limited by the cable.
+ * Unknown or 80 wire cables reported host side are checked
+ * drive side as well. Cases where we know a 40wire cable
+ * is used safely for 80 are not checked here.
+ */
+ if (xfer_mask & (0xF8 << ATA_SHIFT_UDMA))
+ /* UDMA/44 or higher would be available */
+ if((ap->cbl == ATA_CBL_PATA40) ||
+ (ata_drive_40wire(dev->id) &&
+ (ap->cbl == ATA_CBL_PATA_UNK ||
+ ap->cbl == ATA_CBL_PATA80))) {
+ ata_dev_printk(dev, KERN_WARNING,
+ "limited to UDMA/33 due to 40-wire cable\n");
+ xfer_mask &= ~(0xF8 << ATA_SHIFT_UDMA);
+ }
+
ata_unpack_xfermask(xfer_mask, &dev->pio_mask, &dev->mwdma_mask, &dev->udma_mask); }
Quoted 2 lines
cleaned up somewhat. Also, it's been inadvertently tested (it seems that
pata_amdNvidia cable detection is broken in current -git..)
Yes. The pata_acpi driver fixes that one, but got kicked out as it didn't match other merges at the same time. It'll be back soon
Quoted 2 lines
Signed-off-by Robert Hancock <[email protected]>
Quoted 47 lines · Robert Hancock
Here's a revised version of my previous patch to warn the user if a drive's transfer rate is limited because of a 40-wire cable detection. This one hopefully addresses Alan's previous comments - we now do this at the very end of the function, and the ugly if condition has been cleaned up somewhat. Also, it's been inadvertently tested (it seems that
pata_amdNvidia cable detection is broken in current -git..)Signed-off-by Robert Hancock <[email protected]>---
linux-2.6.20-git6/drivers/ata/libata-core.c2007-02-11
17:31:19.000000000 -0600
+++linux-2.6.20-git6edit/drivers/ata/libata-core.c2007-02-13
19:15:58.000000000 -0600(quoted diff) +1
-13@@ -3305,19 +3305,7 @@ static void ata_dev_xfermask(struct ata_ xfer_mask = ata_pack_xfermask(ap->pio_mask, ap->mwdma_mask, ap->udma_mask); - /* Apply cable rule here. Don't apply it early because when - * we handle hot plug the cable type can itself change. - */ - if (ap->cbl == ATA_CBL_PATA40) - xfer_mask &= ~(0xF8 << ATA_SHIFT_UDMA); - /* Apply drive side cable rule. Unknown or 80 pin cables reported - * host side are checked drive side as well. Cases where we know a - * 40wire cable is used safely for 80 are not checked here. - */ - if (ata_drive_40wire(dev->id) && (ap->cbl == ATA_CBL_PATA_UNK || ap->cbl == ATA_CBL_PATA80)) - xfer_mask &= ~(0xF8 << ATA_SHIFT_UDMA); - - + /* drive modes available */ xfer_mask &= ata_pack_xfermask(dev->pio_mask, dev->mwdma_mask, dev->udma_mask);xfer_mask &= ata_id_xfermask(dev->id);(quoted diff) +8
-0@@ -3348,6 +3336,25 @@ static void ata_dev_xfermask(struct ata_ if (ap->ops->mode_filter) xfer_mask = ap->ops->mode_filter(ap, dev, xfer_mask); + /* Apply cable rule here. Don't apply it early because when + * we handle hot plug the cable type can itself change. + * Check this last so that we know if the transfer rate was + * solely limited by the cable. + * Unknown or 80 wire cables reported host side are checked + * drive side as well. Cases where we know a 40wire cable + * is used safely for 80 are not checked here. + */
Review static void ata_dev_xfermask(struct ata_ · line 3356
Quoted 10 lines
(quoted diff) +10
-0+ if (xfer_mask & (0xF8 << ATA_SHIFT_UDMA)) + /* UDMA/44 or higher would be available */ + if((ap->cbl == ATA_CBL_PATA40) || + (ata_drive_40wire(dev->id) && + (ap->cbl == ATA_CBL_PATA_UNK || + ap->cbl == ATA_CBL_PATA80))) { + ata_dev_printk(dev, KERN_WARNING, + "limited to UDMA/33 due to 40-wire cable\n"); + xfer_mask &= ~(0xF8 << ATA_SHIFT_UDMA); + }
it seems broken to manipulate xfer_mask after returning from the driver's ->mode_filter hook.
this patch is more than just a speed-limited warning printk, afaics
Quoted 4 lines
it seems broken to manipulate
xfer_maskafter returning from the driver's ->mode_filter hook.this patch is more than just a speed-limited warning printk, afaics
I actually suggested that order because the only way the printk can be done correctly is for it to be the very last test made. Since the mode filter is not told what mode will be used but just subtracts modes that are not allowed this should be safe.
I don't otherwise see how we deal with the case where a 40wire cable is used and the mode filter function decides itself to drop to UDMA33 or lower due to incompatibilities or errata.
Alan
Quoted 8 lines · Alan Cox
Quoted 3 lines
it seems broken to manipulate
xfer_maskafter returning from the driver's ->mode_filter hook.this patch is more than just a speed-limited warning printk, afaics
I actually suggested that order because the only way the printk can be done correctly is for it to be the very last test made. Since the mode filter is not told what mode will be used but just subtracts modes that are not allowed this should be safe.
OK
Quoted 8 lines · Alan Cox
Quoted 3 lines
it seems broken to manipulate
xfer_maskafter returning from the driver's ->mode_filter hook.this patch is more than just a speed-limited warning printk, afaics
I actually suggested that order because the only way the printk can be done correctly is for it to be the very last test made. Since the mode filter is not told what mode will be used but just subtracts modes that are not allowed this should be safe.
Far better to have a drive which works slowly than one which works unreliably.
Signature · 3 lines
Bill Davidsen <[email protected]> "We have more to fear from the bungling of the incompetent than from the machinations of the wicked." - from Slashdot
Quoted 8 lines · Bill Davidsen
Quoted 5 lines · Alan Cox
Quoted 5 lines
it seems broken to manipulate
xfer_maskafter returning from the driver's ->mode_filter hook.this patch is more than just a speed-limited warning printk, afaics
Quoted 11 lines
Quoted 6 lines
I actually suggested that order because the only way the printk can be done correctly is for it to be the very last test made. Since the mode filter is not told what mode will be used but just subtracts modes that are not allowed this should be safe.
Far better to have a drive which works slowly than one which works unreliably.
That would be true if the 40 wire detection was 100% accurate!
Signature · 3 lines
"They that give up essential liberty to obtain temporary safety, deserve neither liberty nor safety." (Ben Franklin) "The course of history shows that as a government grows, liberty decreases." (Thomas Jefferson)
Quoted 15 lines · Stephen Clark
Quoted 13 lines · Bill Davidsen
Quoted 9 lines · Alan Cox
Quoted 4 lines
it seems broken to manipulate
xfer_maskafter returning from the driver's ->mode_filter hook.this patch is more than just a speed-limited warning printk, afaics
I actually suggested that order because the only way the printk can be done correctly is for it to be the very last test made. Since the mode filter is not told what mode will be used but just subtracts modes that are not allowed this should be safe.
Quoted 5 lines
Quoted 4 lines
Far better to have a drive which works slowly than one which works unreliably.
That would be true if the 40 wire detection was 100% accurate!
The statement is completely correct, even though the detection may not be. ;-)
With the current set(s) of patches to do better detection, cable evaluation should be better. But even if not, a slow system is more useful than one which doesn't work, crashes because of swap i/o errors, etc.
Signature · 3 lines
bill davidsen <[email protected]> CTO TMR Associates, Inc Doing interesting things with small computers since 1979
Quoted 31 lines · Bill Davidsen
Quoted 28 lines · Stephen Clark
Quoted 24 lines · Bill Davidsen
Quoted 13 lines · Alan Cox
Quoted 6 lines
it seems broken to manipulate
xfer_maskafter returning from the driver's ->mode_filter hook.this patch is more than just a speed-limited warning printk, afaics
I actually suggested that order because the only way the printk can be done correctly is for it to be the very last test made. Since the mode filter is not told what mode will be used but just subtracts modes that are not allowed this should be safe.
Far better to have a drive which works slowly than one which works unreliably.
Quoted 9 lines
Quoted 3 lines
That would be true if the 40 wire detection was 100% accurate!
The statement is completely correct, even though the detection may not be. ;-)
With the current set(s) of patches to do better detection, cable evaluation should be better. But even if not, a slow system is more useful than one which doesn't work, crashes because of swap i/o errors, etc.
I have had problems with cable detection on my previous laptop and my current laptop. It almost made my systems unusable. On my current laptop I was getting a thruput of a little over 1 mbps instead of the 44 mbps I get with udma set to the correct value. It took hours to upgrade my laptop from fc5 to fc6 because of this mis detection.
Signature · 3 lines
"They that give up essential liberty to obtain temporary safety, deserve neither liberty nor safety." (Ben Franklin) "The course of history shows that as a government grows, liberty decreases." (Thomas Jefferson)
Quoted 31 lines · Stephen Clark
Quoted 29 lines · Bill Davidsen
Quoted 17 lines · Stephen Clark
Quoted 14 lines · Bill Davidsen
Quoted 10 lines · Alan Cox
Quoted 4 lines
it seems broken to manipulate
xfer_maskafter returning from the driver's ->mode_filter hook.this patch is more than just a speed-limited warning printk, afaics
I actually suggested that order because the only way the printk can be done correctly is for it to be the very last test made. Since the mode filter is not told what mode will be used but just subtracts modes that are not allowed this should be safe.
Quoted 7 lines
Quoted 5 lines
Far better to have a drive which works slowly than one which works unreliably.
That would be true if the 40 wire detection was 100% accurate!
The statement is completely correct, even though the detection may not be. ;-)
Quoted 9 lines
Quoted 4 lines
With the current set(s) of patches to do better detection, cable evaluation should be better. But even if not, a slow system is more useful than one which doesn't work, crashes because of swap i/o errors, etc.
I have had problems with cable detection on my previous laptop and my current laptop. It almost made my systems unusable. On my current laptop I was getting a thruput of a little over 1 mbps instead of the 44 mbps I get with udma set to the correct value. It took hours to upgrade my laptop from fc5 to fc6 because of this mis detection.
As far as I can see, if you are getting that low a speed, you have other problems. I have a system with old slow drives which are really on a 40 pin cable, and they run at UDMA(33). One of the experts in this can undoubtedly tell us more, but your system should run faster than that, mine does, and I really HAVE a 40 pin cable (and drive).
If your system drops to PIO modes, I doubt cable is the only issue, I think there are other issues (acpi comes to mind).
Signature · 3 lines
bill davidsen <[email protected]> CTO TMR Associates, Inc Doing interesting things with small computers since 1979