[v3,1/2] dt-bindings: i2c: support property idle-state

[v3,1/2] dt-bindings: i2c: support property idle-state

[Biwen Li]

This supports property idle-state

Signed-off-by: Biwen Li <biwen.li@nxp.com>
---
Change in v3:
	- update subject and description
	- add some information for property idle-state

Change in v2:
	- update subject and description
	- add property idle-state

 Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
index 30ac6a60f041..7abda506b828 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
@@ -25,6 +25,8 @@ Required Properties:
 Optional Properties:
 
   - reset-gpios: Reference to the GPIO connected to the reset input.
+  - idle-state: if present, overrides i2c-mux-idle-disconnect,
+    Please refer to Documentation/devicetree/bindings/mux/mux-controller.txt
   - i2c-mux-idle-disconnect: Boolean; if defined, forces mux to disconnect all
     children in idle state. This is necessary for example, if there are several
     multiplexers on the bus and the devices behind them use same I2C addresses.
-- 
2.17.1

[v3,2/2] i2c: mux: pca954x: support property idle-state

[Biwen Li]

This supports property idle-state

Signed-off-by: Biwen Li <biwen.li@nxp.com>
---
Change in v3:
	- update subject and description
	- add a helper function pca954x_calculate_chan()

Change in v2:
	- update subject and description
	- add property idle-state

 drivers/i2c/muxes/i2c-mux-pca954x.c | 64 ++++++++++++++++++-----------
 1 file changed, 39 insertions(+), 25 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index 923aa3a5a3dc..8777d429269c 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -86,7 +86,7 @@ struct pca954x {
 
 	u8 last_chan;		/* last register value */
 	/* MUX_IDLE_AS_IS, MUX_IDLE_DISCONNECT or >= 0 for channel */
-	s8 idle_state;
+	s32 idle_state;
 
 	struct i2c_client *client;
 
@@ -229,22 +229,25 @@ static int pca954x_reg_write(struct i2c_adapter *adap,
 				I2C_SMBUS_BYTE, &dummy);
 }
 
