[PATCH 0/9] iio: Remove duplicated error reporting in .remove()

[PATCH 0/9] iio: Remove duplicated error reporting in .remove()

[Uwe Kleine-König]

Hello,

this series adapts several i2c drivers that emit two error messages if
something in their remove function fails. The relevant issue is that the
i2c core emits an error message if the remove callback returns a
non-zero value but the drivers already emit a (better) message. So
these patches change the drivers to return 0 even after an error. Note
there is no further error handling in the i2c core, if a remove callback
returns an error code, the device is removed anyhow, so the only effect
of making the return value zero is that the error message is suppressed.

The motivation for this series is to eventually change the prototype of
the i2c remove callback to return void. As a preparation all remove
functions should return 0 such that changing the prototype doesn't
change behaviour of individual drivers.

Best regards
Uwe

Uwe Kleine-König (9):
  iio:accel:mc3230: Remove duplicated error reporting in .remove()
  iio:accel:stk8312: Remove duplicated error reporting in .remove()
  iio:accel:stk8ba50: Remove duplicated error reporting in .remove()
  iio:light:bh1780: Remove duplicated error reporting in .remove()
  iio:light:isl29028: Remove duplicated error reporting in .remove()
  iio:light:jsa1212: Remove duplicated error reporting in .remove()
  iio:light:opt3001: Remove duplicated error reporting in .remove()
  iio:light:stk3310: Remove duplicated error reporting in .remove()
  iio:light:tsl2583: Remove duplicated error reporting in .remove()

 drivers/iio/accel/mc3230.c   | 4 +++-
 drivers/iio/accel/stk8312.c  | 4 +++-
 drivers/iio/accel/stk8ba50.c | 4 +++-
 drivers/iio/light/bh1780.c   | 7 +++----
 drivers/iio/light/isl29028.c | 4 +++-
 drivers/iio/light/jsa1212.c  | 4 +++-
 drivers/iio/light/opt3001.c  | 3 +--
 drivers/iio/light/stk3310.c  | 5 ++++-
 drivers/iio/light/tsl2583.c  | 4 +++-
 9 files changed, 26 insertions(+), 13 deletions(-)


base-commit: 3123109284176b1532874591f7c81f3837bbdc17
-- 
2.35.1

[PATCH 1/9] iio:accel:mc3230: Remove duplicated error reporting in .remove()

[Uwe Kleine-König]

Returning an error value in an i2c remove callback results in an error
message being emitted by the i2c core, but otherwise it doesn't make a
difference. The device goes away anyhow and the devm cleanups are
called.

As mc3230_set_opcon() already emits an error message on failure and the
additional error message by the i2c core doesn't add any useful
information, change the return value to zero to suppress this message.

This patch is a preparation for making i2c remove callbacks return void.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/iio/accel/mc3230.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/accel/mc3230.c b/drivers/iio/accel/mc3230.c
index 679e69cd7657..c15d16e7f1da 100644
--- a/drivers/iio/accel/mc3230.c
+++ b/drivers/iio/accel/mc3230.c
@@ -157,7 +157,9 @@ static int mc3230_remove(struct i2c_client *client)
 
 	iio_device_unregister(indio_dev);
 
-	return mc3230_set_opcon(iio_priv(indio_dev), MC3230_MODE_OPCON_STANDBY);
+	mc3230_set_opcon(iio_priv(indio_dev), MC3230_MODE_OPCON_STANDBY);
+
+	return 0;
 }
 
 static int mc3230_suspend(struct device *dev)
-- 
2.35.1

[PATCH 2/9] iio:accel:stk8312: Remove duplicated error reporting in .remove()

[Uwe Kleine-König]

Returning an error value in an i2c remove callback results in an error
message being emitted by the i2c core, but otherwise it doesn't make a
difference. The device goes away anyhow and the devm cleanups are
called.

As stk8312_set_mode() already emits an error message on failure and the
additional error message by the i2c core doesn't add any useful
information, change the return value to zero to suppress this message.

This patch is a preparation for making i2c remove callbacks return void.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/iio/accel/stk8312.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/accel/stk8312.c b/drivers/iio/accel/stk8312.c
index a71dfff3ca4a..ceca28913355 100644
--- a/drivers/iio/accel/stk8312.c
+++ b/drivers/iio/accel/stk8312.c
@@ -608,7 +608,9 @@ static int stk8312_remove(struct i2c_client *client)
 	if (data->dready_trig)
 		iio_trigger_unregister(data->dready_trig);
 
-	return stk8312_set_mode(data, STK8312_MODE_STANDBY);
+	stk8312_set_mode(data, STK8312_MODE_STANDBY);
+
+	return 0;
 }
 
 static int stk8312_suspend(struct device *dev)
-- 
2.35.1

[PATCH 3/9] iio:accel:stk8ba50: Remove duplicated error reporting in .remove()

[Uwe Kleine-König]

Returning an error value in an i2c remove callback results in an error
message being emitted by the i2c core, but otherwise it doesn't make a
difference. The device goes away anyhow and the devm cleanups are
called.

As stk8ba50_set_mode() already emits an error message on failure and the
additional error message by the i2c core doesn't add any useful
information, change the return value to zero to suppress this message.

