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

[Rohit Agarwal <quic_rohiagar@quicinc.com>]

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@quicinc.com>
>>>>>> Suggested-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>>>>>> ---
>>>>>>    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
>>>
>