Some APIs used by this driver don't have devm_ helpers yet. Instead of
using the devm_add_action_or_reset() the current driver is based on hand
crafted error goto paths and a .remove() callback.
Convert the driver to devm_ APIs only by making use of
devm_add_action_or_reset() and devm_pm_runtime_enable() to simplify the
release and error path.
Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
drivers/pmdomain/imx/imx93-blk-ctrl.c | 64 +++++++++++++++--------------------
1 file changed, 27 insertions(+), 37 deletions(-)
diff --git a/drivers/pmdomain/imx/imx93-blk-ctrl.c b/drivers/pmdomain/imx/imx93-blk-ctrl.c
index 2aa163aef8de4ee901b9cde76ce2aad246c8c08a..13b0de6dd689cf944e034f7666a4e97b0118e3bd 100644
--- a/drivers/pmdomain/imx/imx93-blk-ctrl.c
+++ b/drivers/pmdomain/imx/imx93-blk-ctrl.c
@@ -188,6 +188,20 @@ static int imx93_blk_ctrl_power_off(struct generic_pm_domain *genpd)
return 0;
}
+static void imx93_release_genpd_provider(void *data)
+{
+ struct device_node *of_node = data;
+
+ of_genpd_del_provider(of_node);
+}
+
+static void imx93_release_pm_genpd(void *data)
+{
+ struct generic_pm_domain *genpd = data;
+
+ pm_genpd_remove(genpd);
+}
+
static struct lock_class_key blk_ctrl_genpd_lock_class;
static int imx93_blk_ctrl_probe(struct platform_device *pdev)
@@ -256,10 +270,8 @@ static int imx93_blk_ctrl_probe(struct platform_device *pdev)
domain->clks[j].id = data->clk_names[j];
ret = devm_clk_bulk_get(dev, data->num_clks, domain->clks);
- if (ret) {
- dev_err_probe(dev, ret, "failed to get clock\n");
- goto cleanup_pds;
- }
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to get clock\n");
domain->genpd.name = data->name;
domain->genpd.power_on = imx93_blk_ctrl_power_on;
@@ -267,11 +279,12 @@ static int imx93_blk_ctrl_probe(struct platform_device *pdev)
domain->bc = bc;
ret = pm_genpd_init(&domain->genpd, NULL, true);
- if (ret) {
- dev_err_probe(dev, ret, "failed to init power domain\n");
- goto cleanup_pds;
- }
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to init power domain\n");
+ ret = devm_add_action_or_reset(dev, imx93_release_pm_genpd, &domain->genpd);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to add pm_genpd release callback\n");
/*
* We use runtime PM to trigger power on/off of the upstream GPC
* domain, as a strict hierarchical parent/child power domain
@@ -288,39 +301,17 @@ static int imx93_blk_ctrl_probe(struct platform_device *pdev)
bc->onecell_data.domains[i] = &domain->genpd;
}
- pm_runtime_enable(dev);
+ devm_pm_runtime_enable(dev);
ret = of_genpd_add_provider_onecell(dev->of_node, &bc->onecell_data);
- if (ret) {
- dev_err_probe(dev, ret, "failed to add power domain provider\n");
- goto cleanup_pds;
- }
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to add power domain provider\n");
- dev_set_drvdata(dev, bc);
+ ret = devm_add_action_or_reset(dev, imx93_release_genpd_provider, dev->of_node);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to add genpd_provider release callback\n");
return 0;
-
-cleanup_pds:
- for (i--; i >= 0; i--)
- pm_genpd_remove(&bc->domains[i].genpd);
-
- return ret;
-}
-
-static void imx93_blk_ctrl_remove(struct platform_device *pdev)
-{
- struct imx93_blk_ctrl *bc = dev_get_drvdata(&pdev->dev);
- int i;
-
- of_genpd_del_provider(pdev->dev.of_node);
-
- pm_runtime_disable(&pdev->dev);
-
- for (i = 0; i < bc->onecell_data.num_domains; i++) {
- struct imx93_blk_ctrl_domain *domain = &bc->domains[i];
-
- pm_genpd_remove(&domain->genpd);
- }
}
static const struct imx93_blk_ctrl_domain_data imx93_media_blk_ctl_domain_data[] = {
@@ -455,7 +446,6 @@ MODULE_DEVICE_TABLE(of, imx93_blk_ctrl_of_match);
static struct platform_driver imx93_blk_ctrl_driver = {
.probe = imx93_blk_ctrl_probe,
- .remove = imx93_blk_ctrl_remove,
.driver = {
.name = "imx93-blk-ctrl",
.of_match_table = imx93_blk_ctrl_of_match,
--
2.47.3On Mon, Dec 01, 2025 at 06:12:06PM +0100, Marco Felsch wrote:
> Some APIs used by this driver don't have devm_ helpers yet. Instead of
> using the devm_add_action_or_reset() the current driver is based on hand
> crafted error goto paths and a .remove() callback.
>
> Convert the driver to devm_ APIs only by making use of
> devm_add_action_or_reset() and devm_pm_runtime_enable() to simplify the
> release and error path.
Nit: I think keep this paragaph should be enough.
>
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
> drivers/pmdomain/imx/imx93-blk-ctrl.c | 64 +++++++++++++++--------------------
> 1 file changed, 27 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/pmdomain/imx/imx93-blk-ctrl.c b/drivers/pmdomain/imx/imx93-blk-ctrl.c
> index 2aa163aef8de4ee901b9cde76ce2aad246c8c08a..13b0de6dd689cf944e034f7666a4e97b0118e3bd 100644
> --- a/drivers/pmdomain/imx/imx93-blk-ctrl.c
> +++ b/drivers/pmdomain/imx/imx93-blk-ctrl.c
> @@ -188,6 +188,20 @@ static int imx93_blk_ctrl_power_off(struct generic_pm_domain *genpd)
> return 0;
> }
>
...
> static struct lock_class_key blk_ctrl_genpd_lock_class;
>
> static int imx93_blk_ctrl_probe(struct platform_device *pdev)
> @@ -256,10 +270,8 @@ static int imx93_blk_ctrl_probe(struct platform_device *pdev)
> domain->clks[j].id = data->clk_names[j];
>
> ret = devm_clk_bulk_get(dev, data->num_clks, domain->clks);
> - if (ret) {
> - dev_err_probe(dev, ret, "failed to get clock\n");
> - goto cleanup_pds;
> - }
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to get clock\n");
>
> domain->genpd.name = data->name;
> domain->genpd.power_on = imx93_blk_ctrl_power_on;
> @@ -267,11 +279,12 @@ static int imx93_blk_ctrl_probe(struct platform_device *pdev)
> domain->bc = bc;
>
> ret = pm_genpd_init(&domain->genpd, NULL, true);
> - if (ret) {
> - dev_err_probe(dev, ret, "failed to init power domain\n");
> - goto cleanup_pds;
> - }
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to init power domain\n");
>
> + ret = devm_add_action_or_reset(dev, imx93_release_pm_genpd, &domain->genpd);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to add pm_genpd release callback\n");
> /*
> * We use runtime PM to trigger power on/off of the upstream GPC
> * domain, as a strict hierarchical parent/child power domain
> @@ -288,39 +301,17 @@ static int imx93_blk_ctrl_probe(struct platform_device *pdev)
> bc->onecell_data.domains[i] = &domain->genpd;
> }
>
> - pm_runtime_enable(dev);
> + devm_pm_runtime_enable(dev);
Need check return value
>
> ret = of_genpd_add_provider_onecell(dev->of_node, &bc->onecell_data);
> - if (ret) {
> - dev_err_probe(dev, ret, "failed to add power domain provider\n");
> - goto cleanup_pds;
> - }
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to add power domain provider\n");
>
> - dev_set_drvdata(dev, bc);
why remove dev_set_drvdata(dev, bc)
Frank
> + ret = devm_add_action_or_reset(dev, imx93_release_genpd_provider, dev->of_node);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to add genpd_provider release callback\n");
>
> return 0;
> -
> -cleanup_pds:
> - for (i--; i >= 0; i--)
> - pm_genpd_remove(&bc->domains[i].genpd);
> -
> - return ret;
> -}
> -
> -static void imx93_blk_ctrl_remove(struct platform_device *pdev)
> -{
> - struct imx93_blk_ctrl *bc = dev_get_drvdata(&pdev->dev);
> - int i;
> -
> - of_genpd_del_provider(pdev->dev.of_node);
> -
> - pm_runtime_disable(&pdev->dev);
> -
> - for (i = 0; i < bc->onecell_data.num_domains; i++) {
> - struct imx93_blk_ctrl_domain *domain = &bc->domains[i];
> -
> - pm_genpd_remove(&domain->genpd);
> - }
> }
>
> static const struct imx93_blk_ctrl_domain_data imx93_media_blk_ctl_domain_data[] = {
> @@ -455,7 +446,6 @@ MODULE_DEVICE_TABLE(of, imx93_blk_ctrl_of_match);
>
> static struct platform_driver imx93_blk_ctrl_driver = {
> .probe = imx93_blk_ctrl_probe,
> - .remove = imx93_blk_ctrl_remove,
> .driver = {
> .name = "imx93-blk-ctrl",
> .of_match_table = imx93_blk_ctrl_of_match,
>
> --
> 2.47.3
>
On 25-12-01, Frank Li wrote:
> On Mon, Dec 01, 2025 at 06:12:06PM +0100, Marco Felsch wrote:
> > Some APIs used by this driver don't have devm_ helpers yet. Instead of
> > using the devm_add_action_or_reset() the current driver is based on hand
> > crafted error goto paths and a .remove() callback.
> >
> > Convert the driver to devm_ APIs only by making use of
> > devm_add_action_or_reset() and devm_pm_runtime_enable() to simplify the
> > release and error path.
>
> Nit: I think keep this paragaph should be enough.
I can re-phrase it.
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> > drivers/pmdomain/imx/imx93-blk-ctrl.c | 64 +++++++++++++++--------------------
> > 1 file changed, 27 insertions(+), 37 deletions(-)
> >
> > diff --git a/drivers/pmdomain/imx/imx93-blk-ctrl.c b/drivers/pmdomain/imx/imx93-blk-ctrl.c
> > index 2aa163aef8de4ee901b9cde76ce2aad246c8c08a..13b0de6dd689cf944e034f7666a4e97b0118e3bd 100644
> > --- a/drivers/pmdomain/imx/imx93-blk-ctrl.c
> > +++ b/drivers/pmdomain/imx/imx93-blk-ctrl.c
> > @@ -188,6 +188,20 @@ static int imx93_blk_ctrl_power_off(struct generic_pm_domain *genpd)
> > return 0;
> > }
> >
> ...
> > static struct lock_class_key blk_ctrl_genpd_lock_class;
> >
> > static int imx93_blk_ctrl_probe(struct platform_device *pdev)
> > @@ -256,10 +270,8 @@ static int imx93_blk_ctrl_probe(struct platform_device *pdev)
> > domain->clks[j].id = data->clk_names[j];
> >
> > ret = devm_clk_bulk_get(dev, data->num_clks, domain->clks);
> > - if (ret) {
> > - dev_err_probe(dev, ret, "failed to get clock\n");
> > - goto cleanup_pds;
> > - }
> > + if (ret)
> > + return dev_err_probe(dev, ret, "failed to get clock\n");
> >
> > domain->genpd.name = data->name;
> > domain->genpd.power_on = imx93_blk_ctrl_power_on;
> > @@ -267,11 +279,12 @@ static int imx93_blk_ctrl_probe(struct platform_device *pdev)
> > domain->bc = bc;
> >
> > ret = pm_genpd_init(&domain->genpd, NULL, true);
> > - if (ret) {
> > - dev_err_probe(dev, ret, "failed to init power domain\n");
> > - goto cleanup_pds;
> > - }
> > + if (ret)
> > + return dev_err_probe(dev, ret, "failed to init power domain\n");
> >
> > + ret = devm_add_action_or_reset(dev, imx93_release_pm_genpd, &domain->genpd);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "failed to add pm_genpd release callback\n");
> > /*
> > * We use runtime PM to trigger power on/off of the upstream GPC
> > * domain, as a strict hierarchical parent/child power domain
> > @@ -288,39 +301,17 @@ static int imx93_blk_ctrl_probe(struct platform_device *pdev)
> > bc->onecell_data.domains[i] = &domain->genpd;
> > }
> >
> > - pm_runtime_enable(dev);
> > + devm_pm_runtime_enable(dev);
>
> Need check return value
Missed this one, thanks.
> > ret = of_genpd_add_provider_onecell(dev->of_node, &bc->onecell_data);
> > - if (ret) {
> > - dev_err_probe(dev, ret, "failed to add power domain provider\n");
> > - goto cleanup_pds;
> > - }
> > + if (ret)
> > + return dev_err_probe(dev, ret, "failed to add power domain provider\n");
> >
> > - dev_set_drvdata(dev, bc);
>
> why remove dev_set_drvdata(dev, bc)
Because it's no longer needed, it was required due to the .remove()
callback. I wasn't sure if I should mention this within the commit
message. I'm going to add it to the commit message since you asked.
Regards,
Marco
>
> Frank
> > + ret = devm_add_action_or_reset(dev, imx93_release_genpd_provider, dev->of_node);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "failed to add genpd_provider release callback\n");
> >
> > return 0;
> > -
> > -cleanup_pds:
> > - for (i--; i >= 0; i--)
> > - pm_genpd_remove(&bc->domains[i].genpd);
> > -
> > - return ret;
> > -}
> > -
> > -static void imx93_blk_ctrl_remove(struct platform_device *pdev)
> > -{
> > - struct imx93_blk_ctrl *bc = dev_get_drvdata(&pdev->dev);
> > - int i;
> > -
> > - of_genpd_del_provider(pdev->dev.of_node);
> > -
> > - pm_runtime_disable(&pdev->dev);
> > -
> > - for (i = 0; i < bc->onecell_data.num_domains; i++) {
> > - struct imx93_blk_ctrl_domain *domain = &bc->domains[i];
> > -
> > - pm_genpd_remove(&domain->genpd);
> > - }
> > }
> >
> > static const struct imx93_blk_ctrl_domain_data imx93_media_blk_ctl_domain_data[] = {
> > @@ -455,7 +446,6 @@ MODULE_DEVICE_TABLE(of, imx93_blk_ctrl_of_match);
> >
> > static struct platform_driver imx93_blk_ctrl_driver = {
> > .probe = imx93_blk_ctrl_probe,
> > - .remove = imx93_blk_ctrl_remove,
> > .driver = {
> > .name = "imx93-blk-ctrl",
> > .of_match_table = imx93_blk_ctrl_of_match,
> >
> > --
> > 2.47.3
> >
>
--
#gernperDu
#CallMeByMyFirstName
Pengutronix e.K. | |
Steuerwalder Str. 21 | https://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
© 2016 - 2026 Red Hat, Inc.