| Message ID | 20241022184727.3206180-1-niklas.soderlund+renesas@ragnatech.se (mailing list archive) |
|---|---|
| Headers | show |
| Series | arm64: dts: renesas: falcon: Wire-up Ethernet breakout board | expand |
Hi Niklas, Thanks for your series! On Tue, Oct 22, 2024 at 8:48 PM Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> wrote: > This small series wires up the Marvell 88Q2110 PHYs found on the Falcon > Ethernet breakout board. With this applied all five PHYs are probed > correctly. > > mv88q2110 e6810000.ethernet-ffffffff:07: attached PHY driver (mii_bus:phy_addr=e6810000.ethernet-ffffffff:07, irq=POLL) > mv88q2110 e6820000.ethernet-ffffffff:07: attached PHY driver (mii_bus:phy_addr=e6820000.ethernet-ffffffff:07, irq=POLL) > mv88q2110 e6830000.ethernet-ffffffff:07: attached PHY driver (mii_bus:phy_addr=e6830000.ethernet-ffffffff:07, irq=POLL) > mv88q2110 e6840000.ethernet-ffffffff:07: attached PHY driver (mii_bus:phy_addr=e6840000.ethernet-ffffffff:07, irq=POLL) > mv88q2110 e6850000.ethernet-ffffffff:07: attached PHY driver (mii_bus:phy_addr=e6850000.ethernet-ffffffff:07, irq=POLL) > > They can be auto detected with just the compatible > "ethernet-phy-ieee802.3-c45", but to keep the style currently used I > have added the specific compatible for the 88Q2110 as done for other > SoCs. If the specific compatible values are not needed, I prefer not to add them, as DT should describe only what cannot be auto-detected[1]. Have you tried kexec and/or unbinding/rebinding the AVB driver (the latter is probably easiest)? > The primary issue we had with this in the past was due to an incorrect > PHY address. After studying the schematics (v100) I found the PHYs > address pins are wired differently on Falcon compared to other Gen4 > boards. On Falcon they are pulled-down, while on other Gen4 boards they > are left unconnected and subjected to the PHYs internal pull-ups. This > gives the PHY an address where the lower 3 bits of the address is > inverted for Falcon. This was changed in v102 of the schematics (REV0043c vs. REV0043b of the schematics for the Ethernet sub board): See "Changed Strap pin settings ==> PHYAD=[0,0,0], pull-down removed" on page 1, and the various PHY configuration notes... Moreover, this might be different in other board revisions, as the BSP uses PHY address 1 for RAVB1, address 2 for RAVB2, and so on... As I only had remote access to Falcon, I never knew the actual board revision I was using. How to handle this (yet another DTS file)? Are non-v100 variants widespread? [1] In theory, we could drop all SoC- and family-specific compatible values, and just look at the Product Register. I do not suggest going that way ;-) Gr{oetje,eeting}s, Geert
> Are non-v100 variants widespread?
I'd say nothing related to the Falcon could be called "widespread"...
Hi Wolfram, On Wed, Oct 23, 2024 at 9:35 AM Wolfram Sang <wsa+renesas@sang-engineering.com> wrote: > > Are non-v100 variants widespread? > > I'd say nothing related to the Falcon could be called "widespread"... I agree (but I didn't want to be the one to spell it out ;-) Gr{oetje,eeting}s, Geert
Hi Geert, On 2024-10-23 09:13:11 +0200, Geert Uytterhoeven wrote: > Hi Niklas, > > Thanks for your series! > > On Tue, Oct 22, 2024 at 8:48 PM Niklas Söderlund > <niklas.soderlund+renesas@ragnatech.se> wrote: > > This small series wires up the Marvell 88Q2110 PHYs found on the Falcon > > Ethernet breakout board. With this applied all five PHYs are probed > > correctly. > > > > mv88q2110 e6810000.ethernet-ffffffff:07: attached PHY driver (mii_bus:phy_addr=e6810000.ethernet-ffffffff:07, irq=POLL) > > mv88q2110 e6820000.ethernet-ffffffff:07: attached PHY driver (mii_bus:phy_addr=e6820000.ethernet-ffffffff:07, irq=POLL) > > mv88q2110 e6830000.ethernet-ffffffff:07: attached PHY driver (mii_bus:phy_addr=e6830000.ethernet-ffffffff:07, irq=POLL) > > mv88q2110 e6840000.ethernet-ffffffff:07: attached PHY driver (mii_bus:phy_addr=e6840000.ethernet-ffffffff:07, irq=POLL) > > mv88q2110 e6850000.ethernet-ffffffff:07: attached PHY driver (mii_bus:phy_addr=e6850000.ethernet-ffffffff:07, irq=POLL) > > > > They can be auto detected with just the compatible > > "ethernet-phy-ieee802.3-c45", but to keep the style currently used I > > have added the specific compatible for the 88Q2110 as done for other > > SoCs. > > If the specific compatible values are not needed, I prefer not to add > them, as DT should describe only what cannot be auto-detected[1]. Ok, I will post a v2 without the specific compatible value. > Have you tried kexec and/or unbinding/rebinding the AVB driver > (the latter is probably easiest)? I have tested unbinding/rebinding the driver both with and without a specific compatible value, both scenarios work. > > > The primary issue we had with this in the past was due to an incorrect > > PHY address. After studying the schematics (v100) I found the PHYs > > address pins are wired differently on Falcon compared to other Gen4 > > boards. On Falcon they are pulled-down, while on other Gen4 boards they > > are left unconnected and subjected to the PHYs internal pull-ups. This > > gives the PHY an address where the lower 3 bits of the address is > > inverted for Falcon. > > This was changed in v102 of the schematics (REV0043c vs. REV0043b of > the schematics for the Ethernet sub board): See "Changed Strap pin > settings ==> PHYAD=[0,0,0], pull-down removed" on page 1, and the > various PHY configuration notes... > Moreover, this might be different in other board revisions, as the > BSP uses PHY address 1 for RAVB1, address 2 for RAVB2, and so on... > > As I only had remote access to Falcon, I never knew the actual board > revision I was using. I could not find any revision markings on the board. But to the best of my ability (the markings are not super clear) I have identified that the pull-down resistors are indeed present on the board I have. > > How to handle this (yet another DTS file)? > Are non-v100 variants widespread? As Wolfram points out, I don't think it's widespread. I think it's OK to move forward with this as all the V3U boards we have seen have this configuration. If one pops-up later we can solve it then right? As solving this properly would need ether a separate DTS source or at least an overlay anyhow. Is this acceptable for you? If so I will post a v2 with the change proposed above. > > [1] In theory, we could drop all SoC- and family-specific compatible > values, and just look at the Product Register. I do not suggest > going that way ;-) > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds >
Hi Niklas, On Wed, Oct 23, 2024 at 2:41 PM Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> wrote: > On 2024-10-23 09:13:11 +0200, Geert Uytterhoeven wrote: > > On Tue, Oct 22, 2024 at 8:48 PM Niklas Söderlund > > <niklas.soderlund+renesas@ragnatech.se> wrote: > > > This small series wires up the Marvell 88Q2110 PHYs found on the Falcon > > > Ethernet breakout board. With this applied all five PHYs are probed > > > correctly. > > > > > > mv88q2110 e6810000.ethernet-ffffffff:07: attached PHY driver (mii_bus:phy_addr=e6810000.ethernet-ffffffff:07, irq=POLL) > > > mv88q2110 e6820000.ethernet-ffffffff:07: attached PHY driver (mii_bus:phy_addr=e6820000.ethernet-ffffffff:07, irq=POLL) > > > mv88q2110 e6830000.ethernet-ffffffff:07: attached PHY driver (mii_bus:phy_addr=e6830000.ethernet-ffffffff:07, irq=POLL) > > > mv88q2110 e6840000.ethernet-ffffffff:07: attached PHY driver (mii_bus:phy_addr=e6840000.ethernet-ffffffff:07, irq=POLL) > > > mv88q2110 e6850000.ethernet-ffffffff:07: attached PHY driver (mii_bus:phy_addr=e6850000.ethernet-ffffffff:07, irq=POLL) > > > > > > They can be auto detected with just the compatible > > > "ethernet-phy-ieee802.3-c45", but to keep the style currently used I > > > have added the specific compatible for the 88Q2110 as done for other > > > SoCs. > > > > If the specific compatible values are not needed, I prefer not to add > > them, as DT should describe only what cannot be auto-detected[1]. > > Ok, I will post a v2 without the specific compatible value. > > > Have you tried kexec and/or unbinding/rebinding the AVB driver > > (the latter is probably easiest)? > > I have tested unbinding/rebinding the driver both with and without a > specific compatible value, both scenarios work. Great! > > > The primary issue we had with this in the past was due to an incorrect > > > PHY address. After studying the schematics (v100) I found the PHYs > > > address pins are wired differently on Falcon compared to other Gen4 > > > boards. On Falcon they are pulled-down, while on other Gen4 boards they > > > are left unconnected and subjected to the PHYs internal pull-ups. This > > > gives the PHY an address where the lower 3 bits of the address is > > > inverted for Falcon. > > > > This was changed in v102 of the schematics (REV0043c vs. REV0043b of > > the schematics for the Ethernet sub board): See "Changed Strap pin > > settings ==> PHYAD=[0,0,0], pull-down removed" on page 1, and the > > various PHY configuration notes... > > Moreover, this might be different in other board revisions, as the > > BSP uses PHY address 1 for RAVB1, address 2 for RAVB2, and so on... > > > > As I only had remote access to Falcon, I never knew the actual board > > revision I was using. > > I could not find any revision markings on the board. But to the best of > my ability (the markings are not super clear) I have identified that the > pull-down resistors are indeed present on the board I have. > > > How to handle this (yet another DTS file)? > > Are non-v100 variants widespread? > > As Wolfram points out, I don't think it's widespread. I think it's OK to > move forward with this as all the V3U boards we have seen have this > configuration. If one pops-up later we can solve it then right? As > solving this properly would need ether a separate DTS source or at least > an overlay anyhow. > > Is this acceptable for you? If so I will post a v2 with the change > proposed above. Yes, sounds good to me. Thanks! Gr{oetje,eeting}s, Geert
Hi Geert, This small series wires up the Marvell 88Q2110 PHYs found on the Falcon Ethernet breakout board. With this applied all five PHYs are probed correctly. mv88q2110 e6810000.ethernet-ffffffff:07: attached PHY driver (mii_bus:phy_addr=e6810000.ethernet-ffffffff:07, irq=POLL) mv88q2110 e6820000.ethernet-ffffffff:07: attached PHY driver (mii_bus:phy_addr=e6820000.ethernet-ffffffff:07, irq=POLL) mv88q2110 e6830000.ethernet-ffffffff:07: attached PHY driver (mii_bus:phy_addr=e6830000.ethernet-ffffffff:07, irq=POLL) mv88q2110 e6840000.ethernet-ffffffff:07: attached PHY driver (mii_bus:phy_addr=e6840000.ethernet-ffffffff:07, irq=POLL) mv88q2110 e6850000.ethernet-ffffffff:07: attached PHY driver (mii_bus:phy_addr=e6850000.ethernet-ffffffff:07, irq=POLL) They can be auto detected with just the compatible "ethernet-phy-ieee802.3-c45", but to keep the style currently used I have added the specific compatible for the 88Q2110 as done for other SoCs. The primary issue we had with this in the past was due to an incorrect PHY address. After studying the schematics (v100) I found the PHYs address pins are wired differently on Falcon compared to other Gen4 boards. On Falcon they are pulled-down, while on other Gen4 boards they are left unconnected and subjected to the PHYs internal pull-ups. This gives the PHY an address where the lower 3 bits of the address is inverted for Falcon. Patch 1/2 is a simple preparation patch removing properties that should not be set in the base SoC include and will produce warnings once the mdio node is added. Patch 2/2 wires up the PHYs. Most likely the change in 1/2 should be applied to AVB0 also, but pending our discussion in a different patch about where to place the reset-gpio property for mdio nodes that don't strictly need it I left AVB0 as is. Niklas Söderlund (2): arm64: dts: renesas: r8a779a0: Remove address- and size-cells from AVB[1-5] arm64: dts: renesas: falcon: ethernet: Describe PHYs connected on the breakout board .../dts/renesas/r8a779a0-falcon-ethernet.dtsi | 248 ++++++++++++++++++ arch/arm64/boot/dts/renesas/r8a779a0.dtsi | 10 - 2 files changed, 248 insertions(+), 10 deletions(-)