Hello,
this series is part of my new quest to make spi remove callbacks return void. Today they return an int, but the only result of returning a non-zero value is a warning message. So it's a bad idea to return an error code in the expectation that not freeing some resources is ok then. The same holds true for i2c and platform devices which benefit en passant for a few drivers.
The patches in this series address some of the spi drivers that might return non-zero and adapt them accordingly to return zero instead. For most drivers it's just about not hiding the fact that they already return zero.
Given that there are quite some more patches of this type to create before I can change the spi remove callback, I suggest the respecive subsystem maintainers pick up these patches. There are no interdependencies in this series.
Uwe Kleine-König (13): drm/panel: s6e63m0: Make s6e63m0_remove() return void hwmon: adt7x10: Make adt7x10_remove() return void hwmon: max31722: Warn about failure to put device in stand-by in .remove() input: adxl34xx: Make adxl34x_remove() return void input: touchscreen: tsc200x: Make tsc200x_remove() return void media: cxd2880: Eliminate dead code mfd: mc13xxx: Make mc13xxx_common_exit() return void mfd: stmpe: Make stmpe_remove() return void mfd: tps65912: Make tps65912_device_exit() return void serial: max310x: Make max310x_remove() return void serial: sc16is7xx: Make sc16is7xx_remove() return void staging: fbtft: Make fbtft_remove_common() return void tpm: st33zp24: Make st33zp24_remove() return void
drivers/char/tpm/st33zp24/i2c.c | 5 +---- drivers/char/tpm/st33zp24/spi.c | 5 +---- drivers/char/tpm/st33zp24/st33zp24.c | 3 +-- drivers/char/tpm/st33zp24/st33zp24.h | 2 +- drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c | 3 ++- drivers/gpu/drm/panel/panel-samsung-s6e63m0-spi.c | 3 ++- drivers/gpu/drm/panel/panel-samsung-s6e63m0.c | 4 +--- drivers/gpu/drm/panel/panel-samsung-s6e63m0.h | 2 +- drivers/hwmon/adt7310.c | 3 ++- drivers/hwmon/adt7410.c | 3 ++- drivers/hwmon/adt7x10.c | 3 +-- drivers/hwmon/adt7x10.h | 2 +- drivers/hwmon/max31722.c | 8 +++++++- drivers/input/misc/adxl34x-i2c.c | 4 +++- drivers/input/misc/adxl34x-spi.c | 4 +++- drivers/input/misc/adxl34x.c | 4 +--- drivers/input/misc/adxl34x.h | 2 +- drivers/input/touchscreen/tsc2004.c | 4 +++- drivers/input/touchscreen/tsc2005.c | 4 +++- drivers/input/touchscreen/tsc200x-core.c | 4 +--- drivers/input/touchscreen/tsc200x-core.h | 2 +- drivers/media/spi/cxd2880-spi.c | 13 +------------ drivers/mfd/mc13xxx-core.c | 4 +--- drivers/mfd/mc13xxx-i2c.c | 3 ++- drivers/mfd/mc13xxx-spi.c | 3 ++- drivers/mfd/mc13xxx.h | 2 +- drivers/mfd/stmpe-i2c.c | 4 +++- drivers/mfd/stmpe-spi.c | 4 +++- drivers/mfd/stmpe.c | 4 +--- drivers/mfd/stmpe.h | 2 +- drivers/mfd/tps65912-core.c | 4 +--- drivers/mfd/tps65912-i2c.c | 4 +++- drivers/mfd/tps65912-spi.c | 4 +++- drivers/staging/fbtft/fbtft-core.c | 8 +------- drivers/staging/fbtft/fbtft.h | 6 ++++-- drivers/tty/serial/max310x.c | 7 +++---- drivers/tty/serial/sc16is7xx.c | 10 +++++++--- include/linux/mfd/tps65912.h | 2 +- 38 files changed, 77 insertions(+), 81 deletions(-)
base-commit: 9e1ff307c779ce1f0f810c7ecce3d95bbae40896
Up to now s6e63m0_remove() 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 spi remove callbacks is ignored anyway.
Signed-off-by: Uwe Kleine-König u.kleine-koenig@pengutronix.de --- drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c | 3 ++- drivers/gpu/drm/panel/panel-samsung-s6e63m0-spi.c | 3 ++- drivers/gpu/drm/panel/panel-samsung-s6e63m0.c | 4 +--- drivers/gpu/drm/panel/panel-samsung-s6e63m0.h | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c b/drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c index e0b1a7e354f3..e0f773678168 100644 --- a/drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c +++ b/drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c @@ -116,7 +116,8 @@ static int s6e63m0_dsi_probe(struct mipi_dsi_device *dsi) static int s6e63m0_dsi_remove(struct mipi_dsi_device *dsi) { mipi_dsi_detach(dsi); - return s6e63m0_remove(&dsi->dev); + s6e63m0_remove(&dsi->dev); + return 0; }
static const struct of_device_id s6e63m0_dsi_of_match[] = { diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e63m0-spi.c b/drivers/gpu/drm/panel/panel-samsung-s6e63m0-spi.c index 3669cc3719ce..c178d962b0d5 100644 --- a/drivers/gpu/drm/panel/panel-samsung-s6e63m0-spi.c +++ b/drivers/gpu/drm/panel/panel-samsung-s6e63m0-spi.c @@ -64,7 +64,8 @@ static int s6e63m0_spi_probe(struct spi_device *spi)
static int s6e63m0_spi_remove(struct spi_device *spi) { - return s6e63m0_remove(&spi->dev); + s6e63m0_remove(&spi->dev); + return 0; }
static const struct of_device_id s6e63m0_spi_of_match[] = { diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e63m0.c b/drivers/gpu/drm/panel/panel-samsung-s6e63m0.c index 35d72ac663d6..b34fa4d5de07 100644 --- a/drivers/gpu/drm/panel/panel-samsung-s6e63m0.c +++ b/drivers/gpu/drm/panel/panel-samsung-s6e63m0.c @@ -749,13 +749,11 @@ int s6e63m0_probe(struct device *dev, void *trsp, } EXPORT_SYMBOL_GPL(s6e63m0_probe);
-int s6e63m0_remove(struct device *dev) +void s6e63m0_remove(struct device *dev) { struct s6e63m0 *ctx = dev_get_drvdata(dev);
drm_panel_remove(&ctx->panel); - - return 0; } EXPORT_SYMBOL_GPL(s6e63m0_remove);
diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e63m0.h b/drivers/gpu/drm/panel/panel-samsung-s6e63m0.h index 306605ed1117..c926eca1c817 100644 --- a/drivers/gpu/drm/panel/panel-samsung-s6e63m0.h +++ b/drivers/gpu/drm/panel/panel-samsung-s6e63m0.h @@ -35,6 +35,6 @@ int s6e63m0_probe(struct device *dev, void *trsp, const u8 *data, size_t len), bool dsi_mode); -int s6e63m0_remove(struct device *dev); +void s6e63m0_remove(struct device *dev);
#endif /* _PANEL_SAMSUNG_S6E63M0_H */
Hi Uwe,
On Mon, Oct 11, 2021 at 03:27:42PM +0200, Uwe Kleine-König wrote:
Up to now s6e63m0_remove() 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 spi remove callbacks is ignored anyway.
Signed-off-by: Uwe Kleine-König u.kleine-koenig@pengutronix.de
Thanks, applied to drm-misc-next.
Sam
fbtft_remove_common() is only called with a non-NULL fb_info. (All callers are in remove callbacks and the matching probe callbacks set driver data accordingly.) So fbtft_remove_common() always returns zero. 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 platform and spi remove callbacks is ignored anyway and not freeing resources in .remove() is a bad idea.
Signed-off-by: Uwe Kleine-König u.kleine-koenig@pengutronix.de --- drivers/staging/fbtft/fbtft-core.c | 8 +------- drivers/staging/fbtft/fbtft.h | 6 ++++-- 2 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c index ed992ca605eb..9c9eab1182a6 100644 --- a/drivers/staging/fbtft/fbtft-core.c +++ b/drivers/staging/fbtft/fbtft-core.c @@ -1318,23 +1318,17 @@ EXPORT_SYMBOL(fbtft_probe_common); * @info: Framebuffer * * Unregisters and releases the framebuffer - * - * Return: 0 if successful, negative if error */ -int fbtft_remove_common(struct device *dev, struct fb_info *info) +void fbtft_remove_common(struct device *dev, struct fb_info *info) { struct fbtft_par *par;
- if (!info) - return -EINVAL; par = info->par; if (par) fbtft_par_dbg(DEBUG_DRIVER_INIT_FUNCTIONS, par, "%s()\n", __func__); fbtft_unregister_framebuffer(info); fbtft_framebuffer_release(info); - - return 0; } EXPORT_SYMBOL(fbtft_remove_common);
diff --git a/drivers/staging/fbtft/fbtft.h b/drivers/staging/fbtft/fbtft.h index 76f8c090a837..68eba6c71b0f 100644 --- a/drivers/staging/fbtft/fbtft.h +++ b/drivers/staging/fbtft/fbtft.h @@ -283,7 +283,8 @@ static int fbtft_driver_remove_spi(struct spi_device *spi) \ { \ struct fb_info *info = spi_get_drvdata(spi); \ \ - return fbtft_remove_common(&spi->dev, info); \ + fbtft_remove_common(&spi->dev, info); \ + return 0; \ } \ \ static int fbtft_driver_probe_pdev(struct platform_device *pdev) \ @@ -295,7 +296,8 @@ static int fbtft_driver_remove_pdev(struct platform_device *pdev) \ { \ struct fb_info *info = platform_get_drvdata(pdev); \ \ - return fbtft_remove_common(&pdev->dev, info); \ + fbtft_remove_common(&pdev->dev, info); \ + return 0; \ } \ \ static const struct of_device_id dt_ids[] = { \
Hello,
On Mon, Oct 11, 2021 at 03:27:41PM +0200, Uwe Kleine-König wrote:
this series is part of my new quest to make spi remove callbacks return void. Today they return an int, but the only result of returning a non-zero value is a warning message. So it's a bad idea to return an error code in the expectation that not freeing some resources is ok then. The same holds true for i2c and platform devices which benefit en passant for a few drivers.
The patches in this series address some of the spi drivers that might return non-zero and adapt them accordingly to return zero instead. For most drivers it's just about not hiding the fact that they already return zero.
Given that there are quite some more patches of this type to create before I can change the spi remove callback, I suggest the respecive subsystem maintainers pick up these patches. There are no interdependencies in this series.
Uwe Kleine-König (13): drm/panel: s6e63m0: Make s6e63m0_remove() return void hwmon: adt7x10: Make adt7x10_remove() return void hwmon: max31722: Warn about failure to put device in stand-by in .remove() input: adxl34xx: Make adxl34x_remove() return void input: touchscreen: tsc200x: Make tsc200x_remove() return void media: cxd2880: Eliminate dead code mfd: mc13xxx: Make mc13xxx_common_exit() return void mfd: stmpe: Make stmpe_remove() return void mfd: tps65912: Make tps65912_device_exit() return void serial: max310x: Make max310x_remove() return void serial: sc16is7xx: Make sc16is7xx_remove() return void staging: fbtft: Make fbtft_remove_common() return void tpm: st33zp24: Make st33zp24_remove() return void
I thought I would be a good enough programmer to not need build tests. Obviously I was wrong and introduced build problems with the following patches:
input: touchscreen: tsc200x: Make tsc200x_remove() return void mfd: mc13xxx: Make mc13xxx_common_exit() return void serial: max310x: Make max310x_remove() return void serial: sc16is7xx: Make sc16is7xx_remove() return void
Please don't apply these (unless you also fix the trivial problems in them). I will prepare a v2 soon.
Best regards and sorry for the inconvenience, Uwe
dri-devel@lists.freedesktop.org
-
Sam Ravnborg -
Uwe Kleine-König