[PATCH] i2c: mux: create proper topology in sysfs

[PATCH] i2c: mux: create proper topology in sysfs

[Wolfram Sang]

The current implementation creates muxed i2c-<n> busses as immediate
children of their i2c-<n> parent bus. In case of multiple muxes on one
bus, it is impossible to determine which muxed bus comes from which mux.

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

we get:

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

Symlinks for backward compatibility are provided.

Signed-off-by: Gerlando Falauto <gerlando.falauto-SkAbAL50j+5BDgjK7y7TUQ@public.gmane.org>

[wsa: Ported to top-of-tree (1 line left :)) and added the compatibility
links.]
Signed-off-by: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
Cc: Jean Delvare <jdelvare-l3A5Bk7waGM@public.gmane.org>
Cc: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
Cc: Martin Belanger <martin.belanger-Ir6+u9MVKBtBDgjK7y7TUQ@public.gmane.org>
Cc: Rodolfo Giometti <giometti-AVVDYK/kqiJWk0Htik3J/w@public.gmane.org>
Cc: Michael Lawnick <ml.lawnick-Mmb7MZpHnFY@public.gmane.org>
Cc: Jeroen De Wachter <jeroen.de.wachter-CNXmb7IdZIWZIoH1IeqzKA@public.gmane.org>
---

So, this is what I came up with to fix the mux-topology problem. Since I don't
really use muxes, please review/comment/test. And give Acks/Tested-by tags if
this works for you.

 drivers/i2c/i2c-mux.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index 2d0847b6be62..5b482ea32faf 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -138,7 +138,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 = mux_dev;
 	priv->adap.retries = parent->retries;
 	priv->adap.timeout = parent->timeout;
 
@@ -183,6 +183,9 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
 		return NULL;
 	}
 
+	WARN(sysfs_create_link(&parent->dev.kobj, &priv->adap.dev.kobj, priv->adap.dev.kobj.name),
+				"can't create compatibility link for old mux name scheme\n");
+
 	dev_info(&parent->dev, "Added multiplexed i2c bus %d\n",
 		 i2c_adapter_id(&priv->adap));
 
@@ -194,6 +197,7 @@ void i2c_del_mux_adapter(struct i2c_adapter *adap)
 {
 	struct i2c_mux_priv *priv = adap->algo_data;
 
+	sysfs_remove_link(&priv->parent->dev.kobj, adap->dev.kobj.name);
 	i2c_del_adapter(adap);
 	kfree(priv);
 }
-- 
2.1.1

Re: [PATCH] i2c: mux: create proper topology in sysfs

[Guenter Roeck]

On Sun, Nov 02, 2014 at 09:40:09PM +0100, Wolfram Sang wrote:
> The current implementation creates muxed i2c-<n> busses as immediate
> children of their i2c-<n> parent bus. In case of multiple muxes on one
> bus, it is impossible to determine which muxed bus comes from which mux.
> 
> 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
> 
> we get:
> 
> /sys/class/i2c-dev/i2c-0/device/0-0070/i2c-1
> ...
> /sys/class/i2c-dev/i2c-0/device/0-0070/i2c-8
> 
> Symlinks for backward compatibility are provided.
> 
> Signed-off-by: Gerlando Falauto <gerlando.falauto-SkAbAL50j+5BDgjK7y7TUQ@public.gmane.org>
> 
> [wsa: Ported to top-of-tree (1 line left :)) and added the compatibility
> links.]
> Signed-off-by: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
> Cc: Jean Delvare <jdelvare-l3A5Bk7waGM@public.gmane.org>
> Cc: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
> Cc: Martin Belanger <martin.belanger-Ir6+u9MVKBtBDgjK7y7TUQ@public.gmane.org>
> Cc: Rodolfo Giometti <giometti-AVVDYK/kqiJWk0Htik3J/w@public.gmane.org>
> Cc: Michael Lawnick <ml.lawnick-Mmb7MZpHnFY@public.gmane.org>
> Cc: Jeroen De Wachter <jeroen.de.wachter-CNXmb7IdZIWZIoH1IeqzKA@public.gmane.org>
> ---
> 
> So, this is what I came up with to fix the mux-topology problem. Since I don't
> really use muxes, please review/comment/test. And give Acks/Tested-by tags if
> this works for you.
> 
>  drivers/i2c/i2c-mux.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
> index 2d0847b6be62..5b482ea32faf 100644
> --- a/drivers/i2c/i2c-mux.c
> +++ b/drivers/i2c/i2c-mux.c
> @@ -138,7 +138,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 = mux_dev;

