[PATCH] leds: pca9532: Make pca9532_destroy_devices() return void

[PATCH] leds: pca9532: Make pca9532_destroy_devices() return void

[Uwe Kleine-König]

Up to now pca9532_destroy_devices() returns zero unconditionally. Make
it return void instead which makes it easier to see in the callers that
there is no error to handle.

Also the return value of i2c remove callbacks is ignored anyway.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/leds/leds-pca9532.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/leds/leds-pca9532.c b/drivers/leds/leds-pca9532.c
index 017794bb87ae..674590576b0a 100644
--- a/drivers/leds/leds-pca9532.c
+++ b/drivers/leds/leds-pca9532.c
@@ -318,7 +318,7 @@ static int pca9532_gpio_direction_output(struct gpio_chip *gc, unsigned offset,
 }
 #endif /* CONFIG_LEDS_PCA9532_GPIO */
 
-static int pca9532_destroy_devices(struct pca9532_data *data, int n_devs)
+static void pca9532_destroy_devices(struct pca9532_data *data, int n_devs)
 {
 	int i = n_devs;
 
@@ -555,7 +555,9 @@ static int pca9532_remove(struct i2c_client *client)
 {
 	struct pca9532_data *data = i2c_get_clientdata(client);
 
-	return pca9532_destroy_devices(data, data->chip_info->num_leds);
+	pca9532_destroy_devices(data, data->chip_info->num_leds);
+
+	return 0;
 }
 
 module_i2c_driver(pca9532_driver);
-- 
2.30.2

[PATCH v2] leds: pca9532: Make pca9532_destroy_devices() return void

[Uwe Kleine-König]

Up to now pca9532_destroy_devices() returns always zero. Remove the
never-taken error path and make it return void which makes it easier to
see in the callers that there is no error to handle.

Also the return value of i2c remove callbacks is ignored anyway.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/leds/leds-pca9532.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/leds/leds-pca9532.c b/drivers/leds/leds-pca9532.c
index 017794bb87ae..674590576b0a 100644
--- a/drivers/leds/leds-pca9532.c
+++ b/drivers/leds/leds-pca9532.c
@@ -318,7 +318,7 @@ static int pca9532_gpio_direction_output(struct gpio_chip *gc, unsigned offset,
 }
 #endif /* CONFIG_LEDS_PCA9532_GPIO */
 
-static int pca9532_destroy_devices(struct pca9532_data *data, int n_devs)
+static void pca9532_destroy_devices(struct pca9532_data *data, int n_devs)
 {
 	int i = n_devs;
 
@@ -555,7 +555,9 @@ static int pca9532_remove(struct i2c_client *client)
 {
 	struct pca9532_data *data = i2c_get_clientdata(client);
 
-	return pca9532_destroy_devices(data, data->chip_info->num_leds);
+	pca9532_destroy_devices(data, data->chip_info->num_leds);
+
+	return 0;
 }
 
 module_i2c_driver(pca9532_driver);
-- 
2.30.2

[PATCH v3] leds: pca9532: Make pca9532_destroy_devices() return void

[Uwe Kleine-König]

Up to now pca9532_destroy_devices() returns always zero because it's
always called with data != NULL. Remove the never-taken error path and
make it return void which makes it easier to see in the callers that
there is no error to handle.

Also the return value of i2c remove callbacks is ignored anyway.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
How embarrassing, I fatfingered v2 that was supposed to fix compilation
issues. I forgot to actuall add the needed changes and only adapted the
commit log :-\

This one is good now (I hope!)

Sorry for the inconvenience,
Uwe

 drivers/leds/leds-pca9532.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/leds/leds-pca9532.c b/drivers/leds/leds-pca9532.c
index 017794bb87ae..f72b5d1be3a6 100644
--- a/drivers/leds/leds-pca9532.c
+++ b/drivers/leds/leds-pca9532.c
@@ -318,13 +318,10 @@ static int pca9532_gpio_direction_output(struct gpio_chip *gc, unsigned offset,
 }
 #endif /* CONFIG_LEDS_PCA9532_GPIO */
 
-static int pca9532_destroy_devices(struct pca9532_data *data, int n_devs)
+static void pca9532_destroy_devices(struct pca9532_data *data, int n_devs)
 {
 	int i = n_devs;
 
-	if (!data)
-		return -EINVAL;
-
 	while (--i >= 0) {
 		switch (data->leds[i].type) {
 		case PCA9532_TYPE_NONE:
@@ -346,8 +343,6 @@ static int pca9532_destroy_devices(struct pca9532_data *data, int n_devs)
 	if (data->gpio.parent)
 		gpiochip_remove(&data->gpio);
 #endif
-
-	return 0;
 }
 
 static int pca9532_configure(struct i2c_client *client,
@@ -555,7 +550,9 @@ static int pca9532_remove(struct i2c_client *client)
 {
 	struct pca9532_data *data = i2c_get_clientdata(client);
 
-	return pca9532_destroy_devices(data, data->chip_info->num_leds);
+	pca9532_destroy_devices(data, data->chip_info->num_leds);
+
+	return 0;
 }
 
 module_i2c_driver(pca9532_driver);
-- 
2.30.2

Re: [PATCH v3] leds: pca9532: Make pca9532_destroy_devices() return void

[Uwe Kleine-König]

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

On Fri, Oct 22, 2021 at 12:17:07PM +0200, Uwe Kleine-König wrote:
> Up to now pca9532_destroy_devices() returns always zero because it's
> always called with data != NULL. Remove the never-taken error path and
> make it return void which makes it easier to see in the callers that
> there is no error to handle.
> 
> Also the return value of i2c remove callbacks is ignored anyway.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Ping. This patch is part of an effort to make the i2c remove callback a
void function, too. Are there any concerns, or plans to pick up this
patch?

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 v3] leds: pca9532: Make pca9532_destroy_devices() return void

[Pavel Machek]

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

On Mon 2022-01-10 08:23:17, Uwe Kleine-König wrote:
> On Fri, Oct 22, 2021 at 12:17:07PM +0200, Uwe Kleine-König wrote:
> > Up to now pca9532_destroy_devices() returns always zero because it's
> > always called with data != NULL. Remove the never-taken error path and
> > make it return void which makes it easier to see in the callers that
> > there is no error to handle.
> > 
> > Also the return value of i2c remove callbacks is ignored anyway.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> Ping. This patch is part of an effort to make the i2c remove callback a
> void function, too. Are there any concerns, or plans to pick up this
> patch?

It looks like a simple cleanup with no effect outside of the
driver. Am I wrong?

Best regards,
								Pavel
-- 
http://www.livejournal.com/~pavelmachek

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

Re: [PATCH v3] leds: pca9532: Make pca9532_destroy_devices() return void

[Uwe Kleine-König]

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

On Tue, Jan 11, 2022 at 10:12:23PM +0100, Pavel Machek wrote:
> On Mon 2022-01-10 08:23:17, Uwe Kleine-König wrote:
> > On Fri, Oct 22, 2021 at 12:17:07PM +0200, Uwe Kleine-König wrote:
> > > Up to now pca9532_destroy_devices() returns always zero because it's
> > > always called with data != NULL. Remove the never-taken error path and
> > > make it return void which makes it easier to see in the callers that
> > > there is no error to handle.
> > > 
> > > Also the return value of i2c remove callbacks is ignored anyway.
> > > 
> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > 
> > Ping. This patch is part of an effort to make the i2c remove callback a
> > void function, too. Are there any concerns, or plans to pick up this
> > patch?
> 
> It looks like a simple cleanup with no effect outside of the
> driver. Am I wrong?

Yes, there is no intended effect on the compiled code.

The reason I want this patch is that I work on making the i2c remove
callback return void. As this has to touch all i2c drivers, the changes
to these should be as simple as possible, so ideally I just want to drop
the "return 0" there. Every return that might return a value != 0 is
more complicated to handle there.

So it's just to prepare this change an to make the code a tad easier to
read for a human.

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 v3] leds: pca9532: Make pca9532_destroy_devices() return void

[Uwe Kleine-König]

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

On Tue, Jan 11, 2022 at 10:30:32PM +0100, Uwe Kleine-König wrote:
> On Tue, Jan 11, 2022 at 10:12:23PM +0100, Pavel Machek wrote:
> > On Mon 2022-01-10 08:23:17, Uwe Kleine-König wrote:
> > > On Fri, Oct 22, 2021 at 12:17:07PM +0200, Uwe Kleine-König wrote:
> > > > Up to now pca9532_destroy_devices() returns always zero because it's
> > > > always called with data != NULL. Remove the never-taken error path and
> > > > make it return void which makes it easier to see in the callers that
> > > > there is no error to handle.
> > > > 
> > > > Also the return value of i2c remove callbacks is ignored anyway.
> > > > 
> > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > 
> > > Ping. This patch is part of an effort to make the i2c remove callback a
> > > void function, too. Are there any concerns, or plans to pick up this
> > > patch?
> > 
> > It looks like a simple cleanup with no effect outside of the
> > driver. Am I wrong?
> 
> Yes, there is no intended effect on the compiled code.

ah, oh, the answer should have been "No". :-)

> The reason I want this patch is that I work on making the i2c remove
> callback return void. As this has to touch all i2c drivers, the changes
> to these should be as simple as possible, so ideally I just want to drop
> the "return 0" there. Every return that might return a value != 0 is
> more complicated to handle there.
> 
> So it's just to prepare this change an to make the code a tad easier to
> read for a human.

Is this convincing? Is this patch still on the "to-review" (or still
better the "to-apply") list?

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 v3] leds: pca9532: Make pca9532_destroy_devices() return void

[Uwe Kleine-König]

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

On Thu, Mar 31, 2022 at 03:19:49PM +0200, Uwe Kleine-König wrote:
> On Tue, Jan 11, 2022 at 10:30:32PM +0100, Uwe Kleine-König wrote:
> > On Tue, Jan 11, 2022 at 10:12:23PM +0100, Pavel Machek wrote:
> > > On Mon 2022-01-10 08:23:17, Uwe Kleine-König wrote:
> > > > On Fri, Oct 22, 2021 at 12:17:07PM +0200, Uwe Kleine-König wrote:
> > > > > Up to now pca9532_destroy_devices() returns always zero because it's
> > > > > always called with data != NULL. Remove the never-taken error path and
> > > > > make it return void which makes it easier to see in the callers that
> > > > > there is no error to handle.
> > > > > 
> > > > > Also the return value of i2c remove callbacks is ignored anyway.
> > > > > 
> > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > > 
> > > > Ping. This patch is part of an effort to make the i2c remove callback a
> > > > void function, too. Are there any concerns, or plans to pick up this
> > > > patch?
> > > 
> > > It looks like a simple cleanup with no effect outside of the
> > > driver. Am I wrong?
> > 
> > Yes, there is no intended effect on the compiled code.
> 
> ah, oh, the answer should have been "No". :-)
> 
> > The reason I want this patch is that I work on making the i2c remove
> > callback return void. As this has to touch all i2c drivers, the changes
> > to these should be as simple as possible, so ideally I just want to drop
> > the "return 0" there. Every return that might return a value != 0 is
> > more complicated to handle there.
> > 
> > So it's just to prepare this change an to make the code a tad easier to
> > read for a human.
> 
> Is this convincing? Is this patch still on the "to-review" (or still
> better the "to-apply") list?

The patch was sent initially before the v5.15 release. So it didn't
receive a maintainer decision during three merge windows. Is this patch
still considered?

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 v3] leds: pca9532: Make pca9532_destroy_devices() return void

[Pavel Machek]

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

Hi!

> Up to now pca9532_destroy_devices() returns always zero because it's
> always called with data != NULL. Remove the never-taken error path and
> make it return void which makes it easier to see in the callers that
> there is no error to handle.
> 
> Also the return value of i2c remove callbacks is ignored anyway.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Thank you, applied.

Best regards,
								Pavel



> ---
> How embarrassing, I fatfingered v2 that was supposed to fix compilation
> issues. I forgot to actuall add the needed changes and only adapted the
> commit log :-\
> 
> This one is good now (I hope!)
> 
> Sorry for the inconvenience,
> Uwe
> 
>  drivers/leds/leds-pca9532.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/leds/leds-pca9532.c b/drivers/leds/leds-pca9532.c
> index 017794bb87ae..f72b5d1be3a6 100644
> --- a/drivers/leds/leds-pca9532.c
> +++ b/drivers/leds/leds-pca9532.c
> @@ -318,13 +318,10 @@ static int pca9532_gpio_direction_output(struct gpio_chip *gc, unsigned offset,
>  }
>  #endif /* CONFIG_LEDS_PCA9532_GPIO */
>  
> -static int pca9532_destroy_devices(struct pca9532_data *data, int n_devs)
> +static void pca9532_destroy_devices(struct pca9532_data *data, int n_devs)
>  {
>  	int i = n_devs;
>  
> -	if (!data)
> -		return -EINVAL;
> -
>  	while (--i >= 0) {
>  		switch (data->leds[i].type) {
>  		case PCA9532_TYPE_NONE:
> @@ -346,8 +343,6 @@ static int pca9532_destroy_devices(struct pca9532_data *data, int n_devs)
>  	if (data->gpio.parent)
>  		gpiochip_remove(&data->gpio);
>  #endif
> -
> -	return 0;
>  }
>  
>  static int pca9532_configure(struct i2c_client *client,
> @@ -555,7 +550,9 @@ static int pca9532_remove(struct i2c_client *client)
>  {
>  	struct pca9532_data *data = i2c_get_clientdata(client);
>  
> -	return pca9532_destroy_devices(data, data->chip_info->num_leds);
> +	pca9532_destroy_devices(data, data->chip_info->num_leds);
> +
> +	return 0;
>  }
>  
>  module_i2c_driver(pca9532_driver);
> --

-- 
People of Russia, stop Putin before his war on Ukraine escalates.

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