2017-04-03 08:37:06

by Peter Rosin

[permalink] [raw]
Subject: [PATCH 0/9] Unify i2c_mux_add_adapter error reporting

Hi!

Many users of the i2c_mux_add_adapter interface log a message
on failure, but the function already logs such a message. One
or two of those users actually add more information than already
provided by the central failure message.

So, first fix the central error reporting to provide as much
information as any current user, and then remove the surplus
error reporting at the call sites.

Cheers,
peda

Peter Rosin (9):
i2c: mux: provide more info on failure in i2c_mux_add_adapter
i2c: arb: gpio-challenge: stop double error reporting
i2c: mux: gpio: stop double error reporting
i2c: mux: pca9541: stop double error reporting
i2c: mux: pca954x: stop double error reporting
i2c: mux: pinctrl: stop double error reporting
i2c: mux: reg: stop double error reporting
iio: gyro: mpu3050: stop double error reporting
[media] cx231xx: stop double error reporting

drivers/i2c/i2c-mux.c | 9 ++++++---
drivers/i2c/muxes/i2c-arb-gpio-challenge.c | 4 +---
drivers/i2c/muxes/i2c-mux-gpio.c | 4 +---
drivers/i2c/muxes/i2c-mux-pca9541.c | 4 +---
drivers/i2c/muxes/i2c-mux-pca954x.c | 7 +------
drivers/i2c/muxes/i2c-mux-pinctrl.c | 4 +---
drivers/i2c/muxes/i2c-mux-reg.c | 4 +---
drivers/iio/gyro/mpu3050-i2c.c | 5 ++---
drivers/media/usb/cx231xx/cx231xx-i2c.c | 15 ++++-----------
9 files changed, 18 insertions(+), 38 deletions(-)

--
2.1.4


2017-04-03 08:37:23

by Peter Rosin

[permalink] [raw]
Subject: [PATCH 1/9] i2c: mux: provide more info on failure in i2c_mux_add_adapter

No callers then need to report any further info, thus reducing both the
amount of code and the log noise.

Signed-off-by: Peter Rosin <[email protected]>
---
drivers/i2c/i2c-mux.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index 2178266bca79..26f7237558ba 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -395,13 +395,16 @@ int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
if (force_nr) {
priv->adap.nr = force_nr;
ret = i2c_add_numbered_adapter(&priv->adap);
+ dev_err(&parent->dev,
+ "failed to add mux-adapter %u as bus %u (error=%d)\n",
+ chan_id, force_nr, ret);
} else {
ret = i2c_add_adapter(&priv->adap);
+ dev_err(&parent->dev,
+ "failed to add mux-adapter %u (error=%d)\n",
+ chan_id, ret);
}
if (ret < 0) {
- dev_err(&parent->dev,
- "failed to add mux-adapter (error=%d)\n",
- ret);
kfree(priv);
return ret;
}
--
2.1.4

2017-04-03 08:37:32

by Peter Rosin

[permalink] [raw]
Subject: [PATCH 2/9] i2c: arb: gpio-challenge: stop double error reporting

i2c_mux_add_adapter already logs a message on failure.

Signed-off-by: Peter Rosin <[email protected]>
---
drivers/i2c/muxes/i2c-arb-gpio-challenge.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-arb-gpio-challenge.c b/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
index 86fc2d4c081b..812b8cff265f 100644
--- a/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
+++ b/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
@@ -202,10 +202,8 @@ static int i2c_arbitrator_probe(struct platform_device *pdev)

/* Actually add the mux adapter */
ret = i2c_mux_add_adapter(muxc, 0, 0, 0);
- if (ret) {
- dev_err(dev, "Failed to add adapter\n");
+ if (ret)
i2c_put_adapter(muxc->parent);
- }

return ret;
}
--
2.1.4

2017-04-03 08:37:41

by Peter Rosin

[permalink] [raw]
Subject: [PATCH 6/9] i2c: mux: pinctrl: stop double error reporting

i2c_mux_add_adapter already logs a message on failure.

Signed-off-by: Peter Rosin <[email protected]>
---
drivers/i2c/muxes/i2c-mux-pinctrl.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pinctrl.c b/drivers/i2c/muxes/i2c-mux-pinctrl.c
index 35bb775e1b74..7c0c264b07bc 100644
--- a/drivers/i2c/muxes/i2c-mux-pinctrl.c
+++ b/drivers/i2c/muxes/i2c-mux-pinctrl.c
@@ -245,10 +245,8 @@ static int i2c_mux_pinctrl_probe(struct platform_device *pdev)
(mux->pdata->base_bus_num + i) : 0;