+static int pca954x_calculate_chan(struct pca954x *data, u32 chan)
+{
+	/* we make switches look like muxes, not sure how to be smarter */
+	if (data->chip->muxtype == pca954x_ismux)
+		return chan | data->chip->enable;
+	else
+		return 1 << chan;
+}
+
 static int pca954x_select_chan(struct i2c_mux_core *muxc, u32 chan)
 {
 	struct pca954x *data = i2c_mux_priv(muxc);
 	struct i2c_client *client = data->client;
-	const struct chip_desc *chip = data->chip;
 	u8 regval;
 	int ret = 0;
 
-	/* we make switches look like muxes, not sure how to be smarter */
-	if (chip->muxtype == pca954x_ismux)
-		regval = chan | chip->enable;
-	else
-		regval = 1 << chan;
-
+	regval = pca954x_calculate_chan(data, chan);
 	/* Only select the channel if its different from the last channel */
-	if (data->last_chan != regval) {
+	if ((data->last_chan & 0xff) != regval) {
 		ret = pca954x_reg_write(muxc->parent, client, regval);
 		data->last_chan = ret < 0 ? 0 : regval;
 	}
@@ -256,7 +259,7 @@ static int pca954x_deselect_mux(struct i2c_mux_core *muxc, u32 chan)
 {
 	struct pca954x *data = i2c_mux_priv(muxc);
 	struct i2c_client *client = data->client;
-	s8 idle_state;
+	s32 idle_state;
 
 	idle_state = READ_ONCE(data->idle_state);
 	if (idle_state >= 0)
@@ -402,6 +405,23 @@ static void pca954x_cleanup(struct i2c_mux_core *muxc)
 	i2c_mux_del_adapters(muxc);
 }
 
+static int pca954x_init(struct i2c_client *client, struct pca954x *data)
+{
+	/*
+	 * Write the mux register at addr to verify
+	 * that the mux is in fact present. This also
+	 * initializes the mux to a channel
+	 * or disconnected state.
+	 */
+	if (data->idle_state >= 0) {
+		data->last_chan = pca954x_calculate_chan(data, data->idle_state);
+	} else {
+		/* Disconnect multiplexer */
+		data->last_chan = 0;
+	}
+	return i2c_smbus_write_byte(client, data->last_chan);
+}
+
 /*
  * I2C init/probing/exit functions
  */
@@ -411,7 +431,6 @@ static int pca954x_probe(struct i2c_client *client,
 	struct i2c_adapter *adap = client->adapter;
 	struct device *dev = &client->dev;
 	struct device_node *np = dev->of_node;
-	bool idle_disconnect_dt;
 	struct gpio_desc *gpio;
 	struct i2c_mux_core *muxc;
 	struct pca954x *data;
@@ -462,22 +481,18 @@ static int pca954x_probe(struct i2c_client *client,
 		}
 	}
 
-	/* Write the mux register at addr to verify
-	 * that the mux is in fact present. This also
-	 * initializes the mux to disconnected state.
-	 */
-	if (i2c_smbus_write_byte(client, 0) < 0) {
+	data->idle_state = MUX_IDLE_AS_IS;
+	if (np && of_property_read_u32(np, "idle-state", &data->idle_state)) {
+		if (np && of_property_read_bool(np, "i2c-mux-idle-disconnect"))
+			data->idle_state = MUX_IDLE_DISCONNECT;
+	}
+
+	ret = pca954x_init(client, data);
+	if (ret < 0) {
 		dev_warn(dev, "probe failed\n");
 		return -ENODEV;
 	}
 
-	data->last_chan = 0;		   /* force the first selection */
-	data->idle_state = MUX_IDLE_AS_IS;
-
-	idle_disconnect_dt = np &&
-		of_property_read_bool(np, "i2c-mux-idle-disconnect");
-	if (idle_disconnect_dt)
-		data->idle_state = MUX_IDLE_DISCONNECT;
 
 	ret = pca954x_irq_setup(muxc);
 	if (ret)
@@ -531,8 +546,7 @@ static int pca954x_resume(struct device *dev)
 	struct i2c_mux_core *muxc = i2c_get_clientdata(client);
 	struct pca954x *data = i2c_mux_priv(muxc);
 
-	data->last_chan = 0;
-	return i2c_smbus_write_byte(client, 0);
+	return pca954x_init(client, data);
 }
 #endif
 
-- 
2.17.1

Re: [v3,2/2] i2c: mux: pca954x: support property idle-state

[Peter Rosin]

On 2019-10-16 06:09, Biwen Li wrote:
> This supports property idle-state
> 
> Signed-off-by: Biwen Li <biwen.li@nxp.com>
> ---
> Change in v3:
> 	- update subject and description
> 	- add a helper function pca954x_calculate_chan()
> 
> Change in v2:
> 	- update subject and description
> 	- add property idle-state
> 
>  drivers/i2c/muxes/i2c-mux-pca954x.c | 64 ++++++++++++++++++-----------
>  1 file changed, 39 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
> index 923aa3a5a3dc..8777d429269c 100644
> --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
> +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
> @@ -86,7 +86,7 @@ struct pca954x {
>  
>  	u8 last_chan;		/* last register value */
>  	/* MUX_IDLE_AS_IS, MUX_IDLE_DISCONNECT or >= 0 for channel */
> -	s8 idle_state;
> +	s32 idle_state;
>  
>  	struct i2c_client *client;
>  
> @@ -229,22 +229,25 @@ static int pca954x_reg_write(struct i2c_adapter *adap,
>  				I2C_SMBUS_BYTE, &dummy);
>  }
>  
> +static int pca954x_calculate_chan(struct pca954x *data, u32 chan)

Should return u8, and "chan" is not what is calculated. Perhaps name
the function pca954x_regval?

(Yes, last_chan is also clearly a bad name, and I suspect you may have
 based this name on it, but changing that is a separate patch.)

> +{
> +	/* we make switches look like muxes, not sure how to be smarter */
> +	if (data->chip->muxtype == pca954x_ismux)
> +		return chan | data->chip->enable;
> +	else
> +		return 1 << chan;
> +}
> +
>  static int pca954x_select_chan(struct i2c_mux_core *muxc, u32 chan)
>  {
>  	struct pca954x *data = i2c_mux_priv(muxc);
>  	struct i2c_client *client = data->client;
> -	const struct chip_desc *chip = data->chip;
>  	u8 regval;
>  	int ret = 0;
>  
> -	/* we make switches look like muxes, not sure how to be smarter */
> -	if (chip->muxtype == pca954x_ismux)
> -		regval = chan | chip->enable;
> -	else
> -		regval = 1 << chan;
> -
> +	regval = pca954x_calculate_chan(data, chan);

I think I would have kept the empty line here. Not important...

>  	/* Only select the channel if its different from the last channel */
> -	if (data->last_chan != regval) {
> +	if ((data->last_chan & 0xff) != regval) {

The changes on this line are not needed (last_chan and regval are
both u8) and just clutters up the code.

>  		ret = pca954x_reg_write(muxc->parent, client, regval);
>  		data->last_chan = ret < 0 ? 0 : regval;
>  	}
> @@ -256,7 +259,7 @@ static int pca954x_deselect_mux(struct i2c_mux_core *muxc, u32 chan)
>  {
>  	struct pca954x *data = i2c_mux_priv(muxc);
>  	struct i2c_client *client = data->client;
> -	s8 idle_state;
> +	s32 idle_state;
>  
>  	idle_state = READ_ONCE(data->idle_state);
>  	if (idle_state >= 0)
> @@ -402,6 +405,23 @@ static void pca954x_cleanup(struct i2c_mux_core *muxc)
>  	i2c_mux_del_adapters(muxc);
>  }
>  
> +static int pca954x_init(struct i2c_client *client, struct pca954x *data)
> +{
> +	/*
> +	 * Write the mux register at addr to verify
> +	 * that the mux is in fact present. This also
> +	 * initializes the mux to a channel
> +	 * or disconnected state.
> +	 */

Again, this comment belongs in pca954x_probe before the call to this function.
It does not apply (at least not the first sentence) when pca954x_init is called
from pca954x_resume.

Hmmm, it could be argued that specifying MUX_IDLE_AS_IS should not trigger a
disconnect on init (since the mux is always idle at init) and that some other
method should be used to determine if the chip is present. The difference is
that with the idle-state property you can explicitly request MUX_IDLE_AS_IS,
while the old code only had some default behavior if i2c-mux-idle-disconnect
was not present.

The easy way out of this is to, in the binding, document the situation and
say that "idle-state = <MUX_IDLE_AS_IS>;" is not supported but that similar
functionality can be obtained by leaving out both the i2c-mux-idle-disconnect
and idle-state properties.

> +	if (data->idle_state >= 0) {
> +		data->last_chan = pca954x_calculate_chan(data, data->idle_state);
> +	} else {
> +		/* Disconnect multiplexer */
> +		data->last_chan = 0;
> +	}
> +	return i2c_smbus_write_byte(client, data->last_chan);
> +}
> +
>  /*
>   * I2C init/probing/exit functions
>   */
> @@ -411,7 +431,6 @@ static int pca954x_probe(struct i2c_client *client,
>  	struct i2c_adapter *adap = client->adapter;
>  	struct device *dev = &client->dev;
>  	struct device_node *np = dev->of_node;
> -	bool idle_disconnect_dt;
>  	struct gpio_desc *gpio;
>  	struct i2c_mux_core *muxc;
>  	struct pca954x *data;
> @@ -462,22 +481,18 @@ static int pca954x_probe(struct i2c_client *client,
>  		}
>  	}
>  
> -	/* Write the mux register at addr to verify
> -	 * that the mux is in fact present. This also
> -	 * initializes the mux to disconnected state.
> -	 */
> -	if (i2c_smbus_write_byte(client, 0) < 0) {
> +	data->idle_state = MUX_IDLE_AS_IS;
> +	if (np && of_property_read_u32(np, "idle-state", &data->idle_state)) {
> +		if (np && of_property_read_bool(np, "i2c-mux-idle-disconnect"))

You do not need to do the "np &&" part for both ifs, since it's already know
that np is non-NULL when you hit the second if. But, it's a NOP in the
first if, since of_property_read_u32 returns -EINVAL if it is. So, I suggest

	if (of_property_read_u32(np, "idle-state", &data->idle_state)) {
		if (np && of_property_read_bool(np, "i2c-mux-idle-disconnect"))

Cheers,
Peter

> +			data->idle_state = MUX_IDLE_DISCONNECT;
> +	}
> +
> +	ret = pca954x_init(client, data);
> +	if (ret < 0) {
>  		dev_warn(dev, "probe failed\n");
>  		return -ENODEV;
>  	}
>  
> -	data->last_chan = 0;		   /* force the first selection */
> -	data->idle_state = MUX_IDLE_AS_IS;
> -
> -	idle_disconnect_dt = np &&
> -		of_property_read_bool(np, "i2c-mux-idle-disconnect");
> -	if (idle_disconnect_dt)
> -		data->idle_state = MUX_IDLE_DISCONNECT;
>  
>  	ret = pca954x_irq_setup(muxc);
>  	if (ret)
> @@ -531,8 +546,7 @@ static int pca954x_resume(struct device *dev)
>  	struct i2c_mux_core *muxc = i2c_get_clientdata(client);
>  	struct pca954x *data = i2c_mux_priv(muxc);
>  
> -	data->last_chan = 0;
> -	return i2c_smbus_write_byte(client, 0);
> +	return pca954x_init(client, data);
>  }
>  #endif
>  
> 

RE: [EXT] Re: [v3,2/2] i2c: mux: pca954x: support property idle-state

[Biwen Li]

> On 2019-10-16 06:09, Biwen Li wrote:
> > This supports property idle-state
> >
> > Signed-off-by: Biwen Li <biwen.li@nxp.com>
> > ---
> > Change in v3:
> >       - update subject and description
> >       - add a helper function pca954x_calculate_chan()
> >
> > Change in v2:
> >       - update subject and description
> >       - add property idle-state
> >
> >  drivers/i2c/muxes/i2c-mux-pca954x.c | 64
> > ++++++++++++++++++-----------
> >  1 file changed, 39 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c
> > b/drivers/i2c/muxes/i2c-mux-pca954x.c
> > index 923aa3a5a3dc..8777d429269c 100644
> > --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
> > +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
> > @@ -86,7 +86,7 @@ struct pca954x {
> >
> >       u8 last_chan;           /* last register value */
> >       /* MUX_IDLE_AS_IS, MUX_IDLE_DISCONNECT or >= 0 for channel */
> > -     s8 idle_state;
> > +     s32 idle_state;
> >
> >       struct i2c_client *client;
> >
> > @@ -229,22 +229,25 @@ static int pca954x_reg_write(struct i2c_adapter
> *adap,
> >                               I2C_SMBUS_BYTE, &dummy);  }
> >
> > +static int pca954x_calculate_chan(struct pca954x *data, u32 chan)
> 
> Should return u8, and "chan" is not what is calculated. Perhaps name the
> function pca954x_regval?
Okay, got it, I will change it in v4.
> 
> (Yes, last_chan is also clearly a bad name, and I suspect you may have
> based this name on it, but changing that is a separate patch.)
> 
> > +{
> > +     /* we make switches look like muxes, not sure how to be smarter */
> > +     if (data->chip->muxtype == pca954x_ismux)
> > +             return chan | data->chip->enable;
> > +     else
> > +             return 1 << chan;
> > +}
> > +
> >  static int pca954x_select_chan(struct i2c_mux_core *muxc, u32 chan)
> > {
> >       struct pca954x *data = i2c_mux_priv(muxc);
> >       struct i2c_client *client = data->client;
> > -     const struct chip_desc *chip = data->chip;
> >       u8 regval;
> >       int ret = 0;
> >
> > -     /* we make switches look like muxes, not sure how to be smarter */
> > -     if (chip->muxtype == pca954x_ismux)
> > -             regval = chan | chip->enable;
> > -     else
> > -             regval = 1 << chan;
> > -
> > +     regval = pca954x_calculate_chan(data, chan);
> 
> I think I would have kept the empty line here. Not important...
> 
> >       /* Only select the channel if its different from the last channel */
> > -     if (data->last_chan != regval) {
> > +     if ((data->last_chan & 0xff) != regval) {
> 
> The changes on this line are not needed (last_chan and regval are both u8)
> and just clutters up the code.
Okay, got it, I will not change it in v4.
> 
> >               ret = pca954x_reg_write(muxc->parent, client, regval);
> >               data->last_chan = ret < 0 ? 0 : regval;
> >       }
> > @@ -256,7 +259,7 @@ static int pca954x_deselect_mux(struct
> > i2c_mux_core *muxc, u32 chan)  {
> >       struct pca954x *data = i2c_mux_priv(muxc);
> >       struct i2c_client *client = data->client;
> > -     s8 idle_state;
> > +     s32 idle_state;
> >
> >       idle_state = READ_ONCE(data->idle_state);
> >       if (idle_state >= 0)
> > @@ -402,6 +405,23 @@ static void pca954x_cleanup(struct
> i2c_mux_core *muxc)
> >       i2c_mux_del_adapters(muxc);
> >  }
> >
> > +static int pca954x_init(struct i2c_client *client, struct pca954x
> > +*data) {
> > +     /*
> > +      * Write the mux register at addr to verify
> > +      * that the mux is in fact present. This also
> > +      * initializes the mux to a channel
> > +      * or disconnected state.
> > +      */
> 
> Again, this comment belongs in pca954x_probe before the call to this
> function.
> It does not apply (at least not the first sentence) when pca954x_init is called
> from pca954x_resume.
Okay, got it, thanks, I will move it in v4.
> 
> Hmmm, it could be argued that specifying MUX_IDLE_AS_IS should not
> trigger a disconnect on init (since the mux is always idle at init) and that
> some other method should be used to determine if the chip is present. The
> difference is that with the idle-state property you can explicitly request
> MUX_IDLE_AS_IS, while the old code only had some default behavior if
> i2c-mux-idle-disconnect was not present.
> 
> The easy way out of this is to, in the binding, document the situation and say
> that "idle-state = <MUX_IDLE_AS_IS>;" is not supported but that similar
> functionality can be obtained by leaving out both the
> i2c-mux-idle-disconnect and idle-state properties.
I will support MUX_IDLE_AS_IS in v4.

> 
> > +     if (data->idle_state >= 0) {
> > +             data->last_chan = pca954x_calculate_chan(data,
> data->idle_state);
> > +     } else {
> > +             /* Disconnect multiplexer */
> > +             data->last_chan = 0;
> > +     }
> > +     return i2c_smbus_write_byte(client, data->last_chan); }
> > +
> >  /*
> >   * I2C init/probing/exit functions
> >   */
> > @@ -411,7 +431,6 @@ static int pca954x_probe(struct i2c_client *client,
> >       struct i2c_adapter *adap = client->adapter;
> >       struct device *dev = &client->dev;
> >       struct device_node *np = dev->of_node;
> > -     bool idle_disconnect_dt;
> >       struct gpio_desc *gpio;
> >       struct i2c_mux_core *muxc;
> >       struct pca954x *data;
> > @@ -462,22 +481,18 @@ static int pca954x_probe(struct i2c_client
> *client,
> >               }
> >       }
> >
> > -     /* Write the mux register at addr to verify
> > -      * that the mux is in fact present. This also
> > -      * initializes the mux to disconnected state.
> > -      */
> > -     if (i2c_smbus_write_byte(client, 0) < 0) {
> > +     data->idle_state = MUX_IDLE_AS_IS;
> > +     if (np && of_property_read_u32(np, "idle-state", &data->idle_state))
> {
> > +             if (np && of_property_read_bool(np,
> > + "i2c-mux-idle-disconnect"))
> 
> You do not need to do the "np &&" part for both ifs, since it's already know
> that np is non-NULL when you hit the second if. But, it's a NOP in the first if,
> since of_property_read_u32 returns -EINVAL if it is. So, I suggest
> 
>         if (of_property_read_u32(np, "idle-state", &data->idle_state)) {
>                 if (np && of_property_read_bool(np,
> "i2c-mux-idle-disconnect"))
Okay, thanks, I will remove it in v4.
> 
> Cheers,
> Peter
> 
> > +                     data->idle_state = MUX_IDLE_DISCONNECT;
> > +     }
> > +
> > +     ret = pca954x_init(client, data);
> > +     if (ret < 0) {
> >               dev_warn(dev, "probe failed\n");
> >               return -ENODEV;
> >       }
> >
> > -     data->last_chan = 0;               /* force the first selection */
> > -     data->idle_state = MUX_IDLE_AS_IS;
> > -
> > -     idle_disconnect_dt = np &&
> > -             of_property_read_bool(np, "i2c-mux-idle-disconnect");
> > -     if (idle_disconnect_dt)
> > -             data->idle_state = MUX_IDLE_DISCONNECT;
> >
> >       ret = pca954x_irq_setup(muxc);
> >       if (ret)
> > @@ -531,8 +546,7 @@ static int pca954x_resume(struct device *dev)
> >       struct i2c_mux_core *muxc = i2c_get_clientdata(client);
> >       struct pca954x *data = i2c_mux_priv(muxc);
> >
> > -     data->last_chan = 0;
> > -     return i2c_smbus_write_byte(client, 0);
> > +     return pca954x_init(client, data);
> >  }
> >  #endif
> >
> >

Re: [v3,1/2] dt-bindings: i2c: support property idle-state

[Rob Herring]

On Wed, Oct 16, 2019 at 12:09:19PM +0800, Biwen Li wrote:
> This supports property idle-state

The subject should indicate this is just for pca954x.

> 
> Signed-off-by: Biwen Li <biwen.li@nxp.com>
> ---
> Change in v3:
> 	- update subject and description
> 	- add some information for property idle-state
> 
> Change in v2:
> 	- update subject and description
> 	- add property idle-state
> 
>  Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
> index 30ac6a60f041..7abda506b828 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
> @@ -25,6 +25,8 @@ Required Properties:
>  Optional Properties:
>  
>    - reset-gpios: Reference to the GPIO connected to the reset input.
> +  - idle-state: if present, overrides i2c-mux-idle-disconnect,
> +    Please refer to Documentation/devicetree/bindings/mux/mux-controller.txt
>    - i2c-mux-idle-disconnect: Boolean; if defined, forces mux to disconnect all
>      children in idle state. This is necessary for example, if there are several
>      multiplexers on the bus and the devices behind them use same I2C addresses.
> -- 
> 2.17.1
> 

Re: [v3,1/2] dt-bindings: i2c: support property idle-state

[Peter Rosin]

On 2019-10-16 06:09, Biwen Li wrote:
> This supports property idle-state
> 
> Signed-off-by: Biwen Li <biwen.li@nxp.com>
> ---
> Change in v3:
> 	- update subject and description
> 	- add some information for property idle-state
> 
> Change in v2:
> 	- update subject and description
> 	- add property idle-state
> 
>  Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
> index 30ac6a60f041..7abda506b828 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
> @@ -25,6 +25,8 @@ Required Properties:
>  Optional Properties:
>  
>    - reset-gpios: Reference to the GPIO connected to the reset input.
> +  - idle-state: if present, overrides i2c-mux-idle-disconnect,
> +    Please refer to Documentation/devicetree/bindings/mux/mux-controller.txt
>    - i2c-mux-idle-disconnect: Boolean; if defined, forces mux to disconnect all
>      children in idle state. This is necessary for example, if there are several
>      multiplexers on the bus and the devices behind them use same I2C addresses.
> 

Rob, should i2c-mux-idle-disconnect perhaps be deprecated here? Is that
appropriate?

idle-state provides a super-set of what i2c-mux-idle-disconnect provides.

Cheers,
Peter

Re: [v3,1/2] dt-bindings: i2c: support property idle-state

[Rob Herring]

On Thu, Oct 17, 2019 at 10:38 AM Peter Rosin <peda@axentia.se> wrote:
>
> On 2019-10-16 06:09, Biwen Li wrote:
> > This supports property idle-state
> >
> > Signed-off-by: Biwen Li <biwen.li@nxp.com>
> > ---
> > Change in v3:
> >       - update subject and description
> >       - add some information for property idle-state
> >
> > Change in v2:
> >       - update subject and description
> >       - add property idle-state
> >
> >  Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
> > index 30ac6a60f041..7abda506b828 100644
> > --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
> > +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
> > @@ -25,6 +25,8 @@ Required Properties:
> >  Optional Properties:
> >
> >    - reset-gpios: Reference to the GPIO connected to the reset input.
> > +  - idle-state: if present, overrides i2c-mux-idle-disconnect,
> > +    Please refer to Documentation/devicetree/bindings/mux/mux-controller.txt
> >    - i2c-mux-idle-disconnect: Boolean; if defined, forces mux to disconnect all
> >      children in idle state. This is necessary for example, if there are several
> >      multiplexers on the bus and the devices behind them use same I2C addresses.
> >
>
> Rob, should i2c-mux-idle-disconnect perhaps be deprecated here? Is that
> appropriate?
>
> idle-state provides a super-set of what i2c-mux-idle-disconnect provides.

Yes, seems like it and it is not too widely used.

Rob