Message ID | 20170927073414.17361-1-clabbe.montjoie@gmail.com |
---|---|
Headers | show |
Series | net: stmmac: dwmac-sun8i: Handle integrated PHY | expand |
On Wed, Sep 27, 2017 at 07:34:09AM +0000, Corentin Labbe wrote: > Since dwmac-sun8i could use either an integrated PHY or an external PHY > (which could be at same MDIO address), we need to represent this selection > by a MDIO switch. > > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com> > --- > arch/arm/boot/dts/sunxi-h3-h5.dtsi | 31 +++++++++++++++++++++++++------ > 1 file changed, 25 insertions(+), 6 deletions(-) > > diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi > index 3b7d953429a6..a8e9b8f378ba 100644 > --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi > +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi > @@ -422,14 +422,33 @@ > #size-cells = <0>; > status = "disabled"; > > - mdio: mdio { > + mdio0: mdio { > #address-cells = <1>; > #size-cells = <0>; > - int_mii_phy: ethernet-phy@1 { > - compatible = "ethernet-phy-ieee802.3-c22"; > - reg = <1>; > - clocks = <&ccu CLK_BUS_EPHY>; > - resets = <&ccu RST_BUS_EPHY>; > + compatible = "snps,dwmac-mdio"; > + > + mdio-mux { > + compatible = "mdio-mux"; > + #address-cells = <1>; > + #size-cells = <0>; Newline > + /* Only one MDIO is usable at the time */ > + internal_mdio: mdio@1 { > + reg = <1>; > + #address-cells = <1>; > + #size-cells = <0>; Newline > + int_mii_phy: ethernet-phy@1 { > + compatible = "ethernet-phy-ieee802.3-c22"; > + reg = <1>; > + clocks = <&ccu CLK_BUS_EPHY>; > + resets = <&ccu RST_BUS_EPHY>; > + phy-is-integrated; > + }; > + }; Newline > + mdio: mdio@2 { This is quite confusing. Why not call the label external_mdio? Thanks
On Wed, Sep 27, 2017 at 12:16:22PM +0200, Maxime Ripard wrote: > On Wed, Sep 27, 2017 at 07:34:09AM +0000, Corentin Labbe wrote: > > Since dwmac-sun8i could use either an integrated PHY or an external PHY > > (which could be at same MDIO address), we need to represent this selection > > by a MDIO switch. > > > > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com> > > --- > > arch/arm/boot/dts/sunxi-h3-h5.dtsi | 31 +++++++++++++++++++++++++------ > > 1 file changed, 25 insertions(+), 6 deletions(-) > > > > diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi > > index 3b7d953429a6..a8e9b8f378ba 100644 > > --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi > > +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi > > @@ -422,14 +422,33 @@ > > #size-cells = <0>; > > status = "disabled"; > > > > - mdio: mdio { > > + mdio0: mdio { > > #address-cells = <1>; > > #size-cells = <0>; > > - int_mii_phy: ethernet-phy@1 { > > - compatible = "ethernet-phy-ieee802.3-c22"; > > - reg = <1>; > > - clocks = <&ccu CLK_BUS_EPHY>; > > - resets = <&ccu RST_BUS_EPHY>; > > + compatible = "snps,dwmac-mdio"; > > + > > + mdio-mux { > > + compatible = "mdio-mux"; > > + #address-cells = <1>; > > + #size-cells = <0>; > > Newline > > > + /* Only one MDIO is usable at the time */ > > + internal_mdio: mdio@1 { > > + reg = <1>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > Newline > > > + int_mii_phy: ethernet-phy@1 { > > + compatible = "ethernet-phy-ieee802.3-c22"; > > + reg = <1>; > > + clocks = <&ccu CLK_BUS_EPHY>; > > + resets = <&ccu RST_BUS_EPHY>; > > + phy-is-integrated; > > + }; > > + }; > > Newline > > > + mdio: mdio@2 { > > This is quite confusing. Why not call the label external_mdio? > I will do it. (at origin I was not changing it for limiting changes on board with external PHY, but now all DT are reverted, it will be easy) Regards -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Sep 27, 2017 at 09:34:14AM +0200, Corentin Labbe wrote: > Each child node of an MDIO node is scanned as a PHY when calling > of_mdiobus_register() givint the following result: > [ 18.175379] mdio_bus stmmac-0: /soc/ethernet@1c30000/mdio/mdio-mux has invalid PHY address > [ 18.175408] mdio_bus stmmac-0: scan phy mdio-mux at address 0 > [ 18.175450] mdio_bus stmmac-0: scan phy mdio-mux at address 1 > [...] > [ 18.176420] mdio_bus stmmac-0: scan phy mdio-mux at address 30 > [ 18.176452] mdio_bus stmmac-0: scan phy mdio-mux at address 31 > > Since mdio-mux nodes are not PHY, this patch a way to to not scan > them. Hi Corentin I still don't like this, but ... > > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com> > --- > drivers/of/of_mdio.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c > index d94dd8b77abd..d90ddb0d90f2 100644 > --- a/drivers/of/of_mdio.c > +++ b/drivers/of/of_mdio.c > @@ -190,6 +190,10 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np) > struct device_node *child; > bool scanphys = false; > int addr, rc; > + static const struct of_device_id do_not_scan[] = { > + { .compatible = "mdio-mux" }, > + {} > + }; Please rename this to some less generic. What i don't want is other compatible strings added here. We want to make the exception for muxes, but nothing else. So something like compatible_muxes? Andrew -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/27/2017 07:12 AM, Andrew Lunn wrote: > On Wed, Sep 27, 2017 at 09:34:14AM +0200, Corentin Labbe wrote: >> Each child node of an MDIO node is scanned as a PHY when calling >> of_mdiobus_register() givint the following result: >> [ 18.175379] mdio_bus stmmac-0: /soc/ethernet@1c30000/mdio/mdio-mux has invalid PHY address >> [ 18.175408] mdio_bus stmmac-0: scan phy mdio-mux at address 0 >> [ 18.175450] mdio_bus stmmac-0: scan phy mdio-mux at address 1 >> [...] >> [ 18.176420] mdio_bus stmmac-0: scan phy mdio-mux at address 30 >> [ 18.176452] mdio_bus stmmac-0: scan phy mdio-mux at address 31 >> >> Since mdio-mux nodes are not PHY, this patch a way to to not scan >> them. > > Hi Corentin > > I still don't like this, but ... Me neither, even more so as I don't understand the reasoning behind putting the mux as a child node of the MDIO bus controller in the first place. Also, you need to re-order patches such that this patch comes before the DTS changes. > >> >> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com> >> --- >> drivers/of/of_mdio.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c >> index d94dd8b77abd..d90ddb0d90f2 100644 >> --- a/drivers/of/of_mdio.c >> +++ b/drivers/of/of_mdio.c >> @@ -190,6 +190,10 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np) >> struct device_node *child; >> bool scanphys = false; >> int addr, rc; >> + static const struct of_device_id do_not_scan[] = { >> + { .compatible = "mdio-mux" }, >> + {} >> + }; > > Please rename this to some less generic. What i don't want is other > compatible strings added here. We want to make the exception for > muxes, but nothing else. So something like compatible_muxes? > > Andrew >
On Wed, Sep 27, 2017 at 09:50:46PM -0700, Florian Fainelli wrote: > > > On 09/27/2017 07:12 AM, Andrew Lunn wrote: > > On Wed, Sep 27, 2017 at 09:34:14AM +0200, Corentin Labbe wrote: > >> Each child node of an MDIO node is scanned as a PHY when calling > >> of_mdiobus_register() givint the following result: > >> [ 18.175379] mdio_bus stmmac-0: /soc/ethernet@1c30000/mdio/mdio-mux has invalid PHY address > >> [ 18.175408] mdio_bus stmmac-0: scan phy mdio-mux at address 0 > >> [ 18.175450] mdio_bus stmmac-0: scan phy mdio-mux at address 1 > >> [...] > >> [ 18.176420] mdio_bus stmmac-0: scan phy mdio-mux at address 30 > >> [ 18.176452] mdio_bus stmmac-0: scan phy mdio-mux at address 31 > >> > >> Since mdio-mux nodes are not PHY, this patch a way to to not scan > >> them. > > > > Hi Corentin > > > > I still don't like this, but ... > > Me neither, even more so as I don't understand the reasoning behind > putting the mux as a child node of the MDIO bus controller in the first > place. If no one agrees with me, then you shouldn't just accept my answers. I'm not always right. Looking at this some more, why can't mdio-mux-mmio be used? Then there is nothing to review or discuss. If you want to extend that then make it clear this is just extending/inheriting already defined bindings. That being said, if we were starting over I would probably do mux designs differently with DT hierarchy reflecting the mux hierarchy, but that ship has sailed. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Looking at this some more, why can't mdio-mux-mmio be used?
Hi Rob
That was my first thought. And then i dug into the details. The mux is
more than just a couple of bits in an address space. Clocks needs to
be enabled/disabled, and the MAC needs a reset, when the mux changes.
So the mux needs intimate knowledge of MAC.
Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html