diff mbox series

[v2,4/4] i2c: piix4: Register SPDs

Message ID 20240627-piix4-spd-v2-4-617ce47b8ff4@weissschuh.net
State Superseded
Headers show
Series i2c: smbus cleanups and SPD support for piix4 | expand

Commit Message

Thomas Weißschuh June 27, 2024, 5:48 p.m. UTC
The piix4 I2C bus can carry SPDs, register them if present.
Only look on bus 0, as this is where the SPDs seem to be located.

Only the first 8 slots are supported. If the system has more,
then these will not be visible.

The AUX bus can not be probed as on some platforms it reports all
devices present and all reads return "0".
This would allow the ee1004 to be probed incorrectly.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/i2c/busses/Kconfig     | 1 +
 drivers/i2c/busses/i2c-piix4.c | 4 ++++
 2 files changed, 5 insertions(+)

Comments

Guenter Roeck June 28, 2024, 11:09 p.m. UTC | #1
On 6/27/24 10:48, Thomas Weißschuh wrote:
> The piix4 I2C bus can carry SPDs, register them if present.
> Only look on bus 0, as this is where the SPDs seem to be located.
> 
> Only the first 8 slots are supported. If the system has more,
> then these will not be visible.
> 
> The AUX bus can not be probed as on some platforms it reports all
> devices present and all reads return "0".
> This would allow the ee1004 to be probed incorrectly.

Was this reported somewhere ? I don't see it happen on any of my systems
(of course that doesn't really mean anything).

> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>

Reviewed-and-tested-by: Guenter Roeck <linux@roeck-us.net>

Thanks,
Guenter
Guenter Roeck June 29, 2024, 2:01 p.m. UTC | #2
On 6/29/24 00:19, Thomas Weißschuh wrote:
> On 2024-06-28 16:09:09+0000, Guenter Roeck wrote:
>> On 6/27/24 10:48, Thomas Weißschuh wrote:
>>> The piix4 I2C bus can carry SPDs, register them if present.
>>> Only look on bus 0, as this is where the SPDs seem to be located.
>>>
>>> Only the first 8 slots are supported. If the system has more,
>>> then these will not be visible.
>>>
>>> The AUX bus can not be probed as on some platforms it reports all
>>> devices present and all reads return "0".
>>> This would allow the ee1004 to be probed incorrectly.
>>
>> Was this reported somewhere ? I don't see it happen on any of my systems
>> (of course that doesn't really mean anything).
> 
> It happened on one of the big server systems I tested on.
> 
>>> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
>>
>> Reviewed-and-tested-by: Guenter Roeck <linux@roeck-us.net>
> 
> Thanks!
> 
> FYI, combined tags are discouraged by
> Documentation/process/maintainer-tip.rst:
> 
>    Please do not use combined tags, e.g. ``Reported-and-tested-by``, as
>    they just complicate automated extraction of tags.
> 
> I'll add the tags in split form to the patch.
> 
> 
> Thomas
Guenter Roeck June 29, 2024, 2:09 p.m. UTC | #3
On 6/29/24 00:19, Thomas Weißschuh wrote:
> On 2024-06-28 16:09:09+0000, Guenter Roeck wrote:
>> On 6/27/24 10:48, Thomas Weißschuh wrote:
>>> The piix4 I2C bus can carry SPDs, register them if present.
>>> Only look on bus 0, as this is where the SPDs seem to be located.
>>>
>>> Only the first 8 slots are supported. If the system has more,
>>> then these will not be visible.
>>>
>>> The AUX bus can not be probed as on some platforms it reports all
>>> devices present and all reads return "0".
>>> This would allow the ee1004 to be probed incorrectly.
>>
>> Was this reported somewhere ? I don't see it happen on any of my systems
>> (of course that doesn't really mean anything).
> 
> It happened on one of the big server systems I tested on.
> 

>>> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
>>
>> Reviewed-and-tested-by: Guenter Roeck <linux@roeck-us.net>
> 
> Thanks!
> 
> FYI, combined tags are discouraged by
> Documentation/process/maintainer-tip.rst:
> 
>    Please do not use combined tags, e.g. ``Reported-and-tested-by``, as
>    they just complicate automated extraction of tags.
> 
> I'll add the tags in split form to the patch.
> 

NP with me. though strictly speaking that only applies to the tip tree.
I started using it after someone else asked for it, but I don't like it
too much anyway. Combining it was an attempt to avoid the "please
combine ..." feedback ;-).

On a side note, I also applied the other three patches, but I am not sure
if that counts as "Tested", so I didn't send a tag for those.

Thanks,
Guenter
diff mbox series

Patch

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index fe6e8a1bb607..ff66e883b348 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -195,6 +195,7 @@  config I2C_ISMT
 config I2C_PIIX4
 	tristate "Intel PIIX4 and compatible (ATI/AMD/Serverworks/Broadcom/SMSC)"
 	depends on PCI && HAS_IOPORT
+	select I2C_SMBUS
 	help
 	  If you say yes to this option, support will be included for the Intel
 	  PIIX4 family of mainboard I2C interfaces.  Specifically, the following
diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
index 6a0392172b2f..14752d946f58 100644
--- a/drivers/i2c/busses/i2c-piix4.c
+++ b/drivers/i2c/busses/i2c-piix4.c
@@ -29,6 +29,7 @@ 
 #include <linux/stddef.h>
 #include <linux/ioport.h>
 #include <linux/i2c.h>
+#include <linux/i2c-smbus.h>
 #include <linux/slab.h>
 #include <linux/dmi.h>
 #include <linux/acpi.h>
@@ -982,6 +983,9 @@  static int piix4_add_adapter(struct pci_dev *dev, unsigned short smba,
 		return retval;
 	}
 
+	if (port == 0)
+		i2c_register_spd(adap);
+
 	*padap = adap;
 	return 0;
 }