[net-next RFC PATCH v2 1/3] dt-bindings: net: dsa: Add Airoha AN8855 Gigabit Switch documentation

Christian Marangi posted 3 patches 1 month ago
There is a newer version of this series
[net-next RFC PATCH v2 1/3] dt-bindings: net: dsa: Add Airoha AN8855 Gigabit Switch documentation
Posted by Christian Marangi 1 month ago
Add Airoha AN8855 5 port Gigabit Switch documentation.

The switch node requires an additional mdio node to describe each internal
PHY relative offset as the PHY address for the switch match the one for
the PHY ports. On top of internal PHY address, the switch base PHY address
is added.

Also the switch base PHY address can be configured and changed after the
first initialization. On reset, the switch PHY address is ALWAYS 1.
This can be configured with the use of "airoha,base_smi_address".

Calibration values might be stored in switch EFUSE and internal PHY
might need to be calibrated, in such case, airoha,ext_surge needs to be
enabled and relative NVMEM cells needs to be defined in nvmem-layout
node.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 .../bindings/net/dsa/airoha,an8855.yaml       | 253 ++++++++++++++++++
 1 file changed, 253 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/dsa/airoha,an8855.yaml

diff --git a/Documentation/devicetree/bindings/net/dsa/airoha,an8855.yaml b/Documentation/devicetree/bindings/net/dsa/airoha,an8855.yaml
new file mode 100644
index 000000000000..5982b4c39536
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/dsa/airoha,an8855.yaml
@@ -0,0 +1,253 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/dsa/airoha,an8855.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Airoha AN8855 Gigabit switch
+
+maintainers:
+  - Christian Marangi <ansuelsmth@gmail.com>
+
+description:
+  Airoha AN8855 is a 5-port Gigabit Switch.
+
+  The switch node requires an additional mdio node to describe each internal
+  PHY relative offset as the PHY address for the switch match the one for
+  the PHY ports. On top of internal PHY address, the switch base PHY address
+  is added.
+
+  Also the switch base PHY address can be configured and changed after the
+  first initialization. On reset, the switch PHY address is ALWAYS 1.
+
+properties:
+  compatible:
+    const: airoha,an8855
+
+  reg:
+    maxItems: 1
+
+  reset-gpios:
+    description:
+      GPIO to be used to reset the whole device
+    maxItems: 1
+
+  airoha,base_smi_address:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Configure and change the base switch PHY address to a new address on
+      the bus.
+      On reset, the switch PHY address is ALWAYS 1.
+    default: 1
+    maximum: 31
+
+  airoha,ext_surge:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description:
+      Calibrate the internal PHY with the calibration values stored in EFUSE
+      for the r50Ohm values.
+
+  '#nvmem-cell-cells':
+    const: 0
+
+  nvmem-layout:
+    $ref: /schemas/nvmem/layouts/nvmem-layout.yaml
+    description:
+      NVMEM Layout for exposed EFUSE. (for example to propagate calibration
+      value for r50Ohm for internal PHYs)
+
+  mdio:
+    $ref: /schemas/net/mdio.yaml#
+    unevaluatedProperties: false
+    description:
+      Define the relative address of the internal PHY for each port.
+
+      Each reg for the PHY is relative to the switch base PHY address.
+
+$ref: dsa.yaml#
+
+required:
+  - compatible
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    mdio {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        switch@1 {
+            compatible = "airoha,an8855";
+            reg = <1>;
+            reset-gpios = <&pio 39 0>;
+
+            airoha,ext_surge;
+
+            #nvmem-cell-cells = <0>;
+
+            nvmem-layout {
+                compatible = "fixed-layout";
+                #address-cells = <1>;
+                #size-cells = <1>;
+
+                shift_sel_port0_tx_a: shift-sel-port0-tx-a@c {
+                    reg = <0xc 0x4>;
+                };
+
+                shift_sel_port0_tx_b: shift-sel-port0-tx-b@10 {
+                    reg = <0x10 0x4>;
+                };
+
+                shift_sel_port0_tx_c: shift-sel-port0-tx-c@14 {
+                    reg = <0x14 0x4>;
+                };
+
+                shift_sel_port0_tx_d: shift-sel-port0-tx-d@18 {
+                    reg = <0x18 0x4>;
+                };
+
+                shift_sel_port1_tx_a: shift-sel-port1-tx-a@1c {
+                    reg = <0x1c 0x4>;
+                };
+
+                shift_sel_port1_tx_b: shift-sel-port1-tx-b@20 {
+                    reg = <0x20 0x4>;
+                };
+
+                shift_sel_port1_tx_c: shift-sel-port1-tx-c@24 {
+                    reg = <0x24 0x4>;
+                };
+
+                shift_sel_port1_tx_d: shift-sel-port1-tx-d@28 {
+                    reg = <0x28 0x4>;
+                };
+
+                shift_sel_port2_tx_a: shift-sel-port2-tx-a@2c {
+                    reg = <0x2c 0x4>;
+                };
+
+                shift_sel_port2_tx_b: shift-sel-port2-tx-b@30 {
+                    reg = <0x30 0x4>;
+                };
+
+                shift_sel_port2_tx_c: shift-sel-port2-tx-c@34 {
+                    reg = <0x34 0x4>;
+                };
+
+                shift_sel_port2_tx_d: shift-sel-port2-tx-d@38 {
+                    reg = <0x38 0x4>;
+                };
+
+                shift_sel_port3_tx_a: shift-sel-port3-tx-a@4c {
+                    reg = <0x4c 0x4>;
+                };
+
+                shift_sel_port3_tx_b: shift-sel-port3-tx-b@50 {
+                    reg = <0x50 0x4>;
+                };
+
+                shift_sel_port3_tx_c: shift-sel-port3-tx-c@54 {
+                    reg = <0x54 0x4>;
+                };
+
+                shift_sel_port3_tx_d: shift-sel-port3-tx-d@58 {
+                    reg = <0x58 0x4>;
+                };
+            };
+
+            ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                port@0 {
+                    reg = <0>;
+                    label = "lan1";
+                    phy-mode = "internal";
+                    phy-handle = <&internal_phy0>;
+                };
+
+                port@1 {
+                    reg = <1>;
+                    label = "lan2";
+                    phy-mode = "internal";
+                    phy-handle = <&internal_phy1>;
+                };
+
+                port@2 {
+                    reg = <2>;
+                    label = "lan3";
+                    phy-mode = "internal";
+                    phy-handle = <&internal_phy2>;
+                };
+
+                port@3 {
+                    reg = <3>;
+                    label = "lan4";
+                    phy-mode = "internal";
+                    phy-handle = <&internal_phy3>;
+                };
+
+                port@5 {
+                    reg = <5>;
+                    label = "cpu";
+                    ethernet = <&gmac0>;
+                    phy-mode = "2500base-x";
+
+                    fixed-link {
+                        speed = <2500>;
+                        full-duplex;
+                        pause;
+                    };
+                };
+            };
+
+            mdio {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                internal_phy0: phy@0 {
+                    reg = <0>;
+
+                    nvmem-cells = <&shift_sel_port0_tx_a>,
+                                  <&shift_sel_port0_tx_b>,
+                                  <&shift_sel_port0_tx_c>,
+                                  <&shift_sel_port0_tx_d>;
+                    nvmem-cell-names = "tx_a", "tx_b", "tx_c", "tx_d";
+                };
+
+                internal_phy1: phy@1 {
+                    reg = <1>;
+
+                    nvmem-cells = <&shift_sel_port1_tx_a>,
+                                  <&shift_sel_port1_tx_b>,
+                                  <&shift_sel_port1_tx_c>,
+                                  <&shift_sel_port1_tx_d>;
+                    nvmem-cell-names = "tx_a", "tx_b", "tx_c", "tx_d";
+                };
+
+                internal_phy2: phy@2 {
+                    reg = <2>;
+
+                    nvmem-cells = <&shift_sel_port2_tx_a>,
+                                  <&shift_sel_port2_tx_b>,
+                                  <&shift_sel_port2_tx_c>,
+                                  <&shift_sel_port2_tx_d>;
+                    nvmem-cell-names = "tx_a", "tx_b", "tx_c", "tx_d";
+                };
+
+                internal_phy3: phy@3 {
+                    reg = <3>;
+
+                    nvmem-cells = <&shift_sel_port3_tx_a>,
+                                  <&shift_sel_port3_tx_b>,
+                                  <&shift_sel_port3_tx_c>,
+                                  <&shift_sel_port3_tx_d>;
+                    nvmem-cell-names = "tx_a", "tx_b", "tx_c", "tx_d";
+                };
+            };
+        };
+    };
-- 
2.45.2
Re: [net-next RFC PATCH v2 1/3] dt-bindings: net: dsa: Add Airoha AN8855 Gigabit Switch documentation
Posted by Rob Herring 1 month ago
On Wed, Oct 23, 2024 at 06:19:50PM +0200, Christian Marangi wrote:
> Add Airoha AN8855 5 port Gigabit Switch documentation.
> 
> The switch node requires an additional mdio node to describe each internal
> PHY relative offset as the PHY address for the switch match the one for
> the PHY ports. On top of internal PHY address, the switch base PHY address
> is added.
> 
> Also the switch base PHY address can be configured and changed after the
> first initialization. On reset, the switch PHY address is ALWAYS 1.
> This can be configured with the use of "airoha,base_smi_address".
> 
> Calibration values might be stored in switch EFUSE and internal PHY
> might need to be calibrated, in such case, airoha,ext_surge needs to be
> enabled and relative NVMEM cells needs to be defined in nvmem-layout
> node.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  .../bindings/net/dsa/airoha,an8855.yaml       | 253 ++++++++++++++++++
>  1 file changed, 253 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/dsa/airoha,an8855.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/airoha,an8855.yaml b/Documentation/devicetree/bindings/net/dsa/airoha,an8855.yaml
> new file mode 100644
> index 000000000000..5982b4c39536
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/dsa/airoha,an8855.yaml
> @@ -0,0 +1,253 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/dsa/airoha,an8855.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Airoha AN8855 Gigabit switch
> +
> +maintainers:
> +  - Christian Marangi <ansuelsmth@gmail.com>
> +
> +description:

You need '>' to preserve paragraphs.

> +  Airoha AN8855 is a 5-port Gigabit Switch.
> +
> +  The switch node requires an additional mdio node to describe each internal
> +  PHY relative offset as the PHY address for the switch match the one for
> +  the PHY ports. On top of internal PHY address, the switch base PHY address
> +  is added.
> +
> +  Also the switch base PHY address can be configured and changed after the
> +  first initialization. On reset, the switch PHY address is ALWAYS 1.
> +
> +properties:
> +  compatible:
> +    const: airoha,an8855
> +
> +  reg:
> +    maxItems: 1
> +
> +  reset-gpios:
> +    description:
> +      GPIO to be used to reset the whole device
> +    maxItems: 1
> +
> +  airoha,base_smi_address:

s/_/-/

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Configure and change the base switch PHY address to a new address on
> +      the bus.
> +      On reset, the switch PHY address is ALWAYS 1.

The paragraphs here won't be maintained without '>' and a blank line in 
between.

Or just move the 2nd sentence up to follow the first one.

> +    default: 1
> +    maximum: 31
> +
> +  airoha,ext_surge:

ditto.

> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description:
> +      Calibrate the internal PHY with the calibration values stored in EFUSE
> +      for the r50Ohm values.
> +
> +  '#nvmem-cell-cells':
> +    const: 0
> +
> +  nvmem-layout:
> +    $ref: /schemas/nvmem/layouts/nvmem-layout.yaml
> +    description:
> +      NVMEM Layout for exposed EFUSE. (for example to propagate calibration
> +      value for r50Ohm for internal PHYs)
> +
> +  mdio:
> +    $ref: /schemas/net/mdio.yaml#
> +    unevaluatedProperties: false
> +    description:
> +      Define the relative address of the internal PHY for each port.
> +
> +      Each reg for the PHY is relative to the switch base PHY address.
> +
> +$ref: dsa.yaml#
> +
> +required:
> +  - compatible
> +  - reg
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    mdio {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        switch@1 {
> +            compatible = "airoha,an8855";
> +            reg = <1>;
> +            reset-gpios = <&pio 39 0>;
> +
> +            airoha,ext_surge;
> +
> +            #nvmem-cell-cells = <0>;
> +
> +            nvmem-layout {
> +                compatible = "fixed-layout";
> +                #address-cells = <1>;
> +                #size-cells = <1>;
> +
> +                shift_sel_port0_tx_a: shift-sel-port0-tx-a@c {
> +                    reg = <0xc 0x4>;
> +                };
> +
> +                shift_sel_port0_tx_b: shift-sel-port0-tx-b@10 {
> +                    reg = <0x10 0x4>;
> +                };
> +
> +                shift_sel_port0_tx_c: shift-sel-port0-tx-c@14 {
> +                    reg = <0x14 0x4>;
> +                };
> +
> +                shift_sel_port0_tx_d: shift-sel-port0-tx-d@18 {
> +                    reg = <0x18 0x4>;
> +                };
> +
> +                shift_sel_port1_tx_a: shift-sel-port1-tx-a@1c {
> +                    reg = <0x1c 0x4>;
> +                };
> +
> +                shift_sel_port1_tx_b: shift-sel-port1-tx-b@20 {
> +                    reg = <0x20 0x4>;
> +                };
> +
> +                shift_sel_port1_tx_c: shift-sel-port1-tx-c@24 {
> +                    reg = <0x24 0x4>;
> +                };
> +
> +                shift_sel_port1_tx_d: shift-sel-port1-tx-d@28 {
> +                    reg = <0x28 0x4>;
> +                };
> +
> +                shift_sel_port2_tx_a: shift-sel-port2-tx-a@2c {
> +                    reg = <0x2c 0x4>;
> +                };
> +
> +                shift_sel_port2_tx_b: shift-sel-port2-tx-b@30 {
> +                    reg = <0x30 0x4>;
> +                };
> +
> +                shift_sel_port2_tx_c: shift-sel-port2-tx-c@34 {
> +                    reg = <0x34 0x4>;
> +                };
> +
> +                shift_sel_port2_tx_d: shift-sel-port2-tx-d@38 {
> +                    reg = <0x38 0x4>;
> +                };
> +
> +                shift_sel_port3_tx_a: shift-sel-port3-tx-a@4c {
> +                    reg = <0x4c 0x4>;
> +                };
> +
> +                shift_sel_port3_tx_b: shift-sel-port3-tx-b@50 {
> +                    reg = <0x50 0x4>;
> +                };
> +
> +                shift_sel_port3_tx_c: shift-sel-port3-tx-c@54 {
> +                    reg = <0x54 0x4>;
> +                };
> +
> +                shift_sel_port3_tx_d: shift-sel-port3-tx-d@58 {
> +                    reg = <0x58 0x4>;
> +                };
> +            };
> +
> +            ports {
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +
> +                port@0 {
> +                    reg = <0>;
> +                    label = "lan1";
> +                    phy-mode = "internal";
> +                    phy-handle = <&internal_phy0>;
> +                };
> +
> +                port@1 {
> +                    reg = <1>;
> +                    label = "lan2";
> +                    phy-mode = "internal";
> +                    phy-handle = <&internal_phy1>;
> +                };
> +
> +                port@2 {
> +                    reg = <2>;
> +                    label = "lan3";
> +                    phy-mode = "internal";
> +                    phy-handle = <&internal_phy2>;
> +                };
> +
> +                port@3 {
> +                    reg = <3>;
> +                    label = "lan4";
> +                    phy-mode = "internal";
> +                    phy-handle = <&internal_phy3>;
> +                };
> +
> +                port@5 {
> +                    reg = <5>;
> +                    label = "cpu";
> +                    ethernet = <&gmac0>;
> +                    phy-mode = "2500base-x";
> +
> +                    fixed-link {
> +                        speed = <2500>;
> +                        full-duplex;
> +                        pause;
> +                    };
> +                };
> +            };
> +
> +            mdio {
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +
> +                internal_phy0: phy@0 {
> +                    reg = <0>;
> +
> +                    nvmem-cells = <&shift_sel_port0_tx_a>,
> +                                  <&shift_sel_port0_tx_b>,
> +                                  <&shift_sel_port0_tx_c>,
> +                                  <&shift_sel_port0_tx_d>;
> +                    nvmem-cell-names = "tx_a", "tx_b", "tx_c", "tx_d";
> +                };
> +
> +                internal_phy1: phy@1 {
> +                    reg = <1>;
> +
> +                    nvmem-cells = <&shift_sel_port1_tx_a>,
> +                                  <&shift_sel_port1_tx_b>,
> +                                  <&shift_sel_port1_tx_c>,
> +                                  <&shift_sel_port1_tx_d>;
> +                    nvmem-cell-names = "tx_a", "tx_b", "tx_c", "tx_d";
> +                };
> +
> +                internal_phy2: phy@2 {
> +                    reg = <2>;
> +
> +                    nvmem-cells = <&shift_sel_port2_tx_a>,
> +                                  <&shift_sel_port2_tx_b>,
> +                                  <&shift_sel_port2_tx_c>,
> +                                  <&shift_sel_port2_tx_d>;
> +                    nvmem-cell-names = "tx_a", "tx_b", "tx_c", "tx_d";
> +                };
> +
> +                internal_phy3: phy@3 {
> +                    reg = <3>;
> +
> +                    nvmem-cells = <&shift_sel_port3_tx_a>,
> +                                  <&shift_sel_port3_tx_b>,
> +                                  <&shift_sel_port3_tx_c>,
> +                                  <&shift_sel_port3_tx_d>;
> +                    nvmem-cell-names = "tx_a", "tx_b", "tx_c", "tx_d";
> +                };
> +            };
> +        };
> +    };
> -- 
> 2.45.2
>
Re: [net-next RFC PATCH v2 1/3] dt-bindings: net: dsa: Add Airoha AN8855 Gigabit Switch documentation
Posted by Andrew Lunn 1 month ago
> +  airoha,base_smi_address:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Configure and change the base switch PHY address to a new address on
> +      the bus.
> +      On reset, the switch PHY address is ALWAYS 1.
> +    default: 1
> +    maximum: 31

