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;
Message-ID: <a7f75d47-30e7-d076-a9fd-baa57688bbf7@linaro.org>
Date:   Thu, 27 Oct 2022 08:46:27 -0400
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.0
Subject: Re: [RFC net-next 1/2] dt-bindings: net: dsa: add bindings for GSW
 Series switches
Content-Language: en-US
To:     Camel Guo <camelg@axis.com>, Camel Guo <Camel.Guo@axis.com>,
        Andrew Lunn <andrew@lunn.ch>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Rob Herring <robh+dt@kernel.org>,
        Russell King <linux@armlinux.org.uk>,
        Vivien Didelot <vivien.didelot@gmail.com>,
        Vladimir Oltean <olteanv@gmail.com>
Cc:     "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        Rob Herring <robh@kernel.org>, kernel <kernel@axis.com>
References: <20221025135243.4038706-1-camel.guo@axis.com>
 <20221025135243.4038706-2-camel.guo@axis.com>
 <16aac887-232a-7141-cc65-eab19c532592@linaro.org>
 <d0179725-0730-5826-caa4-228469d3bad4@axis.com>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <d0179725-0730-5826-caa4-228469d3bad4@axis.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 27/10/2022 02:34, Camel Guo wrote:
> On 10/25/22 16:27, Krzysztof Kozlowski wrote:
>  > On 25/10/2022 09:52, Camel Guo wrote:
>  >> Add documentation and an example for Maxlinear's GSW Series Ethernet
>  >> switches.
>  >>
>  >> Signed-off-by: Camel Guo <camel.guo@axis.com>
>  >> ---
>  >>  .../devicetree/bindings/net/dsa/mxl,gsw.yaml  | 140 ++++++++++++++++++
>  >>  .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
>  >>  MAINTAINERS                                   |   6 +
>  >>  3 files changed, 148 insertions(+)
>  >>  create mode 100644 
> Documentation/devicetree/bindings/net/dsa/mxl,gsw.yaml
>  >>
>  >> diff --git a/Documentation/devicetree/bindings/net/dsa/mxl,gsw.yaml 
> b/Documentation/devicetree/bindings/net/dsa/mxl,gsw.yaml
>  >> new file mode 100644
>  >> index 000000000000..8e124b7ec58c
>  >> --- /dev/null
>  >> +++ b/Documentation/devicetree/bindings/net/dsa/mxl,gsw.yaml
>  >
>  > Filename based on compatible, so mxl,gsw145-mdio.yaml. But see below.
> 
> I hope in future more gsw1xx chips (e.g: GSW150, GSW120) can be added
> and more management modes (e.g: uart, spi) can be supported. 

Maybe you will add support, maybe not. If these compatibles were
mentioned now, would be different topic, but there are not.

> So I think
> maybe mxl.gsw.yaml is more generic. Otherwise maybe in future someone
> has to add files like mxl,gsw150-uart.yaml, mxl,gsw120-spi.yaml

No, they can be added here, with or without renaming the file.

> 
>  >
>  >> @@ -0,0 +1,140 @@
>  >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>  >> +%YAML 1.2
>  >> +---
>  >> +$id: http://devicetree.org/schemas/net/dsa/mxl,gsw.yaml#
>  > <http://devicetree.org/schemas/net/dsa/mxl,gsw.yaml#>
>  >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>  > <http://devicetree.org/meta-schemas/core.yaml#>
>  >> +
>  >> +title: Maxlinear GSW Series Switch Device Tree Bindings
>  >
>  > Drop "Device Tree Bindings"
> 
> Ack. will do in v2
> 
>  >
>  >> +
>  >> +allOf:
>  >> +  - $ref: dsa.yaml#
>  >> +
>  >> +maintainers:
>  >> +  - Camel Guo <camel.guo@axis.com>
>  >> +
>  >> +description:
>  >> +  The Maxlinear's GSW Series Ethernet Switch is a highly 
> integrated, low-power,
>  >> +  non-blocking Gigabit Ethernet Switch.
>  >> +
>  >> +properties:
>  >> +  compatible:
>  >> +    oneOf:
>  >
>  > You do not have multiple choices, so no need for oneOf >
> 
> Currently that is true. But according to the datasheet of gsw1xx, it
> could be easily to expand to other gsw1xx chips and other management
> interfaces (e.g: spi, uart). If so, we could add something like:
> mxl,gsw145-spi
> mxl,gsw150-mdio


So expand it now... Anyway enum allows you to add new types, so why why
oneOf?

> ...
> 
> 
>  >> +      - enum:
>  >> +          - mxl,gsw145-mdio
>  >
>  > Why "mdio" suffix?
> 
> Inspired by others dsa chips.
> lan9303.txt:  - "smsc,lan9303-mdio" for mdio managed mode
> lantiq-gswip.txt:- compatible   : "lantiq,xrx200-mdio" for the MDIO bus
> inside the GSWIP
> nxp,sja1105.yaml:                  - nxp,sja1110-base-t1-mdio

As I replied to Andrew, this is discouraged.

Best regards,
Krzysztof