[PATCH v3 1/2] i2c: mux: create symlink to actual mux device

[PATCH v3 1/2] i2c: mux: create symlink to actual mux device

[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.

It could be argued that the parent device should be changed from the
parent adapter to the mux device. This has pros and cons. To improve the
topology, simply add a "muxed_device" symlink pointing to the actual
muxing device, so we can distinguish muxed busses. Doing it this way, we
don't break the ABI.

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>
Cc: Gerlando Falauto <gerlando.falauto-SkAbAL50j+5BDgjK7y7TUQ@public.gmane.org>
---
 drivers/i2c/i2c-mux.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index 2d0847b6be62..f246a9f7dd22 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -183,6 +183,9 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
 		return NULL;
 	}
 
+	WARN(sysfs_create_link(&priv->adap.dev.kobj, &mux_dev->kobj, "mux_device"),
+			       "can't create symlink to mux device\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->adap.dev.kobj, "mux_device");
 	i2c_del_adapter(adap);
 	kfree(priv);
 }
-- 
2.1.1

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

[Wolfram Sang]

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

This makes the topology clearer. 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>
[wsa: simplified sysfs-usage and fixed format string usage]
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>
---
 drivers/i2c/i2c-mux.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index f246a9f7dd22..593f7ca9adc7 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -110,6 +110,7 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
 						 void *, u32))
 {
 	struct i2c_mux_priv *priv;
+	char symlink_name[20];
 	int ret;
 
 	priv = kzalloc(sizeof(struct i2c_mux_priv), GFP_KERNEL);
@@ -186,6 +187,9 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
 	WARN(sysfs_create_link(&priv->adap.dev.kobj, &mux_dev->kobj, "mux_device"),
 			       "can't create symlink to mux device\n");
 
+	snprintf(symlink_name, sizeof(symlink_name), "channel-%u", chan_id);
+	WARN(sysfs_create_link(&mux_dev->kobj, &priv->adap.dev.kobj, symlink_name),
+			       "can't create symlink for channel %u\n", chan_id);
 	dev_info(&parent->dev, "Added multiplexed i2c bus %d\n",
 		 i2c_adapter_id(&priv->adap));
 
@@ -196,6 +200,10 @@ EXPORT_SYMBOL_GPL(i2c_add_mux_adapter);
 void i2c_del_mux_adapter(struct i2c_adapter *adap)
 {
 	struct i2c_mux_priv *priv = adap->algo_data;
+	char symlink_name[20];
+
+	snprintf(symlink_name, sizeof(symlink_name), "channel-%u", priv->chan_id);
+	sysfs_remove_link(&adap->dev.parent->kobj, symlink_name);
 
 	sysfs_remove_link(&priv->adap.dev.kobj, "mux_device");
 	i2c_del_adapter(adap);
-- 
2.1.1

Re: [PATCH v3 2/2] i2c: mux: create "channel-n" symlinks for child segments in the mux device

[Wolfram Sang]

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

On Thu, Nov 13, 2014 at 02:39:56PM +0100, Wolfram Sang wrote:
> From: Gerlando Falauto <gerlando.falauto-SkAbAL50j+5BDgjK7y7TUQ@public.gmane.org>
> 
> This makes the topology clearer. 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>
> [wsa: simplified sysfs-usage and fixed format string usage]
> 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>

Applied to for-next, thanks!


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

Re: [PATCH v3 2/2] i2c: mux: create "channel-n" symlinks for child segments in the mux device

[Danielle Costantino]

I know this has already been applied but I wanted to comment that on
embedded systems this patch increases the probe time of mux devices.
Otherwise the patch works and provides much needed parent device
information without breaking the api.

On Tue, Nov 18, 2014 at 7:12 AM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
> On Thu, Nov 13, 2014 at 02:39:56PM +0100, Wolfram Sang wrote:
>> From: Gerlando Falauto <gerlando.falauto-SkAbAL50j+5BDgjK7y7TUQ@public.gmane.org>
>>
>> This makes the topology clearer. 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>
>> [wsa: simplified sysfs-usage and fixed format string usage]
>> 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>
>
> Applied to for-next, thanks!
>



-- 
- Danielle Costantino

Re: [PATCH v3 2/2] i2c: mux: create "channel-n" symlinks for child segments in the mux device

[Wolfram Sang]

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

On Sat, Nov 29, 2014 at 03:29:46PM -0800, Danielle Costantino wrote:

> I know this has already been applied but I wanted to comment that on
> embedded systems this patch increases the probe time of mux devices.

In what magnitude is the delay? And in what setup? How did you measure?

I have doubts. This patch just adds a symlink to the sysfs filesystem.
sysfs is not needed for probing.

> Otherwise the patch works and provides much needed parent device
> information without breaking the api.

Any news on lsi2c, by the way?


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

Re: [PATCH v3 1/2] i2c: mux: create symlink to actual mux device

[Guenter Roeck]

On 11/13/2014 05:39 AM, 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.
>
> It could be argued that the parent device should be changed from the
> parent adapter to the mux device. This has pros and cons. To improve the
> topology, simply add a "muxed_device" symlink pointing to the actual
> muxing device, so we can distinguish muxed busses. Doing it this way, we
> don't break the ABI.
>
> 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>
> Cc: Gerlando Falauto <gerlando.falauto-SkAbAL50j+5BDgjK7y7TUQ@public.gmane.org>

This version works for me and passes testing with our application code.

Tested-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>

> ---
>   drivers/i2c/i2c-mux.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
> index 2d0847b6be62..f246a9f7dd22 100644
> --- a/drivers/i2c/i2c-mux.c
> +++ b/drivers/i2c/i2c-mux.c
> @@ -183,6 +183,9 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
>   		return NULL;
>   	}
>
> +	WARN(sysfs_create_link(&priv->adap.dev.kobj, &mux_dev->kobj, "mux_device"),
> +			       "can't create symlink to mux device\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->adap.dev.kobj, "mux_device");
>   	i2c_del_adapter(adap);
>   	kfree(priv);
>   }
>

Re: [PATCH v3 1/2] i2c: mux: create symlink to actual mux device

[Jeroen De Wachter]

Hey guys,

It's been a while since I contacted you about this issue and I know I haven't been helping to solve it (because there were/are more pressing issues, as usual).

We're working with an older kernel here and have found a way of working around this issue (there's only one application adding muxes, so I'm scanning the i2c busses before and after adding the mux to figure out which busses were added by the new mux). Our solution works for the moment (although it's not as clean, obviously), so I don't think we'll be backporting the changes, but it's nice to know it's being solved. If we ever switch to a kernel that has these changes, we can do a better job with our application :-)