This patch is a preparation for making i2c remove callbacks return void.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/iio/accel/stk8ba50.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/accel/stk8ba50.c b/drivers/iio/accel/stk8ba50.c
index 0067ec5cbae8..7d59efb41e22 100644
--- a/drivers/iio/accel/stk8ba50.c
+++ b/drivers/iio/accel/stk8ba50.c
@@ -501,7 +501,9 @@ static int stk8ba50_remove(struct i2c_client *client)
 	if (data->dready_trig)
 		iio_trigger_unregister(data->dready_trig);
 
-	return stk8ba50_set_power(data, STK8BA50_MODE_SUSPEND);
+	stk8ba50_set_power(data, STK8BA50_MODE_SUSPEND);
+
+	return 0;
 }
 
 static int stk8ba50_suspend(struct device *dev)
-- 
2.35.1

[PATCH 4/9] iio:light:bh1780: Remove duplicated error reporting in .remove()

[Uwe Kleine-König]

Returning an error value in an i2c remove callback results in an error
message being emitted by the i2c core, but otherwise it doesn't make a
difference. The device goes away anyhow and the devm cleanups are
called.

As bh1780_remove() already emits an error message on failure and the
additional error message by the i2c core doesn't add any useful
information, change the return value to zero to suppress this message.
While at it, add the error code to the remaining error message.

This patch is a preparation for making i2c remove callbacks return void.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/iio/light/bh1780.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/light/bh1780.c b/drivers/iio/light/bh1780.c
index 790d3d613979..fc7141390117 100644
--- a/drivers/iio/light/bh1780.c
+++ b/drivers/iio/light/bh1780.c
@@ -213,10 +213,9 @@ static int bh1780_remove(struct i2c_client *client)
 	pm_runtime_put_noidle(&client->dev);
 	pm_runtime_disable(&client->dev);
 	ret = bh1780_write(bh1780, BH1780_REG_CONTROL, BH1780_POFF);
-	if (ret < 0) {
-		dev_err(&client->dev, "failed to power off\n");
-		return ret;
-	}
+	if (ret < 0)
+		dev_err(&client->dev, "failed to power off (%pe)\n",
+			ERR_PTR(ret));
 
 	return 0;
 }
-- 
2.35.1

Re: [PATCH 4/9] iio:light:bh1780: Remove duplicated error reporting in .remove()

[Linus Walleij]

On Sat, Apr 30, 2022 at 10:16 AM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:

> Returning an error value in an i2c remove callback results in an error
> message being emitted by the i2c core, but otherwise it doesn't make a
> difference. The device goes away anyhow and the devm cleanups are
> called.
>
> As bh1780_remove() already emits an error message on failure and the
> additional error message by the i2c core doesn't add any useful
> information, change the return value to zero to suppress this message.
> While at it, add the error code to the remaining error message.
>
> This patch is a preparation for making i2c remove callbacks return void.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 5/9] iio:light:isl29028: Remove duplicated error reporting in .remove()

[Uwe Kleine-König]

Returning an error value in an i2c remove callback results in an error
message being emitted by the i2c core, but otherwise it doesn't make a
difference. The device goes away anyhow and the devm cleanups are
called.

As isl29028_clear_configure_ret() already emits an error message on
failure and the additional error message by the i2c core doesn't add any
useful information, change the return value to zero to suppress this
message.

This patch is a preparation for making i2c remove callbacks return void.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/iio/light/isl29028.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/light/isl29028.c b/drivers/iio/light/isl29028.c
index 9de3262aa688..720fa83d44e0 100644
--- a/drivers/iio/light/isl29028.c
+++ b/drivers/iio/light/isl29028.c
@@ -646,7 +646,9 @@ static int isl29028_remove(struct i2c_client *client)
 	pm_runtime_disable(&client->dev);
 	pm_runtime_set_suspended(&client->dev);
 
-	return isl29028_clear_configure_reg(chip);
+	isl29028_clear_configure_reg(chip);
+
+	return 0;
 }
 
 static int __maybe_unused isl29028_suspend(struct device *dev)
-- 
2.35.1

[PATCH 6/9] iio:light:jsa1212: Remove duplicated error reporting in .remove()

[Uwe Kleine-König]

Returning an error value in an i2c remove callback results in an error
message being emitted by the i2c core, but otherwise it doesn't make a
difference. The device goes away anyhow and the devm cleanups are
called.

As jsa1212_power_off() already emits an error message on failure and the
additional error message by the i2c core doesn't add any useful
information, change the return value to zero to suppress this message.

This patch is a preparation for making i2c remove callbacks return void.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/iio/light/jsa1212.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/light/jsa1212.c b/drivers/iio/light/jsa1212.c
index a55194263d23..5387c12231cf 100644
--- a/drivers/iio/light/jsa1212.c
+++ b/drivers/iio/light/jsa1212.c
@@ -380,7 +380,9 @@ static int jsa1212_remove(struct i2c_client *client)
 
 	iio_device_unregister(indio_dev);
 
-	return jsa1212_power_off(data);
+	jsa1212_power_off(data);
+
+	return 0;
 }
 
 static int jsa1212_suspend(struct device *dev)
-- 
2.35.1

[PATCH 7/9] iio:light:opt3001: Remove duplicated error reporting in .remove()

[Uwe Kleine-König]

Returning an error value in an i2c remove callback results in an error
message being emitted by the i2c core, but otherwise it doesn't make a
difference. The device goes away anyhow and the devm cleanups are
called.

As opt3001_remove() already emits an error message on failure and the
additional error message by the i2c core doesn't add any useful
information, change the return value to zero to suppress this message.

