diff mbox series

[v2] spi: bcm2835: Convert to platform remove callback returning void

Message ID 20230330211022.2460233-1-u.kleine-koenig@pengutronix.de
State Accepted
Commit 497667ab9016ba508bb0591b6e9d15597127b034
Headers show
Series [v2] spi: bcm2835: Convert to platform remove callback returning void | expand

Commit Message

Uwe Kleine-König March 30, 2023, 9:10 p.m. UTC
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.

Convert this driver from always returning zero in the remove callback to
the void returning variant.

Now that bcm2835_spi_remove returns no error code any more,
bcm2835_spi_shutdown() does the same thing as bcm2835_spi_remove(). So
drop the shutdown function and use bcm2835_spi_remove() as .shutdown
callback.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,

(implicit) v1 of this patch was sent in the big conversion series for
(nearly) all spi drivers at https://lore.kernel.org/linux-spi/20230303172041.2103336-1-u.kleine-koenig@pengutronix.de

Unfortunately the patch sent there broke compilation. So here is a v2
that is compile tested without uncommitted changes in the tree.

Best regards
Uwe

 drivers/spi/spi-bcm2835.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

Comments

Florian Fainelli March 30, 2023, 11:22 p.m. UTC | #1
On 3/30/23 14:10, Uwe Kleine-König wrote:
> The .remove() callback for a platform driver returns an int which makes
> many driver authors wrongly assume it's possible to do error handling by
> returning an error code. However the value returned is (mostly) ignored
> and this typically results in resource leaks. To improve here there is a
> quest to make the remove callback return void. In the first step of this
> quest all drivers are converted to .remove_new() which already returns
> void.
> 
> Convert this driver from always returning zero in the remove callback to
> the void returning variant.
> 
> Now that bcm2835_spi_remove returns no error code any more,
> bcm2835_spi_shutdown() does the same thing as bcm2835_spi_remove(). So
> drop the shutdown function and use bcm2835_spi_remove() as .shutdown
> callback.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
Mark Brown March 31, 2023, 3:08 p.m. UTC | #2
On Thu, 30 Mar 2023 23:10:22 +0200, Uwe Kleine-König wrote:
> The .remove() callback for a platform driver returns an int which makes
> many driver authors wrongly assume it's possible to do error handling by
> returning an error code. However the value returned is (mostly) ignored
> and this typically results in resource leaks. To improve here there is a
> quest to make the remove callback return void. In the first step of this
> quest all drivers are converted to .remove_new() which already returns
> void.
> 
> [...]

Applied to

   broonie/spi.git for-next

Thanks!

[1/1] spi: bcm2835: Convert to platform remove callback returning void
      commit: 497667ab9016ba508bb0591b6e9d15597127b034

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark
diff mbox series

Patch

diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c
index 747e03228c48..215b4a02166d 100644
--- a/drivers/spi/spi-bcm2835.c
+++ b/drivers/spi/spi-bcm2835.c
@@ -1398,7 +1398,7 @@  static int bcm2835_spi_probe(struct platform_device *pdev)
 	return err;
 }
 
-static int bcm2835_spi_remove(struct platform_device *pdev)
+static void bcm2835_spi_remove(struct platform_device *pdev)
 {
 	struct spi_controller *ctlr = platform_get_drvdata(pdev);
 	struct bcm2835_spi *bs = spi_controller_get_devdata(ctlr);
@@ -1414,17 +1414,6 @@  static int bcm2835_spi_remove(struct platform_device *pdev)
 		   BCM2835_SPI_CS_CLEAR_RX | BCM2835_SPI_CS_CLEAR_TX);
 
 	clk_disable_unprepare(bs->clk);
-
-	return 0;
-}
-
-static void bcm2835_spi_shutdown(struct platform_device *pdev)
-{
-	int ret;
-
-	ret = bcm2835_spi_remove(pdev);
-	if (ret)
-		dev_err(&pdev->dev, "failed to shutdown\n");
 }
 
 static const struct of_device_id bcm2835_spi_match[] = {
@@ -1439,8 +1428,8 @@  static struct platform_driver bcm2835_spi_driver = {
 		.of_match_table	= bcm2835_spi_match,
 	},
 	.probe		= bcm2835_spi_probe,
-	.remove		= bcm2835_spi_remove,
-	.shutdown	= bcm2835_spi_shutdown,
+	.remove_new	= bcm2835_spi_remove,
+	.shutdown	= bcm2835_spi_remove,
 };
 module_platform_driver(bcm2835_spi_driver);