[PATCH v2 0/4] i2c: Support i2c_register_spd() on multiplexed bus segments

[PATCH v2 0/4] i2c: Support i2c_register_spd() on multiplexed bus segments

[Heiner Kallweit]

i801 is the last bus driver supporting I2C_CLASS_SPD. It's used for
device probing on muxed bus segments. Only known use case so far is
systems with more than 8 memory modules (with thermal sensor) on
muxed smbus segments.
As discussed with Jean, to be able to remove I2C_CLASS_SPD completely
the following has to be done:

1. Extend i2c_register_spd() for use on muxed bus segments
2. Enable explicit instantiation of thermal sensors on memory modules
3. Extend i801 to call i2c_register_spd() on muxed bus segments

Step 2 has been accomplished:
caba40ec3531 ("eeprom: at24: Probe for DDR3 thermal sensor in the SPD case")
393bd1000f81 ("eeprom: ee1004: add support for temperature sensor")

Patch 1 does step 1
Patches 2 and 3 provide the basis for patch 4
Patch 4 does step 3

Note: i801 creates the mux platform device, loading and probing of the
mux driver may be asynchronous. Therefore we can't call i2c_register_spd()
for the muxed segments from i801. Instead we have to add a flag to the
platform data, so that the mux driver knows it's supposed to call
i2c_register_spd().

This series replaces the earlier RFC series.

v2:
- remove now obsolete comment in patch 1
- fix link error in some configs in patch 2

Heiner Kallweit (4):
  i2c: smbus: Prepare i2c_register_spd for usage on muxed segments
  i2c: mux: add basic support for calling i2c_register_spd on muxed bus
    segments
  i2c: mux: gpio: Allow to call i2c_register_spd on a muxed segment
  i2c: i801: Call i2c_register_spd() on muxed bus segments

 drivers/i2c/Kconfig                        |  1 +
 drivers/i2c/busses/i2c-i801.c              |  1 +
 drivers/i2c/i2c-mux.c                      |  4 ++++
 drivers/i2c/i2c-smbus.c                    | 20 ++++++++++++--------
 drivers/i2c/muxes/i2c-mux-gpio.c           |  1 +
 include/linux/i2c-mux.h                    |  1 +
 include/linux/platform_data/i2c-mux-gpio.h |  2 ++
 7 files changed, 22 insertions(+), 8 deletions(-)

-- 
2.43.0

[PATCH v2 1/4] i2c: smbus: Prepare i2c_register_spd for usage on muxed segments

[Heiner Kallweit]

If this is an adapter on a muxed bus segment, assume that each segment
is connected to a subset of the (> 8) overall memory slots. In this
case let's probe the maximum of 8 slots, however stop if the number
of overall populated slots is reached.

If we're not on a muxed segment and the total number of slots is > 8,
report an error, because then not all SPD eeproms can be addressed.
Presumably the bus is muxed, but the mux config is missing.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- remove now obsolete comment
---
 drivers/i2c/i2c-smbus.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
index 74807c6db..44331c5e2 100644
--- a/drivers/i2c/i2c-smbus.c
+++ b/drivers/i2c/i2c-smbus.c
@@ -309,7 +309,6 @@ EXPORT_SYMBOL_GPL(i2c_free_slave_host_notify_device);
  * Restrictions to automatic SPD instantiation:
  *  - Only works if all filled slots have the same memory type
  *  - Only works for DDR, DDR2, DDR3 and DDR4 for now
- *  - Only works on systems with 1 to 8 memory slots
  */
 #if IS_ENABLED(CONFIG_DMI)
 void i2c_register_spd(struct i2c_adapter *adap)
@@ -351,13 +350,18 @@ void i2c_register_spd(struct i2c_adapter *adap)
 	if (!dimm_count)
 		return;
 
-	dev_info(&adap->dev, "%d/%d memory slots populated (from DMI)\n",
-		 dimm_count, slot_count);
-
-	if (slot_count > 8) {
-		dev_warn(&adap->dev,
-			 "Systems with more than 8 memory slots not supported yet, not instantiating SPD\n");
-		return;
+	/* Check whether we're a child adapter on a muxed segment */
+	if (i2c_parent_is_i2c_adapter(adap)) {
+		if (slot_count > 8)
+			slot_count = 8;
+	} else {
+		dev_info(&adap->dev, "%d/%d memory slots populated (from DMI)\n",
+			 dimm_count, slot_count);
+		if (slot_count > 8) {
+			dev_err(&adap->dev,
+				"More than 8 memory slots on a single bus, mux config missing?\n");
+			return;
+		}
 	}
 
 	/*
-- 
2.43.0

[PATCH v2 2/4] i2c: mux: add basic support for calling i2c_register_spd on muxed bus segments

[Heiner Kallweit]

This extension allows mux drivers to instruct i2c_mux_add_adapter to
call i2c_register_spd. First user of this feature will be gpio mux.

Note: In order to avoid a link error we have to ensure that I2C_SMBUS=y
if I2C_MUX=y.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- Select I2C_SMBUS to avoid a link error if I2C_MUX=y and I2C_SMBUS=m
---
 drivers/i2c/Kconfig     | 1 +
 drivers/i2c/i2c-mux.c   | 4 ++++
 include/linux/i2c-mux.h | 1 +
 3 files changed, 6 insertions(+)

diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
index 9388823bb..f57a4d3fe 100644
--- a/drivers/i2c/Kconfig
+++ b/drivers/i2c/Kconfig
@@ -61,6 +61,7 @@ config I2C_CHARDEV
 
 config I2C_MUX
 	tristate "I2C bus multiplexing support"
+	select I2C_SMBUS
 	help
 	  Say Y here if you want the I2C core to support the ability to
 	  handle multiplexed I2C bus topologies, by presenting each
diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index 57ff09f18..ada9c764f 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -22,6 +22,7 @@
 #include <linux/acpi.h>
 #include <linux/i2c.h>
 #include <linux/i2c-mux.h>
+#include <linux/i2c-smbus.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -429,6 +430,9 @@ int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
 	dev_info(&parent->dev, "Added multiplexed i2c bus %d\n",
 		 i2c_adapter_id(&priv->adap));
 
+	if (muxc->register_spd)
+		i2c_register_spd(&priv->adap);
+
 	muxc->adapter[muxc->num_adapters++] = &priv->adap;
 	return 0;
 
diff --git a/include/linux/i2c-mux.h b/include/linux/i2c-mux.h
index 98ef73b7c..ec51d9bc4 100644
--- a/include/linux/i2c-mux.h
+++ b/include/linux/i2c-mux.h
@@ -21,6 +21,7 @@ struct i2c_mux_core {
 	unsigned int mux_locked:1;
 	unsigned int arbitrator:1;
 	unsigned int gate:1;
+	unsigned int register_spd:1;
 
 	void *priv;
 
-- 
2.43.0

[PATCH v2 3/4] i2c: mux: gpio: Allow to call i2c_register_spd on a muxed segment

[Heiner Kallweit]

Allow the gpio-based multiplexer to call i2c_register_spd on
muxed segments. First user will be i801.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/i2c/muxes/i2c-mux-gpio.c           | 1 +
 include/linux/platform_data/i2c-mux-gpio.h | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index 6b979a0a6..c841407c7 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -161,6 +161,7 @@ static int i2c_mux_gpio_probe(struct platform_device *pdev)
 	}
 	mux->gpios = muxc->priv;
 	muxc->priv = mux;
+	muxc->register_spd = mux->data.register_spd;
 
 	platform_set_drvdata(pdev, muxc);
 
diff --git a/include/linux/platform_data/i2c-mux-gpio.h b/include/linux/platform_data/i2c-mux-gpio.h
index 5e4c2c272..cbeb74f92 100644
--- a/include/linux/platform_data/i2c-mux-gpio.h
+++ b/include/linux/platform_data/i2c-mux-gpio.h
@@ -20,6 +20,7 @@
  * @n_values: Number of multiplexer positions (busses to instantiate)
  * @classes: Optional I2C auto-detection classes
  * @idle: Bitmask to write to MUX when idle or GPIO_I2CMUX_NO_IDLE if not used
+ * @register_spd: call i2c_register_spd for the child adapters on muxed segments
  */
 struct i2c_mux_gpio_platform_data {
 	int parent;
@@ -28,6 +29,7 @@ struct i2c_mux_gpio_platform_data {
 	int n_values;
 	const unsigned *classes;
 	unsigned idle;
+	unsigned int register_spd:1;
 };
 
 #endif /* _LINUX_I2C_MUX_GPIO_H */
-- 
2.43.0

[PATCH v2 4/4] i2c: i801: Call i2c_register_spd() on muxed bus segments

[Heiner Kallweit]

Instruct child adapters to call i2c_register_spd() for each muxed
bus segment.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/i2c/busses/i2c-i801.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 3932e8d96..15c93ec5b 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1404,6 +1404,7 @@ static void i801_add_mux(struct i801_priv *priv)
 	gpio_data.n_values = mux_config->n_values;
 	gpio_data.classes = mux_config->classes;
 	gpio_data.idle = I2C_MUX_GPIO_NO_IDLE;
+	gpio_data.register_spd = 1;
 
 	/* Register GPIO descriptor lookup table */
 	lookup = devm_kzalloc(dev,
-- 
2.43.0

Re: [PATCH v2 0/4] i2c: Support i2c_register_spd() on multiplexed bus segments

[Heiner Kallweit]

On 13.01.2024 12:23, Heiner Kallweit wrote:
> i801 is the last bus driver supporting I2C_CLASS_SPD. It's used for
> device probing on muxed bus segments. Only known use case so far is
> systems with more than 8 memory modules (with thermal sensor) on
> muxed smbus segments.
> As discussed with Jean, to be able to remove I2C_CLASS_SPD completely
> the following has to be done:
> 
> 1. Extend i2c_register_spd() for use on muxed bus segments
> 2. Enable explicit instantiation of thermal sensors on memory modules
> 3. Extend i801 to call i2c_register_spd() on muxed bus segments
> 
> Step 2 has been accomplished:
> caba40ec3531 ("eeprom: at24: Probe for DDR3 thermal sensor in the SPD case")
> 393bd1000f81 ("eeprom: ee1004: add support for temperature sensor")
> 
> Patch 1 does step 1
> Patches 2 and 3 provide the basis for patch 4
> Patch 4 does step 3
> 
> Note: i801 creates the mux platform device, loading and probing of the
> mux driver may be asynchronous. Therefore we can't call i2c_register_spd()
> for the muxed segments from i801. Instead we have to add a flag to the
> platform data, so that the mux driver knows it's supposed to call
> i2c_register_spd().
> 
> This series replaces the earlier RFC series.
> 
> v2:
> - remove now obsolete comment in patch 1
> - fix link error in some configs in patch 2
> 
> Heiner Kallweit (4):
>   i2c: smbus: Prepare i2c_register_spd for usage on muxed segments
>   i2c: mux: add basic support for calling i2c_register_spd on muxed bus
>     segments
>   i2c: mux: gpio: Allow to call i2c_register_spd on a muxed segment
>   i2c: i801: Call i2c_register_spd() on muxed bus segments
> 
>  drivers/i2c/Kconfig                        |  1 +
>  drivers/i2c/busses/i2c-i801.c              |  1 +
>  drivers/i2c/i2c-mux.c                      |  4 ++++
>  drivers/i2c/i2c-smbus.c                    | 20 ++++++++++++--------
>  drivers/i2c/muxes/i2c-mux-gpio.c           |  1 +
>  include/linux/i2c-mux.h                    |  1 +
>  include/linux/platform_data/i2c-mux-gpio.h |  2 ++
>  7 files changed, 22 insertions(+), 8 deletions(-)
> 
Any further feedback on this series, or is it ready to go?

Re: [PATCH v2 0/4] i2c: Support i2c_register_spd() on multiplexed bus segments

[Wolfram Sang]

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

Hi Heiner,

> Note: i801 creates the mux platform device, loading and probing of the
> mux driver may be asynchronous. Therefore we can't call i2c_register_spd()
> for the muxed segments from i801. Instead we have to add a flag to the
> platform data, so that the mux driver knows it's supposed to call
> i2c_register_spd().

Has it been considered to use a bus_notifier and check for
BUS_NOTIFY_BOUND_DRIVER?

I'd really like to keep it inside i801 if possible. First, all these
flags in mux drivers only for this corner case are relatively intrusive.
Second, selecting SMBUS for I2C_MUX is also a tad too much for my taste.

I understand that removing CLASS_SPD is a worthy goal. So, if all fails
we could still try this. But I'd think with bus_notifiers it should be
possible to keep it all in i801.

Do you think this could work?

Happy hacking,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 0/4] i2c: Support i2c_register_spd() on multiplexed bus segments

[Heiner Kallweit]

On 21.02.2024 18:19, Wolfram Sang wrote:
> Hi Heiner,
> 
>> Note: i801 creates the mux platform device, loading and probing of the
>> mux driver may be asynchronous. Therefore we can't call i2c_register_spd()
>> for the muxed segments from i801. Instead we have to add a flag to the
>> platform data, so that the mux driver knows it's supposed to call
>> i2c_register_spd().
> 
> Has it been considered to use a bus_notifier and check for
> BUS_NOTIFY_BOUND_DRIVER?
> 
> I'd really like to keep it inside i801 if possible. First, all these
> flags in mux drivers only for this corner case are relatively intrusive.
> Second, selecting SMBUS for I2C_MUX is also a tad too much for my taste.
> 
Right, it's not as clean as I would like to have it.
So far I didn't see any other/better option.

> I understand that removing CLASS_SPD is a worthy goal. So, if all fails
> we could still try this. But I'd think with bus_notifiers it should be
> possible to keep it all in i801.
> 
> Do you think this could work?
> 
Thanks for the hint. I have to admit I wasn't aware of this option.
I'll check and will come back to you.

> Happy hacking,
> 
>    Wolfram
> 
Heiner

Re: [PATCH v2 0/4] i2c: Support i2c_register_spd() on multiplexed bus segments

[Heiner Kallweit]

On 21.02.2024 18:19, Wolfram Sang wrote:
> Hi Heiner,
> 
>> Note: i801 creates the mux platform device, loading and probing of the
>> mux driver may be asynchronous. Therefore we can't call i2c_register_spd()
>> for the muxed segments from i801. Instead we have to add a flag to the
>> platform data, so that the mux driver knows it's supposed to call
>> i2c_register_spd().
> 
> Has it been considered to use a bus_notifier and check for
> BUS_NOTIFY_BOUND_DRIVER?
> 
I checked, and it looks like this:
Best would be to check for binding gpio mux driver to platform device
"i2c-mux-gpio" has completed. But the bus notification doesn't work for
platform devices.
Instead we can check for the BUS_NOTIFY_ADD_DEVICE event for the
child i2c adapters. This *should* work.
I tested that the events are properly recognized. However I don't have
hw with a muxed SMBUS, so I can't test the actual functionality.
I'll submit a RFC patch.

> I'd really like to keep it inside i801 if possible. First, all these
> flags in mux drivers only for this corner case are relatively intrusive.
> Second, selecting SMBUS for I2C_MUX is also a tad too much for my taste.
> 
> I understand that removing CLASS_SPD is a worthy goal. So, if all fails
> we could still try this. But I'd think with bus_notifiers it should be
> possible to keep it all in i801.
> 
> Do you think this could work?
> 
> Happy hacking,
> 
>    Wolfram
> 
Heiner