Re: [PATCH] PCI: modify SB700 SATA MSI quirk

2008-02-01 Thread Jeff Garzik

Tejun Heo wrote:

From: Shane Huang <[EMAIL PROTECTED]>

SB700 SATA MSI bug will be fixed in SB700 revision A21 at hardware
level, but the SB700 revision older than A21 will also be found in the
market.  This patch modify the original quirk commit
bc38b411fe696fad32b261f492cb4afbf1835256 instead of withdrawing it.
The patch also removes quirk to 0x4395 because 0x4395 is SB800 device
ID.

Signed-off-by: Shane Huang <[EMAIL PROTECTED]>
Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
Okay, here's reformatted in-line version.  Shane, please invest some
time into setting up email environment.  Sending patches via email is
an important part of the linux kernel development process and if
you're gonna submit patches, you're just gonna have to do it.

 drivers/pci/quirks.c |   29 ++---
 1 file changed, 22 insertions(+), 7 deletions(-)


FWIW, I'm happy with whatever this thread results in...   it sounds like 
Tejun and Shane are iterating towards a satisfactory final result.


Just let me know if I need to merge something, since I'm assuming that 
GregKH will push this through the PCI tree.



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH] PCI: modify SB700 SATA MSI quirk

2008-01-24 Thread Shane Huang

 Hi Tejun:


> Okay, here's reformatted in-line version.  Shane, please invest some
> time into setting up email environment.  Sending patches via email is
> an important part of the linux kernel development process and if
> you're gonna submit patches, you're just gonna have to do it.


Right. In fact, I tried to modify some settings before I sent my mail,
but it seems that it does not take effect. I will check it again. Sorry
for that
and thank you for your help~~


Thanks
Shane


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] PCI: modify SB700 SATA MSI quirk

2008-01-24 Thread Greg KH
On Fri, Jan 25, 2008 at 11:48:29AM +0800, Shane Huang wrote:
> Hi Tejun:
> 
> 
> > You need to merge the above two messages into one patch description.
> > 
> > After S-O-B, you can put --- and between it and the patch 
> > body, you can say things which you wanna mention but don't 
> > think should be included in commit message.
> 
> OK, I'll have to submit another update patch later.

I recommend running the scripts/checkpatch.pl script on any proposed
patches like this before you send them.  It will find a lot of these
problems for you :)

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [patch] PCI: modify SB700 SATA MSI quirk

2008-01-24 Thread Shane Huang
Hi Tejun:


> You need to merge the above two messages into one patch description.
> 
> After S-O-B, you can put --- and between it and the patch 
> body, you can say things which you wanna mention but don't 
> think should be included in commit message.

OK, I'll have to submit another update patch later.




Thanks
Shane


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] PCI: modify SB700 SATA MSI quirk

2008-01-24 Thread Tejun Heo
Shane Huang wrote:
> I did some modification to this patch and send it again, Please check
> it.
> The quirk to 0x4395 has been removed because 0x4395 only belongs to
> SB800.
> 
>> SB700 SATA MSI bug will be fixed in SB700 revision A21 at 
>> hardware level,
>> but the SB700 revision older than A21 will also be found in 
>> the market.
>> This patch modify the original quirk commit
>> bc38b411fe696fad32b261f492cb4afbf1835256 instead of withdrawing it.

You need to merge the above two messages into one patch description.

>> Signed-off-by: Shane Huang <[EMAIL PROTECTED]>

After S-O-B, you can put --- and between it and the patch body, you can
say things which you wanna mention but don't think should be included in
commit message.

