Re: [PATCH] ide: Fix ata_id_has_dword_io to return DWORD I/O support properly

Bartlomiej Zolnierkiewicz
2008-12-18T20:45:50Z

On Thursday 18 December 2008, Sergei Shtylyov wrote:
> Hello.
>
> Jeff Garzik wrote:
>
> >> This patch fixes ata_id_has_dword_io to return 1 (supported) if
> >> the drive is compliant to ATA2 or newer and evaluates the config
> >> word for older drives.
>
> >> Signed-off-by: Mario Schwalbe <schwalbe@inf.tu-dresden.de>
>
> >> diff --git a/include/linux/ata.h b/include/linux/ata.h
> >> index a53318b..d0d6a9c 100644
> >> --- a/include/linux/ata.h
> >> +++ b/include/linux/ata.h
> >> @@ -700,12 +700,12 @@ static inline int ata_id_has_tpm(const u16 *id)
>
> >> static inline int ata_id_has_dword_io(const u16 *id)
> >> {
> >> - /* ATA 8 reuses this flag for "trusted" computing */
> >> - if (ata_id_major_version(id) > 7)
> >> - return 0;
> >> - if (id[ATA_ID_DWORD_IO] & (1 << 0))
> >> - return 1;
> >> - return 0;
> >> + /* This flag is defined up to ATA1 and deprecated since then
> >> + (ATA 8 reuses this flag for "trusted" computing). */
> >> + if (ata_id_major_version(id) <= 1)
> >> + return (id[ATA_ID_DWORD_IO] & (1 << 0)) != 0;
> >> + /* later revision drives support DWORD I/O just fine */
> >> + return 1;
>
> > This seems like a risky assumption...
>
> Not at all, I think. It's total mystery how "DWORD I/O" could've been even
> (not) supported by a *drive* at all. Support of "DWORD I/O" is a property of
> the PATA controller. Frankly speaking, I don't think that this function is at
> all useful...

Indeed, this function can be probably removed altogether later...

OTOH the current patch is safe even for 2.6.28 (based on years of experience
with the check that we had in IDE subsystem) and will fix some libata drivers
(pata_legacy, pata_qdi and pata_winbond) to use dword IO on >= ATA-2 devices.

[ In reality this a regression fix for IDE -> libata conversion as it is a
huge performance improvement for the above mentioned DMA-less drivers. ]

Jeff, I would like to merge it through IDE tree since the other patch depends
on it but if you want to go ahead and push it to Linus earlier feel free to
do it (or I can include it into the next IDE fixes pull request if you like).

Thanks,
Bart