ret = i2c_mux_add_adapter(muxc, bus, i, 0);
- if (ret) {
- dev_err(&pdev->dev, "Failed to add adapter %d\n", i);
+ if (ret)
goto err_del_adapter;
- }
}

return 0;
--
2.1.4

2017-04-03 08:37:52

by Peter Rosin

[permalink] [raw]
Subject: [PATCH 8/9] iio: gyro: mpu3050: stop double error reporting

i2c_mux_add_adapter already logs a message on failure.

Signed-off-by: Peter Rosin <[email protected]>
---
drivers/iio/gyro/mpu3050-i2c.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/gyro/mpu3050-i2c.c b/drivers/iio/gyro/mpu3050-i2c.c
index 06007200bf49..93f08b304a63 100644
--- a/drivers/iio/gyro/mpu3050-i2c.c
+++ b/drivers/iio/gyro/mpu3050-i2c.c
@@ -70,9 +70,8 @@ static int mpu3050_i2c_probe(struct i2c_client *client,
dev_err(&client->dev, "failed to allocate I2C mux\n");
else {
mpu3050->i2cmux->priv = mpu3050;
- ret = i2c_mux_add_adapter(mpu3050->i2cmux, 0, 0, 0);
- if (ret)
- dev_err(&client->dev, "failed to add I2C mux\n");
+ /* Ignore failure, not critical */
+ i2c_mux_add_adapter(mpu3050->i2cmux, 0, 0, 0);
}

return 0;
--
2.1.4

2017-04-03 08:38:10

by Peter Rosin

[permalink] [raw]
Subject: [PATCH 9/9] [media] cx231xx: stop double error reporting

i2c_mux_add_adapter already logs a message on failure.

Signed-off-by: Peter Rosin <[email protected]>
---
drivers/media/usb/cx231xx/cx231xx-i2c.c | 15 ++++-----------
1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/media/usb/cx231xx/cx231xx-i2c.c b/drivers/media/usb/cx231xx/cx231xx-i2c.c
index 35e9acfe63d3..dff514e147da 100644
--- a/drivers/media/usb/cx231xx/cx231xx-i2c.c
+++ b/drivers/media/usb/cx231xx/cx231xx-i2c.c
@@ -576,17 +576,10 @@ int cx231xx_i2c_mux_create(struct cx231xx *dev)

int cx231xx_i2c_mux_register(struct cx231xx *dev, int mux_no)
{
- int rc;
-
- rc = i2c_mux_add_adapter(dev->muxc,
- 0,
- mux_no /* chan_id */,
- 0 /* class */);
- if (rc)
- dev_warn(dev->dev,
- "i2c mux %d register FAILED\n", mux_no);
-
- return rc;
+ return i2c_mux_add_adapter(dev->muxc,
+ 0,
+ mux_no /* chan_id */,
+ 0 /* class */);
}

void cx231xx_i2c_mux_unregister(struct cx231xx *dev)
--
2.1.4

2017-04-03 08:37:37

by Peter Rosin

[permalink] [raw]
Subject: [PATCH 4/9] i2c: mux: pca9541: stop double error reporting

i2c_mux_add_adapter already logs a message on failure.

Signed-off-by: Peter Rosin <[email protected]>
---
drivers/i2c/muxes/i2c-mux-pca9541.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pca9541.c b/drivers/i2c/muxes/i2c-mux-pca9541.c
index 77840f7845a1..9e318c9516c7 100644
--- a/drivers/i2c/muxes/i2c-mux-pca9541.c
+++ b/drivers/i2c/muxes/i2c-mux-pca9541.c
@@ -369,10 +369,8 @@ static int pca9541_probe(struct i2c_client *client,
i2c_set_clientdata(client, muxc);

ret = i2c_mux_add_adapter(muxc, force, 0, 0);
- if (ret) {
- dev_err(&client->dev, "failed to register master selector\n");
+ if (ret)
return ret;
- }

dev_info(&client->dev, "registered master selector for I2C %s\n",
client->name);
--
2.1.4

2017-04-03 08:38:32

by Peter Rosin

[permalink] [raw]
Subject: [PATCH 7/9] i2c: mux: reg: stop double error reporting

i2c_mux_add_adapter already logs a message on failure.

Signed-off-by: Peter Rosin <[email protected]>
---
drivers/i2c/muxes/i2c-mux-reg.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-reg.c b/drivers/i2c/muxes/i2c-mux-reg.c
index c6a90b4a9c62..406d5059072c 100644
--- a/drivers/i2c/muxes/i2c-mux-reg.c
+++ b/drivers/i2c/muxes/i2c-mux-reg.c
@@ -222,10 +222,8 @@ static int i2c_mux_reg_probe(struct platform_device *pdev)
class = mux->data.classes ? mux->data.classes[i] : 0;

ret = i2c_mux_add_adapter(muxc, nr, mux->data.values[i], class);
- if (ret) {
- dev_err(&pdev->dev, "Failed to add adapter %d\n", i);
+ if (ret)
goto add_adapter_failed;
- }
}

dev_dbg(&pdev->dev, "%d port mux on %s adapter\n",
--
2.1.4

2017-04-03 08:39:03

by Peter Rosin

[permalink] [raw]
Subject: [PATCH 5/9] i2c: mux: pca954x: stop double error reporting

i2c_mux_add_adapter already logs a message on failure.

Signed-off-by: Peter Rosin <[email protected]>
---
drivers/i2c/muxes/i2c-mux-pca954x.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index 15dfc1648716..b2a85a2d00f7 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -434,13 +434,8 @@ static int pca954x_probe(struct i2c_client *client,
idle_disconnect_dt) << num;

ret = i2c_mux_add_adapter(muxc, force, num, class);
-
- if (ret) {
- dev_err(&client->dev,
- "failed to register multiplexed adapter"
- " %d as bus %d\n", num, force);
+ if (ret)
goto fail_del_adapters;
- }
}

dev_info(&client->dev,
--
2.1.4

2017-04-03 08:39:44

by Peter Rosin

[permalink] [raw]
Subject: [PATCH 3/9] i2c: mux: gpio: stop double error reporting

i2c_mux_add_adapter already logs a message on failure.

Signed-off-by: Peter Rosin <[email protected]>
---
drivers/i2c/muxes/i2c-mux-gpio.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index 655684d621a4..1a9973ede443 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -245,10 +245,8 @@ static int i2c_mux_gpio_probe(struct platform_device *pdev)
unsigned int class = mux->data.classes ? mux->data.classes[i] : 0;

ret = i2c_mux_add_adapter(muxc, nr, mux->data.values[i], class);
- if (ret) {
- dev_err(&pdev->dev, "Failed to add adapter %d\n", i);
+ if (ret)
goto add_adapter_failed;
- }
}

dev_info(&pdev->dev, "%d port mux on %s adapter\n",
--
2.1.4

2017-04-03 10:26:51

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH 9/9] [media] cx231xx: stop double error reporting

On Mon, Apr 03, 2017 at 10:38:38AM +0200, Peter Rosin wrote:
> i2c_mux_add_adapter already logs a message on failure.
>
> Signed-off-by: Peter Rosin <[email protected]>
> ---
> drivers/media/usb/cx231xx/cx231xx-i2c.c | 15 ++++-----------
> 1 file changed, 4 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/media/usb/cx231xx/cx231xx-i2c.c b/drivers/media/usb/cx231xx/cx231xx-i2c.c
> index 35e9acfe63d3..dff514e147da 100644
> --- a/drivers/media/usb/cx231xx/cx231xx-i2c.c
> +++ b/drivers/media/usb/cx231xx/cx231xx-i2c.c
> @@ -576,17 +576,10 @@ int cx231xx_i2c_mux_create(struct cx231xx *dev)
>
> int cx231xx_i2c_mux_register(struct cx231xx *dev, int mux_no)
> {
> - int rc;
> -
> - rc = i2c_mux_add_adapter(dev->muxc,
> - 0,
> - mux_no /* chan_id */,
> - 0 /* class */);
> - if (rc)
> - dev_warn(dev->dev,
> - "i2c mux %d register FAILED\n", mux_no);
> -
> - return rc;
> + return i2c_mux_add_adapter(dev->muxc,
> + 0,
> + mux_no /* chan_id */,
> + 0 /* class */);

Could be argued that the whole function is obsolete now and the
c231xx-core can call i2c_mux_add_adapter() directly. But maybe this is a
seperate patch.

> }
>
> void cx231xx_i2c_mux_unregister(struct cx231xx *dev)
> --
> 2.1.4
>


Attachments:
(No filename) (1.22 kB)
signature.asc (819.00 B)
Download all attachments

2017-04-03 10:27:28

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH 0/9] Unify i2c_mux_add_adapter error reporting