Not sure if the reason is that i2c mux adapters assume that the parent 
device is the i2c adapter, but the code crashes all over the place for me
after applying this patch. I'll try to track it down.

Guenter

Re: [PATCH] i2c: mux: create proper topology in sysfs

[Guenter Roeck]

On Sun, Nov 02, 2014 at 09:40:09PM +0100, Wolfram Sang wrote:
> The current implementation creates muxed i2c-<n> busses as immediate
> children of their i2c-<n> parent bus. In case of multiple muxes on one
> bus, it is impossible to determine which muxed bus comes from which mux.
> 
> 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
> 
> we get:
> 
> /sys/class/i2c-dev/i2c-0/device/0-0070/i2c-1
> ...
> /sys/class/i2c-dev/i2c-0/device/0-0070/i2c-8
> 
> Symlinks for backward compatibility are provided.
> 
> Signed-off-by: Gerlando Falauto <gerlando.falauto-SkAbAL50j+5BDgjK7y7TUQ@public.gmane.org>
> 
> [wsa: Ported to top-of-tree (1 line left :)) and added the compatibility
> links.]
> Signed-off-by: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
> Cc: Jean Delvare <jdelvare-l3A5Bk7waGM@public.gmane.org>
> Cc: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
> Cc: Martin Belanger <martin.belanger-Ir6+u9MVKBtBDgjK7y7TUQ@public.gmane.org>
> Cc: Rodolfo Giometti <giometti-AVVDYK/kqiJWk0Htik3J/w@public.gmane.org>
> Cc: Michael Lawnick <ml.lawnick-Mmb7MZpHnFY@public.gmane.org>
> Cc: Jeroen De Wachter <jeroen.de.wachter-CNXmb7IdZIWZIoH1IeqzKA@public.gmane.org>
> ---
> 
> So, this is what I came up with to fix the mux-topology problem. Since I don't
> really use muxes, please review/comment/test. And give Acks/Tested-by tags if
> this works for you.
> 
>  drivers/i2c/i2c-mux.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
> index 2d0847b6be62..5b482ea32faf 100644
> --- a/drivers/i2c/i2c-mux.c
> +++ b/drivers/i2c/i2c-mux.c
> @@ -138,7 +138,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 = mux_dev;
>  	priv->adap.retries = parent->retries;
>  	priv->adap.timeout = parent->timeout;
>  
> @@ -183,6 +183,9 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
>  		return NULL;
>  	}
>  
> +	WARN(sysfs_create_link(&parent->dev.kobj, &priv->adap.dev.kobj, priv->adap.dev.kobj.name),
> +				"can't create compatibility link for old mux name scheme\n");
> +
Something seems to be wrong with the linking.

I get lots of messages such as

sysfs: cannot create duplicate filename
'/devices/fff70a000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/0000:02:09.0/0000:10:00.0/i2c-sam.16/i2c-88/i2c-89'

followed by "can't create compatibility link for old mux name scheme".

Guenter

Re: [PATCH] i2c: mux: create proper topology in sysfs

[Wolfram Sang]

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