> +static void __devinit quirk_msi_intx_disable_ati_bug(struct pci_dev
> *dev)
> +{
> + struct pci_dev *p;
> + u8 rev = 0;
> +
> + /* SB700 MSI issue will be fixed at HW level from revision A21,
> +  * we need check PCI REVISION ID of SMBus controller to get
> SB700
> +  * revision.
> +  */
> + p = pci_get_device(PCI_VENDOR_ID_ATI,
> PCI_DEVICE_ID_ATI_SBX00_SMBUS,
> +NULL);
> + if (p) {
> + pci_read_config_byte(p, PCI_CLASS_REVISION, &rev);

Please drop unnecessary braces and you can you p->revision.

> + }
> + if ((rev < 0x3B) && (rev >= 0x30)) {
> + dev->dev_flags |= PCI_DEV_FLAGS_MSI_INTX_DISABLE_BUG;
> + }

Hmm... So, if there's no SMBUS device the quirk applies.  Is this
intended?  If that can't happen, just do if (!p) return;

Also, pci_get_device() requires matching pci_dev_put().

> +}
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_BROADCOM,
>   PCI_DEVICE_ID_TIGON3_5780,
>   quirk_msi_intx_disable_bug);
> @@ -1729,17 +1747,15 @@
>   quirk_msi_intx_disable_bug);
>  
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x4390,
> - quirk_msi_intx_disable_bug);
> + quirk_msi_intx_disable_ati_bug);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x4391,
> - quirk_msi_intx_disable_bug);
> + quirk_msi_intx_disable_ati_bug);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x4392,
> - quirk_msi_intx_disable_bug);
> + quirk_msi_intx_disable_ati_bug);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x4393,
> - quirk_msi_intx_disable_bug);
> + quirk_msi_intx_disable_ati_bug);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x4394,
> - quirk_msi_intx_disable_bug);
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x4395,
> - quirk_msi_intx_disable_bug);
> + quirk_msi_intx_disable_ati_bug);

You tested this, right?

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [patch] PCI: modify SB700 SATA MSI quirk

2008-01-24 Thread Shane Huang
I did some modification to this patch and send it again, Please check
it.
The quirk to 0x4395 has been removed because 0x4395 only belongs to
SB800.


Thanks
Shane


> -Original Message-
> From: Shane Huang 
> 
> SB700 SATA MSI bug will be fixed in SB700 revision A21 at 
> hardware level,
> but the SB700 revision older than A21 will also be found in 
> the market.
> This patch modify the original quirk commit
> bc38b411fe696fad32b261f492cb4afbf1835256 instead of withdrawing it.
> 
> 
> Signed-off-by: Shane Huang <[EMAIL PROTECTED]>


diff -ruN linux-2.6.24-rc7_org/drivers/pci/quirks.c
linux-2.6.24-rc7_new/drivers/pci/quirks.c
--- linux-2.6.24-rc7_org/drivers/pci/quirks.c   2008-01-23
14:44:53.0 +0800
+++ linux-2.6.24-rc7_new/drivers/pci/quirks.c   2008-01-25
10:55:21.0 +0800
@@ -1709,6 +1709,24 @@
 {
dev->dev_flags |= PCI_DEV_FLAGS_MSI_INTX_DISABLE_BUG;
 }
+static void __devinit quirk_msi_intx_disable_ati_bug(struct pci_dev
*dev)
+{
+   struct pci_dev *p;
+   u8 rev = 0;
+
+   /* SB700 MSI issue will be fixed at HW level from revision A21,
+* we need check PCI REVISION ID of SMBus controller to get
SB700
+* revision.
+*/
+   p = pci_get_device(PCI_VENDOR_ID_ATI,
PCI_DEVICE_ID_ATI_SBX00_SMBUS,
+  NULL);
+   if (p) {
+   pci_read_config_byte(p, PCI_CLASS_REVISION, &rev);
+   }
+   if ((rev < 0x3B) && (rev >= 0x30)) {
+   dev->dev_flags |= PCI_DEV_FLAGS_MSI_INTX_DISABLE_BUG;
+   }
+}
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_BROADCOM,
PCI_DEVICE_ID_TIGON3_5780,
quirk_msi_intx_disable_bug);
@@ -1729,17 +1747,15 @@
quirk_msi_intx_disable_bug);
 
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x4390,
-   quirk_msi_intx_disable_bug);
+   quirk_msi_intx_disable_ati_bug);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x4391,
-   quirk_msi_intx_disable_bug);
+   quirk_msi_intx_disable_ati_bug);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x4392,
-   quirk_msi_intx_disable_bug);
+   quirk_msi_intx_disable_ati_bug);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x4393,
-   quirk_msi_intx_disable_bug);
+   quirk_msi_intx_disable_ati_bug);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x4394,
-   quirk_msi_intx_disable_bug);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x4395,
-   quirk_msi_intx_disable_bug);
+   quirk_msi_intx_disable_ati_bug);
 
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x4373,
quirk_msi_intx_disable_bug);