This patch is a preparation for making i2c remove callbacks return void.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/iio/light/opt3001.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/iio/light/opt3001.c b/drivers/iio/light/opt3001.c
index 1880bd5bb258..a326d47afc9b 100644
--- a/drivers/iio/light/opt3001.c
+++ b/drivers/iio/light/opt3001.c
@@ -808,7 +808,7 @@ static int opt3001_remove(struct i2c_client *client)
 	if (ret < 0) {
 		dev_err(opt->dev, "failed to read register %02x\n",
 				OPT3001_CONFIGURATION);
-		return ret;
+		return 0;
 	}
 
 	reg = ret;
@@ -819,7 +819,6 @@ static int opt3001_remove(struct i2c_client *client)
 	if (ret < 0) {
 		dev_err(opt->dev, "failed to write register %02x\n",
 				OPT3001_CONFIGURATION);
-		return ret;
 	}
 
 	return 0;
-- 
2.35.1

[PATCH 8/9] iio:light:stk3310: Remove duplicated error reporting in .remove()

[Uwe Kleine-König]

Returning an error value in an i2c remove callback results in an error
message being emitted by the i2c core, but otherwise it doesn't make a
difference. The device goes away anyhow and the devm cleanups are
called.

As stk3310_set_state() already emits an error message on failure and the
additional error message by the i2c core doesn't add any useful
information, change the return value to zero to suppress this message.