> > +	WARN(sysfs_create_link(&parent->dev.kobj, &priv->adap.dev.kobj, priv->adap.dev.kobj.name),
> > +				"can't create compatibility link for old mux name scheme\n");
> > +
> Something seems to be wrong with the linking.
> 
> I get lots of messages such as
> 
> sysfs: cannot create duplicate filename
> '/devices/fff70a000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/0000:02:09.0/0000:10:00.0/i2c-sam.16/i2c-88/i2c-89'
> 
> followed by "can't create compatibility link for old mux name scheme".

What is your muxing setup?

Thanks for testing BTW!


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

Re: [PATCH] i2c: mux: create proper topology in sysfs

[Guenter Roeck]

On Mon, Nov 03, 2014 at 10:02:00PM +0100, Wolfram Sang wrote:
> > > +	WARN(sysfs_create_link(&parent->dev.kobj, &priv->adap.dev.kobj, priv->adap.dev.kobj.name),
> > > +				"can't create compatibility link for old mux name scheme\n");
> > > +
> > Something seems to be wrong with the linking.
> > 
> > I get lots of messages such as
> > 
> > sysfs: cannot create duplicate filename
> > '/devices/fff70a000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/0000:02:09.0/0000:10:00.0/i2c-sam.16/i2c-88/i2c-89'
> > 
> > followed by "can't create compatibility link for old mux name scheme".
> 
> What is your muxing setup?
> 
Complicated ;-).

There are really various muxes. The one above is a multi-function FPGA connected
to a PCIe bus. The FPGA core has up to 8 i2c controllers. Each of those controllers
supports up to 8 mux channels, for a total of up to 64 virtual i2c adapters.

There are also gpio based muxes in the same system, only they are not
'simple' gpio muxes where 1 pin matches 1 channel, but complex ones where
the channel is determined by a three-bit value followed by an 8-bit mask
(so there can be up to 64 mux channels on a single adapter).

Plus of course there are some pca9548 and pca9546 based multiplexers.

This is all mixed with gpio based bus master selectors permitting access from
different boards. The overall architecture is so complex that it is actually
possible to configure the i2c bus in circles (bus master selector to mux to
another mux back to the other channel of the bus master selector).

Just don't ask me who came up with that architecture ;-)

I'll add some debugging to try to find out why the symlinks don't work.

Guenter

Re: [PATCH] i2c: mux: create proper topology in sysfs

[Wolfram Sang]

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

> > What is your muxing setup?
> > 
> Complicated ;-).

...

Ooooookay... well, for testing these patches, your setup is absolutely
awesome, of course :D May I ask what you are doing with that amount of
I2C?


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

Re: [PATCH] i2c: mux: create proper topology in sysfs

[Danielle Costantino]

This is a tool I have been developing for complex I2C bus structures
(lsi2c) the output of this should help with this complex multiplexer
testing. I was able to overcome the problem by creating persistent
paths describing where the devices reside. I will be testing
compatibility with this setup. I will be reporting on my test results
within a few days.

[root@p2020 ~]# ./lsi2c -h
Usage: lsi2c [OPTION]...
  -c, --config-file     Specify a config file
  -C, --print-config    Display i2c devices in configuration file
  -a, --all             Print all i2c-devs in bus tree
  -d, --print-devices   Display sysfs i2c devices
  -t, --tree            Print i2c bus and children
  -p, --path            Parse an i2c-dev path
  -P, --probe           Probe an i2c-dev at addr on path
  -F, --func            Print I2C bus functionality
  -T, --timeout         Set adapter timeout in milliseconds
  -S, --retry-count     Set adapter max retry count
  -h, --help            Display this help text
  -V, --version         Display the program version
  -v, --verbose         Be verbose
  -i, --initialize      Initialize devices in configuration file
  -r, --remove          Remove devices in configuration file
  -k, --kmod            Try to initialize i2c_dev kernel module

Use `-' after `-c' to read the config file from stdin.