I justed wanted to say thank you for keeping me in the loop :-)

Regards,
Jeroen


----- Original Message -----
From: "Guenter Roeck" <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
To: "Wolfram Sang" <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>, linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: "Jean Delvare" <jdelvare-l3A5Bk7waGM@public.gmane.org>, "Martin Belanger" <martin.belanger-Ir6+u9MVKBtBDgjK7y7TUQ@public.gmane.org>, "Rodolfo Giometti" <giometti-AVVDYK/kqiJWk0Htik3J/w@public.gmane.org>, "Michael Lawnick" <ml.lawnick-Mmb7MZpHnFY@public.gmane.org>, "Jeroen De Wachter" <jeroen.de.wachter-CNXmb7IdZIWZIoH1IeqzKA@public.gmane.org>, "Gerlando Falauto" <gerlando.falauto-SkAbAL50j+5BDgjK7y7TUQ@public.gmane.org>
Sent: Friday, November 14, 2014 4:55:45 AM
Subject: Re: [PATCH v3 1/2] i2c: mux: create symlink to actual mux device

On 11/13/2014 05:39 AM, 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.
>
> It could be argued that the parent device should be changed from the
> parent adapter to the mux device. This has pros and cons. To improve the
> topology, simply add a "muxed_device" symlink pointing to the actual
> muxing device, so we can distinguish muxed busses. Doing it this way, we
> don't break the ABI.
>
> 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>
> Cc: Gerlando Falauto <gerlando.falauto-SkAbAL50j+5BDgjK7y7TUQ@public.gmane.org>

This version works for me and passes testing with our application code.

Tested-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>

> ---
>   drivers/i2c/i2c-mux.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
> index 2d0847b6be62..f246a9f7dd22 100644
> --- a/drivers/i2c/i2c-mux.c
> +++ b/drivers/i2c/i2c-mux.c
> @@ -183,6 +183,9 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
>   		return NULL;
>   	}
>
> +	WARN(sysfs_create_link(&priv->adap.dev.kobj, &mux_dev->kobj, "mux_device"),
> +			       "can't create symlink to mux device\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->adap.dev.kobj, "mux_device");
>   	i2c_del_adapter(adap);
>   	kfree(priv);
>   }
>

Re: [PATCH v3 1/2] i2c: mux: create symlink to actual mux device

[Wolfram Sang]

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


> I justed wanted to say thank you for keeping me in the loop :-)

You are welcome. And even if you don't test/backport the changes, you
can still donate an Acked-by: if you think the patches are useful and
going to the right direction. If not, then not :)

Thanks,

   Wolfram


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

Re: [PATCH v3 1/2] i2c: mux: create symlink to actual mux device

[Wolfram Sang]

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

On Thu, Nov 13, 2014 at 02:39:55PM +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.
> 
> It could be argued that the parent device should be changed from the
> parent adapter to the mux device. This has pros and cons. To improve the
> topology, simply add a "muxed_device" symlink pointing to the actual
> muxing device, so we can distinguish muxed busses. Doing it this way, we
> don't break the ABI.
> 
> 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>
> Cc: Gerlando Falauto <gerlando.falauto-SkAbAL50j+5BDgjK7y7TUQ@public.gmane.org>

Applied to for-next, thanks!


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