linux-kernel - Re: [PATCH v3 1/2] dt-bindings: gpio: gpio-mvebu: convert txt binding to YAML

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] [day] [month] [year] [list]

Date:   Sun, 15 May 2022 21:20:39 +0000
From:   Chris Packham <Chris.Packham@...iedtelesis.co.nz>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "krzysztof.kozlowski+dt@...aro.org" 
        <krzysztof.kozlowski+dt@...aro.org>,
        "linus.walleij@...aro.org" <linus.walleij@...aro.org>,
        "brgl@...ev.pl" <brgl@...ev.pl>,
        "thierry.reding@...il.com" <thierry.reding@...il.com>,
        "u.kleine-koenig@...gutronix.de" <u.kleine-koenig@...gutronix.de>,
        "lee.jones@...aro.org" <lee.jones@...aro.org>,
        "andrew@...n.ch" <andrew@...n.ch>,
        Vadym Kochan <vadym.kochan@...ision.eu>,
        "enachman@...vell.com" <enachman@...vell.com>
CC:     "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
        "linux-pwm@...r.kernel.org" <linux-pwm@...r.kernel.org>
Subject: Re: [PATCH v3 1/2] dt-bindings: gpio: gpio-mvebu: convert txt binding
 to YAML


On 15/05/22 08:28, Krzysztof Kozlowski wrote:
> On 14/05/2022 04:20, Chris Packham wrote:
>>>> +
>>>> +allOf:
>>>> +  - if:
>>>> +      properties:
>>>> +        compatible:
>>>> +          contains:
>>>> +            const: marvell,armada-8k-gpio
>>>> +    then:
>>>> +      required:
>>>> +        - offset
>>>> +    else:
>>>> +      required:
>>>> +        - reg
>>> one blank line please
>>>
>>>> +  - if:
>>>> +      properties:
>>>> +        compatible:
>>>> +          contains:
>>>> +            const: marvell,armadaxp-gpio
>>> Original bindings are saying that second reg is optional for
>>> marvell,armada-370-gpio. What about other cases, e.g. mv78200-gpio? Is
>>> it also allowed (and optional) there?
>> This is where things get interesting. The armadaxp (and only the
>> armadaxp) requires a second register value for some per-cpu registers.
>> All of the other SoCs can have an optional 2nd register value if they
>> want to use the PWM function. I guess that implies that the armadaxp
>> can't do PWM.
>>>> +    then:
>>>> +      properties:
>>>> +        reg:
>>>> +          minItems: 2
>>> Then you also should require two reg-names.
>> Simple enough to add. But currently we've said that the reg-names are
>> "gpio" and "pwm" but on the armadaxp the 2nd one is not "pwm" but
>> something else ("per-cpu" perhaps?)
> In such case they would be failing with current bindings, because they
> expect "pwm" as second name, right?

The driver is alright because it checks for the 
"marvell,armada-370-gpio" compatible before using 
platform_get_resource_byname("pwm"). The unused (at least in theory) 
armadaxp code just calls devm_platform_ioremap_resource(pdev, 1) so it 
doesn't care about the name.

I'm tempted to leave the regName: minItems: 2 out because the armadaxp 
code doesn't care about them and setting the 2nd regname to "pwm" when 
the compatible is armadaxp would be misleading.

>> On the other hand this is all completely moot because the
>> armada-xp-mv78*.dtsi actually use the "marvell,armada-370-gpio"
>> compatible so this appears to be documenting something that is no longer
>> used. Indeed it appears that the armadaxp specific usage was remove in
>> 5f79c651e81e ("arm: mvebu: use global interrupts for GPIOs on Armada XP").
>>
>> So perhaps the best course of action is to drop marvell,armadaxp-gpio
>> from the new binding (noting that we've done so in the commit message).
>
> That's fine, maybe in a separate patch (2nd one)?
I'll add another patch on top that marks the compatible as deprecated 
including some of the above info in the commit message.
>
> Best regards,
> Krzysztof