This patch is a preparation for making i2c remove callbacks return void.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/iio/light/stk3310.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/light/stk3310.c b/drivers/iio/light/stk3310.c
index 1d02dfbc29d1..95a98af08b8a 100644
--- a/drivers/iio/light/stk3310.c
+++ b/drivers/iio/light/stk3310.c
@@ -627,9 +627,12 @@ static int stk3310_probe(struct i2c_client *client,
 static int stk3310_remove(struct i2c_client *client)
 {
 	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+	int ret;
 
 	iio_device_unregister(indio_dev);
-	return stk3310_set_state(iio_priv(indio_dev), STK3310_STATE_STANDBY);
+	stk3310_set_state(iio_priv(indio_dev), STK3310_STATE_STANDBY);
+
+	return 0;
 }
 
 static int stk3310_suspend(struct device *dev)
-- 
2.35.1

Re: [PATCH 8/9] iio:light:stk3310: Remove duplicated error reporting in .remove()

[Uwe Kleine-König]

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

Hello,

On Sat, Apr 30, 2022 at 10:16:06AM +0200, Uwe Kleine-König wrote:
> Returning an error value in an i2c remove callback results in an error
> message being emitted by the i2c core, but otherwise it doesn't make a
> difference. The device goes away anyhow and the devm cleanups are
> called.
> 
> As stk3310_set_state() already emits an error message on failure and the
> additional error message by the i2c core doesn't add any useful
> information, change the return value to zero to suppress this message.
> 
> This patch is a preparation for making i2c remove callbacks return void.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/iio/light/stk3310.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/light/stk3310.c b/drivers/iio/light/stk3310.c
> index 1d02dfbc29d1..95a98af08b8a 100644
> --- a/drivers/iio/light/stk3310.c
> +++ b/drivers/iio/light/stk3310.c
> @@ -627,9 +627,12 @@ static int stk3310_probe(struct i2c_client *client,
>  static int stk3310_remove(struct i2c_client *client)
>  {
>  	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +	int ret;
>  
>  	iio_device_unregister(indio_dev);
> -	return stk3310_set_state(iio_priv(indio_dev), STK3310_STATE_STANDBY);
> +	stk3310_set_state(iio_priv(indio_dev), STK3310_STATE_STANDBY);
> +
> +	return 0;
>  }

I just found a fixup in my tree that drops the

	+	int ret;

that I failed to squash in before sending.

Jonathan: Tell me if you want to fixup yourself when you apply, or if
you prefer a v2. If the latter, only for this patch or the whole series?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

Re: [PATCH 8/9] iio:light:stk3310: Remove duplicated error reporting in .remove()

[Jonathan Cameron]

On Mon, 2 May 2022 09:37:55 +0200
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:

> Hello,
> 
> On Sat, Apr 30, 2022 at 10:16:06AM +0200, Uwe Kleine-König wrote:
> > Returning an error value in an i2c remove callback results in an error
> > message being emitted by the i2c core, but otherwise it doesn't make a
> > difference. The device goes away anyhow and the devm cleanups are
> > called.
> > 
> > As stk3310_set_state() already emits an error message on failure and the
> > additional error message by the i2c core doesn't add any useful
> > information, change the return value to zero to suppress this message.
> > 
> > This patch is a preparation for making i2c remove callbacks return void.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> >  drivers/iio/light/stk3310.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iio/light/stk3310.c b/drivers/iio/light/stk3310.c
> > index 1d02dfbc29d1..95a98af08b8a 100644
> > --- a/drivers/iio/light/stk3310.c
> > +++ b/drivers/iio/light/stk3310.c
> > @@ -627,9 +627,12 @@ static int stk3310_probe(struct i2c_client *client,
> >  static int stk3310_remove(struct i2c_client *client)
> >  {
> >  	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> > +	int ret;
> >  
> >  	iio_device_unregister(indio_dev);
> > -	return stk3310_set_state(iio_priv(indio_dev), STK3310_STATE_STANDBY);
> > +	stk3310_set_state(iio_priv(indio_dev), STK3310_STATE_STANDBY);
> > +
> > +	return 0;
> >  }  
> 
> I just found a fixup in my tree that drops the
> 
> 	+	int ret;
> 
> that I failed to squash in before sending.
> 
> Jonathan: Tell me if you want to fixup yourself when you apply, or if
> you prefer a v2. If the latter, only for this patch or the whole series?

I'll fix it up when applying..

Thanks for letting me know.

Jonathan

> 
> Best regards
> Uwe
> 

[PATCH 9/9] iio:light:tsl2583: Remove duplicated error reporting in .remove()

[Uwe Kleine-König]

Returning an error value in an i2c remove callback results in an error
message being emitted by the i2c core, but otherwise it doesn't make a
difference. The device goes away anyhow and the devm cleanups are
called.

As tsl2583_set_power_state() already emits an error message on failure
and the additional error message by the i2c core doesn't add any useful
information, change the return value to zero to suppress this message.

This patch is a preparation for making i2c remove callbacks return void.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/iio/light/tsl2583.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/light/tsl2583.c b/drivers/iio/light/tsl2583.c
index 7e101d5f72ee..efb3c13cfc87 100644
--- a/drivers/iio/light/tsl2583.c
+++ b/drivers/iio/light/tsl2583.c
@@ -883,7 +883,9 @@ static int tsl2583_remove(struct i2c_client *client)
 	pm_runtime_disable(&client->dev);
 	pm_runtime_set_suspended(&client->dev);
 
-	return tsl2583_set_power_state(chip, TSL2583_CNTL_PWR_OFF);
+	tsl2583_set_power_state(chip, TSL2583_CNTL_PWR_OFF);
+
+	return 0;
 }
 
 static int __maybe_unused tsl2583_suspend(struct device *dev)
-- 
2.35.1

Re: [PATCH 0/9] iio: Remove duplicated error reporting in .remove()

[Jonathan Cameron]

On Sat, 30 Apr 2022 10:15:58 +0200
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:

> Hello,
> 
> this series adapts several i2c drivers that emit two error messages if
> something in their remove function fails. The relevant issue is that the
> i2c core emits an error message if the remove callback returns a
> non-zero value but the drivers already emit a (better) message. So
> these patches change the drivers to return 0 even after an error. Note
> there is no further error handling in the i2c core, if a remove callback
> returns an error code, the device is removed anyhow, so the only effect
> of making the return value zero is that the error message is suppressed.
> 
> The motivation for this series is to eventually change the prototype of
> the i2c remove callback to return void. As a preparation all remove
> functions should return 0 such that changing the prototype doesn't
> change behaviour of individual drivers.

I think I'd rather have seen these called out as simply moving towards
this second change as it feels wrong to deliberately not report an error
so as to avoid repeated error messages!

Meh, I don't care that strongly and you call out the real reason in each
patch.

> 
> Best regards
> Uwe
> 
> Uwe Kleine-König (9):
>   iio:accel:mc3230: Remove duplicated error reporting in .remove()
>   iio:accel:stk8312: Remove duplicated error reporting in .remove()
>   iio:accel:stk8ba50: Remove duplicated error reporting in .remove()
>   iio:light:bh1780: Remove duplicated error reporting in .remove()
>   iio:light:isl29028: Remove duplicated error reporting in .remove()
>   iio:light:jsa1212: Remove duplicated error reporting in .remove()
>   iio:light:opt3001: Remove duplicated error reporting in .remove()
>   iio:light:stk3310: Remove duplicated error reporting in .remove()
>   iio:light:tsl2583: Remove duplicated error reporting in .remove()
> 
>  drivers/iio/accel/mc3230.c   | 4 +++-
>  drivers/iio/accel/stk8312.c  | 4 +++-
>  drivers/iio/accel/stk8ba50.c | 4 +++-
>  drivers/iio/light/bh1780.c   | 7 +++----
>  drivers/iio/light/isl29028.c | 4 +++-
>  drivers/iio/light/jsa1212.c  | 4 +++-
>  drivers/iio/light/opt3001.c  | 3 +--
>  drivers/iio/light/stk3310.c  | 5 ++++-
>  drivers/iio/light/tsl2583.c  | 4 +++-
>  9 files changed, 26 insertions(+), 13 deletions(-)
> 
> 
> base-commit: 3123109284176b1532874591f7c81f3837bbdc17

Re: [PATCH 0/9] iio: Remove duplicated error reporting in .remove()

[Jonathan Cameron]

On Sun, 1 May 2022 18:41:49 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Sat, 30 Apr 2022 10:15:58 +0200
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> 
> > Hello,
> > 
> > this series adapts several i2c drivers that emit two error messages if
> > something in their remove function fails. The relevant issue is that the
> > i2c core emits an error message if the remove callback returns a
> > non-zero value but the drivers already emit a (better) message. So
> > these patches change the drivers to return 0 even after an error. Note
> > there is no further error handling in the i2c core, if a remove callback
> > returns an error code, the device is removed anyhow, so the only effect
> > of making the return value zero is that the error message is suppressed.
> > 
> > The motivation for this series is to eventually change the prototype of
> > the i2c remove callback to return void. As a preparation all remove
> > functions should return 0 such that changing the prototype doesn't
> > change behaviour of individual drivers.  
> 
> I think I'd rather have seen these called out as simply moving towards
> this second change as it feels wrong to deliberately not report an error
> so as to avoid repeated error messages!
> 
> Meh, I don't care that strongly and you call out the real reason in each
> patch.

Series looks fine to me, but I'll leave the on list for a few days to let
others have time to take a look.

Worth noting that some of these are crying out for use
of devm_add_action_or_reset() and getting rid of the remove functions
entirely now you've dropped the oddity of them returning non 0.

Low hanging fruit for any newbies who want to do it, or maybe I will
if I get bored :)

Thanks,

Jonathan

> 
> > 
> > Best regards
> > Uwe
> > 
> > Uwe Kleine-König (9):
> >   iio:accel:mc3230: Remove duplicated error reporting in .remove()
> >   iio:accel:stk8312: Remove duplicated error reporting in .remove()
> >   iio:accel:stk8ba50: Remove duplicated error reporting in .remove()
> >   iio:light:bh1780: Remove duplicated error reporting in .remove()
> >   iio:light:isl29028: Remove duplicated error reporting in .remove()
> >   iio:light:jsa1212: Remove duplicated error reporting in .remove()
> >   iio:light:opt3001: Remove duplicated error reporting in .remove()
> >   iio:light:stk3310: Remove duplicated error reporting in .remove()
> >   iio:light:tsl2583: Remove duplicated error reporting in .remove()
> > 
> >  drivers/iio/accel/mc3230.c   | 4 +++-
> >  drivers/iio/accel/stk8312.c  | 4 +++-
> >  drivers/iio/accel/stk8ba50.c | 4 +++-
> >  drivers/iio/light/bh1780.c   | 7 +++----
> >  drivers/iio/light/isl29028.c | 4 +++-
> >  drivers/iio/light/jsa1212.c  | 4 +++-
> >  drivers/iio/light/opt3001.c  | 3 +--
> >  drivers/iio/light/stk3310.c  | 5 ++++-
> >  drivers/iio/light/tsl2583.c  | 4 +++-
> >  9 files changed, 26 insertions(+), 13 deletions(-)
> > 
> > 
> > base-commit: 3123109284176b1532874591f7c81f3837bbdc17  
> 

Re: [PATCH 0/9] iio: Remove duplicated error reporting in .remove()

[Jonathan Cameron]

On Sun, 1 May 2022 18:51:23 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Sun, 1 May 2022 18:41:49 +0100
> Jonathan Cameron <jic23@kernel.org> wrote:
> 
> > On Sat, 30 Apr 2022 10:15:58 +0200
> > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> >   
> > > Hello,
> > > 
> > > this series adapts several i2c drivers that emit two error messages if
> > > something in their remove function fails. The relevant issue is that the
> > > i2c core emits an error message if the remove callback returns a
> > > non-zero value but the drivers already emit a (better) message. So
> > > these patches change the drivers to return 0 even after an error. Note
> > > there is no further error handling in the i2c core, if a remove callback
> > > returns an error code, the device is removed anyhow, so the only effect
> > > of making the return value zero is that the error message is suppressed.
> > > 
> > > The motivation for this series is to eventually change the prototype of
> > > the i2c remove callback to return void. As a preparation all remove
> > > functions should return 0 such that changing the prototype doesn't
> > > change behaviour of individual drivers.    
> > 
> > I think I'd rather have seen these called out as simply moving towards
> > this second change as it feels wrong to deliberately not report an error
> > so as to avoid repeated error messages!
> > 
> > Meh, I don't care that strongly and you call out the real reason in each
> > patch.  
> 
> Series looks fine to me, but I'll leave the on list for a few days to let
> others have time to take a look.
> 
> Worth noting that some of these are crying out for use
> of devm_add_action_or_reset() and getting rid of the remove functions
> entirely now you've dropped the oddity of them returning non 0.
> 
> Low hanging fruit for any newbies who want to do it, or maybe I will
> if I get bored :)

