Message ID | 20220815080230.37408-1-u.kleine-koenig@pengutronix.de |
---|---|
Headers | show |
Series | i2c: Make remove callback return void | expand |
Hello, On Mon, Aug 15, 2022 at 10:02:30AM +0200, Uwe Kleine-König wrote: > The value returned by an i2c driver's remove function is mostly ignored. > (Only an error message is printed if the value is non-zero that the > error is ignored.) > > So change the prototype of the remove function to return no value. This > way driver authors are not tempted to assume that passing an error to > the upper layer is a good idea. All drivers are adapted accordingly. > There is no intended change of behaviour, all callbacks were prepared to > return 0 before. > > [...] > --- > [...] > 619 files changed, 646 insertions(+), 1733 deletions(-) There seems to be a 300k limit for mails on vger.kernel.org, so this patch didn't make it to the list. Please get it from https://git.pengutronix.de/git/ukl/linux i2c-remove-void or https://git.pengutronix.de/cgit/ukl/linux/commit/?h=i2c-remove-void or send me a (private) mail, then I can bounce you the message to properly reply to it. (Or if that is enough for you: The Message-Id is <20220815080230.37408-7-u.kleine-koenig@pengutronix.de>.) Best regards Uwe
On Mon, Aug 15, 2022 at 10:02 AM Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > > All platforms that provide a teardown callback return 0. New users are > supposed to not make use of platform support, so there is no > functionality lost. > > This patch is a preparation for making i2c remove callbacks return void. > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > arch/arm/mach-davinci/board-da850-evm.c | 12 ++++-------- > drivers/gpio/gpio-pca953x.c | 11 +++-------- > include/linux/platform_data/pca953x.h | 2 +- > 3 files changed, 8 insertions(+), 17 deletions(-) > > diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c > index 92d74bc71967..d752ee2b30ff 100644 > --- a/arch/arm/mach-davinci/board-da850-evm.c > +++ b/arch/arm/mach-davinci/board-da850-evm.c > @@ -516,8 +516,8 @@ static int da850_evm_ui_expander_setup(struct i2c_client *client, unsigned gpio, > return ret; > } > > -static int da850_evm_ui_expander_teardown(struct i2c_client *client, > - unsigned gpio, unsigned ngpio, void *c) > +static void da850_evm_ui_expander_teardown(struct i2c_client *client, > + unsigned gpio, unsigned ngpio, void *c) > { > platform_device_unregister(&da850_evm_ui_keys_device); > > @@ -529,8 +529,6 @@ static int da850_evm_ui_expander_teardown(struct i2c_client *client, > gpio_free(gpio + DA850_EVM_UI_EXP_SEL_C); > gpio_free(gpio + DA850_EVM_UI_EXP_SEL_B); > gpio_free(gpio + DA850_EVM_UI_EXP_SEL_A); > - > - return 0; > } > > /* assign the baseboard expander's GPIOs after the UI board's */ > @@ -697,13 +695,11 @@ static int da850_evm_bb_expander_setup(struct i2c_client *client, > return ret; > } > > -static int da850_evm_bb_expander_teardown(struct i2c_client *client, > - unsigned gpio, unsigned ngpio, void *c) > +static void da850_evm_bb_expander_teardown(struct i2c_client *client, > + unsigned gpio, unsigned ngpio, void *c) > { > platform_device_unregister(&da850_evm_bb_leds_device); > platform_device_unregister(&da850_evm_bb_keys_device); > - > - return 0; > } > > static struct pca953x_platform_data da850_evm_ui_expander_info = { > diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c > index ecd7d169470b..1860e566eb94 100644 > --- a/drivers/gpio/gpio-pca953x.c > +++ b/drivers/gpio/gpio-pca953x.c > @@ -1105,20 +1105,15 @@ static int pca953x_remove(struct i2c_client *client) > { > struct pca953x_platform_data *pdata = dev_get_platdata(&client->dev); > struct pca953x_chip *chip = i2c_get_clientdata(client); > - int ret; > > if (pdata && pdata->teardown) { > - ret = pdata->teardown(client, chip->gpio_chip.base, > - chip->gpio_chip.ngpio, pdata->context); > - if (ret < 0) > - dev_err(&client->dev, "teardown failed, %d\n", ret); > - } else { > - ret = 0; > + pdata->teardown(client, chip->gpio_chip.base, > + chip->gpio_chip.ngpio, pdata->context); > } > > regulator_disable(chip->regulator); > > - return ret; > + return 0; > } > > #ifdef CONFIG_PM_SLEEP > diff --git a/include/linux/platform_data/pca953x.h b/include/linux/platform_data/pca953x.h > index 4eb53e023997..96c1a14ab365 100644 > --- a/include/linux/platform_data/pca953x.h > +++ b/include/linux/platform_data/pca953x.h > @@ -22,7 +22,7 @@ struct pca953x_platform_data { > int (*setup)(struct i2c_client *client, > unsigned gpio, unsigned ngpio, > void *context); > - int (*teardown)(struct i2c_client *client, > + void (*teardown)(struct i2c_client *client, > unsigned gpio, unsigned ngpio, > void *context); > const char *const *names; > -- > 2.36.1 > Acked-by: Bartosz Golaszewski <brgl@bgdev.pl>
On Mon, Aug 15, 2022 at 11:02 AM Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > > All platforms that provide a teardown callback return 0. New users are > supposed to not make use of platform support, so there is no > functionality lost. > > This patch is a preparation for making i2c remove callbacks return void. In case you need to send another version, consider below... ... > if (pdata && pdata->teardown) { > - ret = pdata->teardown(client, chip->gpio_chip.base, > - chip->gpio_chip.ngpio, pdata->context); > - if (ret < 0) > - dev_err(&client->dev, "teardown failed, %d\n", ret); > - } else { > - ret = 0; > + pdata->teardown(client, chip->gpio_chip.base, > + chip->gpio_chip.ngpio, pdata->context); > } First of all, after this change the {} should not be needed. Second, with a temporary variable struct gpio_chip *gc = &chip->gpio_chip; this becomes a one liner like pdata->teardown(client, gc->base, gc->ngpio, pdata->context);
On Tue, Aug 16, 2022 at 11:16 AM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Mon, Aug 15, 2022 at 11:02 AM Uwe Kleine-König > <u.kleine-koenig@pengutronix.de> wrote: > > > > All platforms that provide a teardown callback return 0. New users are > > supposed to not make use of platform support, so there is no > > functionality lost. > > > > This patch is a preparation for making i2c remove callbacks return void. > > In case you need to send another version, consider below... (forgot to add) otherwise it can be a follow up. > > if (pdata && pdata->teardown) { > > - ret = pdata->teardown(client, chip->gpio_chip.base, > > - chip->gpio_chip.ngpio, pdata->context); > > - if (ret < 0) > > - dev_err(&client->dev, "teardown failed, %d\n", ret); > > - } else { > > - ret = 0; > > + pdata->teardown(client, chip->gpio_chip.base, > > + chip->gpio_chip.ngpio, pdata->context); > > } > > First of all, after this change the {} should not be needed. > Second, with a temporary variable > > struct gpio_chip *gc = &chip->gpio_chip; > > this becomes a one liner like > > pdata->teardown(client, gc->base, gc->ngpio, pdata->context); -- With Best Regards, Andy Shevchenko
For mlx90614 and mlx90632: Acked-by: Crt Mori<cmo@melexis.com> Best regards, Crt On Mon, 15 Aug 2022 at 10:50, Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > > Hello, > > On Mon, Aug 15, 2022 at 10:02:30AM +0200, Uwe Kleine-König wrote: > > The value returned by an i2c driver's remove function is mostly ignored. > > (Only an error message is printed if the value is non-zero that the > > error is ignored.) > > > > So change the prototype of the remove function to return no value. This > > way driver authors are not tempted to assume that passing an error to > > the upper layer is a good idea. All drivers are adapted accordingly. > > There is no intended change of behaviour, all callbacks were prepared to > > return 0 before. > > > > [...] > > --- > > [...] > > 619 files changed, 646 insertions(+), 1733 deletions(-) > > There seems to be a 300k limit for mails on vger.kernel.org, so this > patch didn't make it to the list. Please get it from > > https://git.pengutronix.de/git/ukl/linux i2c-remove-void > > or > > https://git.pengutronix.de/cgit/ukl/linux/commit/?h=i2c-remove-void > > or send me a (private) mail, then I can bounce you the message to > properly reply to it. (Or if that is enough for you: The Message-Id is > <20220815080230.37408-7-u.kleine-koenig@pengutronix.de>.) > > Best regards > Uwe > > -- > Pengutronix e.K. | Uwe Kleine-König | > Industrial Linux Solutions | https://www.pengutronix.de/ |
On Mon, Aug 15, 2022 at 10:02:25AM +0200, Uwe Kleine-König wrote: > A remove callback that just returns 0 is equivalent to no callback at all > as can be seen in i2c_device_remove(). So simplify accordingly. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Applied to an immutable branch, thanks!
On Mon, Aug 15, 2022 at 10:02:27AM +0200, Uwe Kleine-König wrote: > The mutex might still be in use until the devm cleanup callback > devm_led_classdev_flash_release() is called. This only happens some time > after lm3601x_remove() completed. > > Fixes: e63a744871a3 ("leds: lm3601x: Convert class registration to device managed") > Acked-by: Pavel Machek <pavel@ucw.cz> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Applied to an immutable branch, thanks!
On Mon, Aug 15, 2022 at 10:02:29AM +0200, Uwe Kleine-König wrote: > All platforms that provide a teardown callback return 0. New users are > supposed to not make use of platform support, so there is no > functionality lost. > > This patch is a preparation for making i2c remove callbacks return void. > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Applied to an immutable branch, thanks!
On Mon, Aug 15, 2022 at 10:02:30AM +0200, Uwe Kleine-König wrote: > The value returned by an i2c driver's remove function is mostly ignored. > (Only an error message is printed if the value is non-zero that the > error is ignored.) > > So change the prototype of the remove function to return no value. This > way driver authors are not tempted to assume that passing an error to > the upper layer is a good idea. All drivers are adapted accordingly. > There is no intended change of behaviour, all callbacks were prepared to > return 0 before. > > Reviewed-by: Peter Senna Tschudin <peter.senna@gmail.com> > Reviewed-by: Jeremy Kerr <jk@codeconstruct.com.au> > Reviewed-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com> > Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> > Reviewed-by: Crt Mori <cmo@melexis.com> > Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Acked-by: Marek Behún <kabel@kernel.org> # for leds-turris-omnia > Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Reviewed-by: Petr Machata <petrm@nvidia.com> # for mlxsw > Reviewed-by: Maximilian Luz <luzmaximilian@gmail.com> # for surface3_power > Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> # for bmc150-accel-i2c + kxcjk-1013 > Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> # for media/* + staging/media/* > Acked-by: Miguel Ojeda <ojeda@kernel.org> # for auxdisplay/ht16k33 + auxdisplay/lcd2s > Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net> # for versaclock5 > Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com> # for versaclock5 > Reviewed-by: Ajay Gupta <ajayg@nvidia.com> # for ucsi_ccg > Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> # for iio > Acked-by: Peter Rosin <peda@axentia.se> # for i2c-mux-*, max9860 > Acked-by: Adrien Grassein <adrien.grassein@gmail.com> # for lontium-lt8912b > Reviewed-by: Jean Delvare <jdelvare@suse.de> # for hwmon, i2c-core and i2c/muxes > Acked-by: Corey Minyard <cninyard@mvista.com> # for IPMI > Reviewed-by: Vladimir Oltean <olteanv@gmail.com> > Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com> # for drivers/power > Acked-by: Krzysztof Hałasa <khalasa@piap.pl> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Applied to an immutable branch, thanks!
> As some conflicts are expected I sent this early after -rc1 such that it > can be included early into next and be put on an immutable branch for > subsystems to resolve merge conflicts. I pushed the series out now, so it should hit -next tomorrow. The immutable branch is here: git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/make_remove_callback_void-immutable Enjoy!