On Mon, Apr 03, 2017 at 10:38:29AM +0200, Peter Rosin wrote:
> Hi!
>
> Many users of the i2c_mux_add_adapter interface log a message
> on failure, but the function already logs such a message. One
> or two of those users actually add more information than already
> provided by the central failure message.
>
> So, first fix the central error reporting to provide as much
> information as any current user, and then remove the surplus
> error reporting at the call sites.

Yes, I like.

Reviewed-by: Wolfram Sang <[email protected]>


Attachments:
(No filename) (536.00 B)
signature.asc (819.00 B)
Download all attachments

2017-04-03 11:13:52

by Peter Rosin

[permalink] [raw]
Subject: Re: [PATCH 9/9] [media] cx231xx: stop double error reporting

On 2017-04-03 12:26, Wolfram Sang wrote:
> On Mon, Apr 03, 2017 at 10:38:38AM +0200, Peter Rosin wrote:
>> i2c_mux_add_adapter already logs a message on failure.
>>
>> Signed-off-by: Peter Rosin <[email protected]>
>> ---
>> drivers/media/usb/cx231xx/cx231xx-i2c.c | 15 ++++-----------
>> 1 file changed, 4 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/media/usb/cx231xx/cx231xx-i2c.c b/drivers/media/usb/cx231xx/cx231xx-i2c.c
>> index 35e9acfe63d3..dff514e147da 100644
>> --- a/drivers/media/usb/cx231xx/cx231xx-i2c.c
>> +++ b/drivers/media/usb/cx231xx/cx231xx-i2c.c
>> @@ -576,17 +576,10 @@ int cx231xx_i2c_mux_create(struct cx231xx *dev)
>>
>> int cx231xx_i2c_mux_register(struct cx231xx *dev, int mux_no)
>> {
>> - int rc;
>> -
>> - rc = i2c_mux_add_adapter(dev->muxc,
>> - 0,
>> - mux_no /* chan_id */,
>> - 0 /* class */);
>> - if (rc)
>> - dev_warn(dev->dev,
>> - "i2c mux %d register FAILED\n", mux_no);
>> -
>> - return rc;
>> + return i2c_mux_add_adapter(dev->muxc,
>> + 0,
>> + mux_no /* chan_id */,
>> + 0 /* class */);
>
> Could be argued that the whole function is obsolete now and the
> c231xx-core can call i2c_mux_add_adapter() directly. But maybe this is a
> seperate patch.

Agreed on all counts. BTW, the ..._unregister function below is equally
"obsolete". I'm going to leave the removal of both functions at the
discretion of whomever takes care of cx231xx maintenance...

Cheers,
peda

>> }
>>
>> void cx231xx_i2c_mux_unregister(struct cx231xx *dev)
>> --
>> 2.1.4
>>

