i2c-piix4 - Add support for secondary SMBus on AMD SB800 and AMD FCH chipsets

PATCH Applied Rudolf Marek 3 participants 9 messages May 17, 2013 – Aug 15, 2013 4 messages not in the archive

Subsystems (3)

I2C subsystem host drivers I2C I2C/smbus controller drivers for pc

lore

Share & tools

lore

Rudolf Marek

author

2013-05-17 22:46 UTC #0 lore raw

PATCH 1/1 i2c-piix4 - Add support for secondary SMBus on AMD SB800 and AMD FCH chipsets

Hello all,

Attached patch adds support for secondary SMBus of AMD SB800 and new AMD FCH chipsets. The base address of secondary SMBus is different from SB700 and it is stored on similar place as SB800 primary SMBus.

More verbose info:

Probing function was just modified to read the SMBus base from address 0x28 or from original 0x2c. The secondary bus does not provide IRQ information.

I think the SB700 has same secondary controller, so revision/IRQ information should not be printed too. This can be fixed in some other patch.

Chipset datasheet can be found here: http://support.amd.com/us/Embedded_TechDocs/45482.pdf

Tested on SB800 and FCH boards.

Tested-by Paul Menzel <[email protected]>

ASRock E350M1 with SB800

Signed-off-by Rudolf Marek <r.marek-/[email protected]>

Thanks
Rudolf

drivers/i2c/busses/i2c-piix4.c +31 ~~-10~~

index 39ab78c..7145d3c 100644
@@ -231,11 +231,11 @@ static int piix4_setup(struct pci_dev *PIIX4_dev,
 }
 
 static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
