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]
Date:   Sun, 20 Nov 2022 11:40:07 +0100
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Kevin Hilman <khilman@...libre.com>,
        Bernhard Rosenkränzer <bero@...libre.com>,
        linux-mediatek@...ts.infradead.org
Cc:     linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, matthias.bgg@...il.com,
        angelogiocchino.delregno@...labora.com
Subject: Re: [PATCH v3 4/7] dt-bindings: pinctrl: add bindings for Mediatek
 MT8365 SoC

On 18/11/2022 20:52, Kevin Hilman wrote:
> Hi Krzysztof,
> 
> Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org> writes:
> 
>> On 17/11/2022 22:03, Bernhard Rosenkränzer wrote:
>>> Add devicetree bindings for Mediatek MT8365 pinctrl driver.
>>>
>>> Signed-off-by: Bernhard Rosenkränzer <bero@...libre.com>
>>
>> Thank you for your patch. There is something to discuss/improve.
>>
>>> +
>>> +  pins-are-numbered:
>>> +    $ref: /schemas/types.yaml#/definitions/flag
>>> +    description: |
>>> +      Specify the subnodes are using numbered pinmux to specify pins.
>>
>> Why would you name pins differently per board? And why this different
>> naming of the same pins is a property of hardware?
>>
>> This looks like something to drop.
> 
> Yeah, having this as a flag kind of implies that this could be present
> for some boards but not others.  But in practice, the driver requires it
> to be present or just fails[1].  What's the right way to describe that?
> We're just trying to add a binding that reflects the existing driver.

Uh, what an interesting property. What's the point of it then? Why
failing to probe on a missing property which does nothing else?

The solution is also to drop that property from the driver.

> We also noticed that there's another documented binding with this
> same flag[2] where similiarily, the driver simply requires it to be
> present[2].
> 
> So is the way this flag is documented in the stm32 binding OK for the
> mediatek one also?  If not, what would you suggest?

I would like to understand why do we need this property and what is
described by it. Because if it's purpose is only to fail or not fail
driver probe, then we should just drop it everywhere.


> 

Best regards,
Krzysztof

Powered by blists - more mailing lists