[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4627C5D1.5010602@ru.mvista.com>
Date: Thu, 19 Apr 2007 23:41:05 +0400
From: Sergei Shtylyov <sshtylyov@...mvista.com>
To: Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
CC: linux-ide@...r.kernel.org, linux-kernel@...r.kernel.org
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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists