| Message ID | 20190520131401.11804-4-jbrunet@baylibre.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 280c17df8fbf0000ba53cb741ee822d9662f8c9e |
| Headers | show |
| Series | arm64: dts: meson: g12a: add ethernet support | expand |
Hi Jerome, On Mon, May 20, 2019 at 3:14 PM Jerome Brunet <jbrunet@baylibre.com> wrote: > > Add the g12a mdio multiplexer which allows to connect to either > an external phy through the SoC pins or the internal 10/100 phy > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> Acked-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> > --- > arch/arm64/boot/dts/amlogic/meson-g12a.dtsi | 32 +++++++++++++++++++++ > 1 file changed, 32 insertions(+) > > diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi > index def02ebf6501..90da7cc81681 100644 > --- a/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi > +++ b/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi > @@ -1698,6 +1698,38 @@ > assigned-clock-rates = <100000000>; > #phy-cells = <1>; > }; > + > + eth_phy: mdio-multiplexer@4c000 { > + compatible = "amlogic,g12a-mdio-mux"; > + reg = <0x0 0x4c000 0x0 0xa4>; > + clocks = <&clkc CLKID_ETH_PHY>, > + <&xtal>, > + <&clkc CLKID_MPLL_50M>; > + clock-names = "pclk", "clkin0", "clkin1"; > + mdio-parent-bus = <&mdio0>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + ext_mdio: mdio@0 { > + reg = <0>; > + #address-cells = <1>; > + #size-cells = <0>; > + }; > + > + int_mdio: mdio@1 { > + reg = <1>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + internal_ephy: ethernet_phy@8 { > + compatible = "ethernet-phy-id0180.3301", > + "ethernet-phy-ieee802.3-c22"; Based on your comment on v1 of this patch [0] the Ethernet PHY ID is defined by this "mdio-multiplexer" (write arbitrary value to a register then that's the PHY ID which will show up on the bus) I'm fine with explicitly listing the ID which the PHY driver binds to because I don't know a better way. +Cc Andrew, Florian and Heiner because I think they should be aware of such cases (it seems like a special case to me). Martin [0] https://patchwork.kernel.org/patch/10939255/
> > + int_mdio: mdio@1 { > > + reg = <1>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + internal_ephy: ethernet_phy@8 { > > + compatible = "ethernet-phy-id0180.3301", > > + "ethernet-phy-ieee802.3-c22"; > Based on your comment on v1 of this patch [0] the Ethernet PHY ID is > defined by this "mdio-multiplexer" (write arbitrary value to a > register then that's the PHY ID which will show up on the bus) > I'm fine with explicitly listing the ID which the PHY driver binds to > because I don't know a better way. Does reading the ID registers give the correct ID, once you have poked registers in the mdio-multiplexer? If so, you don't need this compatible string. If the read is giving the wrong ID, then yes, you do want this. But then please add a comment in the DT blob. This is very unusual, so should have some explanation why it is needed. Thanks Andrew
On Mon, 2019-05-20 at 21:05 +0200, Andrew Lunn wrote: > > > + int_mdio: mdio@1 { > > > + reg = <1>; > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + > > > + internal_ephy: ethernet_phy@8 { > > > + compatible = "ethernet-phy-id0180.3301", > > > + "ethernet-phy-ieee802.3-c22"; > > Based on your comment on v1 of this patch [0] the Ethernet PHY ID is > > defined by this "mdio-multiplexer" (write arbitrary value to a > > register then that's the PHY ID which will show up on the bus) > > I'm fine with explicitly listing the ID which the PHY driver binds to > > because I don't know a better way. ... > > Does reading the ID registers give the correct ID, once you have poked > registers in the mdio-multiplexer? If so, you don't need this > compatible string. Hi Andrew, In 5.1 the mdio-mux set a wrong simply because I got it wrong. I pushed a fix for that, and maybe it has already hit mainline. As I pointed to Martin on v1, this situation just shows that this setting is weaker than the usual phy setup. I do understand why we don't want to put the PHY id in DT. If the PHY fitted on the board changes, we want to pick it up. This particular phy is embedded in SoC, we know it won't change for this SoC, whatever the mdio-mux sets. So yes it should (soon) work as usual, setting this id is not strictly necessary but it nicely reflect that this particular phy is integrated in the SoC and we know which driver to use. So, if this is ok with you, I'd prefer to keep this particular id around. > > If the read is giving the wrong ID, then yes, you do want this. But > then please add a comment in the DT blob. This is very unusual, so > should have some explanation why it is needed. Sure, can add a comment. I suggest to do it in follow-up. At least it keeps things aligned between the gxl, which has been like this for quite a while now, and g12a. > > Thanks > Andrew
Jerome Brunet <jbrunet@baylibre.com> writes: > On Mon, 2019-05-20 at 21:05 +0200, Andrew Lunn wrote: >> > > + int_mdio: mdio@1 { >> > > + reg = <1>; >> > > + #address-cells = <1>; >> > > + #size-cells = <0>; >> > > + >> > > + internal_ephy: ethernet_phy@8 { >> > > + compatible = "ethernet-phy-id0180.3301", >> > > + "ethernet-phy-ieee802.3-c22"; >> > Based on your comment on v1 of this patch [0] the Ethernet PHY ID is >> > defined by this "mdio-multiplexer" (write arbitrary value to a >> > register then that's the PHY ID which will show up on the bus) >> > I'm fine with explicitly listing the ID which the PHY driver binds to >> > because I don't know a better way. > > ... > >> >> Does reading the ID registers give the correct ID, once you have poked >> registers in the mdio-multiplexer? If so, you don't need this >> compatible string. > > Hi Andrew, > > In 5.1 the mdio-mux set a wrong simply because I got it wrong. I pushed a > fix for that, and maybe it has already hit mainline. > > As I pointed to Martin on v1, this situation just shows that this setting is > weaker than the usual phy setup. > > I do understand why we don't want to put the PHY id in DT. If the PHY fitted on > the board changes, we want to pick it up. This particular phy is embedded in > SoC, we know it won't change for this SoC, whatever the mdio-mux sets. > > So yes it should (soon) work as usual, setting this id is not strictly > necessary but it nicely reflect that this particular phy is integrated in > the SoC and we know which driver to use. > > So, if this is ok with you, I'd prefer to keep this particular id around. Seems OK to me, so I'm queuing this for v5.3 because we really need network support. It can be removed later if it's really insisted on. >> >> If the read is giving the wrong ID, then yes, you do want this. But >> then please add a comment in the DT blob. This is very unusual, so >> should have some explanation why it is needed. > > Sure, can add a comment. I suggest to do it in follow-up. At least it keeps > things aligned between the gxl, which has been like this for quite a while now, > and g12a. Follow-up is good for me, Kevin
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi index def02ebf6501..90da7cc81681 100644 --- a/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi +++ b/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi @@ -1698,6 +1698,38 @@ assigned-clock-rates = <100000000>; #phy-cells = <1>; }; + + eth_phy: mdio-multiplexer@4c000 { + compatible = "amlogic,g12a-mdio-mux"; + reg = <0x0 0x4c000 0x0 0xa4>; + clocks = <&clkc CLKID_ETH_PHY>, + <&xtal>, + <&clkc CLKID_MPLL_50M>; + clock-names = "pclk", "clkin0", "clkin1"; + mdio-parent-bus = <&mdio0>; + #address-cells = <1>; + #size-cells = <0>; + + ext_mdio: mdio@0 { + reg = <0>; + #address-cells = <1>; + #size-cells = <0>; + }; + + int_mdio: mdio@1 { + reg = <1>; + #address-cells = <1>; + #size-cells = <0>; + + internal_ephy: ethernet_phy@8 { + compatible = "ethernet-phy-id0180.3301", + "ethernet-phy-ieee802.3-c22"; + interrupts = <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>; + reg = <8>; + max-speed = <100>; + }; + }; + }; }; aobus: bus@ff800000 {
Add the g12a mdio multiplexer which allows to connect to either an external phy through the SoC pins or the internal 10/100 phy Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> --- arch/arm64/boot/dts/amlogic/meson-g12a.dtsi | 32 +++++++++++++++++++++ 1 file changed, 32 insertions(+)