On Saturday 25 August 2007, Sergei Shtylyov wrote:
> Hello.
>
> Bartlomiej Zolnierkiewicz wrote:
>
> >>The Marvell bridge chips used on HighPoint SATA cards do not seem to support
> >>the UltraDMA modes 1, 2, and 3 (as well as any MWDMA modes), so the driver
> >>needs to account for this in the udma_filter() method. In order to achieve
> >>that, do the following changes:
>
> >>- install the method for all chips, not only HPT36x/370 (improve code formatting
> >> by killing an extra tabs while at it);
>
> >>- add to the end of the 'switch' statement in hpt3xx_udma_filter() case for
> >> HPT372[AN] and HPT374 chips upon which the SATA cards are based and check
> >> there whether we're dealing with SATA drive (by looking at words 80 and 93
> >> of the drive's identify data), reorder HPT370[A] cases for consistency...
>
> >>Signed-off-by: Sergei Shtylyov <sshtylyov@xxxxxxxxxxxxx>
>
> [...]
>
> > Also now that ->udma_filter is always present the initial hwif->ultra_mask
>
> Aha, so this method's semantics intended to *completely override* the
> ultra_mask field?! Wouldn't it be better to make the code behave more
> consistent, i.e. in ide_get_mode_mask() do:
>
> unsigned int mask = 0;
>
> switch(base) {
> case XFER_UDMA_0:
> if ((id->field_valid & 4) == 0)
> break;
>
> if (hwif->udma_filter)
> mask = hwif->udma_filter(drive);
> else
> mask = hwif->ultra_mask;
>
> mask &= id->dma_ultra;
> if ((mask & 0x78) && (eighty_ninty_three(drive) == 0))
> mask &= 0x07;
> break;
> case XFER_MW_DMA_0:
> if ((id->field_valid & 2) == 0)
> break;
>
> if (hwif->mdma_filter)
> mask = hwif->mdma_filter(drive);
> else
> mask = hwif->mwdma_mask;
> mask &= id->dma_mword;
> break;
>
> to avoid the further confusion? ;-)
Fine with me but you forgot to attach a patch. ;)
Bart
-
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
