mbox series

[0/3] Add support of rpmhpd for SDX75

Message ID 1688647793-20950-1-git-send-email-quic_rohiagar@quicinc.com
Headers show
Series Add support of rpmhpd for SDX75 | expand

Message

Rohit Agarwal July 6, 2023, 12:49 p.m. UTC
Hi,

This series adds the support of rpmhpd for sdx75 and also
adds the generic bindings for the PD to be used henceforth.

Thanks,
Rohit.

Rohit Agarwal (3):
  dt-bindings: power: rpmpd: Add Generic RPM(h) PD indexes
  dt-bindings: power: Add compatible for sdx75
  soc: qcom: rpmhpd: Add SDX75 power domains

 .../devicetree/bindings/power/qcom,rpmpd.yaml      |  1 +
 drivers/soc/qcom/rpmhpd.c                          | 16 +++++++
 include/dt-bindings/power/qcom-rpmpd.h             | 49 ++++++++++++++++++++++
 3 files changed, 66 insertions(+)

Comments

Pavan Kondeti July 6, 2023, 2:30 p.m. UTC | #1
On Thu, Jul 06, 2023 at 06:19:51PM +0530, Rohit Agarwal wrote:
> Add Generic RPM(h) Power Domain indexes that can be used
> for all the Qualcomm SoC henceforth.
> 
> Signed-off-by: Rohit Agarwal <quic_rohiagar@quicinc.com>
> Suggested-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Does it make sense to give this link [1] so that we know what is
Konrad's suggestion and the discussion around it?

[1]
https://lore.kernel.org/all/0d468d08-6410-e424-b4f3-5245cdb0334a@linaro.org/
> ---
>  include/dt-bindings/power/qcom-rpmpd.h | 49 ++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
> 
> diff --git a/include/dt-bindings/power/qcom-rpmpd.h b/include/dt-bindings/power/qcom-rpmpd.h
> index 83be996..6498251 100644
> --- a/include/dt-bindings/power/qcom-rpmpd.h
> +++ b/include/dt-bindings/power/qcom-rpmpd.h
> @@ -4,6 +4,55 @@
>  #ifndef _DT_BINDINGS_POWER_QCOM_RPMPD_H
>  #define _DT_BINDINGS_POWER_QCOM_RPMPD_H
>  
> +/* Generic RPMH Power Domain Indexes */
> +#define RPMHPD_CX		0
> +#define RPMHPD_MX		1
> +#define RPMHPD_CX_AO		2
> +#define RPMHPD_MX_AO		3
> +#define RPMHPD_GFX		4
> +#define RPMHPD_MSS		5
> +#define RPMHPD_EBI		6
> +#define RPMHPD_LCX		7
> +#define RPMHPD_LMX		8
> +#define RPMHPD_MMCX		9
> +#define RPMHPD_MMCX_AO		10
> +#define RPMHPD_MXC		11
> +#define RPMHPD_MXC_AO		12
> +#define RPMHPD_NSP		13
> +#define RPMHPD_NSP0		14
> +#define RPMHPD_NSP1		15
> +#define RPMHPD_QPHY		16
> +#define RPMHPD_DDR		17
> +#define RPMHPD_XO		18
> +
> +/* Generic RPM Power Domain Indexes */
> +#define RPMPD_VDDCX		0
> +#define RPMPD_VDDCX_AO		1
> +#define RPMPD_VDDMX		2
> +#define RPMPD_VDDMX_AO		3
> +#define RPMPD_VDDCX_VFL		4
> +#define RPMPD_VDDMX_VFL		5
> +#define RPMPD_VDDCX_VFC		6
> +#define RPMPD_LPI_CX		7
> +#define RPMPD_LPI_MX		8
> +#define RPMPD_SSCCX		9
> +#define RPMPD_SSCCX_VFL		10
> +#define RPMPD_SSCMX		11
> +#define RPMPD_SSCMX_VFL		12
> +#define RPMPD_VDDSSCX		13
> +#define RPMPD_VDDSSCX_VFC	14
> +#define RPMPD_VDDGFX		15
> +#define RPMPD_VDDGFX_VFC	16
> +#define RPMPD_VDDGX		17
> +#define RPMPD_VDDGX_AO		18
> +#define RPMPD_VDDMDCX		19
> +#define RPMPD_VDDMDCX_AO	20
> +#define RPMPD_VDDMDCX_VFC	21
> +#define RPMPD_VDDMD		22
> +#define RPMPD_VDDMD_AO		23
> +#define RPMPD_LPICX_VFL		24
> +#define RPMPD_LPIMX_VFL		25
> +

