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

[Rohit Agarwal]

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@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.
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