[root@p2020 ~]# ./lsi2c -v
Searching for i2c devices
I2C Adapters:
bus: i2c-0 path: 0               type: i2c name: MPC adapter at
0xffe03000 parent: ffe03000.i2c
bus: i2c-2 path: 0:0.0           type: mux name: i2c-0-mux (chan_id 0)
parent: i2c-0
bus: i2c-3 path: 0:0.1           type: mux name: i2c-0-mux (chan_id 1)
parent: i2c-0
bus: i2c-4 path: 0:0.2           type: mux name: i2c-0-mux (chan_id 2)
parent: i2c-0
bus: i2c-5 path: 0:0.3           type: mux name: i2c-0-mux (chan_id 3)
parent: i2c-0
bus: i2c-6 path: 0:0.4           type: mux name: i2c-0-mux (chan_id 4)
parent: i2c-0
bus: i2c-7 path: 0:0.5           type: mux name: i2c-0-mux (chan_id 5)
parent: i2c-0
bus: i2c-8 path: 0:0.6           type: mux name: i2c-0-mux (chan_id 6)
parent: i2c-0
bus: i2c-9 path: 0:0.7           type: mux name: i2c-0-mux (chan_id 7)
parent: i2c-0
bus: i2c-1 path: 1               type: i2c name: MPC adapter at
0xffe03100 parent: ffe03100.i2c
bus: i2c-10 path: 1:0.0           type: mux name: i2c-1-mux (chan_id
0) parent: i2c-1
bus: i2c-11 path: 1:0.1           type: mux name: i2c-1-mux (chan_id
1) parent: i2c-1
bus: i2c-18 path: 1:0.1:0.0       type: mux name: i2c-11-mux (chan_id
0) parent: i2c-11
bus: i2c-12 path: 1:0.2           type: mux name: i2c-1-mux (chan_id
2) parent: i2c-1
bus: i2c-40 path: 1:0.2:0.0       type: mux name: i2c-12-mux (chan_id
0) parent: i2c-12
bus: i2c-41 path: 1:0.2:0.0:0.0   type: mux name: i2c-40-mux (chan_id
0) parent: i2c-40
bus: i2c-42 path: 1:0.2:0.0:0.1   type: mux name: i2c-40-mux (chan_id
1) parent: i2c-40
bus: i2c-43 path: 1:0.2:0.0:0.2   type: mux name: i2c-40-mux (chan_id
2) parent: i2c-40
bus: i2c-44 path: 1:0.2:0.0:0.3   type: mux name: i2c-40-mux (chan_id
3) parent: i2c-40
bus: i2c-45 path: 1:0.2:0.0:0.4   type: mux name: i2c-40-mux (chan_id
4) parent: i2c-40
bus: i2c-46 path: 1:0.2:0.0:0.5   type: mux name: i2c-40-mux (chan_id
5) parent: i2c-40
bus: i2c-47 path: 1:0.2:0.0:0.6   type: mux name: i2c-40-mux (chan_id
6) parent: i2c-40
bus: i2c-48 path: 1:0.2:0.0:0.7   type: mux name: i2c-40-mux (chan_id
7) parent: i2c-40
bus: i2c-49 path: 1:0.2:0.0:1.0   type: mux name: i2c-40-mux (chan_id
0) parent: i2c-40
bus: i2c-50 path: 1:0.2:0.0:1.1   type: mux name: i2c-40-mux (chan_id
1) parent: i2c-40
bus: i2c-51 path: 1:0.2:0.0:1.2   type: mux name: i2c-40-mux (chan_id
2) parent: i2c-40
bus: i2c-52 path: 1:0.2:0.0:1.3   type: mux name: i2c-40-mux (chan_id
3) parent: i2c-40
bus: i2c-53 path: 1:0.2:0.0:1.4   type: mux name: i2c-40-mux (chan_id
4) parent: i2c-40
bus: i2c-54 path: 1:0.2:0.0:1.5   type: mux name: i2c-40-mux (chan_id
5) parent: i2c-40
bus: i2c-55 path: 1:0.2:0.0:1.6   type: mux name: i2c-40-mux (chan_id
6) parent: i2c-40
bus: i2c-56 path: 1:0.2:0.0:1.7   type: mux name: i2c-40-mux (chan_id
7) parent: i2c-40
bus: i2c-13 path: 1:0.3           type: mux name: i2c-1-mux (chan_id
3) parent: i2c-1
bus: i2c-23 path: 1:0.3:0.0       type: mux name: i2c-13-mux (chan_id
0) parent: i2c-13
bus: i2c-24 path: 1:0.3:0.0:0.0   type: mux name: i2c-23-mux (chan_id
0) parent: i2c-23
bus: i2c-25 path: 1:0.3:0.0:0.1   type: mux name: i2c-23-mux (chan_id
1) parent: i2c-23
bus: i2c-26 path: 1:0.3:0.0:0.2   type: mux name: i2c-23-mux (chan_id
2) parent: i2c-23
bus: i2c-27 path: 1:0.3:0.0:0.3   type: mux name: i2c-23-mux (chan_id
3) parent: i2c-23
bus: i2c-28 path: 1:0.3:0.0:0.4   type: mux name: i2c-23-mux (chan_id
4) parent: i2c-23
bus: i2c-29 path: 1:0.3:0.0:0.5   type: mux name: i2c-23-mux (chan_id
5) parent: i2c-23
bus: i2c-30 path: 1:0.3:0.0:0.6   type: mux name: i2c-23-mux (chan_id
6) parent: i2c-23
bus: i2c-31 path: 1:0.3:0.0:0.7   type: mux name: i2c-23-mux (chan_id
7) parent: i2c-23
bus: i2c-32 path: 1:0.3:0.0:1.0   type: mux name: i2c-23-mux (chan_id
0) parent: i2c-23
bus: i2c-33 path: 1:0.3:0.0:1.1   type: mux name: i2c-23-mux (chan_id
1) parent: i2c-23
bus: i2c-34 path: 1:0.3:0.0:1.2   type: mux name: i2c-23-mux (chan_id
2) parent: i2c-23
bus: i2c-35 path: 1:0.3:0.0:1.3   type: mux name: i2c-23-mux (chan_id
3) parent: i2c-23
bus: i2c-36 path: 1:0.3:0.0:1.4   type: mux name: i2c-23-mux (chan_id
4) parent: i2c-23
bus: i2c-37 path: 1:0.3:0.0:1.5   type: mux name: i2c-23-mux (chan_id
5) parent: i2c-23
bus: i2c-38 path: 1:0.3:0.0:1.6   type: mux name: i2c-23-mux (chan_id
6) parent: i2c-23
bus: i2c-39 path: 1:0.3:0.0:1.7   type: mux name: i2c-23-mux (chan_id
7) parent: i2c-23
bus: i2c-14 path: 1:0.4           type: mux name: i2c-1-mux (chan_id
4) parent: i2c-1
bus: i2c-19 path: 1:0.4:0.0       type: mux name: i2c-14-mux (chan_id
0) parent: i2c-14
bus: i2c-15 path: 1:0.5           type: mux name: i2c-1-mux (chan_id
5) parent: i2c-1
bus: i2c-20 path: 1:0.5:0.0       type: mux name: i2c-15-mux (chan_id
0) parent: i2c-15
bus: i2c-16 path: 1:0.6           type: mux name: i2c-1-mux (chan_id
6) parent: i2c-1
bus: i2c-21 path: 1:0.6:0.0       type: mux name: i2c-16-mux (chan_id
0) parent: i2c-16
bus: i2c-17 path: 1:0.7           type: mux name: i2c-1-mux (chan_id
7) parent: i2c-1
bus: i2c-22 path: 1:0.7:0.0       type: mux name: i2c-17-mux (chan_id
0) parent: i2c-17
Count: 57
[root@p2020 ~]# ./lsi2c -v -d
Searching for i2c devices
I2C Devices:
bus=0 type=i2c bus_path=0               address=0x70 name=pca9548
   driver=pca954x         module=(null)