How did you come up with this list? A union of all SoCs supported by
RPMh driver?

>  /* SA8775P Power Domain Indexes */
>  #define SA8775P_CX	0
>  #define SA8775P_CX_AO	1
> -- 
> 2.7.4
> 

Thanks,
Pavan
Krzysztof Kozlowski July 6, 2023, 2:36 p.m. UTC | #2
On 06/07/2023 14:49, Rohit Agarwal wrote:
> Add Generic RPM(h) Power Domain indexes that can be used
> for all the Qualcomm SoC henceforth.
> 
> Signed-off-by: Rohit Agarwal <quic_rohiagar@quicinc.com>
> Suggested-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>  include/dt-bindings/power/qcom-rpmpd.h | 49 ++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)

Didn't you just send a patch doing similar? There is no changelog, no
versioning, how can anyone figure out which patch is the latest or which
one should be ignored?

Best regards,
Krzysztof
Rohit Agarwal July 6, 2023, 3:15 p.m. UTC | #3
On 7/6/2023 8:30 PM, Konrad Dybcio wrote:
> On 6.07.2023 16:47, Rohit Agarwal wrote:
>> On 7/6/2023 8:00 PM, Pavan Kondeti wrote:
>>> On Thu, Jul 06, 2023 at 06:19:51PM +0530, Rohit Agarwal wrote:
>>>> Add Generic RPM(h) Power Domain indexes that can be used
>>>> for all the Qualcomm SoC henceforth.
>>>>
>>>> Signed-off-by: Rohit Agarwal <quic_rohiagar@quicinc.com>
>>>> Suggested-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>>> Does it make sense to give this link [1] so that we know what is
>>> Konrad's suggestion and the discussion around it?
>>>
>>> [1]
>>> https://lore.kernel.org/all/0d468d08-6410-e424-b4f3-5245cdb0334a@linaro.org/
>> Yes, could be given in the cover letter.
>>>> ---
> [...]
>
>>>> +#define RPMPD_VDDMD        22
>>>> +#define RPMPD_VDDMD_AO        23
>>>> +#define RPMPD_LPICX_VFL        24
>>>> +#define RPMPD_LPIMX_VFL        25
>>>> +
>>> How did you come up with this list? A union of all SoCs supported by
>>> RPMh driver?
>> Yes, union of all the SoCs and arranged based on frequencies of usage.
> The latter part is very thoughtful, thanks for taking that into account.
>
> That said (and I really don't wanna be picky here, I'm just coming up with
> ideas a bit later than I'd like to).. Perhaps this patch should be limited
> to RPMhPD [1] and the definitions could be moved to a new binding, so:
So should we not update anything in this old binding and completely move 
to the new bindings?
rpmhpd.h?
Not even rpmpd_* bindings?

Thanks,
Rohit.
> include/dt-bindings/power/qcom,rpmhpd.h
> // this way we don't have to add RPMHPD_
> #define CX	0
Ok, will remove this as well.
> which would result in us being able to do:
>
> #include ....rpmhpd.h
> [...]
> power-domains = <&rpmhpd CX>;
>
> in the device tree
>
> which is even more concise!

Yes

Thanks,
Rohit.

>
> [1] The old RPM SMD platforms have some duplications in the names..
>      No point in duplicating that. The oldest entries remember 2013 so
>      it's easy to see how we had some dirt build up there.
>
> Konrad
>> Thanks,
>> Rohit.
>>>>    /* SA8775P Power Domain Indexes */
>>>>    #define SA8775P_CX    0
>>>>    #define SA8775P_CX_AO    1
>>>> -- 
>>>> 2.7.4
>>>>
>>> Thanks,
>>> Pavan
Rohit Agarwal July 6, 2023, 3:28 p.m. UTC | #4
On 7/6/2023 8:52 PM, Konrad Dybcio wrote:
> On 6.07.2023 17:15, Rohit Agarwal wrote:
>> On 7/6/2023 8:30 PM, Konrad Dybcio wrote:
>>> On 6.07.2023 16:47, Rohit Agarwal wrote:
>>>> On 7/6/2023 8:00 PM, Pavan Kondeti wrote:
>>>>> On Thu, Jul 06, 2023 at 06:19:51PM +0530, Rohit Agarwal wrote:
>>>>>> Add Generic RPM(h) Power Domain indexes that can be used
>>>>>> for all the Qualcomm SoC henceforth.
>>>>>>
>>>>>> Signed-off-by: Rohit Agarwal <quic_rohiagar@quicinc.com>
>>>>>> Suggested-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>>>>> Does it make sense to give this link [1] so that we know what is
>>>>> Konrad's suggestion and the discussion around it?
>>>>>
>>>>> [1]
>>>>> https://lore.kernel.org/all/0d468d08-6410-e424-b4f3-5245cdb0334a@linaro.org/
>>>> Yes, could be given in the cover letter.
>>>>>> ---
>>> [...]
>>>
>>>>>> +#define RPMPD_VDDMD        22
>>>>>> +#define RPMPD_VDDMD_AO        23
>>>>>> +#define RPMPD_LPICX_VFL        24
>>>>>> +#define RPMPD_LPIMX_VFL        25
>>>>>> +
>>>>> How did you come up with this list? A union of all SoCs supported by
>>>>> RPMh driver?
>>>> Yes, union of all the SoCs and arranged based on frequencies of usage.
>>> The latter part is very thoughtful, thanks for taking that into account.
>>>
>>> That said (and I really don't wanna be picky here, I'm just coming up with
>>> ideas a bit later than I'd like to).. Perhaps this patch should be limited
>>> to RPMhPD [1] and the definitions could be moved to a new binding, so:
>> So should we not update anything in this old binding and completely move to the new bindings?
> Yes, create qcom,rpmhpd.h and add new common entries there and let this
> ship sink
>
>> rpmhpd.h?
>> Not even rpmpd_* bindings?
> Again, due to [1], let's not touch that for now. We'll worry about that
> when somebody will try to add a new entry to that driver.
Yes.

Thanks,
Rohit.
>
> Konrad
>> Thanks,
>> Rohit.
>>> include/dt-bindings/power/qcom,rpmhpd.h
>>> // this way we don't have to add RPMHPD_
>>> #define CX    0
>> Ok, will remove this as well.
>>> which would result in us being able to do:
>>>
>>> #include ....rpmhpd.h
>>> [...]
>>> power-domains = <&rpmhpd CX>;
>>>
>>> in the device tree
>>>
>>> which is even more concise!
>> Yes
>>
>> Thanks,
>> Rohit.
>>
>>> [1] The old RPM SMD platforms have some duplications in the names..
>>>       No point in duplicating that. The oldest entries remember 2013 so
>>>       it's easy to see how we had some dirt build up there.
>>>
>>> Konrad
>>>> Thanks,
>>>> Rohit.
>>>>>>     /* SA8775P Power Domain Indexes */
>>>>>>     #define SA8775P_CX    0
>>>>>>     #define SA8775P_CX_AO    1
>>>>>> -- 
>>>>>> 2.7.4
>>>>>>
>>>>> Thanks,
>>>>> Pavan