[PATCH 0/2] i2c: mux: demux-pinctrl: improve device relationships

[PATCH 0/2] i2c: mux: demux-pinctrl: improve device relationships

[Wolfram Sang]

While researching some PM behaviour within I2C, I found out that the i2c-demux
driver does not play well with that due to broken relationship with other
devices. Patch 1 ensures the right parent-child relationship. Patch 2 makes the
connection between the demux device and the demuxed bus similar to that we have
in the mux core.

Tested on a Renesas Lager board (R-Car H2).


Wolfram Sang (2):
  i2c: mux: demux-pinctrl: use proper parent device for demux adapter
  i2c: mux: demux-pinctrl: add symlinks to the demux device in sysfs

 drivers/i2c/muxes/i2c-demux-pinctrl.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

-- 
2.11.0

[PATCH 1/2] i2c: mux: demux-pinctrl: use proper parent device for demux adapter

[Wolfram Sang]

Due to a typo, the wrong parent device was assigned to the newly created
demuxing adapter device. It got connected to the demuxing platform
device but not to the selected parent I2C adapter device. Fix it to get
a proper parent-child relationship of the demuxed busses, needed also for
proper PM.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/muxes/i2c-demux-pinctrl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/muxes/i2c-demux-pinctrl.c b/drivers/i2c/muxes/i2c-demux-pinctrl.c
index 428de4c97fb2..035032e20327 100644
--- a/drivers/i2c/muxes/i2c-demux-pinctrl.c
+++ b/drivers/i2c/muxes/i2c-demux-pinctrl.c
@@ -106,7 +106,7 @@ static int i2c_demux_activate_master(struct i2c_demux_pinctrl_priv *priv, u32 ne
 	priv->cur_adap.owner = THIS_MODULE;
 	priv->cur_adap.algo = &priv->algo;
 	priv->cur_adap.algo_data = priv;
-	priv->cur_adap.dev.parent = priv->dev;
+	priv->cur_adap.dev.parent = &adap->dev;
 	priv->cur_adap.class = adap->class;
 	priv->cur_adap.retries = adap->retries;
 	priv->cur_adap.timeout = adap->timeout;
-- 
2.11.0

[PATCH 2/2] i2c: mux: demux-pinctrl: add symlinks to the demux device in sysfs

[Wolfram Sang]

Similar to mux devices, create special symlinks to connect the demuxed
bus with the demux device.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/muxes/i2c-demux-pinctrl.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/i2c/muxes/i2c-demux-pinctrl.c b/drivers/i2c/muxes/i2c-demux-pinctrl.c
index 035032e20327..d5e7d4aa6ee1 100644
--- a/drivers/i2c/muxes/i2c-demux-pinctrl.c
+++ b/drivers/i2c/muxes/i2c-demux-pinctrl.c
@@ -116,6 +116,11 @@ static int i2c_demux_activate_master(struct i2c_demux_pinctrl_priv *priv, u32 ne
 	if (ret < 0)
 		goto err_with_put;
 
+	sysfs_create_link(&priv->cur_adap.dev.kobj, &priv->dev->kobj,
+			       "demux_device");
+	sysfs_create_link(&priv->dev->kobj, &priv->cur_adap.dev.kobj,
+			       "channel-0");
+
 	return 0;
 
  err_with_put:
@@ -135,6 +140,9 @@ static int i2c_demux_deactivate_master(struct i2c_demux_pinctrl_priv *priv)
 	if (cur < 0)
 		return 0;
 
+	sysfs_remove_link(&priv->dev->kobj, "channel-0");
+	sysfs_remove_link(&priv->cur_adap.dev.kobj, "demux_device");
+
 	i2c_del_adapter(&priv->cur_adap);
 	i2c_put_adapter(priv->chan[cur].parent_adap);
 
-- 
2.11.0

Re: [PATCH 0/2] i2c: mux: demux-pinctrl: improve device relationships

[Wolfram Sang]

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

On Mon, Apr 30, 2018 at 01:55:42PM +0200, Wolfram Sang wrote:
> While researching some PM behaviour within I2C, I found out that the i2c-demux
> driver does not play well with that due to broken relationship with other
> devices. Patch 1 ensures the right parent-child relationship. Patch 2 makes the
> connection between the demux device and the demuxed bus similar to that we have
> in the mux core.
> 
> Tested on a Renesas Lager board (R-Car H2).

Peter, I'd think these demux patches (and the single one later) should
go via your tree. Let me know if you prefer that I pick them up.


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

Re: [PATCH 0/2] i2c: mux: demux-pinctrl: improve device relationships

[Peter Rosin]

On 2018-05-17 16:11, Wolfram Sang wrote:
> On Mon, Apr 30, 2018 at 01:55:42PM +0200, Wolfram Sang wrote:
>> While researching some PM behaviour within I2C, I found out that the i2c-demux
>> driver does not play well with that due to broken relationship with other
>> devices. Patch 1 ensures the right parent-child relationship. Patch 2 makes the
>> connection between the demux device and the demuxed bus similar to that we have
>> in the mux core.
>>
>> Tested on a Renesas Lager board (R-Car H2).
> 
> Peter, I'd think these demux patches (and the single one later) should
> go via your tree. Let me know if you prefer that I pick them up.

Agreed, sorry for being a bit slow. I'll add some comments to the individual
patches...

Cheers,
Peter

Re: [PATCH 1/2] i2c: mux: demux-pinctrl: use proper parent device for demux adapter

[Peter Rosin]

On 2018-04-30 13:55, Wolfram Sang wrote:
> Due to a typo, the wrong parent device was assigned to the newly created
> demuxing adapter device. It got connected to the demuxing platform
> device but not to the selected parent I2C adapter device. Fix it to get
> a proper parent-child relationship of the demuxed busses, needed also for
> proper PM.
> 

Should this one have a Fixes: tag? Should it go to current or next? Is
a backport to stable good enough?

Cheers,
Peter

> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>  drivers/i2c/muxes/i2c-demux-pinctrl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/muxes/i2c-demux-pinctrl.c b/drivers/i2c/muxes/i2c-demux-pinctrl.c
> index 428de4c97fb2..035032e20327 100644
> --- a/drivers/i2c/muxes/i2c-demux-pinctrl.c
> +++ b/drivers/i2c/muxes/i2c-demux-pinctrl.c
> @@ -106,7 +106,7 @@ static int i2c_demux_activate_master(struct i2c_demux_pinctrl_priv *priv, u32 ne
>  	priv->cur_adap.owner = THIS_MODULE;
>  	priv->cur_adap.algo = &priv->algo;
>  	priv->cur_adap.algo_data = priv;
> -	priv->cur_adap.dev.parent = priv->dev;
> +	priv->cur_adap.dev.parent = &adap->dev;
>  	priv->cur_adap.class = adap->class;
>  	priv->cur_adap.retries = adap->retries;
>  	priv->cur_adap.timeout = adap->timeout;
> 

Re: [PATCH 2/2] i2c: mux: demux-pinctrl: add symlinks to the demux device in sysfs

[Peter Rosin]

On 2018-04-30 13:55, Wolfram Sang wrote:
> Similar to mux devices, create special symlinks to connect the demuxed
> bus with the demux device.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>  drivers/i2c/muxes/i2c-demux-pinctrl.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/i2c/muxes/i2c-demux-pinctrl.c b/drivers/i2c/muxes/i2c-demux-pinctrl.c
> index 035032e20327..d5e7d4aa6ee1 100644
> --- a/drivers/i2c/muxes/i2c-demux-pinctrl.c
> +++ b/drivers/i2c/muxes/i2c-demux-pinctrl.c
> @@ -116,6 +116,11 @@ static int i2c_demux_activate_master(struct i2c_demux_pinctrl_priv *priv, u32 ne
>  	if (ret < 0)
>  		goto err_with_put;
>  
> +	sysfs_create_link(&priv->cur_adap.dev.kobj, &priv->dev->kobj,
> +			       "demux_device");
> +	sysfs_create_link(&priv->dev->kobj, &priv->cur_adap.dev.kobj,
> +			       "channel-0");
> +

>From sysfs.h:
int __must_check sysfs_create_link(...);

Cheers,
Peter

>  	return 0;
>  
>   err_with_put:
> @@ -135,6 +140,9 @@ static int i2c_demux_deactivate_master(struct i2c_demux_pinctrl_priv *priv)
>  	if (cur < 0)
>  		return 0;
>  
> +	sysfs_remove_link(&priv->dev->kobj, "channel-0");
> +	sysfs_remove_link(&priv->cur_adap.dev.kobj, "demux_device");
> +
>  	i2c_del_adapter(&priv->cur_adap);
>  	i2c_put_adapter(priv->chan[cur].parent_adap);
>  
> 

Re: [PATCH 1/2] i2c: mux: demux-pinctrl: use proper parent device for demux adapter

[Wolfram Sang]

On Sat, May 19, 2018 at 11:40:04PM +0200, Peter Rosin wrote:
> On 2018-04-30 13:55, Wolfram Sang wrote:
> > Due to a typo, the wrong parent device was assigned to the newly created
> > demuxing adapter device. It got connected to the demuxing platform
> > device but not to the selected parent I2C adapter device. Fix it to get
> > a proper parent-child relationship of the demuxed busses, needed also for
> > proper PM.
> > 
> 
> Should this one have a Fixes: tag? Should it go to current or next? Is
> a backport to stable good enough?

A Fixes tag is probably apropriate. I don't think it should go to
stable, though, because it will break a scheme a user might be using.
This can't be avoided for a kernel upgrade here, but IMO we shouldn't
enforce it for a stable update. Especially, given this is not a real
bug, but more something improper. For the same reason, I'd think -next
is good enough.

Thanks, will resend!

   Wolfram

Re: [PATCH 2/2] i2c: mux: demux-pinctrl: add symlinks to the demux device in sysfs

[Wolfram Sang]

> > +	sysfs_create_link(&priv->cur_adap.dev.kobj, &priv->dev->kobj,
> > +			       "demux_device");
> > +	sysfs_create_link(&priv->dev->kobj, &priv->cur_adap.dev.kobj,
> > +			       "channel-0");
> > +
> 
> From sysfs.h:
> int __must_check sysfs_create_link(...);

Darn, right... Will fix!

Re: [PATCH 1/2] i2c: mux: demux-pinctrl: use proper parent device for demux adapter

[Peter Rosin]

On 2018-05-20 08:45, Wolfram Sang wrote:
> On Sat, May 19, 2018 at 11:40:04PM +0200, Peter Rosin wrote:
>> On 2018-04-30 13:55, Wolfram Sang wrote:
>>> Due to a typo, the wrong parent device was assigned to the newly created
>>> demuxing adapter device. It got connected to the demuxing platform
>>> device but not to the selected parent I2C adapter device. Fix it to get
>>> a proper parent-child relationship of the demuxed busses, needed also for
>>> proper PM.
>>>
>>
>> Should this one have a Fixes: tag? Should it go to current or next? Is
>> a backport to stable good enough?
> 
> A Fixes tag is probably apropriate. I don't think it should go to
> stable, though, because it will break a scheme a user might be using.
> This can't be avoided for a kernel upgrade here, but IMO we shouldn't
> enforce it for a stable update. Especially, given this is not a real
> bug, but more something improper. For the same reason, I'd think -next
> is good enough.
> 
> Thanks, will resend!

Hmm, in that case, I think a Fixes tag is best left out, because I suspect
the net effect is mostly a bunch of mails from the stable trees as the
automatic patch-picker finds this patch. So, I plan to go with this patch.

Ok?

Cheers,
Peter