bus=6 type=mux bus_path=0:0.4           address=0x68 name=ds1339
  driver=rtc-ds1307     module=(null)
bus=1 type=i2c bus_path=1               address=0x74 name=pca9548
   driver=pca954x         module=(null)
bus=11 type=mux bus_path=1:0.1           address=0x73 name=pca9541
    driver=pca9541         module=(null)
bus=12 type=mux bus_path=1:0.2           address=0x70 name=pca9541
    driver=pca9541         module=(null)
bus=40 type=mux bus_path=1:0.2:0.0       address=0x73 name=pca9547
    driver=pca954x         module=(null)
bus=40 type=mux bus_path=1:0.2:0.0       address=0x71 name=pca9547
    driver=pca954x         module=(null)
bus=13 type=mux bus_path=1:0.3           address=0x70 name=pca9541
    driver=pca9541         module=(null)
bus=23 type=mux bus_path=1:0.3:0.0       address=0x73 name=pca9547
    driver=pca954x         module=(null)
bus=23 type=mux bus_path=1:0.3:0.0       address=0x71 name=pca9547
    driver=pca954x         module=(null)
bus=14 type=mux bus_path=1:0.4           address=0x73 name=pca9541
    driver=pca9541         module=(null)
bus=15 type=mux bus_path=1:0.5           address=0x73 name=pca9541
    driver=pca9541         module=(null)
