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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5ac7f79a-68d3-49ef-9615-3c0076591d78@linaro.org>
Date:   Tue, 28 Nov 2023 09:33:33 +0100
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Jacky Huang <ychuang570808@...il.com>, linus.walleij@...aro.org,
        robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
        conor+dt@...nel.org, p.zabel@...gutronix.de, j.neuschaefer@....net
Cc:     linux-arm-kernel@...ts.infradead.org, linux-gpio@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        ychuang3@...oton.com, schung@...oton.com
Subject: Re: [PATCH v2 2/4] dt-bindings: pinctrl: Document nuvoton ma35d1 pin
 control

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@...oton.com>
>>>
>>> Add documentation to describe nuvoton ma35d1 pin control and GPIO.
>>>
>>> Signed-off-by: Jacky Huang <ychuang3@...oton.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.

..


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