diff mbox

[RFC] i2c: mux: add mux device to the adapter name

Message ID 1415048456-13456-1-git-send-email-wsa@the-dreams.de
State Rejected
Headers show

Commit Message

Wolfram Sang Nov. 3, 2014, 9 p.m. UTC
If there are multiple muxes on one bus, then specifying the channel only
is not sufficient for a distinguishable name. We need the actual device,
too.

Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
Cc: Jean Delvare <jdelvare@suse.de>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Martin Belanger <martin.belanger@cyaninc.com>
Cc: Rodolfo Giometti <giometti@enneenne.com>
Cc: Michael Lawnick <ml.lawnick@gmx.de>
Cc: Jeroen De Wachter <jeroen.de.wachter@telenet.be>
---

This is probably the least "ABI" breaking solution? RFC for now...

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

Comments

Guenter Roeck Nov. 5, 2014, 6:38 p.m. UTC | #1
On Mon, Nov 03, 2014 at 10:00:56PM +0100, Wolfram Sang wrote:
> If there are multiple muxes on one bus, then specifying the channel only
> is not sufficient for a distinguishable name. We need the actual device,
> too.
> 
> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
> Cc: Jean Delvare <jdelvare@suse.de>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Martin Belanger <martin.belanger@cyaninc.com>
> Cc: Rodolfo Giometti <giometti@enneenne.com>
> Cc: Michael Lawnick <ml.lawnick@gmx.de>
> Cc: Jeroen De Wachter <jeroen.de.wachter@telenet.be>
> ---
> 
> This is probably the least "ABI" breaking solution? RFC for now...
> 
>  drivers/i2c/i2c-mux.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
> index 5b482ea32faf..26aa84902ada 100644
> --- a/drivers/i2c/i2c-mux.c
> +++ b/drivers/i2c/i2c-mux.c
> @@ -134,7 +134,8 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
>  
>  	/* Now fill out new adapter structure */
>  	snprintf(priv->adap.name, sizeof(priv->adap.name),
> -		 "i2c-%d-mux (chan_id %d)", i2c_adapter_id(parent), chan_id);
> +		 "i2c-%d-mux (chan_id %d) (mux_device %s)",
> +		 i2c_adapter_id(parent), chan_id, dev_name(mux_dev));

This yields pretty long names, longer than the maximum supported length,
if the mux is not an i2c adapter (eg i2c-mux-pinctrl).

i2c-17-mux (chan_id 5) (mux_device i2c-mux-pinctrl)

has 52 characters, and the maximum name length is 48.
Maybe just use "mux" instead of "mux_adapter" ?

The result still fails for me because the application code doesn't expect
to see "(mux...)", but I guess there will always be some problem :-(.

I still need to figure out what causes the failure with the other patch.

Guenter

>  	priv->adap.owner = THIS_MODULE;
>  	priv->adap.algo = &priv->algo;
>  	priv->adap.algo_data = priv;
> -- 
> 2.1.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wolfram Sang Nov. 7, 2014, 11:52 a.m. UTC | #2
On Wed, Nov 05, 2014 at 10:38:34AM -0800, Guenter Roeck wrote:
> On Mon, Nov 03, 2014 at 10:00:56PM +0100, Wolfram Sang wrote:
> > If there are multiple muxes on one bus, then specifying the channel only
> > is not sufficient for a distinguishable name. We need the actual device,
> > too.
> > 
> > Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
> > Cc: Jean Delvare <jdelvare@suse.de>
> > Cc: Guenter Roeck <linux@roeck-us.net>
> > Cc: Martin Belanger <martin.belanger@cyaninc.com>
> > Cc: Rodolfo Giometti <giometti@enneenne.com>
> > Cc: Michael Lawnick <ml.lawnick@gmx.de>
> > Cc: Jeroen De Wachter <jeroen.de.wachter@telenet.be>
> > ---
> > 
> > This is probably the least "ABI" breaking solution? RFC for now...
> > 
> >  drivers/i2c/i2c-mux.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
> > index 5b482ea32faf..26aa84902ada 100644
> > --- a/drivers/i2c/i2c-mux.c
> > +++ b/drivers/i2c/i2c-mux.c
> > @@ -134,7 +134,8 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
> >  
> >  	/* Now fill out new adapter structure */
> >  	snprintf(priv->adap.name, sizeof(priv->adap.name),
> > -		 "i2c-%d-mux (chan_id %d)", i2c_adapter_id(parent), chan_id);
> > +		 "i2c-%d-mux (chan_id %d) (mux_device %s)",
> > +		 i2c_adapter_id(parent), chan_id, dev_name(mux_dev));
> 
> This yields pretty long names, longer than the maximum supported length,
> if the mux is not an i2c adapter (eg i2c-mux-pinctrl).
> 
> i2c-17-mux (chan_id 5) (mux_device i2c-mux-pinctrl)
> 
> has 52 characters, and the maximum name length is 48.
> Maybe just use "mux" instead of "mux_adapter" ?

Argh, right. With that length limit, it doesn't make sense to use %s, at
all. For DT the name comes from the node name and that could be even
longer, so shortening to "mux" wouldn't help much.

> The result still fails for me because the application code doesn't expect
> to see "(mux...)", but I guess there will always be some problem :-(.

Yes, because it is ABI breakage. Honestly, with that size limit as
another obstacle, I think we should leave the 'name' file as it is and
use proper topology.

> I still need to figure out what causes the failure with the other patch.

Yes, please. Thanks for doing that!

   Wolfram
diff mbox

Patch

diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index 5b482ea32faf..26aa84902ada 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -134,7 +134,8 @@  struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
 
 	/* Now fill out new adapter structure */
 	snprintf(priv->adap.name, sizeof(priv->adap.name),
-		 "i2c-%d-mux (chan_id %d)", i2c_adapter_id(parent), chan_id);
+		 "i2c-%d-mux (chan_id %d) (mux_device %s)",
+		 i2c_adapter_id(parent), chan_id, dev_name(mux_dev));
 	priv->adap.owner = THIS_MODULE;
 	priv->adap.algo = &priv->algo;
 	priv->adap.algo_data = priv;