Messages in this thread | | | Date | Tue, 28 Nov 2023 17:32:43 +0800 | Subject | Re: [PATCH v2 2/4] dt-bindings: pinctrl: Document nuvoton ma35d1 pin control | From | 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@nuvoton.com> >>>> >>>> Add documentation to describe nuvoton ma35d1 pin control and GPIO. >>>> >>>> Signed-off-by: Jacky Huang <ychuang3@nuvoton.com> >>>> --- >>> 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
| |