Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 14 Jun 2022 14:43:45 -0600
From:   Rob Herring <robh@kernel.org>
To:     Colin Foster <colin.foster@in-advantage.com>
Cc:     devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-gpio@vger.kernel.org,
        Vladimir Oltean <vladimir.oltean@nxp.com>,
        Lee Jones <lee.jones@linaro.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Andrew Lunn <andrew@lunn.ch>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Russell King <linux@armlinux.org.uk>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Lars Povlsen <lars.povlsen@microchip.com>,
        Steen Hegelund <Steen.Hegelund@microchip.com>,
        UNGLinuxDriver@microchip.com,
        Linus Walleij <linus.walleij@linaro.org>,
        Wolfram Sang <wsa@kernel.org>,
        Terry Bowman <terry.bowman@amd.com>,
        Andy Shevchenko <andy.shevchenko@gmail.com>
Subject: Re: [PATCH v10 net-next 6/7] dt-bindings: mfd: ocelot: add bindings
 for VSC7512
Message-ID: <20220614204345.GA2419690-robh@kernel.org>
References: <20220610202330.799510-1-colin.foster@in-advantage.com>
 <20220610202330.799510-7-colin.foster@in-advantage.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220610202330.799510-7-colin.foster@in-advantage.com>
Precedence: bulk

On Fri, Jun 10, 2022 at 01:23:29PM -0700, Colin Foster wrote:
> Add devicetree bindings for SPI-controlled Ocelot chips, specifically the
> VSC7512.
> 
> Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
> ---
>  .../devicetree/bindings/mfd/mscc,ocelot.yaml  | 160 ++++++++++++++++++
>  MAINTAINERS                                   |   1 +
>  2 files changed, 161 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/mscc,ocelot.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/mscc,ocelot.yaml b/Documentation/devicetree/bindings/mfd/mscc,ocelot.yaml
> new file mode 100644
> index 000000000000..e298ca8d616d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/mscc,ocelot.yaml
> @@ -0,0 +1,160 @@
> +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause */
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/mscc,ocelot.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Ocelot Externally-Controlled Ethernet Switch
> +
> +maintainers:
> +  - Colin Foster <colin.foster@in-advantage.com>
> +
> +description: |
> +  The Ocelot ethernet switch family contains chips that have an internal CPU
> +  (VSC7513, VSC7514) and chips that don't (VSC7511, VSC7512). All switches have
> +  the option to be controlled externally, which is the purpose of this driver.
> +
> +  The switch family is a multi-port networking switch that supports many
> +  interfaces. Additionally, the device can perform pin control, MDIO buses, and
> +  external GPIO expanders.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - mscc,vsc7512-spi

'-spi' is redundant as we know what bus this is on looking at the 
parent.

> +
> +  reg:
> +    maxItems: 1
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0

No size? That's odd given the child nodes are the same as memory mapped 
peripherals which expect a size.

> +
> +  spi-max-frequency:
> +    maxItems: 1
> +
> +patternProperties:
> +  "^pinctrl@[0-9a-f]+$":
> +    type: object
> +    $ref: /schemas/pinctrl/mscc,ocelot-pinctrl.yaml
> +
> +  "^gpio@[0-9a-f]+$":
> +    type: object
> +    $ref: /schemas/pinctrl/microchip,sparx5-sgpio.yaml
> +    properties:
> +      compatible:
> +        enum:
> +          - mscc,ocelot-sgpio
> +
> +  "^mdio@[0-9a-f]+$":
> +    type: object
> +    $ref: /schemas/net/mscc,miim.yaml
> +    properties:
> +      compatible:
> +        enum:
> +          - mscc,ocelot-miim
> +
> +required:
> +  - compatible
> +  - reg
> +  - '#address-cells'
> +  - '#size-cells'
> +  - spi-max-frequency
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    ocelot_clock: ocelot-clock {
> +          compatible = "fixed-clock";
> +          #clock-cells = <0>;
> +          clock-frequency = <125000000>;
> +      };
> +
> +    spi0 {

spi {

> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        ocelot-chip@0 {

Node names should be generic. I don't think we have a formal definition, 
but 'switch' seems to be most common.

> +            compatible = "mscc,vsc7512-spi";
> +            spi-max-frequency = <2500000>;
> +            reg = <0>;
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            mdio0: mdio@7107009c {

Drop unused labels.

> +                compatible = "mscc,ocelot-miim";
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +                reg = <0x7107009c>;
> +
> +                sw_phy0: ethernet-phy@0 {
> +                    reg = <0x0>;
> +                };
> +            };
> +
> +            mdio1: mdio@710700c0 {
> +                compatible = "mscc,ocelot-miim";
> +                pinctrl-names = "default";
> +                pinctrl-0 = <&miim1_pins>;
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +                reg = <0x710700c0>;
> +
> +                sw_phy4: ethernet-phy@4 {
> +                    reg = <0x4>;
> +                };
> +            };
> +
> +            gpio: pinctrl@71070034 {
> +                compatible = "mscc,ocelot-pinctrl";
> +                gpio-controller;
> +                #gpio-cells = <2>;
> +                gpio-ranges = <&gpio 0 0 22>;
> +                reg = <0x71070034>;
> +
> +                sgpio_pins: sgpio-pins {
> +                    pins = "GPIO_0", "GPIO_1", "GPIO_2", "GPIO_3";
> +                    function = "sg0";
> +                };
> +
> +                miim1_pins: miim1-pins {
> +                    pins = "GPIO_14", "GPIO_15";
> +                    function = "miim";
> +                };
> +            };
> +
> +            sgpio: gpio@710700f8 {
> +                compatible = "mscc,ocelot-sgpio";
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +                bus-frequency = <12500000>;
> +                clocks = <&ocelot_clock>;
> +                microchip,sgpio-port-ranges = <0 15>;
> +                pinctrl-names = "default";
> +                pinctrl-0 = <&sgpio_pins>;
> +                reg = <0x710700f8>;
> +
> +                sgpio_in0: gpio@0 {
> +                    compatible = "microchip,sparx5-sgpio-bank";
> +                    reg = <0>;
> +                    gpio-controller;
> +                    #gpio-cells = <3>;
> +                    ngpios = <64>;
> +                };
> +
> +                sgpio_out1: gpio@1 {
> +                    compatible = "microchip,sparx5-sgpio-bank";
> +                    reg = <1>;
> +                    gpio-controller;
> +                    #gpio-cells = <3>;
> +                    ngpios = <64>;
> +                };
> +            };
> +        };
> +    };
> +
> +...
> +
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 91b4151c5ad1..119fb4207ba3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14355,6 +14355,7 @@ F:	tools/testing/selftests/drivers/net/ocelot/*
>  OCELOT EXTERNAL SWITCH CONTROL
>  M:	Colin Foster <colin.foster@in-advantage.com>
>  S:	Supported
> +F:	Documentation/devicetree/bindings/mfd/mscc,ocelot.yaml
>  F:	include/linux/mfd/ocelot.h
>  
>  OCXL (Open Coherent Accelerator Processor Interface OpenCAPI) DRIVER
> -- 
> 2.25.1
> 
>