Re: [PATCH 03/18] dt-bindings: soc: qcom: qcom,saw2: define optional regulator node

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 15/06/2023 00:49, Dmitry Baryshkov wrote:
> On 14/06/2023 19:05, Krzysztof Kozlowski wrote:
>> On 12/06/2023 07:39, Dmitry Baryshkov wrote:
>>> The SAW2 device can optionally provide a voltage regulator supplying the
>>> CPU core, cluster or L2 cache. Describe it in the device bindings.
>>>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> ---
>>>   .../devicetree/bindings/soc/qcom/qcom,saw2.yaml | 17 +++++++++++++++++
>>>   1 file changed, 17 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,saw2.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,saw2.yaml
>>> index a016242367b9..b809a9cc0916 100644
>>> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,saw2.yaml
>>> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,saw2.yaml
>>> @@ -47,6 +47,10 @@ properties:
>>>         - description: Base address and size of the alias register region
>>>       minItems: 1
>>>   
>>> +  regulator:
>>> +    $ref: /schemas/regulator/regulator.yaml#
>>
>> There was such property in the binding (and DTS!) but a bool. Previous
>> patch silently dropped it, so re-introducing it with different type is
>> confusing.
> 
> Could you please propose a better name here? saw-regulator? Or maybe 
> regulator-saw? (as we might get regulator-avs at some point).

regulator name is OK for me, but any ABI change should be:
1. Clearly expressed with rationale,
2. Done probably in one DT commit, not two. IOW, first silently dropping
a property and then adding a new one like nothing happened is not good.
It should be clear that old property is wrong because foo bar and we
make it now different with breaking all the DTS because foo bar.

Best regards,
Krzysztof