Messages in this thread Patch in this message | | | Date | Fri, 25 Jan 2008 12:35:55 +0900 | From | Tejun Heo <> | Subject | Re: [patch] PCI: modify SB700 SATA MSI quirk |
| |
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 <shane.huang@amd.com>
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
| |