Series applied to the togreg branch of iio.git and pushed out as testing for
0-day to see if it can find anything we missed.

Thanks,

Jonathan

> 
> Thanks,
> 
> Jonathan
> 
> >   
> > > 
> > > Best regards
> > > Uwe
> > > 
> > > Uwe Kleine-König (9):
> > >   iio:accel:mc3230: Remove duplicated error reporting in .remove()
> > >   iio:accel:stk8312: Remove duplicated error reporting in .remove()
> > >   iio:accel:stk8ba50: Remove duplicated error reporting in .remove()
> > >   iio:light:bh1780: Remove duplicated error reporting in .remove()
> > >   iio:light:isl29028: Remove duplicated error reporting in .remove()
> > >   iio:light:jsa1212: Remove duplicated error reporting in .remove()
> > >   iio:light:opt3001: Remove duplicated error reporting in .remove()
> > >   iio:light:stk3310: Remove duplicated error reporting in .remove()
> > >   iio:light:tsl2583: Remove duplicated error reporting in .remove()
> > > 
> > >  drivers/iio/accel/mc3230.c   | 4 +++-
> > >  drivers/iio/accel/stk8312.c  | 4 +++-
> > >  drivers/iio/accel/stk8ba50.c | 4 +++-
> > >  drivers/iio/light/bh1780.c   | 7 +++----
> > >  drivers/iio/light/isl29028.c | 4 +++-
> > >  drivers/iio/light/jsa1212.c  | 4 +++-
> > >  drivers/iio/light/opt3001.c  | 3 +--
> > >  drivers/iio/light/stk3310.c  | 5 ++++-
> > >  drivers/iio/light/tsl2583.c  | 4 +++-
> > >  9 files changed, 26 insertions(+), 13 deletions(-)
> > > 
> > > 
> > > base-commit: 3123109284176b1532874591f7c81f3837bbdc17    
> >   
> 

