linux-kernel - Re: [PATCH v2 1/8] dt-bindings: pinctrl: mvebu: Document bindings for AC5

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: <cb0af80e-3e5a-fbd9-cd8b-7b252ebe33fe@canonical.com>
Date:   Wed, 16 Mar 2022 09:16:09 +0100
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>
To:     Chris Packham <Chris.Packham@...iedtelesis.co.nz>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "linus.walleij@...aro.org" <linus.walleij@...aro.org>,
        "catalin.marinas@....com" <catalin.marinas@....com>,
        "andrew@...n.ch" <andrew@...n.ch>,
        "gregory.clement@...tlin.com" <gregory.clement@...tlin.com>,
        "sebastian.hesselbarth@...il.com" <sebastian.hesselbarth@...il.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-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v2 1/8] dt-bindings: pinctrl: mvebu: Document bindings for
 AC5

On 15/03/2022 22:12, Chris Packham wrote:
> (trimmed cc list to the arm, pinctrl and dt people)
> 
> On 15/03/22 23:46, Krzysztof Kozlowski wrote:
>> On 14/03/2022 22:31, Chris Packham wrote:
>>> Add JSON schema for marvell,ac5-pinctrl present on the Marvell 98DX2530
>>> SoC.
>>>
>>> Signed-off-by: Chris Packham <chris.packham@...iedtelesis.co.nz>
>>> ---
>>>
>>> Notes:
>>>      Changes in v2:
>>>      - Remove syscon and simple-mfd compatibles
>>>
>>>   .../bindings/pinctrl/marvell,ac5-pinctrl.yaml | 70 +++++++++++++++++++
>>>   1 file changed, 70 insertions(+)
>>>   create mode 100644 Documentation/devicetree/bindings/pinctrl/marvell,ac5-pinctrl.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/pinctrl/marvell,ac5-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/marvell,ac5-pinctrl.yaml
>>> new file mode 100644
>>> index 000000000000..65af1d5f5fe0
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/pinctrl/marvell,ac5-pinctrl.yaml
>>> @@ -0,0 +1,70 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>> +%YAML 1.2
>>> +---
>>> +$id: http://scanmail.trustwave.com/?c=20988&d=vu6w4lGvpbdx5x7Y5wSGMQ_aPa00Bnj19ce8eGP0QA&u=http%3a%2f%2fdevicetree%2eorg%2fschemas%2fpinctrl%2fmarvell%2cac5-pinctrl%2eyaml%23
>>> +$schema: http://scanmail.trustwave.com/?c=20988&d=vu6w4lGvpbdx5x7Y5wSGMQ_aPa00Bnj19cPrfjTyTg&u=http%3a%2f%2fdevicetree%2eorg%2fmeta-schemas%2fcore%2eyaml%23
>>> +
>>> +title: Marvell AC5 pin controller
>>> +
>>> +maintainers:
>>> +  - Chris Packham <chris.packham@...iedtelesis.co.nz>
>>> +
>>> +description:
>>> +  Bindings for Marvell's AC5 memory-mapped pin controller.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: marvell,ac5-pinctrl
>>> +
>>> +patternProperties:
>>> +  '-pins$':
>>> +    type: object
>>> +    $ref: pinmux-node.yaml#
>>> +
>>> +    properties:
>>> +      marvell,function:
>>> +        $ref: "/schemas/types.yaml#/definitions/string"
>>> +        description:
>>> +          Indicates the function to select.
>>> +        enum: [ gpio, i2c0, i2c1, nand, sdio, spi0, spi1, uart0, uart1, uart2, uart3 ]
>>> +
>>> +      marvell,pins:
>>> +        $ref: /schemas/types.yaml#/definitions/string-array
>>> +        description:
>>> +          Array of MPP pins to be used for the given function.
>>> +        minItems: 1
>>> +        items:
>>> +          enum: [ mpp0, mpp1, mpp2, mpp3, mpp4, mpp5, mpp6, mpp7, mpp8, mpp9,
>>> +                  mpp10, mpp11, mpp12, mpp13, mpp14, mpp15, mpp16, mpp17, mpp18, mpp19,
>>> +                  mpp20, mpp21, mpp22, mpp23, mpp24, mpp25, mpp26, mpp27, mpp28, mpp29,
>>> +                  mpp30, mpp31, mpp32, mpp33, mpp34, mpp35, mpp36, mpp37, mpp38, mpp39,
>>> +                  mpp40, mpp41, mpp42, mpp43, mpp44, mpp45 ]
>>> +
>>> +allOf:
>>> +  - $ref: "pinctrl.yaml#"
>>> +
>>> +required:
>>> +  - compatible
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    system-controller@...20100 {
>>> +      compatible = "syscon", "simple-mfd";
>>> +      reg = <0x80020000 0x20>;
>> This is unusual. Usually the pinctrl should be a device @80020100, not
>> child of syscon node. Why do you need it? In v1 you mentioned that
>> vendor sources do like this, but it's not correct to copy wrong DTS. :)
> 
> The vendor dts has this
> 
>          pinctrl0: pinctrl@...20100 {
>              compatible = "marvell,ac5-pinctrl",
>                       "syscon", "simple-mfd";
>              reg = <0 0x80020100 0 0x20>;
>              i2c_mpps: i2c-mpps {
>                  marvell,pins = "mpp26", "mpp27";
>                  marvell,function = "i2c0-opt";
>              };
>       };
> 
> Rob pointed out that "syscon", "simple-mfd" don't belong. I went looking 
> and found marvell,armada-7k-pinctrl which has the pinctrl as a child of 
> a syscon node and what you see in v2 is the result.
> 
> I probably went a bit too far off the deep end and should have just 
> dropped the "syscon", "simple-mfd" compatibles. I even wrote that 
> version but decided to add some gold plating before I submitted it.

More or less it is explained in
Documentation/devicetree/bindings/arm/marvell/cp110-system-controller.txt why
armada-7k uses it that way. The pinctrl is part of system registers
which apparently has to be shared with others (on shared SFR range).

It depends on your case, your SFR ranges for pinctrl and other blocks.


Best regards,
Krzysztof