[PATCH 0/2] i2c-mux: expose i2c bus topology under sysfs

[PATCH 0/2] i2c-mux: expose i2c bus topology under sysfs

[Gerlando Falauto]

Hi everyone,

I noticed that when you have multiple muxes connected to the same branch, implementation hides some information about the complete i2c topology.

So for instance if I have something like (notice there are three PCA9544's "in parallel", each with its own address, which I assume is a bit unusual):

#  CPU (i2c-0)
#  '--I2C Mux0 (PCA9547) @ 0x70
#      +--Port0 (i2c-1)
#      +--Port1 (i2c-2)
#      +--Port2 (i2c-3)
#      +--Port3 (i2c-4)
#      +--Port4 (i2c-5)
#      |   '--- I2C Mux1 (PCA9544) @ 0x74
#      |   |     +--Port0 (i2c-9)
#      |   |     +--Port1 (i2c-10)
#      |   |     +--Port2 (i2c-11)
#      |   |     '--Port3 (i2c-12)
#      |   +--- I2C Mux2 (PCA9544) @ 0x71
#      |   |     +--Port0 (i2c-13)
#      |   |     +--Port1 (i2c-14)
#      |   |     +--Port2 (i2c-15)
#      |   |     '--Port3 (i2c-16)
#      |   '--- I2C Mux3 (PCA9544) @ 0x72
#      |         +--Port0 (i2c-17)
#      |         +--Port1 (i2c-18)
#      |         +--Port2 (i2c-19)
#      |         '--Port3 (i2c-20)
#      +--Port5 (i2c-6)
#      +--Port6 (i2c-7)
#      '--Port7 (i2c-8)

under /sys/bus/i2c/devices/ I get:

0-0010 -> ../../../devices/platform/i2c-gpio/i2c-0/0-0010
0-0011 -> ../../../devices/platform/i2c-gpio/i2c-0/0-0011
0-0070 -> ../../../devices/platform/i2c-gpio/i2c-0/0-0070
5-0071 -> ../../../devices/platform/i2c-gpio/i2c-0/i2c-5/5-0071
5-0072 -> ../../../devices/platform/i2c-gpio/i2c-0/i2c-5/5-0072
5-0074 -> ../../../devices/platform/i2c-gpio/i2c-0/i2c-5/5-0074
|2c-0 -> ../../../devices/platform/i2c-gpio/i2c-0
i2c-1 -> ../../../devices/platform/i2c-gpio/i2c-0/i2c-1
i2c-2 -> ../../../devices/platform/i2c-gpio/i2c-0/i2c-2
i2c-3 -> ../../../devices/platform/i2c-gpio/i2c-0/i2c-3
i2c-4 -> ../../../devices/platform/i2c-gpio/i2c-0/i2c-4
i2c-5 -> ../../../devices/platform/i2c-gpio/i2c-0/i2c-5
i2c-6 -> ../../../devices/platform/i2c-gpio/i2c-0/i2c-6
i2c-7 -> ../../../devices/platform/i2c-gpio/i2c-0/i2c-7
i2c-8 -> ../../../devices/platform/i2c-gpio/i2c-0/i2c-8
i2c-9 -> ../../../devices/platform/i2c-gpio/i2c-0/i2c-5/i2c-9
i2c-10 -> ../../../devices/platform/i2c-gpio/i2c-0/i2c-5/i2c-10
i2c-11 -> ../../../devices/platform/i2c-gpio/i2c-0/i2c-5/i2c-11
i2c-12 -> ../../../devices/platform/i2c-gpio/i2c-0/i2c-5/i2c-12
i2c-13 -> ../../../devices/platform/i2c-gpio/i2c-0/i2c-5/i2c-13
i2c-14 -> ../../../devices/platform/i2c-gpio/i2c-0/i2c-5/i2c-14
i2c-15 -> ../../../devices/platform/i2c-gpio/i2c-0/i2c-5/i2c-15
i2c-16 -> ../../../devices/platform/i2c-gpio/i2c-0/i2c-5/i2c-16
i2c-17 -> ../../../devices/platform/i2c-gpio/i2c-0/i2c-5/i2c-17
i2c-18 -> ../../../devices/platform/i2c-gpio/i2c-0/i2c-5/i2c-18
i2c-19 -> ../../../devices/platform/i2c-gpio/i2c-0/i2c-5/i2c-19
i2c-20 -> ../../../devices/platform/i2c-gpio/i2c-0/i2c-5/i2c-20

So I have no way of telling that:

 i2c-{9,10,11,12} are connected to mux1 at address 0x74 of i2c-5
 i2c-{13,14,15,16} are connected to mux2 at address 0x71 of i2c-5
 i2c-{17,18,19,20} are connected to mux3 at address 0x72 of i2c-5

because I get:

# cat /sys/bus/i2c/devices/i2c-9/name
i2c-5-mux (chan_id 0)
# cat /sys/bus/i2c/devices/i2c-13/name
i2c-5-mux (chan_id 0)

which only reveals i2c-9 is the bus segment connected to channel 0 of a mux behind i2c-5, but since there's two of them, I have no way to tell which one...

The first patch moves i2c-{n} devices under the mux device, so instead of:

/sys/class/i2c-dev/i2c-0/device/i2c-5/i2c-9
/sys/class/i2c-dev/i2c-0/device/i2c-5/i2c-10
/sys/class/i2c-dev/i2c-0/device/i2c-5/i2c-11
/sys/class/i2c-dev/i2c-0/device/i2c-5/i2c-12

you get:

/sys/class/i2c-dev/i2c-0/device/0-0070/i2c-5/5-0071/i2c-9
/sys/class/i2c-dev/i2c-0/device/0-0070/i2c-5/5-0071/i2c-10
/sys/class/i2c-dev/i2c-0/device/0-0070/i2c-5/5-0071/i2c-11
/sys/class/i2c-dev/i2c-0/device/0-0070/i2c-5/5-0071/i2c-12

The second one creates symlinks like:

/sys/class/i2c-dev/i2c-0/device/0-0070/channel-4 -> i2c-5
/sys/class/i2c-dev/i2c-0/device/0-0070/channel-4/5-0074/channel-0 -> i2c-9
/sys/class/i2c-dev/i2c-0/device/0-0070/channel-4/5-0074/channel-1 -> i2c-10
/sys/class/i2c-dev/i2c-0/device/0-0070/channel-4/5-0074/channel-3 -> i2c-11
/sys/class/i2c-dev/i2c-0/device/0-0070/channel-4/5-0074/channel-3 -> i2c-12

Gerlando Falauto (2):
  i2c-mux: create mux busses under a different parent device
  i2c-mux: create "channel-n" symlinks for child segments

 drivers/i2c/i2c-mux.c           |   20 +++++++++++++++++++-
 drivers/i2c/muxes/gpio-i2cmux.c |    2 +-
 drivers/i2c/muxes/pca9541.c     |    2 +-
 drivers/i2c/muxes/pca954x.c     |    2 +-
 include/linux/i2c-mux.h         |    1 +
 5 files changed, 23 insertions(+), 4 deletions(-)

-- 
1.7.10.1

[PATCH 1/2] i2c-mux: create mux busses under a different parent device

[Gerlando Falauto]

Current implementation creates i2c-<n> busses as immediate children of their
i2c-<n> parent bus.
Change the topology so that they are created under the corresponding
mux device. This way, multiple muxes on the same segment can coexist
and children busses are grouped accordingly.

So for instance, by adding a pca9547 device with address 0x70 to
bus i2c-0, instead of:

/sys/class/i2c-dev/i2c-0/device/i2c-1
...
/sys/class/i2c-dev/i2c-0/device/i2c-8

you get:

/sys/class/i2c-dev/i2c-0/device/0-0070/i2c-1
...
/sys/class/i2c-dev/i2c-0/device/0-0070/i2c-8

Signed-off-by: Gerlando Falauto <gerlando.falauto-SkAbAL50j+5BDgjK7y7TUQ@public.gmane.org>
---
 drivers/i2c/i2c-mux.c           |    3 ++-
 drivers/i2c/muxes/gpio-i2cmux.c |    2 +-
 drivers/i2c/muxes/pca9541.c     |    2 +-
 drivers/i2c/muxes/pca954x.c     |    2 +-
 include/linux/i2c-mux.h         |    1 +
 5 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index d7a4833..01b8f8e 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -87,6 +87,7 @@ static u32 i2c_mux_functionality(struct i2c_adapter *adap)
 }
 
 struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
+				struct device *parent_device,
 				void *mux_dev, u32 force_nr, u32 chan_id,
 				int (*select) (struct i2c_adapter *,
 					       void *, u32),
@@ -122,7 +123,7 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
 	priv->adap.owner = THIS_MODULE;
 	priv->adap.algo = &priv->algo;
 	priv->adap.algo_data = priv;
-	priv->adap.dev.parent = &parent->dev;
+	priv->adap.dev.parent = parent_device;
 
 	if (force_nr) {
 		priv->adap.nr = force_nr;
diff --git a/drivers/i2c/muxes/gpio-i2cmux.c b/drivers/i2c/muxes/gpio-i2cmux.c
index 7b6ce62..c93046e 100644
--- a/drivers/i2c/muxes/gpio-i2cmux.c
+++ b/drivers/i2c/muxes/gpio-i2cmux.c
@@ -105,7 +105,7 @@ static int __devinit gpiomux_probe(struct platform_device *pdev)
 	for (i = 0; i < pdata->n_values; i++) {
 		u32 nr = pdata->base_nr ? (pdata->base_nr + i) : 0;
 
-		mux->adap[i] = i2c_add_mux_adapter(parent, mux, nr, i,
+		mux->adap[i] = i2c_add_mux_adapter(parent, &pdev->dev, mux, nr, i,
 						   gpiomux_select, deselect);
 		if (!mux->adap[i]) {
 			ret = -ENODEV;
diff --git a/drivers/i2c/muxes/pca9541.c b/drivers/i2c/muxes/pca9541.c
index ed699c5..4867d43 100644
--- a/drivers/i2c/muxes/pca9541.c
+++ b/drivers/i2c/muxes/pca9541.c
@@ -353,7 +353,7 @@ static int pca9541_probe(struct i2c_client *client,
 	force = 0;
 	if (pdata)
 		force = pdata->modes[0].adap_id;
-	data->mux_adap = i2c_add_mux_adapter(adap, client, force, 0,
+	data->mux_adap = i2c_add_mux_adapter(adap, &client->dev, client, force, 0,
 					     pca9541_select_chan,
 					     pca9541_release_chan);
 
diff --git a/drivers/i2c/muxes/pca954x.c b/drivers/i2c/muxes/pca954x.c
index 6f89536..5c6ecc7 100644
--- a/drivers/i2c/muxes/pca954x.c
+++ b/drivers/i2c/muxes/pca954x.c
@@ -226,7 +226,7 @@ static int pca954x_probe(struct i2c_client *client,
 		}
 
 		data->virt_adaps[num] =
-			i2c_add_mux_adapter(adap, client,
+			i2c_add_mux_adapter(adap, &client->dev, client,
 				force, num, pca954x_select_chan,
 				(pdata && pdata->modes[num].deselect_on_exit)
 					? pca954x_deselect_mux : NULL);
diff --git a/include/linux/i2c-mux.h b/include/linux/i2c-mux.h
index 34536ef..5c0e8d6 100644
--- a/include/linux/i2c-mux.h
+++ b/include/linux/i2c-mux.h
@@ -33,6 +33,7 @@
  * mux control.
  */
 struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
+				struct device *parent_device,
 				void *mux_dev, u32 force_nr, u32 chan_id,
 				int (*select) (struct i2c_adapter *,
 					       void *mux_dev, u32 chan_id),
-- 
1.7.10.1

[PATCH 2/2] i2c-mux: create "channel-n" symlinks for child segments

[Gerlando Falauto]

For instance, by adding a pca9547 device with address 0x70 to
bus i2c-0, you get

/sys/class/i2c-dev/i2c-0/device/0-0070/channel-0 -> i2c-1
...
/sys/class/i2c-dev/i2c-0/device/0-0070/channel-7 -> i2c-8

Signed-off-by: Gerlando Falauto <gerlando.falauto-SkAbAL50j+5BDgjK7y7TUQ@public.gmane.org>
---
 drivers/i2c/i2c-mux.c |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index 01b8f8e..c984862 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -96,6 +96,7 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
 {
 	struct i2c_mux_priv *priv;
 	int ret;
+	char symlink_name[12];
 
 	priv = kzalloc(sizeof(struct i2c_mux_priv), GFP_KERNEL);
 	if (!priv)
@@ -142,6 +143,17 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
 	dev_info(&parent->dev, "Added multiplexed i2c bus %d\n",
 		 i2c_adapter_id(&priv->adap));
 
+	snprintf(symlink_name, sizeof(symlink_name), "channel-%d", chan_id);
+	ret = sysfs_create_link(&parent_device->kobj,
+				&priv->adap.dev.kobj,
+				symlink_name);
+	if (ret)
+		dev_err(parent_device, "Failed creating symlink %s\n",
+			symlink_name);
+	else
+		dev_info(parent_device, "Created symlink %s\n",
+			symlink_name);
+
 	return &priv->adap;
 }
 EXPORT_SYMBOL_GPL(i2c_add_mux_adapter);
@@ -150,6 +162,11 @@ int i2c_del_mux_adapter(struct i2c_adapter *adap)
 {
 	struct i2c_mux_priv *priv = adap->algo_data;
 	int ret;
+	char symlink_name[12];
+
+	snprintf(symlink_name, sizeof(symlink_name), "channel-%d", priv->chan_id);
+
+	sysfs_delete_link(&adap->dev.parent->kobj, &priv->adap.dev.kobj, symlink_name);
 
 	ret = i2c_del_adapter(adap);
 	if (ret < 0)
-- 
1.7.10.1