Re: [PATCH 0/9] iio: Remove duplicated error reporting in .remove()

[Uwe Kleine-König]

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

Hello Jonathan,

On Sat, May 07, 2022 at 03:38:55PM +0100, Jonathan Cameron wrote:
> On Sun, 1 May 2022 18:51:23 +0100
> Jonathan Cameron <jic23@kernel.org> wrote:
> 
> > On Sun, 1 May 2022 18:41:49 +0100
> > Jonathan Cameron <jic23@kernel.org> wrote:
> > 
> > > On Sat, 30 Apr 2022 10:15:58 +0200
> > > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> > >   
> > > > Hello,
> > > > 
> > > > this series adapts several i2c drivers that emit two error messages if
> > > > something in their remove function fails. The relevant issue is that the
> > > > i2c core emits an error message if the remove callback returns a
> > > > non-zero value but the drivers already emit a (better) message. So
> > > > these patches change the drivers to return 0 even after an error. Note
> > > > there is no further error handling in the i2c core, if a remove callback
> > > > returns an error code, the device is removed anyhow, so the only effect
> > > > of making the return value zero is that the error message is suppressed.
> > > > 
> > > > The motivation for this series is to eventually change the prototype of
> > > > the i2c remove callback to return void. As a preparation all remove
> > > > functions should return 0 such that changing the prototype doesn't
> > > > change behaviour of individual drivers.    
> > > 
> > > I think I'd rather have seen these called out as simply moving towards
> > > this second change as it feels wrong to deliberately not report an error
> > > so as to avoid repeated error messages!
> > > 
> > > Meh, I don't care that strongly and you call out the real reason in each
> > > patch.  
> > 
> > Series looks fine to me, but I'll leave the on list for a few days to let
> > others have time to take a look.
> > 
> > Worth noting that some of these are crying out for use
> > of devm_add_action_or_reset() and getting rid of the remove functions
> > entirely now you've dropped the oddity of them returning non 0.
> > 
> > Low hanging fruit for any newbies who want to do it, or maybe I will
> > if I get bored :)
> 
> Series applied to the togreg branch of iio.git and pushed out as testing for
> 0-day to see if it can find anything we missed.

They are in testing
(https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/log/?h=testing)
but not in togreg
(https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/log/?h=togreg).

Not sure if that is how it's supposed to be? Only togreg seems to be in
next.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

Re: [PATCH 0/9] iio: Remove duplicated error reporting in .remove()

[Jonathan Cameron]

On Fri, 13 May 2022 09:24:24 +0200
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:

> Hello Jonathan,
> 
> On Sat, May 07, 2022 at 03:38:55PM +0100, Jonathan Cameron wrote:
> > On Sun, 1 May 2022 18:51:23 +0100
> > Jonathan Cameron <jic23@kernel.org> wrote:
> >   
> > > On Sun, 1 May 2022 18:41:49 +0100
> > > Jonathan Cameron <jic23@kernel.org> wrote:
> > >   
> > > > On Sat, 30 Apr 2022 10:15:58 +0200
> > > > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> > > >     
> > > > > Hello,
> > > > > 
> > > > > this series adapts several i2c drivers that emit two error messages if
> > > > > something in their remove function fails. The relevant issue is that the
> > > > > i2c core emits an error message if the remove callback returns a
> > > > > non-zero value but the drivers already emit a (better) message. So
> > > > > these patches change the drivers to return 0 even after an error. Note
> > > > > there is no further error handling in the i2c core, if a remove callback
> > > > > returns an error code, the device is removed anyhow, so the only effect
> > > > > of making the return value zero is that the error message is suppressed.
> > > > > 
> > > > > The motivation for this series is to eventually change the prototype of
> > > > > the i2c remove callback to return void. As a preparation all remove
> > > > > functions should return 0 such that changing the prototype doesn't
> > > > > change behaviour of individual drivers.      
> > > > 
> > > > I think I'd rather have seen these called out as simply moving towards
> > > > this second change as it feels wrong to deliberately not report an error
> > > > so as to avoid repeated error messages!
> > > > 
> > > > Meh, I don't care that strongly and you call out the real reason in each
> > > > patch.    
> > > 
> > > Series looks fine to me, but I'll leave the on list for a few days to let
> > > others have time to take a look.
> > > 
> > > Worth noting that some of these are crying out for use
> > > of devm_add_action_or_reset() and getting rid of the remove functions
> > > entirely now you've dropped the oddity of them returning non 0.
> > > 
> > > Low hanging fruit for any newbies who want to do it, or maybe I will
> > > if I get bored :)  
> > 
> > Series applied to the togreg branch of iio.git and pushed out as testing for
> > 0-day to see if it can find anything we missed.  
> 
> They are in testing
> (https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/log/?h=testing)
> but not in togreg
> (https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/log/?h=togreg).
> 
> Not sure if that is how it's supposed to be? Only togreg seems to be in
> next.
Yup. That's intentional because I don't rebase the togreg branch unless
something goes wrong when it hits next.  The intent being it's a stable
base to build upon.  Normally there is a delay of up to a week to let
0-day take a look at testing and for me to happen to get time sat at
the right computer, but sometimes it's longer :(

Right now I'm waiting on a pull request being picked up by Greg KH,
after which I'll fast forward the togreg branch as I have some patches
waiting to be queued up that are dependent on things that have reached
char-misc-next via other paths.

