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