Messages in this thread | | | Date | Mon, 30 Aug 2004 12:06:58 -0400 | From | Jeff Garzik <> | Subject | Re: [PATCH] libata ATA vs SATA detection and workaround. |
| |
Brad Campbell wrote: > + /* limit bridge transfers to udma5, 200 sectors */ > + if ((ap->cbl == ATA_CBL_SATA) && (!ata_id_is_sata(ap->device))) { > + printk(KERN_INFO "ata%u(%u): applying bridge limits\n", > + ap->id, ap->device->devno); > + ap->udma_mask &= ATA_UDMA5; > + ap->host->max_sectors = ATA_MAX_SECTORS; > + ap->host->hostt->max_sectors = ATA_MAX_SECTORS; > + ap->device->flags |= ATA_DFLAG_LOCK_SECTORS; > + } > if (ap->ops->dev_config) > ap->ops->dev_config(ap, &ap->device[i]);
Close! Please move the entire quoted section, including the two lines of code calling ->dev_config(), into a new function 'ata_dev_config'. Export it (bottom of libata-core.c) and prototype it (libata.h) as well.
I'm still pondering what Alan was hinting at, a bit. You (Brad) are correct in pointing out that this code should only trigger for the correct situations (lba48, etc.) which are only present on modern drives, but... there is still a chance that word 93 will be zero on some weird (probably non-compliant) device.
However, Alan's comment is actually more relevant for unrelated sections of libata. Whenever we test a feature bit in words 82-87, we should check for "word != 0 && word != 0xffff" which is how one knows the word is implemented. There are no feature bits indicating that feature bits exist :)
Jeff
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |