diff mbox series

[3/5] clk: qcom: common: Attach clock power domains conditionally

Message ID 20250218-videocc-pll-multi-pd-voting-v1-3-cfe6289ea29b@quicinc.com (mailing list archive)
State New
Headers show
Series clk: qcom: Add support to attach multiple power domains in cc probe | expand

Commit Message

Jagadeesh Kona Feb. 18, 2025, 2:26 p.m. UTC
Attach clock power domains in qcom_cc_really_probe() only
if the clock controller has not already attached to them.

Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
---
 drivers/clk/qcom/common.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Dmitry Baryshkov Feb. 18, 2025, 5:18 p.m. UTC | #1
On Tue, Feb 18, 2025 at 07:56:48PM +0530, Jagadeesh Kona wrote:
> Attach clock power domains in qcom_cc_really_probe() only
> if the clock controller has not already attached to them.

Squash this to the previous patch and call the new function. No need to
duplicate the code.

> 
> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> ---
>  drivers/clk/qcom/common.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
> index ec27f70b24bdec24edd2f6b3df0d766fc1cdcbf0..eb7e2a56d1d135f839fd9bd470ba6231ce775a8c 100644
> --- a/drivers/clk/qcom/common.c
> +++ b/drivers/clk/qcom/common.c
> @@ -300,9 +300,12 @@ int qcom_cc_really_probe(struct device *dev,
>  	if (!cc)
>  		return -ENOMEM;
>  
> -	ret = devm_pm_domain_attach_list(dev, NULL, &cc->pd_list);
> -	if (ret < 0 && ret != -EEXIST)
> -		return ret;
> +	cc->pd_list = desc->pd_list;
> +	if (!cc->pd_list) {
> +		ret = devm_pm_domain_attach_list(dev, NULL, &cc->pd_list);
> +		if (ret < 0 && ret != -EEXIST)
> +			return ret;
> +	}
>  
>  	reset = &cc->reset;
>  	reset->rcdev.of_node = dev->of_node;
> 
> -- 
> 2.34.1
>
Jagadeesh Kona Feb. 19, 2025, 11:36 a.m. UTC | #2
On 2/18/2025 10:48 PM, Dmitry Baryshkov wrote:
> On Tue, Feb 18, 2025 at 07:56:48PM +0530, Jagadeesh Kona wrote:
>> Attach clock power domains in qcom_cc_really_probe() only
>> if the clock controller has not already attached to them.
> 
> Squash this to the previous patch and call the new function. No need to
> duplicate the code.
> 

I tried calling the new function here instead of duplicating code, but that
is leading to below warning since the desc passed to qcom_cc_really_probe()
has a const qualifier and hence we cannot update desc->pd_list inside
qcom_cc_really_probe().

drivers/clk/qcom/common.c:305:33:   WARNING : passing argument 2 of ‘qcom_cc_attach_pds’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]

Thanks,
Jagadeesh

>>
>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>> ---
>>  drivers/clk/qcom/common.c | 9 ++++++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
>> index ec27f70b24bdec24edd2f6b3df0d766fc1cdcbf0..eb7e2a56d1d135f839fd9bd470ba6231ce775a8c 100644
>> --- a/drivers/clk/qcom/common.c
>> +++ b/drivers/clk/qcom/common.c
>> @@ -300,9 +300,12 @@ int qcom_cc_really_probe(struct device *dev,
>>  	if (!cc)
>>  		return -ENOMEM;
>>  
>> -	ret = devm_pm_domain_attach_list(dev, NULL, &cc->pd_list);
>> -	if (ret < 0 && ret != -EEXIST)
>> -		return ret;
>> +	cc->pd_list = desc->pd_list;
>> +	if (!cc->pd_list) {
>> +		ret = devm_pm_domain_attach_list(dev, NULL, &cc->pd_list);
>> +		if (ret < 0 && ret != -EEXIST)
>> +			return ret;
>> +	}
>>  
>>  	reset = &cc->reset;
>>  	reset->rcdev.of_node = dev->of_node;
>>
>> -- 
>> 2.34.1
>>
>
Dmitry Baryshkov Feb. 19, 2025, 11:57 a.m. UTC | #3
On Wed, Feb 19, 2025 at 05:06:11PM +0530, Jagadeesh Kona wrote:
> 
> 
> On 2/18/2025 10:48 PM, Dmitry Baryshkov wrote:
> > On Tue, Feb 18, 2025 at 07:56:48PM +0530, Jagadeesh Kona wrote:
> >> Attach clock power domains in qcom_cc_really_probe() only
> >> if the clock controller has not already attached to them.
> > 
> > Squash this to the previous patch and call the new function. No need to
> > duplicate the code.
> > 
> 
> I tried calling the new function here instead of duplicating code, but that
> is leading to below warning since the desc passed to qcom_cc_really_probe()
> has a const qualifier and hence we cannot update desc->pd_list inside
> qcom_cc_really_probe().
> 
> drivers/clk/qcom/common.c:305:33:   WARNING : passing argument 2 of ‘qcom_cc_attach_pds’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]

It sounds like this can be fixed with a one-line patch.

> 
> Thanks,
> Jagadeesh
> 
> >>
> >> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> >> ---
> >>  drivers/clk/qcom/common.c | 9 ++++++---
> >>  1 file changed, 6 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
> >> index ec27f70b24bdec24edd2f6b3df0d766fc1cdcbf0..eb7e2a56d1d135f839fd9bd470ba6231ce775a8c 100644
> >> --- a/drivers/clk/qcom/common.c
> >> +++ b/drivers/clk/qcom/common.c
> >> @@ -300,9 +300,12 @@ int qcom_cc_really_probe(struct device *dev,
> >>  	if (!cc)
> >>  		return -ENOMEM;
> >>  
> >> -	ret = devm_pm_domain_attach_list(dev, NULL, &cc->pd_list);
> >> -	if (ret < 0 && ret != -EEXIST)
> >> -		return ret;
> >> +	cc->pd_list = desc->pd_list;
> >> +	if (!cc->pd_list) {
> >> +		ret = devm_pm_domain_attach_list(dev, NULL, &cc->pd_list);
> >> +		if (ret < 0 && ret != -EEXIST)
> >> +			return ret;
> >> +	}
> >>  
> >>  	reset = &cc->reset;
> >>  	reset->rcdev.of_node = dev->of_node;
> >>
> >> -- 
> >> 2.34.1
> >>
> >
Jagadeesh Kona Feb. 20, 2025, 7:13 a.m. UTC | #4
On 2/19/2025 5:27 PM, Dmitry Baryshkov wrote:
> On Wed, Feb 19, 2025 at 05:06:11PM +0530, Jagadeesh Kona wrote:
>>
>>
>> On 2/18/2025 10:48 PM, Dmitry Baryshkov wrote:
>>> On Tue, Feb 18, 2025 at 07:56:48PM +0530, Jagadeesh Kona wrote:
>>>> Attach clock power domains in qcom_cc_really_probe() only
>>>> if the clock controller has not already attached to them.
>>>
>>> Squash this to the previous patch and call the new function. No need to
>>> duplicate the code.
>>>
>>
>> I tried calling the new function here instead of duplicating code, but that
>> is leading to below warning since the desc passed to qcom_cc_really_probe()
>> has a const qualifier and hence we cannot update desc->pd_list inside
>> qcom_cc_really_probe().
>>
>> drivers/clk/qcom/common.c:305:33:   WARNING : passing argument 2 of ‘qcom_cc_attach_pds’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
> 
> It sounds like this can be fixed with a one-line patch.
> 

Removing const qualifier to qcom_cc_really_probe() will fix this, but that requires changes in
many other drivers which are currently passing const descriptor to it.

But I can squash this to my previous patch by updating my qcom_cc_attach_pds() function
prototype as below and then calling that new function here

-int qcom_cc_attach_pds(struct device *dev, struct qcom_cc_desc *desc)
+int qcom_cc_attach_pds(struct device *dev, struct dev_pm_domain_list *pd_list)

-               ret = devm_pm_domain_attach_list(dev, NULL, &cc->pd_list);
-               if (ret < 0 && ret != -EEXIST)
+               ret = qcom_cc_attach_pds(dev, cc->pd_list);
+               if (ret)

Thanks,
Jagadeesh

>>
>> Thanks,
>> Jagadeesh
>>
>>>>
>>>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>>>> ---
>>>>  drivers/clk/qcom/common.c | 9 ++++++---
>>>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
>>>> index ec27f70b24bdec24edd2f6b3df0d766fc1cdcbf0..eb7e2a56d1d135f839fd9bd470ba6231ce775a8c 100644
>>>> --- a/drivers/clk/qcom/common.c
>>>> +++ b/drivers/clk/qcom/common.c
>>>> @@ -300,9 +300,12 @@ int qcom_cc_really_probe(struct device *dev,
>>>>  	if (!cc)
>>>>  		return -ENOMEM;
>>>>  
>>>> -	ret = devm_pm_domain_attach_list(dev, NULL, &cc->pd_list);
>>>> -	if (ret < 0 && ret != -EEXIST)
>>>> -		return ret;
>>>> +	cc->pd_list = desc->pd_list;
>>>> +	if (!cc->pd_list) {
>>>> +		ret = devm_pm_domain_attach_list(dev, NULL, &cc->pd_list);
>>>> +		if (ret < 0 && ret != -EEXIST)
>>>> +			return ret;
>>>> +	}
>>>>  
>>>>  	reset = &cc->reset;
>>>>  	reset->rcdev.of_node = dev->of_node;
>>>>
>>>> -- 
>>>> 2.34.1
>>>>
>>>
>
Dmitry Baryshkov Feb. 20, 2025, 10:48 a.m. UTC | #5
On Thu, Feb 20, 2025 at 12:43:42PM +0530, Jagadeesh Kona wrote:
> 
> 
> On 2/19/2025 5:27 PM, Dmitry Baryshkov wrote:
> > On Wed, Feb 19, 2025 at 05:06:11PM +0530, Jagadeesh Kona wrote:
> >>
> >>
> >> On 2/18/2025 10:48 PM, Dmitry Baryshkov wrote:
> >>> On Tue, Feb 18, 2025 at 07:56:48PM +0530, Jagadeesh Kona wrote:
> >>>> Attach clock power domains in qcom_cc_really_probe() only
> >>>> if the clock controller has not already attached to them.
> >>>
> >>> Squash this to the previous patch and call the new function. No need to
> >>> duplicate the code.
> >>>
> >>
> >> I tried calling the new function here instead of duplicating code, but that
> >> is leading to below warning since the desc passed to qcom_cc_really_probe()
> >> has a const qualifier and hence we cannot update desc->pd_list inside
> >> qcom_cc_really_probe().
> >>
> >> drivers/clk/qcom/common.c:305:33:   WARNING : passing argument 2 of ‘qcom_cc_attach_pds’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
> > 
> > It sounds like this can be fixed with a one-line patch.
> > 
> 
> Removing const qualifier to qcom_cc_really_probe() will fix this, but that requires changes in
> many other drivers which are currently passing const descriptor to it.

And this points out that the pd_list should not be a part of the
struct qcom_cc_desc. You are not using it in the code, so allocate that
memory on the fly, pass it to devm_pm_domain_attach_list() and then
forget about it.

> 
> But I can squash this to my previous patch by updating my qcom_cc_attach_pds() function
> prototype as below and then calling that new function here
> 
> -int qcom_cc_attach_pds(struct device *dev, struct qcom_cc_desc *desc)
> +int qcom_cc_attach_pds(struct device *dev, struct dev_pm_domain_list *pd_list)
> 
> -               ret = devm_pm_domain_attach_list(dev, NULL, &cc->pd_list);
> -               if (ret < 0 && ret != -EEXIST)
> +               ret = qcom_cc_attach_pds(dev, cc->pd_list);
> +               if (ret)
> 
> Thanks,
> Jagadeesh
> 
> >>
> >> Thanks,
> >> Jagadeesh
> >>
> >>>>
> >>>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> >>>> ---
> >>>>  drivers/clk/qcom/common.c | 9 ++++++---
> >>>>  1 file changed, 6 insertions(+), 3 deletions(-)
> >>>>
> >>>> diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
> >>>> index ec27f70b24bdec24edd2f6b3df0d766fc1cdcbf0..eb7e2a56d1d135f839fd9bd470ba6231ce775a8c 100644
> >>>> --- a/drivers/clk/qcom/common.c
> >>>> +++ b/drivers/clk/qcom/common.c
> >>>> @@ -300,9 +300,12 @@ int qcom_cc_really_probe(struct device *dev,
> >>>>  	if (!cc)
> >>>>  		return -ENOMEM;
> >>>>  
> >>>> -	ret = devm_pm_domain_attach_list(dev, NULL, &cc->pd_list);
> >>>> -	if (ret < 0 && ret != -EEXIST)
> >>>> -		return ret;
> >>>> +	cc->pd_list = desc->pd_list;
> >>>> +	if (!cc->pd_list) {
> >>>> +		ret = devm_pm_domain_attach_list(dev, NULL, &cc->pd_list);
> >>>> +		if (ret < 0 && ret != -EEXIST)
> >>>> +			return ret;
> >>>> +	}
> >>>>  
> >>>>  	reset = &cc->reset;
> >>>>  	reset->rcdev.of_node = dev->of_node;
> >>>>
> >>>> -- 
> >>>> 2.34.1
> >>>>
> >>>
> >
Jagadeesh Kona Feb. 21, 2025, 11:42 a.m. UTC | #6
On 2/20/2025 4:18 PM, Dmitry Baryshkov wrote:
> On Thu, Feb 20, 2025 at 12:43:42PM +0530, Jagadeesh Kona wrote:
>>
>>
>> On 2/19/2025 5:27 PM, Dmitry Baryshkov wrote:
>>> On Wed, Feb 19, 2025 at 05:06:11PM +0530, Jagadeesh Kona wrote:
>>>>
>>>>
>>>> On 2/18/2025 10:48 PM, Dmitry Baryshkov wrote:
>>>>> On Tue, Feb 18, 2025 at 07:56:48PM +0530, Jagadeesh Kona wrote:
>>>>>> Attach clock power domains in qcom_cc_really_probe() only
>>>>>> if the clock controller has not already attached to them.
>>>>>
>>>>> Squash this to the previous patch and call the new function. No need to
>>>>> duplicate the code.
>>>>>
>>>>
>>>> I tried calling the new function here instead of duplicating code, but that
>>>> is leading to below warning since the desc passed to qcom_cc_really_probe()
>>>> has a const qualifier and hence we cannot update desc->pd_list inside
>>>> qcom_cc_really_probe().
>>>>
>>>> drivers/clk/qcom/common.c:305:33:   WARNING : passing argument 2 of ‘qcom_cc_attach_pds’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
>>>
>>> It sounds like this can be fixed with a one-line patch.
>>>
>>
>> Removing const qualifier to qcom_cc_really_probe() will fix this, but that requires changes in
>> many other drivers which are currently passing const descriptor to it.
> 
> And this points out that the pd_list should not be a part of the
> struct qcom_cc_desc. You are not using it in the code, so allocate that
> memory on the fly, pass it to devm_pm_domain_attach_list() and then
> forget about it.
> 

Above suggestion looks good, but we need to store the pd_list to pass it to GDSC driver to attach
the power domains as GDSC parent domains. Instead, we can add a new API wrapper for attaching PDs
and to map the regmap(qcom_cc_attach_pds_map) and all clock drivers that need multiple power domains
support can update to use below new API and all new clock drivers can just use the new API.

The implementation would be something like below

--- a/drivers/clk/qcom/common.c
+++ b/drivers/clk/qcom/common.c
+struct regmap *qcom_cc_attach_pds_map(struct platform_device *pdev, struct qcom_cc_desc *desc)
+{
+       int ret;
+
+       ret = devm_pm_domain_attach_list(&pdev->dev, NULL, &desc->pd_list);
+       if (ret < 0 && ret != -EEXIST)
+               return ERR_PTR(ret);
+
+       return qcom_cc_map(pdev, desc);
+}
+EXPORT_SYMBOL_GPL(qcom_cc_attach_pds_map);
+


--- a/drivers/clk/qcom/videocc-sm8550.c
+++ b/drivers/clk/qcom/videocc-sm8550.c
@@ -542,6 +542,12 @@ static int video_cc_sm8550_probe(struct platform_device *pdev)
        int ret;
        u32 sleep_clk_offset = 0x8140;

+       regmap = qcom_cc_attach_pds_map(pdev, &video_cc_sm8550_desc);
+       if (IS_ERR(regmap)) {
+               pm_runtime_put(&pdev->dev);
+               return PTR_ERR(regmap);
+       }
+
        ret = devm_pm_runtime_enable(&pdev->dev);
        if (ret)
                return ret;
@@ -550,12 +556,6 @@ static int video_cc_sm8550_probe(struct platform_device *pdev)
        if (ret)
                return ret;

-       regmap = qcom_cc_map(pdev, &video_cc_sm8550_desc);
-       if (IS_ERR(regmap)) {
-               pm_runtime_put(&pdev->dev);
-               return PTR_ERR(regmap);
-       }
-

This way also, we are aligning more towards common code and the code will be uniform across all
clock drivers and this doesn't require separate callback in each individual clock driver.

Thanks,
Jagadeesh 

>>
>> But I can squash this to my previous patch by updating my qcom_cc_attach_pds() function
>> prototype as below and then calling that new function here
>>
>> -int qcom_cc_attach_pds(struct device *dev, struct qcom_cc_desc *desc)
>> +int qcom_cc_attach_pds(struct device *dev, struct dev_pm_domain_list *pd_list)
>>
>> -               ret = devm_pm_domain_attach_list(dev, NULL, &cc->pd_list);
>> -               if (ret < 0 && ret != -EEXIST)
>> +               ret = qcom_cc_attach_pds(dev, cc->pd_list);
>> +               if (ret)
>>
>> Thanks,
>> Jagadeesh
>>
>>>>
>>>> Thanks,
>>>> Jagadeesh
>>>>
>>>>>>
>>>>>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>>>>>> ---
>>>>>>  drivers/clk/qcom/common.c | 9 ++++++---
>>>>>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
>>>>>> index ec27f70b24bdec24edd2f6b3df0d766fc1cdcbf0..eb7e2a56d1d135f839fd9bd470ba6231ce775a8c 100644
>>>>>> --- a/drivers/clk/qcom/common.c
>>>>>> +++ b/drivers/clk/qcom/common.c
>>>>>> @@ -300,9 +300,12 @@ int qcom_cc_really_probe(struct device *dev,
>>>>>>  	if (!cc)
>>>>>>  		return -ENOMEM;
>>>>>>  
>>>>>> -	ret = devm_pm_domain_attach_list(dev, NULL, &cc->pd_list);
>>>>>> -	if (ret < 0 && ret != -EEXIST)
>>>>>> -		return ret;
>>>>>> +	cc->pd_list = desc->pd_list;
>>>>>> +	if (!cc->pd_list) {
>>>>>> +		ret = devm_pm_domain_attach_list(dev, NULL, &cc->pd_list);
>>>>>> +		if (ret < 0 && ret != -EEXIST)
>>>>>> +			return ret;
>>>>>> +	}
>>>>>>  
>>>>>>  	reset = &cc->reset;
>>>>>>  	reset->rcdev.of_node = dev->of_node;
>>>>>>
>>>>>> -- 
>>>>>> 2.34.1
>>>>>>
>>>>>
>>>
>
Dmitry Baryshkov Feb. 21, 2025, 1:35 p.m. UTC | #7
On Fri, Feb 21, 2025 at 05:12:58PM +0530, Jagadeesh Kona wrote:
> 
> 
> On 2/20/2025 4:18 PM, Dmitry Baryshkov wrote:
> > On Thu, Feb 20, 2025 at 12:43:42PM +0530, Jagadeesh Kona wrote:
> >>
> >>
> >> On 2/19/2025 5:27 PM, Dmitry Baryshkov wrote:
> >>> On Wed, Feb 19, 2025 at 05:06:11PM +0530, Jagadeesh Kona wrote:
> >>>>
> >>>>
> >>>> On 2/18/2025 10:48 PM, Dmitry Baryshkov wrote:
> >>>>> On Tue, Feb 18, 2025 at 07:56:48PM +0530, Jagadeesh Kona wrote:
> >>>>>> Attach clock power domains in qcom_cc_really_probe() only
> >>>>>> if the clock controller has not already attached to them.
> >>>>>
> >>>>> Squash this to the previous patch and call the new function. No need to
> >>>>> duplicate the code.
> >>>>>
> >>>>
> >>>> I tried calling the new function here instead of duplicating code, but that
> >>>> is leading to below warning since the desc passed to qcom_cc_really_probe()
> >>>> has a const qualifier and hence we cannot update desc->pd_list inside
> >>>> qcom_cc_really_probe().
> >>>>
> >>>> drivers/clk/qcom/common.c:305:33:   WARNING : passing argument 2 of ‘qcom_cc_attach_pds’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
> >>>
> >>> It sounds like this can be fixed with a one-line patch.
> >>>
> >>
> >> Removing const qualifier to qcom_cc_really_probe() will fix this, but that requires changes in
> >> many other drivers which are currently passing const descriptor to it.
> > 
> > And this points out that the pd_list should not be a part of the
> > struct qcom_cc_desc. You are not using it in the code, so allocate that
> > memory on the fly, pass it to devm_pm_domain_attach_list() and then
> > forget about it.
> > 
> 
> Above suggestion looks good, but we need to store the pd_list to pass it to GDSC driver to attach
> the power domains as GDSC parent domains. Instead, we can add a new API wrapper for attaching PDs
> and to map the regmap(qcom_cc_attach_pds_map) and all clock drivers that need multiple power domains
> support can update to use below new API and all new clock drivers can just use the new API.
> 
> The implementation would be something like below
> 
> --- a/drivers/clk/qcom/common.c
> +++ b/drivers/clk/qcom/common.c
> +struct regmap *qcom_cc_attach_pds_map(struct platform_device *pdev, struct qcom_cc_desc *desc)

I think it was established that qcom_cc_desc should be read-only. In the
end it is a description. If you want to pass this data to gdsc probing,
add new argument to qcom_cc_really_probe().

> +{
> +       int ret;
> +
> +       ret = devm_pm_domain_attach_list(&pdev->dev, NULL, &desc->pd_list);
> +       if (ret < 0 && ret != -EEXIST)
> +               return ERR_PTR(ret);
> +
> +       return qcom_cc_map(pdev, desc);
> +}
> +EXPORT_SYMBOL_GPL(qcom_cc_attach_pds_map);
> +
> 
> 
> --- a/drivers/clk/qcom/videocc-sm8550.c
> +++ b/drivers/clk/qcom/videocc-sm8550.c
> @@ -542,6 +542,12 @@ static int video_cc_sm8550_probe(struct platform_device *pdev)
>         int ret;
>         u32 sleep_clk_offset = 0x8140;
> 
> +       regmap = qcom_cc_attach_pds_map(pdev, &video_cc_sm8550_desc);
> +       if (IS_ERR(regmap)) {
> +               pm_runtime_put(&pdev->dev);
> +               return PTR_ERR(regmap);
> +       }
> +
>         ret = devm_pm_runtime_enable(&pdev->dev);
>         if (ret)
>                 return ret;
> @@ -550,12 +556,6 @@ static int video_cc_sm8550_probe(struct platform_device *pdev)
>         if (ret)
>                 return ret;
> 
> -       regmap = qcom_cc_map(pdev, &video_cc_sm8550_desc);
> -       if (IS_ERR(regmap)) {
> -               pm_runtime_put(&pdev->dev);
> -               return PTR_ERR(regmap);
> -       }
> -
> 
> This way also, we are aligning more towards common code and the code will be uniform across all
> clock drivers and this doesn't require separate callback in each individual clock driver.
> 
> Thanks,
> Jagadeesh 
> 
> >>
> >> But I can squash this to my previous patch by updating my qcom_cc_attach_pds() function
> >> prototype as below and then calling that new function here
> >>
> >> -int qcom_cc_attach_pds(struct device *dev, struct qcom_cc_desc *desc)
> >> +int qcom_cc_attach_pds(struct device *dev, struct dev_pm_domain_list *pd_list)
> >>
> >> -               ret = devm_pm_domain_attach_list(dev, NULL, &cc->pd_list);
> >> -               if (ret < 0 && ret != -EEXIST)
> >> +               ret = qcom_cc_attach_pds(dev, cc->pd_list);
> >> +               if (ret)
> >>
> >> Thanks,
> >> Jagadeesh
> >>
> >>>>
> >>>> Thanks,
> >>>> Jagadeesh
> >>>>
> >>>>>>
> >>>>>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> >>>>>> ---
> >>>>>>  drivers/clk/qcom/common.c | 9 ++++++---
> >>>>>>  1 file changed, 6 insertions(+), 3 deletions(-)
> >>>>>>
> >>>>>> diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
> >>>>>> index ec27f70b24bdec24edd2f6b3df0d766fc1cdcbf0..eb7e2a56d1d135f839fd9bd470ba6231ce775a8c 100644
> >>>>>> --- a/drivers/clk/qcom/common.c
> >>>>>> +++ b/drivers/clk/qcom/common.c
> >>>>>> @@ -300,9 +300,12 @@ int qcom_cc_really_probe(struct device *dev,
> >>>>>>  	if (!cc)
> >>>>>>  		return -ENOMEM;
> >>>>>>  
> >>>>>> -	ret = devm_pm_domain_attach_list(dev, NULL, &cc->pd_list);
> >>>>>> -	if (ret < 0 && ret != -EEXIST)
> >>>>>> -		return ret;
> >>>>>> +	cc->pd_list = desc->pd_list;
> >>>>>> +	if (!cc->pd_list) {
> >>>>>> +		ret = devm_pm_domain_attach_list(dev, NULL, &cc->pd_list);
> >>>>>> +		if (ret < 0 && ret != -EEXIST)
> >>>>>> +			return ret;
> >>>>>> +	}
> >>>>>>  
> >>>>>>  	reset = &cc->reset;
> >>>>>>  	reset->rcdev.of_node = dev->of_node;
> >>>>>>
> >>>>>> -- 
> >>>>>> 2.34.1
> >>>>>>
> >>>>>
> >>>
> >
diff mbox series

Patch

diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
index ec27f70b24bdec24edd2f6b3df0d766fc1cdcbf0..eb7e2a56d1d135f839fd9bd470ba6231ce775a8c 100644
--- a/drivers/clk/qcom/common.c
+++ b/drivers/clk/qcom/common.c
@@ -300,9 +300,12 @@  int qcom_cc_really_probe(struct device *dev,
 	if (!cc)
 		return -ENOMEM;
 
-	ret = devm_pm_domain_attach_list(dev, NULL, &cc->pd_list);
-	if (ret < 0 && ret != -EEXIST)
-		return ret;
+	cc->pd_list = desc->pd_list;
+	if (!cc->pd_list) {
+		ret = devm_pm_domain_attach_list(dev, NULL, &cc->pd_list);
+		if (ret < 0 && ret != -EEXIST)
+			return ret;
+	}
 
 	reset = &cc->reset;
 	reset->rcdev.of_node = dev->of_node;