Unfortunately I'm doubtful about whether I can squeeze in a second
pull request this cycle, so they may get queued up for next cycle.
A bit of bad timing :(

Jonathan



> 
> Best regards
> Uwe
> 

Re: [PATCH 0/9] iio: Remove duplicated error reporting in .remove()

[Uwe Kleine-König]

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

Hello Jonathan,

On Sat, May 14, 2022 at 02:31:51PM +0100, Jonathan Cameron wrote:
> On Fri, 13 May 2022 09:24:24 +0200
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> > On Sat, May 07, 2022 at 03:38:55PM +0100, Jonathan Cameron wrote:
> > > On Sun, 1 May 2022 18:51:23 +0100
> > > Jonathan Cameron <jic23@kernel.org> wrote:
> > >   
> > > > On Sun, 1 May 2022 18:41:49 +0100
> > > > Jonathan Cameron <jic23@kernel.org> wrote:
> > > >   
> > > > > On Sat, 30 Apr 2022 10:15:58 +0200
> > > > > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> > > > >     
> > > > > > Hello,
> > > > > > 
> > > > > > this series adapts several i2c drivers that emit two error messages if
> > > > > > something in their remove function fails. The relevant issue is that the
> > > > > > i2c core emits an error message if the remove callback returns a
> > > > > > non-zero value but the drivers already emit a (better) message. So
> > > > > > these patches change the drivers to return 0 even after an error. Note
> > > > > > there is no further error handling in the i2c core, if a remove callback
> > > > > > returns an error code, the device is removed anyhow, so the only effect
> > > > > > of making the return value zero is that the error message is suppressed.
> > > > > > 
> > > > > > The motivation for this series is to eventually change the prototype of
> > > > > > the i2c remove callback to return void. As a preparation all remove
> > > > > > functions should return 0 such that changing the prototype doesn't
> > > > > > change behaviour of individual drivers.      
> > > > > 
> > > > > I think I'd rather have seen these called out as simply moving towards
> > > > > this second change as it feels wrong to deliberately not report an error
> > > > > so as to avoid repeated error messages!
> > > > > 
> > > > > Meh, I don't care that strongly and you call out the real reason in each
> > > > > patch.    
> > > > 
> > > > Series looks fine to me, but I'll leave the on list for a few days to let
> > > > others have time to take a look.
> > > > 
> > > > Worth noting that some of these are crying out for use
> > > > of devm_add_action_or_reset() and getting rid of the remove functions
> > > > entirely now you've dropped the oddity of them returning non 0.
> > > > 
> > > > Low hanging fruit for any newbies who want to do it, or maybe I will
> > > > if I get bored :)  
> > > 
> > > Series applied to the togreg branch of iio.git and pushed out as testing for
> > > 0-day to see if it can find anything we missed.  
> > 
> > They are in testing
> > (https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/log/?h=testing)
> > but not in togreg
> > (https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/log/?h=togreg).
> > 
> > Not sure if that is how it's supposed to be? Only togreg seems to be in
> > next.
> Yup. That's intentional because I don't rebase the togreg branch unless
> something goes wrong when it hits next.  The intent being it's a stable
> base to build upon.  Normally there is a delay of up to a week to let
> 0-day take a look at testing and for me to happen to get time sat at
> the right computer, but sometimes it's longer :(
> 
> Right now I'm waiting on a pull request being picked up by Greg KH,
> after which I'll fast forward the togreg branch as I have some patches
> waiting to be queued up that are dependent on things that have reached
> char-misc-next via other paths.

Not sure I understood that correctly. Do you let Greg pull the togreg
branch? If you instead let him pull tags, you don't have to wait in such
a situation to apply new patches to a for-next branch. (Or just don't
use "togreg" for both, sending PRs to Greg and put patches into next.)

> Unfortunately I'm doubtful about whether I can squeeze in a second
> pull request this cycle, so they may get queued up for next cycle.
> A bit of bad timing :(

It's not a big problem for me. There is still much to do (also a bit in
drivers/iio) before I tackle the final bits of my quest and actually
change struct i2c_driver (and so depend on these patches having hit
mainline). The only downside for you is, that you might have to endure
me asking again for the state of these patches ;-)

Thanks for your feedback. Compared to pinging repeatedly and getting no
maintainer reply, knowing about your problems is much appreciated.

Best regards and have a nice week-end,
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

Re: [PATCH 0/9] iio: Remove duplicated error reporting in .remove()

[Jonathan Cameron]

On Sat, 14 May 2022 15:41:59 +0200
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:

> Hello Jonathan,
> 
> On Sat, May 14, 2022 at 02:31:51PM +0100, Jonathan Cameron wrote:
> > On Fri, 13 May 2022 09:24:24 +0200
> > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:  
> > > On Sat, May 07, 2022 at 03:38:55PM +0100, Jonathan Cameron wrote:  
> > > > On Sun, 1 May 2022 18:51:23 +0100
> > > > Jonathan Cameron <jic23@kernel.org> wrote:
> > > >     
> > > > > On Sun, 1 May 2022 18:41:49 +0100
> > > > > Jonathan Cameron <jic23@kernel.org> wrote:
> > > > >     
> > > > > > On Sat, 30 Apr 2022 10:15:58 +0200
> > > > > > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> > > > > >       
> > > > > > > Hello,
> > > > > > > 
> > > > > > > this series adapts several i2c drivers that emit two error messages if
> > > > > > > something in their remove function fails. The relevant issue is that the
> > > > > > > i2c core emits an error message if the remove callback returns a
> > > > > > > non-zero value but the drivers already emit a (better) message. So
> > > > > > > these patches change the drivers to return 0 even after an error. Note
> > > > > > > there is no further error handling in the i2c core, if a remove callback
> > > > > > > returns an error code, the device is removed anyhow, so the only effect
> > > > > > > of making the return value zero is that the error message is suppressed.
> > > > > > > 
> > > > > > > The motivation for this series is to eventually change the prototype of
> > > > > > > the i2c remove callback to return void. As a preparation all remove
> > > > > > > functions should return 0 such that changing the prototype doesn't
> > > > > > > change behaviour of individual drivers.        
> > > > > > 
> > > > > > I think I'd rather have seen these called out as simply moving towards
> > > > > > this second change as it feels wrong to deliberately not report an error
> > > > > > so as to avoid repeated error messages!
> > > > > > 
> > > > > > Meh, I don't care that strongly and you call out the real reason in each
> > > > > > patch.      
> > > > > 
> > > > > Series looks fine to me, but I'll leave the on list for a few days to let
> > > > > others have time to take a look.
> > > > > 
> > > > > Worth noting that some of these are crying out for use
> > > > > of devm_add_action_or_reset() and getting rid of the remove functions
> > > > > entirely now you've dropped the oddity of them returning non 0.
> > > > > 
> > > > > Low hanging fruit for any newbies who want to do it, or maybe I will
> > > > > if I get bored :)    
> > > > 
> > > > Series applied to the togreg branch of iio.git and pushed out as testing for
> > > > 0-day to see if it can find anything we missed.    
> > > 
> > > They are in testing
> > > (https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/log/?h=testing)
> > > but not in togreg
> > > (https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/log/?h=togreg).
> > > 
> > > Not sure if that is how it's supposed to be? Only togreg seems to be in
> > > next.  
> > Yup. That's intentional because I don't rebase the togreg branch unless
> > something goes wrong when it hits next.  The intent being it's a stable
> > base to build upon.  Normally there is a delay of up to a week to let
> > 0-day take a look at testing and for me to happen to get time sat at
> > the right computer, but sometimes it's longer :(
> > 
> > Right now I'm waiting on a pull request being picked up by Greg KH,
> > after which I'll fast forward the togreg branch as I have some patches
> > waiting to be queued up that are dependent on things that have reached
> > char-misc-next via other paths.  
> 
> Not sure I understood that correctly. Do you let Greg pull the togreg
> branch? If you instead let him pull tags, you don't have to wait in such
> a situation to apply new patches to a for-next branch. (Or just don't
> use "togreg" for both, sending PRs to Greg and put patches into next.)

The pull request is indeed for a tag.

I could expose what is currently only out as testing to next, but that
means pushing it out as togreg.
As a general rule (the vast majority of the time) I do not rebase togreg
once I've pushed that out to kernel.org. (Testing gets rebased all the
time but hopefully the name makes it clear it's not stable!)

I need to rebase my local togreg currently to get some fixes that are
in Greg's tree, but not mine. If I do that before he's take my pull
things will look unecessarily messy in the history.

It's one of those silly corner cases where by waiting a little while
I can hide the mess under the covers :)


> 
> > Unfortunately I'm doubtful about whether I can squeeze in a second
> > pull request this cycle, so they may get queued up for next cycle.
> > A bit of bad timing :(  
> 
> It's not a big problem for me. There is still much to do (also a bit in
> drivers/iio) before I tackle the final bits of my quest and actually
> change struct i2c_driver (and so depend on these patches having hit
> mainline). The only downside for you is, that you might have to endure
> me asking again for the state of these patches ;-)
> 
> Thanks for your feedback. Compared to pinging repeatedly and getting no
> maintainer reply, knowing about your problems is much appreciated.
> 
> Best regards and have a nice week-end,
Likewise!

Jonathan

> Uwe
> 

Re: [PATCH 0/9] iio: Remove duplicated error reporting in .remove()

[Uwe Kleine-König]

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

Hello Jonathan,

On Sun, May 01, 2022 at 06:41:49PM +0100, Jonathan Cameron wrote:
> On Sat, 30 Apr 2022 10:15:58 +0200
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> 
> > Hello,
> > 
> > this series adapts several i2c drivers that emit two error messages if
> > something in their remove function fails. The relevant issue is that the
> > i2c core emits an error message if the remove callback returns a
> > non-zero value but the drivers already emit a (better) message. So
> > these patches change the drivers to return 0 even after an error. Note
> > there is no further error handling in the i2c core, if a remove callback
> > returns an error code, the device is removed anyhow, so the only effect
> > of making the return value zero is that the error message is suppressed.
> > 
> > The motivation for this series is to eventually change the prototype of
> > the i2c remove callback to return void. As a preparation all remove
> > functions should return 0 such that changing the prototype doesn't
> > change behaviour of individual drivers.
> 
> I think I'd rather have seen these called out as simply moving towards
> this second change as it feels wrong to deliberately not report an error
> so as to avoid repeated error messages!

I admit this is a bit strange. Once the i2c remove callback is changed
accordingly (and the platform remove callback, and the ac97_codec_driver
remove callback ...) this goes away. Working on it.

The reason it's that way is that for similar patches the maintainer
feedback was to include the preparatory patch in the series that
actually changes the remove callback. As I like to have as much of the
preparatory patches already applied, I thought it a good idea to find a
motivation to apply already today :-)

Thanks for your cooperation,
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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