bus=16 type=mux bus_path=1:0.6           address=0x73 name=pca9541
    driver=pca9541         module=(null)
bus=17 type=mux bus_path=1:0.7           address=0x73 name=pca9541
    driver=pca9541         module=(null)
Count: 10


On Mon, Nov 3, 2014 at 11:33 PM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
>> > What is your muxing setup?
>> >
>> Complicated ;-).
>
> ...
>
> Ooooookay... well, for testing these patches, your setup is absolutely
> awesome, of course :D May I ask what you are doing with that amount of
> I2C?
>



-- 
- Danielle Costantino

Re: [PATCH] i2c: mux: create proper topology in sysfs

[Jean Delvare]

Hi Danielle,

On Tue, 4 Nov 2014 01:07:08 -0800, Danielle Costantino wrote:
> This is a tool I have been developing for complex I2C bus structures
> (lsi2c) the output of this should help with this complex multiplexer
> testing. I was able to overcome the problem by creating persistent
> paths describing where the devices reside. I will be testing
> compatibility with this setup. I will be reporting on my test results
> within a few days.
> 
> [root@p2020 ~]# ./lsi2c -h
> Usage: lsi2c [OPTION]...
>   -c, --config-file     Specify a config file
>   -C, --print-config    Display i2c devices in configuration file
>   -a, --all             Print all i2c-devs in bus tree
>   -d, --print-devices   Display sysfs i2c devices
>   -t, --tree            Print i2c bus and children
>   -p, --path            Parse an i2c-dev path
>   -P, --probe           Probe an i2c-dev at addr on path
>   -F, --func            Print I2C bus functionality
>   -T, --timeout         Set adapter timeout in milliseconds
>   -S, --retry-count     Set adapter max retry count
>   -h, --help            Display this help text
>   -V, --version         Display the program version
>   -v, --verbose         Be verbose
>   -i, --initialize      Initialize devices in configuration file
>   -r, --remove          Remove devices in configuration file
>   -k, --kmod            Try to initialize i2c_dev kernel module
> 
> Use `-' after `-c' to read the config file from stdin.

Is the source code of this tool available somewhere?

Thanks,
-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH] i2c: mux: create proper topology in sysfs

[Danielle Costantino]

lsi2c is still under final development and code review. once its
complete source will be released.

