[PATCH 0/5] clocksource: Convert to platform remove callback returning void

[PATCH 0/5] clocksource: Convert to platform remove callback returning void

[Uwe Kleine-König]

Hello,

this patch series adapts the platform drivers below drivers/clk
to use the .remove_new() callback. Compared to the traditional .remove()
callback .remove_new() returns no value. This is a good thing because
the driver core doesn't (and cannot) cope for errors during remove. The
only effect of a non-zero return value in .remove() is that the driver
core emits a warning. The device is removed anyhow and an early return
from .remove() usually yields a resource leak.

Most clocksource drivers are not supposed to be removed. Two drivers are
adapted here to actually prevent removal. One driver is fixed not to
return an error code in .remove() and then the two remaining drivers
with a remove callback are trivially converted to .remove_new().

Best regards
Uwe

Uwe Kleine-König (5):
  clocksource: sh_mtu2: Mark driver as non-removable
  clocksource: timer-stm32-lp: Mark driver as non-removable
  clocksource: timer-ti-dm: Improve error message in .remove
  clocksource: timer-tegra186: Convert to platform remove callback
    returning void
  clocksource: timer-ti-dm: Convert to platform remove callback
    returning void

 drivers/clocksource/sh_mtu2.c        | 7 +------
 drivers/clocksource/timer-stm32-lp.c | 7 +------
 drivers/clocksource/timer-tegra186.c | 6 ++----
 drivers/clocksource/timer-ti-dm.c    | 7 ++++---
 4 files changed, 8 insertions(+), 19 deletions(-)


base-commit: fe15c26ee26efa11741a7b632e9f23b01aca4cc6
-- 
2.39.1

[PATCH 1/5] clocksource: sh_mtu2: Mark driver as non-removable

[Uwe Kleine-König]

The comment in the remove callback suggests that the driver is not
supposed to be unbound. However returning an error code in the remove
callback doesn't accomplish that. Instead set the suppress_bind_attrs
property (which makes it impossible to unbind the driver via sysfs).
The only remaining way to unbind a sh_tmu2 device would be module
unloading, but that doesn't apply here, as the driver cannot be built as
a module.

