| Message ID | 20240825143428.556439-2-dario.binacchi@amarulasolutions.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/3] pmdomain: imx93-pd: replace dev_err() with dev_err_probe() | expand |
On Sun, 25 Aug 2024 at 16:34, Dario Binacchi <dario.binacchi@amarulasolutions.com> wrote: > > The removed code was added to handle the case where the power domain is > already on during the driver's probing. In this use case, the "is_off" > parameter is passed as false to pm_genpd_init() to inform it not to call > the power_on() callback, as it's unnecessary to perform the hardware > power-on procedure since the power domain is already on. Therefore, with > the call to clk_bulk_prepare_enable() by probe(), the system is in the > same operational state as when "is_off" is passed as true after the > power_on() callback execution: > > probe() -> is_off == true -> clk_bulk_prepare_enable() called by power_on() > probe() -> is_off == false -> clk_bulk_prepare_enable() called by probe() > > Reaching the same logical and operational state, it follows that upon > driver removal, there is no need to perform different actions depending > on the power domain's on/off state during probing. Well, I guess in most cases genpd would have tried to turn off the PM domain when it is getting removed. However, there are really no guarantees for that to happen. We should probably look into that as a general improvement for genpd. > > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com> The above said, this still looks like an improvement to me, so I decided to queue this up for next, thanks! Kind regards Uffe > --- > > (no changes since v1) > > drivers/pmdomain/imx/imx93-pd.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/pmdomain/imx/imx93-pd.c b/drivers/pmdomain/imx/imx93-pd.c > index 44daecbe5cc3..fb53a8e359bc 100644 > --- a/drivers/pmdomain/imx/imx93-pd.c > +++ b/drivers/pmdomain/imx/imx93-pd.c > @@ -90,9 +90,6 @@ static void imx93_pd_remove(struct platform_device *pdev) > struct device *dev = &pdev->dev; > struct device_node *np = dev->of_node; > > - if (!domain->init_off) > - clk_bulk_disable_unprepare(domain->num_clks, domain->clks); > - > of_genpd_del_provider(np); > pm_genpd_remove(&domain->genpd); > } > -- > 2.43.0 >
diff --git a/drivers/pmdomain/imx/imx93-pd.c b/drivers/pmdomain/imx/imx93-pd.c index 44daecbe5cc3..fb53a8e359bc 100644 --- a/drivers/pmdomain/imx/imx93-pd.c +++ b/drivers/pmdomain/imx/imx93-pd.c @@ -90,9 +90,6 @@ static void imx93_pd_remove(struct platform_device *pdev) struct device *dev = &pdev->dev; struct device_node *np = dev->of_node; - if (!domain->init_off) - clk_bulk_disable_unprepare(domain->num_clks, domain->clks); - of_genpd_del_provider(np); pm_genpd_remove(&domain->genpd); }
The removed code was added to handle the case where the power domain is already on during the driver's probing. In this use case, the "is_off" parameter is passed as false to pm_genpd_init() to inform it not to call the power_on() callback, as it's unnecessary to perform the hardware power-on procedure since the power domain is already on. Therefore, with the call to clk_bulk_prepare_enable() by probe(), the system is in the same operational state as when "is_off" is passed as true after the power_on() callback execution: probe() -> is_off == true -> clk_bulk_prepare_enable() called by power_on() probe() -> is_off == false -> clk_bulk_prepare_enable() called by probe() Reaching the same logical and operational state, it follows that upon driver removal, there is no need to perform different actions depending on the power domain's on/off state during probing. Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com> --- (no changes since v1) drivers/pmdomain/imx/imx93-pd.c | 3 --- 1 file changed, 3 deletions(-)