Re: [PATCH v4 1/2] dt-bindings: power: reset: qcom-pon: Add new compatible PMM8654AU

[Krzysztof Kozlowski]

On 27/03/2026 10:53, Rakesh Kota wrote:
> On Mon, Mar 23, 2026 at 01:18:20PM -0500, Rob Herring wrote:
>> On Mon, Mar 23, 2026 at 04:15:15PM +0530, Rakesh Kota wrote:
>>> PMM8654AU is a different PMIC from PMM8650AU, even though both share
>>> the same PMIC subtype. Add PON compatible string for PMM8654AU PMIC
>>> variant.
>>>
>>> The PMM8654AU PON block is compatible with the PMK8350 PON
>>> implementation, but PMM8654AU also implements additional PON registers
>>> beyond the baseline. Use the PMM8654AU naming to match the compatible
>>> string already present in the upstream pinctrl-spmi-gpio driver, keeping
>>> device tree and kernel driver naming consistent.
>>>
>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxxxxxxxx>
>>> Signed-off-by: Rakesh Kota <rakesh.kota@xxxxxxxxxxxxxxxx>
>>> ---
>>> Changes in v4:
>>>  - Remove the contain for PMK8350 and new if:then for PMM8654AU as
>>>    suggested by Krzysztof Kozlowski
>>>
>>> Changes in v3:
>>>  - Update the commit message.
>>>
>>> Changes in v2:
>>>  - Introduces PMM8654AU compatible strings as suggested by Konrad Dybcio.
>>> ---
>>>  .../devicetree/bindings/power/reset/qcom,pon.yaml  | 32 +++++++++++++++++-----
>>>  1 file changed, 25 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml b/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml
>>> index 979a377cb4ffd577bfa51b9a3cd089acc202de0c..2a5d9182b8d5c1a286716ab175c7bb5e39b334e0 100644
>>> --- a/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml
>>> +++ b/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml
>>> @@ -17,12 +17,16 @@ description: |
>>>  
>>>  properties:
>>>    compatible:
>>> -    enum:
>>> -      - qcom,pm8916-pon
>>> -      - qcom,pm8941-pon
>>> -      - qcom,pms405-pon
>>> -      - qcom,pm8998-pon
>>> -      - qcom,pmk8350-pon
>>> +    oneOf:
>>> +      - enum:
>>> +          - qcom,pm8916-pon
>>> +          - qcom,pm8941-pon
>>> +          - qcom,pms405-pon
>>> +          - qcom,pm8998-pon
>>> +          - qcom,pmk8350-pon
>>> +      - items:
>>> +          - const: qcom,pmm8654au-pon
>>> +          - const: qcom,pmk8350-pon
>>>  
>>>    reg:
>>>      description: |
>>> @@ -100,7 +104,6 @@ allOf:
>>>    - if:
>>>        properties:
>>>          compatible:
>>> -          contains:
>>>              const: qcom,pmk8350-pon
>>>      then:
>>>        properties:
>>> @@ -113,6 +116,21 @@ allOf:
>>>              - const: hlos
>>>              - const: pbs
>>>  
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +            const: qcom,pmm8654au-pon
>>> +    then:
>>> +      properties:
>>> +        reg:
>>> +          minItems: 1
>>> +          maxItems: 2
>>> +        reg-names:
>>> +          minItems: 1
>>> +          items:
>>> +            - const: hlos
>>> +            - const: pbs
>>
>> I don't understand this. The existing if/then schema did the exact same 
>> thing until you removed 'contains'. Now we just have the same schema 
>> duplicated.
>>
>> What does need changing now that I've looked at it is dropping 'reg' 
>> in this schema as it just repeats what the top-level schema has.
>>
> 
> we have got suggestion to add a new if:then block for the new compatible from Krzysztof Kozlowski.
> 

But I did not suggest to add the contents in new if:then: block. I
certainly did not suggest to not check this patch before submitting, either.

We had long discussion where I asked you how many address spaces you
have there?

Answer above.

And then answer why the patch says the device has one address space or
two address spaces. You engaged me, Konrad and now Rob in reviewing this
triviality. This is on the verge of wasting of our time.

Best regards,
Krzysztof