Also drop the useless remove callback.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/clocksource/sh_mtu2.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/clocksource/sh_mtu2.c b/drivers/clocksource/sh_mtu2.c
index 169a1fccc497..6bd2d0299397 100644
--- a/drivers/clocksource/sh_mtu2.c
+++ b/drivers/clocksource/sh_mtu2.c
@@ -484,11 +484,6 @@ static int sh_mtu2_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int sh_mtu2_remove(struct platform_device *pdev)
-{
-	return -EBUSY; /* cannot unregister clockevent */
-}
-
 static const struct platform_device_id sh_mtu2_id_table[] = {
 	{ "sh-mtu2", 0 },
 	{ },
@@ -503,10 +498,10 @@ MODULE_DEVICE_TABLE(of, sh_mtu2_of_table);
 
 static struct platform_driver sh_mtu2_device_driver = {
 	.probe		= sh_mtu2_probe,
-	.remove		= sh_mtu2_remove,
 	.driver		= {
 		.name	= "sh_mtu2",
 		.of_match_table = of_match_ptr(sh_mtu2_of_table),
+		.suppress_bind_attrs = true,
 	},
 	.id_table	= sh_mtu2_id_table,
 };
-- 
2.39.1

[tip: timers/core] clocksource/drivers/sh_mtu2: Mark driver as non-removable

[tip-bot2 for Uwe Kleine-König]

The following commit has been merged into the timers/core branch of tip:

Commit-ID:     78012e3880a62e0eb130a0b5a10230162ad42a06
Gitweb:        https://git.kernel.org/tip/78012e3880a62e0eb130a0b5a10230162ad42a06
Author:        Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
AuthorDate:    Mon, 13 Mar 2023 08:54:26 +01:00
Committer:     Daniel Lezcano <daniel.lezcano@linaro.org>
CommitterDate: Mon, 24 Apr 2023 16:56:13 +02:00

clocksource/drivers/sh_mtu2: Mark driver as non-removable

The comment in the remove callback suggests that the driver is not
supposed to be unbound. However returning an error code in the remove
callback doesn't accomplish that. Instead set the suppress_bind_attrs
property (which makes it impossible to unbind the driver via sysfs).
The only remaining way to unbind a sh_tmu2 device would be module
unloading, but that doesn't apply here, as the driver cannot be built as
a module.

Also drop the useless remove callback.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Link: https://lore.kernel.org/r/20230313075430.2730803-2-u.kleine-koenig@pengutronix.de
---
 drivers/clocksource/sh_mtu2.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/clocksource/sh_mtu2.c b/drivers/clocksource/sh_mtu2.c
index 169a1fc..6bd2d02 100644
--- a/drivers/clocksource/sh_mtu2.c
+++ b/drivers/clocksource/sh_mtu2.c
@@ -484,11 +484,6 @@ static int sh_mtu2_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int sh_mtu2_remove(struct platform_device *pdev)
-{
-	return -EBUSY; /* cannot unregister clockevent */
-}
-
 static const struct platform_device_id sh_mtu2_id_table[] = {
 	{ "sh-mtu2", 0 },
 	{ },
@@ -503,10 +498,10 @@ MODULE_DEVICE_TABLE(of, sh_mtu2_of_table);
 
 static struct platform_driver sh_mtu2_device_driver = {
 	.probe		= sh_mtu2_probe,
-	.remove		= sh_mtu2_remove,
 	.driver		= {
 		.name	= "sh_mtu2",
 		.of_match_table = of_match_ptr(sh_mtu2_of_table),
+		.suppress_bind_attrs = true,
 	},
 	.id_table	= sh_mtu2_id_table,
 };

[PATCH 2/5] clocksource: timer-stm32-lp: Mark driver as non-removable

[Uwe Kleine-König]

The comment in the remove callback suggests that the driver is not
supposed to be unbound. However returning an error code in the remove
callback doesn't accomplish that. Instead set the suppress_bind_attrs
property (which makes it impossible to unbind the driver via sysfs).
The only remaining way to unbind an stm32-lp device would be module
unloading, but that doesn't apply here, as the driver cannot be built as
a module.

Also drop the useless remove callback.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/clocksource/timer-stm32-lp.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/clocksource/timer-stm32-lp.c b/drivers/clocksource/timer-stm32-lp.c
index db2841d0beb8..616ea4fe4234 100644
--- a/drivers/clocksource/timer-stm32-lp.c
+++ b/drivers/clocksource/timer-stm32-lp.c
@@ -195,11 +195,6 @@ static int stm32_clkevent_lp_probe(struct platform_device *pdev)
 	return ret;
 }
 
-static int stm32_clkevent_lp_remove(struct platform_device *pdev)
-{
-	return -EBUSY; /* cannot unregister clockevent */
-}
-
 static const struct of_device_id stm32_clkevent_lp_of_match[] = {
 	{ .compatible = "st,stm32-lptimer-timer", },
 	{},
@@ -207,11 +202,11 @@ static const struct of_device_id stm32_clkevent_lp_of_match[] = {
 MODULE_DEVICE_TABLE(of, stm32_clkevent_lp_of_match);
 
 static struct platform_driver stm32_clkevent_lp_driver = {
-	.probe	= stm32_clkevent_lp_probe,
 	.remove = stm32_clkevent_lp_remove,
 	.driver	= {
 		.name = "stm32-lptimer-timer",
 		.of_match_table = of_match_ptr(stm32_clkevent_lp_of_match),
+		.suppress_bind_attrs = true,
 	},
 };
 module_platform_driver(stm32_clkevent_lp_driver);
-- 
2.39.1

Re: [PATCH 2/5] clocksource: timer-stm32-lp: Mark driver as non-removable

[Uwe Kleine-König]

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

Hello,

On Mon, Mar 13, 2023 at 08:54:27AM +0100, Uwe Kleine-König wrote:
> The comment in the remove callback suggests that the driver is not
> supposed to be unbound. However returning an error code in the remove
> callback doesn't accomplish that. Instead set the suppress_bind_attrs
> property (which makes it impossible to unbind the driver via sysfs).
> The only remaining way to unbind an stm32-lp device would be module
> unloading, but that doesn't apply here, as the driver cannot be built as
> a module.
> 
> Also drop the useless remove callback.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/clocksource/timer-stm32-lp.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/drivers/clocksource/timer-stm32-lp.c b/drivers/clocksource/timer-stm32-lp.c
> index db2841d0beb8..616ea4fe4234 100644
> --- a/drivers/clocksource/timer-stm32-lp.c
> +++ b/drivers/clocksource/timer-stm32-lp.c
> @@ -195,11 +195,6 @@ static int stm32_clkevent_lp_probe(struct platform_device *pdev)
>  	return ret;
>  }
>  
> -static int stm32_clkevent_lp_remove(struct platform_device *pdev)
> -{
> -	return -EBUSY; /* cannot unregister clockevent */
> -}
> -
>  static const struct of_device_id stm32_clkevent_lp_of_match[] = {
>  	{ .compatible = "st,stm32-lptimer-timer", },
>  	{},
> @@ -207,11 +202,11 @@ static const struct of_device_id stm32_clkevent_lp_of_match[] = {
>  MODULE_DEVICE_TABLE(of, stm32_clkevent_lp_of_match);
>  
>  static struct platform_driver stm32_clkevent_lp_driver = {
> -	.probe	= stm32_clkevent_lp_probe,
>  	.remove = stm32_clkevent_lp_remove,

This is of course broken, I intended to drop the remove line ... and
only noticed that breakage after sending out the patch set :-\

So please either skip this patch, or fixup while applying. If you do the
former I'll come back to this driver and send a fixed patch.

Best regards and sorry
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

Re: [PATCH 2/5] clocksource: timer-stm32-lp: Mark driver as non-removable

[kernel test robot]

Hi Uwe,

I love your patch! Yet something to improve:

[auto build test ERROR on fe15c26ee26efa11741a7b632e9f23b01aca4cc6]

url:    https://github.com/intel-lab-lkp/linux/commits/Uwe-Kleine-K-nig/clocksource-sh_mtu2-Mark-driver-as-non-removable/20230313-155913
base:   fe15c26ee26efa11741a7b632e9f23b01aca4cc6
patch link:    https://lore.kernel.org/r/20230313075430.2730803-3-u.kleine-koenig%40pengutronix.de
patch subject: [PATCH 2/5] clocksource: timer-stm32-lp: Mark driver as non-removable
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230313/202303132013.6jB1U6Dg-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/d4016ca907c0dd473c1f28ce43f4ef2495cf1dd5
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Uwe-Kleine-K-nig/clocksource-sh_mtu2-Mark-driver-as-non-removable/20230313-155913
        git checkout d4016ca907c0dd473c1f28ce43f4ef2495cf1dd5
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303132013.6jB1U6Dg-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/clocksource/timer-stm32-lp.c:205:19: error: 'stm32_clkevent_lp_remove' undeclared here (not in a function); did you mean 'stm32_clkevent_lp_probe'?
     205 |         .remove = stm32_clkevent_lp_remove,
         |                   ^~~~~~~~~~~~~~~~~~~~~~~~
         |                   stm32_clkevent_lp_probe
   drivers/clocksource/timer-stm32-lp.c:142:12: warning: 'stm32_clkevent_lp_probe' defined but not used [-Wunused-function]
     142 | static int stm32_clkevent_lp_probe(struct platform_device *pdev)
         |            ^~~~~~~~~~~~~~~~~~~~~~~


vim +205 drivers/clocksource/timer-stm32-lp.c

48b41c5e2de6c5 Benjamin Gaignard 2020-06-03  203  
48b41c5e2de6c5 Benjamin Gaignard 2020-06-03  204  static struct platform_driver stm32_clkevent_lp_driver = {
48b41c5e2de6c5 Benjamin Gaignard 2020-06-03 @205  	.remove = stm32_clkevent_lp_remove,
48b41c5e2de6c5 Benjamin Gaignard 2020-06-03  206  	.driver	= {
48b41c5e2de6c5 Benjamin Gaignard 2020-06-03  207  		.name = "stm32-lptimer-timer",
48b41c5e2de6c5 Benjamin Gaignard 2020-06-03  208  		.of_match_table = of_match_ptr(stm32_clkevent_lp_of_match),
d4016ca907c0dd Uwe Kleine-König  2023-03-13  209  		.suppress_bind_attrs = true,
48b41c5e2de6c5 Benjamin Gaignard 2020-06-03  210  	},
48b41c5e2de6c5 Benjamin Gaignard 2020-06-03  211  };
48b41c5e2de6c5 Benjamin Gaignard 2020-06-03  212  module_platform_driver(stm32_clkevent_lp_driver);
48b41c5e2de6c5 Benjamin Gaignard 2020-06-03  213  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

[tip: timers/core] clocksource/drivers/timer-stm32-lp: Mark driver as non-removable

[tip-bot2 for Uwe Kleine-König]

The following commit has been merged into the timers/core branch of tip:

Commit-ID:     ede38f924a9e3c60382a13576347dc41967e8762
Gitweb:        https://git.kernel.org/tip/ede38f924a9e3c60382a13576347dc41967e8762
Author:        Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
AuthorDate:    Mon, 13 Mar 2023 08:54:27 +01:00
Committer:     Daniel Lezcano <daniel.lezcano@linaro.org>
CommitterDate: Mon, 24 Apr 2023 16:56:13 +02:00

clocksource/drivers/timer-stm32-lp: Mark driver as non-removable

The comment in the remove callback suggests that the driver is not
supposed to be unbound. However returning an error code in the remove
callback doesn't accomplish that. Instead set the suppress_bind_attrs
property (which makes it impossible to unbind the driver via sysfs).
The only remaining way to unbind an stm32-lp device would be module
unloading, but that doesn't apply here, as the driver cannot be built as
a module.

Also drop the useless remove callback.

[dlezcano] : Fixed up the wrong function removed

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Link: https://lore.kernel.org/r/20230313075430.2730803-3-u.kleine-koenig@pengutronix.de
---
 drivers/clocksource/timer-stm32-lp.c |  9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/clocksource/timer-stm32-lp.c b/drivers/clocksource/timer-stm32-lp.c
index db2841d..0adf22d 100644
--- a/drivers/clocksource/timer-stm32-lp.c
+++ b/drivers/clocksource/timer-stm32-lp.c
@@ -195,11 +195,6 @@ out_clk_disable:
 	return ret;
 }
 
-static int stm32_clkevent_lp_remove(struct platform_device *pdev)
-{
-	return -EBUSY; /* cannot unregister clockevent */
-}
-
 static const struct of_device_id stm32_clkevent_lp_of_match[] = {
 	{ .compatible = "st,stm32-lptimer-timer", },
 	{},
@@ -207,11 +202,11 @@ static const struct of_device_id stm32_clkevent_lp_of_match[] = {
 MODULE_DEVICE_TABLE(of, stm32_clkevent_lp_of_match);
 
 static struct platform_driver stm32_clkevent_lp_driver = {
-	.probe	= stm32_clkevent_lp_probe,
-	.remove = stm32_clkevent_lp_remove,
+	.probe  = stm32_clkevent_lp_probe,
 	.driver	= {
 		.name = "stm32-lptimer-timer",
 		.of_match_table = of_match_ptr(stm32_clkevent_lp_of_match),
+		.suppress_bind_attrs = true,
 	},
 };
 module_platform_driver(stm32_clkevent_lp_driver);

[PATCH 3/5] clocksource: timer-ti-dm: Improve error message in .remove

[Uwe Kleine-König]

If a platform driver's remove callback returns an error code, the driver
core emits a generic (and thus little helpful) error message.

Instead emit a more specifc error message about the actual error and
return zero to suppress the core's message.

Note that returning zero has no side effects apart from not emitting
said error message. This prepares converting platform driver's remove
message to return void.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/clocksource/timer-ti-dm.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/timer-ti-dm.c b/drivers/clocksource/timer-ti-dm.c
index b24b903a8822..098562bda487 100644
--- a/drivers/clocksource/timer-ti-dm.c
+++ b/drivers/clocksource/timer-ti-dm.c
@@ -1197,7 +1197,10 @@ static int omap_dm_timer_remove(struct platform_device *pdev)
 
 	pm_runtime_disable(&pdev->dev);
 
-	return ret;
+	if (ret)
+		dev_err(&pdev->dev, "Unable to determine timer entry in list of drivers on remove\n");
+
+	return 0;
 }
 
 static const struct omap_dm_timer_ops dmtimer_ops = {
-- 
2.39.1

[tip: timers/core] clocksource/drivers/timer-ti-dm: Improve error message in .remove

[tip-bot2 for Uwe Kleine-König]

The following commit has been merged into the timers/core branch of tip:

Commit-ID:     8efcbe927c5129d5b2528bbb40034c7dde87a6b6
Gitweb:        https://git.kernel.org/tip/8efcbe927c5129d5b2528bbb40034c7dde87a6b6
Author:        Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
AuthorDate:    Mon, 13 Mar 2023 08:54:28 +01:00
Committer:     Daniel Lezcano <daniel.lezcano@linaro.org>
CommitterDate: Mon, 24 Apr 2023 16:56:13 +02:00

clocksource/drivers/timer-ti-dm: Improve error message in .remove

If a platform driver's remove callback returns an error code, the driver
core emits a generic (and thus little helpful) error message.

Instead emit a more specifc error message about the actual error and
return zero to suppress the core's message.

Note that returning zero has no side effects apart from not emitting
said error message. This prepares converting platform driver's remove
message to return void.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Link: https://lore.kernel.org/r/20230313075430.2730803-4-u.kleine-koenig@pengutronix.de
---
 drivers/clocksource/timer-ti-dm.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/timer-ti-dm.c b/drivers/clocksource/timer-ti-dm.c
index b24b903..098562b 100644
--- a/drivers/clocksource/timer-ti-dm.c
+++ b/drivers/clocksource/timer-ti-dm.c
@@ -1197,7 +1197,10 @@ static int omap_dm_timer_remove(struct platform_device *pdev)
 
 	pm_runtime_disable(&pdev->dev);
 
-	return ret;
+	if (ret)
+		dev_err(&pdev->dev, "Unable to determine timer entry in list of drivers on remove\n");
+
+	return 0;
 }
 
 static const struct omap_dm_timer_ops dmtimer_ops = {

[PATCH 4/5] clocksource: timer-tegra186: Convert to platform remove callback returning void

[Uwe Kleine-König]

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.

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

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

diff --git a/drivers/clocksource/timer-tegra186.c b/drivers/clocksource/timer-tegra186.c
index ea742889ee06..ccc762d32422 100644
--- a/drivers/clocksource/timer-tegra186.c
+++ b/drivers/clocksource/timer-tegra186.c
@@ -447,15 +447,13 @@ static int tegra186_timer_probe(struct platform_device *pdev)
 	return err;
 }
 
-static int tegra186_timer_remove(struct platform_device *pdev)
+static void tegra186_timer_remove(struct platform_device *pdev)
 {
 	struct tegra186_timer *tegra = platform_get_drvdata(pdev);
 
 	clocksource_unregister(&tegra->usec);
 	clocksource_unregister(&tegra->osc);
 	clocksource_unregister(&tegra->tsc);
-
-	return 0;
 }
 
 static int __maybe_unused tegra186_timer_suspend(struct device *dev)
@@ -505,7 +503,7 @@ static struct platform_driver tegra186_wdt_driver = {
 		.of_match_table = tegra186_timer_of_match,
 	},
 	.probe = tegra186_timer_probe,
-	.remove = tegra186_timer_remove,
+	.remove_new = tegra186_timer_remove,
 };
 module_platform_driver(tegra186_wdt_driver);
 
-- 
2.39.1

[tip: timers/core] clocksource/drivers/timer-tegra186: Convert to platform remove callback returning void

[tip-bot2 for Uwe Kleine-König]

The following commit has been merged into the timers/core branch of tip:

Commit-ID:     d7b76421c03fa58d16a52eb839302f582602997a
Gitweb:        https://git.kernel.org/tip/d7b76421c03fa58d16a52eb839302f582602997a
Author:        Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
AuthorDate:    Mon, 13 Mar 2023 08:54:29 +01:00
Committer:     Daniel Lezcano <daniel.lezcano@linaro.org>
CommitterDate: Mon, 24 Apr 2023 16:56:13 +02:00

clocksource/drivers/timer-tegra186: Convert to platform remove callback returning void

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.

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

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Link: https://lore.kernel.org/r/20230313075430.2730803-5-u.kleine-koenig@pengutronix.de
---
 drivers/clocksource/timer-tegra186.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/clocksource/timer-tegra186.c b/drivers/clocksource/timer-tegra186.c
index ea74288..ccc762d 100644
--- a/drivers/clocksource/timer-tegra186.c
+++ b/drivers/clocksource/timer-tegra186.c
@@ -447,15 +447,13 @@ unregister_tsc:
 	return err;
 }
 
-static int tegra186_timer_remove(struct platform_device *pdev)
+static void tegra186_timer_remove(struct platform_device *pdev)
 {
 	struct tegra186_timer *tegra = platform_get_drvdata(pdev);
 
 	clocksource_unregister(&tegra->usec);
 	clocksource_unregister(&tegra->osc);
 	clocksource_unregister(&tegra->tsc);
-
-	return 0;
 }
 
 static int __maybe_unused tegra186_timer_suspend(struct device *dev)
@@ -505,7 +503,7 @@ static struct platform_driver tegra186_wdt_driver = {
 		.of_match_table = tegra186_timer_of_match,
 	},
 	.probe = tegra186_timer_probe,
-	.remove = tegra186_timer_remove,
+	.remove_new = tegra186_timer_remove,
 };
 module_platform_driver(tegra186_wdt_driver);
 

[PATCH 5/5] clocksource: timer-ti-dm: Convert to platform remove callback returning void

[Uwe Kleine-König]

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.

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

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

diff --git a/drivers/clocksource/timer-ti-dm.c b/drivers/clocksource/timer-ti-dm.c
index 098562bda487..ab7a6caa36c5 100644
--- a/drivers/clocksource/timer-ti-dm.c
+++ b/drivers/clocksource/timer-ti-dm.c
@@ -1177,7 +1177,7 @@ static int omap_dm_timer_probe(struct platform_device *pdev)
  * In addition to freeing platform resources it also deletes the timer
  * entry from the local list.
  */
-static int omap_dm_timer_remove(struct platform_device *pdev)
+static void omap_dm_timer_remove(struct platform_device *pdev)
 {
 	struct dmtimer *timer;
 	unsigned long flags;
@@ -1199,8 +1199,6 @@ static int omap_dm_timer_remove(struct platform_device *pdev)
 
 	if (ret)
 		dev_err(&pdev->dev, "Unable to determine timer entry in list of drivers on remove\n");
-
-	return 0;
 }
 
 static const struct omap_dm_timer_ops dmtimer_ops = {
@@ -1275,7 +1273,7 @@ MODULE_DEVICE_TABLE(of, omap_timer_match);
 
 static struct platform_driver omap_dm_timer_driver = {
 	.probe  = omap_dm_timer_probe,
-	.remove = omap_dm_timer_remove,
+	.remove_new = omap_dm_timer_remove,
 	.driver = {
 		.name   = "omap_timer",
 		.of_match_table = omap_timer_match,
-- 
2.39.1

[tip: timers/core] clocksource/drivers/timer-ti-dm: Convert to platform remove callback returning void

[tip-bot2 for Uwe Kleine-König]

The following commit has been merged into the timers/core branch of tip:

Commit-ID:     b1f0390048e2641d3451f8cdbbef24c79d1a8fdd
Gitweb:        https://git.kernel.org/tip/b1f0390048e2641d3451f8cdbbef24c79d1a8fdd
Author:        Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
AuthorDate:    Mon, 13 Mar 2023 08:54:30 +01:00
Committer:     Daniel Lezcano <daniel.lezcano@linaro.org>
CommitterDate: Mon, 24 Apr 2023 16:56:13 +02:00

clocksource/drivers/timer-ti-dm: Convert to platform remove callback returning void

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.

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

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Link: https://lore.kernel.org/r/20230313075430.2730803-6-u.kleine-koenig@pengutronix.de
---
 drivers/clocksource/timer-ti-dm.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/clocksource/timer-ti-dm.c b/drivers/clocksource/timer-ti-dm.c
index 098562b..ab7a6ca 100644
--- a/drivers/clocksource/timer-ti-dm.c
+++ b/drivers/clocksource/timer-ti-dm.c
@@ -1177,7 +1177,7 @@ err_disable:
  * In addition to freeing platform resources it also deletes the timer
  * entry from the local list.
  */
-static int omap_dm_timer_remove(struct platform_device *pdev)
+static void omap_dm_timer_remove(struct platform_device *pdev)
 {
 	struct dmtimer *timer;
 	unsigned long flags;
@@ -1199,8 +1199,6 @@ static int omap_dm_timer_remove(struct platform_device *pdev)
 
 	if (ret)
 		dev_err(&pdev->dev, "Unable to determine timer entry in list of drivers on remove\n");
-
-	return 0;
 }
 
 static const struct omap_dm_timer_ops dmtimer_ops = {
@@ -1275,7 +1273,7 @@ MODULE_DEVICE_TABLE(of, omap_timer_match);
 
 static struct platform_driver omap_dm_timer_driver = {
 	.probe  = omap_dm_timer_probe,
-	.remove = omap_dm_timer_remove,
+	.remove_new = omap_dm_timer_remove,
 	.driver = {
 		.name   = "omap_timer",
 		.of_match_table = omap_timer_match,

Re: [PATCH 0/5] clocksource: Convert to platform remove callback returning void

[Daniel Lezcano]

On 13/03/2023 08:54, Uwe Kleine-König wrote:
> Hello,
> 
> this patch series adapts the platform drivers below drivers/clk
> to use the .remove_new() callback. Compared to the traditional .remove()
> callback .remove_new() returns no value. This is a good thing because
> the driver core doesn't (and cannot) cope for errors during remove. The
> only effect of a non-zero return value in .remove() is that the driver
> core emits a warning. The device is removed anyhow and an early return
> from .remove() usually yields a resource leak.
> 
> Most clocksource drivers are not supposed to be removed. Two drivers are
> adapted here to actually prevent removal. One driver is fixed not to
> return an error code in .remove() and then the two remaining drivers
> with a remove callback are trivially converted to .remove_new().
> 

Applied and fixed up patch #2


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH 0/5] clocksource: Convert to platform remove callback returning void

[Uwe Kleine-König]

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

Hello Daniel,

On Thu, Apr 06, 2023 at 03:54:11PM +0200, Daniel Lezcano wrote:
> On 13/03/2023 08:54, Uwe Kleine-König wrote:
> > this patch series adapts the platform drivers below drivers/clk
> > to use the .remove_new() callback. Compared to the traditional .remove()
> > callback .remove_new() returns no value. This is a good thing because
> > the driver core doesn't (and cannot) cope for errors during remove. The
> > only effect of a non-zero return value in .remove() is that the driver
> > core emits a warning. The device is removed anyhow and an early return
> > from .remove() usually yields a resource leak.
> > 
> > Most clocksource drivers are not supposed to be removed. Two drivers are
> > adapted here to actually prevent removal. One driver is fixed not to
> > return an error code in .remove() and then the two remaining drivers
> > with a remove callback are trivially converted to .remove_new().
> > 
> 
> Applied and fixed up patch #2

Great. Thank you.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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