-			     const struct pci_device_id *id)
+			     const struct pci_device_id *id, u8 aux)
 {
 	unsigned short piix4_smba;
 	unsigned short smba_idx = 0xcd6;
-	u8 smba_en_lo, smba_en_hi, i2ccfg, i2ccfg_offset = 0x10, smb_en = 0x2c;
+	u8 smba_en_lo, smba_en_hi, i2ccfg, i2ccfg_offset = 0x10, smb_en;
 
 	/* SB800 and later SMBus does not support forcing address */
 	if (force || force_addr) {

@@ -245,6 +245,8 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
 	}
 
 	/* Determine the address of the SMBus areas */
+	smb_en = (aux) ? 0x28 : 0x2c;
+
 	if (!request_region(smba_idx, 2, "smba_idx")) {
 		dev_err(&PIIX4_dev->dev, "SMBus base address index region "
 			"0x%x already in use!\n", smba_idx);

@@ -272,6 +274,13 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
 		return -EBUSY;
 	}
 
+	/* Aux SMBus does not support IRQ information */
+	if (aux) {
+		dev_info(&PIIX4_dev->dev,
+			 "SMBus Host Controller at 0x%x\n", piix4_smba);
+		return piix4_smba;
+	}
+
 	/* Request the SMBus I2C bus config region */
 	if (!request_region(piix4_smba + i2ccfg_offset, 1, "i2ccfg")) {
 		dev_err(&PIIX4_dev->dev, "SMBus I2C bus config region "

@@ -596,7 +605,7 @@ static int piix4_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	     dev->revision >= 0x40) ||
 	    dev->vendor == PCI_VENDOR_ID_AMD)
 		/* base address location etc changed in SB800 */
-		retval = piix4_setup_sb800(dev, id);
+		retval = piix4_setup_sb800(dev, id, 0);
 	else
 		retval = piix4_setup(dev, id);

@@ -610,17 +619,29 @@ static int piix4_probe(struct pci_dev *dev, const struct pci_device_id *id)
 		return retval;
 
 	/* Check for auxiliary SMBus on some AMD chipsets */
+	retval = -ENODEV;
+
 	if (dev->vendor == PCI_VENDOR_ID_ATI &&
-	    dev->device == PCI_DEVICE_ID_ATI_SBX00_SMBUS &&
-	    dev->revision < 0x40) {
-		retval = piix4_setup_aux(dev, id, 0x58);
-		if (retval > 0) {
-			/* Try to add the aux adapter if it exists,
-			 * piix4_add_adapter will clean up if this fails */
-			piix4_add_adapter(dev, retval, &piix4_aux_adapter);
+	    dev->device == PCI_DEVICE_ID_ATI_SBX00_SMBUS) {
+		if (dev->revision < 0x40) {
+			retval = piix4_setup_aux(dev, id, 0x58);
+		} else {
+			/* SB800 added aux bus too */
+			retval = piix4_setup_sb800(dev, id, 1);
 		}
 	}
 
+	if (dev->vendor == PCI_VENDOR_ID_AMD &&
+	    dev->device == PCI_DEVICE_ID_AMD_HUDSON2_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 */
+		piix4_add_adapter(dev, retval, &piix4_aux_adapter);
+	}
+
 	return 0;
 }

A message that is not in the archive (sent off-list or not yet received): 5196B32D.7060501-/[email protected]

Paul Menzel

2013-05-23 07:35 UTC #2 lore raw

Dear Linux folks,

Am Samstag, den 18.05.2013, 00:46 +0200 schrieb Rudolf Marek:

Quoted 12 lines

Attached patch adds support for secondary SMBus of AMD SB800 and new AMD FCH chipsets. The base address of secondary SMBus is different from SB700 and it is stored on similar place as SB800 primary SMBus.
More verbose info:
Probing function was just modified to read the SMBus base from address 0x28 or from original 0x2c. The secondary bus does not provide IRQ information.
I think the SB700 has same secondary controller, so revision/IRQ information should not be printed too. This can be fixed in some other patch.

Quoted 7 lines

Chipset datasheet can be found here:
http://support.amd.com/us/Embedded_TechDocs/45482.pdf
Tested on SB800 and FCH boards.
Tested-by Paul Menzel <[email protected]>
ASRock E350M1 with SB800

Below are my detailed test results according to Rudolf’s instructions, he kindly gave me.

Quoted 6 lines

i2c-0	smbus     	SMBus PIIX4 adapter at 0b00    SMBus adapter
i2c-9	smbus     	SMBus PIIX4 adapter at 8060    SMBus adapter
I dont know if 8060 is correct, can you do please?
isadump 0xcd6 0xcd7

        $ sudo isadump 0xcd6 0xcd7
        WARNING! Running this program can cause system crashes, data loss and worse!
        I will probe address register 0xcd6 and data register 0xcd7.
        Continue? [Y/n] 
             0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
        00: 0e 00 4f 4a 00 04 00 02 34 05 04 01 06 21 43 55 
        10: ff 0f 00 00 ff 0f 00 00 00 00 00 00 00 00 00 00 
        20: 00 00 d1 fe 01 00 d8 fe 65 80 00 00 01 0b 00 00 
        30: 00 00 00 00 07 00 c0 fe 00 00 00 00 00 00 00 00 
        40: 00 00 00 00 00 00 00 80 f1 00 c0 fe 03 00 00 00 
        50: 1f 00 d0 fe 90 00 00 97 01 ff 15 66 00 05 00 00 
        60: 00 08 04 08 08 08 10 08 20 08 00 00 00 fe 08 00 
        70: 00 00 00 00 17 00 00 04 ff ff 00 00 85 01 e0 a0 
        80: 1f 1a 00 20 00 00 00 00 30 10 00 00 9e 00 80 00 
        90: 02 05 02 0b 01 00 10 00 00 10 00 10 00 00 00 00 
        a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
        b0: 00 00 00 00 00 0b 05 05 08 00 08 40 02 00 73 00 
        c0: f9 0b 78 40 3c 00 00 00 0c 40 28 00 ff ff ff ff 
        d0: 00 00 30 00 00 00 00 00 15 00 19 00 01 00 10 03 
        e0: d8 0c 00 00 00 00 00 02 00 00 00 01 03 02 00 4f 
        f0: 9c 03 00 00 00 00 00 21 6c 27 00 00 00 00 00 00

Quoted 5 lines

Also you could now invoke
i2cdetect 9
To detect i2c devices on the second bus.

        $ sudo i2cdetect 9
        WARNING! This program can confuse your I2C bus, cause data loss and worse!
        I will probe file /dev/i2c-9.
        I will probe address range 0x03-0x77.
        Continue? [Y/n] 
             0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
        00:          -- -- -- -- -- -- -- -- -- -- -- -- -- 
        10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
        20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
        30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
        40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
        50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
        60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
        70: -- -- -- -- -- -- -- --

It looks like no devices on the second bus are found. Here is the output from device i2c-0 for comparison.

        $ sudo i2cdetect 0
        WARNING! This program can confuse your I2C bus, cause data loss and worse!
        I will probe file /dev/i2c-0.
        I will probe address range 0x03-0x77.
        Continue? [Y/n] 
             0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
        00:          -- -- -- -- -- -- -- -- -- -- -- -- -- 
        10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
        20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
        30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
        40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
        50: 50 -- 52 -- -- -- -- -- -- -- -- -- -- -- -- -- 
        60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
        70: -- -- -- -- -- -- -- --

Quoted 1 line

Signed-off-by Rudolf Marek <r.marek-/[email protected]>

Rudolf, it looks like your patch could also update the i2c files under Documentation. At least there are some patches doing that.

Thanks,

Paul

Wolfram Sang

2013-06-11 19:35 UTC #3 lore raw

Quoted 16 lines

Quoted 1 line

To detect i2c devices on the second bus.

        $ sudo i2cdetect 9
        WARNING! This program can confuse your I2C bus, cause data loss and worse!
        I will probe file /dev/i2c-9.
        I will probe address range 0x03-0x77.
        Continue? [Y/n] 
             0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
        00:          -- -- -- -- -- -- -- -- -- -- -- -- -- 
        10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
        20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
        30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
        40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
        50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
        60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
        70: -- -- -- -- -- -- -- --

Do you know if there should be something connected?

Quoted 2 lines

Rudolf, it looks like your patch could also update the i2c files under Documentation. At least there are some patches doing that.

Do you mean 'busses/i2c-piix4'?

Rudolf Marek

author

2013-06-11 19:47 UTC #4 lore raw

Quoted 1 line

Do you know if there should be something connected?

In Paul case, most likely not. In my case there is a voltage controller for the DDR3 voltage regulator. I asked Paul test it on his SB800 system, I have the Hudson successor..

Quoted 4 lines

Quoted 2 lines
Rudolf, it looks like your patch could also update the i2c files under Documentation. At least there are some patches doing that.
Do you mean 'busses/i2c-piix4'?

Yes he means that. I will add documentation to the patch.

Thanks
Rudolf

A message that is not in the archive (sent off-list or not yet received): 51B77EC0.2020204-/[email protected]

Wolfram Sang

2013-06-17 07:16 UTC #6 lore raw

Quoted 6 lines · Rudolf Marek

Quoted 1 line
Do you know if there should be something connected?
In Paul case, most likely not. In my case there is a voltage controller for the DDR3 voltage regulator. I asked Paul test it on his SB800 system, I have the Hudson successor..

Quoted 6 lines

Quoted 4 lines
Quoted 2 lines
Rudolf, it looks like your patch could also update the i2c files under Documentation. At least there are some patches doing that.
Do you mean 'busses/i2c-piix4'?
Yes he means that. I will add documentation to the patch.

OK, so I will wait for V2. Thanks!

Rudolf Marek

author

2013-06-17 21:37 UTC #7 lore raw

Hi all,

I added just a few lines to the documentation file, the code part of the patch in unchanged.

Signed-off-by Rudolf Marek <r.marek-/[email protected]>

Attached patch adds support for secondary SMBus of AMD SB800 and new AMD FCH Hudson chipsets. The base address of secondary SMBus is different from SB700 and it is stored on similar place as SB800 primary SMBus.

Thanks
Rudolf

drivers/i2c/busses/i2c-piix4.c +31 ~~-10~~

Index: linux-3.10-rc6/drivers/i2c/busses/i2c-piix4.c
===================================================================
@@ -231,11 +231,11 @@
 }
 
 static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
