Re: [PATCH 1/3] dt-bindings: power: rpmpd: Add Generic RPM(h) PD indexes

[Konrad Dybcio]

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@xxxxxxxxxxx>
>>>>> Suggested-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>
>>>> 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@xxxxxxxxxx/
>>> 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.

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