On Mon, Nov 10, 2014 at 12:57 PM, Jean Delvare <jdelvare-l3A5Bk7waGM@public.gmane.org> wrote:
> Hi Danielle,
>
> On Tue, 4 Nov 2014 01:07:08 -0800, Danielle Costantino wrote:
>> This is a tool I have been developing for complex I2C bus structures
>> (lsi2c) the output of this should help with this complex multiplexer
>> testing. I was able to overcome the problem by creating persistent
>> paths describing where the devices reside. I will be testing
>> compatibility with this setup. I will be reporting on my test results
>> within a few days.
>>
>> [root@p2020 ~]# ./lsi2c -h
>> Usage: lsi2c [OPTION]...
>>   -c, --config-file     Specify a config file
>>   -C, --print-config    Display i2c devices in configuration file
>>   -a, --all             Print all i2c-devs in bus tree
>>   -d, --print-devices   Display sysfs i2c devices
>>   -t, --tree            Print i2c bus and children
>>   -p, --path            Parse an i2c-dev path
>>   -P, --probe           Probe an i2c-dev at addr on path
>>   -F, --func            Print I2C bus functionality
>>   -T, --timeout         Set adapter timeout in milliseconds
>>   -S, --retry-count     Set adapter max retry count
>>   -h, --help            Display this help text
>>   -V, --version         Display the program version
>>   -v, --verbose         Be verbose
>>   -i, --initialize      Initialize devices in configuration file
>>   -r, --remove          Remove devices in configuration file
>>   -k, --kmod            Try to initialize i2c_dev kernel module
>>
>> Use `-' after `-c' to read the config file from stdin.
>
> Is the source code of this tool available somewhere?
>
> Thanks,
> --
> Jean Delvare
> SUSE L3 Support



-- 
- Danielle Costantino

Re: [PATCH] i2c: mux: create proper topology in sysfs

[Guenter Roeck]

On 11/03/2014 11:33 PM, Wolfram Sang wrote:
>>> What is your muxing setup?
>>>
>> Complicated ;-).
>
> ...
>
> Ooooookay... well, for testing these patches, your setup is absolutely
> awesome, of course :D May I ask what you are doing with that amount of
> I2C?
>

The HW folks are trying to have one bus per device. So a card with 48x10GE
needs 48 I2C busses, plus one or more busses for its other devices.

Some of the HW tends to block its I2C bus if looked at in a wrong way,
so that isn't actually too bad. The bus master selectors let either
one of the main controller CPUs or the CPU on a line card have access
to the bus.

Guenter

Re: [PATCH] i2c: mux: create proper topology in sysfs

[Guenter Roeck]

On Mon, Nov 03, 2014 at 10:02:00PM +0100, Wolfram Sang wrote:
> > > +	WARN(sysfs_create_link(&parent->dev.kobj, &priv->adap.dev.kobj, priv->adap.dev.kobj.name),
> > > +				"can't create compatibility link for old mux name scheme\n");
> > > +
> > Something seems to be wrong with the linking.
> > 
> > I get lots of messages such as
> > 
> > sysfs: cannot create duplicate filename
> > '/devices/fff70a000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/0000:02:09.0/0000:10:00.0/i2c-sam.16/i2c-88/i2c-89'
> > 
> > followed by "can't create compatibility link for old mux name scheme".
> 
> What is your muxing setup?
> 
Ok, I think I know the problem. In my case, the parent adapter
and the mux device are the same (since both are on the same chip
and the mux is really merged into the i2c controller).

I'll have to figure out how to fix that.

Guenter

Re: [PATCH] i2c: mux: create proper topology in sysfs

[Wolfram Sang]

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


> Ok, I think I know the problem. In my case, the parent adapter
> and the mux device are the same (since both are on the same chip
> and the mux is really merged into the i2c controller).
> 
> I'll have to figure out how to fix that.

Maybe something like this on top helps already? It skips link creation
if the parent device is the parent i2c adapter. We should have this
anyway...

--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -184,8 +184,11 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
 		return NULL;
 	}
 
