Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Message-ID: <cb0af80e-3e5a-fbd9-cd8b-7b252ebe33fe@canonical.com>
Date:   Wed, 16 Mar 2022 09:16:09 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Subject: Re: [PATCH v2 1/8] dt-bindings: pinctrl: mvebu: Document bindings for
 AC5
Content-Language: en-US
To:     Chris Packham <Chris.Packham@alliedtelesis.co.nz>,
        "robh+dt@kernel.org" <robh+dt@kernel.org>,
        "linus.walleij@linaro.org" <linus.walleij@linaro.org>,
        "catalin.marinas@arm.com" <catalin.marinas@arm.com>,
        "andrew@lunn.ch" <andrew@lunn.ch>,
        "gregory.clement@bootlin.com" <gregory.clement@bootlin.com>,
        "sebastian.hesselbarth@gmail.com" <sebastian.hesselbarth@gmail.com>
Cc:     "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>
References: <20220314213143.2404162-1-chris.packham@alliedtelesis.co.nz>
 <20220314213143.2404162-2-chris.packham@alliedtelesis.co.nz>
 <4e6df448-5562-8f50-6f46-91acb279bc1a@canonical.com>
 <7e73bba0-8b54-772c-2e94-8fca4e4e3294@alliedtelesis.co.nz>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
In-Reply-To: <7e73bba0-8b54-772c-2e94-8fca4e4e3294@alliedtelesis.co.nz>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

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@alliedtelesis.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@alliedtelesis.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@80020100 {
>>> +      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@80020100 {
>              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