LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Sergei Shtylyov <sshtylyov@ru.mvista.com>
To: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Cc: linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 14/15] ide: rework the code for selecting the best DMA transfer mode
Date: Thu, 19 Apr 2007 23:41:05 +0400	[thread overview]
Message-ID: <4627C5D1.5010602@ru.mvista.com> (raw)
In-Reply-To: <4627C386.9020407@ru.mvista.com>

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


  reply	other threads:[~2007-04-19 19:40 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-02-03 13:53 [PATCH 11/15] ide: make ide_hwif_t.ide_dma_{host_off,off_quietly} void Bartlomiej Zolnierkiewicz
2007-02-03 13:53 ` [PATCH 12/15] ide: make ide_hwif_t.ide_dma_host_on void Bartlomiej Zolnierkiewicz
2007-02-19 22:20   ` Olaf Hering
2007-02-19 23:21     ` Bartlomiej Zolnierkiewicz
2007-02-03 13:53 ` [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks Bartlomiej Zolnierkiewicz
2007-02-03 13:53 ` [PATCH 14/15] ide: rework the code for selecting the best DMA transfer mode Bartlomiej Zolnierkiewicz
2007-04-19 19:31   ` Sergei Shtylyov
2007-04-19 19:41     ` Sergei Shtylyov [this message]
2007-04-19 19:46       ` Sergei Shtylyov
2007-04-20 15:03       ` Sergei Shtylyov
2007-04-20 18:00         ` Sergei Shtylyov
2007-04-23 22:25           ` Bartlomiej Zolnierkiewicz
  -- strict thread matches above, loose matches on Subject: below --
2007-01-19  0:30 [PATCH 0/15] IDE quilt tree updated Bartlomiej Zolnierkiewicz
2007-01-19  0:32 ` [PATCH 14/15] ide: rework the code for selecting the best DMA transfer mode Bartlomiej Zolnierkiewicz
2007-01-22 19:48   ` Sergei Shtylyov
     [not found]     ` <58cb370e0702021207o435f39cdsf3abb0d55829fc45@mail.gmail.com>
2007-02-02 23:57       ` Bartlomiej Zolnierkiewicz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4627C5D1.5010602@ru.mvista.com \
    --to=sshtylyov@ru.mvista.com \
    --cc=bzolnier@gmail.com \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --subject='Re: [PATCH 14/15] ide: rework the code for selecting the best DMA transfer mode' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).