Hi,
This patch series fixes the various Broadcom SoCs DTS files and the
existing YAML binding for missing properties before adding a proper b53
switch YAML binding from Kurt.
If this all looks good, given that there are quite a few changes to the
DTS files, it might be best if I take them through the upcoming Broadcom
ARM SoC pull requests. Let me know if you would like those patches to be
applied differently.
Thanks!
Florian Fainelli (9):
dt-bindings: net: dsa: Extend switch nodes pattern
dt-bindings: net: dsa: Document sfp and managed properties
ARM: dts: BCM5301X: Update Ethernet switch node name
ARM: dts: BCM5301X: Add a default compatible for switch node
ARM: dts: BCM5301X: Provide defaults ports container node
ARM: dts: NSP: Update ethernet switch node name
ARM: dts: NSP: Fix Ethernet switch SGMII register name
ARM: dts: NSP: Add a default compatible for switch node
ARM: dts: NSP: Provide defaults ports container node
Kurt Kanzenbach (1):
dt-bindings: net: dsa: b53: Add YAML bindings
.../devicetree/bindings/net/dsa/b53.txt | 149 -----------
.../devicetree/bindings/net/dsa/b53.yaml | 249 ++++++++++++++++++
.../devicetree/bindings/net/dsa/dsa.yaml | 6 +-
MAINTAINERS | 2 +-
arch/arm/boot/dts/bcm-nsp.dtsi | 10 +-
arch/arm/boot/dts/bcm5301x.dtsi | 8 +-
6 files changed, 268 insertions(+), 156 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/net/dsa/b53.txt
create mode 100644 Documentation/devicetree/bindings/net/dsa/b53.yaml
--
2.25.1
Provide an empty 'ports' container node with the correct #address-cells
and #size-cells properties. This silences the following warning:
arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dt.yaml:
ethernet-switch@18007000: 'oneOf' conditional failed, one must be fixed:
'ports' is a required property
'ethernet-ports' is a required property
From schema:
Documentation/devicetree/bindings/net/dsa/b53.yaml
Signed-off-by: Florian Fainelli <[email protected]>
---
arch/arm/boot/dts/bcm5301x.dtsi | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi
index 807580dd89f5..89993a8a6765 100644
--- a/arch/arm/boot/dts/bcm5301x.dtsi
+++ b/arch/arm/boot/dts/bcm5301x.dtsi
@@ -489,6 +489,10 @@ srab: ethernet-switch@18007000 {
status = "disabled";
/* ports are defined in board DTS */
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
};
rng: rng@18004000 {
--
2.25.1
Update the switch unit name from srab to ethernet-switch, allowing us to
fix warnings such as:
CHECK arch/arm/boot/dts/bcm4708-buffalo-wzr-1750dhp.dt.yaml
arch/arm/boot/dts/bcm4708-buffalo-wzr-1750dhp.dt.yaml:
srab@18007000: $nodename:0: 'srab@18007000' does not match
'^(ethernet-)?switch(@.*)?$'
From schema:
Documentation/devicetree/bindings/net/dsa/b53.yaml
Signed-off-by: Florian Fainelli <[email protected]>
---
arch/arm/boot/dts/bcm5301x.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi
index ac3a99cf2079..ee23c0841699 100644
--- a/arch/arm/boot/dts/bcm5301x.dtsi
+++ b/arch/arm/boot/dts/bcm5301x.dtsi
@@ -482,7 +482,7 @@ thermal: thermal@1800c2c0 {
#thermal-sensor-cells = <0>;
};
- srab: srab@18007000 {
+ srab: ethernet-switch@18007000 {
compatible = "brcm,bcm5301x-srab";
reg = <0x18007000 0x1000>;
--
2.25.1
From: Kurt Kanzenbach <[email protected]>
Convert the b53 DSA device tree bindings to YAML in order to allow
for automatic checking and such.
Suggested-by: Florian Fainelli <[email protected]>
Signed-off-by: Kurt Kanzenbach <[email protected]>
---
.../devicetree/bindings/net/dsa/b53.txt | 149 -----------
.../devicetree/bindings/net/dsa/b53.yaml | 249 ++++++++++++++++++
MAINTAINERS | 2 +-
3 files changed, 250 insertions(+), 150 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/net/dsa/b53.txt
create mode 100644 Documentation/devicetree/bindings/net/dsa/b53.yaml
diff --git a/Documentation/devicetree/bindings/net/dsa/b53.txt b/Documentation/devicetree/bindings/net/dsa/b53.txt
deleted file mode 100644
index f1487a751b1a..000000000000
--- a/Documentation/devicetree/bindings/net/dsa/b53.txt
+++ /dev/null
@@ -1,149 +0,0 @@
-Broadcom BCM53xx Ethernet switches
-==================================
-
-Required properties:
-
-- compatible: For external switch chips, compatible string must be exactly one
- of: "brcm,bcm5325"
- "brcm,bcm53115"
- "brcm,bcm53125"
- "brcm,bcm53128"
- "brcm,bcm5365"
- "brcm,bcm5395"
- "brcm,bcm5389"
- "brcm,bcm5397"
- "brcm,bcm5398"
-
- For the BCM11360 SoC, must be:
- "brcm,bcm11360-srab" and the mandatory "brcm,cygnus-srab" string
-
- For the BCM5310x SoCs with an integrated switch, must be one of:
- "brcm,bcm53010-srab"
- "brcm,bcm53011-srab"
- "brcm,bcm53012-srab"
- "brcm,bcm53018-srab"
- "brcm,bcm53019-srab" and the mandatory "brcm,bcm5301x-srab" string
-
- For the BCM5831X/BCM1140x SoCs with an integrated switch, must be one of:
- "brcm,bcm11404-srab"
- "brcm,bcm11407-srab"
- "brcm,bcm11409-srab"
- "brcm,bcm58310-srab"
- "brcm,bcm58311-srab"
- "brcm,bcm58313-srab" and the mandatory "brcm,omega-srab" string
-
- For the BCM585xx/586XX/88312 SoCs with an integrated switch, must be one of:
- "brcm,bcm58522-srab"
- "brcm,bcm58523-srab"
- "brcm,bcm58525-srab"
- "brcm,bcm58622-srab"
- "brcm,bcm58623-srab"
- "brcm,bcm58625-srab"
- "brcm,bcm88312-srab" and the mandatory "brcm,nsp-srab string
-
- For the BCM63xx/33xx SoCs with an integrated switch, must be one of:
- "brcm,bcm3384-switch"
- "brcm,bcm6328-switch"
- "brcm,bcm6368-switch" and the mandatory "brcm,bcm63xx-switch"
-
-Required properties for BCM585xx/586xx/88312 SoCs:
-
- - reg: a total of 3 register base addresses, the first one must be the
- Switch Register Access block base, the second is the port 5/4 mux
- configuration register and the third one is the SGMII configuration
- and status register base address.
-
- - interrupts: a total of 13 interrupts must be specified, in the following
- order: port 0-5, 7-8 link status change, then the integrated PHY interrupt,
- then the timestamping interrupt and the sleep timer interrupts for ports
- 5,7,8.
-
-Optional properties for BCM585xx/586xx/88312 SoCs:
-
- - reg-names: a total of 3 names matching the 3 base register address, must
- be in the following order:
- "srab"
- "mux_config"
- "sgmii_config"
-
- - interrupt-names: a total of 13 names matching the 13 interrupts specified
- must be in the following order:
- "link_state_p0"
- "link_state_p1"
- "link_state_p2"
- "link_state_p3"
- "link_state_p4"
- "link_state_p5"
- "link_state_p7"
- "link_state_p8"
- "phy"
- "ts"
- "imp_sleep_timer_p5"
- "imp_sleep_timer_p7"
- "imp_sleep_timer_p8"
-
-See Documentation/devicetree/bindings/net/dsa/dsa.txt for a list of additional
-required and optional properties.
-
-Examples:
-
-Ethernet switch connected via MDIO to the host, CPU port wired to eth0:
-
- eth0: ethernet@10001000 {
- compatible = "brcm,unimac";
- reg = <0x10001000 0x1000>;
-
- fixed-link {
- speed = <1000>;
- full-duplex;
- };
- };
-
- mdio0: mdio@10000000 {
- compatible = "brcm,unimac-mdio";
- #address-cells = <1>;
- #size-cells = <0>;
-
- switch0: ethernet-switch@1e {
- compatible = "brcm,bcm53125";
- reg = <30>;
- #address-cells = <1>;
- #size-cells = <0>;
-
- ports {
- #address-cells = <1>;
- #size-cells = <0>;
-
- port0@0 {
- reg = <0>;
- label = "lan1";
- };
-
- port1@1 {
- reg = <1>;
- label = "lan2";
- };
-
- port5@5 {
- reg = <5>;
- label = "cable-modem";
- fixed-link {
- speed = <1000>;
- full-duplex;
- };
- phy-mode = "rgmii-txid";
- };
-
- port8@8 {
- reg = <8>;
- label = "cpu";
- fixed-link {
- speed = <1000>;
- full-duplex;
- };
- phy-mode = "rgmii-txid";
- ethernet = <ð0>;
- };
- };
- };
- };
diff --git a/Documentation/devicetree/bindings/net/dsa/b53.yaml b/Documentation/devicetree/bindings/net/dsa/b53.yaml
new file mode 100644
index 000000000000..4fcbac1de95b
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/dsa/b53.yaml
@@ -0,0 +1,249 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/dsa/b53.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom BCM53xx Ethernet switches
+
+allOf:
+ - $ref: dsa.yaml#
+
+maintainers:
+ - Florian Fainelli <[email protected]>
+
+description:
+ Broadcom BCM53xx Ethernet switches
+
+properties:
+ compatible:
+ oneOf:
+ - const: brcm,bcm5325
+ - const: brcm,bcm53115
+ - const: brcm,bcm53125
+ - const: brcm,bcm53128
+ - const: brcm,bcm5365
+ - const: brcm,bcm5395
+ - const: brcm,bcm5389
+ - const: brcm,bcm5397
+ - const: brcm,bcm5398
+ - items:
+ - const: brcm,bcm11360-srab
+ - const: brcm,cygnus-srab
+ - items:
+ - enum:
+ - brcm,bcm53010-srab
+ - brcm,bcm53011-srab
+ - brcm,bcm53012-srab
+ - brcm,bcm53018-srab
+ - brcm,bcm53019-srab
+ - const: brcm,bcm5301x-srab
+ - items:
+ - enum:
+ - brcm,bcm11404-srab
+ - brcm,bcm11407-srab
+ - brcm,bcm11409-srab
+ - brcm,bcm58310-srab
+ - brcm,bcm58311-srab
+ - brcm,bcm58313-srab
+ - const: brcm,omega-srab
+ - items:
+ - enum:
+ - brcm,bcm58522-srab
+ - brcm,bcm58523-srab
+ - brcm,bcm58525-srab
+ - brcm,bcm58622-srab
+ - brcm,bcm58623-srab
+ - brcm,bcm58625-srab
+ - brcm,bcm88312-srab
+ - const: brcm,nsp-srab
+ - items:
+ - enum:
+ - brcm,bcm3384-switch
+ - brcm,bcm6328-switch
+ - brcm,bcm6368-switch
+ - const: brcm,bcm63xx-switch
+
+required:
+ - compatible
+ - reg
+
+# BCM585xx/586xx/88312 SoCs
+if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - brcm,bcm58522-srab
+ - brcm,bcm58523-srab
+ - brcm,bcm58525-srab
+ - brcm,bcm58622-srab
+ - brcm,bcm58623-srab
+ - brcm,bcm58625-srab
+ - brcm,bcm88312-srab
+then:
+ properties:
+ reg:
+ minItems: 3
+ maxItems: 3
+ reg-names:
+ items:
+ - const: srab
+ - const: mux_config
+ - const: sgmii_config
+ interrupts:
+ minItems: 13
+ maxItems: 13
+ interrupt-names:
+ items:
+ - const: link_state_p0
+ - const: link_state_p1
+ - const: link_state_p2
+ - const: link_state_p3
+ - const: link_state_p4
+ - const: link_state_p5
+ - const: link_state_p7
+ - const: link_state_p8
+ - const: phy
+ - const: ts
+ - const: imp_sleep_timer_p5
+ - const: imp_sleep_timer_p7
+ - const: imp_sleep_timer_p8
+ required:
+ - interrupts
+else:
+ properties:
+ reg:
+ maxItems: 1
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ switch@1e {
+ compatible = "brcm,bcm53125";
+ reg = <30>;
+
+ ethernet-ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ label = "lan1";
+ };
+
+ port@1 {
+ reg = <1>;
+ label = "lan2";
+ };
+
+ port@5 {
+ reg = <5>;
+ label = "cable-modem";
+ phy-mode = "rgmii-txid";
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+ };
+
+ port@8 {
+ reg = <8>;
+ label = "cpu";
+ phy-mode = "rgmii-txid";
+ ethernet = <ð0>;
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+ };
+ };
+ };
+ };
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ axi {
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ switch@36000 {
+ compatible = "brcm,bcm58623-srab", "brcm,nsp-srab";
+ reg = <0x36000 0x1000>,
+ <0x3f308 0x8>,
+ <0x3f410 0xc>;
+ reg-names = "srab", "mux_config", "sgmii_config";
+ interrupts = <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 107 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "link_state_p0",
+ "link_state_p1",
+ "link_state_p2",
+ "link_state_p3",
+ "link_state_p4",
+ "link_state_p5",
+ "link_state_p7",
+ "link_state_p8",
+ "phy",
+ "ts",
+ "imp_sleep_timer_p5",
+ "imp_sleep_timer_p7",
+ "imp_sleep_timer_p8";
+
+ ethernet-ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ label = "port0";
+ reg = <0>;
+ };
+
+ port@1 {
+ label = "port1";
+ reg = <1>;
+ };
+
+ port@2 {
+ label = "port2";
+ reg = <2>;
+ };
+
+ port@3 {
+ label = "port3";
+ reg = <3>;
+ };
+
+ port@4 {
+ label = "port4";
+ reg = <4>;
+ };
+
+ port@8 {
+ ethernet = <&amac2>;
+ label = "cpu";
+ reg = <8>;
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+ };
+ };
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index 3da6d8c154e4..d2e2ea9eb527 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3377,7 +3377,7 @@ M: Florian Fainelli <[email protected]>
L: [email protected]
L: [email protected] (subscribers-only)
S: Supported
-F: Documentation/devicetree/bindings/net/dsa/b53.txt
+F: Documentation/devicetree/bindings/net/dsa/b53.yaml
F: drivers/net/dsa/b53/*
F: include/linux/platform_data/b53.h
--
2.25.1
Provide an empty 'ports' container node with the correct #address-cells
and #size-cells properties. This silences the following warning:
arch/arm/boot/dts/bcm958522er.dt.yaml:
ethernet-switch@36000: 'oneOf' conditional failed, one must be fixed:
'ports' is a required property
'ethernet-ports' is a required property
From schema:
Documentation/devicetree/bindings/net/dsa/b53.yaml
Signed-off-by: Florian Fainelli <[email protected]>
---
arch/arm/boot/dts/bcm-nsp.dtsi | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi
index 8453865d1439..d75bf37260c2 100644
--- a/arch/arm/boot/dts/bcm-nsp.dtsi
+++ b/arch/arm/boot/dts/bcm-nsp.dtsi
@@ -420,6 +420,10 @@ srab: ethernet-switch@36000 {
status = "disabled";
/* ports are defined in board DTS */
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
};
i2c0: i2c@38000 {
--
2.25.1
Provide a default compatible string which is based on the 58522 SRAB
compatible, this allows us to have sane defaults and silences the
following warnings:
arch/arm/boot/dts/bcm958522er.dt.yaml:
ethernet-switch@36000: compatible: 'oneOf' conditional failed,
one
must be fixed:
['brcm,bcm5301x-srab'] is too short
'brcm,bcm5325' was expected
'brcm,bcm53115' was expected
'brcm,bcm53125' was expected
'brcm,bcm53128' was expected
'brcm,bcm5365' was expected
'brcm,bcm5395' was expected
'brcm,bcm5389' was expected
'brcm,bcm5397' was expected
'brcm,bcm5398' was expected
'brcm,bcm11360-srab' was expected
'brcm,bcm5301x-srab' is not one of ['brcm,bcm53010-srab',
'brcm,bcm53011-srab', 'brcm,bcm53012-srab', 'brcm,bcm53018-srab',
'brcm,bcm53019-srab']
'brcm,bcm5301x-srab' is not one of ['brcm,bcm11404-srab',
'brcm,bcm11407-srab', 'brcm,bcm11409-srab', 'brcm,bcm58310-srab',
'brcm,bcm58311-srab', 'brcm,bcm58313-srab']
'brcm,bcm5301x-srab' is not one of ['brcm,bcm58522-srab',
'brcm,bcm58523-srab', 'brcm,bcm58525-srab', 'brcm,bcm58622-srab',
'brcm,bcm58623-srab', 'brcm,bcm58625-srab', 'brcm,bcm88312-srab']
'brcm,bcm5301x-srab' is not one of ['brcm,bcm3384-switch',
'brcm,bcm6328-switch', 'brcm,bcm6368-switch']
From schema:
Documentation/devicetree/bindings/net/dsa/b53.yaml
Signed-off-by: Florian Fainelli <[email protected]>
---
arch/arm/boot/dts/bcm-nsp.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi
index 09fd7e55c069..8453865d1439 100644
--- a/arch/arm/boot/dts/bcm-nsp.dtsi
+++ b/arch/arm/boot/dts/bcm-nsp.dtsi
@@ -386,7 +386,7 @@ ccbtimer1: timer@35000 {
};
srab: ethernet-switch@36000 {
- compatible = "brcm,nsp-srab";
+ compatible = "brcm,bcm58522-srab", "brcm,nsp-srab";
reg = <0x36000 0x1000>,
<0x3f308 0x8>,
<0x3f410 0xc>;
--
2.25.1
The register name should be "sgmii_config", not "sgmii", this is not a
functional change since no code is currently looking for that register
by name (or at all).
Signed-off-by: Florian Fainelli <[email protected]>
---
arch/arm/boot/dts/bcm-nsp.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi
index e7d08959d5fe..09fd7e55c069 100644
--- a/arch/arm/boot/dts/bcm-nsp.dtsi
+++ b/arch/arm/boot/dts/bcm-nsp.dtsi
@@ -390,7 +390,7 @@ srab: ethernet-switch@36000 {
reg = <0x36000 0x1000>,
<0x3f308 0x8>,
<0x3f410 0xc>;
- reg-names = "srab", "mux_config", "sgmii";
+ reg-names = "srab", "mux_config", "sgmii_config";
interrupts = <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>,
--
2.25.1
Update the switch unit name from srab to ethernet-switch, allowing us
to fix warnings such as:
CHECK arch/arm/boot/dts/bcm4708-buffalo-wzr-1750dhp.dt.yaml
arch/arm/boot/dts/bcm4708-buffalo-wzr-1750dhp.dt.yaml:
srab@18007000: $nodename:0: 'srab@18007000' does not match
'^(ethernet-)?switch(@.*)?$'
From schema:
Documentation/devicetree/bindings/net/dsa/b53.yaml
Signed-off-by: Florian Fainelli <[email protected]>
---
arch/arm/boot/dts/bcm-nsp.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi
index e895f7cb8c9f..e7d08959d5fe 100644
--- a/arch/arm/boot/dts/bcm-nsp.dtsi
+++ b/arch/arm/boot/dts/bcm-nsp.dtsi
@@ -385,7 +385,7 @@ ccbtimer1: timer@35000 {
clock-names = "apb_pclk";
};
- srab: srab@36000 {
+ srab: ethernet-switch@36000 {
compatible = "brcm,nsp-srab";
reg = <0x36000 0x1000>,
<0x3f308 0x8>,
--
2.25.1
Upon discussion with Kurt, Rob and Vladimir it appears that we should be
allowing ethernet-switch as a node name, update dsa.yaml accordingly.
Signed-off-by: Florian Fainelli <[email protected]>
---
Documentation/devicetree/bindings/net/dsa/dsa.yaml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/net/dsa/dsa.yaml b/Documentation/devicetree/bindings/net/dsa/dsa.yaml
index a765ceba28c6..5f8f5177938a 100644
--- a/Documentation/devicetree/bindings/net/dsa/dsa.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/dsa.yaml
@@ -20,7 +20,7 @@ select: false
properties:
$nodename:
- pattern: "^switch(@.*)?$"
+ pattern: "^(ethernet-)?switch(@.*)?$"
dsa,member:
minItems: 2
--
2.25.1
The 'sfp' and 'managed' properties are commonly used to describe
Ethernet switch ports connecting to SFP/SFF cages, describe these two
properties as valid that we inherit from ethernet-controller.yaml.
Signed-off-by: Florian Fainelli <[email protected]>
---
Documentation/devicetree/bindings/net/dsa/dsa.yaml | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/dsa/dsa.yaml b/Documentation/devicetree/bindings/net/dsa/dsa.yaml
index 5f8f5177938a..8e044631bcf7 100644
--- a/Documentation/devicetree/bindings/net/dsa/dsa.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/dsa.yaml
@@ -78,6 +78,10 @@ patternProperties:
mac-address: true
+ sfp: true
+
+ managed: true
+
required:
- reg
--
2.25.1
Provide a default compatible string which is based on the 53010 SRAB
compatible, this allows us to have sane defaults and silences the
following warnings:
arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dt.yaml:
ethernet-switch@18007000: compatible: 'oneOf' conditional failed, one
must be fixed:
['brcm,bcm5301x-srab'] is too short
'brcm,bcm5325' was expected
'brcm,bcm53115' was expected
'brcm,bcm53125' was expected
'brcm,bcm53128' was expected
'brcm,bcm5365' was expected
'brcm,bcm5395' was expected
'brcm,bcm5389' was expected
'brcm,bcm5397' was expected
'brcm,bcm5398' was expected
'brcm,bcm11360-srab' was expected
'brcm,bcm5301x-srab' is not one of ['brcm,bcm53010-srab',
'brcm,bcm53011-srab', 'brcm,bcm53012-srab', 'brcm,bcm53018-srab',
'brcm,bcm53019-srab']
'brcm,bcm5301x-srab' is not one of ['brcm,bcm11404-srab',
'brcm,bcm11407-srab', 'brcm,bcm11409-srab', 'brcm,bcm58310-srab',
'brcm,bcm58311-srab', 'brcm,bcm58313-srab']
'brcm,bcm5301x-srab' is not one of ['brcm,bcm58522-srab',
'brcm,bcm58523-srab', 'brcm,bcm58525-srab', 'brcm,bcm58622-srab',
'brcm,bcm58623-srab', 'brcm,bcm58625-srab', 'brcm,bcm88312-srab']
'brcm,bcm5301x-srab' is not one of ['brcm,bcm3384-switch',
'brcm,bcm6328-switch', 'brcm,bcm6368-switch']
From schema:
Documentation/devicetree/bindings/net/dsa/b53.yaml
Signed-off-by: Florian Fainelli <[email protected]>
---
arch/arm/boot/dts/bcm5301x.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi
index ee23c0841699..807580dd89f5 100644
--- a/arch/arm/boot/dts/bcm5301x.dtsi
+++ b/arch/arm/boot/dts/bcm5301x.dtsi
@@ -483,7 +483,7 @@ thermal: thermal@1800c2c0 {
};
srab: ethernet-switch@18007000 {
- compatible = "brcm,bcm5301x-srab";
+ compatible = "brcm,bcm53010-srab", "brcm,bcm5301x-srab";
reg = <0x18007000 0x1000>;
status = "disabled";
--
2.25.1
10.11.2020 04:31, Florian Fainelli wrote:
> Provide an empty 'ports' container node with the correct #address-cells
> and #size-cells properties. This silences the following warning:
>
> arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dt.yaml:
> ethernet-switch@18007000: 'oneOf' conditional failed, one must be fixed:
> 'ports' is a required property
> 'ethernet-ports' is a required property
> From schema:
> Documentation/devicetree/bindings/net/dsa/b53.yaml
>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
> arch/arm/boot/dts/bcm5301x.dtsi | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi
> index 807580dd89f5..89993a8a6765 100644
> --- a/arch/arm/boot/dts/bcm5301x.dtsi
> +++ b/arch/arm/boot/dts/bcm5301x.dtsi
> @@ -489,6 +489,10 @@ srab: ethernet-switch@18007000 {
> status = "disabled";
>
> /* ports are defined in board DTS */
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
You can drop those two lines from board files now I believe.
grep "ports {" arch/arm/boot/dts/bcm470*
+ arch/arm/boot/dts/bcm953012er.dts
On Mon Nov 09 2020, Florian Fainelli wrote:
> From: Kurt Kanzenbach <[email protected]>
>
> Convert the b53 DSA device tree bindings to YAML in order to allow
> for automatic checking and such.
>
> Suggested-by: Florian Fainelli <[email protected]>
> Signed-off-by: Kurt Kanzenbach <[email protected]>
> ---
> .../devicetree/bindings/net/dsa/b53.txt | 149 -----------
> .../devicetree/bindings/net/dsa/b53.yaml | 249 ++++++++++++++++++
Maybe it should be renamed to brcm,b53.yaml to be consistent with the
ksz and hellcreek bindings.
Thanks,
Kurt
On 11/10/2020 5:21 AM, Kurt Kanzenbach wrote:
> On Mon Nov 09 2020, Florian Fainelli wrote:
>> From: Kurt Kanzenbach <[email protected]>
>>
>> Convert the b53 DSA device tree bindings to YAML in order to allow
>> for automatic checking and such.
>>
>> Suggested-by: Florian Fainelli <[email protected]>
>> Signed-off-by: Kurt Kanzenbach <[email protected]>
>> ---
>> .../devicetree/bindings/net/dsa/b53.txt | 149 -----------
>> .../devicetree/bindings/net/dsa/b53.yaml | 249 ++++++++++++++++++
>
> Maybe it should be renamed to brcm,b53.yaml to be consistent with the
> ksz and hellcreek bindings.
Certainly.
--
Florian
On 11/10/2020 1:31 AM, Rafał Miłecki wrote:
> 10.11.2020 04:31, Florian Fainelli wrote:
>> Provide an empty 'ports' container node with the correct #address-cells
>> and #size-cells properties. This silences the following warning:
>>
>> arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dt.yaml:
>> ethernet-switch@18007000: 'oneOf' conditional failed, one must be fixed:
>> 'ports' is a required property
>> 'ethernet-ports' is a required property
>> From schema:
>> Documentation/devicetree/bindings/net/dsa/b53.yaml
>>
>> Signed-off-by: Florian Fainelli <[email protected]>
>> ---
>> arch/arm/boot/dts/bcm5301x.dtsi | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/bcm5301x.dtsi
>> b/arch/arm/boot/dts/bcm5301x.dtsi
>> index 807580dd89f5..89993a8a6765 100644
>> --- a/arch/arm/boot/dts/bcm5301x.dtsi
>> +++ b/arch/arm/boot/dts/bcm5301x.dtsi
>> @@ -489,6 +489,10 @@ srab: ethernet-switch@18007000 {
>> status = "disabled";
>> /* ports are defined in board DTS */
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + };
>
> You can drop those two lines from board files now I believe.
>
> grep "ports {" arch/arm/boot/dts/bcm470*
> + arch/arm/boot/dts/bcm953012er.dts
Yes, indeed, thanks!
--
Florian
On Mon, Nov 09, 2020 at 07:31:04PM -0800, Florian Fainelli wrote:
> Upon discussion with Kurt, Rob and Vladimir it appears that we should be
> allowing ethernet-switch as a node name, update dsa.yaml accordingly.
>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
Reviewed-by: Vladimir Oltean <[email protected]>
On Mon, Nov 09, 2020 at 07:31:06PM -0800, Florian Fainelli wrote:
> Update the switch unit name from srab to ethernet-switch, allowing us to
> fix warnings such as:
>
> CHECK arch/arm/boot/dts/bcm4708-buffalo-wzr-1750dhp.dt.yaml
> arch/arm/boot/dts/bcm4708-buffalo-wzr-1750dhp.dt.yaml:
> srab@18007000: $nodename:0: 'srab@18007000' does not match
> '^(ethernet-)?switch(@.*)?$'
> From schema:
> Documentation/devicetree/bindings/net/dsa/b53.yaml
>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
You're lucky that you don't have any firmware that fixes up DT
properties by path.
Reviewed-by: Vladimir Oltean <[email protected]>
On Mon, Nov 09, 2020 at 07:31:07PM -0800, Florian Fainelli wrote:
> Provide a default compatible string which is based on the 53010 SRAB
> compatible, this allows us to have sane defaults and silences the
> following warnings:
>
> arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dt.yaml:
> ethernet-switch@18007000: compatible: 'oneOf' conditional failed, one
> must be fixed:
> ['brcm,bcm5301x-srab'] is too short
> 'brcm,bcm5325' was expected
> 'brcm,bcm53115' was expected
> 'brcm,bcm53125' was expected
> 'brcm,bcm53128' was expected
> 'brcm,bcm5365' was expected
> 'brcm,bcm5395' was expected
> 'brcm,bcm5389' was expected
> 'brcm,bcm5397' was expected
> 'brcm,bcm5398' was expected
> 'brcm,bcm11360-srab' was expected
> 'brcm,bcm5301x-srab' is not one of ['brcm,bcm53010-srab',
> 'brcm,bcm53011-srab', 'brcm,bcm53012-srab', 'brcm,bcm53018-srab',
> 'brcm,bcm53019-srab']
> 'brcm,bcm5301x-srab' is not one of ['brcm,bcm11404-srab',
> 'brcm,bcm11407-srab', 'brcm,bcm11409-srab', 'brcm,bcm58310-srab',
> 'brcm,bcm58311-srab', 'brcm,bcm58313-srab']
> 'brcm,bcm5301x-srab' is not one of ['brcm,bcm58522-srab',
> 'brcm,bcm58523-srab', 'brcm,bcm58525-srab', 'brcm,bcm58622-srab',
> 'brcm,bcm58623-srab', 'brcm,bcm58625-srab', 'brcm,bcm88312-srab']
> 'brcm,bcm5301x-srab' is not one of ['brcm,bcm3384-switch',
> 'brcm,bcm6328-switch', 'brcm,bcm6368-switch']
> From schema:
> Documentation/devicetree/bindings/net/dsa/b53.yaml
>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
Nice, I didn't know DSA supported the switch inside this device. In the
default AsusWRT, the switch is well hidden from the kernel :)
Not that it makes any difference as far as I can see, but how do you
know this a BCM53010 SRAB specifically?
> arch/arm/boot/dts/bcm5301x.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi
> index ee23c0841699..807580dd89f5 100644
> --- a/arch/arm/boot/dts/bcm5301x.dtsi
> +++ b/arch/arm/boot/dts/bcm5301x.dtsi
> @@ -483,7 +483,7 @@ thermal: thermal@1800c2c0 {
> };
>
> srab: ethernet-switch@18007000 {
> - compatible = "brcm,bcm5301x-srab";
> + compatible = "brcm,bcm53010-srab", "brcm,bcm5301x-srab";
> reg = <0x18007000 0x1000>;
>
> status = "disabled";
> --
> 2.25.1
>
On 11/10/20 2:06 PM, Vladimir Oltean wrote:
> On Mon, Nov 09, 2020 at 07:31:07PM -0800, Florian Fainelli wrote:
>> Provide a default compatible string which is based on the 53010 SRAB
>> compatible, this allows us to have sane defaults and silences the
>> following warnings:
>>
>> arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dt.yaml:
>> ethernet-switch@18007000: compatible: 'oneOf' conditional failed, one
>> must be fixed:
>> ['brcm,bcm5301x-srab'] is too short
>> 'brcm,bcm5325' was expected
>> 'brcm,bcm53115' was expected
>> 'brcm,bcm53125' was expected
>> 'brcm,bcm53128' was expected
>> 'brcm,bcm5365' was expected
>> 'brcm,bcm5395' was expected
>> 'brcm,bcm5389' was expected
>> 'brcm,bcm5397' was expected
>> 'brcm,bcm5398' was expected
>> 'brcm,bcm11360-srab' was expected
>> 'brcm,bcm5301x-srab' is not one of ['brcm,bcm53010-srab',
>> 'brcm,bcm53011-srab', 'brcm,bcm53012-srab', 'brcm,bcm53018-srab',
>> 'brcm,bcm53019-srab']
>> 'brcm,bcm5301x-srab' is not one of ['brcm,bcm11404-srab',
>> 'brcm,bcm11407-srab', 'brcm,bcm11409-srab', 'brcm,bcm58310-srab',
>> 'brcm,bcm58311-srab', 'brcm,bcm58313-srab']
>> 'brcm,bcm5301x-srab' is not one of ['brcm,bcm58522-srab',
>> 'brcm,bcm58523-srab', 'brcm,bcm58525-srab', 'brcm,bcm58622-srab',
>> 'brcm,bcm58623-srab', 'brcm,bcm58625-srab', 'brcm,bcm88312-srab']
>> 'brcm,bcm5301x-srab' is not one of ['brcm,bcm3384-switch',
>> 'brcm,bcm6328-switch', 'brcm,bcm6368-switch']
>> From schema:
>> Documentation/devicetree/bindings/net/dsa/b53.yaml
>>
>> Signed-off-by: Florian Fainelli <[email protected]>
>> ---
>
> Nice, I didn't know DSA supported the switch inside this device. In the
> default AsusWRT, the switch is well hidden from the kernel :)
Yeah right :) The YAML validation works even when the nodes are
disabled, so we somehow need to account for all of these devices' DTS.
>
> Not that it makes any difference as far as I can see, but how do you
> know this a BCM53010 SRAB specifically?
The 4708 is effectively a 53010 which is why this is valid as a
fallback. Any node that defines switch ports and uses a SoC different
than the 53010 should use an appropriate "compatible" string though.
That's what is done for NSP.
--
Florian
On Mon, Nov 09, 2020 at 07:31:08PM -0800, Florian Fainelli wrote:
> Provide an empty 'ports' container node with the correct #address-cells
> and #size-cells properties. This silences the following warning:
>
> arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dt.yaml:
> ethernet-switch@18007000: 'oneOf' conditional failed, one must be fixed:
> 'ports' is a required property
> 'ethernet-ports' is a required property
> From schema:
> Documentation/devicetree/bindings/net/dsa/b53.yaml
>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
> arch/arm/boot/dts/bcm5301x.dtsi | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi
> index 807580dd89f5..89993a8a6765 100644
> --- a/arch/arm/boot/dts/bcm5301x.dtsi
> +++ b/arch/arm/boot/dts/bcm5301x.dtsi
> @@ -489,6 +489,10 @@ srab: ethernet-switch@18007000 {
> status = "disabled";
>
> /* ports are defined in board DTS */
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
This look a bit 'lone wolf' here. Not sure how much time you intend to
spend on this, but FWIW, others prefer to declare all ports in the SoC
DTSI with status = "disabled", and just enable the ones used per-board,
and add labels and PHY handles also per-board. Example: fsl-ls1028a.dtsi
and fsl-ls1028a-rdb.dts.
> };
>
> rng: rng@18004000 {
> --
> 2.25.1
>
On Mon, Nov 09, 2020 at 07:31:09PM -0800, Florian Fainelli wrote:
> Update the switch unit name from srab to ethernet-switch, allowing us
> to fix warnings such as:
>
> CHECK arch/arm/boot/dts/bcm4708-buffalo-wzr-1750dhp.dt.yaml
> arch/arm/boot/dts/bcm4708-buffalo-wzr-1750dhp.dt.yaml:
> srab@18007000: $nodename:0: 'srab@18007000' does not match
> '^(ethernet-)?switch(@.*)?$'
> From schema:
> Documentation/devicetree/bindings/net/dsa/b53.yaml
>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
Reviewed-by: Vladimir Oltean <[email protected]>
> arch/arm/boot/dts/bcm-nsp.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi
> index e895f7cb8c9f..e7d08959d5fe 100644
> --- a/arch/arm/boot/dts/bcm-nsp.dtsi
> +++ b/arch/arm/boot/dts/bcm-nsp.dtsi
> @@ -385,7 +385,7 @@ ccbtimer1: timer@35000 {
> clock-names = "apb_pclk";
> };
>
> - srab: srab@36000 {
> + srab: ethernet-switch@36000 {
> compatible = "brcm,nsp-srab";
> reg = <0x36000 0x1000>,
> <0x3f308 0x8>,
> --
> 2.25.1
>
On 11/10/20 2:12 PM, Vladimir Oltean wrote:
> On Mon, Nov 09, 2020 at 07:31:08PM -0800, Florian Fainelli wrote:
>> Provide an empty 'ports' container node with the correct #address-cells
>> and #size-cells properties. This silences the following warning:
>>
>> arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dt.yaml:
>> ethernet-switch@18007000: 'oneOf' conditional failed, one must be fixed:
>> 'ports' is a required property
>> 'ethernet-ports' is a required property
>> From schema:
>> Documentation/devicetree/bindings/net/dsa/b53.yaml
>>
>> Signed-off-by: Florian Fainelli <[email protected]>
>> ---
>> arch/arm/boot/dts/bcm5301x.dtsi | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi
>> index 807580dd89f5..89993a8a6765 100644
>> --- a/arch/arm/boot/dts/bcm5301x.dtsi
>> +++ b/arch/arm/boot/dts/bcm5301x.dtsi
>> @@ -489,6 +489,10 @@ srab: ethernet-switch@18007000 {
>> status = "disabled";
>>
>> /* ports are defined in board DTS */
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + };
>
> This look a bit 'lone wolf' here. Not sure how much time you intend to
> spend on this, but FWIW, others prefer to declare all ports in the SoC
> DTSI with status = "disabled", and just enable the ones used per-board,
> and add labels and PHY handles also per-board. Example: fsl-ls1028a.dtsi
> and fsl-ls1028a-rdb.dts.
That's a good suggestion, I could do that.
--
Florian
On Mon, Nov 09, 2020 at 07:31:10PM -0800, Florian Fainelli wrote:
> The register name should be "sgmii_config", not "sgmii", this is not a
> functional change since no code is currently looking for that register
> by name (or at all).
>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
I.e. no one calls devm_platform_ioremap_resource(pdev, 2), and even if
it did, the register name wouldn't matter.
...but at least it's documented that it should be called "sgmii_config".
And the address is the same as the one that's documented for the SGMII
register base in SoCs with "brcm,nsp-srab", so even without
documentation, it is pretty clear to me that it was a mistake calling it
simply "sgmii". And if that address is incorrect anyways, at least that
would be a separate issue.
Reviewed-by: Vladimir Oltean <[email protected]>
> arch/arm/boot/dts/bcm-nsp.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi
> index e7d08959d5fe..09fd7e55c069 100644
> --- a/arch/arm/boot/dts/bcm-nsp.dtsi
> +++ b/arch/arm/boot/dts/bcm-nsp.dtsi
> @@ -390,7 +390,7 @@ srab: ethernet-switch@36000 {
> reg = <0x36000 0x1000>,
> <0x3f308 0x8>,
> <0x3f410 0xc>;
> - reg-names = "srab", "mux_config", "sgmii";
> + reg-names = "srab", "mux_config", "sgmii_config";
> interrupts = <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>,
> --
> 2.25.1
>
On Mon, Nov 09, 2020 at 07:31:11PM -0800, Florian Fainelli wrote:
> Provide a default compatible string which is based on the 58522 SRAB
> compatible, this allows us to have sane defaults and silences the
> following warnings:
>
> arch/arm/boot/dts/bcm958522er.dt.yaml:
> ethernet-switch@36000: compatible: 'oneOf' conditional failed,
> one
> must be fixed:
> ['brcm,bcm5301x-srab'] is too short
> 'brcm,bcm5325' was expected
> 'brcm,bcm53115' was expected
> 'brcm,bcm53125' was expected
> 'brcm,bcm53128' was expected
> 'brcm,bcm5365' was expected
> 'brcm,bcm5395' was expected
> 'brcm,bcm5389' was expected
> 'brcm,bcm5397' was expected
> 'brcm,bcm5398' was expected
> 'brcm,bcm11360-srab' was expected
> 'brcm,bcm5301x-srab' is not one of ['brcm,bcm53010-srab',
> 'brcm,bcm53011-srab', 'brcm,bcm53012-srab', 'brcm,bcm53018-srab',
> 'brcm,bcm53019-srab']
> 'brcm,bcm5301x-srab' is not one of ['brcm,bcm11404-srab',
> 'brcm,bcm11407-srab', 'brcm,bcm11409-srab', 'brcm,bcm58310-srab',
> 'brcm,bcm58311-srab', 'brcm,bcm58313-srab']
> 'brcm,bcm5301x-srab' is not one of ['brcm,bcm58522-srab',
> 'brcm,bcm58523-srab', 'brcm,bcm58525-srab', 'brcm,bcm58622-srab',
> 'brcm,bcm58623-srab', 'brcm,bcm58625-srab', 'brcm,bcm88312-srab']
> 'brcm,bcm5301x-srab' is not one of ['brcm,bcm3384-switch',
> 'brcm,bcm6328-switch', 'brcm,bcm6368-switch']
> From schema:
> Documentation/devicetree/bindings/net/dsa/b53.yaml
>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
> arch/arm/boot/dts/bcm-nsp.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi
> index 09fd7e55c069..8453865d1439 100644
> --- a/arch/arm/boot/dts/bcm-nsp.dtsi
> +++ b/arch/arm/boot/dts/bcm-nsp.dtsi
> @@ -386,7 +386,7 @@ ccbtimer1: timer@35000 {
> };
>
> srab: ethernet-switch@36000 {
> - compatible = "brcm,nsp-srab";
> + compatible = "brcm,bcm58522-srab", "brcm,nsp-srab";
> reg = <0x36000 0x1000>,
> <0x3f308 0x8>,
> <0x3f410 0xc>;
> --
> 2.25.1
>
I am not getting this.
The line:
#include "bcm-nsp.dtsi"
can be found in:
arch/arm/boot/dts/bcm988312hr.dts
arch/arm/boot/dts/bcm958625hr.dts
arch/arm/boot/dts/bcm958622hr.dts
arch/arm/boot/dts/bcm958625k.dts
arch/arm/boot/dts/bcm958522er.dts
arch/arm/boot/dts/bcm958525er.dts
arch/arm/boot/dts/bcm958623hr.dts
arch/arm/boot/dts/bcm958525xmc.dts
The pattern for the other DTS files that include this seems to be to
overwrite the compatible locally in bcm958522er.dts, like this:
&srab {
compatible = "brcm,bcm58522-srab", "brcm,nsp-srab";
};
Is there a reason why you are choosing to put an SoC specific compatible
in the common bcm-nsp.dtsi?
On 11/10/20 2:37 PM, Vladimir Oltean wrote:
> On Mon, Nov 09, 2020 at 07:31:11PM -0800, Florian Fainelli wrote:
>> Provide a default compatible string which is based on the 58522 SRAB
>> compatible, this allows us to have sane defaults and silences the
>> following warnings:
>>
>> arch/arm/boot/dts/bcm958522er.dt.yaml:
>> ethernet-switch@36000: compatible: 'oneOf' conditional failed,
>> one
>> must be fixed:
>> ['brcm,bcm5301x-srab'] is too short
>> 'brcm,bcm5325' was expected
>> 'brcm,bcm53115' was expected
>> 'brcm,bcm53125' was expected
>> 'brcm,bcm53128' was expected
>> 'brcm,bcm5365' was expected
>> 'brcm,bcm5395' was expected
>> 'brcm,bcm5389' was expected
>> 'brcm,bcm5397' was expected
>> 'brcm,bcm5398' was expected
>> 'brcm,bcm11360-srab' was expected
>> 'brcm,bcm5301x-srab' is not one of ['brcm,bcm53010-srab',
>> 'brcm,bcm53011-srab', 'brcm,bcm53012-srab', 'brcm,bcm53018-srab',
>> 'brcm,bcm53019-srab']
>> 'brcm,bcm5301x-srab' is not one of ['brcm,bcm11404-srab',
>> 'brcm,bcm11407-srab', 'brcm,bcm11409-srab', 'brcm,bcm58310-srab',
>> 'brcm,bcm58311-srab', 'brcm,bcm58313-srab']
>> 'brcm,bcm5301x-srab' is not one of ['brcm,bcm58522-srab',
>> 'brcm,bcm58523-srab', 'brcm,bcm58525-srab', 'brcm,bcm58622-srab',
>> 'brcm,bcm58623-srab', 'brcm,bcm58625-srab', 'brcm,bcm88312-srab']
>> 'brcm,bcm5301x-srab' is not one of ['brcm,bcm3384-switch',
>> 'brcm,bcm6328-switch', 'brcm,bcm6368-switch']
>> From schema:
>> Documentation/devicetree/bindings/net/dsa/b53.yaml
>>
>> Signed-off-by: Florian Fainelli <[email protected]>
>> ---
>> arch/arm/boot/dts/bcm-nsp.dtsi | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi
>> index 09fd7e55c069..8453865d1439 100644
>> --- a/arch/arm/boot/dts/bcm-nsp.dtsi
>> +++ b/arch/arm/boot/dts/bcm-nsp.dtsi
>> @@ -386,7 +386,7 @@ ccbtimer1: timer@35000 {
>> };
>>
>> srab: ethernet-switch@36000 {
>> - compatible = "brcm,nsp-srab";
>> + compatible = "brcm,bcm58522-srab", "brcm,nsp-srab";
>> reg = <0x36000 0x1000>,
>> <0x3f308 0x8>,
>> <0x3f410 0xc>;
>> --
>> 2.25.1
>>
>
> I am not getting this.
> The line:
> #include "bcm-nsp.dtsi"
>
> can be found in:
>
> arch/arm/boot/dts/bcm988312hr.dts
> arch/arm/boot/dts/bcm958625hr.dts
> arch/arm/boot/dts/bcm958622hr.dts
> arch/arm/boot/dts/bcm958625k.dts
> arch/arm/boot/dts/bcm958522er.dts
> arch/arm/boot/dts/bcm958525er.dts
> arch/arm/boot/dts/bcm958623hr.dts
> arch/arm/boot/dts/bcm958525xmc.dts
>
>
> The pattern for the other DTS files that include this seems to be to
> overwrite the compatible locally in bcm958522er.dts, like this:
>
> &srab {
> compatible = "brcm,bcm58522-srab", "brcm,nsp-srab";
> };
>
> Is there a reason why you are choosing to put an SoC specific compatible
> in the common bcm-nsp.dtsi?
It is necessary to silence the warnings provided in the commit message
even when the srab node is disabled, since the dt_binding_check rule
will check all of the nodes matching the pattern. If there is a better
way to do this, I would gladly do it differently.
--
Florian
On Mon, Nov 09, 2020 at 07:31:13PM -0800, Florian Fainelli wrote:
> diff --git a/Documentation/devicetree/bindings/net/dsa/b53.yaml b/Documentation/devicetree/bindings/net/dsa/b53.yaml
> new file mode 100644
> index 000000000000..4fcbac1de95b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/dsa/b53.yaml
> @@ -0,0 +1,249 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/dsa/b53.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Broadcom BCM53xx Ethernet switches
> +
> +allOf:
> + - $ref: dsa.yaml#
> +
> +maintainers:
> + - Florian Fainelli <[email protected]>
> +
> +description:
> + Broadcom BCM53xx Ethernet switches
> +
> +properties:
> + compatible:
> + oneOf:
> + - const: brcm,bcm5325
> + - const: brcm,bcm53115
> + - const: brcm,bcm53125
> + - const: brcm,bcm53128
> + - const: brcm,bcm5365
> + - const: brcm,bcm5395
> + - const: brcm,bcm5389
> + - const: brcm,bcm5397
> + - const: brcm,bcm5398
> + - items:
> + - const: brcm,bcm11360-srab
> + - const: brcm,cygnus-srab
> + - items:
> + - enum:
> + - brcm,bcm53010-srab
> + - brcm,bcm53011-srab
> + - brcm,bcm53012-srab
> + - brcm,bcm53018-srab
> + - brcm,bcm53019-srab
> + - const: brcm,bcm5301x-srab
> + - items:
> + - enum:
> + - brcm,bcm11404-srab
> + - brcm,bcm11407-srab
> + - brcm,bcm11409-srab
> + - brcm,bcm58310-srab
> + - brcm,bcm58311-srab
> + - brcm,bcm58313-srab
> + - const: brcm,omega-srab
> + - items:
> + - enum:
> + - brcm,bcm58522-srab
> + - brcm,bcm58523-srab
> + - brcm,bcm58525-srab
> + - brcm,bcm58622-srab
> + - brcm,bcm58623-srab
> + - brcm,bcm58625-srab
> + - brcm,bcm88312-srab
> + - const: brcm,nsp-srab
> + - items:
> + - enum:
> + - brcm,bcm3384-switch
> + - brcm,bcm6328-switch
> + - brcm,bcm6368-switch
> + - const: brcm,bcm63xx-switch
> +
> +required:
> + - compatible
> + - reg
> +
> +# BCM585xx/586xx/88312 SoCs
> +if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - brcm,bcm58522-srab
> + - brcm,bcm58523-srab
> + - brcm,bcm58525-srab
> + - brcm,bcm58622-srab
> + - brcm,bcm58623-srab
> + - brcm,bcm58625-srab
> + - brcm,bcm88312-srab
> +then:
> + properties:
> + reg:
> + minItems: 3
> + maxItems: 3
> + reg-names:
> + items:
> + - const: srab
> + - const: mux_config
> + - const: sgmii_config
I am only reading these with a human eye, I don't parse YAML syntax.
Does the syntax enforce that these reg-names are declared in this
precise order, which is necessary for the proper operation of the
driver?
> + interrupts:
> + minItems: 13
> + maxItems: 13
> + interrupt-names:
> + items:
> + - const: link_state_p0
> + - const: link_state_p1
> + - const: link_state_p2
> + - const: link_state_p3
> + - const: link_state_p4
> + - const: link_state_p5
> + - const: link_state_p7
> + - const: link_state_p8
> + - const: phy
> + - const: ts
> + - const: imp_sleep_timer_p5
> + - const: imp_sleep_timer_p7
> + - const: imp_sleep_timer_p8
> + required:
> + - interrupts
> +else:
> + properties:
> + reg:
> + maxItems: 1
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + mdio {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + switch@1e {
You have renamed a node called 'ethernet-switch' into one called
'switch'. Was it deliberate?
> + compatible = "brcm,bcm53125";
> + reg = <30>;
> +
> + ethernet-ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + label = "lan1";
> + };
> +
> + port@1 {
> + reg = <1>;
> + label = "lan2";
> + };
> +
> + port@5 {
> + reg = <5>;
> + label = "cable-modem";
> + phy-mode = "rgmii-txid";
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> + };
> +
> + port@8 {
> + reg = <8>;
> + label = "cpu";
> + phy-mode = "rgmii-txid";
> + ethernet = <ð0>;
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> + };
> + };
> + };
> + };
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + axi {
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + switch@36000 {
> + compatible = "brcm,bcm58623-srab", "brcm,nsp-srab";
> + reg = <0x36000 0x1000>,
> + <0x3f308 0x8>,
> + <0x3f410 0xc>;
> + reg-names = "srab", "mux_config", "sgmii_config";
> + interrupts = <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 107 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "link_state_p0",
> + "link_state_p1",
> + "link_state_p2",
> + "link_state_p3",
> + "link_state_p4",
> + "link_state_p5",
> + "link_state_p7",
> + "link_state_p8",
> + "phy",
> + "ts",
> + "imp_sleep_timer_p5",
> + "imp_sleep_timer_p7",
> + "imp_sleep_timer_p8";
> +
> + ethernet-ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + label = "port0";
> + reg = <0>;
> + };
> +
> + port@1 {
> + label = "port1";
> + reg = <1>;
> + };
> +
> + port@2 {
> + label = "port2";
> + reg = <2>;
> + };
> +
> + port@3 {
> + label = "port3";
> + reg = <3>;
> + };
> +
> + port@4 {
> + label = "port4";
> + reg = <4>;
> + };
> +
> + port@8 {
> + ethernet = <&amac2>;
> + label = "cpu";
> + reg = <8>;
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> + };
> + };
> + };
> + };
On Tue, Nov 10, 2020 at 02:40:43PM -0800, Florian Fainelli wrote:
> On 11/10/20 2:37 PM, Vladimir Oltean wrote:
> > On Mon, Nov 09, 2020 at 07:31:11PM -0800, Florian Fainelli wrote:
> >> Provide a default compatible string which is based on the 58522 SRAB
> >> compatible, this allows us to have sane defaults and silences the
> >> following warnings:
> >>
> >> arch/arm/boot/dts/bcm958522er.dt.yaml:
> >> ethernet-switch@36000: compatible: 'oneOf' conditional failed,
> >> one
> >> must be fixed:
> >> ['brcm,bcm5301x-srab'] is too short
> >> 'brcm,bcm5325' was expected
> >> 'brcm,bcm53115' was expected
> >> 'brcm,bcm53125' was expected
> >> 'brcm,bcm53128' was expected
> >> 'brcm,bcm5365' was expected
> >> 'brcm,bcm5395' was expected
> >> 'brcm,bcm5389' was expected
> >> 'brcm,bcm5397' was expected
> >> 'brcm,bcm5398' was expected
> >> 'brcm,bcm11360-srab' was expected
> >> 'brcm,bcm5301x-srab' is not one of ['brcm,bcm53010-srab',
> >> 'brcm,bcm53011-srab', 'brcm,bcm53012-srab', 'brcm,bcm53018-srab',
> >> 'brcm,bcm53019-srab']
> >> 'brcm,bcm5301x-srab' is not one of ['brcm,bcm11404-srab',
> >> 'brcm,bcm11407-srab', 'brcm,bcm11409-srab', 'brcm,bcm58310-srab',
> >> 'brcm,bcm58311-srab', 'brcm,bcm58313-srab']
> >> 'brcm,bcm5301x-srab' is not one of ['brcm,bcm58522-srab',
> >> 'brcm,bcm58523-srab', 'brcm,bcm58525-srab', 'brcm,bcm58622-srab',
> >> 'brcm,bcm58623-srab', 'brcm,bcm58625-srab', 'brcm,bcm88312-srab']
> >> 'brcm,bcm5301x-srab' is not one of ['brcm,bcm3384-switch',
> >> 'brcm,bcm6328-switch', 'brcm,bcm6368-switch']
> >> From schema:
> >> Documentation/devicetree/bindings/net/dsa/b53.yaml
> >>
> >> Signed-off-by: Florian Fainelli <[email protected]>
> >> ---
> >> arch/arm/boot/dts/bcm-nsp.dtsi | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi
> >> index 09fd7e55c069..8453865d1439 100644
> >> --- a/arch/arm/boot/dts/bcm-nsp.dtsi
> >> +++ b/arch/arm/boot/dts/bcm-nsp.dtsi
> >> @@ -386,7 +386,7 @@ ccbtimer1: timer@35000 {
> >> };
> >>
> >> srab: ethernet-switch@36000 {
> >> - compatible = "brcm,nsp-srab";
> >> + compatible = "brcm,bcm58522-srab", "brcm,nsp-srab";
> >> reg = <0x36000 0x1000>,
> >> <0x3f308 0x8>,
> >> <0x3f410 0xc>;
> >> --
> >> 2.25.1
> >>
> >
> > I am not getting this.
> > The line:
> > #include "bcm-nsp.dtsi"
> >
> > can be found in:
> >
> > arch/arm/boot/dts/bcm988312hr.dts
> > arch/arm/boot/dts/bcm958625hr.dts
> > arch/arm/boot/dts/bcm958622hr.dts
> > arch/arm/boot/dts/bcm958625k.dts
> > arch/arm/boot/dts/bcm958522er.dts
> > arch/arm/boot/dts/bcm958525er.dts
> > arch/arm/boot/dts/bcm958623hr.dts
> > arch/arm/boot/dts/bcm958525xmc.dts
> >
> >
> > The pattern for the other DTS files that include this seems to be to
> > overwrite the compatible locally in bcm958522er.dts, like this:
> >
> > &srab {
> > compatible = "brcm,bcm58522-srab", "brcm,nsp-srab";
> > };
> >
> > Is there a reason why you are choosing to put an SoC specific compatible
> > in the common bcm-nsp.dtsi?
>
> It is necessary to silence the warnings provided in the commit message
> even when the srab node is disabled, since the dt_binding_check rule
> will check all of the nodes matching the pattern. If there is a better
> way to do this, I would gladly do it differently.
> --
> Florian
I am still not getting it. The exact 3 lines from above will not change
the "status" property from "disabled" to "okay", so I don't understand
why it matters whether it's enabled or not. The dt_binding_check error
isn't in the DTSI, it's in bcm958522er.dts. All that needs to be done is
that the bcm958522er.dts needs to override the compatible from the DTSI
and only the compatible, I believe. With no occurrence of an incomplete
list of compatibles in any final DTS, the dt_binding_check should not
complain about that single occurrence in the DTSI as far as I know (and
I did not test this).
On 11/10/20 2:48 PM, Vladimir Oltean wrote:
> On Tue, Nov 10, 2020 at 02:40:43PM -0800, Florian Fainelli wrote:
>> On 11/10/20 2:37 PM, Vladimir Oltean wrote:
>>> On Mon, Nov 09, 2020 at 07:31:11PM -0800, Florian Fainelli wrote:
>>>> Provide a default compatible string which is based on the 58522 SRAB
>>>> compatible, this allows us to have sane defaults and silences the
>>>> following warnings:
>>>>
>>>> arch/arm/boot/dts/bcm958522er.dt.yaml:
>>>> ethernet-switch@36000: compatible: 'oneOf' conditional failed,
>>>> one
>>>> must be fixed:
>>>> ['brcm,bcm5301x-srab'] is too short
>>>> 'brcm,bcm5325' was expected
>>>> 'brcm,bcm53115' was expected
>>>> 'brcm,bcm53125' was expected
>>>> 'brcm,bcm53128' was expected
>>>> 'brcm,bcm5365' was expected
>>>> 'brcm,bcm5395' was expected
>>>> 'brcm,bcm5389' was expected
>>>> 'brcm,bcm5397' was expected
>>>> 'brcm,bcm5398' was expected
>>>> 'brcm,bcm11360-srab' was expected
>>>> 'brcm,bcm5301x-srab' is not one of ['brcm,bcm53010-srab',
>>>> 'brcm,bcm53011-srab', 'brcm,bcm53012-srab', 'brcm,bcm53018-srab',
>>>> 'brcm,bcm53019-srab']
>>>> 'brcm,bcm5301x-srab' is not one of ['brcm,bcm11404-srab',
>>>> 'brcm,bcm11407-srab', 'brcm,bcm11409-srab', 'brcm,bcm58310-srab',
>>>> 'brcm,bcm58311-srab', 'brcm,bcm58313-srab']
>>>> 'brcm,bcm5301x-srab' is not one of ['brcm,bcm58522-srab',
>>>> 'brcm,bcm58523-srab', 'brcm,bcm58525-srab', 'brcm,bcm58622-srab',
>>>> 'brcm,bcm58623-srab', 'brcm,bcm58625-srab', 'brcm,bcm88312-srab']
>>>> 'brcm,bcm5301x-srab' is not one of ['brcm,bcm3384-switch',
>>>> 'brcm,bcm6328-switch', 'brcm,bcm6368-switch']
>>>> From schema:
>>>> Documentation/devicetree/bindings/net/dsa/b53.yaml
>>>>
>>>> Signed-off-by: Florian Fainelli <[email protected]>
>>>> ---
>>>> arch/arm/boot/dts/bcm-nsp.dtsi | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi
>>>> index 09fd7e55c069..8453865d1439 100644
>>>> --- a/arch/arm/boot/dts/bcm-nsp.dtsi
>>>> +++ b/arch/arm/boot/dts/bcm-nsp.dtsi
>>>> @@ -386,7 +386,7 @@ ccbtimer1: timer@35000 {
>>>> };
>>>>
>>>> srab: ethernet-switch@36000 {
>>>> - compatible = "brcm,nsp-srab";
>>>> + compatible = "brcm,bcm58522-srab", "brcm,nsp-srab";
>>>> reg = <0x36000 0x1000>,
>>>> <0x3f308 0x8>,
>>>> <0x3f410 0xc>;
>>>> --
>>>> 2.25.1
>>>>
>>>
>>> I am not getting this.
>>> The line:
>>> #include "bcm-nsp.dtsi"
>>>
>>> can be found in:
>>>
>>> arch/arm/boot/dts/bcm988312hr.dts
>>> arch/arm/boot/dts/bcm958625hr.dts
>>> arch/arm/boot/dts/bcm958622hr.dts
>>> arch/arm/boot/dts/bcm958625k.dts
>>> arch/arm/boot/dts/bcm958522er.dts
>>> arch/arm/boot/dts/bcm958525er.dts
>>> arch/arm/boot/dts/bcm958623hr.dts
>>> arch/arm/boot/dts/bcm958525xmc.dts
>>>
>>>
>>> The pattern for the other DTS files that include this seems to be to
>>> overwrite the compatible locally in bcm958522er.dts, like this:
>>>
>>> &srab {
>>> compatible = "brcm,bcm58522-srab", "brcm,nsp-srab";
>>> };
>>>
>>> Is there a reason why you are choosing to put an SoC specific compatible
>>> in the common bcm-nsp.dtsi?
>>
>> It is necessary to silence the warnings provided in the commit message
>> even when the srab node is disabled, since the dt_binding_check rule
>> will check all of the nodes matching the pattern. If there is a better
>> way to do this, I would gladly do it differently.
>> --
>> Florian
>
> I am still not getting it. The exact 3 lines from above will not change
> the "status" property from "disabled" to "okay", so I don't understand
> why it matters whether it's enabled or not. The dt_binding_check error
> isn't in the DTSI, it's in bcm958522er.dts. All that needs to be done is
> that the bcm958522er.dts needs to override the compatible from the DTSI
> and only the compatible, I believe. With no occurrence of an incomplete
> list of compatibles in any final DTS, the dt_binding_check should not
> complain about that single occurrence in the DTSI as far as I know (and
> I did not test this).
There is not a switch being enabled in
arch/arm/boot/dts/bcm958522er.dts, so sure, I could add the 3 lines you
quote above and that would silence the warning, but that does not scale
at all across DTS files including bcm5301x.dtsi for instance, it sort of
does for those including bcm-nsp.dtsi.
--
Florian
On Tue, Nov 10, 2020 at 02:52:57PM -0800, Florian Fainelli wrote:
> On 11/10/20 2:48 PM, Vladimir Oltean wrote:
> > On Tue, Nov 10, 2020 at 02:40:43PM -0800, Florian Fainelli wrote:
> >> On 11/10/20 2:37 PM, Vladimir Oltean wrote:
> >>> On Mon, Nov 09, 2020 at 07:31:11PM -0800, Florian Fainelli wrote:
> >>>> Provide a default compatible string which is based on the 58522 SRAB
> >>>> compatible, this allows us to have sane defaults and silences the
> >>>> following warnings:
> >>>>
> >>>> arch/arm/boot/dts/bcm958522er.dt.yaml:
> >>>> ethernet-switch@36000: compatible: 'oneOf' conditional failed,
> >>>> one
> >>>> must be fixed:
> >>>> ['brcm,bcm5301x-srab'] is too short
> >>>> 'brcm,bcm5325' was expected
> >>>> 'brcm,bcm53115' was expected
> >>>> 'brcm,bcm53125' was expected
> >>>> 'brcm,bcm53128' was expected
> >>>> 'brcm,bcm5365' was expected
> >>>> 'brcm,bcm5395' was expected
> >>>> 'brcm,bcm5389' was expected
> >>>> 'brcm,bcm5397' was expected
> >>>> 'brcm,bcm5398' was expected
> >>>> 'brcm,bcm11360-srab' was expected
> >>>> 'brcm,bcm5301x-srab' is not one of ['brcm,bcm53010-srab',
> >>>> 'brcm,bcm53011-srab', 'brcm,bcm53012-srab', 'brcm,bcm53018-srab',
> >>>> 'brcm,bcm53019-srab']
> >>>> 'brcm,bcm5301x-srab' is not one of ['brcm,bcm11404-srab',
> >>>> 'brcm,bcm11407-srab', 'brcm,bcm11409-srab', 'brcm,bcm58310-srab',
> >>>> 'brcm,bcm58311-srab', 'brcm,bcm58313-srab']
> >>>> 'brcm,bcm5301x-srab' is not one of ['brcm,bcm58522-srab',
> >>>> 'brcm,bcm58523-srab', 'brcm,bcm58525-srab', 'brcm,bcm58622-srab',
> >>>> 'brcm,bcm58623-srab', 'brcm,bcm58625-srab', 'brcm,bcm88312-srab']
> >>>> 'brcm,bcm5301x-srab' is not one of ['brcm,bcm3384-switch',
> >>>> 'brcm,bcm6328-switch', 'brcm,bcm6368-switch']
> >>>> From schema:
> >>>> Documentation/devicetree/bindings/net/dsa/b53.yaml
> >>>>
> >>>> Signed-off-by: Florian Fainelli <[email protected]>
> >>>> ---
> >>>> arch/arm/boot/dts/bcm-nsp.dtsi | 2 +-
> >>>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi
> >>>> index 09fd7e55c069..8453865d1439 100644
> >>>> --- a/arch/arm/boot/dts/bcm-nsp.dtsi
> >>>> +++ b/arch/arm/boot/dts/bcm-nsp.dtsi
> >>>> @@ -386,7 +386,7 @@ ccbtimer1: timer@35000 {
> >>>> };
> >>>>
> >>>> srab: ethernet-switch@36000 {
> >>>> - compatible = "brcm,nsp-srab";
> >>>> + compatible = "brcm,bcm58522-srab", "brcm,nsp-srab";
> >>>> reg = <0x36000 0x1000>,
> >>>> <0x3f308 0x8>,
> >>>> <0x3f410 0xc>;
> >>>> --
> >>>> 2.25.1
> >>>>
> >>>
> >>> I am not getting this.
> >>> The line:
> >>> #include "bcm-nsp.dtsi"
> >>>
> >>> can be found in:
> >>>
> >>> arch/arm/boot/dts/bcm988312hr.dts
> >>> arch/arm/boot/dts/bcm958625hr.dts
> >>> arch/arm/boot/dts/bcm958622hr.dts
> >>> arch/arm/boot/dts/bcm958625k.dts
> >>> arch/arm/boot/dts/bcm958522er.dts
> >>> arch/arm/boot/dts/bcm958525er.dts
> >>> arch/arm/boot/dts/bcm958623hr.dts
> >>> arch/arm/boot/dts/bcm958525xmc.dts
> >>>
> >>>
> >>> The pattern for the other DTS files that include this seems to be to
> >>> overwrite the compatible locally in bcm958522er.dts, like this:
> >>>
> >>> &srab {
> >>> compatible = "brcm,bcm58522-srab", "brcm,nsp-srab";
> >>> };
> >>>
> >>> Is there a reason why you are choosing to put an SoC specific compatible
> >>> in the common bcm-nsp.dtsi?
> >>
> >> It is necessary to silence the warnings provided in the commit message
> >> even when the srab node is disabled, since the dt_binding_check rule
> >> will check all of the nodes matching the pattern. If there is a better
> >> way to do this, I would gladly do it differently.
> >> --
> >> Florian
> >
> > I am still not getting it. The exact 3 lines from above will not change
> > the "status" property from "disabled" to "okay", so I don't understand
> > why it matters whether it's enabled or not. The dt_binding_check error
> > isn't in the DTSI, it's in bcm958522er.dts. All that needs to be done is
> > that the bcm958522er.dts needs to override the compatible from the DTSI
> > and only the compatible, I believe. With no occurrence of an incomplete
> > list of compatibles in any final DTS, the dt_binding_check should not
> > complain about that single occurrence in the DTSI as far as I know (and
> > I did not test this).
>
> There is not a switch being enabled in
> arch/arm/boot/dts/bcm958522er.dts, so sure, I could add the 3 lines you
> quote above and that would silence the warning, but that does not scale
> at all across DTS files including bcm5301x.dtsi for instance, it sort of
> does for those including bcm-nsp.dtsi.
> --
> Florian
I see only bcm47081.dtsi and bcm4708.dtsi to include bcm5301x.dtsi, and
you did point out in the other email that the BCM4708* SoCs always
contain a BCM53010 switch, so that would make the other patch not wrong.
Either way, it's up to you and Rafal.
On 11/10/2020 2:13 PM, Florian Fainelli wrote:
> On 11/10/20 2:12 PM, Vladimir Oltean wrote:
>> On Mon, Nov 09, 2020 at 07:31:08PM -0800, Florian Fainelli wrote:
>>> Provide an empty 'ports' container node with the correct #address-cells
>>> and #size-cells properties. This silences the following warning:
>>>
>>> arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dt.yaml:
>>> ethernet-switch@18007000: 'oneOf' conditional failed, one must be fixed:
>>> 'ports' is a required property
>>> 'ethernet-ports' is a required property
>>> From schema:
>>> Documentation/devicetree/bindings/net/dsa/b53.yaml
>>>
>>> Signed-off-by: Florian Fainelli <[email protected]>
>>> ---
>>> arch/arm/boot/dts/bcm5301x.dtsi | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi
>>> index 807580dd89f5..89993a8a6765 100644
>>> --- a/arch/arm/boot/dts/bcm5301x.dtsi
>>> +++ b/arch/arm/boot/dts/bcm5301x.dtsi
>>> @@ -489,6 +489,10 @@ srab: ethernet-switch@18007000 {
>>> status = "disabled";
>>>
>>> /* ports are defined in board DTS */
>>> + ports {
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> + };
>>
>> This look a bit 'lone wolf' here. Not sure how much time you intend to
>> spend on this, but FWIW, others prefer to declare all ports in the SoC
>> DTSI with status = "disabled", and just enable the ones used per-board,
>> and add labels and PHY handles also per-board. Example: fsl-ls1028a.dtsi
>> and fsl-ls1028a-rdb.dts.
>
> That's a good suggestion, I could do that.
There is quite a bit of variation between designs and how the ports are
assigned and it would end up being quite verbose, so I will punt that
for now.
--
Florian
On 11.11.2020 02:48, Florian Fainelli wrote:
> On 11/10/2020 2:13 PM, Florian Fainelli wrote:
>> On 11/10/20 2:12 PM, Vladimir Oltean wrote:
>>> On Mon, Nov 09, 2020 at 07:31:08PM -0800, Florian Fainelli wrote:
>>>> Provide an empty 'ports' container node with the correct #address-cells
>>>> and #size-cells properties. This silences the following warning:
>>>>
>>>> arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dt.yaml:
>>>> ethernet-switch@18007000: 'oneOf' conditional failed, one must be fixed:
>>>> 'ports' is a required property
>>>> 'ethernet-ports' is a required property
>>>> From schema:
>>>> Documentation/devicetree/bindings/net/dsa/b53.yaml
>>>>
>>>> Signed-off-by: Florian Fainelli <[email protected]>
>>>> ---
>>>> arch/arm/boot/dts/bcm5301x.dtsi | 4 ++++
>>>> 1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi
>>>> index 807580dd89f5..89993a8a6765 100644
>>>> --- a/arch/arm/boot/dts/bcm5301x.dtsi
>>>> +++ b/arch/arm/boot/dts/bcm5301x.dtsi
>>>> @@ -489,6 +489,10 @@ srab: ethernet-switch@18007000 {
>>>> status = "disabled";
>>>>
>>>> /* ports are defined in board DTS */
>>>> + ports {
>>>> + #address-cells = <1>;
>>>> + #size-cells = <0>;
>>>> + };
>>>
>>> This look a bit 'lone wolf' here. Not sure how much time you intend to
>>> spend on this, but FWIW, others prefer to declare all ports in the SoC
>>> DTSI with status = "disabled", and just enable the ones used per-board,
>>> and add labels and PHY handles also per-board. Example: fsl-ls1028a.dtsi
>>> and fsl-ls1028a-rdb.dts.
>>
>> That's a good suggestion, I could do that.
>
> There is quite a bit of variation between designs and how the ports are
> assigned and it would end up being quite verbose, so I will punt that
> for now.
I agree with Florian, boards (vendors) use ports really randomly so pretty
much every device needs that defined from the scratch.
On 10.11.2020 04:31, Florian Fainelli wrote:
> Provide a default compatible string which is based on the 53010 SRAB
> compatible, this allows us to have sane defaults and silences the
> following warnings:
>
> arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dt.yaml:
> ethernet-switch@18007000: compatible: 'oneOf' conditional failed, one
> must be fixed:
> ['brcm,bcm5301x-srab'] is too short
> 'brcm,bcm5325' was expected
> 'brcm,bcm53115' was expected
> 'brcm,bcm53125' was expected
> 'brcm,bcm53128' was expected
> 'brcm,bcm5365' was expected
> 'brcm,bcm5395' was expected
> 'brcm,bcm5389' was expected
> 'brcm,bcm5397' was expected
> 'brcm,bcm5398' was expected
> 'brcm,bcm11360-srab' was expected
> 'brcm,bcm5301x-srab' is not one of ['brcm,bcm53010-srab',
> 'brcm,bcm53011-srab', 'brcm,bcm53012-srab', 'brcm,bcm53018-srab',
> 'brcm,bcm53019-srab']
> 'brcm,bcm5301x-srab' is not one of ['brcm,bcm11404-srab',
> 'brcm,bcm11407-srab', 'brcm,bcm11409-srab', 'brcm,bcm58310-srab',
> 'brcm,bcm58311-srab', 'brcm,bcm58313-srab']
> 'brcm,bcm5301x-srab' is not one of ['brcm,bcm58522-srab',
> 'brcm,bcm58523-srab', 'brcm,bcm58525-srab', 'brcm,bcm58622-srab',
> 'brcm,bcm58623-srab', 'brcm,bcm58625-srab', 'brcm,bcm88312-srab']
> 'brcm,bcm5301x-srab' is not one of ['brcm,bcm3384-switch',
> 'brcm,bcm6328-switch', 'brcm,bcm6368-switch']
> From schema:
> Documentation/devicetree/bindings/net/dsa/b53.yaml
>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
> arch/arm/boot/dts/bcm5301x.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi
> index ee23c0841699..807580dd89f5 100644
> --- a/arch/arm/boot/dts/bcm5301x.dtsi
> +++ b/arch/arm/boot/dts/bcm5301x.dtsi
> @@ -483,7 +483,7 @@ thermal: thermal@1800c2c0 {
> };
>
> srab: ethernet-switch@18007000 {
> - compatible = "brcm,bcm5301x-srab";
> + compatible = "brcm,bcm53010-srab", "brcm,bcm5301x-srab";
I've never seen Northstar device with BCM53010, see below list.
*** BCM47081 ***
Buffalo WZR-600DHP2
[ 1.816948] b53_common: found switch: BCM53011, rev 2
Luxul XWR-1200 V1
[ 2.602445] b53_common: found switch: BCM53011, rev 5
TP-LINK Archer C5 V2
[ 0.606353] b53_common: found switch: BCM53011, rev 5
*** BCM4708 ***
Buffalo WZR-1750DHP
[ 1.961584] b53_common: found switch: BCM53011, rev 2
Netgear R6250 V1
[ 2.445594] b53_common: found switch: BCM53011, rev 2
SmartRG SR400ac
[ 4.258116] b53_common: found switch: BCM53011, rev 5
*** BCM4709 ***
TP-LINK Archer C9 V1
[ 0.640041] b53_common: found switch: BCM53012, rev 5
*** BCM47094 ***
D-Link DIR-885L
[ 1.373423] b53_common: found switch: BCM53012, rev 0
Luxul XWR-3150 V1
[ 5.893989] b53_common: found switch: BCM53012, rev 0
Luxul XAP-1610 V1
[ 0.761285] b53_common: found switch: BCM53012, rev 0
On Mon, 09 Nov 2020 19:31:05 -0800, Florian Fainelli wrote:
> The 'sfp' and 'managed' properties are commonly used to describe
> Ethernet switch ports connecting to SFP/SFF cages, describe these two
> properties as valid that we inherit from ethernet-controller.yaml.
>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
> Documentation/devicetree/bindings/net/dsa/dsa.yaml | 4 ++++
> 1 file changed, 4 insertions(+)
>
Acked-by: Rob Herring <[email protected]>
On Mon, 09 Nov 2020 19:31:13 -0800, Florian Fainelli wrote:
> From: Kurt Kanzenbach <[email protected]>
>
> Convert the b53 DSA device tree bindings to YAML in order to allow
> for automatic checking and such.
>
> Suggested-by: Florian Fainelli <[email protected]>
> Signed-off-by: Kurt Kanzenbach <[email protected]>
> ---
> .../devicetree/bindings/net/dsa/b53.txt | 149 -----------
> .../devicetree/bindings/net/dsa/b53.yaml | 249 ++++++++++++++++++
> MAINTAINERS | 2 +-
> 3 files changed, 250 insertions(+), 150 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/net/dsa/b53.txt
> create mode 100644 Documentation/devicetree/bindings/net/dsa/b53.yaml
>
With the rename (don't forget $id):
Reviewed-by: Rob Herring <[email protected]>
On Mon, 09 Nov 2020 19:31:04 -0800, Florian Fainelli wrote:
> Upon discussion with Kurt, Rob and Vladimir it appears that we should be
> allowing ethernet-switch as a node name, update dsa.yaml accordingly.
>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
> Documentation/devicetree/bindings/net/dsa/dsa.yaml | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Acked-by: Rob Herring <[email protected]>