| 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 |
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
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>
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>
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 >
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
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 --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);
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