Re: [-mm patch] drivers/pci/quirks.c: cleanup
Hi Mark, On Tue, 9 Jan 2007 22:58:20 -0500, Mark M. Hoffman wrote: > * Jean Delvare <[EMAIL PROTECTED]> [2007-01-09 14:17:21 +0100]: > > I am worried about the Intel/Asus SMBus quirk then, which affects many > > more users than the SiS SMBus one, and would suffer from a reordering as > > well. > > Intel/Asus users on FC[456] would surely have screamed if that was true. But > I > was curious so I looked deeper. There is a fundamental difference between the > Intel SMBus quirks and the SiS SMBus quirk... > > Intel: > 1) The first quirk keys off the host bridge, setting a flag. > 2) The second quirk keys off the LPC, enabling the SMBus if the flag was set. > > SiS: > 1) The first quirk keys off the *old* LPC ID... this causes the ID to > change.[1] > 2) The second quirk keys off the *new* LPC ID; this one enables the SMBus. > > In the SiS case, both quirks key off the *same* *device*, but with potentially > different IDs. The quirk list ordering matters there because the list is > scanned only once per device. > > For the Intel case, the only ordering that matters is that the host bridge > device is added [pci_device_add()] before the LPC; AFAICT, that is reliable, > perhaps even by definition. > > So I don't think you have to worry about the Intel SMBus quirks. Ah, OK, I think I get it now, thanks for the clarification. I thought that the quirks were processed in file order for all devices at once, while I now understand they are processed for each device in turn. In which case, indeed, as long as the host bridge PCI device is listed before the ISA bridge PCI device (and as you suggest this appears to be guaranteed), the Intel SMBus quirk will work fine, regardless of the linking order. -- Jean Delvare - 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: [-mm patch] drivers/pci/quirks.c: cleanup
Hi Mark, On Tue, 9 Jan 2007 22:58:20 -0500, Mark M. Hoffman wrote: * Jean Delvare [EMAIL PROTECTED] [2007-01-09 14:17:21 +0100]: I am worried about the Intel/Asus SMBus quirk then, which affects many more users than the SiS SMBus one, and would suffer from a reordering as well. Intel/Asus users on FC[456] would surely have screamed if that was true. But I was curious so I looked deeper. There is a fundamental difference between the Intel SMBus quirks and the SiS SMBus quirk... Intel: 1) The first quirk keys off the host bridge, setting a flag. 2) The second quirk keys off the LPC, enabling the SMBus if the flag was set. SiS: 1) The first quirk keys off the *old* LPC ID... this causes the ID to change.[1] 2) The second quirk keys off the *new* LPC ID; this one enables the SMBus. In the SiS case, both quirks key off the *same* *device*, but with potentially different IDs. The quirk list ordering matters there because the list is scanned only once per device. For the Intel case, the only ordering that matters is that the host bridge device is added [pci_device_add()] before the LPC; AFAICT, that is reliable, perhaps even by definition. So I don't think you have to worry about the Intel SMBus quirks. Ah, OK, I think I get it now, thanks for the clarification. I thought that the quirks were processed in file order for all devices at once, while I now understand they are processed for each device in turn. In which case, indeed, as long as the host bridge PCI device is listed before the ISA bridge PCI device (and as you suggest this appears to be guaranteed), the Intel SMBus quirk will work fine, regardless of the linking order. -- Jean Delvare - 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: [-mm patch] drivers/pci/quirks.c: cleanup
Hi Jean: > On Mon, 8 Jan 2007 22:02:26 -0500, Mark M. Hoffman wrote: > > 3) I've just confirmed that this quirk is still broken on recent FC6 > > kernels. > > Perhaps they should have picked my patch out of their bugzilla, but they > > didn't. * Jean Delvare <[EMAIL PROTECTED]> [2007-01-09 14:17:21 +0100]: > I am worried about the Intel/Asus SMBus quirk then, which affects many > more users than the SiS SMBus one, and would suffer from a reordering as > well. Intel/Asus users on FC[456] would surely have screamed if that was true. But I was curious so I looked deeper. There is a fundamental difference between the Intel SMBus quirks and the SiS SMBus quirk... Intel: 1) The first quirk keys off the host bridge, setting a flag. 2) The second quirk keys off the LPC, enabling the SMBus if the flag was set. SiS: 1) The first quirk keys off the *old* LPC ID... this causes the ID to change.[1] 2) The second quirk keys off the *new* LPC ID; this one enables the SMBus. In the SiS case, both quirks key off the *same* *device*, but with potentially different IDs. The quirk list ordering matters there because the list is scanned only once per device. For the Intel case, the only ordering that matters is that the host bridge device is added [pci_device_add()] before the LPC; AFAICT, that is reliable, perhaps even by definition. So I don't think you have to worry about the Intel SMBus quirks. [1] That's right, the first quirk actually changes the PCI device ID of the LPC. Unless it actually *is* older hardware... in which case the quirk just tickles a reserved bit that is ignored. Thank you SiS, that's just beautiful. Regards, -- Mark M. Hoffman [EMAIL PROTECTED] - 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: [-mm patch] drivers/pci/quirks.c: cleanup
Hi Mark, On Mon, 8 Jan 2007 22:02:26 -0500, Mark M. Hoffman wrote: > Jean: > > * Jean Delvare <[EMAIL PROTECTED]> [2007-01-08 12:10:55 +0100]: > > Hi Mark, > > > > On Sun, 7 Jan 2007 10:44:41 -0500, Mark M. Hoffman wrote: > > > It is fragile for this code to depend on link order; Adrian's obvious and > > > trivial cleanups broke it. Not only that, but some FC kernels had/have > > > the > > > link order reversed such that this quirk is broken anyway. > > > > The former problem would be addressed just fine by a proper ordering > > (as Adrian's patch was attempting to bring) and a comment explaining > > the dependency. > > That's still fragile. Someone is bound to reorder the stupid things by > mistake (again). I'm tired of screwing around with this quirk already. > The patch that I sent last May would have fixed it permanently. And the > funny part is that *you* suggested the fix. ;) I seem to remember I suggested an improvement to make your fix better, but the fix was originally yours. Not that it really matters, anyway. > > > I sent a patch for this back in May: > > > http://lists.lm-sensors.org/pipermail/lm-sensors/2006-May/016113.html > > > > > > There was some discussion on the linux-pci mailing list as well; can't > > > seem to > > > find an archive of that though. Basically, it was not understood how the > > > FC > > > kernels could have a reversed link order. I never followed up on it, my > > > bad. > > > > As long as it isn't explained, I call it a compiler bug in FC. > > 1) What standard specifies the link order of objects in a module? I have seen > other compilers reorder objects this way. I can't say, sorry, I'm no compilation expert. It just sounded logical to me that the compiler should keep the code in the same order it found it in the sources, but it looks like this quirks code is very special. > 2) So what if it was a compiler bug? I guess we don't allow patches to work > around compiler bugs. Depends, as far as I remember, workarounds for mainline gcc are OK, but workarounds for distro-specific gcc are not. But one may argue that your patch isn't adding a workaround but cleaning up the code, then it no longer matters. > 3) I've just confirmed that this quirk is still broken on recent FC6 kernels. > Perhaps they should have picked my patch out of their bugzilla, but they > didn't. I am worried about the Intel/Asus SMBus quirk then, which affects many more users than the SiS SMBus one, and would suffer from a reordering as well. -- Jean Delvare - 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: [-mm patch] drivers/pci/quirks.c: cleanup
Hi Mark, On Mon, 8 Jan 2007 22:02:26 -0500, Mark M. Hoffman wrote: Jean: * Jean Delvare [EMAIL PROTECTED] [2007-01-08 12:10:55 +0100]: Hi Mark, On Sun, 7 Jan 2007 10:44:41 -0500, Mark M. Hoffman wrote: It is fragile for this code to depend on link order; Adrian's obvious and trivial cleanups broke it. Not only that, but some FC kernels had/have the link order reversed such that this quirk is broken anyway. The former problem would be addressed just fine by a proper ordering (as Adrian's patch was attempting to bring) and a comment explaining the dependency. That's still fragile. Someone is bound to reorder the stupid things by mistake (again). I'm tired of screwing around with this quirk already. The patch that I sent last May would have fixed it permanently. And the funny part is that *you* suggested the fix. ;) I seem to remember I suggested an improvement to make your fix better, but the fix was originally yours. Not that it really matters, anyway. I sent a patch for this back in May: http://lists.lm-sensors.org/pipermail/lm-sensors/2006-May/016113.html There was some discussion on the linux-pci mailing list as well; can't seem to find an archive of that though. Basically, it was not understood how the FC kernels could have a reversed link order. I never followed up on it, my bad. As long as it isn't explained, I call it a compiler bug in FC. 1) What standard specifies the link order of objects in a module? I have seen other compilers reorder objects this way. I can't say, sorry, I'm no compilation expert. It just sounded logical to me that the compiler should keep the code in the same order it found it in the sources, but it looks like this quirks code is very special. 2) So what if it was a compiler bug? I guess we don't allow patches to work around compiler bugs. Depends, as far as I remember, workarounds for mainline gcc are OK, but workarounds for distro-specific gcc are not. But one may argue that your patch isn't adding a workaround but cleaning up the code, then it no longer matters. 3) I've just confirmed that this quirk is still broken on recent FC6 kernels. Perhaps they should have picked my patch out of their bugzilla, but they didn't. I am worried about the Intel/Asus SMBus quirk then, which affects many more users than the SiS SMBus one, and would suffer from a reordering as well. -- Jean Delvare - 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: [-mm patch] drivers/pci/quirks.c: cleanup
Hi Jean: On Mon, 8 Jan 2007 22:02:26 -0500, Mark M. Hoffman wrote: 3) I've just confirmed that this quirk is still broken on recent FC6 kernels. Perhaps they should have picked my patch out of their bugzilla, but they didn't. * Jean Delvare [EMAIL PROTECTED] [2007-01-09 14:17:21 +0100]: I am worried about the Intel/Asus SMBus quirk then, which affects many more users than the SiS SMBus one, and would suffer from a reordering as well. Intel/Asus users on FC[456] would surely have screamed if that was true. But I was curious so I looked deeper. There is a fundamental difference between the Intel SMBus quirks and the SiS SMBus quirk... Intel: 1) The first quirk keys off the host bridge, setting a flag. 2) The second quirk keys off the LPC, enabling the SMBus if the flag was set. SiS: 1) The first quirk keys off the *old* LPC ID... this causes the ID to change.[1] 2) The second quirk keys off the *new* LPC ID; this one enables the SMBus. In the SiS case, both quirks key off the *same* *device*, but with potentially different IDs. The quirk list ordering matters there because the list is scanned only once per device. For the Intel case, the only ordering that matters is that the host bridge device is added [pci_device_add()] before the LPC; AFAICT, that is reliable, perhaps even by definition. So I don't think you have to worry about the Intel SMBus quirks. [1] That's right, the first quirk actually changes the PCI device ID of the LPC. Unless it actually *is* older hardware... in which case the quirk just tickles a reserved bit that is ignored. Thank you SiS, that's just beautiful. Regards, -- Mark M. Hoffman [EMAIL PROTECTED] - 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: [-mm patch] drivers/pci/quirks.c: cleanup
Jean: * Jean Delvare <[EMAIL PROTECTED]> [2007-01-08 12:10:55 +0100]: > Hi Mark, > > On Sun, 7 Jan 2007 10:44:41 -0500, Mark M. Hoffman wrote: > > Hi Jean, Adrian: > > > > > On Tue, 19 Dec 2006 05:13:15 +0100, Adrian Bunk wrote: > > > > @@ -1122,6 +1123,14 @@ static void quirk_sis_96x_smbus(struct p > > > > pci_write_config_byte(dev, 0x77, val & ~0x10); > > > > pci_read_config_byte(dev, 0x77, ); > > > > } > > > > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_961, > > > > quirk_sis_96x_smbus ); > > > > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_962, > > > > quirk_sis_96x_smbus ); > > > > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_963, > > > > quirk_sis_96x_smbus ); > > > > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_LPC, > > > > quirk_sis_96x_smbus ); > > > > +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_961, > > > > quirk_sis_96x_smbus ); > > > > +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_962, > > > > quirk_sis_96x_smbus ); > > > > +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_963, > > > > quirk_sis_96x_smbus ); > > > > +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_LPC, > > > > quirk_sis_96x_smbus ); > > > > > > > > /* > > > > * ... This is further complicated by the fact that some SiS96x south > > > > @@ -1158,6 +1167,8 @@ static void quirk_sis_503(struct pci_dev > > > > */ > > > > dev->device = devid; > > > > } > > > > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503, > > > > quirk_sis_503 ); > > > > +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503, > > > > quirk_sis_503 ); > > > > * Jean Delvare <[EMAIL PROTECTED]> [2007-01-05 09:52:33 +0100]: > > > Was this patch tested on the SiS-based boards which need these quirks? > > > I think you broke them. If I remember correctly, quirk_sis_503() must > > > be called before quirk_sis_96x_smbus() for some boards to work > > > properly, and we currently rely on the linking order to guarantee that. > > > Likewise, quirk_sis_96x_compatible() should be called before > > > quirk_sis_503() otherwise the warning message in quirk_sis_503() will > > > no longer be correct. > > > > > > So if you want to put the calls right after the quirk functions, you > > > need to reorder the functions themselves as well. Feel free to add a > > > comment explaining the order requirement so that nobody breaks it > > > accidentally again in the future. > > > > It is fragile for this code to depend on link order; Adrian's obvious and > > trivial cleanups broke it. Not only that, but some FC kernels had/have the > > link order reversed such that this quirk is broken anyway. > > The former problem would be addressed just fine by a proper ordering > (as Adrian's patch was attempting to bring) and a comment explaining > the dependency. That's still fragile. Someone is bound to reorder the stupid things by mistake (again). I'm tired of screwing around with this quirk already. The patch that I sent last May would have fixed it permanently. And the funny part is that *you* suggested the fix. ;) > > I sent a patch for this back in May: > > http://lists.lm-sensors.org/pipermail/lm-sensors/2006-May/016113.html > > > > There was some discussion on the linux-pci mailing list as well; can't seem > > to > > find an archive of that though. Basically, it was not understood how the FC > > kernels could have a reversed link order. I never followed up on it, my > > bad. > > As long as it isn't explained, I call it a compiler bug in FC. 1) What standard specifies the link order of objects in a module? I have seen other compilers reorder objects this way. 2) So what if it was a compiler bug? I guess we don't allow patches to work around compiler bugs. 3) I've just confirmed that this quirk is still broken on recent FC6 kernels. Perhaps they should have picked my patch out of their bugzilla, but they didn't. > > At any rate, can we please get the patch above applied? I will send a new > > one > > if necessary. > > This is a PCI patch, so I'm not the one picking it. I seem to remember > Greg was fine with the patch except for the comment about the linking > order. I'll resend it shortly... Regards, -- Mark M. Hoffman [EMAIL PROTECTED] - 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: [-mm patch] drivers/pci/quirks.c: cleanup
Hi Mark, On Sun, 7 Jan 2007 10:44:41 -0500, Mark M. Hoffman wrote: > Hi Jean, Adrian: > > > On Tue, 19 Dec 2006 05:13:15 +0100, Adrian Bunk wrote: > > > @@ -1122,6 +1123,14 @@ static void quirk_sis_96x_smbus(struct p > > > pci_write_config_byte(dev, 0x77, val & ~0x10); > > > pci_read_config_byte(dev, 0x77, ); > > > } > > > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_961, > > > quirk_sis_96x_smbus ); > > > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_962, > > > quirk_sis_96x_smbus ); > > > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_963, > > > quirk_sis_96x_smbus ); > > > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_LPC, > > > quirk_sis_96x_smbus ); > > > +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_961, > > > quirk_sis_96x_smbus ); > > > +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_962, > > > quirk_sis_96x_smbus ); > > > +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_963, > > > quirk_sis_96x_smbus ); > > > +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_LPC, > > > quirk_sis_96x_smbus ); > > > > > > /* > > > * ... This is further complicated by the fact that some SiS96x south > > > @@ -1158,6 +1167,8 @@ static void quirk_sis_503(struct pci_dev > > >*/ > > > dev->device = devid; > > > } > > > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503, > > > quirk_sis_503 ); > > > +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503, > > > quirk_sis_503 ); > > * Jean Delvare <[EMAIL PROTECTED]> [2007-01-05 09:52:33 +0100]: > > Was this patch tested on the SiS-based boards which need these quirks? > > I think you broke them. If I remember correctly, quirk_sis_503() must > > be called before quirk_sis_96x_smbus() for some boards to work > > properly, and we currently rely on the linking order to guarantee that. > > Likewise, quirk_sis_96x_compatible() should be called before > > quirk_sis_503() otherwise the warning message in quirk_sis_503() will > > no longer be correct. > > > > So if you want to put the calls right after the quirk functions, you > > need to reorder the functions themselves as well. Feel free to add a > > comment explaining the order requirement so that nobody breaks it > > accidentally again in the future. > > It is fragile for this code to depend on link order; Adrian's obvious and > trivial cleanups broke it. Not only that, but some FC kernels had/have the > link order reversed such that this quirk is broken anyway. The former problem would be addressed just fine by a proper ordering (as Adrian's patch was attempting to bring) and a comment explaining the dependency. > I sent a patch for this back in May: > http://lists.lm-sensors.org/pipermail/lm-sensors/2006-May/016113.html > > There was some discussion on the linux-pci mailing list as well; can't seem to > find an archive of that though. Basically, it was not understood how the FC > kernels could have a reversed link order. I never followed up on it, my bad. As long as it isn't explained, I call it a compiler bug in FC. > At any rate, can we please get the patch above applied? I will send a new one > if necessary. This is a PCI patch, so I'm not the one picking it. I seem to remember Greg was fine with the patch except for the comment about the linking order. BTW, the Intel (Asus) SMBus unhiding quirk also depends on the linking order if I'm not mistaking, and quite frankly I don't see a way to "fix" it if we decide to no longer trust the linking order. -- Jean Delvare - 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: [-mm patch] drivers/pci/quirks.c: cleanup
Hi Mark, On Sun, 7 Jan 2007 10:44:41 -0500, Mark M. Hoffman wrote: Hi Jean, Adrian: On Tue, 19 Dec 2006 05:13:15 +0100, Adrian Bunk wrote: @@ -1122,6 +1123,14 @@ static void quirk_sis_96x_smbus(struct p pci_write_config_byte(dev, 0x77, val ~0x10); pci_read_config_byte(dev, 0x77, val); } +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_961, quirk_sis_96x_smbus ); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_962, quirk_sis_96x_smbus ); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_963, quirk_sis_96x_smbus ); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_LPC, quirk_sis_96x_smbus ); +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_961, quirk_sis_96x_smbus ); +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_962, quirk_sis_96x_smbus ); +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_963, quirk_sis_96x_smbus ); +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_LPC, quirk_sis_96x_smbus ); /* * ... This is further complicated by the fact that some SiS96x south @@ -1158,6 +1167,8 @@ static void quirk_sis_503(struct pci_dev */ dev-device = devid; } +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503, quirk_sis_503 ); +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503, quirk_sis_503 ); * Jean Delvare [EMAIL PROTECTED] [2007-01-05 09:52:33 +0100]: Was this patch tested on the SiS-based boards which need these quirks? I think you broke them. If I remember correctly, quirk_sis_503() must be called before quirk_sis_96x_smbus() for some boards to work properly, and we currently rely on the linking order to guarantee that. Likewise, quirk_sis_96x_compatible() should be called before quirk_sis_503() otherwise the warning message in quirk_sis_503() will no longer be correct. So if you want to put the calls right after the quirk functions, you need to reorder the functions themselves as well. Feel free to add a comment explaining the order requirement so that nobody breaks it accidentally again in the future. It is fragile for this code to depend on link order; Adrian's obvious and trivial cleanups broke it. Not only that, but some FC kernels had/have the link order reversed such that this quirk is broken anyway. The former problem would be addressed just fine by a proper ordering (as Adrian's patch was attempting to bring) and a comment explaining the dependency. I sent a patch for this back in May: http://lists.lm-sensors.org/pipermail/lm-sensors/2006-May/016113.html There was some discussion on the linux-pci mailing list as well; can't seem to find an archive of that though. Basically, it was not understood how the FC kernels could have a reversed link order. I never followed up on it, my bad. As long as it isn't explained, I call it a compiler bug in FC. At any rate, can we please get the patch above applied? I will send a new one if necessary. This is a PCI patch, so I'm not the one picking it. I seem to remember Greg was fine with the patch except for the comment about the linking order. BTW, the Intel (Asus) SMBus unhiding quirk also depends on the linking order if I'm not mistaking, and quite frankly I don't see a way to fix it if we decide to no longer trust the linking order. -- Jean Delvare - 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: [-mm patch] drivers/pci/quirks.c: cleanup
Jean: * Jean Delvare [EMAIL PROTECTED] [2007-01-08 12:10:55 +0100]: Hi Mark, On Sun, 7 Jan 2007 10:44:41 -0500, Mark M. Hoffman wrote: Hi Jean, Adrian: On Tue, 19 Dec 2006 05:13:15 +0100, Adrian Bunk wrote: @@ -1122,6 +1123,14 @@ static void quirk_sis_96x_smbus(struct p pci_write_config_byte(dev, 0x77, val ~0x10); pci_read_config_byte(dev, 0x77, val); } +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_961, quirk_sis_96x_smbus ); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_962, quirk_sis_96x_smbus ); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_963, quirk_sis_96x_smbus ); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_LPC, quirk_sis_96x_smbus ); +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_961, quirk_sis_96x_smbus ); +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_962, quirk_sis_96x_smbus ); +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_963, quirk_sis_96x_smbus ); +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_LPC, quirk_sis_96x_smbus ); /* * ... This is further complicated by the fact that some SiS96x south @@ -1158,6 +1167,8 @@ static void quirk_sis_503(struct pci_dev */ dev-device = devid; } +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503, quirk_sis_503 ); +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503, quirk_sis_503 ); * Jean Delvare [EMAIL PROTECTED] [2007-01-05 09:52:33 +0100]: Was this patch tested on the SiS-based boards which need these quirks? I think you broke them. If I remember correctly, quirk_sis_503() must be called before quirk_sis_96x_smbus() for some boards to work properly, and we currently rely on the linking order to guarantee that. Likewise, quirk_sis_96x_compatible() should be called before quirk_sis_503() otherwise the warning message in quirk_sis_503() will no longer be correct. So if you want to put the calls right after the quirk functions, you need to reorder the functions themselves as well. Feel free to add a comment explaining the order requirement so that nobody breaks it accidentally again in the future. It is fragile for this code to depend on link order; Adrian's obvious and trivial cleanups broke it. Not only that, but some FC kernels had/have the link order reversed such that this quirk is broken anyway. The former problem would be addressed just fine by a proper ordering (as Adrian's patch was attempting to bring) and a comment explaining the dependency. That's still fragile. Someone is bound to reorder the stupid things by mistake (again). I'm tired of screwing around with this quirk already. The patch that I sent last May would have fixed it permanently. And the funny part is that *you* suggested the fix. ;) I sent a patch for this back in May: http://lists.lm-sensors.org/pipermail/lm-sensors/2006-May/016113.html There was some discussion on the linux-pci mailing list as well; can't seem to find an archive of that though. Basically, it was not understood how the FC kernels could have a reversed link order. I never followed up on it, my bad. As long as it isn't explained, I call it a compiler bug in FC. 1) What standard specifies the link order of objects in a module? I have seen other compilers reorder objects this way. 2) So what if it was a compiler bug? I guess we don't allow patches to work around compiler bugs. 3) I've just confirmed that this quirk is still broken on recent FC6 kernels. Perhaps they should have picked my patch out of their bugzilla, but they didn't. At any rate, can we please get the patch above applied? I will send a new one if necessary. This is a PCI patch, so I'm not the one picking it. I seem to remember Greg was fine with the patch except for the comment about the linking order. I'll resend it shortly... Regards, -- Mark M. Hoffman [EMAIL PROTECTED] - 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: [-mm patch] drivers/pci/quirks.c: cleanup
Hi Jean, Adrian, et. al.:
* Jean Delvare <[EMAIL PROTECTED]> [2007-01-07 12:30:13 +0100]:
> Hi Adrian,
>
> On Sat, 6 Jan 2007 00:29:13 +0100, Adrian Bunk wrote:
> > While looking at the code, I also noted the following:
> >
> > quirk_sis_96x_compatible() is pretty useless since all it does is to set
> > a static variable that is only used in a printk().
> >
> > quirk_sis_96x_compatible() was added with:
> >
> >
> > 2003/05/13 13:48:50-07:00 mhoffman
> > [PATCH] i2c: Add SiS96x I2C/SMBus driver
> >
> > This patch adds support for the SMBus of SiS96x south
> > bridges. It is based on i2c-sis645.c from the lm sensors
> > project, which never made it into an official kernel and
> > was anyway mis-named.
> >
> > This driver works on my SiS 645/961 board vs w83781d.
> >
> >
> > It's usage in
> >
> >
> > static void __init quirk_sis_503_smbus(struct pci_dev *dev)
> > {
> >if (sis_96x_compatible)
> >quirk_sis_96x_smbus(dev);
> > }
> >
> >
> > Was removed in
> >
> >
> > Author: torvalds
> > Date: Thu Oct 30 19:03:38 2003 +
> >
> > Stop SIS 96x chips from lying about themselves.
> >
> > Some machines with the SIS 96x southbridge have it set up
> > to claim it is a SIS 503 chip. That breaks irq routing logic
> > among other things. Fix it properly by making everybody aware
> > of the duplicity.
> >
> >
> > Was this intentional (and quirk_sis_96x_compatible() should be removed),
> > or is this a bug that should be fixed?
>
> I noticed this too in April 2006, see:
> http://lists.lm-sensors.org/pipermail/lm-sensors/2006-April/016016.html
>
> Quoting myself back then:
> "The whole sis_96x_compatible stuff looks superfluous now. It was used
> before 2.6.0-test10, but we could certainly get rid of it now."
>
> I do not think there is a bug here, or someone would have complained by
> now. Note though that I do not have a SiS-based motherboard to test on.
> Mark may be able to help with testing.
It's just cruft from the original quirk. The "compatible" printk could have
had value as a diagnostic in case the new quirk didn't work for some reason,
but I never saw any complaints about it (apart from the link order problem,
which is something different.) It's safe to remove by now.
Regards,
--
Mark M. Hoffman
[EMAIL PROTECTED]
-
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: [-mm patch] drivers/pci/quirks.c: cleanup
Hi Jean, Adrian: > On Tue, 19 Dec 2006 05:13:15 +0100, Adrian Bunk wrote: > > @@ -1122,6 +1123,14 @@ static void quirk_sis_96x_smbus(struct p > > pci_write_config_byte(dev, 0x77, val & ~0x10); > > pci_read_config_byte(dev, 0x77, ); > > } > > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_961, > > quirk_sis_96x_smbus ); > > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_962, > > quirk_sis_96x_smbus ); > > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_963, > > quirk_sis_96x_smbus ); > > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_LPC, > > quirk_sis_96x_smbus ); > > +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_961, > > quirk_sis_96x_smbus ); > > +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_962, > > quirk_sis_96x_smbus ); > > +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_963, > > quirk_sis_96x_smbus ); > > +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_LPC, > > quirk_sis_96x_smbus ); > > > > /* > > * ... This is further complicated by the fact that some SiS96x south > > @@ -1158,6 +1167,8 @@ static void quirk_sis_503(struct pci_dev > > */ > > dev->device = devid; > > } > > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503, > > quirk_sis_503 ); > > +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503, > > quirk_sis_503 ); * Jean Delvare <[EMAIL PROTECTED]> [2007-01-05 09:52:33 +0100]: > Was this patch tested on the SiS-based boards which need these quirks? > I think you broke them. If I remember correctly, quirk_sis_503() must > be called before quirk_sis_96x_smbus() for some boards to work > properly, and we currently rely on the linking order to guarantee that. > Likewise, quirk_sis_96x_compatible() should be called before > quirk_sis_503() otherwise the warning message in quirk_sis_503() will > no longer be correct. > > So if you want to put the calls right after the quirk functions, you > need to reorder the functions themselves as well. Feel free to add a > comment explaining the order requirement so that nobody breaks it > accidentally again in the future. It is fragile for this code to depend on link order; Adrian's obvious and trivial cleanups broke it. Not only that, but some FC kernels had/have the link order reversed such that this quirk is broken anyway. I sent a patch for this back in May: http://lists.lm-sensors.org/pipermail/lm-sensors/2006-May/016113.html There was some discussion on the linux-pci mailing list as well; can't seem to find an archive of that though. Basically, it was not understood how the FC kernels could have a reversed link order. I never followed up on it, my bad. At any rate, can we please get the patch above applied? I will send a new one if necessary. Regards, -- Mark M. Hoffman [EMAIL PROTECTED] - 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: [-mm patch] drivers/pci/quirks.c: cleanup
Hi Adrian,
On Sat, 6 Jan 2007 00:29:13 +0100, Adrian Bunk wrote:
> While looking at the code, I also noted the following:
>
> quirk_sis_96x_compatible() is pretty useless since all it does is to set
> a static variable that is only used in a printk().
>
> quirk_sis_96x_compatible() was added with:
>
>
> 2003/05/13 13:48:50-07:00 mhoffman
> [PATCH] i2c: Add SiS96x I2C/SMBus driver
>
> This patch adds support for the SMBus of SiS96x south
> bridges. It is based on i2c-sis645.c from the lm sensors
> project, which never made it into an official kernel and
> was anyway mis-named.
>
> This driver works on my SiS 645/961 board vs w83781d.
>
>
> It's usage in
>
>
> static void __init quirk_sis_503_smbus(struct pci_dev *dev)
> {
>if (sis_96x_compatible)
>quirk_sis_96x_smbus(dev);
> }
>
>
> Was removed in
>
>
> Author: torvalds
> Date: Thu Oct 30 19:03:38 2003 +
>
> Stop SIS 96x chips from lying about themselves.
>
> Some machines with the SIS 96x southbridge have it set up
> to claim it is a SIS 503 chip. That breaks irq routing logic
> among other things. Fix it properly by making everybody aware
> of the duplicity.
>
>
> Was this intentional (and quirk_sis_96x_compatible() should be removed),
> or is this a bug that should be fixed?
I noticed this too in April 2006, see:
http://lists.lm-sensors.org/pipermail/lm-sensors/2006-April/016016.html
Quoting myself back then:
"The whole sis_96x_compatible stuff looks superfluous now. It was used
before 2.6.0-test10, but we could certainly get rid of it now."
I do not think there is a bug here, or someone would have complained by
now. Note though that I do not have a SiS-based motherboard to test on.
Mark may be able to help with testing.
Thanks,
--
Jean Delvare
-
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: [-mm patch] drivers/pci/quirks.c: cleanup
Hi Adrian,
On Sat, 6 Jan 2007 00:29:13 +0100, Adrian Bunk wrote:
While looking at the code, I also noted the following:
quirk_sis_96x_compatible() is pretty useless since all it does is to set
a static variable that is only used in a printk().
quirk_sis_96x_compatible() was added with:
2003/05/13 13:48:50-07:00 mhoffman
[PATCH] i2c: Add SiS96x I2C/SMBus driver
This patch adds support for the SMBus of SiS96x south
bridges. It is based on i2c-sis645.c from the lm sensors
project, which never made it into an official kernel and
was anyway mis-named.
This driver works on my SiS 645/961 board vs w83781d.
It's usage in
static void __init quirk_sis_503_smbus(struct pci_dev *dev)
{
if (sis_96x_compatible)
quirk_sis_96x_smbus(dev);
}
Was removed in
Author: torvalds torvalds
Date: Thu Oct 30 19:03:38 2003 +
Stop SIS 96x chips from lying about themselves.
Some machines with the SIS 96x southbridge have it set up
to claim it is a SIS 503 chip. That breaks irq routing logic
among other things. Fix it properly by making everybody aware
of the duplicity.
Was this intentional (and quirk_sis_96x_compatible() should be removed),
or is this a bug that should be fixed?
I noticed this too in April 2006, see:
http://lists.lm-sensors.org/pipermail/lm-sensors/2006-April/016016.html
Quoting myself back then:
The whole sis_96x_compatible stuff looks superfluous now. It was used
before 2.6.0-test10, but we could certainly get rid of it now.
I do not think there is a bug here, or someone would have complained by
now. Note though that I do not have a SiS-based motherboard to test on.
Mark may be able to help with testing.
Thanks,
--
Jean Delvare
-
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: [-mm patch] drivers/pci/quirks.c: cleanup
Hi Jean, Adrian: On Tue, 19 Dec 2006 05:13:15 +0100, Adrian Bunk wrote: @@ -1122,6 +1123,14 @@ static void quirk_sis_96x_smbus(struct p pci_write_config_byte(dev, 0x77, val ~0x10); pci_read_config_byte(dev, 0x77, val); } +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_961, quirk_sis_96x_smbus ); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_962, quirk_sis_96x_smbus ); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_963, quirk_sis_96x_smbus ); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_LPC, quirk_sis_96x_smbus ); +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_961, quirk_sis_96x_smbus ); +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_962, quirk_sis_96x_smbus ); +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_963, quirk_sis_96x_smbus ); +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_LPC, quirk_sis_96x_smbus ); /* * ... This is further complicated by the fact that some SiS96x south @@ -1158,6 +1167,8 @@ static void quirk_sis_503(struct pci_dev */ dev-device = devid; } +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503, quirk_sis_503 ); +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503, quirk_sis_503 ); * Jean Delvare [EMAIL PROTECTED] [2007-01-05 09:52:33 +0100]: Was this patch tested on the SiS-based boards which need these quirks? I think you broke them. If I remember correctly, quirk_sis_503() must be called before quirk_sis_96x_smbus() for some boards to work properly, and we currently rely on the linking order to guarantee that. Likewise, quirk_sis_96x_compatible() should be called before quirk_sis_503() otherwise the warning message in quirk_sis_503() will no longer be correct. So if you want to put the calls right after the quirk functions, you need to reorder the functions themselves as well. Feel free to add a comment explaining the order requirement so that nobody breaks it accidentally again in the future. It is fragile for this code to depend on link order; Adrian's obvious and trivial cleanups broke it. Not only that, but some FC kernels had/have the link order reversed such that this quirk is broken anyway. I sent a patch for this back in May: http://lists.lm-sensors.org/pipermail/lm-sensors/2006-May/016113.html There was some discussion on the linux-pci mailing list as well; can't seem to find an archive of that though. Basically, it was not understood how the FC kernels could have a reversed link order. I never followed up on it, my bad. At any rate, can we please get the patch above applied? I will send a new one if necessary. Regards, -- Mark M. Hoffman [EMAIL PROTECTED] - 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: [-mm patch] drivers/pci/quirks.c: cleanup
Hi Jean, Adrian, et. al.:
* Jean Delvare [EMAIL PROTECTED] [2007-01-07 12:30:13 +0100]:
Hi Adrian,
On Sat, 6 Jan 2007 00:29:13 +0100, Adrian Bunk wrote:
While looking at the code, I also noted the following:
quirk_sis_96x_compatible() is pretty useless since all it does is to set
a static variable that is only used in a printk().
quirk_sis_96x_compatible() was added with:
2003/05/13 13:48:50-07:00 mhoffman
[PATCH] i2c: Add SiS96x I2C/SMBus driver
This patch adds support for the SMBus of SiS96x south
bridges. It is based on i2c-sis645.c from the lm sensors
project, which never made it into an official kernel and
was anyway mis-named.
This driver works on my SiS 645/961 board vs w83781d.
It's usage in
static void __init quirk_sis_503_smbus(struct pci_dev *dev)
{
if (sis_96x_compatible)
quirk_sis_96x_smbus(dev);
}
Was removed in
Author: torvalds torvalds
Date: Thu Oct 30 19:03:38 2003 +
Stop SIS 96x chips from lying about themselves.
Some machines with the SIS 96x southbridge have it set up
to claim it is a SIS 503 chip. That breaks irq routing logic
among other things. Fix it properly by making everybody aware
of the duplicity.
Was this intentional (and quirk_sis_96x_compatible() should be removed),
or is this a bug that should be fixed?
I noticed this too in April 2006, see:
http://lists.lm-sensors.org/pipermail/lm-sensors/2006-April/016016.html
Quoting myself back then:
The whole sis_96x_compatible stuff looks superfluous now. It was used
before 2.6.0-test10, but we could certainly get rid of it now.
I do not think there is a bug here, or someone would have complained by
now. Note though that I do not have a SiS-based motherboard to test on.
Mark may be able to help with testing.
It's just cruft from the original quirk. The compatible printk could have
had value as a diagnostic in case the new quirk didn't work for some reason,
but I never saw any complaints about it (apart from the link order problem,
which is something different.) It's safe to remove by now.
Regards,
--
Mark M. Hoffman
[EMAIL PROTECTED]
-
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: [-mm patch] drivers/pci/quirks.c: cleanup
On Fri, Jan 05, 2007 at 09:52:33AM +0100, Jean Delvare wrote:
> Hi Adrian,
>
> On Tue, 19 Dec 2006 05:13:15 +0100, Adrian Bunk wrote:
> > This patch contains the following cleanups:
> > - move all EXPORT_SYMBOL's directly below the code they are exporting
> > - move all DECLARE_PCI_FIXUP_*'s directly below the functions they
> > are calling
>
> Thanks for doing this cleanup, it was really needed. I think you didn't
> get everything right though:
>...
> > @@ -1122,6 +1123,14 @@ static void quirk_sis_96x_smbus(struct p
> > pci_write_config_byte(dev, 0x77, val & ~0x10);
> > pci_read_config_byte(dev, 0x77, );
> > }
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_961,
> > quirk_sis_96x_smbus );
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_962,
> > quirk_sis_96x_smbus );
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_963,
> > quirk_sis_96x_smbus );
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_LPC,
> > quirk_sis_96x_smbus );
> > +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_961,
> > quirk_sis_96x_smbus );
> > +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_962,
> > quirk_sis_96x_smbus );
> > +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_963,
> > quirk_sis_96x_smbus );
> > +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_LPC,
> > quirk_sis_96x_smbus );
> >
> > /*
> > * ... This is further complicated by the fact that some SiS96x south
> > @@ -1158,6 +1167,8 @@ static void quirk_sis_503(struct pci_dev
> > */
> > dev->device = devid;
> > }
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503,
> > quirk_sis_503 );
> > +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503,
> > quirk_sis_503 );
>
> Was this patch tested on the SiS-based boards which need these quirks?
> I think you broke them. If I remember correctly, quirk_sis_503() must
> be called before quirk_sis_96x_smbus() for some boards to work
> properly, and we currently rely on the linking order to guarantee that.
> Likewise, quirk_sis_96x_compatible() should be called before
> quirk_sis_503() otherwise the warning message in quirk_sis_503() will
> no longer be correct.
>
> So if you want to put the calls right after the quirk functions, you
> need to reorder the functions themselves as well. Feel free to add a
> comment explaining the order requirement so that nobody breaks it
> accidentally again in the future.
>...
Thanks for this information.
While looking at the code, I also noted the following:
quirk_sis_96x_compatible() is pretty useless since all it does is to set
a static variable that is only used in a printk().
quirk_sis_96x_compatible() was added with:
2003/05/13 13:48:50-07:00 mhoffman
[PATCH] i2c: Add SiS96x I2C/SMBus driver
This patch adds support for the SMBus of SiS96x south
bridges. It is based on i2c-sis645.c from the lm sensors
project, which never made it into an official kernel and
was anyway mis-named.
This driver works on my SiS 645/961 board vs w83781d.
It's usage in
static void __init quirk_sis_503_smbus(struct pci_dev *dev)
{
if (sis_96x_compatible)
quirk_sis_96x_smbus(dev);
}
Was removed in
Author: torvalds
Date: Thu Oct 30 19:03:38 2003 +
Stop SIS 96x chips from lying about themselves.
Some machines with the SIS 96x southbridge have it set up
to claim it is a SIS 503 chip. That breaks irq routing logic
among other things. Fix it properly by making everybody aware
of the duplicity.
Was this intentional (and quirk_sis_96x_compatible() should be removed),
or is this a bug that should be fixed?
> Thanks,
> Jean Delvare
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
-
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: [-mm patch] drivers/pci/quirks.c: cleanup
Hi Adrian,
On Tue, 19 Dec 2006 05:13:15 +0100, Adrian Bunk wrote:
> This patch contains the following cleanups:
> - move all EXPORT_SYMBOL's directly below the code they are exporting
> - move all DECLARE_PCI_FIXUP_*'s directly below the functions they
> are calling
Thanks for doing this cleanup, it was really needed. I think you didn't
get everything right though:
>
> Signed-off-by: Adrian Bunk <[EMAIL PROTECTED]>
>
> ---
>
> drivers/pci/pci.c|4
> drivers/pci/quirks.c | 42 +-
> 2 files changed, 17 insertions(+), 29 deletions(-)
>
> --- linux-2.6.20-rc1-mm1/drivers/pci/quirks.c.old 2006-12-19
> 04:12:39.0 +0100
> +++ linux-2.6.20-rc1-mm1/drivers/pci/quirks.c 2006-12-19 04:59:22.0
> +0100
> @@ -61,7 +61,8 @@ DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_I
>
> This appears to be BIOS not version dependent. So presumably there is a
> chipset level fix */
> -int isa_dma_bridge_buggy;/* Exported */
> +int isa_dma_bridge_buggy;
> +EXPORT_SYMBOL(isa_dma_bridge_buggy);
>
> static void __devinit quirk_isa_dma_hangs(struct pci_dev *dev)
> {
> @@ -83,6 +84,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NE
> DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NEC, PCI_DEVICE_ID_NEC_CBUS_3,
> quirk_isa_dma_hangs );
>
> int pci_pci_problems;
> +EXPORT_SYMBOL(pci_pci_problems);
>
> /*
> * Chipsets where PCI->PCI transfers vanish or hang
> @@ -94,6 +96,8 @@ static void __devinit quirk_nopcipci(str
> pci_pci_problems |= PCIPCI_FAIL;
> }
> }
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SI,PCI_DEVICE_ID_SI_5597,
> quirk_nopcipci );
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SI,PCI_DEVICE_ID_SI_496,
> quirk_nopcipci );
>
> static void __devinit quirk_nopciamd(struct pci_dev *dev)
> {
> @@ -105,9 +109,6 @@ static void __devinit quirk_nopciamd(str
> pci_pci_problems |= PCIAGP_FAIL;
> }
> }
> -
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SI,PCI_DEVICE_ID_SI_5597,
> quirk_nopcipci );
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SI,PCI_DEVICE_ID_SI_496,
> quirk_nopcipci );
> DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_8151_0,
> quirk_nopciamd );
>
> /*
> @@ -1122,6 +1123,14 @@ static void quirk_sis_96x_smbus(struct p
> pci_write_config_byte(dev, 0x77, val & ~0x10);
> pci_read_config_byte(dev, 0x77, );
> }
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_961,
> quirk_sis_96x_smbus );
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_962,
> quirk_sis_96x_smbus );
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_963,
> quirk_sis_96x_smbus );
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_LPC,
> quirk_sis_96x_smbus );
> +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_961,
> quirk_sis_96x_smbus );
> +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_962,
> quirk_sis_96x_smbus );
> +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_963,
> quirk_sis_96x_smbus );
> +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_LPC,
> quirk_sis_96x_smbus );
>
> /*
> * ... This is further complicated by the fact that some SiS96x south
> @@ -1158,6 +1167,8 @@ static void quirk_sis_503(struct pci_dev
>*/
> dev->device = devid;
> }
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503,
> quirk_sis_503 );
> +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503,
> quirk_sis_503 );
Was this patch tested on the SiS-based boards which need these quirks?
I think you broke them. If I remember correctly, quirk_sis_503() must
be called before quirk_sis_96x_smbus() for some boards to work
properly, and we currently rely on the linking order to guarantee that.
Likewise, quirk_sis_96x_compatible() should be called before
quirk_sis_503() otherwise the warning message in quirk_sis_503() will
no longer be correct.
So if you want to put the calls right after the quirk functions, you
need to reorder the functions themselves as well. Feel free to add a
comment explaining the order requirement so that nobody breaks it
accidentally again in the future.
>
> static void __init quirk_sis_96x_compatible(struct pci_dev *dev)
> {
> @@ -1170,8 +1181,6 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_S
> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_651,
> quirk_sis_96x_compatible );
> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_735,
> quirk_sis_96x_compatible );
>
> -DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503,
> quirk_sis_503 );
> -DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503,
> quirk_sis_503 );
> /*
> * On ASUS A8V and A8V Deluxe
Re: [-mm patch] drivers/pci/quirks.c: cleanup
Hi Adrian,
On Tue, 19 Dec 2006 05:13:15 +0100, Adrian Bunk wrote:
This patch contains the following cleanups:
- move all EXPORT_SYMBOL's directly below the code they are exporting
- move all DECLARE_PCI_FIXUP_*'s directly below the functions they
are calling
Thanks for doing this cleanup, it was really needed. I think you didn't
get everything right though:
Signed-off-by: Adrian Bunk [EMAIL PROTECTED]
---
drivers/pci/pci.c|4
drivers/pci/quirks.c | 42 +-
2 files changed, 17 insertions(+), 29 deletions(-)
--- linux-2.6.20-rc1-mm1/drivers/pci/quirks.c.old 2006-12-19
04:12:39.0 +0100
+++ linux-2.6.20-rc1-mm1/drivers/pci/quirks.c 2006-12-19 04:59:22.0
+0100
@@ -61,7 +61,8 @@ DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_I
This appears to be BIOS not version dependent. So presumably there is a
chipset level fix */
-int isa_dma_bridge_buggy;/* Exported */
+int isa_dma_bridge_buggy;
+EXPORT_SYMBOL(isa_dma_bridge_buggy);
static void __devinit quirk_isa_dma_hangs(struct pci_dev *dev)
{
@@ -83,6 +84,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NE
DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NEC, PCI_DEVICE_ID_NEC_CBUS_3,
quirk_isa_dma_hangs );
int pci_pci_problems;
+EXPORT_SYMBOL(pci_pci_problems);
/*
* Chipsets where PCI-PCI transfers vanish or hang
@@ -94,6 +96,8 @@ static void __devinit quirk_nopcipci(str
pci_pci_problems |= PCIPCI_FAIL;
}
}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SI,PCI_DEVICE_ID_SI_5597,
quirk_nopcipci );
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SI,PCI_DEVICE_ID_SI_496,
quirk_nopcipci );
static void __devinit quirk_nopciamd(struct pci_dev *dev)
{
@@ -105,9 +109,6 @@ static void __devinit quirk_nopciamd(str
pci_pci_problems |= PCIAGP_FAIL;
}
}
-
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SI,PCI_DEVICE_ID_SI_5597,
quirk_nopcipci );
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SI,PCI_DEVICE_ID_SI_496,
quirk_nopcipci );
DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_8151_0,
quirk_nopciamd );
/*
@@ -1122,6 +1123,14 @@ static void quirk_sis_96x_smbus(struct p
pci_write_config_byte(dev, 0x77, val ~0x10);
pci_read_config_byte(dev, 0x77, val);
}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_961,
quirk_sis_96x_smbus );
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_962,
quirk_sis_96x_smbus );
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_963,
quirk_sis_96x_smbus );
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_LPC,
quirk_sis_96x_smbus );
+DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_961,
quirk_sis_96x_smbus );
+DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_962,
quirk_sis_96x_smbus );
+DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_963,
quirk_sis_96x_smbus );
+DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_LPC,
quirk_sis_96x_smbus );
/*
* ... This is further complicated by the fact that some SiS96x south
@@ -1158,6 +1167,8 @@ static void quirk_sis_503(struct pci_dev
*/
dev-device = devid;
}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503,
quirk_sis_503 );
+DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503,
quirk_sis_503 );
Was this patch tested on the SiS-based boards which need these quirks?
I think you broke them. If I remember correctly, quirk_sis_503() must
be called before quirk_sis_96x_smbus() for some boards to work
properly, and we currently rely on the linking order to guarantee that.
Likewise, quirk_sis_96x_compatible() should be called before
quirk_sis_503() otherwise the warning message in quirk_sis_503() will
no longer be correct.
So if you want to put the calls right after the quirk functions, you
need to reorder the functions themselves as well. Feel free to add a
comment explaining the order requirement so that nobody breaks it
accidentally again in the future.
static void __init quirk_sis_96x_compatible(struct pci_dev *dev)
{
@@ -1170,8 +1181,6 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_S
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_651,
quirk_sis_96x_compatible );
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_735,
quirk_sis_96x_compatible );
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503,
quirk_sis_503 );
-DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503,
quirk_sis_503 );
/*
* On ASUS A8V and A8V Deluxe boards, the onboard AC97 audio controller
* and MC97 modem controller are disabled when a second PCI
Re: [-mm patch] drivers/pci/quirks.c: cleanup
On Fri, Jan 05, 2007 at 09:52:33AM +0100, Jean Delvare wrote:
Hi Adrian,
On Tue, 19 Dec 2006 05:13:15 +0100, Adrian Bunk wrote:
This patch contains the following cleanups:
- move all EXPORT_SYMBOL's directly below the code they are exporting
- move all DECLARE_PCI_FIXUP_*'s directly below the functions they
are calling
Thanks for doing this cleanup, it was really needed. I think you didn't
get everything right though:
...
@@ -1122,6 +1123,14 @@ static void quirk_sis_96x_smbus(struct p
pci_write_config_byte(dev, 0x77, val ~0x10);
pci_read_config_byte(dev, 0x77, val);
}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_961,
quirk_sis_96x_smbus );
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_962,
quirk_sis_96x_smbus );
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_963,
quirk_sis_96x_smbus );
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_LPC,
quirk_sis_96x_smbus );
+DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_961,
quirk_sis_96x_smbus );
+DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_962,
quirk_sis_96x_smbus );
+DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_963,
quirk_sis_96x_smbus );
+DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_LPC,
quirk_sis_96x_smbus );
/*
* ... This is further complicated by the fact that some SiS96x south
@@ -1158,6 +1167,8 @@ static void quirk_sis_503(struct pci_dev
*/
dev-device = devid;
}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503,
quirk_sis_503 );
+DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503,
quirk_sis_503 );
Was this patch tested on the SiS-based boards which need these quirks?
I think you broke them. If I remember correctly, quirk_sis_503() must
be called before quirk_sis_96x_smbus() for some boards to work
properly, and we currently rely on the linking order to guarantee that.
Likewise, quirk_sis_96x_compatible() should be called before
quirk_sis_503() otherwise the warning message in quirk_sis_503() will
no longer be correct.
So if you want to put the calls right after the quirk functions, you
need to reorder the functions themselves as well. Feel free to add a
comment explaining the order requirement so that nobody breaks it
accidentally again in the future.
...
Thanks for this information.
While looking at the code, I also noted the following:
quirk_sis_96x_compatible() is pretty useless since all it does is to set
a static variable that is only used in a printk().
quirk_sis_96x_compatible() was added with:
2003/05/13 13:48:50-07:00 mhoffman
[PATCH] i2c: Add SiS96x I2C/SMBus driver
This patch adds support for the SMBus of SiS96x south
bridges. It is based on i2c-sis645.c from the lm sensors
project, which never made it into an official kernel and
was anyway mis-named.
This driver works on my SiS 645/961 board vs w83781d.
It's usage in
static void __init quirk_sis_503_smbus(struct pci_dev *dev)
{
if (sis_96x_compatible)
quirk_sis_96x_smbus(dev);
}
Was removed in
Author: torvalds torvalds
Date: Thu Oct 30 19:03:38 2003 +
Stop SIS 96x chips from lying about themselves.
Some machines with the SIS 96x southbridge have it set up
to claim it is a SIS 503 chip. That breaks irq routing logic
among other things. Fix it properly by making everybody aware
of the duplicity.
Was this intentional (and quirk_sis_96x_compatible() should be removed),
or is this a bug that should be fixed?
Thanks,
Jean Delvare
cu
Adrian
--
Is there not promise of rain? Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
Only a promise, Lao Er said.
Pearl S. Buck - Dragon Seed
-
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: [-mm patch] drivers/pci/quirks.c: cleanup
On Tue, Dec 19, 2006 at 01:52:49AM -0700, Matthew Wilcox wrote: > On Tue, Dec 19, 2006 at 05:13:15AM +0100, Adrian Bunk wrote: > > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_5597, > > quirk_nopcipci ); > > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_496, > > quirk_nopcipci ); > > Why all the crazy spacing? > > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_5597, > quirk_nopcipci); I also saw this, but it's consistent through the file. But if it's agreed upon, I can also change this. cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed - 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: [-mm patch] drivers/pci/quirks.c: cleanup
On Tue, Dec 19, 2006 at 05:13:15AM +0100, Adrian Bunk wrote: > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SI,PCI_DEVICE_ID_SI_5597, > quirk_nopcipci ); > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SI,PCI_DEVICE_ID_SI_496, > quirk_nopcipci ); Why all the crazy spacing? +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_5597, quirk_nopcipci); - 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: [-mm patch] drivers/pci/quirks.c: cleanup
On Tue, Dec 19, 2006 at 05:13:15AM +0100, Adrian Bunk wrote: +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SI,PCI_DEVICE_ID_SI_5597, quirk_nopcipci ); +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SI,PCI_DEVICE_ID_SI_496, quirk_nopcipci ); Why all the crazy spacing? +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_5597, quirk_nopcipci); - 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: [-mm patch] drivers/pci/quirks.c: cleanup
On Tue, Dec 19, 2006 at 01:52:49AM -0700, Matthew Wilcox wrote: On Tue, Dec 19, 2006 at 05:13:15AM +0100, Adrian Bunk wrote: +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_5597, quirk_nopcipci ); +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_496, quirk_nopcipci ); Why all the crazy spacing? +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_5597, quirk_nopcipci); I also saw this, but it's consistent through the file. But if it's agreed upon, I can also change this. cu Adrian -- Is there not promise of rain? Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. Only a promise, Lao Er said. Pearl S. Buck - Dragon Seed - 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/
[-mm patch] drivers/pci/quirks.c: cleanup
This patch contains the following cleanups:
- move all EXPORT_SYMBOL's directly below the code they are exporting
- move all DECLARE_PCI_FIXUP_*'s directly below the functions they
are calling
Signed-off-by: Adrian Bunk <[EMAIL PROTECTED]>
---
drivers/pci/pci.c|4
drivers/pci/quirks.c | 42 +-
2 files changed, 17 insertions(+), 29 deletions(-)
--- linux-2.6.20-rc1-mm1/drivers/pci/quirks.c.old 2006-12-19
04:12:39.0 +0100
+++ linux-2.6.20-rc1-mm1/drivers/pci/quirks.c 2006-12-19 04:59:22.0
+0100
@@ -61,7 +61,8 @@ DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_I
This appears to be BIOS not version dependent. So presumably there is a
chipset level fix */
-int isa_dma_bridge_buggy; /* Exported */
+int isa_dma_bridge_buggy;
+EXPORT_SYMBOL(isa_dma_bridge_buggy);
static void __devinit quirk_isa_dma_hangs(struct pci_dev *dev)
{
@@ -83,6 +84,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NE
DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NEC, PCI_DEVICE_ID_NEC_CBUS_3,
quirk_isa_dma_hangs );
int pci_pci_problems;
+EXPORT_SYMBOL(pci_pci_problems);
/*
* Chipsets where PCI->PCI transfers vanish or hang
@@ -94,6 +96,8 @@ static void __devinit quirk_nopcipci(str
pci_pci_problems |= PCIPCI_FAIL;
}
}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_5597,
quirk_nopcipci );
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_496,
quirk_nopcipci );
static void __devinit quirk_nopciamd(struct pci_dev *dev)
{
@@ -105,9 +109,6 @@ static void __devinit quirk_nopciamd(str
pci_pci_problems |= PCIAGP_FAIL;
}
}
-
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_5597,
quirk_nopcipci );
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_496,
quirk_nopcipci );
DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_8151_0,
quirk_nopciamd );
/*
@@ -1122,6 +1123,14 @@ static void quirk_sis_96x_smbus(struct p
pci_write_config_byte(dev, 0x77, val & ~0x10);
pci_read_config_byte(dev, 0x77, );
}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_961,
quirk_sis_96x_smbus );
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_962,
quirk_sis_96x_smbus );
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_963,
quirk_sis_96x_smbus );
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_LPC,
quirk_sis_96x_smbus );
+DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_961,
quirk_sis_96x_smbus );
+DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_962,
quirk_sis_96x_smbus );
+DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_963,
quirk_sis_96x_smbus );
+DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_LPC,
quirk_sis_96x_smbus );
/*
* ... This is further complicated by the fact that some SiS96x south
@@ -1158,6 +1167,8 @@ static void quirk_sis_503(struct pci_dev
*/
dev->device = devid;
}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503,
quirk_sis_503 );
+DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503,
quirk_sis_503 );
static void __init quirk_sis_96x_compatible(struct pci_dev *dev)
{
@@ -1170,8 +1181,6 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_S
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_651,
quirk_sis_96x_compatible );
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_735,
quirk_sis_96x_compatible );
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503,
quirk_sis_503 );
-DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503,
quirk_sis_503 );
/*
* On ASUS A8V and A8V Deluxe boards, the onboard AC97 audio controller
* and MC97 modem controller are disabled when a second PCI soundcard is
@@ -1202,21 +1211,8 @@ static void asus_hides_ac97_lpc(struct p
}
}
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_VIA,PCI_DEVICE_ID_VIA_8237,
asus_hides_ac97_lpc );
-
-
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_961,
quirk_sis_96x_smbus );
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_962,
quirk_sis_96x_smbus );
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_963,
quirk_sis_96x_smbus );
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_LPC,
quirk_sis_96x_smbus );
-
DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_VIA,PCI_DEVICE_ID_VIA_8237,
asus_hides_ac97_lpc );
-
-DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_961,
quirk_sis_96x_smbus );
-DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_962,
[-mm patch] drivers/pci/quirks.c: cleanup
This patch contains the following cleanups:
- move all EXPORT_SYMBOL's directly below the code they are exporting
- move all DECLARE_PCI_FIXUP_*'s directly below the functions they
are calling
Signed-off-by: Adrian Bunk [EMAIL PROTECTED]
---
drivers/pci/pci.c|4
drivers/pci/quirks.c | 42 +-
2 files changed, 17 insertions(+), 29 deletions(-)
--- linux-2.6.20-rc1-mm1/drivers/pci/quirks.c.old 2006-12-19
04:12:39.0 +0100
+++ linux-2.6.20-rc1-mm1/drivers/pci/quirks.c 2006-12-19 04:59:22.0
+0100
@@ -61,7 +61,8 @@ DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_I
This appears to be BIOS not version dependent. So presumably there is a
chipset level fix */
-int isa_dma_bridge_buggy; /* Exported */
+int isa_dma_bridge_buggy;
+EXPORT_SYMBOL(isa_dma_bridge_buggy);
static void __devinit quirk_isa_dma_hangs(struct pci_dev *dev)
{
@@ -83,6 +84,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NE
DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NEC, PCI_DEVICE_ID_NEC_CBUS_3,
quirk_isa_dma_hangs );
int pci_pci_problems;
+EXPORT_SYMBOL(pci_pci_problems);
/*
* Chipsets where PCI-PCI transfers vanish or hang
@@ -94,6 +96,8 @@ static void __devinit quirk_nopcipci(str
pci_pci_problems |= PCIPCI_FAIL;
}
}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_5597,
quirk_nopcipci );
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_496,
quirk_nopcipci );
static void __devinit quirk_nopciamd(struct pci_dev *dev)
{
@@ -105,9 +109,6 @@ static void __devinit quirk_nopciamd(str
pci_pci_problems |= PCIAGP_FAIL;
}
}
-
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_5597,
quirk_nopcipci );
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_496,
quirk_nopcipci );
DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_8151_0,
quirk_nopciamd );
/*
@@ -1122,6 +1123,14 @@ static void quirk_sis_96x_smbus(struct p
pci_write_config_byte(dev, 0x77, val ~0x10);
pci_read_config_byte(dev, 0x77, val);
}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_961,
quirk_sis_96x_smbus );
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_962,
quirk_sis_96x_smbus );
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_963,
quirk_sis_96x_smbus );
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_LPC,
quirk_sis_96x_smbus );
+DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_961,
quirk_sis_96x_smbus );
+DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_962,
quirk_sis_96x_smbus );
+DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_963,
quirk_sis_96x_smbus );
+DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_LPC,
quirk_sis_96x_smbus );
/*
* ... This is further complicated by the fact that some SiS96x south
@@ -1158,6 +1167,8 @@ static void quirk_sis_503(struct pci_dev
*/
dev-device = devid;
}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503,
quirk_sis_503 );
+DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503,
quirk_sis_503 );
static void __init quirk_sis_96x_compatible(struct pci_dev *dev)
{
@@ -1170,8 +1181,6 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_S
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_651,
quirk_sis_96x_compatible );
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_735,
quirk_sis_96x_compatible );
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503,
quirk_sis_503 );
-DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503,
quirk_sis_503 );
/*
* On ASUS A8V and A8V Deluxe boards, the onboard AC97 audio controller
* and MC97 modem controller are disabled when a second PCI soundcard is
@@ -1202,21 +1211,8 @@ static void asus_hides_ac97_lpc(struct p
}
}
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_VIA,PCI_DEVICE_ID_VIA_8237,
asus_hides_ac97_lpc );
-
-
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_961,
quirk_sis_96x_smbus );
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_962,
quirk_sis_96x_smbus );
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_963,
quirk_sis_96x_smbus );
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_LPC,
quirk_sis_96x_smbus );
-
DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_VIA,PCI_DEVICE_ID_VIA_8237,
asus_hides_ac97_lpc );
-
-DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_961,
quirk_sis_96x_smbus );
-DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_962,