2017-04-03 11:28:00

by Peter Rosin

[permalink] [raw]
Subject: Re: [PATCH 0/9] Unify i2c_mux_add_adapter error reporting

On 2017-04-03 12:27, Wolfram Sang wrote:
> On Mon, Apr 03, 2017 at 10:38:29AM +0200, Peter Rosin wrote:
>> Hi!
>>
>> Many users of the i2c_mux_add_adapter interface log a message
>> on failure, but the function already logs such a message. One
>> or two of those users actually add more information than already
>> provided by the central failure message.
>>
>> So, first fix the central error reporting to provide as much
>> information as any current user, and then remove the surplus
>> error reporting at the call sites.
>
> Yes, I like.
>
> Reviewed-by: Wolfram Sang <[email protected]>

Thanks!

BTW, the improved error reporting in patch 1/9 is not needed for
patches 8/9 and 9/9 to make sense, the existing central error
message is already good enough. So, iio and media maintainers,
feel free to just grab those two patches. Or, they can go via
Wolfram and the i2c tree with the rest of the series. Either way
is fine with me, just let me know.

Cheers,
peda

2017-04-03 17:56:07

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 8/9] iio: gyro: mpu3050: stop double error reporting

On Mon, Apr 3, 2017 at 10:38 AM, Peter Rosin <[email protected]> wrote:

