Re: [PATCH v3 1/3] dt-bindings: power: rpmhpd: Add Generic RPMh PD indexes

[Rohit Agarwal]

On 7/11/2023 8:29 PM, Dmitry Baryshkov wrote:
> On 11/07/2023 14:44, Konrad Dybcio wrote:
> > On 11.07.2023 08:22, Krzysztof Kozlowski wrote:
> > > On 11/07/2023 08:17, Rohit Agarwal wrote:
> > > > On 7/11/2023 11:22 AM, Krzysztof Kozlowski wrote:
> > > > > On 11/07/2023 07:42, Rohit Agarwal wrote:
> > > > > > Add Generic RPMh 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>
> > > > > > ---
> > > > > >    include/dt-bindings/power/qcom-rpmhpd.h | 30 
> > > > > > ++++++++++++++++++++++++++++++
> > > > > >    1 file changed, 30 insertions(+)
> > > > > >    create mode 100644 include/dt-bindings/power/qcom-rpmhpd.h
> > > > > >
> > > > > > diff --git a/include/dt-bindings/power/qcom-rpmhpd.h 
> > > > > > b/include/dt-bindings/power/qcom-rpmhpd.h
> > > > > > new file mode 100644
> > > > > > index 0000000..4da2e04
> > > > > > --- /dev/null
> > > > > > +++ b/include/dt-bindings/power/qcom-rpmhpd.h
> > > > > Filename based on compatible.
> > > > This is not specific for SDX75. These are generic ones that should be
> > > > used for all other targets.
> > > > Konrad suggested in v1 to avoid target specific prefixes everytime and
> > > > to create a new generic
> > > > dt-bindings that can be reused.
> > > > > > @@ -0,0 +1,30 @@
> > > > > > +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> > > > > > +/*
> > > > > > + * Copyright (c) 2023, Qualcomm Innovation Center, Inc. All 
> > > > > > rights reserved.
> > > > > > + */
> > > > > > +
> > > > > > +#ifndef _DT_BINDINGS_POWER_QCOM_RPMHPD_H
> > > > > > +#define _DT_BINDINGS_POWER_QCOM_RPMHPD_H
> > > > > > +
> > > > > > +/* Generic RPMH Power Domain Indexes */
> > > > > > +#define CX               0
> > > > > These are very generic names, usually not used in global headers. 
> > > > > Please
> > > > > use some reasonable prefix.
> > > > This was based on the suggestion from Konrad in v2 to drop the RPMHPD
> > > > prefix and we can go only with names like CX, etc.
> > >
> > > I don't think having so generic name in tree-wide header is good idea.
> > Conversely, I think that it would be very clear in files including 
> > this header.
>
> It is expected to be included into dtsi and into rpmhpd driver (only). 
> However I'd also suggest adding some generic prefix (e.g. RPMHPD_CX). 
> Just `CX' is too short and has a sensible possibility of getting  a 
> symbol conflict.
Ok, Will add them back as in v2.

Thanks,
Rohit.
> > Konrad
> > > CX can mean anything.
> > >
> > > Best regards,
> > > Krzysztof