| Message ID | 20220909132614.1967276-6-yoshihiro.shimoda.uh@renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | treewide: Add R-Car S4-8 Ethernet Switch support | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply, async |
> + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + port@0 { > + reg = <0>; > + phy-handle = <ða0>; > + phy-mode = "sgmii"; > + #address-cells = <1>; > + #size-cells = <0>; > + etha0: ethernet-phy@0 { > + reg = <1>; reg = 1 means you should have @1. > + compatible = "ethernet-phy-ieee802.3-c45"; > + }; > + }; You are mixing Ethernet and MDIO properties in one node. Past experience says this is a bad idea, particularly when you have switches involved. I would suggest you add an mdio container: > + port@1 { > + reg = <1>; > + phy-handle = <ða1>; > + phy-mode = "sgmii"; > + #address-cells = <1>; > + #size-cells = <0>; mdio { > + etha1: ethernet-phy@1 { > + reg = <2>; > + compatible = "ethernet-phy-ieee802.3-c45"; > + }; }; > + }; > + port@2 { > + reg = <2>; > + phy-handle = <ða2>; > + phy-mode = "sgmii"; > + #address-cells = <1>; > + #size-cells = <0>; > + etha2: ethernet-phy@2 { > + reg = <3>; > + compatible = "ethernet-phy-ieee802.3-c45"; > + }; > + }; I find it interesting you have PHYs are address 1, 2, 3, even though they are on individual busses. Why pay for the extra pullup/down resistors when they could all have the same address? Andrew
Hi Andrew, Thank you for your review! > From: Andrew Lunn, Sent: Tuesday, September 13, 2022 9:16 AM > > > + ports { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + port@0 { > > + reg = <0>; > > + phy-handle = <ða0>; > > + phy-mode = "sgmii"; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + etha0: ethernet-phy@0 { > > + reg = <1>; > > reg = 1 means you should have @1. I'll fix it. > > + compatible = "ethernet-phy-ieee802.3-c45"; > > + }; > > + }; > > You are mixing Ethernet and MDIO properties in one node. Past > experience says this is a bad idea, particularly when you have > switches involved. I would suggest you add an mdio container: > > > > + port@1 { > > + reg = <1>; > > + phy-handle = <ða1>; > > + phy-mode = "sgmii"; > > + #address-cells = <1>; > > + #size-cells = <0>; > > mdio { > > + etha1: ethernet-phy@1 { > > + reg = <2>; > > + compatible = "ethernet-phy-ieee802.3-c45"; > > + }; > }; > > + }; Thank you for the suggestion. I'll fix it. > > + port@2 { > > + reg = <2>; > > + phy-handle = <ða2>; > > + phy-mode = "sgmii"; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + etha2: ethernet-phy@2 { > > + reg = <3>; > > + compatible = "ethernet-phy-ieee802.3-c45"; > > + }; > > + }; > > I find it interesting you have PHYs are address 1, 2, 3, even though > they are on individual busses. Why pay for the extra pullup/down > resistors when they could all have the same address? I don't know why. But, the board really configured such PHY addresses... Best regards, Yoshihiro Shimoda > Andrew
> > > + port@2 { > > > + reg = <2>; > > > + phy-handle = <ða2>; > > > + phy-mode = "sgmii"; > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + etha2: ethernet-phy@2 { > > > + reg = <3>; > > > + compatible = "ethernet-phy-ieee802.3-c45"; > > > + }; > > > + }; > > > > I find it interesting you have PHYs are address 1, 2, 3, even though > > they are on individual busses. Why pay for the extra pullup/down > > resistors when they could all have the same address? > > I don't know why. But, the board really configured such PHY addresses... That is not wrong. It could be the hardware engineer is used to shared MDIO busses, and just copy/pasted an existing design, but then separated the busses? You might see actual customer boards putting all the PHYs on one MDIO bus, to save pins. Linux has no problem with that, the phy-handle can point anywhere. One last thought. Is there anything in the data sheet about the switch hardware directly talking the PHY? Some of the Marvell switches can do that, but we disable that feature. The hardware has no idea what the PHY driver is doing, such as selecting different pages. Andrew
Hi Andrew, > From: Andrew Lunn, Sent: Wednesday, September 14, 2022 9:14 PM > > > > > + port@2 { > > > > + reg = <2>; > > > > + phy-handle = <ða2>; > > > > + phy-mode = "sgmii"; > > > > + #address-cells = <1>; > > > > + #size-cells = <0>; > > > > + etha2: ethernet-phy@2 { > > > > + reg = <3>; > > > > + compatible = "ethernet-phy-ieee802.3-c45"; > > > > + }; > > > > + }; > > > > > > I find it interesting you have PHYs are address 1, 2, 3, even though > > > they are on individual busses. Why pay for the extra pullup/down > > > resistors when they could all have the same address? > > > > I don't know why. But, the board really configured such PHY addresses... > > That is not wrong. It could be the hardware engineer is used to shared > MDIO busses, and just copy/pasted an existing design, but then > separated the busses? It's possible. > You might see actual customer boards putting all the PHYs on one MDIO > bus, to save pins. Linux has no problem with that, the phy-handle can > point anywhere. I see. > One last thought. Is there anything in the data sheet about the switch > hardware directly talking the PHY? Yes, the switch hardware can talk the PHY directly. > Some of the Marvell switches can do > that, but we disable that feature. The hardware has no idea what the > PHY driver is doing, such as selecting different pages. That's interesting. Best regards, Yoshihiro Shimoda
diff --git a/arch/arm64/boot/dts/renesas/r8a779f0-spider-ethernet.dtsi b/arch/arm64/boot/dts/renesas/r8a779f0-spider-ethernet.dtsi index 15e8d1ebf575..db3b6e51e2b6 100644 --- a/arch/arm64/boot/dts/renesas/r8a779f0-spider-ethernet.dtsi +++ b/arch/arm64/boot/dts/renesas/r8a779f0-spider-ethernet.dtsi @@ -13,3 +13,47 @@ eeprom@52 { pagesize = <8>; }; }; + +&rswitch { + status = "okay"; + #address-cells = <1>; + #size-cells = <0>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + port@0 { + reg = <0>; + phy-handle = <ða0>; + phy-mode = "sgmii"; + #address-cells = <1>; + #size-cells = <0>; + etha0: ethernet-phy@0 { + reg = <1>; + compatible = "ethernet-phy-ieee802.3-c45"; + }; + }; + port@1 { + reg = <1>; + phy-handle = <ða1>; + phy-mode = "sgmii"; + #address-cells = <1>; + #size-cells = <0>; + etha1: ethernet-phy@1 { + reg = <2>; + compatible = "ethernet-phy-ieee802.3-c45"; + }; + }; + port@2 { + reg = <2>; + phy-handle = <ða2>; + phy-mode = "sgmii"; + #address-cells = <1>; + #size-cells = <0>; + etha2: ethernet-phy@2 { + reg = <3>; + compatible = "ethernet-phy-ieee802.3-c45"; + }; + }; + }; +};
Enable Ethernet Switch for R-Car S4-8 (r8a779f0). Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> --- .../dts/renesas/r8a779f0-spider-ethernet.dtsi | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+)