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

[Rakesh Kota]

On Fri, Mar 27, 2026 at 11:32:57AM +0100, Krzysztof Kozlowski wrote:
> 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.

Apologies for the late response ....!

Both PMK8350 and PMM8654AU have two address spaces (pon + pon_pbs).
While PMM8654AU updates some register definitions (offsets/bits
added/removed) in the pon + pon_pbs address spaces, we currently
have no use case that requires separate handling. For this reason,
PMM8654AU falls back to qcom,pmk8350-pon for now, and we can revisit
this when a concrete use case arises.

The minItems: 1 in the original pmk8350 if/then block was incorrect —
it just repeated the top-level default and did not enforce the GEN3
requirement of two address spaces.

I will fix in v5 as a separate preceding commit:
  - Set minItems: 2 for both reg: and reg-names: in the existing pmk8350 if/then block.
  - Drop the new pmm8654au if/then block — covered by the pmk8350 block.

Proposed change:
- if: 
   properties:
      compatible:
	  contains:
	    const: qcom,pmk8350-pon
				         
   then:
     properties:
        reg:
	  minItems: 2
	  maxItems: 2
	reg-names:
	  minItems: 2
	  maxItems: 2
> 
> 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.
> 
Apologies for the extra review cycles.

regards
Rakesh Kota