diff mbox series

cpufreq: Make cpufreq_unregister_driver() return void

Message ID 20230207195909.474953-1-u.kleine-koenig@pengutronix.de (mailing list archive)
State Not Applicable
Headers show
Series cpufreq: Make cpufreq_unregister_driver() return void | expand

Commit Message

Uwe Kleine-König Feb. 7, 2023, 7:59 p.m. UTC
All but a few drivers ignore the return value of
cpufreq_unregister_driver(). Those few that don't only call it after
cpufreq_register_driver() succeeded, in which case the call doesn't
fail.

Make the function return no value and add a WARN_ON for the case that
the function is called in an invalid situation (i.e. without a previous
successful call to cpufreq_register_driver()).

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/cpufreq/brcmstb-avs-cpufreq.c | 5 +----
 drivers/cpufreq/cpufreq.c             | 8 +++-----
 drivers/cpufreq/davinci-cpufreq.c     | 4 +++-
 drivers/cpufreq/mediatek-cpufreq-hw.c | 4 +++-
 drivers/cpufreq/omap-cpufreq.c        | 4 +++-
 drivers/cpufreq/qcom-cpufreq-hw.c     | 4 +++-
 include/linux/cpufreq.h               | 2 +-
 7 files changed, 17 insertions(+), 14 deletions(-)


base-commit: 05ecb680708a1dbe6554d6fc17e5d9a8a7cb5e6a

Comments

Florian Fainelli Feb. 7, 2023, 8:33 p.m. UTC | #1
On 2/7/23 11:59, Uwe Kleine-König wrote:
> All but a few drivers ignore the return value of
> cpufreq_unregister_driver(). Those few that don't only call it after
> cpufreq_register_driver() succeeded, in which case the call doesn't
> fail.
> 
> Make the function return no value and add a WARN_ON for the case that
> the function is called in an invalid situation (i.e. without a previous
> successful call to cpufreq_register_driver()).
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>   drivers/cpufreq/brcmstb-avs-cpufreq.c | 5 +----

Acked-by: Florian Fainelli <f.fainelli@gmail.com> # brcmstb-avs-cpufreq.c
Viresh Kumar Feb. 8, 2023, 5:05 a.m. UTC | #2
On 07-02-23, 20:59, Uwe Kleine-König wrote:
> All but a few drivers ignore the return value of
> cpufreq_unregister_driver(). Those few that don't only call it after
> cpufreq_register_driver() succeeded, in which case the call doesn't
> fail.
> 
> Make the function return no value and add a WARN_ON for the case that
> the function is called in an invalid situation (i.e. without a previous
> successful call to cpufreq_register_driver()).
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/cpufreq/brcmstb-avs-cpufreq.c | 5 +----
>  drivers/cpufreq/cpufreq.c             | 8 +++-----
>  drivers/cpufreq/davinci-cpufreq.c     | 4 +++-
>  drivers/cpufreq/mediatek-cpufreq-hw.c | 4 +++-
>  drivers/cpufreq/omap-cpufreq.c        | 4 +++-
>  drivers/cpufreq/qcom-cpufreq-hw.c     | 4 +++-
>  include/linux/cpufreq.h               | 2 +-
>  7 files changed, 17 insertions(+), 14 deletions(-)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
AngeloGioacchino Del Regno Feb. 8, 2023, 1:12 p.m. UTC | #3
Il 07/02/23 20:59, Uwe Kleine-König ha scritto:
> All but a few drivers ignore the return value of
> cpufreq_unregister_driver(). Those few that don't only call it after
> cpufreq_register_driver() succeeded, in which case the call doesn't
> fail.
> 
> Make the function return no value and add a WARN_ON for the case that
> the function is called in an invalid situation (i.e. without a previous
> successful call to cpufreq_register_driver()).
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

For MTK and generic part:

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Wyes Karny Feb. 8, 2023, 4:04 p.m. UTC | #4
Hi Uwe Kleine-König,

On 07 Feb 20:59, Uwe Kleine-König wrote:
> All but a few drivers ignore the return value of
> cpufreq_unregister_driver(). Those few that don't only call it after
> cpufreq_register_driver() succeeded, in which case the call doesn't
> fail.
> 
> Make the function return no value and add a WARN_ON for the case that
> the function is called in an invalid situation (i.e. without a previous
> successful call to cpufreq_register_driver()).
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/cpufreq/brcmstb-avs-cpufreq.c | 5 +----
>  drivers/cpufreq/cpufreq.c             | 8 +++-----
>  drivers/cpufreq/davinci-cpufreq.c     | 4 +++-
>  drivers/cpufreq/mediatek-cpufreq-hw.c | 4 +++-
>  drivers/cpufreq/omap-cpufreq.c        | 4 +++-
>  drivers/cpufreq/qcom-cpufreq-hw.c     | 4 +++-
>  include/linux/cpufreq.h               | 2 +-
>  7 files changed, 17 insertions(+), 14 deletions(-)

> base-commit: 05ecb680708a1dbe6554d6fc17e5d9a8a7cb5e6a

