Hello. Bartlomiej Zolnierkiewicz wrote:
This patch allows users to override both host and device side cable detection with "ideX=ata66" kernel parameter. Thanks to this it should be now possible to use UDMA > 2 modes on systems (laptops mainly) which use short 40-pin cable instead of 80-pin one.
Next patches add automatic detection of some systems using short cables.
Changes:
* Rename hwif->udma_four to hwif->cbl and make it u8.
Not sure if already short word "cable" was worth further shortening. :-)
The name/usage is the same as in libata.
When we add ->cable_detect method it will make grepping easier. 8)
Hm... :-)
Index: b/drivers/ide/pci/alim15x3.c =================================================================== --- a/drivers/ide/pci/alim15x3.c +++ b/drivers/ide/pci/alim15x3.c @@ -594,7 +594,7 @@ out: * FIXME: frobs bits that are not defined on newer ALi devicea */ -static unsigned int __devinit ata66_ali15x3 (ide_hwif_t *hwif) +static u8 __devinit ata66_ali15x3(ide_hwif_t *hwif) { struct pci_dev *dev = hwif->pci_dev; unsigned int ata66 = 0; @@ -657,7 +657,7 @@ static unsigned int __devinit ata66_ali1 local_irq_restore(flags); - return(ata66); + return ata66 ? ATA_CBL_PATA80 : ATA_CBL_PATA40;
Ahem... I'd think it was about the right time to fix the abomination which those ata66 and cable_80_pin[2] are, something like this:
static unsigned int __devinit ata66_ali15x3 (ide_hwif_t *hwif) { struct pci_dev *dev = hwif->pci_dev; unsigned int cbl = ATA_CBL_PATA40; unsigned long flags; u8 tmpbyte, mask = hwif->channel ? 0x02 : 0x01; local_irq_save(flags); /* Not sure if it's necessary... */ if (m5229_revision >= 0xC2) { /* * Ultra66 cable detection (from Host View) * m5229, 0x4a, bit0: primary, bit1: secondary * 0: 80 pin, 1: 40 pin */ pci_read_config_byte(dev, 0x4a, &tmpbyte); /* * Allow ata66 if cable of current channel has 80 pins */ cbl = (tmpbyte & mask) ? ATA_CBL_PATA40 : ATA_CBL_PATA80; } else {
[Following code frankly speaking has no business being in this function]
patch #3/5 already takes care of cable_80_pin[2] part,
I really should have looked thru all the series but lacked the time. :-<
ata66 part was left as an exercise for the reader ;-)
Hm, now that I've looked at that patch I saw both these things killed. What, you don't know your own code? ;-)
local_irq_restore(flags); /* Not sure if it's necessary... */
return cbl;
}
/** Index: b/drivers/ide/pci/serverworks.c =================================================================== --- a/drivers/ide/pci/serverworks.c +++ b/drivers/ide/pci/serverworks.c @@ -329,9 +329,9 @@ static unsigned int __devinit init_chips return dev->irq; } -static unsigned int __devinit ata66_svwks_svwks (ide_hwif_t *hwif) +static u8 __devinit ata66_svwks_svwks(ide_hwif_t *hwif) { - return 1; + return ATA_CBL_PATA80; }
Hm, worth folding into ata66_svwks()...
Yes and no...
Alan did a very nice rewrite of cable detection code for pata_serverworks.c driver. We may be better off just back-porting it (patches are welcomed).
Thanks, no. I still have much to do. :-)
Index: b/drivers/ide/pci/sis5513.c =================================================================== --- a/drivers/ide/pci/sis5513.c +++ b/drivers/ide/pci/sis5513.c @@ -796,7 +796,7 @@ static unsigned int __devinit init_chips return 0; } -static unsigned int __devinit ata66_sis5513 (ide_hwif_t *hwif) +static u8 __devinit ata66_sis5513(ide_hwif_t *hwif) { u8 ata66 = 0; @@ -811,7 +811,8 @@ static unsigned int __devinit ata66_sis5 pci_read_config_byte(hwif->pci_dev, 0x48, ®48h); ata66 = (reg48h & mask) ? 0 : 1; } - return ata66; + + return ata66 ? ATA_CBL_PATA80 : ATA_CBL_PATA40; }
That doesn't look good as well. I think we should part with 'ata66' variable here completely...
Could be fixed in the follow-up patch as we should also split ata66_sis5513() into two separate functions (one for chipset_family >= ATA_133 and one for chipset_family >= ATA_66).
That's only worth doing if you actually intend to introduce cable_detect() method...
Thanks, Bart
MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
