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 13/15] ide: fix UDMA/MWDMA/SWDMA masks
Date: Mon, 22 Jan 2007 19:19:13 +0300	[thread overview]
Message-ID: <45B4E401.6070208@ru.mvista.com> (raw)
In-Reply-To: <20070119003226.14846.87052.sendpatchset@localhost.localdomain>

Hello.

Bartlomiej Zolnierkiewicz wrote:

> [PATCH] ide: fix UDMA/MWDMA/SWDMA masks

> * use 0x00 instead of 0x80 to disable ->{ultra,mwdma,swdma}_mask
> * add udma_mask field to ide_pci_device_t and use it to initialize
>   ->ultra_mask in aec62xx, pdc202xx_new and pdc202xx_old drivers
> * fix UDMA masks to match with chipset specific *_ratemask()
>   (alim15x3, hpt366, serverworks and siimage drivers need UDMA mask
>    filtering method - done in the next patch)

    More nit picking (-:

> Index: b/drivers/ide/pci/cmd64x.c
> ===================================================================
> --- a/drivers/ide/pci/cmd64x.c
> +++ b/drivers/ide/pci/cmd64x.c
> @@ -695,9 +695,10 @@ static void __devinit init_hwif_cmd64x(i
>  	hwif->swdma_mask = 0x07;
>  
>  	if (dev->device == PCI_DEVICE_ID_CMD_643)
> -		hwif->ultra_mask = 0x80;
> +		hwif->ultra_mask = 0x00;
>  	if (dev->device == PCI_DEVICE_ID_CMD_646)
> -		hwif->ultra_mask = (class_rev > 0x04) ? 0x07 : 0x80;
> +		hwif->ultra_mask =
> +			(class_rev == 0x05 || class_rev == 0x07) ? 0x07 : 0x00;
>  	if (dev->device == PCI_DEVICE_ID_CMD_648)
>  		hwif->ultra_mask = 0x1f;

    Hm, well, this doesn't look consistent with the changes in other drivers.
This driver asks for explicit hwif->cds->ultra_mask initializers, IMO...
    You'd only have to check for PCI-646 revisions < 5 then...

> Index: b/drivers/ide/pci/piix.c
> ===================================================================
> --- a/drivers/ide/pci/piix.c
> +++ b/drivers/ide/pci/piix.c
> @@ -493,7 +493,7 @@ static void __devinit init_hwif_piix(ide
>  		case PCI_DEVICE_ID_INTEL_82371FB_0:
>  		case PCI_DEVICE_ID_INTEL_82371FB_1:
>  		case PCI_DEVICE_ID_INTEL_82371SB_1:
> -			hwif->ultra_mask = 0x80;
> +			hwif->ultra_mask = 0x00;
>  			break;
>  		case PCI_DEVICE_ID_INTEL_82371AB:
>  		case PCI_DEVICE_ID_INTEL_82443MX_1:
> @@ -501,6 +501,10 @@ static void __devinit init_hwif_piix(ide
>  		case PCI_DEVICE_ID_INTEL_82801AB_1:
>  			hwif->ultra_mask = 0x07;
>  			break;
> +		case PCI_DEVICE_ID_INTEL_82801AA_1:
> +		case PCI_DEVICE_ID_INTEL_82372FB_1:
> +			hwif->ultra_mask = 0x1f;
> +			break;

    Alas, I'm afraid this part is wrong!
    At least, the cable detection should work for 82801AA the same way as for 
the 82801Bx and newer chips, if Intel's datasheet is to be trusted... I think 
we should fall thru here.

>  		default:
>  			if (!hwif->udma_four)
>  				hwif->udma_four = piix_cable_detect(hwif);

    This one also certainly asks for explicit hwif->cds->ultra_mask 
initializers... Thus almost all of this switch statement could go away...

MBR, Sergei

  reply	other threads:[~2007-01-22 16:19 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-01-19  0:30 [PATCH 0/15] IDE quilt tree updated Bartlomiej Zolnierkiewicz
2007-01-19  0:31 ` [PATCH 1/15] ACPI support for IDE devices Bartlomiej Zolnierkiewicz
2007-01-19  0:31 ` [PATCH 2/15] via82cxxx/pata_via: correct PCI_DEVICE_ID_VIA_SATA_EIDE ID and add support for CX700 and 8237S Bartlomiej Zolnierkiewicz
2007-01-19  0:31 ` [PATCH 3/15] it8213: fix build and ->ultra_mask Bartlomiej Zolnierkiewicz
2007-01-19  0:31 ` [PATCH 4/15] ide: convert ide_hwif_t.mmio into flag Bartlomiej Zolnierkiewicz
2007-01-19  0:31 ` [PATCH 5/15] hpt34x: hpt34x_tune_chipset() (->speedproc) fix Bartlomiej Zolnierkiewicz
2007-01-19  0:31 ` [PATCH 6/15] atiixp/jmicron/triflex: fix PIO fallback Bartlomiej Zolnierkiewicz
2007-01-19  0:31 ` [PATCH 7/15] piix: cleanup Bartlomiej Zolnierkiewicz
2007-01-19  0:31 ` [PATCH 8/15] ide: disable DMA in ->ide_dma_check for "no IORDY" case Bartlomiej Zolnierkiewicz
2007-01-19 16:26   ` Sergei Shtylyov
     [not found]     ` <58cb370e0701191047h524434eobdb9d86ed614bc71@mail.gmail.com>
2007-01-19 19:11       ` Bartlomiej Zolnierkiewicz
2007-01-19 19:35         ` Sergei Shtylyov
     [not found]           ` <58cb370e0701191200i10119313i4aacae9c504a02e4@mail.gmail.com>
2007-01-19 21:43             ` Bartlomiej Zolnierkiewicz
2007-01-20 17:09               ` Sergei Shtylyov
     [not found]                 ` <58cb370e0701200947p578f4d74g1fee511ded6c9a77@mail.gmail.com>
2007-01-20 18:21                   ` Bartlomiej Zolnierkiewicz
2007-01-20 18:41                     ` Sergei Shtylyov
2007-01-25 17:03                     ` Sergei Shtylyov
2007-01-19  0:32 ` [PATCH 9/15] sgiioc4: fix sgiioc4_ide_dma_check() to enable/disable DMA properly Bartlomiej Zolnierkiewicz
2007-01-19  0:32 ` [PATCH 10/15] ide: add ide_set_dma() helper Bartlomiej Zolnierkiewicz
2007-01-20 20:22   ` Sergei Shtylyov
2007-01-19  0:32 ` [PATCH 11/15] ide: make ide_hwif_t.ide_dma_{host_off,off_quietly} void Bartlomiej Zolnierkiewicz
2007-01-20 20:56   ` Sergei Shtylyov
     [not found]     ` <58cb370e0702021206h205ff983r88fb4bdbea42ed9f@mail.gmail.com>
2007-02-02 22:39       ` Bartlomiej Zolnierkiewicz
2007-01-19  0:32 ` [PATCH 12/15] ide: make ide_hwif_t.ide_dma_host_on void Bartlomiej Zolnierkiewicz
2007-01-20 20:46   ` Sergei Shtylyov
     [not found]     ` <58cb370e0702021128g15cac87ar507c20e78ded9464@mail.gmail.com>
2007-02-02 21:16       ` Bartlomiej Zolnierkiewicz
2007-03-26 17:19   ` Sergei Shtylyov
2007-04-20 20:36     ` Sergei Shtylyov
2007-01-19  0:32 ` [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks Bartlomiej Zolnierkiewicz
2007-01-22 16:19   ` Sergei Shtylyov [this message]
     [not found]     ` <58cb370e0702021606v4efa6143lf060e6aab9782c35@mail.gmail.com>
2007-02-03  0:39       ` Bartlomiej Zolnierkiewicz
2007-02-03 16:30         ` Sergei Shtylyov
2007-01-22 18:17   ` Sergei Shtylyov
2007-01-22 18:46     ` Alan
2007-01-22 20:28       ` Sergei Shtylyov
2007-01-22 21:31         ` Alan
2007-01-22 22:39           ` Jeff Garzik
2007-01-23 14:51           ` Sergei Shtylyov
2007-01-25 15:40             ` Sergei Shtylyov
2007-01-31 20:38       ` Sergei Shtylyov
2007-01-31 23:24         ` Alan
     [not found]     ` <58cb370e0702021606m4eb6f682xfa4bf769d398cf9@mail.gmail.com>
2007-02-03  0:50       ` 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
2007-01-19  0:32 ` [PATCH 15/15] ide: add ide_tune_dma() helper Bartlomiej Zolnierkiewicz
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 13/15] ide: fix UDMA/MWDMA/SWDMA masks Bartlomiej Zolnierkiewicz
2007-02-03 17:24 Bartlomiej Zolnierkiewicz
2007-04-22 16:19 ` Sergei Shtylyov

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=45B4E401.6070208@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 13/15] ide: fix UDMA/MWDMA/SWDMA masks' \
    /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).