LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Matthew Wilcox <matthew@wil.cx>
To: Greg Kroah-Hartman <gregkh@suse.de>
Cc: linux-pci@atrey.karlin.mff.cuni.cz, Crane Cai <crane.cai@amd.com>,
	linux-kernel@vger.kernel.org, Linus Torvalds <torvalds@osdl.org>,
	Andrew Morton <akpm@osdl.org>
Subject: Re: [PATCH 03/10] PCI: AMD SATA IDE mode quirk
Date: Thu, 21 Feb 2008 21:17:33 -0700	[thread overview]
Message-ID: <20080222041733.GD16995@parisc-linux.org> (raw)
In-Reply-To: <1203637660-7247-3-git-send-email-gregkh@suse.de>

On Thu, Feb 21, 2008 at 03:47:33PM -0800, Greg Kroah-Hartman wrote:
> +static void __devinit quirk_amd_ide_mode(struct pci_dev *pdev)
>  {
> -	/* set sb600 sata to ahci mode */
> -	if ((pdev->class >> 8) == PCI_CLASS_STORAGE_IDE) {
> -		u8 tmp;
> +	/* set sb600/sb700/sb800 sata to ahci mode */
> +	u8 tmp;
>  
> +	pci_read_config_byte(pdev, PCI_CLASS_DEVICE, &tmp);
> +	if (tmp == 0x01) {
>  		pci_read_config_byte(pdev, 0x40, &tmp);

This seems like a dis-improvement.  Why are we reading a config byte for
something we already have in the pci_dev?  Why are we now checking
against 0x01 instead of a symbolic constant?  Why are we no longer
checking that this is PCI_BASE_CLASS_STORAGE?

> -DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_IXP600_SATA, quirk_sb600_sata);
> -DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_IXP700_SATA, quirk_sb600_sata);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_IXP600_SATA, quirk_amd_ide_mode);
> +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_IXP600_SATA, quirk_amd_ide_mode);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_IXP700_SATA, quirk_amd_ide_mode);
> +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_IXP700_SATA, quirk_amd_ide_mode);

Nothing in the changelog entry suggests why we now need FIXUP_RESUME
entries when we didn't before.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

  parent reply	other threads:[~2008-02-22  4:17 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-21 23:46 [GIT PATCH] PCI fixes for 2.6.25-rc2 git tree Greg KH
     [not found] ` <1203637660-7247-3-git-send-email-gregkh@suse.de>
2008-02-22  4:17   ` Matthew Wilcox [this message]
2008-02-22  5:49     ` [PATCH 03/10] PCI: AMD SATA IDE mode quirk Cai, Crane
2008-02-22 11:11       ` Matthew Wilcox
2008-02-25  1:43         ` Cai, Crane
2008-02-25  7:31           ` Grant Grundler
2008-02-25  8:00             ` Cai, Crane

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=20080222041733.GD16995@parisc-linux.org \
    --to=matthew@wil.cx \
    --cc=akpm@osdl.org \
    --cc=crane.cai@amd.com \
    --cc=gregkh@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@atrey.karlin.mff.cuni.cz \
    --cc=torvalds@osdl.org \
    --subject='Re: [PATCH 03/10] PCI: AMD SATA IDE mode quirk' \
    /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).