| Message ID | 20200329174440.19342-1-calcprogrammer1@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | i2c: Detect secondary SMBus controller on AMD AM4 chipsets | expand |
Hi Adam, On Sun, 29 Mar 2020 12:44:40 -0500, Adam Honse wrote: > The AMD X370 and other AM4 chipsets (A/B/X 3/4/5 parts) and Threadripper equivalents have a secondary SMBus controller at I/O port address 0x0B20. This bus is used by several manufacturers to control motherboard RGB lighting via embedded controllers. I have been using this bus in my OpenRGB project to control the Aura RGB on many motherboards and ASRock also uses this bus for their Polychrome RGB controller. > > See this kernel bug report: https://bugzilla.kernel.org/show_bug.cgi?id=202587 > > Thanks, > > Adam Honse (calcprogrammer1@gmail.com) In order for this patch to be acceptable, this would need to be turned into a proper Signed-off-by statement. Please see: https://www.kernel.org/doc/html/v5.5/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin > > --- > drivers/i2c/busses/i2c-piix4.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c > index 30ded6422e7b..6068364b84f6 100644 > --- a/drivers/i2c/busses/i2c-piix4.c > +++ b/drivers/i2c/busses/i2c-piix4.c > @@ -981,6 +981,11 @@ static int piix4_probe(struct pci_dev *dev, const struct pci_device_id *id) > retval = piix4_setup_sb800(dev, id, 1); > } > > + if (dev->vendor == PCI_VENDOR_ID_AMD && > + dev->device == PCI_DEVICE_ID_AMD_KERNCZ_SMBUS) { > + retval = piix4_setup_sb800(dev, id, 1); > + } > + > if (retval > 0) { > /* Try to add the aux adapter if it exists, > * piix4_add_adapter will clean up if this fails */ I'm a bit worried about this change. Sure it works on the systems which do have the second SMBus channel, but what about the systems which don't? If there no device revision that needs to be checked to ensure that the second channel is present? This patch needs to be tested on tested on systems with CZ-compatible CPUs which do not have the second SMBus channel. Or do you believe they all do have it?
Hi Adam, On Sat, 4 Apr 2020 13:20:58 -0500, Adam Honse wrote: > I'm not sure what all chipsets are covered by the KERNCZ ID. We have found > that the secondary bus exists on all AM4 chipsets and on X399 as well. > Considering the older Hudson 2 has the same secondary bus (which is > correctly detected without a patch on my old FM1 home server, also at > 0x0B20) I would believe the secondary bus exists on all chipsets. I could > add a revision check though. My X370 reports (rev 59) in lspci. Fair enough, let's assume it's present on all chipsets and we can refine later if needed. If so, can you please merge the kerncz check with the hudson2 check right before as we do the same for both? Then please submit with your Signed-off-by statement and I'll review and ack the patch for Wolfram to commit. Thanks,
diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c index 30ded6422e7b..6068364b84f6 100644 --- a/drivers/i2c/busses/i2c-piix4.c +++ b/drivers/i2c/busses/i2c-piix4.c @@ -981,6 +981,11 @@ static int piix4_probe(struct pci_dev *dev, const struct pci_device_id *id) retval = piix4_setup_sb800(dev, id, 1); } + if (dev->vendor == PCI_VENDOR_ID_AMD && + dev->device == PCI_DEVICE_ID_AMD_KERNCZ_SMBUS) { + retval = piix4_setup_sb800(dev, id, 1); + } + if (retval > 0) { /* Try to add the aux adapter if it exists, * piix4_add_adapter will clean up if this fails */