> i2c_mux_add_adapter already logs a message on failure.
>
> Signed-off-by: Peter Rosin <[email protected]>

Reviewed-by: Linus Walleij <[email protected]>

Yours,
Linus Walleij

2017-04-03 19:42:09

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 8/9] iio: gyro: mpu3050: stop double error reporting

On 03/04/17 18:56, Linus Walleij wrote:
> On Mon, Apr 3, 2017 at 10:38 AM, Peter Rosin <[email protected]> wrote:
>
>> i2c_mux_add_adapter already logs a message on failure.
>>
>> Signed-off-by: Peter Rosin <[email protected]>
>
> Reviewed-by: Linus Walleij <[email protected]>
Applied to the togreg branch of iio.git.

Can't see any reason not to split this set up and apply through
the various trees so I've picked this one up.

Good little series. We should do more of these in general!

Jonathan
>
> Yours,
> Linus Walleij
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2017-04-11 08:09:37

by Peter Rosin

[permalink] [raw]
Subject: Re: [PATCH 0/9] Unify i2c_mux_add_adapter error reporting

On 2017-04-03 10:38, Peter Rosin wrote:
> Hi!
>
> Many users of the i2c_mux_add_adapter interface log a message
> on failure, but the function already logs such a message. One
> or two of those users actually add more information than already
> provided by the central failure message.
>
> So, first fix the central error reporting to provide as much
> information as any current user, and then remove the surplus
> error reporting at the call sites.

I have now pushed patches 1-7 to i2c-mux/for-next.
Jonathan grabbed patch 8 and it's going through the iio tree.
Still waiting on patch 9 and the media maintainers.

Cheers,
Peter

>
> Cheers,
> peda
>
> Peter Rosin (9):
> i2c: mux: provide more info on failure in i2c_mux_add_adapter
> i2c: arb: gpio-challenge: stop double error reporting
> i2c: mux: gpio: stop double error reporting
> i2c: mux: pca9541: stop double error reporting
> i2c: mux: pca954x: stop double error reporting
> i2c: mux: pinctrl: stop double error reporting
> i2c: mux: reg: stop double error reporting
> iio: gyro: mpu3050: stop double error reporting
> [media] cx231xx: stop double error reporting
>
> drivers/i2c/i2c-mux.c | 9 ++++++---
> drivers/i2c/muxes/i2c-arb-gpio-challenge.c | 4 +---
> drivers/i2c/muxes/i2c-mux-gpio.c | 4 +---
> drivers/i2c/muxes/i2c-mux-pca9541.c | 4 +---
> drivers/i2c/muxes/i2c-mux-pca954x.c | 7 +------
> drivers/i2c/muxes/i2c-mux-pinctrl.c | 4 +---
> drivers/i2c/muxes/i2c-mux-reg.c | 4 +---
> drivers/iio/gyro/mpu3050-i2c.c | 5 ++---
> drivers/media/usb/cx231xx/cx231xx-i2c.c | 15 ++++-----------
> 9 files changed, 18 insertions(+), 38 deletions(-)
>

2017-04-19 10:29:51

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: Re: [PATCH 0/9] Unify i2c_mux_add_adapter error reporting

Em Mon, 3 Apr 2017 13:27:48 +0200
Peter Rosin <[email protected]> escreveu:

> On 2017-04-03 12:27, Wolfram Sang wrote:
> > On Mon, Apr 03, 2017 at 10:38:29AM +0200, Peter Rosin wrote:
> >> Hi!
> >>
> >> Many users of the i2c_mux_add_adapter interface log a message
> >> on failure, but the function already logs such a message. One
> >> or two of those users actually add more information than already
> >> provided by the central failure message.
> >>
> >> So, first fix the central error reporting to provide as much
> >> information as any current user, and then remove the surplus
> >> error reporting at the call sites.
> >
> > Yes, I like.
> >
> > Reviewed-by: Wolfram Sang <[email protected]>
>
> Thanks!
>
> BTW, the improved error reporting in patch 1/9 is not needed for
> patches 8/9 and 9/9 to make sense, the existing central error
> message is already good enough. So, iio and media maintainers,
> feel free to just grab those two patches. Or, they can go via
> Wolfram and the i2c tree with the rest of the series. Either way
> is fine with me, just let me know.

Feel free to submit via I2C tree, together with the patch series:

Reviewed-by: Mauro Carvalho Chehab <[email protected]>

>
> Cheers,
> peda



Thanks,
Mauro