You may have to rebase it on top of this [1].
Recently this patch series was picked up by Rafael.
You have to add the below hunk in your patch.

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 168a28bed6ee..70debd5a9f40 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -831,7 +831,7 @@ static void amd_pstate_driver_cleanup(void)

 static int amd_pstate_update_status(const char *buf, size_t size)
 {
-       int ret;
+       int ret = 0;
        int mode_idx;

        if (size > 7 || size < 6)
@@ -844,7 +844,7 @@ static int amd_pstate_update_status(const char *buf,
size_t size)
                        return -EINVAL;
                if (cppc_state == AMD_PSTATE_ACTIVE)
                        return -EBUSY;
-               ret = cpufreq_unregister_driver(current_pstate_driver);
+               cpufreq_unregister_driver(current_pstate_driver);
                amd_pstate_driver_cleanup();
                break;
        case AMD_PSTATE_PASSIVE:


Otherwise the patch looks good to me.

[1]: https://lore.kernel.org/linux-pm/20230131090016.3970625-1-perry.yuan@amd.com/#t

Thanks,
Wyes
> -- 
> 2.39.0
>
Uwe Kleine-König Feb. 8, 2023, 4:56 p.m. UTC | #5
Hello,

On Wed, Feb 08, 2023 at 04:04:57PM +0000, Wyes Karny wrote:
> On 07 Feb 20:59, Uwe Kleine-König wrote:
> > All but a few drivers ignore the return value of
> > cpufreq_unregister_driver(). Those few that don't only call it after
> > cpufreq_register_driver() succeeded, in which case the call doesn't
> > fail.
> > 
> > Make the function return no value and add a WARN_ON for the case that
> > the function is called in an invalid situation (i.e. without a previous
> > successful call to cpufreq_register_driver()).
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> >  drivers/cpufreq/brcmstb-avs-cpufreq.c | 5 +----
> >  drivers/cpufreq/cpufreq.c             | 8 +++-----
> >  drivers/cpufreq/davinci-cpufreq.c     | 4 +++-
> >  drivers/cpufreq/mediatek-cpufreq-hw.c | 4 +++-
> >  drivers/cpufreq/omap-cpufreq.c        | 4 +++-
> >  drivers/cpufreq/qcom-cpufreq-hw.c     | 4 +++-
> >  include/linux/cpufreq.h               | 2 +-
> >  7 files changed, 17 insertions(+), 14 deletions(-)
> 
> > base-commit: 05ecb680708a1dbe6554d6fc17e5d9a8a7cb5e6a
> 
> You may have to rebase it on top of this [1].
> Recently this patch series was picked up by Rafael.
> You have to add the below hunk in your patch.
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 168a28bed6ee..70debd5a9f40 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -831,7 +831,7 @@ static void amd_pstate_driver_cleanup(void)
> 
>  static int amd_pstate_update_status(const char *buf, size_t size)
>  {
> -       int ret;
> +       int ret = 0;
>         int mode_idx;
> 
>         if (size > 7 || size < 6)
> @@ -844,7 +844,7 @@ static int amd_pstate_update_status(const char *buf,
> size_t size)
>                         return -EINVAL;
>                 if (cppc_state == AMD_PSTATE_ACTIVE)
>                         return -EBUSY;
> -               ret = cpufreq_unregister_driver(current_pstate_driver);
> +               cpufreq_unregister_driver(current_pstate_driver);
>                 amd_pstate_driver_cleanup();
>                 break;
>         case AMD_PSTATE_PASSIVE:

Good catch. The adaption looks right. @Rafael, please tell me, if you
want me to adapt my patch and resend.

Best regards
Uwe
Rafael J. Wysocki Feb. 9, 2023, 7:12 p.m. UTC | #6
On Wed, Feb 8, 2023 at 6:05 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 07-02-23, 20:59, Uwe Kleine-König wrote:
> > All but a few drivers ignore the return value of
> > cpufreq_unregister_driver(). Those few that don't only call it after
> > cpufreq_register_driver() succeeded, in which case the call doesn't
> > fail.
> >
> > Make the function return no value and add a WARN_ON for the case that
> > the function is called in an invalid situation (i.e. without a previous
> > successful call to cpufreq_register_driver()).
> >
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> >  drivers/cpufreq/brcmstb-avs-cpufreq.c | 5 +----
> >  drivers/cpufreq/cpufreq.c             | 8 +++-----
> >  drivers/cpufreq/davinci-cpufreq.c     | 4 +++-
> >  drivers/cpufreq/mediatek-cpufreq-hw.c | 4 +++-
> >  drivers/cpufreq/omap-cpufreq.c        | 4 +++-
> >  drivers/cpufreq/qcom-cpufreq-hw.c     | 4 +++-
> >  include/linux/cpufreq.h               | 2 +-
> >  7 files changed, 17 insertions(+), 14 deletions(-)
>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

Applied as 6.3 material, thanks!
diff mbox series

Patch

diff --git a/drivers/cpufreq/brcmstb-avs-cpufreq.c b/drivers/cpufreq/brcmstb-avs-cpufreq.c
index 4153150e20db..ffea6402189d 100644
--- a/drivers/cpufreq/brcmstb-avs-cpufreq.c
+++ b/drivers/cpufreq/brcmstb-avs-cpufreq.c
@@ -751,10 +751,7 @@  static int brcm_avs_cpufreq_probe(struct platform_device *pdev)
 
 static int brcm_avs_cpufreq_remove(struct platform_device *pdev)
 {
-	int ret;
-
-	ret = cpufreq_unregister_driver(&brcm_avs_driver);
-	WARN_ON(ret);
+	cpufreq_unregister_driver(&brcm_avs_driver);
 
 	brcm_avs_prepare_uninit(pdev);
 
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 7e56a42750ea..85a0bea2dbf1 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -2904,12 +2904,12 @@  EXPORT_SYMBOL_GPL(cpufreq_register_driver);
  * Returns zero if successful, and -EINVAL if the cpufreq_driver is
  * currently not initialised.
  */
-int cpufreq_unregister_driver(struct cpufreq_driver *driver)
+void cpufreq_unregister_driver(struct cpufreq_driver *driver)
 {
 	unsigned long flags;
 
-	if (!cpufreq_driver || (driver != cpufreq_driver))
-		return -EINVAL;
+	if (WARN_ON(!cpufreq_driver || (driver != cpufreq_driver)))
+		return;
 
 	pr_debug("unregistering driver %s\n", driver->name);
 
@@ -2926,8 +2926,6 @@  int cpufreq_unregister_driver(struct cpufreq_driver *driver)
 
 	write_unlock_irqrestore(&cpufreq_driver_lock, flags);
 	cpus_read_unlock();
-
-	return 0;
 }
 EXPORT_SYMBOL_GPL(cpufreq_unregister_driver);
 
diff --git a/drivers/cpufreq/davinci-cpufreq.c b/drivers/cpufreq/davinci-cpufreq.c
index 9e97f60f8199..2d23015e2abd 100644
--- a/drivers/cpufreq/davinci-cpufreq.c
+++ b/drivers/cpufreq/davinci-cpufreq.c
@@ -138,7 +138,9 @@  static int __exit davinci_cpufreq_remove(struct platform_device *pdev)
 	if (cpufreq.asyncclk)
 		clk_put(cpufreq.asyncclk);
 
-	return cpufreq_unregister_driver(&davinci_driver);
+	cpufreq_unregister_driver(&davinci_driver);
+
+	return 0;
 }
 
 static struct platform_driver davinci_cpufreq_driver = {
diff --git a/drivers/cpufreq/mediatek-cpufreq-hw.c b/drivers/cpufreq/mediatek-cpufreq-hw.c
index f80339779084..f21a9e3df53d 100644
--- a/drivers/cpufreq/mediatek-cpufreq-hw.c
+++ b/drivers/cpufreq/mediatek-cpufreq-hw.c
@@ -317,7 +317,9 @@  static int mtk_cpufreq_hw_driver_probe(struct platform_device *pdev)
 
 static int mtk_cpufreq_hw_driver_remove(struct platform_device *pdev)
 {
-	return cpufreq_unregister_driver(&cpufreq_mtk_hw_driver);
+	cpufreq_unregister_driver(&cpufreq_mtk_hw_driver);
+
+	return 0;
 }
 
 static const struct of_device_id mtk_cpufreq_hw_match[] = {
diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c
index 1b50df06c6bc..81649a1969b6 100644
--- a/drivers/cpufreq/omap-cpufreq.c
+++ b/drivers/cpufreq/omap-cpufreq.c
@@ -184,7 +184,9 @@  static int omap_cpufreq_probe(struct platform_device *pdev)
 
 static int omap_cpufreq_remove(struct platform_device *pdev)
 {
-	return cpufreq_unregister_driver(&omap_driver);
+	cpufreq_unregister_driver(&omap_driver);
+
+	return 0;
 }
 
 static struct platform_driver omap_cpufreq_platdrv = {
diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
index 9505a812d6a1..8a1b140884af 100644
--- a/drivers/cpufreq/qcom-cpufreq-hw.c
+++ b/drivers/cpufreq/qcom-cpufreq-hw.c
@@ -766,7 +766,9 @@  static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev)
 
 static int qcom_cpufreq_hw_driver_remove(struct platform_device *pdev)
 {
-	return cpufreq_unregister_driver(&cpufreq_qcom_hw_driver);
+	cpufreq_unregister_driver(&cpufreq_qcom_hw_driver);
+
+	return 0;
 }
 
 static struct platform_driver qcom_cpufreq_hw_driver = {
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 6a94a6eaad27..65623233ab2f 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -448,7 +448,7 @@  struct cpufreq_driver {
 #define CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING	BIT(6)
 
 int cpufreq_register_driver(struct cpufreq_driver *driver_data);
-int cpufreq_unregister_driver(struct cpufreq_driver *driver_data);
+void cpufreq_unregister_driver(struct cpufreq_driver *driver_data);
 
 bool cpufreq_driver_test_flags(u16 flags);
 const char *cpufreq_get_current_driver(void);