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