Re: [PATCH v6 02/11] dt-bindings: pwm: rzg2l-gpt: Document renesas,poegs property

Uwe Kleine-König <[email protected]> Thu, 16 Jul 2026 09:50:05 +0200

Newsgroups	org.kernel.vger.linux-pwm,dev.linux.lists.sashiko-reviews,org.kernel.vger.linux-devicetree
Message-ID	<aliM3iVA1LY7tW38@monoceros>

Hello Rob,

On Thu, Jun 04, 2026 at 10:06:19AM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The `renesas,poegs` property improperly encodes the consumer's channel index as a phandle specifier, which is a Devicetree anti-pattern and breaks standard phandle-array parsing semantics.
> --
> 
> commit 9d7f9847d3f320fb715022b6159c9c7e869aeedd
> Author: Biju Das <[email protected]>
> 
> dt-bindings: pwm: rzg2l-gpt: Document renesas,poegs property
> 
> RZ/G2L GPT IP supports output pin disable function by dead time
> error and detecting short-circuits between output pins.
> Add documentation for the optional property renesas,poegs to
> link a pair of GPT IOs with POEG.
> 
> > diff --git a/Documentation/devicetree/bindings/pwm/renesas,rzg2l-gpt.yaml b/Documentation/devicetree/bindings/pwm/renesas,rzg2l-gpt.yaml
> > --- a/Documentation/devicetree/bindings/pwm/renesas,rzg2l-gpt.yaml
> > +++ b/Documentation/devicetree/bindings/pwm/renesas,rzg2l-gpt.yaml
> 
> [ ... ]
> 
> > @@ -245,6 +245,28 @@ properties:
> >    resets:
> >      maxItems: 1
> >  
> > +  renesas,poegs:
> > +    minItems: 1
> > +    maxItems: 8
> > +    $ref: /schemas/types.yaml#/definitions/phandle-array
> > +    items:
> > +      items:
> > +        - description: phandle to POEG instance that serves the output disable
> > +        - enum: [ 0, 1, 2, 3, 4, 5, 6, 7 ]
> > +          description: |
> > +            An index identifying pair of GPT channels.
> 
> [Severity: Medium]
> Does this property correctly follow standard Devicetree phandle semantics?
> 
> By Devicetree specification, phandle arguments in a phandle-array are
> defined and interpreted by the target provider node, with their length
> dictated by a cell-count property on the provider.
> 
> This definition appears to invert that model by using the specifier to encode
> the consumer's (GPT) own channel index rather than a provider argument.
> 
> Additionally, standard OS-agnostic DT parsers might fail to resolve this
> because the referenced provider binding (renesas,rzg2l-poeg.yaml) enforces
> additionalProperties: false and natively lacks a #poeg-cells property to
> dictate the specifier length.
> 
> Is it possible to redesign this binding so it doesn't encode the consumer's
> index into the provider's phandle argument, or at least ensure the POEG
> provider specifies a #poeg-cells property?

You acked the patch. Is that review concern by Sashiko valid?

Best regards
Uwe

signature.asc (application/pgp-signature, 488 B)