| Message ID | 1390428346-2235-1-git-send-email-shane.huang@amd.com |
|---|---|
| State | Accepted |
| Headers | show |
Hi Shane, hi Wolfram, On Wed, 22 Jan 2014 14:05:46 -0800, Shane Huang wrote: > The locations of SMBus register base address and enablement bit are changed > from AMD ML, which need this patch to be supported. > > Signed-off-by: Shane Huang <shane.huang@amd.com> > --- > Documentation/i2c/busses/i2c-piix4 | 2 +- > drivers/i2c/busses/Kconfig | 1 + > drivers/i2c/busses/i2c-piix4.c | 28 +++++++++++++++++++++++----- > 3 files changed, 25 insertions(+), 6 deletions(-) Overall it looks good. Reviewed-by: Jean Delvare <khali@linux-fr.org> Wolfram, I think this is a candidate for stable kernel branches. It should apply fine down to 3.12. Shane, I still have one additional question: > (...) > @@ -258,13 +267,22 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev, > smba_en_hi = inb_p(smba_idx + 1); > release_region(smba_idx, 2); > > - if ((smba_en_lo & 1) == 0) { > + if (!smb_en) { > + smb_en_status = smba_en_lo & 0x10; > + piix4_smba = smba_en_hi << 8; > + if (aux) > + piix4_smba |= 0x20; > + } (...) As the mask used for smb_en_status doesn't depend on the value of "aux", this implies that on the Hudson-2, a single bit controls if both SMBus controllers are enabled. It's not possible to enable one and disable the other. Is it correct, or is it an overlook? BTW it would be really great if we could have access to the documentation for these new AMD chipsets. I looked for both Hudson and FCH at http://developer.amd.com/ but these searches returned nothing. I could help better if I had access to the documentation / datasheets.
Hi Jean, > As the mask used for smb_en_status doesn't depend on the value of > "aux", this implies that on the Hudson-2, a single bit controls if both > SMBus controllers are enabled. It's not possible to enable one and > disable the other. Is it correct, or is it an overlook? Good question. This is the current design, not an overlook. > BTW it would be really great if we could have access to the > documentation for these new AMD chipsets. I looked for both Hudson and > FCH at http://developer.amd.com/ but these searches returned nothing. I > could help better if I had access to the documentation / datasheets. I don't know who is the website maintainer but forwarded your question to our FCH datasheet maintainer, and he will follow up. I hope that it will be improved in near future. :-) BTW, will this patch appear from kernel 3.14-rc1 ? Thanks, Shane -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Shane, On Thu, 23 Jan 2014 22:26:49 +0000, Huang, Shane wrote: > > As the mask used for smb_en_status doesn't depend on the value of > > "aux", this implies that on the Hudson-2, a single bit controls if both > > SMBus controllers are enabled. It's not possible to enable one and > > disable the other. Is it correct, or is it an overlook? > > Good question. This is the current design, not an overlook. OK. > > BTW it would be really great if we could have access to the > > documentation for these new AMD chipsets. I looked for both Hudson and > > FCH at http://developer.amd.com/ but these searches returned nothing. I > > could help better if I had access to the documentation / datasheets. > > I don't know who is the website maintainer but forwarded your question > to our FCH datasheet maintainer, and he will follow up. I hope that it will > be improved in near future. :-) Thanks! > BTW, will this patch appear from kernel 3.14-rc1 ? This is Wolfram's call, not mine. I sure would like to see this patch in 3.14-rc1, and if that is not possible, in 3.14-rc2. This is a bug fix, so it should be applied ASAP.
On Wed, Jan 22, 2014 at 02:05:46PM -0800, Shane Huang wrote: > The locations of SMBus register base address and enablement bit are changed > from AMD ML, which need this patch to be supported. > > Signed-off-by: Shane Huang <shane.huang@amd.com> Applied to for-current, thanks! stable added.
Hi Jean, > > > BTW it would be really great if we could have access to the > > > documentation for these new AMD chipsets. I looked for both Hudson and > > > FCH at http://developer.amd.com/ but these searches returned nothing. I > > > could help better if I had access to the documentation / datasheets. > > > > I don't know who is the website maintainer but forwarded your question > > to our FCH datasheet maintainer, and he will follow up. I hope that it will > > be improved in near future. :-) > > Thanks! AMD Kabini integrated FCH data sheet can be found in the BKDG (page 611), which is almost same as Hudson-2. The data sheet of ML is not released. http://developer.amd.com/wordpress/media/2012/10/48751_BKDG_Fam_16h_Mod_00h-0Fh.pdf Thanks, Shane -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 27 Jan 2014 19:38:09 +0000, Huang, Shane wrote: > Hi Jean, > > > > > BTW it would be really great if we could have access to the > > > > documentation for these new AMD chipsets. I looked for both Hudson and > > > > FCH at http://developer.amd.com/ but these searches returned nothing. I > > > > could help better if I had access to the documentation / datasheets. > > > > > > I don't know who is the website maintainer but forwarded your question > > > to our FCH datasheet maintainer, and he will follow up. I hope that it will > > > be improved in near future. :-) > > > > Thanks! > > AMD Kabini integrated FCH data sheet can be found in the BKDG (page 611), > which is almost same as Hudson-2. The data sheet of ML is not released. > > http://developer.amd.com/wordpress/media/2012/10/48751_BKDG_Fam_16h_Mod_00h-0Fh.pdf Thanks Shane, this is very useful. I had not realized that the SMBus controller was now part of the CPU itself.
diff --git a/Documentation/i2c/busses/i2c-piix4 b/Documentation/i2c/busses/i2c-piix4 index c097e0f..aa959fd 100644 --- a/Documentation/i2c/busses/i2c-piix4 +++ b/Documentation/i2c/busses/i2c-piix4 @@ -13,7 +13,7 @@ Supported adapters: * AMD SP5100 (SB700 derivative found on some server mainboards) Datasheet: Publicly available at the AMD website http://support.amd.com/us/Embedded_TechDocs/44413.pdf - * AMD Hudson-2, CZ + * AMD Hudson-2, ML, CZ Datasheet: Not publicly available * Standard Microsystems (SMSC) SLC90E66 (Victory66) southbridge Datasheet: Publicly available at the SMSC website http://www.smsc.com diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index 6bcdea5..f5ed031 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -152,6 +152,7 @@ config I2C_PIIX4 ATI SB700/SP5100 ATI SB800 AMD Hudson-2 + AMD ML AMD CZ Serverworks OSB4 Serverworks CSB5 diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c index a028617..f71b4d3 100644 --- a/drivers/i2c/busses/i2c-piix4.c +++ b/drivers/i2c/busses/i2c-piix4.c @@ -22,7 +22,7 @@ Intel PIIX4, 440MX Serverworks OSB4, CSB5, CSB6, HT-1000, HT-1100 ATI IXP200, IXP300, IXP400, SB600, SB700/SP5100, SB800 - AMD Hudson-2, CZ + AMD Hudson-2, ML, CZ SMSC Victory66 Note: we assume there can only be one device, with one or more @@ -235,7 +235,8 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev, { unsigned short piix4_smba; unsigned short smba_idx = 0xcd6; - u8 smba_en_lo, smba_en_hi, i2ccfg, i2ccfg_offset = 0x10, smb_en; + u8 smba_en_lo, smba_en_hi, smb_en, smb_en_status; + u8 i2ccfg, i2ccfg_offset = 0x10; /* SB800 and later SMBus does not support forcing address */ if (force || force_addr) { @@ -245,7 +246,15 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev, } /* Determine the address of the SMBus areas */ - smb_en = (aux) ? 0x28 : 0x2c; + if ((PIIX4_dev->vendor == PCI_VENDOR_ID_AMD && + PIIX4_dev->device == PCI_DEVICE_ID_AMD_HUDSON2_SMBUS && + PIIX4_dev->revision >= 0x41) || + (PIIX4_dev->vendor == PCI_VENDOR_ID_AMD && + PIIX4_dev->device == 0x790b && + PIIX4_dev->revision >= 0x49)) + smb_en = 0x00; + else + smb_en = (aux) ? 0x28 : 0x2c; if (!request_region(smba_idx, 2, "smba_idx")) { dev_err(&PIIX4_dev->dev, "SMBus base address index region " @@ -258,13 +267,22 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev, smba_en_hi = inb_p(smba_idx + 1); release_region(smba_idx, 2); - if ((smba_en_lo & 1) == 0) { + if (!smb_en) { + smb_en_status = smba_en_lo & 0x10; + piix4_smba = smba_en_hi << 8; + if (aux) + piix4_smba |= 0x20; + } else { + smb_en_status = smba_en_lo & 0x01; + piix4_smba = ((smba_en_hi << 8) | smba_en_lo) & 0xffe0; + } + + if (!smb_en_status) { dev_err(&PIIX4_dev->dev, "Host SMBus controller not enabled!\n"); return -ENODEV; } - piix4_smba = ((smba_en_hi << 8) | smba_en_lo) & 0xffe0; if (acpi_check_region(piix4_smba, SMBIOSIZE, piix4_driver.name)) return -ENODEV;
The locations of SMBus register base address and enablement bit are changed from AMD ML, which need this patch to be supported. Signed-off-by: Shane Huang <shane.huang@amd.com> --- Documentation/i2c/busses/i2c-piix4 | 2 +- drivers/i2c/busses/Kconfig | 1 + drivers/i2c/busses/i2c-piix4.c | 28 +++++++++++++++++++++++----- 3 files changed, 25 insertions(+), 6 deletions(-)