diff mbox series

[v3,3/7] clk: qcom: gdsc: enable optional power domain support

Message ID 20210709173202.667820-4-dmitry.baryshkov@linaro.org
State New
Headers show
Series clk: qcom: use power-domain for sm8250's clock controllers | expand

Commit Message

Dmitry Baryshkov July 9, 2021, 5:31 p.m. UTC 
On sm8250 dispcc and videocc registers are powered up by the MMCX power
domain. Currently we use a regulator to enable this domain on demand,
however this has some consequences, as genpd code is not reentrant.

Teach Qualcomm clock controller code about setting up runtime PM and
using specified for gdsc powerup.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

---
 drivers/clk/qcom/common.c | 37 +++++++++++++++++++++++++++++++------
 drivers/clk/qcom/gdsc.c   |  4 ++++
 2 files changed, 35 insertions(+), 6 deletions(-)

-- 
2.30.2

Comments

Bjorn Andersson July 9, 2021, 6:38 p.m. UTC | #1 
On Fri 09 Jul 12:31 CDT 2021, Dmitry Baryshkov wrote:

> On sm8250 dispcc and videocc registers are powered up by the MMCX power
> domain. Currently we use a regulator to enable this domain on demand,
> however this has some consequences, as genpd code is not reentrant.
> 
> Teach Qualcomm clock controller code about setting up runtime PM and
> using specified for gdsc powerup.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/clk/qcom/common.c | 37 +++++++++++++++++++++++++++++++------
>  drivers/clk/qcom/gdsc.c   |  4 ++++
>  2 files changed, 35 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
> index 60d2a78d1395..43d8f8feeb3c 100644
> --- a/drivers/clk/qcom/common.c
> +++ b/drivers/clk/qcom/common.c
> @@ -10,6 +10,7 @@
>  #include <linux/clk-provider.h>
>  #include <linux/reset-controller.h>
>  #include <linux/of.h>
> +#include <linux/pm_runtime.h>
>  
>  #include "common.h"
>  #include "clk-rcg.h"
> @@ -224,6 +225,11 @@ static struct clk_hw *qcom_cc_clk_hw_get(struct of_phandle_args *clkspec,
>  	return cc->rclks[idx] ? &cc->rclks[idx]->hw : NULL;
>  }
>  
> +static void qcom_cc_pm_runtime_disable(void *data)
> +{
> +	pm_runtime_disable(data);
> +}
> +
>  int qcom_cc_really_probe(struct platform_device *pdev,
>  			 const struct qcom_cc_desc *desc, struct regmap *regmap)
>  {
> @@ -241,6 +247,18 @@ int qcom_cc_really_probe(struct platform_device *pdev,
>  	if (!cc)
>  		return -ENOMEM;
>  
> +	pm_runtime_enable(dev);

In turingcc-qcs404.c I'm using pm_runtime to have the clock framework
ensure that the iface clock is enabled during clock operations, so this
will result in a "unbalanced enable" warning.

> +	ret = pm_runtime_get_sync(dev);

I don't think you should wrap the entire initialization in a
pm_runtime_get_sync()/put() region. Instead follow the clock framework
and wrap gdsc initialization that needs to touch the hardware in:

	if (pm_runtime_enabled())
		pm_runtime_get_sync();

I do however think that as of this patch, when probe returns MMCX might
very well be turned off, as the only user (this driver) has pm_runtime
enabled and it's idle. So I think you should introduce the
pm_runtime_get()/put() in the gdsc functions before this patch.


To summarize, I think you should rely on the individual clock drivers to
pm_runtime_enable()/disable().

> +	if (ret < 0) {
> +		pm_runtime_put(dev);
> +		pm_runtime_disable(dev);
> +		return ret;
> +	}
> +
> +	ret = devm_add_action_or_reset(dev, qcom_cc_pm_runtime_disable, dev);
> +	if (ret)
> +		goto err;
> +
>  	reset = &cc->reset;
>  	reset->rcdev.of_node = dev->of_node;
>  	reset->rcdev.ops = &qcom_reset_ops;
> @@ -251,7 +269,7 @@ int qcom_cc_really_probe(struct platform_device *pdev,
>  
>  	ret = devm_reset_controller_register(dev, &reset->rcdev);
>  	if (ret)
> -		return ret;
> +		goto err;
>  
>  	if (desc->gdscs && desc->num_gdscs) {
>  		scd = devm_kzalloc(dev, sizeof(*scd), GFP_KERNEL);
> @@ -262,11 +280,11 @@ int qcom_cc_really_probe(struct platform_device *pdev,
>  		scd->num = desc->num_gdscs;
>  		ret = gdsc_register(scd, &reset->rcdev, regmap);
>  		if (ret)
> -			return ret;
> +			goto err;
>  		ret = devm_add_action_or_reset(dev, qcom_cc_gdsc_unregister,
>  					       scd);
>  		if (ret)
> -			return ret;
> +			goto err;
>  	}
>  
>  	cc->rclks = rclks;
> @@ -277,7 +295,7 @@ int qcom_cc_really_probe(struct platform_device *pdev,
>  	for (i = 0; i < num_clk_hws; i++) {
>  		ret = devm_clk_hw_register(dev, clk_hws[i]);
>  		if (ret)
> -			return ret;
> +			goto err;
>  	}
>  
>  	for (i = 0; i < num_clks; i++) {
> @@ -286,14 +304,21 @@ int qcom_cc_really_probe(struct platform_device *pdev,
>  
>  		ret = devm_clk_register_regmap(dev, rclks[i]);
>  		if (ret)
> -			return ret;
> +			goto err;
>  	}
>  
>  	ret = devm_of_clk_add_hw_provider(dev, qcom_cc_clk_hw_get, cc);
>  	if (ret)
> -		return ret;
> +		goto err;
> +
> +	pm_runtime_put(dev);
>  
>  	return 0;
> +
> +err:
> +	pm_runtime_put(dev);
> +
> +	return ret;
>  }
>  EXPORT_SYMBOL_GPL(qcom_cc_really_probe);
>  
> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
> index 51ed640e527b..ccd36617d067 100644
> --- a/drivers/clk/qcom/gdsc.c
> +++ b/drivers/clk/qcom/gdsc.c
> @@ -439,6 +439,8 @@ int gdsc_register(struct gdsc_desc *desc,
>  			continue;
>  		if (scs[i]->parent)
>  			pm_genpd_add_subdomain(scs[i]->parent, &scs[i]->pd);
> +		else if (!IS_ERR_OR_NULL(dev->pm_domain))
> +			pm_genpd_add_subdomain(pd_to_genpd(dev->pm_domain), &scs[i]->pd);

Nice, I didn't know that we could fish it out of the dev.

Regards,
Bjorn

>  	}
>  
>  	return of_genpd_add_provider_onecell(dev->of_node, data);
> @@ -457,6 +459,8 @@ void gdsc_unregister(struct gdsc_desc *desc)
>  			continue;
>  		if (scs[i]->parent)
>  			pm_genpd_remove_subdomain(scs[i]->parent, &scs[i]->pd);
> +		else if (!IS_ERR_OR_NULL(dev->pm_domain))
> +			pm_genpd_remove_subdomain(pd_to_genpd(dev->pm_domain), &scs[i]->pd);
>  	}
>  	of_genpd_del_provider(dev->of_node);
>  }
> -- 
> 2.30.2
>
Dmitry Baryshkov July 9, 2021, 9:28 p.m. UTC | #2 
On 09/07/2021 21:38, Bjorn Andersson wrote:
> On Fri 09 Jul 12:31 CDT 2021, Dmitry Baryshkov wrote:
> 
>> On sm8250 dispcc and videocc registers are powered up by the MMCX power
>> domain. Currently we use a regulator to enable this domain on demand,
>> however this has some consequences, as genpd code is not reentrant.
>>
>> Teach Qualcomm clock controller code about setting up runtime PM and
>> using specified for gdsc powerup.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>>   drivers/clk/qcom/common.c | 37 +++++++++++++++++++++++++++++++------
>>   drivers/clk/qcom/gdsc.c   |  4 ++++
>>   2 files changed, 35 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
>> index 60d2a78d1395..43d8f8feeb3c 100644
>> --- a/drivers/clk/qcom/common.c
>> +++ b/drivers/clk/qcom/common.c
>> @@ -10,6 +10,7 @@
>>   #include <linux/clk-provider.h>
>>   #include <linux/reset-controller.h>
>>   #include <linux/of.h>
>> +#include <linux/pm_runtime.h>
>>   
>>   #include "common.h"
>>   #include "clk-rcg.h"
>> @@ -224,6 +225,11 @@ static struct clk_hw *qcom_cc_clk_hw_get(struct of_phandle_args *clkspec,
>>   	return cc->rclks[idx] ? &cc->rclks[idx]->hw : NULL;
>>   }
>>   
>> +static void qcom_cc_pm_runtime_disable(void *data)
>> +{
>> +	pm_runtime_disable(data);
>> +}
>> +
>>   int qcom_cc_really_probe(struct platform_device *pdev,
>>   			 const struct qcom_cc_desc *desc, struct regmap *regmap)
>>   {
>> @@ -241,6 +247,18 @@ int qcom_cc_really_probe(struct platform_device *pdev,
>>   	if (!cc)
>>   		return -ENOMEM;
>>   
>> +	pm_runtime_enable(dev);
> 
> In turingcc-qcs404.c I'm using pm_runtime to have the clock framework
> ensure that the iface clock is enabled during clock operations, so this
> will result in a "unbalanced enable" warning.

And later I register the disabler:

  ret = devm_add_action_or_reset(dev, qcom_cc_pm_runtime_disable, dev);

You might want to add this to qcs404 code.

> 
>> +	ret = pm_runtime_get_sync(dev);
> 
> I don't think you should wrap the entire initialization in a
> pm_runtime_get_sync()/put() region. Instead follow the clock framework
> and wrap gdsc initialization that needs to touch the hardware in:

Init should be wrapped in the pm_runtime_get/put calls, so that the MMCX 
domain is on through the fall init sequence. Otherwise it can get turned 
off during it, boom, failed register access and reboot.

> 
> 	if (pm_runtime_enabled())
> 		pm_runtime_get_sync();

I don't think it's worth doing that. Having single lock for the whole 
init sequence is safer (and cleaner).

If you check other pm-enabled drivers, they would either call 
pm_runtime_enable at the end of the probe or get_sync in the beginning 
of the probe and put_FOO in the end. In this driver calling 
pm_runtime_enable() at the end of the probe function will not work, 
since this way clk subsystem will not pick up the device for runtime 
power management (as pm_runtime_enabled() would return false).

> 
> I do however think that as of this patch, when probe returns MMCX might
> very well be turned off, as the only user (this driver) has pm_runtime
> enabled and it's idle. So I think you should introduce the
> pm_runtime_get()/put() in the gdsc functions before this patch.

Maybe I'd just squash them together.

> 
> 
> To summarize, I think you should rely on the individual clock drivers to
> pm_runtime_enable()/disable().
> 
>> +	if (ret < 0) {
>> +		pm_runtime_put(dev);
>> +		pm_runtime_disable(dev);
>> +		return ret;
>> +	}
>> +
>> +	ret = devm_add_action_or_reset(dev, qcom_cc_pm_runtime_disable, dev);
>> +	if (ret)
>> +		goto err;
>> +
>>   	reset = &cc->reset;
>>   	reset->rcdev.of_node = dev->of_node;
>>   	reset->rcdev.ops = &qcom_reset_ops;
>> @@ -251,7 +269,7 @@ int qcom_cc_really_probe(struct platform_device *pdev,
>>   
>>   	ret = devm_reset_controller_register(dev, &reset->rcdev);
>>   	if (ret)
>> -		return ret;
>> +		goto err;
>>   
>>   	if (desc->gdscs && desc->num_gdscs) {
>>   		scd = devm_kzalloc(dev, sizeof(*scd), GFP_KERNEL);
>> @@ -262,11 +280,11 @@ int qcom_cc_really_probe(struct platform_device *pdev,
>>   		scd->num = desc->num_gdscs;
>>   		ret = gdsc_register(scd, &reset->rcdev, regmap);
>>   		if (ret)
>> -			return ret;
>> +			goto err;
>>   		ret = devm_add_action_or_reset(dev, qcom_cc_gdsc_unregister,
>>   					       scd);
>>   		if (ret)
>> -			return ret;
>> +			goto err;
>>   	}
>>   
>>   	cc->rclks = rclks;
>> @@ -277,7 +295,7 @@ int qcom_cc_really_probe(struct platform_device *pdev,
>>   	for (i = 0; i < num_clk_hws; i++) {
>>   		ret = devm_clk_hw_register(dev, clk_hws[i]);
>>   		if (ret)
>> -			return ret;
>> +			goto err;
>>   	}
>>   
>>   	for (i = 0; i < num_clks; i++) {
>> @@ -286,14 +304,21 @@ int qcom_cc_really_probe(struct platform_device *pdev,
>>   
>>   		ret = devm_clk_register_regmap(dev, rclks[i]);
>>   		if (ret)
>> -			return ret;
>> +			goto err;
>>   	}
>>   
>>   	ret = devm_of_clk_add_hw_provider(dev, qcom_cc_clk_hw_get, cc);
>>   	if (ret)
>> -		return ret;
>> +		goto err;
>> +
>> +	pm_runtime_put(dev);
>>   
>>   	return 0;
>> +
>> +err:
>> +	pm_runtime_put(dev);
>> +
>> +	return ret;
>>   }
>>   EXPORT_SYMBOL_GPL(qcom_cc_really_probe);
>>   
>> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
>> index 51ed640e527b..ccd36617d067 100644
>> --- a/drivers/clk/qcom/gdsc.c
>> +++ b/drivers/clk/qcom/gdsc.c
>> @@ -439,6 +439,8 @@ int gdsc_register(struct gdsc_desc *desc,
>>   			continue;
>>   		if (scs[i]->parent)
>>   			pm_genpd_add_subdomain(scs[i]->parent, &scs[i]->pd);
>> +		else if (!IS_ERR_OR_NULL(dev->pm_domain))
>> +			pm_genpd_add_subdomain(pd_to_genpd(dev->pm_domain), &scs[i]->pd);
> 
> Nice, I didn't know that we could fish it out of the dev.
> 
> Regards,
> Bjorn
> 
>>   	}
>>   
>>   	return of_genpd_add_provider_onecell(dev->of_node, data);
>> @@ -457,6 +459,8 @@ void gdsc_unregister(struct gdsc_desc *desc)
>>   			continue;
>>   		if (scs[i]->parent)
>>   			pm_genpd_remove_subdomain(scs[i]->parent, &scs[i]->pd);
>> +		else if (!IS_ERR_OR_NULL(dev->pm_domain))
>> +			pm_genpd_remove_subdomain(pd_to_genpd(dev->pm_domain), &scs[i]->pd);
>>   	}
>>   	of_genpd_del_provider(dev->of_node);
>>   }
>> -- 
>> 2.30.2
>>
Bjorn Andersson July 10, 2021, 12:48 a.m. UTC | #3 
On Fri 09 Jul 16:28 CDT 2021, Dmitry Baryshkov wrote:

> On 09/07/2021 21:38, Bjorn Andersson wrote:

> > On Fri 09 Jul 12:31 CDT 2021, Dmitry Baryshkov wrote:

> > 

> > > On sm8250 dispcc and videocc registers are powered up by the MMCX power

> > > domain. Currently we use a regulator to enable this domain on demand,

> > > however this has some consequences, as genpd code is not reentrant.

> > > 

> > > Teach Qualcomm clock controller code about setting up runtime PM and

> > > using specified for gdsc powerup.

> > > 

> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

> > > ---

> > >   drivers/clk/qcom/common.c | 37 +++++++++++++++++++++++++++++++------

> > >   drivers/clk/qcom/gdsc.c   |  4 ++++

> > >   2 files changed, 35 insertions(+), 6 deletions(-)

> > > 

> > > diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c

> > > index 60d2a78d1395..43d8f8feeb3c 100644

> > > --- a/drivers/clk/qcom/common.c

> > > +++ b/drivers/clk/qcom/common.c

> > > @@ -10,6 +10,7 @@

> > >   #include <linux/clk-provider.h>

> > >   #include <linux/reset-controller.h>

> > >   #include <linux/of.h>

> > > +#include <linux/pm_runtime.h>

> > >   #include "common.h"

> > >   #include "clk-rcg.h"

> > > @@ -224,6 +225,11 @@ static struct clk_hw *qcom_cc_clk_hw_get(struct of_phandle_args *clkspec,

> > >   	return cc->rclks[idx] ? &cc->rclks[idx]->hw : NULL;

> > >   }

> > > +static void qcom_cc_pm_runtime_disable(void *data)

> > > +{

> > > +	pm_runtime_disable(data);

> > > +}

> > > +

> > >   int qcom_cc_really_probe(struct platform_device *pdev,

> > >   			 const struct qcom_cc_desc *desc, struct regmap *regmap)

> > >   {

> > > @@ -241,6 +247,18 @@ int qcom_cc_really_probe(struct platform_device *pdev,

> > >   	if (!cc)

> > >   		return -ENOMEM;

> > > +	pm_runtime_enable(dev);

> > 

> > In turingcc-qcs404.c I'm using pm_runtime to have the clock framework

> > ensure that the iface clock is enabled during clock operations, so this

> > will result in a "unbalanced enable" warning.

> 

> And later I register the disabler:

> 

>  ret = devm_add_action_or_reset(dev, qcom_cc_pm_runtime_disable, dev);

> 

> You might want to add this to qcs404 code.

> 


Let's land it using the apis that exist, then I think there's plenty of
examples throughout the kernel to make a case for introducing
devm_pm_clk_create() and devm_pm_runtime_enable().

But introducing that would complicate the path your patches would have
to take towards mainline.

> > 

> > > +	ret = pm_runtime_get_sync(dev);

> > 

> > I don't think you should wrap the entire initialization in a

> > pm_runtime_get_sync()/put() region. Instead follow the clock framework

> > and wrap gdsc initialization that needs to touch the hardware in:

> 

> Init should be wrapped in the pm_runtime_get/put calls, so that the MMCX

> domain is on through the fall init sequence. Otherwise it can get turned off

> during it, boom, failed register access and reboot.

> 


Right, we need to wrap init in pm_runtime_get()/put(), to ensure that
the registers are accessible. But the clock code has these surrounding
the initialization and therefor I think you should do the same in
gdsc_init() - instead of here.

> > 

> > 	if (pm_runtime_enabled())

> > 		pm_runtime_get_sync();

> 

> I don't think it's worth doing that. Having single lock for the whole init

> sequence is safer (and cleaner).

> 


Right, you need to cache the pm_runtime_enabled(), just as is done with
core->rpm_enabled in the clock framework.

The majority of the code involved in dispcc's initialization already has
more granular pm_runtime_get()/put() sections, so I don't think you
should override that with a big section here.

> If you check other pm-enabled drivers, they would either call

> pm_runtime_enable at the end of the probe or get_sync in the beginning of

> the probe and put_FOO in the end. In this driver calling pm_runtime_enable()

> at the end of the probe function will not work, since this way clk subsystem

> will not pick up the device for runtime power management (as

> pm_runtime_enabled() would return false).

> 


Right, just as turingcc does, we need to call pm_runtime_enable() early
and gdsc needs to check during initialization if pm_runtime should be
used for the particular clock controller.

> > 

> > I do however think that as of this patch, when probe returns MMCX might

> > very well be turned off, as the only user (this driver) has pm_runtime

> > enabled and it's idle. So I think you should introduce the

> > pm_runtime_get()/put() in the gdsc functions before this patch.

> 

> Maybe I'd just squash them together.

> 


With what I suggest the two patches are reduced to adding pm_runtime
support in the gdsc driver, and then a separate patch adding
pm_runtime_enable() in dispcc and videocc.

> > 

> > 

> > To summarize, I think you should rely on the individual clock drivers to

> > pm_runtime_enable()/disable().

> > 

> > > +	if (ret < 0) {

> > > +		pm_runtime_put(dev);

> > > +		pm_runtime_disable(dev);

> > > +		return ret;

> > > +	}

> > > +

> > > +	ret = devm_add_action_or_reset(dev, qcom_cc_pm_runtime_disable, dev);

> > > +	if (ret)

> > > +		goto err;

> > > +

> > >   	reset = &cc->reset;

> > >   	reset->rcdev.of_node = dev->of_node;

> > >   	reset->rcdev.ops = &qcom_reset_ops;

> > > @@ -251,7 +269,7 @@ int qcom_cc_really_probe(struct platform_device *pdev,

> > >   	ret = devm_reset_controller_register(dev, &reset->rcdev);

> > >   	if (ret)

> > > -		return ret;

> > > +		goto err;

> > >   	if (desc->gdscs && desc->num_gdscs) {

> > >   		scd = devm_kzalloc(dev, sizeof(*scd), GFP_KERNEL);

> > > @@ -262,11 +280,11 @@ int qcom_cc_really_probe(struct platform_device *pdev,

> > >   		scd->num = desc->num_gdscs;

> > >   		ret = gdsc_register(scd, &reset->rcdev, regmap);

> > >   		if (ret)

> > > -			return ret;

> > > +			goto err;

> > >   		ret = devm_add_action_or_reset(dev, qcom_cc_gdsc_unregister,

> > >   					       scd);

> > >   		if (ret)

> > > -			return ret;

> > > +			goto err;

> > >   	}

> > >   	cc->rclks = rclks;

> > > @@ -277,7 +295,7 @@ int qcom_cc_really_probe(struct platform_device *pdev,

> > >   	for (i = 0; i < num_clk_hws; i++) {

> > >   		ret = devm_clk_hw_register(dev, clk_hws[i]);

> > >   		if (ret)

> > > -			return ret;

> > > +			goto err;

> > >   	}

> > >   	for (i = 0; i < num_clks; i++) {

> > > @@ -286,14 +304,21 @@ int qcom_cc_really_probe(struct platform_device *pdev,

> > >   		ret = devm_clk_register_regmap(dev, rclks[i]);

> > >   		if (ret)

> > > -			return ret;

> > > +			goto err;

> > >   	}

> > >   	ret = devm_of_clk_add_hw_provider(dev, qcom_cc_clk_hw_get, cc);

> > >   	if (ret)

> > > -		return ret;

> > > +		goto err;

> > > +

> > > +	pm_runtime_put(dev);

> > >   	return 0;

> > > +

> > > +err:

> > > +	pm_runtime_put(dev);

> > > +

> > > +	return ret;

> > >   }

> > >   EXPORT_SYMBOL_GPL(qcom_cc_really_probe);

> > > diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c

> > > index 51ed640e527b..ccd36617d067 100644

> > > --- a/drivers/clk/qcom/gdsc.c

> > > +++ b/drivers/clk/qcom/gdsc.c

> > > @@ -439,6 +439,8 @@ int gdsc_register(struct gdsc_desc *desc,

> > >   			continue;

> > >   		if (scs[i]->parent)

> > >   			pm_genpd_add_subdomain(scs[i]->parent, &scs[i]->pd);

> > > +		else if (!IS_ERR_OR_NULL(dev->pm_domain))

> > > +			pm_genpd_add_subdomain(pd_to_genpd(dev->pm_domain), &scs[i]->pd);

> > 

> > Nice, I didn't know that we could fish it out of the dev.

> > 

> > Regards,

> > Bjorn

> > 

> > >   	}

> > >   	return of_genpd_add_provider_onecell(dev->of_node, data);

> > > @@ -457,6 +459,8 @@ void gdsc_unregister(struct gdsc_desc *desc)

> > >   			continue;

> > >   		if (scs[i]->parent)

> > >   			pm_genpd_remove_subdomain(scs[i]->parent, &scs[i]->pd);

> > > +		else if (!IS_ERR_OR_NULL(dev->pm_domain))

> > > +			pm_genpd_remove_subdomain(pd_to_genpd(dev->pm_domain), &scs[i]->pd);

> > >   	}

> > >   	of_genpd_del_provider(dev->of_node);

> > >   }

> > > -- 

> > > 2.30.2

> > > 

> 

> 

> -- 

> With best wishes

> Dmitry
diff mbox series

Patch

diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
index 60d2a78d1395..43d8f8feeb3c 100644
--- a/drivers/clk/qcom/common.c
+++ b/drivers/clk/qcom/common.c
@@ -10,6 +10,7 @@ 
 #include <linux/clk-provider.h>
 #include <linux/reset-controller.h>
 #include <linux/of.h>
+#include <linux/pm_runtime.h>
 
 #include "common.h"
 #include "clk-rcg.h"
@@ -224,6 +225,11 @@  static struct clk_hw *qcom_cc_clk_hw_get(struct of_phandle_args *clkspec,
 	return cc->rclks[idx] ? &cc->rclks[idx]->hw : NULL;
 }
 
+static void qcom_cc_pm_runtime_disable(void *data)
+{
+	pm_runtime_disable(data);
+}
+
 int qcom_cc_really_probe(struct platform_device *pdev,
 			 const struct qcom_cc_desc *desc, struct regmap *regmap)
 {
@@ -241,6 +247,18 @@  int qcom_cc_really_probe(struct platform_device *pdev,
 	if (!cc)
 		return -ENOMEM;
 
+	pm_runtime_enable(dev);
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0) {
+		pm_runtime_put(dev);
+		pm_runtime_disable(dev);
+		return ret;
+	}
+
+	ret = devm_add_action_or_reset(dev, qcom_cc_pm_runtime_disable, dev);
+	if (ret)
+		goto err;
+
 	reset = &cc->reset;
 	reset->rcdev.of_node = dev->of_node;
 	reset->rcdev.ops = &qcom_reset_ops;
@@ -251,7 +269,7 @@  int qcom_cc_really_probe(struct platform_device *pdev,
 
 	ret = devm_reset_controller_register(dev, &reset->rcdev);
 	if (ret)
-		return ret;
+		goto err;
 
 	if (desc->gdscs && desc->num_gdscs) {
 		scd = devm_kzalloc(dev, sizeof(*scd), GFP_KERNEL);
@@ -262,11 +280,11 @@  int qcom_cc_really_probe(struct platform_device *pdev,
 		scd->num = desc->num_gdscs;
 		ret = gdsc_register(scd, &reset->rcdev, regmap);
 		if (ret)
-			return ret;
+			goto err;
 		ret = devm_add_action_or_reset(dev, qcom_cc_gdsc_unregister,
 					       scd);
 		if (ret)
-			return ret;
+			goto err;
 	}
 
 	cc->rclks = rclks;
@@ -277,7 +295,7 @@  int qcom_cc_really_probe(struct platform_device *pdev,
 	for (i = 0; i < num_clk_hws; i++) {
 		ret = devm_clk_hw_register(dev, clk_hws[i]);
 		if (ret)
-			return ret;
+			goto err;
 	}
 
 	for (i = 0; i < num_clks; i++) {
@@ -286,14 +304,21 @@  int qcom_cc_really_probe(struct platform_device *pdev,
 
 		ret = devm_clk_register_regmap(dev, rclks[i]);
 		if (ret)
-			return ret;
+			goto err;
 	}
 
 	ret = devm_of_clk_add_hw_provider(dev, qcom_cc_clk_hw_get, cc);
 	if (ret)
-		return ret;
+		goto err;
+
+	pm_runtime_put(dev);
 
 	return 0;
+
+err:
+	pm_runtime_put(dev);
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(qcom_cc_really_probe);
 
diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
index 51ed640e527b..ccd36617d067 100644
--- a/drivers/clk/qcom/gdsc.c
+++ b/drivers/clk/qcom/gdsc.c
@@ -439,6 +439,8 @@  int gdsc_register(struct gdsc_desc *desc,
 			continue;
 		if (scs[i]->parent)
 			pm_genpd_add_subdomain(scs[i]->parent, &scs[i]->pd);
+		else if (!IS_ERR_OR_NULL(dev->pm_domain))
+			pm_genpd_add_subdomain(pd_to_genpd(dev->pm_domain), &scs[i]->pd);
 	}
 
 	return of_genpd_add_provider_onecell(dev->of_node, data);
@@ -457,6 +459,8 @@  void gdsc_unregister(struct gdsc_desc *desc)
 			continue;
 		if (scs[i]->parent)
 			pm_genpd_remove_subdomain(scs[i]->parent, &scs[i]->pd);
+		else if (!IS_ERR_OR_NULL(dev->pm_domain))
+			pm_genpd_remove_subdomain(pd_to_genpd(dev->pm_domain), &scs[i]->pd);
 	}
 	of_genpd_del_provider(dev->of_node);
 }