| Message ID | 20250120040214.2538839-2-chris.packham@alliedtelesis.co.nz (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | RTL9300 MDIO driver | expand |
On Mon, Jan 20, 2025 at 05:02:11PM +1300, Chris Packham wrote: > Add dtschema for the MDIO controller found in the RTL9300 SoCs. The > controller is slightly unusual in that direct MDIO communication is not > possible. We model the MDIO controller with the MDIO buses as child > nodes and the PHYs as children of the buses. Because we do need the > switch port number to actually communicate over the MDIO bus this needs > to be supplied via the "realtek,port" property. > > Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> > --- > > Notes: > Changes in v4: > - Model the MDIO controller with the buses as child nodes. We still need > to deal with the switch port number so this is represented with the > "realtek,port" property which needs to be added to the MDIO bus > children (i.e. the PHYs) > - Because the above is quite a departure from earlier I've dropped the > r-by > Changes in v3: > - Add r-by from Connor > Changes in v2: > - None > > .../bindings/net/realtek,rtl9301-mdio.yaml | 93 +++++++++++++++++++ > 1 file changed, 93 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml > > diff --git a/Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml b/Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml > new file mode 100644 > index 000000000000..e3ecb1b4afd3 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml > @@ -0,0 +1,93 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/net/realtek,rtl9301-mdio.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Realtek RTL9300 MDIO Controller > + > +maintainers: > + - Chris Packham <chris.packham@alliedtelesis.co.nz> > + > +properties: > + compatible: > + oneOf: > + - items: > + - enum: > + - realtek,rtl9302b-mdio > + - realtek,rtl9302c-mdio > + - realtek,rtl9303-mdio > + - const: realtek,rtl9301-mdio > + - const: realtek,rtl9301-mdio > + > + '#address-cells': > + const: 1 > + > + '#size-cells': > + const: 0 > + > +patternProperties: > + '^mdio-bus@[0-4]$': > + $ref: mdio.yaml# > + > + properties: > + reg: > + maxItems: 1 > + > + required: > + - reg > + > + patternProperties: > + '^ethernet-phy(@[a-f0-9]+)?': Why is the unit address optional? > + type: object > + $ref: ethernet-phy.yaml# > + > + properties: > + realtek,port: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + The MDIO communication on the RTL9300 is abstracted by the switch. At > + the software level communication uses the switch port to address the > + PHY with the actual MDIO bus and address having been setup via the > + parent mdio-bus and reg property. I don't quite get why this cannot be the 'reg' property. I understood that 'reg' of this node is not really used? Or you meant here this 'reg', not parent's 'reg'? Best regards, Krzysztof
Hi Krzyztof, Sorry meant to reply to this yesterday but ran out of time On 22/01/2025 21:12, Krzysztof Kozlowski wrote: > On Mon, Jan 20, 2025 at 05:02:11PM +1300, Chris Packham wrote: >> Add dtschema for the MDIO controller found in the RTL9300 SoCs. The >> controller is slightly unusual in that direct MDIO communication is not >> possible. We model the MDIO controller with the MDIO buses as child >> nodes and the PHYs as children of the buses. Because we do need the >> switch port number to actually communicate over the MDIO bus this needs >> to be supplied via the "realtek,port" property. >> >> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> >> --- >> >> Notes: >> Changes in v4: >> - Model the MDIO controller with the buses as child nodes. We still need >> to deal with the switch port number so this is represented with the >> "realtek,port" property which needs to be added to the MDIO bus >> children (i.e. the PHYs) >> - Because the above is quite a departure from earlier I've dropped the >> r-by >> Changes in v3: >> - Add r-by from Connor >> Changes in v2: >> - None >> >> .../bindings/net/realtek,rtl9301-mdio.yaml | 93 +++++++++++++++++++ >> 1 file changed, 93 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml >> >> diff --git a/Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml b/Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml >> new file mode 100644 >> index 000000000000..e3ecb1b4afd3 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml >> @@ -0,0 +1,93 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/net/realtek,rtl9301-mdio.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Realtek RTL9300 MDIO Controller >> + >> +maintainers: >> + - Chris Packham <chris.packham@alliedtelesis.co.nz> >> + >> +properties: >> + compatible: >> + oneOf: >> + - items: >> + - enum: >> + - realtek,rtl9302b-mdio >> + - realtek,rtl9302c-mdio >> + - realtek,rtl9303-mdio >> + - const: realtek,rtl9301-mdio >> + - const: realtek,rtl9301-mdio >> + >> + '#address-cells': >> + const: 1 >> + >> + '#size-cells': >> + const: 0 >> + >> +patternProperties: >> + '^mdio-bus@[0-4]$': >> + $ref: mdio.yaml# >> + >> + properties: >> + reg: >> + maxItems: 1 >> + >> + required: >> + - reg >> + >> + patternProperties: >> + '^ethernet-phy(@[a-f0-9]+)?': > Why is the unit address optional? No specific reason. I can make it mandatory, in all likelihood any board using this chip will have more than one PHY connected so it would have been defacto required anyway. >> + type: object >> + $ref: ethernet-phy.yaml# >> + >> + properties: >> + realtek,port: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + description: >> + The MDIO communication on the RTL9300 is abstracted by the switch. At >> + the software level communication uses the switch port to address the >> + PHY with the actual MDIO bus and address having been setup via the >> + parent mdio-bus and reg property. > I don't quite get why this cannot be the 'reg' property. I understood that > 'reg' of this node is not really used? Or you meant here this 'reg', not > parent's 'reg'? It's is a bit confusing (any suggestions for improving the description and/or commit message are welcome). The 'reg' property here is the MDIO address of the PHY, the parent 'reg' is the MDIO bus number. That's the information the "normal" bindings for Ethernet PHYs use. The MDIO (a.k.a. SMI) accesses via the RTL9300 use the switch port so we need to know the switch port number. An earlier incarnation of this patchset had the switch port number masquerading as the MDIO address but that would have allowed nonsensical addresses >0x1f and meant that aspects of the hardware design were tucked away in special vendor specific properties. The MDIO bus/address is still used it's just setup once at probe time and now my driver maps the MDIO bus/address to the switch port number that the hardware ultimately uses.
> >> + properties: > >> + realtek,port: > >> + $ref: /schemas/types.yaml#/definitions/uint32 > >> + description: > >> + The MDIO communication on the RTL9300 is abstracted by the switch. At > >> + the software level communication uses the switch port to address the > >> + PHY with the actual MDIO bus and address having been setup via the > >> + parent mdio-bus and reg property. > > I don't quite get why this cannot be the 'reg' property. I understood that > > 'reg' of this node is not really used? Or you meant here this 'reg', not > > parent's 'reg'? > > It's is a bit confusing (any suggestions for improving the description > and/or commit message are welcome). I don't know if it will actually help, but.... We have two entangled configurations here. 1) You have 4 MDIO busses which you need to describe using mdio.yaml In this binding, reg is the address of the device on the bus, in the range 0-31. 2) The hardware was a pool of PHYs which you can map to address on the MDIO busses. Rather than combining them, maybe it would be better to keep them separate. It is probably more error prone, but simpler to understand. And hopefully errors result in PHYs not being found during probe, so the problems are obvious. Maybe you can actually use phandles. You have the usual MDIO bus nodes: mdio@5c030000 { #address-cells = <1>; #size-cells = <0>; ethphy0: ethernet-phy@1 { reg = <1>; }; ethphy1: ethernet-phy@3 { reg = <3>; }; }; mdio@5c040000 { #address-cells = <1>; #size-cells = <0>; ethphy2: ethernet-phy@1 { reg = <1>; }; ethphy3: ethernet-phy@3 { reg = <3>; }; }; mdio@5c050000 { ... } mdio@5c060000 { ... } And then a node which is a list of PHY phandles: [ðphy0, ðphy1, ðphy2, ðphy3, ....] The 0th entry in the list tells you have to map the 0th PHY in the pool to an MDIO bus and address. Follow the phandle to get the MDIO bus and the address on the bus. Andrew
On 24/01/2025 11:08, Andrew Lunn wrote: >>>> + properties: >>>> + realtek,port: >>>> + $ref: /schemas/types.yaml#/definitions/uint32 >>>> + description: >>>> + The MDIO communication on the RTL9300 is abstracted by the switch. At >>>> + the software level communication uses the switch port to address the >>>> + PHY with the actual MDIO bus and address having been setup via the >>>> + parent mdio-bus and reg property. >>> I don't quite get why this cannot be the 'reg' property. I understood that >>> 'reg' of this node is not really used? Or you meant here this 'reg', not >>> parent's 'reg'? >> It's is a bit confusing (any suggestions for improving the description >> and/or commit message are welcome). > I don't know if it will actually help, but.... > > We have two entangled configurations here. > > 1) You have 4 MDIO busses which you need to describe using mdio.yaml > In this binding, reg is the address of the device on the bus, in > the range 0-31. > > 2) The hardware was a pool of PHYs which you can map to address on the > MDIO busses. > > Rather than combining them, maybe it would be better to keep them > separate. It is probably more error prone, but simpler to > understand. And hopefully errors result in PHYs not being found during > probe, so the problems are obvious. > > Maybe you can actually use phandles. You have the usual MDIO bus > nodes: > > mdio@5c030000 { > #address-cells = <1>; > #size-cells = <0>; > > ethphy0: ethernet-phy@1 { > reg = <1>; > }; > > ethphy1: ethernet-phy@3 { > reg = <3>; > }; > }; > > mdio@5c040000 { > #address-cells = <1>; > #size-cells = <0>; > > ethphy2: ethernet-phy@1 { > reg = <1>; > }; > > ethphy3: ethernet-phy@3 { > reg = <3>; > }; > }; > > mdio@5c050000 { > ... > } > > mdio@5c060000 { > ... > } > > And then a node which is a list of PHY phandles: > > [ðphy0, ðphy1, ðphy2, ðphy3, ....] > > The 0th entry in the list tells you have to map the 0th PHY in the > pool to an MDIO bus and address. Follow the phandle to get the MDIO > bus and the address on the bus. A fuller dts would be something like this (for the 8-port board I have in front of me) mdio-controller@ca00 { compatible = "realtek,rtl9301-mdio"; reg = <0xca00>; mdio-bus@0 { reg = <0x00>; ethphy0: ethernet-phy@0 { reg = <0x00>; compatible = "ethernet-phy-ieee802.3-c45"; }; ethphy1: ethernet-phy@1 { reg = <0x01>; compatible = "ethernet-phy-ieee802.3-c45"; }; ethphy2: ethernet-phy@2 { reg = <0x02>; compatible = "ethernet-phy-ieee802.3-c45"; }; ethphy3: ethernet-phy@3 { reg = <0x03>; compatible = "ethernet-phy-ieee802.3-c45"; }; }; mdio-bus@1 { reg = <0x01>; ethphy4: ethernet-phy@8 { reg = <0x00>; compatible = "ethernet-phy-ieee802.3-c45"; }; ethphy5: ethernet-phy@9 { reg = <0x01>; compatible = "ethernet-phy-ieee802.3-c45"; }; ethphy6: ethernet-phy@10 { reg = <0x02>; compatible = "ethernet-phy-ieee802.3-c45"; }; ethphy7: ethernet-phy@11 { reg = <0x03>; compatible = "ethernet-phy-ieee802.3-c45"; }; }; realtek,ports = [ðphy0, ðphy1, ðphy2, ðphy3, /* need a gap here as there are 4 unused ports*/ 0, 0, 0, 0, ðphy4, ðphy5, ðphy6, ðphy7]; }; I could probably make it work but I'm not sure that it is any more understandable.
diff --git a/Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml b/Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml new file mode 100644 index 000000000000..e3ecb1b4afd3 --- /dev/null +++ b/Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml @@ -0,0 +1,93 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/net/realtek,rtl9301-mdio.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Realtek RTL9300 MDIO Controller + +maintainers: + - Chris Packham <chris.packham@alliedtelesis.co.nz> + +properties: + compatible: + oneOf: + - items: + - enum: + - realtek,rtl9302b-mdio + - realtek,rtl9302c-mdio + - realtek,rtl9303-mdio + - const: realtek,rtl9301-mdio + - const: realtek,rtl9301-mdio + + '#address-cells': + const: 1 + + '#size-cells': + const: 0 + +patternProperties: + '^mdio-bus@[0-4]$': + $ref: mdio.yaml# + + properties: + reg: + maxItems: 1 + + required: + - reg + + patternProperties: + '^ethernet-phy(@[a-f0-9]+)?': + type: object + $ref: ethernet-phy.yaml# + + properties: + realtek,port: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + The MDIO communication on the RTL9300 is abstracted by the switch. At + the software level communication uses the switch port to address the + PHY with the actual MDIO bus and address having been setup via the + parent mdio-bus and reg property. + + unevaluatedProperties: false + + unevaluatedProperties: false + +required: + - compatible + +unevaluatedProperties: false + +examples: + - | + mdio-controller { + compatible = "realtek,rtl9301-mdio"; + #address-cells = <1>; + #size-cells = <0>; + + mdio-bus@0 { + reg = <0>; + #address-cells = <1>; + #size-cells = <0>; + + ethernet-phy@0 { + compatible = "ethernet-phy-ieee802.3-c45"; + reg = <0>; + realtek,port = <0>; + }; + }; + + mdio-bus@1 { + reg = <1>; + #address-cells = <1>; + #size-cells = <0>; + + ethernet-phy@0 { + compatible = "ethernet-phy-ieee802.3-c45"; + reg = <0>; + realtek,port = <8>; + }; + }; + };
Add dtschema for the MDIO controller found in the RTL9300 SoCs. The controller is slightly unusual in that direct MDIO communication is not possible. We model the MDIO controller with the MDIO buses as child nodes and the PHYs as children of the buses. Because we do need the switch port number to actually communicate over the MDIO bus this needs to be supplied via the "realtek,port" property. Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> --- Notes: Changes in v4: - Model the MDIO controller with the buses as child nodes. We still need to deal with the switch port number so this is represented with the "realtek,port" property which needs to be added to the MDIO bus children (i.e. the PHYs) - Because the above is quite a departure from earlier I've dropped the r-by Changes in v3: - Add r-by from Connor Changes in v2: - None .../bindings/net/realtek,rtl9301-mdio.yaml | 93 +++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml