linux-spi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/13] Make some spi device drivers return zero in .remove()
@ 2021-10-11 13:27 Uwe Kleine-König
  2021-10-11 13:27 ` [PATCH 01/13] drm/panel: s6e63m0: Make s6e63m0_remove() return void Uwe Kleine-König
                   ` (13 more replies)
  0 siblings, 14 replies; 21+ messages in thread
From: Uwe Kleine-König @ 2021-10-11 13:27 UTC (permalink / raw)
  To: Alexandre Torgue, Dmitry Torokhov, Greg Kroah-Hartman,
	Guenter Roeck, Jarkko Sakkinen, Jean Delvare, Jiri Slaby,
	Lee Jones, Mauro Carvalho Chehab, Maxime Coquelin,
	Michael Hennerich, Peter Huewe, Thierry Reding,
	Yasunari Takiguchi
  Cc: Mark Brown, Wolfram Sang, Rafael J. Wysocki,
	Jason Gunthorpe linux-integrity @ vger . kernel . org,
	Sam Ravnborg, dri-devel, kernel, linux-hwmon, linux-i2c,
	linux-input, linux-kernel, linux-media, linux-serial, linux-spi,
	linux-staging, linux-stm32

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
-- 
2.30.2


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 01/13] drm/panel: s6e63m0: Make s6e63m0_remove() return void
  2021-10-11 13:27 [PATCH 00/13] Make some spi device drivers return zero in .remove() Uwe Kleine-König
@ 2021-10-11 13:27 ` Uwe Kleine-König
  2021-10-11 13:27 ` [PATCH 02/13] hwmon: adt7x10: Make adt7x10_remove() " Uwe Kleine-König
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 21+ messages in thread
From: Uwe Kleine-König @ 2021-10-11 13:27 UTC (permalink / raw)
  To: Thierry Reding; +Cc: Mark Brown, linux-spi, Sam Ravnborg, kernel, dri-devel

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 */
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 02/13] hwmon: adt7x10: Make adt7x10_remove() return void
  2021-10-11 13:27 [PATCH 00/13] Make some spi device drivers return zero in .remove() Uwe Kleine-König
  2021-10-11 13:27 ` [PATCH 01/13] drm/panel: s6e63m0: Make s6e63m0_remove() return void Uwe Kleine-König
@ 2021-10-11 13:27 ` Uwe Kleine-König
  2021-10-11 14:19   ` Guenter Roeck
  2021-10-11 13:27 ` [PATCH 03/13] hwmon: max31722: Warn about failure to put device in stand-by in .remove() Uwe Kleine-König
                   ` (11 subsequent siblings)
  13 siblings, 1 reply; 21+ messages in thread
From: Uwe Kleine-König @ 2021-10-11 13:27 UTC (permalink / raw)
  To: Jean Delvare, Guenter Roeck
  Cc: Mark Brown, linux-spi, kernel, linux-hwmon, Wolfram Sang, linux-i2c

Up to now adt7x10_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 i2c and spi remove callbacks is ignored anyway.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/hwmon/adt7310.c | 3 ++-
 drivers/hwmon/adt7410.c | 3 ++-
 drivers/hwmon/adt7x10.c | 3 +--
 drivers/hwmon/adt7x10.h | 2 +-
 4 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/hwmon/adt7310.c b/drivers/hwmon/adt7310.c
index 9fad01191620..c40cac16af68 100644
--- a/drivers/hwmon/adt7310.c
+++ b/drivers/hwmon/adt7310.c
@@ -90,7 +90,8 @@ static int adt7310_spi_probe(struct spi_device *spi)
 
 static int adt7310_spi_remove(struct spi_device *spi)
 {
-	return adt7x10_remove(&spi->dev, spi->irq);
+	adt7x10_remove(&spi->dev, spi->irq);
+	return 0;
 }
 
 static const struct spi_device_id adt7310_id[] = {
diff --git a/drivers/hwmon/adt7410.c b/drivers/hwmon/adt7410.c
index 9d80895d0266..973db057427b 100644
--- a/drivers/hwmon/adt7410.c
+++ b/drivers/hwmon/adt7410.c
@@ -50,7 +50,8 @@ static int adt7410_i2c_probe(struct i2c_client *client)
 
 static int adt7410_i2c_remove(struct i2c_client *client)
 {
-	return adt7x10_remove(&client->dev, client->irq);
+	adt7x10_remove(&client->dev, client->irq);
+	return 0;
 }
 
 static const struct i2c_device_id adt7410_ids[] = {
diff --git a/drivers/hwmon/adt7x10.c b/drivers/hwmon/adt7x10.c
index 3f03b4cf5858..e9d33aa78a19 100644
--- a/drivers/hwmon/adt7x10.c
+++ b/drivers/hwmon/adt7x10.c
@@ -444,7 +444,7 @@ int adt7x10_probe(struct device *dev, const char *name, int irq,
 }
 EXPORT_SYMBOL_GPL(adt7x10_probe);
 
-int adt7x10_remove(struct device *dev, int irq)
+void adt7x10_remove(struct device *dev, int irq)
 {
 	struct adt7x10_data *data = dev_get_drvdata(dev);
 
@@ -457,7 +457,6 @@ int adt7x10_remove(struct device *dev, int irq)
 	sysfs_remove_group(&dev->kobj, &adt7x10_group);
 	if (data->oldconfig != data->config)
 		adt7x10_write_byte(dev, ADT7X10_CONFIG, data->oldconfig);
-	return 0;
 }
 EXPORT_SYMBOL_GPL(adt7x10_remove);
 
diff --git a/drivers/hwmon/adt7x10.h b/drivers/hwmon/adt7x10.h
index 21ad15ce3163..a1ae682eb32e 100644
--- a/drivers/hwmon/adt7x10.h
+++ b/drivers/hwmon/adt7x10.h
@@ -26,7 +26,7 @@ struct adt7x10_ops {
 
 int adt7x10_probe(struct device *dev, const char *name, int irq,
 	const struct adt7x10_ops *ops);
-int adt7x10_remove(struct device *dev, int irq);
+void adt7x10_remove(struct device *dev, int irq);
 
 #ifdef CONFIG_PM_SLEEP
 extern const struct dev_pm_ops adt7x10_dev_pm_ops;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 03/13] hwmon: max31722: Warn about failure to put device in stand-by in .remove()
  2021-10-11 13:27 [PATCH 00/13] Make some spi device drivers return zero in .remove() Uwe Kleine-König
  2021-10-11 13:27 ` [PATCH 01/13] drm/panel: s6e63m0: Make s6e63m0_remove() return void Uwe Kleine-König
  2021-10-11 13:27 ` [PATCH 02/13] hwmon: adt7x10: Make adt7x10_remove() " Uwe Kleine-König
@ 2021-10-11 13:27 ` Uwe Kleine-König
  2021-10-11 13:41   ` Hennerich, Michael
  2021-10-11 13:27 ` [PATCH 04/13] input: adxl34xx: Make adxl34x_remove() return void Uwe Kleine-König
                   ` (10 subsequent siblings)
  13 siblings, 1 reply; 21+ messages in thread
From: Uwe Kleine-König @ 2021-10-11 13:27 UTC (permalink / raw)
  To: Dmitry Torokhov, Michael Hennerich
  Cc: Mark Brown, linux-spi, kernel, linux-input

When an spi driver's remove function returns a non-zero error code
nothing happens apart from emitting a generic error message. Make this
error message more device specific and return zero instead.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/hwmon/max31722.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/max31722.c b/drivers/hwmon/max31722.c
index 613338cbcb17..4cf4fe6809a3 100644
--- a/drivers/hwmon/max31722.c
+++ b/drivers/hwmon/max31722.c
@@ -103,10 +103,16 @@ static int max31722_probe(struct spi_device *spi)
 static int max31722_remove(struct spi_device *spi)
 {
 	struct max31722_data *data = spi_get_drvdata(spi);
+	int ret;
 
 	hwmon_device_unregister(data->hwmon_dev);
 
-	return max31722_set_mode(data, MAX31722_MODE_STANDBY);
+	ret = max31722_set_mode(data, MAX31722_MODE_STANDBY);
+	if (ret)
+		/* There is nothing we can do about this ... */
+		dev_warn(&spi->dev, "Failed to put device in stand-by mode\n");
+
+	return 0;
 }
 
 static int __maybe_unused max31722_suspend(struct device *dev)
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 04/13] input: adxl34xx: Make adxl34x_remove() return void
  2021-10-11 13:27 [PATCH 00/13] Make some spi device drivers return zero in .remove() Uwe Kleine-König
                   ` (2 preceding siblings ...)
  2021-10-11 13:27 ` [PATCH 03/13] hwmon: max31722: Warn about failure to put device in stand-by in .remove() Uwe Kleine-König
@ 2021-10-11 13:27 ` Uwe Kleine-König
  2021-10-11 13:27 ` [PATCH 05/13] input: touchscreen: tsc200x: Make tsc200x_remove() " Uwe Kleine-König
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 21+ messages in thread
From: Uwe Kleine-König @ 2021-10-11 13:27 UTC (permalink / raw)
  Cc: Mark Brown, linux-spi, kernel, Wolfram Sang, linux-i2c

Up to now adxl34x_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 i2c and spi remove callbacks is ignored anyway.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 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 +-
 4 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/input/misc/adxl34x-i2c.c b/drivers/input/misc/adxl34x-i2c.c
index e64368a63346..a3b5f88d2bd1 100644
--- a/drivers/input/misc/adxl34x-i2c.c
+++ b/drivers/input/misc/adxl34x-i2c.c
@@ -103,7 +103,9 @@ static int adxl34x_i2c_remove(struct i2c_client *client)
 {
 	struct adxl34x *ac = i2c_get_clientdata(client);
 
-	return adxl34x_remove(ac);
+	adxl34x_remove(ac);
+
+	return 0;
 }
 
 static int __maybe_unused adxl34x_i2c_suspend(struct device *dev)
diff --git a/drivers/input/misc/adxl34x-spi.c b/drivers/input/misc/adxl34x-spi.c
index df6afa455e46..6e51c9bc619f 100644
--- a/drivers/input/misc/adxl34x-spi.c
+++ b/drivers/input/misc/adxl34x-spi.c
@@ -91,7 +91,9 @@ static int adxl34x_spi_remove(struct spi_device *spi)
 {
 	struct adxl34x *ac = spi_get_drvdata(spi);
 
-	return adxl34x_remove(ac);
+	adxl34x_remove(ac);
+
+	return 0;
 }
 
 static int __maybe_unused adxl34x_spi_suspend(struct device *dev)
diff --git a/drivers/input/misc/adxl34x.c b/drivers/input/misc/adxl34x.c
index 4cc4e8ff42b3..34beac80e6f0 100644
--- a/drivers/input/misc/adxl34x.c
+++ b/drivers/input/misc/adxl34x.c
@@ -896,15 +896,13 @@ struct adxl34x *adxl34x_probe(struct device *dev, int irq,
 }
 EXPORT_SYMBOL_GPL(adxl34x_probe);
 
-int adxl34x_remove(struct adxl34x *ac)
+void adxl34x_remove(struct adxl34x *ac)
 {
 	sysfs_remove_group(&ac->dev->kobj, &adxl34x_attr_group);
 	free_irq(ac->irq, ac);
 	input_unregister_device(ac->input);
 	dev_dbg(ac->dev, "unregistered accelerometer\n");
 	kfree(ac);
-
-	return 0;
 }
 EXPORT_SYMBOL_GPL(adxl34x_remove);
 
diff --git a/drivers/input/misc/adxl34x.h b/drivers/input/misc/adxl34x.h
index 83a0eeccf613..febf85270fff 100644
--- a/drivers/input/misc/adxl34x.h
+++ b/drivers/input/misc/adxl34x.h
@@ -25,6 +25,6 @@ void adxl34x_resume(struct adxl34x *ac);
 struct adxl34x *adxl34x_probe(struct device *dev, int irq,
 			      bool fifo_delay_default,
 			      const struct adxl34x_bus_ops *bops);
-int adxl34x_remove(struct adxl34x *ac);
+void adxl34x_remove(struct adxl34x *ac);
 
 #endif
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 05/13] input: touchscreen: tsc200x: Make tsc200x_remove() return void
  2021-10-11 13:27 [PATCH 00/13] Make some spi device drivers return zero in .remove() Uwe Kleine-König
                   ` (3 preceding siblings ...)
  2021-10-11 13:27 ` [PATCH 04/13] input: adxl34xx: Make adxl34x_remove() return void Uwe Kleine-König
@ 2021-10-11 13:27 ` Uwe Kleine-König
  2021-10-11 13:27 ` [PATCH 06/13] media: cxd2880: Eliminate dead code Uwe Kleine-König
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 21+ messages in thread
From: Uwe Kleine-König @ 2021-10-11 13:27 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Mark Brown, linux-spi, kernel, linux-input, Wolfram Sang, linux-i2c

Up to now tsc200x_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 i2c and spi remove callbacks is ignored anyway.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 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 +-
 4 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/input/touchscreen/tsc2004.c b/drivers/input/touchscreen/tsc2004.c
index 0272cedcc726..9fdd870c4c0b 100644
--- a/drivers/input/touchscreen/tsc2004.c
+++ b/drivers/input/touchscreen/tsc2004.c
@@ -45,7 +45,9 @@ static int tsc2004_probe(struct i2c_client *i2c,
 
 static int tsc2004_remove(struct i2c_client *i2c)
 {
-	return tsc200x_remove(&i2c->dev);
+	tsc200x_remove(&i2c->dev);
+
+	return 0;
 }
 
 static const struct i2c_device_id tsc2004_idtable[] = {
diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
index 923496bbb368..edaa4afbaab4 100644
--- a/drivers/input/touchscreen/tsc2005.c
+++ b/drivers/input/touchscreen/tsc2005.c
@@ -66,7 +66,9 @@ static int tsc2005_probe(struct spi_device *spi)
 
 static int tsc2005_remove(struct spi_device *spi)
 {
-	return tsc200x_remove(&spi->dev);
+	tsc200x_remove(&spi->dev);
+
+	return 0
 }
 
 #ifdef CONFIG_OF
diff --git a/drivers/input/touchscreen/tsc200x-core.c b/drivers/input/touchscreen/tsc200x-core.c
index b8d720d52013..27810f6c69f6 100644
--- a/drivers/input/touchscreen/tsc200x-core.c
+++ b/drivers/input/touchscreen/tsc200x-core.c
@@ -577,15 +577,13 @@ int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id,
 }
 EXPORT_SYMBOL_GPL(tsc200x_probe);
 
-int tsc200x_remove(struct device *dev)
+void tsc200x_remove(struct device *dev)
 {
 	struct tsc200x *ts = dev_get_drvdata(dev);
 
 	sysfs_remove_group(&dev->kobj, &tsc200x_attr_group);
 
 	regulator_disable(ts->vio);
-
-	return 0;
 }
 EXPORT_SYMBOL_GPL(tsc200x_remove);
 
diff --git a/drivers/input/touchscreen/tsc200x-core.h b/drivers/input/touchscreen/tsc200x-core.h
index a43c08ccfd3d..4ded34425b21 100644
--- a/drivers/input/touchscreen/tsc200x-core.h
+++ b/drivers/input/touchscreen/tsc200x-core.h
@@ -74,6 +74,6 @@ extern const struct dev_pm_ops tsc200x_pm_ops;
 int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id,
 		  struct regmap *regmap,
 		  int (*tsc200x_cmd)(struct device *dev, u8 cmd));
-int tsc200x_remove(struct device *dev);
+void tsc200x_remove(struct device *dev);
 
 #endif
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 06/13] media: cxd2880: Eliminate dead code
  2021-10-11 13:27 [PATCH 00/13] Make some spi device drivers return zero in .remove() Uwe Kleine-König
                   ` (4 preceding siblings ...)
  2021-10-11 13:27 ` [PATCH 05/13] input: touchscreen: tsc200x: Make tsc200x_remove() " Uwe Kleine-König
@ 2021-10-11 13:27 ` Uwe Kleine-König
  2021-10-11 13:27 ` [PATCH 07/13] mfd: mc13xxx: Make mc13xxx_common_exit() return void Uwe Kleine-König
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 21+ messages in thread
From: Uwe Kleine-König @ 2021-10-11 13:27 UTC (permalink / raw)
  To: Yasunari Takiguchi, Mauro Carvalho Chehab
  Cc: Mark Brown, linux-spi, kernel, linux-media

An spi remove callback is never called with an spi_device pointer that
is NULL. Also it is only called for devices that probed successfully. As
cxd2880_spi_probe() always sets driver data, spi_get_drvdata() cannot be
NULL.

Also the return value of 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/media/spi/cxd2880-spi.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/media/spi/cxd2880-spi.c b/drivers/media/spi/cxd2880-spi.c
index b91a1e845b97..67cacf29a61e 100644
--- a/drivers/media/spi/cxd2880-spi.c
+++ b/drivers/media/spi/cxd2880-spi.c
@@ -628,19 +628,8 @@ cxd2880_spi_probe(struct spi_device *spi)
 static int
 cxd2880_spi_remove(struct spi_device *spi)
 {
-	struct cxd2880_dvb_spi *dvb_spi;
+	struct cxd2880_dvb_spi *dvb_spi = spi_get_drvdata(spi);
 
-	if (!spi) {
-		pr_err("invalid arg\n");
-		return -EINVAL;
-	}
-
-	dvb_spi = spi_get_drvdata(spi);
-
-	if (!dvb_spi) {
-		pr_err("failed\n");
-		return -EINVAL;
-	}
 	dvb_spi->demux.dmx.remove_frontend(&dvb_spi->demux.dmx,
 					   &dvb_spi->dmx_fe);
 	dvb_dmxdev_release(&dvb_spi->dmxdev);
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 07/13] mfd: mc13xxx: Make mc13xxx_common_exit() return void
  2021-10-11 13:27 [PATCH 00/13] Make some spi device drivers return zero in .remove() Uwe Kleine-König
                   ` (5 preceding siblings ...)
  2021-10-11 13:27 ` [PATCH 06/13] media: cxd2880: Eliminate dead code Uwe Kleine-König
@ 2021-10-11 13:27 ` Uwe Kleine-König
  2021-10-11 13:27 ` [PATCH 08/13] mfd: stmpe: Make stmpe_remove() " Uwe Kleine-König
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 21+ messages in thread
From: Uwe Kleine-König @ 2021-10-11 13:27 UTC (permalink / raw)
  To: Lee Jones
  Cc: Mark Brown, linux-spi, kernel, linux-kernel, Wolfram Sang, linux-i2c

Up to now mc13xxx_common_exit() 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 and spi remove callbacks is ignored anyway.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/mfd/mc13xxx-core.c | 4 +---
 drivers/mfd/mc13xxx-i2c.c  | 3 ++-
 drivers/mfd/mc13xxx-spi.c  | 3 ++-
 drivers/mfd/mc13xxx.h      | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/mfd/mc13xxx-core.c b/drivers/mfd/mc13xxx-core.c
index 1abe7432aad8..8a4f1d90dcfd 100644
--- a/drivers/mfd/mc13xxx-core.c
+++ b/drivers/mfd/mc13xxx-core.c
@@ -496,15 +496,13 @@ int mc13xxx_common_init(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(mc13xxx_common_init);
 
-int mc13xxx_common_exit(struct device *dev)
+void mc13xxx_common_exit(struct device *dev)
 {
 	struct mc13xxx *mc13xxx = dev_get_drvdata(dev);
 
 	mfd_remove_devices(dev);
 	regmap_del_irq_chip(mc13xxx->irq, mc13xxx->irq_data);
 	mutex_destroy(&mc13xxx->lock);
-
-	return 0;
 }
 EXPORT_SYMBOL_GPL(mc13xxx_common_exit);
 
diff --git a/drivers/mfd/mc13xxx-i2c.c b/drivers/mfd/mc13xxx-i2c.c
index 65b4dd8e5afb..fb937f66277e 100644
--- a/drivers/mfd/mc13xxx-i2c.c
+++ b/drivers/mfd/mc13xxx-i2c.c
@@ -87,7 +87,8 @@ static int mc13xxx_i2c_probe(struct i2c_client *client,
 
 static int mc13xxx_i2c_remove(struct i2c_client *client)
 {
-	return mc13xxx_common_exit(&client->dev);
+	mc13xxx_common_exit(&client->dev);
+	return 0;
 }
 
 static struct i2c_driver mc13xxx_i2c_driver = {
diff --git a/drivers/mfd/mc13xxx-spi.c b/drivers/mfd/mc13xxx-spi.c
index 286ddcf5ddc6..f60227723b4a 100644
--- a/drivers/mfd/mc13xxx-spi.c
+++ b/drivers/mfd/mc13xxx-spi.c
@@ -168,7 +168,8 @@ static int mc13xxx_spi_probe(struct spi_device *spi)
 
 static int mc13xxx_spi_remove(struct spi_device *spi)
 {
-	return mc13xxx_common_exit(&spi->dev);
+	mc13xxx_common_exit(&spi->dev);
+	return 0
 }
 
 static struct spi_driver mc13xxx_spi_driver = {
diff --git a/drivers/mfd/mc13xxx.h b/drivers/mfd/mc13xxx.h
index ce6eec52e8eb..bd5ba9a0e14f 100644
--- a/drivers/mfd/mc13xxx.h
+++ b/drivers/mfd/mc13xxx.h
@@ -44,6 +44,6 @@ struct mc13xxx {
 };
 
 int mc13xxx_common_init(struct device *dev);
-int mc13xxx_common_exit(struct device *dev);
+void mc13xxx_common_exit(struct device *dev);
 
 #endif /* __DRIVERS_MFD_MC13XXX_H */
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 08/13] mfd: stmpe: Make stmpe_remove() return void
  2021-10-11 13:27 [PATCH 00/13] Make some spi device drivers return zero in .remove() Uwe Kleine-König
                   ` (6 preceding siblings ...)
  2021-10-11 13:27 ` [PATCH 07/13] mfd: mc13xxx: Make mc13xxx_common_exit() return void Uwe Kleine-König
@ 2021-10-11 13:27 ` Uwe Kleine-König
  2021-10-11 13:27 ` [PATCH 09/13] mfd: tps65912: Make tps65912_device_exit() " Uwe Kleine-König
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 21+ messages in thread
From: Uwe Kleine-König @ 2021-10-11 13:27 UTC (permalink / raw)
  To: Lee Jones, Maxime Coquelin, Alexandre Torgue
  Cc: Mark Brown, linux-spi, kernel, linux-stm32, linux-kernel,
	Wolfram Sang, linux-i2c

Up to now stmpe_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 i2c and spi remove callbacks is ignored anyway.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/mfd/stmpe-i2c.c | 4 +++-
 drivers/mfd/stmpe-spi.c | 4 +++-
 drivers/mfd/stmpe.c     | 4 +---
 drivers/mfd/stmpe.h     | 2 +-
 4 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/mfd/stmpe-i2c.c b/drivers/mfd/stmpe-i2c.c
index cd2f45257dc1..d3eedf3d607e 100644
--- a/drivers/mfd/stmpe-i2c.c
+++ b/drivers/mfd/stmpe-i2c.c
@@ -95,7 +95,9 @@ static int stmpe_i2c_remove(struct i2c_client *i2c)
 {
 	struct stmpe *stmpe = dev_get_drvdata(&i2c->dev);
 
-	return stmpe_remove(stmpe);
+	stmpe_remove(stmpe);
+
+	return 0;
 }
 
 static const struct i2c_device_id stmpe_i2c_id[] = {
diff --git a/drivers/mfd/stmpe-spi.c b/drivers/mfd/stmpe-spi.c
index 7351734f7593..6c5915016be5 100644
--- a/drivers/mfd/stmpe-spi.c
+++ b/drivers/mfd/stmpe-spi.c
@@ -106,7 +106,9 @@ static int stmpe_spi_remove(struct spi_device *spi)
 {
 	struct stmpe *stmpe = spi_get_drvdata(spi);
 
-	return stmpe_remove(stmpe);
+	stmpe_remove(stmpe);
+
+	return 0;
 }
 
 static const struct of_device_id stmpe_spi_of_match[] = {
diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c
index 58d09c615e67..e928df95e316 100644
--- a/drivers/mfd/stmpe.c
+++ b/drivers/mfd/stmpe.c
@@ -1496,7 +1496,7 @@ int stmpe_probe(struct stmpe_client_info *ci, enum stmpe_partnum partnum)
 	return ret;
 }
 
-int stmpe_remove(struct stmpe *stmpe)
+void stmpe_remove(struct stmpe *stmpe)
 {
 	if (!IS_ERR(stmpe->vio))
 		regulator_disable(stmpe->vio);
@@ -1506,8 +1506,6 @@ int stmpe_remove(struct stmpe *stmpe)
 	__stmpe_disable(stmpe, STMPE_BLOCK_ADC);
 
 	mfd_remove_devices(stmpe->dev);
-
-	return 0;
 }
 
 #ifdef CONFIG_PM
diff --git a/drivers/mfd/stmpe.h b/drivers/mfd/stmpe.h
index 83491e99ba3c..1b4f91d03bbf 100644
--- a/drivers/mfd/stmpe.h
+++ b/drivers/mfd/stmpe.h
@@ -98,7 +98,7 @@ struct stmpe_client_info {
 };
 
 int stmpe_probe(struct stmpe_client_info *ci, enum stmpe_partnum partnum);
-int stmpe_remove(struct stmpe *stmpe);
+void stmpe_remove(struct stmpe *stmpe);
 
 #define STMPE_ICR_LSB_HIGH	(1 << 2)
 #define STMPE_ICR_LSB_EDGE	(1 << 1)
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 09/13] mfd: tps65912: Make tps65912_device_exit() return void
  2021-10-11 13:27 [PATCH 00/13] Make some spi device drivers return zero in .remove() Uwe Kleine-König
                   ` (7 preceding siblings ...)
  2021-10-11 13:27 ` [PATCH 08/13] mfd: stmpe: Make stmpe_remove() " Uwe Kleine-König
@ 2021-10-11 13:27 ` Uwe Kleine-König
  2021-10-11 13:27 ` [PATCH 10/13] serial: max310x: Make max310x_remove() " Uwe Kleine-König
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 21+ messages in thread
From: Uwe Kleine-König @ 2021-10-11 13:27 UTC (permalink / raw)
  To: Lee Jones
  Cc: Mark Brown, linux-spi, kernel, linux-kernel, Wolfram Sang, linux-i2c

Up to now tps65912_device_exit() 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 and spi remove callbacks is ignored anyway.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/mfd/tps65912-core.c  | 4 +---
 drivers/mfd/tps65912-i2c.c   | 4 +++-
 drivers/mfd/tps65912-spi.c   | 4 +++-
 include/linux/mfd/tps65912.h | 2 +-
 4 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/mfd/tps65912-core.c b/drivers/mfd/tps65912-core.c
index b55b1d5d6955..c282a05e7146 100644
--- a/drivers/mfd/tps65912-core.c
+++ b/drivers/mfd/tps65912-core.c
@@ -115,11 +115,9 @@ int tps65912_device_init(struct tps65912 *tps)
 }
 EXPORT_SYMBOL_GPL(tps65912_device_init);
 
-int tps65912_device_exit(struct tps65912 *tps)
+void tps65912_device_exit(struct tps65912 *tps)
 {
 	regmap_del_irq_chip(tps->irq, tps->irq_data);
-
-	return 0;
 }
 EXPORT_SYMBOL_GPL(tps65912_device_exit);
 
diff --git a/drivers/mfd/tps65912-i2c.c b/drivers/mfd/tps65912-i2c.c
index f7c22ea7b36c..06eb2784d322 100644
--- a/drivers/mfd/tps65912-i2c.c
+++ b/drivers/mfd/tps65912-i2c.c
@@ -55,7 +55,9 @@ static int tps65912_i2c_remove(struct i2c_client *client)
 {
 	struct tps65912 *tps = i2c_get_clientdata(client);
 
-	return tps65912_device_exit(tps);
+	tps65912_device_exit(tps);
+
+	return 0;
 }
 
 static const struct i2c_device_id tps65912_i2c_id_table[] = {
diff --git a/drivers/mfd/tps65912-spi.c b/drivers/mfd/tps65912-spi.c
index 21a8d6ac5c4a..d701926aa46e 100644
--- a/drivers/mfd/tps65912-spi.c
+++ b/drivers/mfd/tps65912-spi.c
@@ -54,7 +54,9 @@ static int tps65912_spi_remove(struct spi_device *spi)
 {
 	struct tps65912 *tps = spi_get_drvdata(spi);
 
-	return tps65912_device_exit(tps);
+	tps65912_device_exit(tps);
+
+	return 0;
 }
 
 static const struct spi_device_id tps65912_spi_id_table[] = {
diff --git a/include/linux/mfd/tps65912.h b/include/linux/mfd/tps65912.h
index 7943e413deae..8a61386cb8c1 100644
--- a/include/linux/mfd/tps65912.h
+++ b/include/linux/mfd/tps65912.h
@@ -322,6 +322,6 @@ struct tps65912 {
 extern const struct regmap_config tps65912_regmap_config;
 
 int tps65912_device_init(struct tps65912 *tps);
-int tps65912_device_exit(struct tps65912 *tps);
+void tps65912_device_exit(struct tps65912 *tps);
 
 #endif /*  __LINUX_MFD_TPS65912_H */
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 10/13] serial: max310x: Make max310x_remove() return void
  2021-10-11 13:27 [PATCH 00/13] Make some spi device drivers return zero in .remove() Uwe Kleine-König
                   ` (8 preceding siblings ...)
  2021-10-11 13:27 ` [PATCH 09/13] mfd: tps65912: Make tps65912_device_exit() " Uwe Kleine-König
@ 2021-10-11 13:27 ` Uwe Kleine-König
  2021-10-11 13:51   ` Greg Kroah-Hartman
  2021-10-11 13:27 ` [PATCH 11/13] serial: sc16is7xx: Make sc16is7xx_remove() " Uwe Kleine-König
                   ` (3 subsequent siblings)
  13 siblings, 1 reply; 21+ messages in thread
From: Uwe Kleine-König @ 2021-10-11 13:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: Mark Brown, linux-spi, kernel, linux-serial

Up to now max310x_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/tty/serial/max310x.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index 3df0788ddeb0..6816eeb1ffef 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -1426,7 +1426,7 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
 	return ret;
 }
 
-static int max310x_remove(struct device *dev)
+static void max310x_remove(struct device *dev)
 {
 	struct max310x_port *s = dev_get_drvdata(dev);
 	int i;
@@ -1441,8 +1441,6 @@ static int max310x_remove(struct device *dev)
 	}
 
 	clk_disable_unprepare(s->clk);
-
-	return 0;
 }
 
 static const struct of_device_id __maybe_unused max310x_dt_ids[] = {
@@ -1491,7 +1489,8 @@ static int max310x_spi_probe(struct spi_device *spi)
 
 static int max310x_spi_remove(struct spi_device *spi)
 {
-	return max310x_remove(&spi->dev);
+	max310x_remove(&spi->dev);
+	return 0
 }
 
 static const struct spi_device_id max310x_id_table[] = {
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 11/13] serial: sc16is7xx: Make sc16is7xx_remove() return void
  2021-10-11 13:27 [PATCH 00/13] Make some spi device drivers return zero in .remove() Uwe Kleine-König
                   ` (9 preceding siblings ...)
  2021-10-11 13:27 ` [PATCH 10/13] serial: max310x: Make max310x_remove() " Uwe Kleine-König
@ 2021-10-11 13:27 ` Uwe Kleine-König
  2021-10-11 13:27 ` [PATCH 12/13] staging: fbtft: Make fbtft_remove_common() " Uwe Kleine-König
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 21+ messages in thread
From: Uwe Kleine-König @ 2021-10-11 13:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: Mark Brown, linux-spi, kernel, linux-serial

Up to now sc16is7xx_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/tty/serial/sc16is7xx.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index acbb615dd28f..f8cbe451107b 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -1365,7 +1365,7 @@ static int sc16is7xx_probe(struct device *dev,
 	return ret;
 }
 
-static int sc16is7xx_remove(struct device *dev)
+static void sc16is7xx_remove(struct device *dev)
 {
 	struct sc16is7xx_port *s = dev_get_drvdata(dev);
 	int i;
@@ -1444,7 +1444,9 @@ static int sc16is7xx_spi_probe(struct spi_device *spi)
 
 static int sc16is7xx_spi_remove(struct spi_device *spi)
 {
-	return sc16is7xx_remove(&spi->dev);
+	sc16is7xx_remove(&spi->dev);
+
+	return 0;
 }
 
 static const struct spi_device_id sc16is7xx_spi_id_table[] = {
@@ -1497,7 +1499,9 @@ static int sc16is7xx_i2c_probe(struct i2c_client *i2c,
 
 static int sc16is7xx_i2c_remove(struct i2c_client *client)
 {
-	return sc16is7xx_remove(&client->dev);
+	sc16is7xx_remove(&client->dev);
+
+	return 0;
 }
 
 static const struct i2c_device_id sc16is7xx_i2c_id_table[] = {
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 12/13] staging: fbtft: Make fbtft_remove_common() return void
  2021-10-11 13:27 [PATCH 00/13] Make some spi device drivers return zero in .remove() Uwe Kleine-König
                   ` (10 preceding siblings ...)
  2021-10-11 13:27 ` [PATCH 11/13] serial: sc16is7xx: Make sc16is7xx_remove() " Uwe Kleine-König
@ 2021-10-11 13:27 ` Uwe Kleine-König
  2021-10-11 13:27 ` [PATCH 13/13] tpm: st33zp24: Make st33zp24_remove() " Uwe Kleine-König
  2021-10-11 20:42 ` [PATCH 00/13] Make some spi device drivers return zero in .remove() Uwe Kleine-König
  13 siblings, 0 replies; 21+ messages in thread
From: Uwe Kleine-König @ 2021-10-11 13:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Mark Brown, linux-spi, kernel, dri-devel, linux-staging,
	Rafael J. Wysocki

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[] = {                              \
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 13/13] tpm: st33zp24: Make st33zp24_remove() return void
  2021-10-11 13:27 [PATCH 00/13] Make some spi device drivers return zero in .remove() Uwe Kleine-König
                   ` (11 preceding siblings ...)
  2021-10-11 13:27 ` [PATCH 12/13] staging: fbtft: Make fbtft_remove_common() " Uwe Kleine-König
@ 2021-10-11 13:27 ` Uwe Kleine-König
  2021-10-12 16:47   ` Jarkko Sakkinen
  2021-10-11 20:42 ` [PATCH 00/13] Make some spi device drivers return zero in .remove() Uwe Kleine-König
  13 siblings, 1 reply; 21+ messages in thread
From: Uwe Kleine-König @ 2021-10-11 13:27 UTC (permalink / raw)
  To: Peter Huewe, Jarkko Sakkinen
  Cc: Mark Brown, linux-spi, Jason Gunthorpe, kernel, linux-integrity,
	Wolfram Sang, linux-i2c

Up to now st33zp24_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 i2c and spi remove callbacks is ignored anyway.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 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 +-
 4 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/char/tpm/st33zp24/i2c.c b/drivers/char/tpm/st33zp24/i2c.c
index 7c617edff4ca..3170d59d660c 100644
--- a/drivers/char/tpm/st33zp24/i2c.c
+++ b/drivers/char/tpm/st33zp24/i2c.c
@@ -267,11 +267,8 @@ static int st33zp24_i2c_probe(struct i2c_client *client,
 static int st33zp24_i2c_remove(struct i2c_client *client)
 {
 	struct tpm_chip *chip = i2c_get_clientdata(client);
-	int ret;
 
-	ret = st33zp24_remove(chip);
-	if (ret)
-		return ret;
+	st33zp24_remove(chip);
 
 	return 0;
 }
diff --git a/drivers/char/tpm/st33zp24/spi.c b/drivers/char/tpm/st33zp24/spi.c
index a75dafd39445..ccd9e42b8eab 100644
--- a/drivers/char/tpm/st33zp24/spi.c
+++ b/drivers/char/tpm/st33zp24/spi.c
@@ -384,11 +384,8 @@ static int st33zp24_spi_probe(struct spi_device *dev)
 static int st33zp24_spi_remove(struct spi_device *dev)
 {
 	struct tpm_chip *chip = spi_get_drvdata(dev);
-	int ret;
 
-	ret = st33zp24_remove(chip);
-	if (ret)
-		return ret;
+	st33zp24_remove(chip);
 
 	return 0;
 }
diff --git a/drivers/char/tpm/st33zp24/st33zp24.c b/drivers/char/tpm/st33zp24/st33zp24.c
index 4ec10ab5e576..2b63654c38d6 100644
--- a/drivers/char/tpm/st33zp24/st33zp24.c
+++ b/drivers/char/tpm/st33zp24/st33zp24.c
@@ -588,10 +588,9 @@ EXPORT_SYMBOL(st33zp24_probe);
  * @param: tpm_data, the tpm phy.
  * @return: 0 in case of success.
  */
-int st33zp24_remove(struct tpm_chip *chip)
+void st33zp24_remove(struct tpm_chip *chip)
 {
 	tpm_chip_unregister(chip);
-	return 0;
 }
 EXPORT_SYMBOL(st33zp24_remove);
 
diff --git a/drivers/char/tpm/st33zp24/st33zp24.h b/drivers/char/tpm/st33zp24/st33zp24.h
index 6747be1e2502..b387a476c555 100644
--- a/drivers/char/tpm/st33zp24/st33zp24.h
+++ b/drivers/char/tpm/st33zp24/st33zp24.h
@@ -34,5 +34,5 @@ int st33zp24_pm_resume(struct device *dev);
 
 int st33zp24_probe(void *phy_id, const struct st33zp24_phy_ops *ops,
 		   struct device *dev, int irq, int io_lpcpd);
-int st33zp24_remove(struct tpm_chip *chip);
+void st33zp24_remove(struct tpm_chip *chip);
 #endif /* __LOCAL_ST33ZP24_H__ */
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* RE: [PATCH 03/13] hwmon: max31722: Warn about failure to put device in stand-by in .remove()
  2021-10-11 13:27 ` [PATCH 03/13] hwmon: max31722: Warn about failure to put device in stand-by in .remove() Uwe Kleine-König
@ 2021-10-11 13:41   ` Hennerich, Michael
  0 siblings, 0 replies; 21+ messages in thread
From: Hennerich, Michael @ 2021-10-11 13:41 UTC (permalink / raw)
  To: Uwe Kleine-König, Dmitry Torokhov
  Cc: Mark Brown, linux-spi, kernel, linux-input



> -----Original Message-----
> From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Sent: Montag, 11. Oktober 2021 15:28
> To: Dmitry Torokhov <dmitry.torokhov@gmail.com>; Hennerich, Michael
> <Michael.Hennerich@analog.com>
> Cc: Mark Brown <broonie@kernel.org>; linux-spi@vger.kernel.org;
> kernel@pengutronix.de; linux-input@vger.kernel.org
> Subject: [PATCH 03/13] hwmon: max31722: Warn about failure to put device in
> stand-by in .remove()
> 
> When an spi driver's remove function returns a non-zero error code nothing
> happens apart from emitting a generic error message. Make this error message
> more device specific and return zero instead.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Acked-by: Michael Hennerich <michael.hennerich@analog.com>

> ---
>  drivers/hwmon/max31722.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/max31722.c b/drivers/hwmon/max31722.c index
> 613338cbcb17..4cf4fe6809a3 100644
> --- a/drivers/hwmon/max31722.c
> +++ b/drivers/hwmon/max31722.c
> @@ -103,10 +103,16 @@ static int max31722_probe(struct spi_device *spi)
> static int max31722_remove(struct spi_device *spi)  {
>  	struct max31722_data *data = spi_get_drvdata(spi);
> +	int ret;
> 
>  	hwmon_device_unregister(data->hwmon_dev);
> 
> -	return max31722_set_mode(data, MAX31722_MODE_STANDBY);
> +	ret = max31722_set_mode(data, MAX31722_MODE_STANDBY);
> +	if (ret)
> +		/* There is nothing we can do about this ... */
> +		dev_warn(&spi->dev, "Failed to put device in stand-by
> mode\n");
> +
> +	return 0;
>  }
> 
>  static int __maybe_unused max31722_suspend(struct device *dev)
> --
> 2.30.2


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 10/13] serial: max310x: Make max310x_remove() return void
  2021-10-11 13:27 ` [PATCH 10/13] serial: max310x: Make max310x_remove() " Uwe Kleine-König
@ 2021-10-11 13:51   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 21+ messages in thread
From: Greg Kroah-Hartman @ 2021-10-11 13:51 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Jiri Slaby, Mark Brown, linux-spi, kernel, linux-serial

On Mon, Oct 11, 2021 at 03:27:51PM +0200, Uwe Kleine-König wrote:
> Up to now max310x_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/tty/serial/max310x.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
> index 3df0788ddeb0..6816eeb1ffef 100644
> --- a/drivers/tty/serial/max310x.c
> +++ b/drivers/tty/serial/max310x.c
> @@ -1426,7 +1426,7 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
>  	return ret;
>  }
>  
> -static int max310x_remove(struct device *dev)
> +static void max310x_remove(struct device *dev)
>  {
>  	struct max310x_port *s = dev_get_drvdata(dev);
>  	int i;
> @@ -1441,8 +1441,6 @@ static int max310x_remove(struct device *dev)
>  	}
>  
>  	clk_disable_unprepare(s->clk);
> -
> -	return 0;
>  }
>  
>  static const struct of_device_id __maybe_unused max310x_dt_ids[] = {
> @@ -1491,7 +1489,8 @@ static int max310x_spi_probe(struct spi_device *spi)
>  
>  static int max310x_spi_remove(struct spi_device *spi)
>  {
> -	return max310x_remove(&spi->dev);
> +	max310x_remove(&spi->dev);
> +	return 0

Does this compile?


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 02/13] hwmon: adt7x10: Make adt7x10_remove() return void
  2021-10-11 13:27 ` [PATCH 02/13] hwmon: adt7x10: Make adt7x10_remove() " Uwe Kleine-König
@ 2021-10-11 14:19   ` Guenter Roeck
  0 siblings, 0 replies; 21+ messages in thread
From: Guenter Roeck @ 2021-10-11 14:19 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Jean Delvare, Mark Brown, linux-spi, kernel, linux-hwmon,
	Wolfram Sang, linux-i2c

On Mon, Oct 11, 2021 at 03:27:43PM +0200, Uwe Kleine-König wrote:
> Up to now adt7x10_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 i2c and spi remove callbacks is ignored anyway.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Applied.

Thanks,
Guenter

> ---
>  drivers/hwmon/adt7310.c | 3 ++-
>  drivers/hwmon/adt7410.c | 3 ++-
>  drivers/hwmon/adt7x10.c | 3 +--
>  drivers/hwmon/adt7x10.h | 2 +-
>  4 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/hwmon/adt7310.c b/drivers/hwmon/adt7310.c
> index 9fad01191620..c40cac16af68 100644
> --- a/drivers/hwmon/adt7310.c
> +++ b/drivers/hwmon/adt7310.c
> @@ -90,7 +90,8 @@ static int adt7310_spi_probe(struct spi_device *spi)
>  
>  static int adt7310_spi_remove(struct spi_device *spi)
>  {
> -	return adt7x10_remove(&spi->dev, spi->irq);
> +	adt7x10_remove(&spi->dev, spi->irq);
> +	return 0;
>  }
>  
>  static const struct spi_device_id adt7310_id[] = {
> diff --git a/drivers/hwmon/adt7410.c b/drivers/hwmon/adt7410.c
> index 9d80895d0266..973db057427b 100644
> --- a/drivers/hwmon/adt7410.c
> +++ b/drivers/hwmon/adt7410.c
> @@ -50,7 +50,8 @@ static int adt7410_i2c_probe(struct i2c_client *client)
>  
>  static int adt7410_i2c_remove(struct i2c_client *client)
>  {
> -	return adt7x10_remove(&client->dev, client->irq);
> +	adt7x10_remove(&client->dev, client->irq);
> +	return 0;
>  }
>  
>  static const struct i2c_device_id adt7410_ids[] = {
> diff --git a/drivers/hwmon/adt7x10.c b/drivers/hwmon/adt7x10.c
> index 3f03b4cf5858..e9d33aa78a19 100644
> --- a/drivers/hwmon/adt7x10.c
> +++ b/drivers/hwmon/adt7x10.c
> @@ -444,7 +444,7 @@ int adt7x10_probe(struct device *dev, const char *name, int irq,
>  }
>  EXPORT_SYMBOL_GPL(adt7x10_probe);
>  
> -int adt7x10_remove(struct device *dev, int irq)
> +void adt7x10_remove(struct device *dev, int irq)
>  {
>  	struct adt7x10_data *data = dev_get_drvdata(dev);
>  
> @@ -457,7 +457,6 @@ int adt7x10_remove(struct device *dev, int irq)
>  	sysfs_remove_group(&dev->kobj, &adt7x10_group);
>  	if (data->oldconfig != data->config)
>  		adt7x10_write_byte(dev, ADT7X10_CONFIG, data->oldconfig);
> -	return 0;
>  }
>  EXPORT_SYMBOL_GPL(adt7x10_remove);
>  
> diff --git a/drivers/hwmon/adt7x10.h b/drivers/hwmon/adt7x10.h
> index 21ad15ce3163..a1ae682eb32e 100644
> --- a/drivers/hwmon/adt7x10.h
> +++ b/drivers/hwmon/adt7x10.h
> @@ -26,7 +26,7 @@ struct adt7x10_ops {
>  
>  int adt7x10_probe(struct device *dev, const char *name, int irq,
>  	const struct adt7x10_ops *ops);
> -int adt7x10_remove(struct device *dev, int irq);
> +void adt7x10_remove(struct device *dev, int irq);
>  
>  #ifdef CONFIG_PM_SLEEP
>  extern const struct dev_pm_ops adt7x10_dev_pm_ops;

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 00/13] Make some spi device drivers return zero in .remove()
  2021-10-11 13:27 [PATCH 00/13] Make some spi device drivers return zero in .remove() Uwe Kleine-König
                   ` (12 preceding siblings ...)
  2021-10-11 13:27 ` [PATCH 13/13] tpm: st33zp24: Make st33zp24_remove() " Uwe Kleine-König
@ 2021-10-11 20:42 ` Uwe Kleine-König
  13 siblings, 0 replies; 21+ messages in thread
From: Uwe Kleine-König @ 2021-10-11 20:42 UTC (permalink / raw)
  To: Alexandre Torgue, Dmitry Torokhov, Greg Kroah-Hartman,
	Guenter Roeck, Jarkko Sakkinen, Jean Delvare, Jiri Slaby,
	Lee Jones, Mauro Carvalho Chehab, Maxime Coquelin,
	Michael Hennerich, Peter Huewe, Thierry Reding,
	Yasunari Takiguchi
  Cc: linux-hwmon, linux-serial, Rafael J. Wysocki, linux-staging,
	linux-kernel, dri-devel, linux-spi, Wolfram Sang,
	Jason Gunthorpe linux-integrity @ vger . kernel . org,
	Mark Brown, linux-i2c, kernel, linux-input, Sam Ravnborg,
	linux-stm32, linux-media

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

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

-- 
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 --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 13/13] tpm: st33zp24: Make st33zp24_remove() return void
  2021-10-11 13:27 ` [PATCH 13/13] tpm: st33zp24: Make st33zp24_remove() " Uwe Kleine-König
@ 2021-10-12 16:47   ` Jarkko Sakkinen
  2021-10-12 17:40     ` Uwe Kleine-König
  0 siblings, 1 reply; 21+ messages in thread
From: Jarkko Sakkinen @ 2021-10-12 16:47 UTC (permalink / raw)
  To: Uwe Kleine-König, Peter Huewe
  Cc: Mark Brown, linux-spi, Jason Gunthorpe, kernel, linux-integrity,
	Wolfram Sang, linux-i2c

On Mon, 2021-10-11 at 15:27 +0200, Uwe Kleine-König wrote:
> Up to now st33zp24_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.

So, void is not a return value.


> 
> Also the return value of i2c and spi remove callbacks is ignored anyway.
                           ~~~     ~~~
			   I2C     SPI

> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  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 +-
>  4 files changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/char/tpm/st33zp24/i2c.c b/drivers/char/tpm/st33zp24/i2c.c
> index 7c617edff4ca..3170d59d660c 100644
> --- a/drivers/char/tpm/st33zp24/i2c.c
> +++ b/drivers/char/tpm/st33zp24/i2c.c
> @@ -267,11 +267,8 @@ static int st33zp24_i2c_probe(struct i2c_client *client,
>  static int st33zp24_i2c_remove(struct i2c_client *client)
>  {
>         struct tpm_chip *chip = i2c_get_clientdata(client);
> -       int ret;
>  
> -       ret = st33zp24_remove(chip);
> -       if (ret)
> -               return ret;
> +       st33zp24_remove(chip);
>  
>         return 0;
>  }
> diff --git a/drivers/char/tpm/st33zp24/spi.c b/drivers/char/tpm/st33zp24/spi.c
> index a75dafd39445..ccd9e42b8eab 100644
> --- a/drivers/char/tpm/st33zp24/spi.c
> +++ b/drivers/char/tpm/st33zp24/spi.c
> @@ -384,11 +384,8 @@ static int st33zp24_spi_probe(struct spi_device *dev)
>  static int st33zp24_spi_remove(struct spi_device *dev)
>  {
>         struct tpm_chip *chip = spi_get_drvdata(dev);
> -       int ret;
>  
> -       ret = st33zp24_remove(chip);
> -       if (ret)
> -               return ret;
> +       st33zp24_remove(chip);
>  
>         return 0;
>  }
> diff --git a/drivers/char/tpm/st33zp24/st33zp24.c b/drivers/char/tpm/st33zp24/st33zp24.c
> index 4ec10ab5e576..2b63654c38d6 100644
> --- a/drivers/char/tpm/st33zp24/st33zp24.c
> +++ b/drivers/char/tpm/st33zp24/st33zp24.c
> @@ -588,10 +588,9 @@ EXPORT_SYMBOL(st33zp24_probe);
>   * @param: tpm_data, the tpm phy.
>   * @return: 0 in case of success.
>   */
> -int st33zp24_remove(struct tpm_chip *chip)
> +void st33zp24_remove(struct tpm_chip *chip)
>  {
>         tpm_chip_unregister(chip);
> -       return 0;
>  }
>  EXPORT_SYMBOL(st33zp24_remove);
>  
> diff --git a/drivers/char/tpm/st33zp24/st33zp24.h b/drivers/char/tpm/st33zp24/st33zp24.h
> index 6747be1e2502..b387a476c555 100644
> --- a/drivers/char/tpm/st33zp24/st33zp24.h
> +++ b/drivers/char/tpm/st33zp24/st33zp24.h
> @@ -34,5 +34,5 @@ int st33zp24_pm_resume(struct device *dev);
>  
>  int st33zp24_probe(void *phy_id, const struct st33zp24_phy_ops *ops,
>                    struct device *dev, int irq, int io_lpcpd);
> -int st33zp24_remove(struct tpm_chip *chip);
> +void st33zp24_remove(struct tpm_chip *chip);
>  #endif /* __LOCAL_ST33ZP24_H__ */

Even though this does not improve things at run-time, this does
help to understand what the code does, so in that sense I do
think that this a sane change to make.

With an appropriate commit message, this can be applied.

/Jarkko


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 13/13] tpm: st33zp24: Make st33zp24_remove() return void
  2021-10-12 16:47   ` Jarkko Sakkinen
@ 2021-10-12 17:40     ` Uwe Kleine-König
  2021-10-12 17:46       ` Jarkko Sakkinen
  0 siblings, 1 reply; 21+ messages in thread
From: Uwe Kleine-König @ 2021-10-12 17:40 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Peter Huewe, linux-spi, Wolfram Sang, Jason Gunthorpe,
	Mark Brown, linux-i2c, kernel, linux-integrity

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

On Tue, Oct 12, 2021 at 07:47:22PM +0300, Jarkko Sakkinen wrote:
> On Mon, 2021-10-11 at 15:27 +0200, Uwe Kleine-König wrote:
> > Up to now st33zp24_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.
> 
> So, void is not a return value.

Hmm, would you be more happy with "Make it return no value"? I think
this is less understandable than "Make it return void". Do you have a
constructive suggestion how to formulate.

> > Also the return value of i2c and spi remove callbacks is ignored anyway.
>                            ~~~     ~~~
> 			   I2C     SPI

I don't agree. "I2C" is fine if you mean the protocol. For the framework
names I consider the small letters better, as it matches the directory
name below drivers/ and also matches the function name prefix and what
is usually used for short log prefixes. Ditto for spi.

> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> >  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 +-
> >  4 files changed, 4 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/char/tpm/st33zp24/i2c.c b/drivers/char/tpm/st33zp24/i2c.c
> > index 7c617edff4ca..3170d59d660c 100644
> > --- a/drivers/char/tpm/st33zp24/i2c.c
> > +++ b/drivers/char/tpm/st33zp24/i2c.c
> > @@ -267,11 +267,8 @@ static int st33zp24_i2c_probe(struct i2c_client *client,
> >  static int st33zp24_i2c_remove(struct i2c_client *client)
> >  {
> >         struct tpm_chip *chip = i2c_get_clientdata(client);
> > -       int ret;
> >  
> > -       ret = st33zp24_remove(chip);
> > -       if (ret)
> > -               return ret;
> > +       st33zp24_remove(chip);
> >  
> >         return 0;
> >  }
> > diff --git a/drivers/char/tpm/st33zp24/spi.c b/drivers/char/tpm/st33zp24/spi.c
> > index a75dafd39445..ccd9e42b8eab 100644
> > --- a/drivers/char/tpm/st33zp24/spi.c
> > +++ b/drivers/char/tpm/st33zp24/spi.c
> > @@ -384,11 +384,8 @@ static int st33zp24_spi_probe(struct spi_device *dev)
> >  static int st33zp24_spi_remove(struct spi_device *dev)
> >  {
> >         struct tpm_chip *chip = spi_get_drvdata(dev);
> > -       int ret;
> >  
> > -       ret = st33zp24_remove(chip);
> > -       if (ret)
> > -               return ret;
> > +       st33zp24_remove(chip);
> >  
> >         return 0;
> >  }
> > diff --git a/drivers/char/tpm/st33zp24/st33zp24.c b/drivers/char/tpm/st33zp24/st33zp24.c
> > index 4ec10ab5e576..2b63654c38d6 100644
> > --- a/drivers/char/tpm/st33zp24/st33zp24.c
> > +++ b/drivers/char/tpm/st33zp24/st33zp24.c
> > @@ -588,10 +588,9 @@ EXPORT_SYMBOL(st33zp24_probe);
> >   * @param: tpm_data, the tpm phy.
> >   * @return: 0 in case of success.
> >   */
> > -int st33zp24_remove(struct tpm_chip *chip)
> > +void st33zp24_remove(struct tpm_chip *chip)
> >  {
> >         tpm_chip_unregister(chip);
> > -       return 0;
> >  }
> >  EXPORT_SYMBOL(st33zp24_remove);
> >  
> > diff --git a/drivers/char/tpm/st33zp24/st33zp24.h b/drivers/char/tpm/st33zp24/st33zp24.h
> > index 6747be1e2502..b387a476c555 100644
> > --- a/drivers/char/tpm/st33zp24/st33zp24.h
> > +++ b/drivers/char/tpm/st33zp24/st33zp24.h
> > @@ -34,5 +34,5 @@ int st33zp24_pm_resume(struct device *dev);
> >  
> >  int st33zp24_probe(void *phy_id, const struct st33zp24_phy_ops *ops,
> >                    struct device *dev, int irq, int io_lpcpd);
> > -int st33zp24_remove(struct tpm_chip *chip);
> > +void st33zp24_remove(struct tpm_chip *chip);
> >  #endif /* __LOCAL_ST33ZP24_H__ */
> 
> Even though this does not improve things at run-time, this does
> help to understand what the code does,

That is for focus for now. As written in the cover letter the long term
goal is to make struct spi_driver::remove return void, too.

> so in that sense I do
> think that this a sane change to make.

Thanks
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 --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 13/13] tpm: st33zp24: Make st33zp24_remove() return void
  2021-10-12 17:40     ` Uwe Kleine-König
@ 2021-10-12 17:46       ` Jarkko Sakkinen
  0 siblings, 0 replies; 21+ messages in thread
From: Jarkko Sakkinen @ 2021-10-12 17:46 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Peter Huewe, linux-spi, Wolfram Sang, Jason Gunthorpe,
	Mark Brown, linux-i2c, kernel, linux-integrity

On Tue, 2021-10-12 at 19:40 +0200, Uwe Kleine-König wrote:
> On Tue, Oct 12, 2021 at 07:47:22PM +0300, Jarkko Sakkinen wrote:
> > On Mon, 2021-10-11 at 15:27 +0200, Uwe Kleine-König wrote:
> > > Up to now st33zp24_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.
> > 
> > So, void is not a return value.
> 
> Hmm, would you be more happy with "Make it return no value"? I think
> this is less understandable than "Make it return void". Do you have a
> constructive suggestion how to formulate.

I think it is semantically more correct to say that it does not return
any value, given that it does not do that :-) You can just state e.g.
"Make st33zp24_remove() a void function.", it is semantically sound and
not really that confusing.

/Jarkko


^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2021-10-12 17:46 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-11 13:27 [PATCH 00/13] Make some spi device drivers return zero in .remove() Uwe Kleine-König
2021-10-11 13:27 ` [PATCH 01/13] drm/panel: s6e63m0: Make s6e63m0_remove() return void Uwe Kleine-König
2021-10-11 13:27 ` [PATCH 02/13] hwmon: adt7x10: Make adt7x10_remove() " Uwe Kleine-König
2021-10-11 14:19   ` Guenter Roeck
2021-10-11 13:27 ` [PATCH 03/13] hwmon: max31722: Warn about failure to put device in stand-by in .remove() Uwe Kleine-König
2021-10-11 13:41   ` Hennerich, Michael
2021-10-11 13:27 ` [PATCH 04/13] input: adxl34xx: Make adxl34x_remove() return void Uwe Kleine-König
2021-10-11 13:27 ` [PATCH 05/13] input: touchscreen: tsc200x: Make tsc200x_remove() " Uwe Kleine-König
2021-10-11 13:27 ` [PATCH 06/13] media: cxd2880: Eliminate dead code Uwe Kleine-König
2021-10-11 13:27 ` [PATCH 07/13] mfd: mc13xxx: Make mc13xxx_common_exit() return void Uwe Kleine-König
2021-10-11 13:27 ` [PATCH 08/13] mfd: stmpe: Make stmpe_remove() " Uwe Kleine-König
2021-10-11 13:27 ` [PATCH 09/13] mfd: tps65912: Make tps65912_device_exit() " Uwe Kleine-König
2021-10-11 13:27 ` [PATCH 10/13] serial: max310x: Make max310x_remove() " Uwe Kleine-König
2021-10-11 13:51   ` Greg Kroah-Hartman
2021-10-11 13:27 ` [PATCH 11/13] serial: sc16is7xx: Make sc16is7xx_remove() " Uwe Kleine-König
2021-10-11 13:27 ` [PATCH 12/13] staging: fbtft: Make fbtft_remove_common() " Uwe Kleine-König
2021-10-11 13:27 ` [PATCH 13/13] tpm: st33zp24: Make st33zp24_remove() " Uwe Kleine-König
2021-10-12 16:47   ` Jarkko Sakkinen
2021-10-12 17:40     ` Uwe Kleine-König
2021-10-12 17:46       ` Jarkko Sakkinen
2021-10-11 20:42 ` [PATCH 00/13] Make some spi device drivers return zero in .remove() Uwe Kleine-König

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).