From: clabbe.montjoie@gmail.com (Corentin Labbe)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3 3/4] net: stmmac: register parent MDIO node for sun8i-h3-emac
Date: Tue, 22 Aug 2017 20:11:40 +0200 [thread overview]
Message-ID: <20170822181140.GA11596@Red> (raw)
In-Reply-To: <bfe784b6-5137-8fcd-dbd0-ad96974f4060@gmail.com>
On Tue, Aug 22, 2017 at 09:40:24AM -0700, Florian Fainelli wrote:
> On 08/22/2017 08:39 AM, Chen-Yu Tsai wrote:
> > On Mon, Aug 21, 2017 at 10:23 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> >>> All muxes are mostly always represented the same way afaik, or do you
> >>> want to simply introduce a new compatible / property?
> >>
> >> + mdio-mux {
> >> + compatible = "allwinner,sun8i-h3-mdio-switch";
> >> + mdio-parent-bus = <&mdio_parent>;
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> +
> >> + internal_mdio: mdio at 1 {
> >> reg = <1>;
> >> - clocks = <&ccu CLK_BUS_EPHY>;
> >> - resets = <&ccu RST_BUS_EPHY>;
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> + int_mii_phy: ethernet-phy at 1 {
> >> + compatible = "ethernet-phy-ieee802.3-c22";
> >> + reg = <1>;
> >> + clocks = <&ccu CLK_BUS_EPHY>;
> >> + resets = <&ccu RST_BUS_EPHY>;
> >> + phy-is-integrated;
> >> + };
> >> + };
> >> + mdio: mdio at 0 {
> >> + reg = <0>;
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> };
> >>
> >> Hi Maxim
> >>
> >> Anybody who knows the MDIO-mux code/binding, knows that it is a run
> >> time mux. You swap the mux per MDIO transaction. You can access all
> >> the PHY and switches on the mux'ed MDIO bus.
> >>
> >> However here, it is effectively a boot-time MUX. You cannot change it
> >> on the fly. What happens when somebody has a phandle to a PHY on the
> >> internal and a phandle to a phy on the external? Does the driver at
> >> least return -EINVAL, or -EBUSY? Is there a representation which
> >> eliminates this possibility?
> >
> > There is only one controller. Either you use the internal PHY, which
> > is then directly coupled (no magnetics needed) to the RJ45 port, or
> > you use an external PHY over MII/RMII/RGMII. You could supposedly
> > have both on a board, and let the user choose one. But why bother
> > with the extra complexity and cost? Either you use the internal PHY
> > at 100M, or an external RGMII PHY for gigabit speeds.
>
> I agree, there is no point in over-engineering any of this. I don't
> think there is actually any MDIO mux per-se in that the MDIO clock and
> data lines are muxed, however there has to be some kind of built-in port
> multiplexer that lets you chose between connecting to the internal PHY
> and any external PHY/MAC, but that is not what a "mdio-mux" node represents.
>
> >
> > So I think what you are saying is either impossible or engineering-wise
> > a very stupid design, like using an external MAC with a discrete PHY
> > connected to the internal MAC's MDIO bus, while using the internal MAC
> > with the internal PHY.
> >
> > Now can we please decide on something? We're a week and a half from
> > the 4.13 release. If mdio-mux is wrong, then we could have two mdio
> > nodes (internal-mdio & external-mdio).
>
> I really don't see a need for a mdio-mux in the first place, just have
> one MDIO controller (current state) sub-node which describes the
> built-in STMMAC MDIO controller and declare the internal PHY as a child
> node (along with 'phy-is-integrated'). If a different configuration is
> used, then just put the external PHY as a child node there.
>
> If fixed-link is required, the mdio node becomes unused anyway.
>
> Works for everyone?
If we put an external PHY with reg=1 as a child of internal MDIO, il will be merged with internal PHY node and get phy-is-integrated.
Does two MDIO node "internal-mdio" and "mdio" works for you ?
(We keep "mdio" for external MDIO for reducing the number of patchs)
Thanks
Regards
diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
index 4b599b5d26f6..d5e7cf0d9454 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
@@ -417,7 +417,8 @@
#size-cells = <0>;
status = "disabled";
- mdio: mdio {
+ /* Only one MDIO is usable at the time */
+ internal_mdio: mdio at 1 {
#address-cells = <1>;
#size-cells = <0>;
int_mii_phy: ethernet-phy at 1 {
@@ -425,8 +426,13 @@
reg = <1>;
clocks = <&ccu CLK_BUS_EPHY>;
resets = <&ccu RST_BUS_EPHY>;
+ phy-is-integrated;
};
};
+ mdio: mdio at 0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
};
spi0: spi at 01c68000 {
WARNING: multiple messages have this Message-ID (diff)
From: Corentin Labbe <clabbe.montjoie@gmail.com>
To: Florian Fainelli <f.fainelli@gmail.com>
Cc: Chen-Yu Tsai <wens@csie.org>, Andrew Lunn <andrew@lunn.ch>,
Maxime Ripard <maxime.ripard@free-electrons.com>,
Rob Herring <robh+dt@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Russell King <linux@armlinux.org.uk>,
Giuseppe Cavallaro <peppe.cavallaro@st.com>,
Alexandre Torgue <alexandre.torgue@st.com>,
devicetree <devicetree@vger.kernel.org>,
linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
linux-kernel <linux-kernel@vger.kernel.org>,
netdev <netdev@vger.kernel.org>
Subject: Re: [PATCH v3 3/4] net: stmmac: register parent MDIO node for sun8i-h3-emac
Date: Tue, 22 Aug 2017 20:11:40 +0200 [thread overview]
Message-ID: <20170822181140.GA11596@Red> (raw)
In-Reply-To: <bfe784b6-5137-8fcd-dbd0-ad96974f4060@gmail.com>
On Tue, Aug 22, 2017 at 09:40:24AM -0700, Florian Fainelli wrote:
> On 08/22/2017 08:39 AM, Chen-Yu Tsai wrote:
> > On Mon, Aug 21, 2017 at 10:23 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> >>> All muxes are mostly always represented the same way afaik, or do you
> >>> want to simply introduce a new compatible / property?
> >>
> >> + mdio-mux {
> >> + compatible = "allwinner,sun8i-h3-mdio-switch";
> >> + mdio-parent-bus = <&mdio_parent>;
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> +
> >> + internal_mdio: mdio@1 {
> >> reg = <1>;
> >> - clocks = <&ccu CLK_BUS_EPHY>;
> >> - resets = <&ccu RST_BUS_EPHY>;
> >> + #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>;
> >> + phy-is-integrated;
> >> + };
> >> + };
> >> + mdio: mdio@0 {
> >> + reg = <0>;
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> };
> >>
> >> Hi Maxim
> >>
> >> Anybody who knows the MDIO-mux code/binding, knows that it is a run
> >> time mux. You swap the mux per MDIO transaction. You can access all
> >> the PHY and switches on the mux'ed MDIO bus.
> >>
> >> However here, it is effectively a boot-time MUX. You cannot change it
> >> on the fly. What happens when somebody has a phandle to a PHY on the
> >> internal and a phandle to a phy on the external? Does the driver at
> >> least return -EINVAL, or -EBUSY? Is there a representation which
> >> eliminates this possibility?
> >
> > There is only one controller. Either you use the internal PHY, which
> > is then directly coupled (no magnetics needed) to the RJ45 port, or
> > you use an external PHY over MII/RMII/RGMII. You could supposedly
> > have both on a board, and let the user choose one. But why bother
> > with the extra complexity and cost? Either you use the internal PHY
> > at 100M, or an external RGMII PHY for gigabit speeds.
>
> I agree, there is no point in over-engineering any of this. I don't
> think there is actually any MDIO mux per-se in that the MDIO clock and
> data lines are muxed, however there has to be some kind of built-in port
> multiplexer that lets you chose between connecting to the internal PHY
> and any external PHY/MAC, but that is not what a "mdio-mux" node represents.
>
> >
> > So I think what you are saying is either impossible or engineering-wise
> > a very stupid design, like using an external MAC with a discrete PHY
> > connected to the internal MAC's MDIO bus, while using the internal MAC
> > with the internal PHY.
> >
> > Now can we please decide on something? We're a week and a half from
> > the 4.13 release. If mdio-mux is wrong, then we could have two mdio
> > nodes (internal-mdio & external-mdio).
>
> I really don't see a need for a mdio-mux in the first place, just have
> one MDIO controller (current state) sub-node which describes the
> built-in STMMAC MDIO controller and declare the internal PHY as a child
> node (along with 'phy-is-integrated'). If a different configuration is
> used, then just put the external PHY as a child node there.
>
> If fixed-link is required, the mdio node becomes unused anyway.
>
> Works for everyone?
If we put an external PHY with reg=1 as a child of internal MDIO, il will be merged with internal PHY node and get phy-is-integrated.
Does two MDIO node "internal-mdio" and "mdio" works for you ?
(We keep "mdio" for external MDIO for reducing the number of patchs)
Thanks
Regards
diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
index 4b599b5d26f6..d5e7cf0d9454 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
@@ -417,7 +417,8 @@
#size-cells = <0>;
status = "disabled";
- mdio: mdio {
+ /* Only one MDIO is usable at the time */
+ internal_mdio: mdio@1 {
#address-cells = <1>;
#size-cells = <0>;
int_mii_phy: ethernet-phy@1 {
@@ -425,8 +426,13 @@
reg = <1>;
clocks = <&ccu CLK_BUS_EPHY>;
resets = <&ccu RST_BUS_EPHY>;
+ phy-is-integrated;
};
};
+ mdio: mdio@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
};
spi0: spi@01c68000 {
next prev parent reply other threads:[~2017-08-22 18:11 UTC|newest]
Thread overview: 82+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-18 12:21 [PATCH v3 0/4] net: stmmac: Detect PHY location with phy-is-integrated Corentin Labbe
2017-08-18 12:21 ` Corentin Labbe
2017-08-18 12:21 ` [PATCH v3 1/4] ARM: dts: sunxi: h3/h5: represent the mdio switch used by sun8i-h3-emac Corentin Labbe
2017-08-18 12:21 ` Corentin Labbe
2017-08-18 12:21 ` [PATCH v3 2/4] net: stmmac: dwmac-sun8i: choose internal PHY via phy-is-integrated Corentin Labbe
2017-08-18 12:21 ` Corentin Labbe
2017-08-18 16:58 ` Chen-Yu Tsai
2017-08-18 16:58 ` Chen-Yu Tsai
2017-08-22 16:40 ` Florian Fainelli
2017-08-22 16:40 ` Florian Fainelli
2017-08-18 12:21 ` [PATCH v3 3/4] net: stmmac: register parent MDIO node for sun8i-h3-emac Corentin Labbe
2017-08-18 12:21 ` Corentin Labbe
2017-08-18 17:05 ` Chen-Yu Tsai
2017-08-18 17:05 ` Chen-Yu Tsai
2017-08-19 18:50 ` Corentin Labbe
2017-08-19 18:50 ` Corentin Labbe
2017-08-19 20:38 ` Andrew Lunn
2017-08-19 20:38 ` Andrew Lunn
2017-08-20 6:57 ` Corentin Labbe
2017-08-20 6:57 ` Corentin Labbe
2017-08-20 6:57 ` Corentin Labbe
2017-08-20 14:25 ` Andrew Lunn
2017-08-20 14:25 ` Andrew Lunn
2017-08-20 14:25 ` Andrew Lunn
2017-08-21 8:10 ` Chen-Yu Tsai
2017-08-21 8:10 ` Chen-Yu Tsai
2017-08-21 13:20 ` Andrew Lunn
2017-08-21 13:20 ` Andrew Lunn
2017-08-21 13:31 ` Maxime Ripard
2017-08-21 13:31 ` Maxime Ripard
2017-08-21 14:23 ` Andrew Lunn
2017-08-21 14:23 ` Andrew Lunn
2017-08-21 14:23 ` Andrew Lunn
2017-08-22 7:59 ` Corentin Labbe
2017-08-22 7:59 ` Corentin Labbe
2017-08-22 15:39 ` Chen-Yu Tsai
2017-08-22 15:39 ` Chen-Yu Tsai
2017-08-22 15:39 ` Chen-Yu Tsai
2017-08-22 16:40 ` Florian Fainelli
2017-08-22 16:40 ` Florian Fainelli
2017-08-22 16:40 ` Florian Fainelli
2017-08-22 18:11 ` Corentin Labbe [this message]
2017-08-22 18:11 ` Corentin Labbe
2017-08-22 18:35 ` Florian Fainelli
2017-08-22 18:35 ` Florian Fainelli
2017-08-22 19:37 ` Corentin Labbe
2017-08-22 19:37 ` Corentin Labbe
2017-08-23 7:49 ` Maxime Ripard
2017-08-23 7:49 ` Maxime Ripard
2017-08-23 16:31 ` Florian Fainelli
2017-08-23 16:31 ` Florian Fainelli
2017-08-24 8:12 ` Maxime Ripard
2017-08-24 8:12 ` Maxime Ripard
2017-08-24 8:12 ` Maxime Ripard
2017-08-24 8:21 ` Corentin Labbe
2017-08-24 8:21 ` Corentin Labbe
2017-08-24 19:44 ` Corentin Labbe
2017-08-24 19:44 ` Corentin Labbe
2017-08-24 19:59 ` Florian Fainelli
2017-08-24 19:59 ` Florian Fainelli
2017-08-24 19:59 ` Florian Fainelli
2017-08-25 2:54 ` Chen-Yu Tsai
2017-08-25 2:54 ` Chen-Yu Tsai
2017-08-25 3:05 ` Florian Fainelli
2017-08-25 3:05 ` Florian Fainelli
2017-08-25 3:05 ` Florian Fainelli
2017-08-25 3:41 ` Chen-Yu Tsai
2017-08-25 3:41 ` Chen-Yu Tsai
2017-08-25 3:41 ` Chen-Yu Tsai
2017-08-25 3:59 ` Florian Fainelli
2017-08-25 3:59 ` Florian Fainelli
2017-08-25 13:26 ` Andrew Lunn
2017-08-25 13:26 ` Andrew Lunn
2017-08-22 16:50 ` Maxime Ripard
2017-08-22 16:50 ` Maxime Ripard
2017-08-22 16:50 ` Maxime Ripard
2017-08-18 12:21 ` [PATCH v3 4/4] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY Corentin Labbe
2017-08-18 12:21 ` Corentin Labbe
2017-08-18 16:57 ` Chen-Yu Tsai
2017-08-18 16:57 ` Chen-Yu Tsai
2017-08-19 18:33 ` Corentin Labbe
2017-08-19 18:33 ` Corentin Labbe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170822181140.GA11596@Red \
--to=clabbe.montjoie@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.