lkml.org 
[lkml]   [2007]   [Apr]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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/

    \
     
     \ /
      Last update: 2007-04-19 21:43    [W:0.033 / U:29.632 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site