-			     const struct pci_device_id *id)
+			     const struct pci_device_id *id, u8 aux)
 {
 	unsigned short piix4_smba;
 	unsigned short smba_idx = 0xcd6;
-	u8 smba_en_lo, smba_en_hi, i2ccfg, i2ccfg_offset = 0x10, smb_en = 0x2c;
+	u8 smba_en_lo, smba_en_hi, i2ccfg, i2ccfg_offset = 0x10, smb_en;
 
 	/* SB800 and later SMBus does not support forcing address */
 	if (force || force_addr) {

@@ -245,6 +245,8 @@
 	}
 
 	/* Determine the address of the SMBus areas */
+	smb_en = (aux) ? 0x28 : 0x2c;
+
 	if (!request_region(smba_idx, 2, "smba_idx")) {
 		dev_err(&PIIX4_dev->dev, "SMBus base address index region "
 			"0x%x already in use!\n", smba_idx);

@@ -272,6 +274,13 @@
 		return -EBUSY;
 	}
 
+	/* Aux SMBus does not support IRQ information */
+	if (aux) {
+		dev_info(&PIIX4_dev->dev,
+			 "SMBus Host Controller at 0x%x\n", piix4_smba);
+		return piix4_smba;
+	}
+
 	/* Request the SMBus I2C bus config region */
 	if (!request_region(piix4_smba + i2ccfg_offset, 1, "i2ccfg")) {
 		dev_err(&PIIX4_dev->dev, "SMBus I2C bus config region "

@@ -596,7 +605,7 @@
 	     dev->revision >= 0x40) ||
 	    dev->vendor == PCI_VENDOR_ID_AMD)
 		/* base address location etc changed in SB800 */
-		retval = piix4_setup_sb800(dev, id);
+		retval = piix4_setup_sb800(dev, id, 0);
 	else
 		retval = piix4_setup(dev, id);

@@ -610,17 +619,29 @@
 		return retval;
 
 	/* Check for auxiliary SMBus on some AMD chipsets */
+	retval = -ENODEV;
+
 	if (dev->vendor == PCI_VENDOR_ID_ATI &&
-	    dev->device == PCI_DEVICE_ID_ATI_SBX00_SMBUS &&
-	    dev->revision < 0x40) {
-		retval = piix4_setup_aux(dev, id, 0x58);
-		if (retval > 0) {
-			/* Try to add the aux adapter if it exists,
-			 * piix4_add_adapter will clean up if this fails */
-			piix4_add_adapter(dev, retval, &piix4_aux_adapter);
+	    dev->device == PCI_DEVICE_ID_ATI_SBX00_SMBUS) {
+		if (dev->revision < 0x40) {
+			retval = piix4_setup_aux(dev, id, 0x58);
+		} else {
+			/* SB800 added aux bus too */
+			retval = piix4_setup_sb800(dev, id, 1);
 		}
 	}
 
+	if (dev->vendor == PCI_VENDOR_ID_AMD &&
+	    dev->device == PCI_DEVICE_ID_AMD_HUDSON2_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 */
+		piix4_add_adapter(dev, retval, &piix4_aux_adapter);
+	}
+
 	return 0;
 }

Documentation/i2c/busses/i2c-piix4 +4 -3

Index: linux-3.10-rc6/Documentation/i2c/busses/i2c-piix4
===================================================================
@@ -73,9 +73,10 @@
 The ServerWorks Southbridges, the Intel 440MX, and the Victory66 are
 identical to the PIIX4 in I2C/SMBus support.
 
-The AMD SB700 and SP5100 chipsets implement two PIIX4-compatible SMBus
-controllers. If your BIOS initializes the secondary controller, it will
-be detected by this driver as an "Auxiliary SMBus Host Controller".
+The AMD SB700, SB800, SP5100 and Hudson-2 chipsets implement two
+PIIX4-compatible SMBus controllers. If your BIOS initializes the
+secondary controller, it will be detected by this driver as
+an "Auxiliary SMBus Host Controller".
 
 If you own Force CPCI735 motherboard or other OSB4 based systems you may need
 to change the SMBus Interrupt Select register so the SMBus controller uses

A message that is not in the archive (sent off-list or not yet received): 51BF8184.8090807-/[email protected]

Wolfram Sang

2013-06-19 09:55 UTC #9 lore raw

Hi Rudolf,

Quoted 6 lines · Rudolf Marek

Hi all,
I added just a few lines to the documentation file, the code part of the patch in unchanged.
Signed-off-by Rudolf Marek <r.marek-/[email protected]>

Thanks for adding the docs, but please resend in canonical patch format. See Documentation/SubmittingPatches, Chapter 15, to find out which paragraph belongs where (or have a glimpse at other patches). I get so many patches, it is too much of a burden to fix them all myself.

Regards,

Wolfram

Rudolf Marek

author

2013-07-14 21:17 UTC #10 lore raw

Re: [PATCH] i2c: i2c-piix4: Add support for secondary SMBus on AMD SB800 and AMD FCH chipsets

In reply to a message not in the archive

From: Rudolf Marek <r.marek-/[email protected]>

Add support for the secondary SMBus controller on the AMD SB800 and AMD FCH chipsets.

Signed-off-by Rudolf Marek <r.marek-/[email protected]>

---

Thanks
Rudolf

drivers/i2c/busses/i2c-piix4.c +31 ~~-10~~

Index: linux-3.10-rc6/drivers/i2c/busses/i2c-piix4.c
===================================================================
@@ -231,11 +231,11 @@
 }
 
 static int piix4_setup_sb800(struct pci_dev *PIIX4_dev,
-			     const struct pci_device_id *id)
+			     const struct pci_device_id *id, u8 aux)
 {
 	unsigned short piix4_smba;
 	unsigned short smba_idx = 0xcd6;
-	u8 smba_en_lo, smba_en_hi, i2ccfg, i2ccfg_offset = 0x10, smb_en = 0x2c;
+	u8 smba_en_lo, smba_en_hi, i2ccfg, i2ccfg_offset = 0x10, smb_en;
 
 	/* SB800 and later SMBus does not support forcing address */
 	if (force || force_addr) {

@@ -245,6 +245,8 @@
 	}
 
 	/* Determine the address of the SMBus areas */
+	smb_en = (aux) ? 0x28 : 0x2c;
+
 	if (!request_region(smba_idx, 2, "smba_idx")) {
 		dev_err(&PIIX4_dev->dev, "SMBus base address index region "
 			"0x%x already in use!\n", smba_idx);

@@ -272,6 +274,13 @@
 		return -EBUSY;
 	}
 
+	/* Aux SMBus does not support IRQ information */
+	if (aux) {
+		dev_info(&PIIX4_dev->dev,
+			 "SMBus Host Controller at 0x%x\n", piix4_smba);
+		return piix4_smba;
+	}
+
 	/* Request the SMBus I2C bus config region */
 	if (!request_region(piix4_smba + i2ccfg_offset, 1, "i2ccfg")) {
 		dev_err(&PIIX4_dev->dev, "SMBus I2C bus config region "

@@ -596,7 +605,7 @@
 	     dev->revision >= 0x40) ||
 	    dev->vendor == PCI_VENDOR_ID_AMD)
 		/* base address location etc changed in SB800 */
-		retval = piix4_setup_sb800(dev, id);
+		retval = piix4_setup_sb800(dev, id, 0);
 	else
 		retval = piix4_setup(dev, id);

@@ -610,17 +619,29 @@
 		return retval;
 
 	/* Check for auxiliary SMBus on some AMD chipsets */
+	retval = -ENODEV;
+
 	if (dev->vendor == PCI_VENDOR_ID_ATI &&
-	    dev->device == PCI_DEVICE_ID_ATI_SBX00_SMBUS &&
-	    dev->revision < 0x40) {
-		retval = piix4_setup_aux(dev, id, 0x58);
-		if (retval > 0) {
-			/* Try to add the aux adapter if it exists,
-			 * piix4_add_adapter will clean up if this fails */
-			piix4_add_adapter(dev, retval, &piix4_aux_adapter);
+	    dev->device == PCI_DEVICE_ID_ATI_SBX00_SMBUS) {
+		if (dev->revision < 0x40) {
+			retval = piix4_setup_aux(dev, id, 0x58);
+		} else {
+			/* SB800 added aux bus too */
+			retval = piix4_setup_sb800(dev, id, 1);
 		}
 	}
 
+	if (dev->vendor == PCI_VENDOR_ID_AMD &&
+	    dev->device == PCI_DEVICE_ID_AMD_HUDSON2_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 */
+		piix4_add_adapter(dev, retval, &piix4_aux_adapter);
+	}
+
 	return 0;
 }

Documentation/i2c/busses/i2c-piix4 +4 -3

Index: linux-3.10-rc6/Documentation/i2c/busses/i2c-piix4
===================================================================
@@ -73,9 +73,10 @@
 The ServerWorks Southbridges, the Intel 440MX, and the Victory66 are
 identical to the PIIX4 in I2C/SMBus support.
 
-The AMD SB700 and SP5100 chipsets implement two PIIX4-compatible SMBus
-controllers. If your BIOS initializes the secondary controller, it will
-be detected by this driver as an "Auxiliary SMBus Host Controller".
+The AMD SB700, SB800, SP5100 and Hudson-2 chipsets implement two
+PIIX4-compatible SMBus controllers. If your BIOS initializes the
+secondary controller, it will be detected by this driver as
+an "Auxiliary SMBus Host Controller".
 
 If you own Force CPCI735 motherboard or other OSB4 based systems you may need
 to change the SMBus Interrupt Select register so the SMBus controller uses

A message that is not in the archive (sent off-list or not yet received): 51E31566.2070509-/[email protected]

Wolfram Sang

Applied

2013-08-15 13:19 UTC #12 lore raw

Re: [PATCH] i2c: i2c-piix4: Add support for secondary SMBus on AMD SB800 and AMD FCH chipsets

Quoted 6 lines · Rudolf Marek

From: Rudolf Marek <r.marek-/[email protected]>
Add support for the secondary SMBus controller on the AMD SB800 and AMD FCH chipsets.
Signed-off-by Rudolf Marek <r.marek-/[email protected]>

Applied to for-next, thanks!