Given that this is a 5 port switch, what happens if i pick a value
greater than 31 - 5 ?

Do you have a real use case for this? A board which requires the PHYs
get shifted from the default of 1? Vendors have all sorts of bells and
whistles which we never use. If its not needed, i would not add it,
until it is actually needed, if ever.

> +  mdio:
> +    $ref: /schemas/net/mdio.yaml#
> +    unevaluatedProperties: false
> +    description:
> +      Define the relative address of the internal PHY for each port.
> +
> +      Each reg for the PHY is relative to the switch base PHY address.

Which is not the usual meaning of reg. 

> +            mdio {
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +
> +                internal_phy0: phy@0 {
> +                    reg = <0>;

So given that airoha,base_smi_address defaults to 1, this is actually
address 1 on the MDIO bus?

	Andrew
Re: [net-next RFC PATCH v2 1/3] dt-bindings: net: dsa: Add Airoha AN8855 Gigabit Switch documentation
Posted by Christian Marangi 1 month ago
On Wed, Oct 23, 2024 at 07:08:57PM +0200, Andrew Lunn wrote:
> > +  airoha,base_smi_address:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description:
> > +      Configure and change the base switch PHY address to a new address on
> > +      the bus.
> > +      On reset, the switch PHY address is ALWAYS 1.
> > +    default: 1
> > +    maximum: 31
> 
> Given that this is a 5 port switch, what happens if i pick a value
> greater than 31 - 5 ?

The PHY at those address won't be reachable, I didn't think of this, you
are right.

> 
> Do you have a real use case for this? A board which requires the PHYs
> get shifted from the default of 1? Vendors have all sorts of bells and
> whistles which we never use. If its not needed, i would not add it,
> until it is actually needed, if ever.

Well the first case that comes to mind is multiple switch and conflict.
I have no idea if there are hw strap to configure this so I assume if a
SoC have 2 switch (maybe of the same type), this permits to configure
them (with reset pin and deasserting them once the base address is
correctly configured)

But yes totally ok to drop this if too strange... I assume it's problematic
that PHY change at runtime.

> 
> > +  mdio:
> > +    $ref: /schemas/net/mdio.yaml#
> > +    unevaluatedProperties: false
> > +    description:
> > +      Define the relative address of the internal PHY for each port.
> > +
> > +      Each reg for the PHY is relative to the switch base PHY address.
> 
> Which is not the usual meaning of reg.
> 
> > +            mdio {
> > +                #address-cells = <1>;
> > +                #size-cells = <0>;
> > +
> > +                internal_phy0: phy@0 {
> > +                    reg = <0>;
> 
> So given that airoha,base_smi_address defaults to 1, this is actually
> address 1 on the MDIO bus?
>

Yes correct. One problem I had was that moving this outside the swich
cause panic as it does conflict with the switch PHY address...

-- 
	Ansuel
Re: [net-next RFC PATCH v2 1/3] dt-bindings: net: dsa: Add Airoha AN8855 Gigabit Switch documentation
Posted by Andrew Lunn 1 month ago
> Well the first case that comes to mind is multiple switch and conflict.
> I have no idea if there are hw strap to configure this so I assume if a
> SoC have 2 switch (maybe of the same type), this permits to configure
> them (with reset pin and deasserting them once the base address is
> correctly configured)

Is this switch internal on an internal MDIO bus, or external?

Most PHYs and switches i've seen have strapping pins to set the base
address. It would be unusual if there was not strapping.

For the Marvell switches, the strapping moves all the MDIO
registers. This is why we have a reg at the top level in mv88e6xxx:

        ethernet-switch@0 {
            compatible = "marvell,mv88e6085";
            reg = <0>;

There is one family which use the values of 0 or 16, and each switch
uses 16 addresses. So you can put two on the bus.

> > > +  mdio:
> > > +    $ref: /schemas/net/mdio.yaml#
> > > +    unevaluatedProperties: false
> > > +    description:
> > > +      Define the relative address of the internal PHY for each port.
> > > +
> > > +      Each reg for the PHY is relative to the switch base PHY address.
> > 
> > Which is not the usual meaning of reg.
> > 
> > > +            mdio {
> > > +                #address-cells = <1>;
> > > +                #size-cells = <0>;
> > > +
> > > +                internal_phy0: phy@0 {
> > > +                    reg = <0>;
> > 
> > So given that airoha,base_smi_address defaults to 1, this is actually
> > address 1 on the MDIO bus?
> >
> 
> Yes correct. One problem I had was that moving this outside the swich
> cause panic as it does conflict with the switch PHY address...

I would make these addresses absolute, not relative. The example above
from the marvell switch, the device using addresses 16-31 has its PHYs
within that range, and we uses the absolute reg values.

	Andrew
Re: [net-next RFC PATCH v2 1/3] dt-bindings: net: dsa: Add Airoha AN8855 Gigabit Switch documentation
Posted by Christian Marangi 1 month ago
On Wed, Oct 23, 2024 at 07:39:01PM +0200, Andrew Lunn wrote:
> > Well the first case that comes to mind is multiple switch and conflict.
> > I have no idea if there are hw strap to configure this so I assume if a
> > SoC have 2 switch (maybe of the same type), this permits to configure
> > them (with reset pin and deasserting them once the base address is
> > correctly configured)
> 
> Is this switch internal on an internal MDIO bus, or external?

External so it can be mounted on any SoC given correct mdio/mdc.

> 
> Most PHYs and switches i've seen have strapping pins to set the base
> address. It would be unusual if there was not strapping.

Same feeling, but I didn't found anything in the documentation.
(actually no mention of hw strap or pin)

> 
> For the Marvell switches, the strapping moves all the MDIO
> registers. This is why we have a reg at the top level in mv88e6xxx:
> 
>         ethernet-switch@0 {
>             compatible = "marvell,mv88e6085";
>             reg = <0>;
> 
> There is one family which use the values of 0 or 16, and each switch
> uses 16 addresses. So you can put two on the bus.
> 

Yes this is what that property does. Everything is shifted.

> > > > +  mdio:
> > > > +    $ref: /schemas/net/mdio.yaml#
> > > > +    unevaluatedProperties: false
> > > > +    description:
> > > > +      Define the relative address of the internal PHY for each port.
> > > > +
> > > > +      Each reg for the PHY is relative to the switch base PHY address.
> > > 
> > > Which is not the usual meaning of reg.
> > > 
> > > > +            mdio {
> > > > +                #address-cells = <1>;
> > > > +                #size-cells = <0>;
> > > > +
> > > > +                internal_phy0: phy@0 {
> > > > +                    reg = <0>;
> > > 
> > > So given that airoha,base_smi_address defaults to 1, this is actually
> > > address 1 on the MDIO bus?
> > >
> > 
> > Yes correct. One problem I had was that moving this outside the swich
> > cause panic as it does conflict with the switch PHY address...
> 
> I would make these addresses absolute, not relative. The example above
> from the marvell switch, the device using addresses 16-31 has its PHYs
> within that range, and we uses the absolute reg values.
>

They were relative with the base SMI implementation in mind (as we would
then offset) If the path is to drop that option then yes, these address
should be absolute.

Or do you think even with that option, these address should be absolute?

-- 
	Ansuel