diff mbox

[2/2] i2c: mux: pinctrl: drop the idle_state member

Message ID 20170802072728.24586-3-peda@axentia.se
State Awaiting Upstream
Headers show

Commit Message

Peter Rosin Aug. 2, 2017, 7:27 a.m. UTC 
The information is available elsewhere.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/i2c/muxes/i2c-mux-pinctrl.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

Comments

Stephen Warren Aug. 2, 2017, 7:06 p.m. UTC | #1 
On 08/02/2017 01:27 AM, Peter Rosin wrote:
> The information is available elsewhere.

> diff --git a/drivers/i2c/muxes/i2c-mux-pinctrl.c b/drivers/i2c/muxes/i2c-mux-pinctrl.c

>   static int i2c_mux_pinctrl_deselect(struct i2c_mux_core *muxc, u32 chan)
>   {
> +	return i2c_mux_pinctrl_select(muxc, muxc->num_adapters);
>   }

> @@ -166,7 +162,7 @@ static int i2c_mux_pinctrl_probe(struct platform_device *pdev)

>   	/* Do not add any adapter for the idle state (if it's there at all). */
> -	for (i = 0; i < num_names - !!mux->state_idle; i++) {
> +	for (i = 0; i < num_names - !!muxc->deselect; i++) {

I think that "num_names - !!muxc->deselect" could just be 
muxc->num_adapters? Otherwise,

Reviewed-by: Stephen Warren <swarren@nvidia.com>
Peter Rosin Aug. 2, 2017, 9:25 p.m. UTC | #2 
On 2017-08-02 21:06, Stephen Warren wrote:
> On 08/02/2017 01:27 AM, Peter Rosin wrote:
>> The information is available elsewhere.
> 
>> diff --git a/drivers/i2c/muxes/i2c-mux-pinctrl.c b/drivers/i2c/muxes/i2c-mux-pinctrl.c
> 
>>   static int i2c_mux_pinctrl_deselect(struct i2c_mux_core *muxc, u32 chan)
>>   {
>> +	return i2c_mux_pinctrl_select(muxc, muxc->num_adapters);
>>   }
> 
>> @@ -166,7 +162,7 @@ static int i2c_mux_pinctrl_probe(struct platform_device *pdev)
> 
>>   	/* Do not add any adapter for the idle state (if it's there at all). */
>> -	for (i = 0; i < num_names - !!mux->state_idle; i++) {
>> +	for (i = 0; i < num_names - !!muxc->deselect; i++) {
> 
> I think that "num_names - !!muxc->deselect" could just be 
> muxc->num_adapters? 

Not really, it's the i2c_mux_add_adapter call in the loop that bumps
muxc->num_adapters, so the loop would not be entered. Not desirable :-)

(and muxc->max_adapters == num_names)

>                     Otherwise,
> Reviewed-by: Stephen Warren <swarren@nvidia.com>

Thanks!

Cheers,
Peter
Stephen Warren Aug. 2, 2017, 10:50 p.m. UTC | #3 
On 08/02/2017 03:25 PM, Peter Rosin wrote:
> On 2017-08-02 21:06, Stephen Warren wrote:
>> On 08/02/2017 01:27 AM, Peter Rosin wrote:
>>> The information is available elsewhere.
>>
>>> diff --git a/drivers/i2c/muxes/i2c-mux-pinctrl.c b/drivers/i2c/muxes/i2c-mux-pinctrl.c
>>
>>>    static int i2c_mux_pinctrl_deselect(struct i2c_mux_core *muxc, u32 chan)
>>>    {
>>> +	return i2c_mux_pinctrl_select(muxc, muxc->num_adapters);
>>>    }
>>
>>> @@ -166,7 +162,7 @@ static int i2c_mux_pinctrl_probe(struct platform_device *pdev)
>>
>>>    	/* Do not add any adapter for the idle state (if it's there at all). */
>>> -	for (i = 0; i < num_names - !!mux->state_idle; i++) {
>>> +	for (i = 0; i < num_names - !!muxc->deselect; i++) {
>>
>> I think that "num_names - !!muxc->deselect" could just be
>> muxc->num_adapters?
> 
> Not really, it's the i2c_mux_add_adapter call in the loop that bumps
> muxc->num_adapters, so the loop would not be entered. Not desirable :-)

Ok, that makes sense.

> (and muxc->max_adapters == num_names)

Well, unless muxc->deselect is true...
Peter Rosin Aug. 3, 2017, 5:19 a.m. UTC | #4 
On 2017-08-03 00:50, Stephen Warren wrote:
> On 08/02/2017 03:25 PM, Peter Rosin wrote:
>> (and muxc->max_adapters == num_names)
> 
> Well, unless muxc->deselect is true...

No, deselect does not affect neither max_adapters nor num_names. They
are always equal.

Cheers,
Peter
Stephen Warren Aug. 3, 2017, 9:41 p.m. UTC | #5 
On 08/02/2017 11:19 PM, Peter Rosin wrote:
> On 2017-08-03 00:50, Stephen Warren wrote:
>> On 08/02/2017 03:25 PM, Peter Rosin wrote:
>>> (and muxc->max_adapters == num_names)
>>
>> Well, unless muxc->deselect is true...
> 
> No, deselect does not affect neither max_adapters nor num_names. They
> are always equal.

Ah yes, I was confusing max_adapters with num_adapters.
diff mbox

Patch

diff --git a/drivers/i2c/muxes/i2c-mux-pinctrl.c b/drivers/i2c/muxes/i2c-mux-pinctrl.c
index aa4a3bf9507f..20b90a7a1e61 100644
--- a/drivers/i2c/muxes/i2c-mux-pinctrl.c
+++ b/drivers/i2c/muxes/i2c-mux-pinctrl.c
@@ -28,7 +28,6 @@ 
 struct i2c_mux_pinctrl {
 	struct pinctrl *pinctrl;
 	struct pinctrl_state **states;
-	struct pinctrl_state *state_idle;
 };
 
 static int i2c_mux_pinctrl_select(struct i2c_mux_core *muxc, u32 chan)
@@ -40,9 +39,7 @@  static int i2c_mux_pinctrl_select(struct i2c_mux_core *muxc, u32 chan)
 
 static int i2c_mux_pinctrl_deselect(struct i2c_mux_core *muxc, u32 chan)
 {
-	struct i2c_mux_pinctrl *mux = i2c_mux_priv(muxc);
-
-	return pinctrl_select_state(mux->pinctrl, mux->state_idle);
+	return i2c_mux_pinctrl_select(muxc, muxc->num_adapters);
 }
 
 static struct i2c_adapter *i2c_mux_pinctrl_root_adapter(
@@ -149,7 +146,6 @@  static int i2c_mux_pinctrl_probe(struct platform_device *pdev)
 			ret = -EINVAL;
 			goto err_put_parent;
 		}
-		mux->state_idle = mux->states[i];
 		muxc->deselect = i2c_mux_pinctrl_deselect;
 	}
 
@@ -166,7 +162,7 @@  static int i2c_mux_pinctrl_probe(struct platform_device *pdev)
 		dev_info(dev, "mux-locked i2c mux\n");
 
 	/* Do not add any adapter for the idle state (if it's there at all). */
-	for (i = 0; i < num_names - !!mux->state_idle; i++) {
+	for (i = 0; i < num_names - !!muxc->deselect; i++) {
 		ret = i2c_mux_add_adapter(muxc, 0, i, 0);
 		if (ret)
 			goto err_del_adapter;