Messages in this thread | | | Date | Thu, 19 Apr 2007 23:41:05 +0400 | From | Sergei Shtylyov <> | Subject | Re: [PATCH 14/15] ide: rework the code for selecting the best DMA transfer mode |
| |
Sergei Shtylyov wrote: > Hello. > > Bartlomiej Zolnierkiewicz wrote: > >> [PATCH] ide: rework the code for selecting the best DMA transfer mode > > >> Depends on the "ide: fix UDMA/MWDMA/SWDMA masks" patch. > > > I'm now trying to rewrite hpt366.c to benefit more from these patches... > and alas, this very patch seems to be breaking filtering (at least) in > this driver. :-] > >> Index: b/drivers/ide/ide-dma.c >> =================================================================== >> --- a/drivers/ide/ide-dma.c >> +++ b/drivers/ide/ide-dma.c >> @@ -705,6 +705,80 @@ int ide_use_dma(ide_drive_t *drive) >> >> EXPORT_SYMBOL_GPL(ide_use_dma); >> >> +static const u8 xfer_mode_bases[] = { >> + XFER_UDMA_0, >> + XFER_MW_DMA_0, >> + XFER_SW_DMA_0, >> +}; >> + >> +static unsigned int ide_get_mode_mask(ide_drive_t *drive, u8 base) >> +{ >> + struct hd_driveid *id = drive->id; >> + ide_hwif_t *hwif = drive->hwif; >> + unsigned int mask = 0; >> + >> + switch(base) { >> + case XFER_UDMA_0: >> + if ((id->field_valid & 4) == 0) >> + break; >> + >> + mask = id->dma_ultra & hwif->ultra_mask; >> + >> + if (hwif->udma_filter) >> + mask &= hwif->udma_filter(drive); >> + >> + if ((mask & 0x78) && (eighty_ninty_three(drive) == 0)) >> + mask &= 0x07;
> Note the subtle difference between the old and new behavior: the old > driver code was applying UltraDMA filter *after* > the cable type limit, and the new code does it *before*.
Was there any particular reason to change that order?
[...]
>> Index: b/drivers/ide/pci/hpt366.c >> =================================================================== >> --- a/drivers/ide/pci/hpt366.c >> +++ b/drivers/ide/pci/hpt366.c >> @@ -513,43 +513,31 @@ static int check_in_drive_list(ide_drive >> return 0; >> } >> >> -static u8 hpt3xx_ratemask(ide_drive_t *drive) >> -{ >> - struct hpt_info *info = pci_get_drvdata(HWIF(drive)->pci_dev); >> - u8 mode = info->max_mode; >> - >> - if (!eighty_ninty_three(drive) && mode) >> - mode = min(mode, (u8)1); >> - return mode; >> -} >> - >> /* >> * Note for the future; the SATA hpt37x we must set >> * either PIO or UDMA modes 0,4,5 >> */ >> - -static u8 hpt3xx_ratefilter(ide_drive_t *drive, u8 speed) >> + >> +static u8 hpt3xx_udma_filter(ide_drive_t *drive) >> { >> struct hpt_info *info = pci_get_drvdata(HWIF(drive)->pci_dev); >> u8 chip_type = info->chip_type; >> - u8 mode = hpt3xx_ratemask(drive); >> - >> - if (drive->media != ide_disk) >> - return min(speed, (u8)XFER_PIO_4); >> + u8 mode = info->max_mode; >> + u8 mask; >> >> switch (mode) { >> case 0x04: >> - speed = min_t(u8, speed, XFER_UDMA_6); >> + mask = 0x7f; >> break; >> case 0x03: >> - speed = min_t(u8, speed, XFER_UDMA_5); >> + mask = 0x3f; >> if (chip_type >= HPT374) >> break; >> if (!check_in_drive_list(drive, bad_ata100_5)) >> goto check_bad_ata33; >> /* fall thru */ >> case 0x02: >> - speed = min_t(u8, speed, XFER_UDMA_4); >> + mask = 0x1f; >> >> /* >> * CHECK ME, Does this need to be changed to HPT374 ?? >> @@ -560,13 +548,13 @@ static u8 hpt3xx_ratefilter(ide_drive_t >> !check_in_drive_list(drive, bad_ata66_4)) >> goto check_bad_ata33; >> >> - speed = min_t(u8, speed, XFER_UDMA_3); >> + mask = 0x0f; >> if (HPT366_ALLOW_ATA66_3 && >> !check_in_drive_list(drive, bad_ata66_3)) >> goto check_bad_ata33; >> /* fall thru */ >> case 0x01: >> - speed = min_t(u8, speed, XFER_UDMA_2); >> + mask = 0x07; >> >> check_bad_ata33: >> if (chip_type >= HPT370A)
> This case 0x01 will *never* be hit for HPT370 chip with the new code, > therefore the filter won't get applied.
Oh, and for HPT36x chips used with 40c cable too (unless they're artificaially reduced to UltraDMA/33 by the driver #define's).
>> @@ -576,10 +564,10 @@ static u8 hpt3xx_ratefilter(ide_drive_t >> /* fall thru */ >> case 0x00: >> default: >> - speed = min_t(u8, speed, XFER_MW_DMA_2); >> + mask = 0x00; >> break;
Well, that case 0x00 should never actually happen.
>> } >> - return speed; >> + return mask; >> } > > [...] > >> @@ -1270,6 +1272,7 @@ static void __devinit init_hwif_hpt366(i >> hwif->intrproc = &hpt3xx_intrproc; >> hwif->maskproc = &hpt3xx_maskproc; >> hwif->busproc = &hpt3xx_busproc; >> + hwif->udma_filter = &hpt3xx_udma_filter; > > > In fact, we only need a filter for HPT36x/370 chips -- I'll address it > in my patch.
>> /* >> * HPT3xxN chips have some complications:
MBR, Sergei
- 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/
| |