Hi netdev,
Recently I had been working on implementing DSA driver for MaxLinear GSW145
switch. Here is my initial draft. Before proceeding futher, I am hoping to
get comments and cooperation as early/many as possible.
On my hand, I have an arm64 board with one GSW145 chip being connected via
MDIO management interface. According to the public datasheet of GSW145, the
chip itself on my board is in self-start (PS_NOWAIT=1b), managed switch
(PS_OP_MD=11) mode. On this board, only two slave ethernet ports (port0,
port1) of this chip are enabled.
As its datasheet says, this chip supports MDIO, SPI, UART management
interfaces in many operation modes (self-start/non-self-start,
managed/standalone). Unfortunately it is impossible for me to run this
draft on all of these combinations. It will be very helpful if anyone can
help to run it on different hardware setups and send the feedback back to
me.
I also noticed that lantiq_gswip.c looks similar as gsw145. I have no
access to the datasheet of the chip this file supports. Maybe they can be
merged together.
Best Regards
Camel Guo
Camel Guo (2):
dt-bindings: net: dsa: add bindings for GSW Series switches
net: dsa: Add driver for Maxlinear GSW1XX switch
.../devicetree/bindings/net/dsa/mxl,gsw.yaml | 140 +++
.../devicetree/bindings/vendor-prefixes.yaml | 2 +
MAINTAINERS | 9 +
drivers/net/dsa/Kconfig | 16 +
drivers/net/dsa/Makefile | 2 +
drivers/net/dsa/gsw1xx.h | 27 +
drivers/net/dsa/gsw1xx_core.c | 823 ++++++++++++++++++
drivers/net/dsa/gsw1xx_mdio.c | 128 +++
8 files changed, 1147 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/dsa/mxl,gsw.yaml
create mode 100644 drivers/net/dsa/gsw1xx.h
create mode 100644 drivers/net/dsa/gsw1xx_core.c
create mode 100644 drivers/net/dsa/gsw1xx_mdio.c
base-commit: 6143eca3578f486e4d58fe6fb5e96a5699c86fbc
--
2.30.2
Add documentation and an example for Maxlinear's GSW Series Ethernet
switches.
Signed-off-by: Camel Guo <[email protected]>
---
.../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
@@ -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#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Maxlinear GSW Series Switch Device Tree Bindings
+
+allOf:
+ - $ref: dsa.yaml#
+
+maintainers:
+ - Camel Guo <[email protected]>
+
+description:
+ The Maxlinear's GSW Series Ethernet Switch is a highly integrated, low-power,
+ non-blocking Gigabit Ethernet Switch.
+
+properties:
+ compatible:
+ oneOf:
+ - enum:
+ - mxl,gsw145-mdio
+
+ reg:
+ maxItems: 1
+
+ mdio:
+ type: object
+
+ description:
+ Container of ethernet phy devices on the MDIO bus of GSW switch
+
+ properties:
+ '#address-cells':
+ const: 1
+ '#size-cells':
+ const: 0
+
+ allOf:
+ - $ref: "http://devicetree.org/schemas/net/ethernet-phy.yaml#"
+
+required:
+ - compatible
+ - reg
+ - mdio
+
+additionalProperties: true
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+
+ mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ switch@0 {
+ compatible = "mxl,gsw145-mdio";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x0>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ label = "lan0";
+ phy-mode = "internal";
+ phy-handle = <&phy0>;
+ };
+
+ port@1 {
+ reg = <1>;
+ label = "lan1";
+ phy-mode = "internal";
+ phy-handle = <&phy1>;
+ };
+
+ port@2 {
+ reg = <2>;
+ label = "lan2";
+ phy-mode = "internal";
+ phy-handle = <&phy2>;
+ status = "disabled";
+ };
+
+ port@3 {
+ reg = <3>;
+ label = "lan3";
+ phy-mode = "internal";
+ phy-handle = <&phy3>;
+ status = "disabled";
+ };
+
+ port@4 {
+ reg = <4>;
+ label = "lan4";
+ phy-mode = "sgmii";
+ status = "disabled";
+ };
+
+ port@5 {
+ reg = <5>;
+ label = "cpu";
+ ethernet = <ð0>;
+ phy-mode = "rgmii-id";
+
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ pause;
+ };
+ };
+ };
+
+ mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ phy0: ethernet-phy@0 {
+ reg = <0>;
+ };
+
+ phy1: ethernet-phy@1 {
+ reg = <1>;
+ };
+
+ phy2: ethernet-phy@2 {
+ reg = <2>;
+ };
+
+ phy3: ethernet-phy@3 {
+ reg = <3>;
+ };
+ };
+ };
+ };
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 6e323a380294..1d209115692c 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -863,6 +863,8 @@ patternProperties:
deprecated: true
"^mxicy,.*":
description: Macronix International Co., Ltd.
+ "^mxl,.*":
+ description: MaxLinear, Inc.
"^myir,.*":
description: MYIR Tech Limited
"^national,.*":
diff --git a/MAINTAINERS b/MAINTAINERS
index 657b223ed6b0..df88faabdb53 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12585,6 +12585,12 @@ L: [email protected]
S: Supported
F: drivers/net/phy/mxl-gpy.c
+MAXLINEAR GSW1XX SERIES ETHERNET SWITCH DRIVER
+M: Camel Guo <[email protected]>
+L: [email protected]
+S: Maintained
+F: Documentation/devicetree/bindings/net/dsa/mxl,gsw.yaml
+
MCBA MICROCHIP CAN BUS ANALYZER TOOL DRIVER
R: Yasushi SHOJI <[email protected]>
L: [email protected]
--
2.30.2
On Tue, 25 Oct 2022 15:52:40 +0200, Camel Guo wrote:
> Add documentation and an example for Maxlinear's GSW Series Ethernet
> switches.
>
> Signed-off-by: Camel Guo <[email protected]>
> ---
> .../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
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/dsa/mxl,gsw.yaml: properties:mdio:allOf:0:$ref: 'http://devicetree.org/schemas/net/ethernet-phy.yaml#' should not be valid under {'pattern': '^https?://'}
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/dsa/mxl,gsw.example.dtb: switch@0: mdio: 'reg' is a required property
From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/dsa/mxl,gsw.yaml
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
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 <[email protected]>
> >> ---
> >> .../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 <[email protected]>
> >> +
> >> +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
Hi Camel,
On Thu, Oct 27, 2022 at 08:46:27AM -0400, Krzysztof Kozlowski wrote:
> > >> +????? - 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.
Let's compare apples to apples, shall we?
"nxp,sja1110-base-t1-mdio" is the 100Base-T1 MDIO controller of the
NXP SJA1110 switch, hence the name. It is not a SJA1110 switch connected
over MDIO.
Hi Camel,
On Tue, Oct 25, 2022 at 03:52:40PM +0200, Camel Guo wrote:
> +additionalProperties: true
I don't think the switch schema should have additionalProperties: true.
Only shared schemas should. WHat should be here is "unevaluatedProperties: false".
> + port@5 {
> + reg = <5>;
> + label = "cpu";
Please drop label = "cpu" for the CPU port, it is not needed/not parsed.
> + ethernet = <ð0>;
> + phy-mode = "rgmii-id";
> +
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + pause;
> + };
> + };
> + };
On Thu, Oct 27, 2022 at 12:08:06PM -0400, Krzysztof Kozlowski wrote:
> On 27/10/2022 09:57, Vladimir Oltean wrote:
> > Hi Camel,
> >
> > On Thu, Oct 27, 2022 at 08:46:27AM -0400, Krzysztof Kozlowski wrote:
> >>> >> +????? - 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.
> >
> > Let's compare apples to apples, shall we?
> > "nxp,sja1110-base-t1-mdio" is the 100Base-T1 MDIO controller of the
> > NXP SJA1110 switch, hence the name. It is not a SJA1110 switch connected
> > over MDIO.
>
> Thanks for clarifying. Then this could be fine. Let me then explain what
> is discouraged:
> 1. Adding bus suffixes to the compatible, so for example foo,bar LED
> controller is on I2C bus, so you call it "foo,bar-i2c".
>
> 2. Adding device types to the compatible, if this is the only
> function/variant of the device, so for example calling foo,bar LED
> controller "foo,bar-led". This makes sense in case of multi functional
> devices (PMICs, SoCs), but not standalone ones.
>
> So what do we have here? Is it one of the cases above?
We are in agreement about SJA1110 (it's in case 2), this is what I said
about Camel's comparison not being apples to apples. "mxl,gsw145-mdio"
is in case 1, and I've also been recommending people to not add such
suffixes to compatible strings (also see the discussion with Colin
Foster about "mscc,vsc7512-switch" vs "mscc,vsc7512-ext-switch" to
denote an "external" switch which is otherwise the exact same hw but on
a different bus).
On 27/10/2022 09:57, Vladimir Oltean wrote:
> Hi Camel,
>
> On Thu, Oct 27, 2022 at 08:46:27AM -0400, Krzysztof Kozlowski wrote:
>>> >> + - 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.
>
> Let's compare apples to apples, shall we?
> "nxp,sja1110-base-t1-mdio" is the 100Base-T1 MDIO controller of the
> NXP SJA1110 switch, hence the name. It is not a SJA1110 switch connected
> over MDIO.
Thanks for clarifying. Then this could be fine. Let me then explain what
is discouraged:
1. Adding bus suffixes to the compatible, so for example foo,bar LED
controller is on I2C bus, so you call it "foo,bar-i2c".
2. Adding device types to the compatible, if this is the only
function/variant of the device, so for example calling foo,bar LED
controller "foo,bar-led". This makes sense in case of multi functional
devices (PMICs, SoCs), but not standalone ones.
So what do we have here? Is it one of the cases above?
Best regards,
Krzysztof