Re: [PATCH v2 2/4] dt-bindings: pinctrl: Document nuvoton ma35d1 pin control

[Jacky Huang]

Dear Krzysztof,

Thanks for your review.

On 2023/11/28 下午 04:33, Krzysztof Kozlowski wrote:
> On 28/11/2023 09:29, Jacky Huang wrote:
> > Dear Krzysztof,
> >
> > Thanks for your review.
> >
> >
> > On 2023/11/28 下午 03:34, Krzysztof Kozlowski wrote:
> > > On 28/11/2023 07:11, Jacky Huang wrote:
> > > > From: Jacky Huang <ychuang3@xxxxxxxxxxx>
> > > >
> > > > Add documentation to describe nuvoton ma35d1 pin control and GPIO.
> > > >
> > > > Signed-off-by: Jacky Huang <ychuang3@xxxxxxxxxxx>
> > > > ---
> > > Your changelog said:
> > >
> > > >     - Remove ma35d1-pinfunc.h which is unused after update definition of
> > > 'nuvoton,pins'.
> > >
> > > You forgot to add:
> > >
> > > " - Do not test the bindings before sending"
> > >
> > > I assume none of the driver changes compile either.
> > It's my mistake. I forgot to remove 'ma35d1-pinfunc.h' from my local
> > copy, and as a consequence, the 'dt_binding_check' did not catch this
> > error. I will fix this.
> But then git status would point you that tree is not clean and you did
> not finish commiting.
>
> ..

Yes, I should have been more careful in checking.

> > > > +
> > > > +allOf:
> > > > +  - $ref: pinctrl.yaml#
> > > allOf goes before additionalProperties.
> > >
> > > > +
> > > > +required:
> > > > +  - compatible
> > > > +  - nuvoton,sys
> > > This goes after patternProperties
> > I will fix the above two as:
> >
> > allOf:
> >     - $ref: pinctrl.yaml#
> Look:
>
> > > allOf goes before additionalProperties.
> Open example-schema.
>
> ..

I found that 'pinctrl.yaml' is not required for this document, so I will 
drop it.

> > > > +      - $ref: pincfg-node.yaml#
> > > > +
> > > > +    properties:
> > > > +      bias-disable: true
> > > Drop this and other "true", why do you need them here?
> > We are following the conventions used in other pinctrl documents, such as
> > 'realtek,rtd1315e-pinctrl.yaml' and 'xlnx,zynqmp-pinctrl.yaml'.
> But they are quite different there.
>
> > After comparing various pinctrl documents, I noticed that they all express
> > it as 'bias-disable: true'. Therefore, may I keep the current format?
> No, you cannot copy pieces of other binding, selectively ignoring the
> rest. Look how these other bindings are constructed - they have
> additionalProperties, which you don't.
>
> Drop all these true properties if the only reason of them being here is
> they were copied.
>
>
>
> Best regards,
> Krzysztof

OK, I will drop these properties.


Best Regards,
Jacky Huang