-	WARN(sysfs_create_link(&parent->dev.kobj, &priv->adap.dev.kobj, priv->adap.dev.kobj.name),
-				"can't create compatibility link for old mux name scheme\n");
+	/* If necessary, create symlink to match old mux name scheme */
+	if (mux_dev != &parent->dev)
+		WARN(sysfs_create_link(&parent->dev.kobj, &priv->adap.dev.kobj,
+					dev_name(&priv->adap.dev)),
+					"can't create compatibility link for old mux name scheme\n");
 
 	dev_info(&parent->dev, "Added multiplexed i2c bus %d\n",
 		 i2c_adapter_id(&priv->adap));
@@ -198,7 +201,8 @@ void i2c_del_mux_adapter(struct i2c_adapter *adap)
 {
 	struct i2c_mux_priv *priv = adap->algo_data;
 
-	sysfs_remove_link(&priv->parent->dev.kobj, adap->dev.kobj.name);
+	if (priv->adap.dev.parent != &priv->parent->dev)
+		sysfs_remove_link(&priv->parent->dev.kobj, dev_name(&adap->dev));
 	i2c_del_adapter(adap);
 	kfree(priv);
 }


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

Re: [PATCH] i2c: mux: create proper topology in sysfs

[Guenter Roeck]

On 11/04/2014 06:33 AM, Wolfram Sang wrote:
>
>> Ok, I think I know the problem. In my case, the parent adapter
>> and the mux device are the same (since both are on the same chip
>> and the mux is really merged into the i2c controller).
>>
>> I'll have to figure out how to fix that.
>
> Maybe something like this on top helps already? It skips link creation
> if the parent device is the parent i2c adapter. We should have this
> anyway...
>

I'll test tonight ... I'll be in meetings all day :-(.

Guenter

> --- a/drivers/i2c/i2c-mux.c
> +++ b/drivers/i2c/i2c-mux.c
> @@ -184,8 +184,11 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
>   		return NULL;
>   	}
>
> -	WARN(sysfs_create_link(&parent->dev.kobj, &priv->adap.dev.kobj, priv->adap.dev.kobj.name),
> -				"can't create compatibility link for old mux name scheme\n");
> +	/* If necessary, create symlink to match old mux name scheme */
> +	if (mux_dev != &parent->dev)
> +		WARN(sysfs_create_link(&parent->dev.kobj, &priv->adap.dev.kobj,
> +					dev_name(&priv->adap.dev)),
> +					"can't create compatibility link for old mux name scheme\n");
>
>   	dev_info(&parent->dev, "Added multiplexed i2c bus %d\n",
>   		 i2c_adapter_id(&priv->adap));
> @@ -198,7 +201,8 @@ void i2c_del_mux_adapter(struct i2c_adapter *adap)
>   {
>   	struct i2c_mux_priv *priv = adap->algo_data;
>
> -	sysfs_remove_link(&priv->parent->dev.kobj, adap->dev.kobj.name);
> +	if (priv->adap.dev.parent != &priv->parent->dev)
> +		sysfs_remove_link(&priv->parent->dev.kobj, dev_name(&adap->dev));
>   	i2c_del_adapter(adap);
>   	kfree(priv);
>   }
>

Re: [PATCH] i2c: mux: create proper topology in sysfs

[Guenter Roeck]

On Tue, Nov 04, 2014 at 03:33:09PM +0100, Wolfram Sang wrote:
> 
> > Ok, I think I know the problem. In my case, the parent adapter
> > and the mux device are the same (since both are on the same chip
> > and the mux is really merged into the i2c controller).
> > 
> > I'll have to figure out how to fix that.
> 
> Maybe something like this on top helps already? It skips link creation
> if the parent device is the parent i2c adapter. We should have this
> anyway...
> 
It fixes the tracebacks, but application code still fails. I'll have to track
that down; most likely it uses a means to find the i2c root adapter which no
longer works. I'll also try your other patch.

Guenter