[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4627C386.9020407@ru.mvista.com>
Date: Thu, 19 Apr 2007 23:31:18 +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
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*.
> + break;
> + case XFER_MW_DMA_0:
> + mask = id->dma_mword & hwif->mwdma_mask;
> + break;
> + case XFER_SW_DMA_0:
> + mask = id->dma_1word & hwif->swdma_mask;
> + break;
> + default:
> + BUG();
> + break;
> + }
> +
> + return mask;
> +}
> +
> +/**
> + * ide_max_dma_mode - compute DMA speed
> + * @drive: IDE device
> + *
> + * Checks the drive capabilities and returns the speed to use
> + * for the DMA transfer. Returns 0 if the drive is incapable
> + * of DMA transfers.
> + */
> +
> +u8 ide_max_dma_mode(ide_drive_t *drive)
> +{
> + ide_hwif_t *hwif = drive->hwif;
> + unsigned int mask;
> + int x, i;
> + u8 mode = 0;
> +
> + if (drive->media != ide_disk && hwif->atapi_dma == 0)
> + return 0;
> +
> + for (i = 0; i < ARRAY_SIZE(xfer_mode_bases); i++) {
> + mask = ide_get_mode_mask(drive, xfer_mode_bases[i]);
> + x = fls(mask) - 1;
> + if (x >= 0) {
> + mode = xfer_mode_bases[i] + x;
> + break;
> + }
> + }
> +
> + printk(KERN_DEBUG "%s: selected mode 0x%x\n", drive->name, mode);
> +
> + return mode;
> +}
> +
> +EXPORT_SYMBOL_GPL(ide_max_dma_mode);
> +
> void ide_dma_verbose(ide_drive_t *drive)
> {
> struct hd_driveid *id = drive->id;
[...]
> 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.
> @@ -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;
> }
> - 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:
WBR, 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