modify_SB700_MSI_2.patch
Description: modify_SB700_MSI_2.patch


Re: [patch] PCI: modify SB700 SATA MSI quirk

2008-01-24 Thread Tejun Heo
Shane Huang wrote:
> SB700 SATA MSI bug will be fixed in SB700 revision A21 at hardware
> level,
> but the SB700 revision older than A21 will also be found in the market.
> This patch modify the original quirk commit
> bc38b411fe696fad32b261f492cb4afbf1835256 instead of withdrawing it.
> 
> 
> Signed-off-by: Shane Huang <[EMAIL PROTECTED]>
> 
> 
> Since there is some word wrapping problem with my mail client MS
> outlook, I also attach the patch as an attachment. Please check it.

Can you please get a decent email client?  Thunderbird + toggle word
wrap + external editor combination works fine.

> diff -ruN old/drivers/pci/quirks.c new/drivers/pci/quirks.c
> --- old/drivers/pci/quirks.c  2008-01-07 05:45:38.0 +0800
> +++ new/drivers/pci/quirks.c  2008-01-22 11:31:09.0 +0800
> @@ -1709,6 +1709,22 @@
>  {
>   dev->dev_flags |= PCI_DEV_FLAGS_MSI_INTX_DISABLE_BUG;
>  }
> +static void __devinit quirk_msi_intx_disable_ati_bug(struct pci_dev
> *dev)
> +{
> + struct pci_dev *p;
> + u8 rev = 0;
> +
> + /* SB700 MSI issue will be fixed at HW level from revision A21,
> +  * we need check PCI REVISION ID of SMBus controller to get
> SB700 revision.
> +  */
> + p = pci_get_device(PCI_VENDOR_ID_ATI,
> PCI_DEVICE_ID_ATI_SBX00_SMBUS, NULL);

It would be nice if things stay under 80col limit although we don't
follow that too well in quirks.c.

> + if (p!=NULL) {

We usually write 'if (p)' and don't use unless necessary.

> + pci_read_config_byte(p, PCI_CLASS_REVISION, &rev);
> + }
> + if ((rev < 0x3B) && (rev >= 0x30)) {
> + dev->dev_flags |= PCI_DEV_FLAGS_MSI_INTX_DISABLE_BUG;
> + }
> +}
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_BROADCOM,
>   PCI_DEVICE_ID_TIGON3_5780,
>   quirk_msi_intx_disable_bug);
> @@ -1729,17 +1745,17 @@
>   quirk_msi_intx_disable_bug);
>  
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x4390,
> - quirk_msi_intx_disable_bug);
> + quirk_msi_intx_disable_ati_bug);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x4391,
> - quirk_msi_intx_disable_bug);
> + quirk_msi_intx_disable_ati_bug);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x4392,
> - quirk_msi_intx_disable_bug);
> + quirk_msi_intx_disable_ati_bug);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x4393,
> - quirk_msi_intx_disable_bug);
> + quirk_msi_intx_disable_ati_bug);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x4394,
> - quirk_msi_intx_disable_bug);
> + quirk_msi_intx_disable_ati_bug);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x4395,
> - quirk_msi_intx_disable_bug);
> + quirk_msi_intx_disable_ati_bug);

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/