[PATCH 1/2] i2c-piix4: Add support for AMD ML and CZ SMBus changes

[PATCH 1/2] i2c-piix4: Add support for AMD ML and CZ SMBus changes

[Shane Huang]

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-5C7GfCeVMHo@public.gmane.org>
---
 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(-)

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;
 
-- 
1.7.9.5

Re: [PATCH 1/2] i2c-piix4: Add support for AMD ML and CZ SMBus changes

[Jean Delvare]

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-5C7GfCeVMHo@public.gmane.org>
> ---
>  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-PUYAD+kWke1g9hUCZPvPmw@public.gmane.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.

-- 
Jean Delvare

RE: [PATCH 1/2] i2c-piix4: Add support for AMD ML and CZ SMBus changes

[Huang, Shane]

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

Re: [PATCH 1/2] i2c-piix4: Add support for AMD ML and CZ SMBus changes

[Jean Delvare]

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.

-- 
Jean Delvare

Re: [PATCH 1/2] i2c-piix4: Add support for AMD ML and CZ SMBus changes

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 324 bytes --]

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-5C7GfCeVMHo@public.gmane.org>

Applied to for-current, thanks! stable added.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

RE: [PATCH 1/2] i2c-piix4: Add support for AMD ML and CZ SMBus changes

[Huang, Shane]

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

Re: [PATCH 1/2] i2c-piix4: Add support for AMD ML and CZ SMBus changes

[Jean Delvare]

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.

-- 
Jean Delvare