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. .. >>> + >>> +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. .. > >>> + - $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
