Hello
The current way to find if the PHY is internal is to compare DT phy-mode
and emac_variant/internal_phy.
But it will negate a possible future SoC where an external PHY use the
same phy mode than the integrated one.
This patchs series adds a new way to find if the PHY is integrated, via
the phy-is-integrated DT property.
Since it exists both integrated and external ethernet-phy@1, they are merged in
the final DTB and so share all properties.
For avoiding this, and better represent the reality, we use a MDIO mux.
The first try was to create a new MDIO mux "mdio-mux-syscon".
mdio-mux-syscon working the same way than mdio-mux-mmioreg with the exception
that the register is used via syscon/regmap.
But this solution does not work for two reason:
- changing the MDIO selection need the reset of MAC which cannot be done by the
mdio-mux-syscon driver
- There were driver loading order problem:
- mdio-mux-syscon needing that stmmac register the parent MDIO
- stmmac needing that child MDIO was registered just after registering parent MDIO
So we cannot use any external MDIO-mux.
The final solution was to represent a mdio-mux but let the MAC handle all things.
The only hack that comes with this solution is that we add a patch
"net: stmmac: Register parent MDIO in case of fake mdio-mux"
because we have ino other way to know which MDIO node to register.
Note that sun8i-v3s-emac have also an integrated PHY, but since it lacks
any external PHY support it is not necessary to add MDIO mux to it.
All patchs should go via the net tree with exception of DT patchs which should
go via the sunxi tree
Note that this serie will need backporting the patch
"Documentation: net: phy: Add phy-is-integrated binding" which is in net-next
Regards
Changes since v3:
- Added a patch for handling fixed-link
- Updated documentation
Changes since v2:
- Add a MDIO mux for creating distinction between integrated and external MDIO.
- phy-is-integrated is not set in dtsi.
Changes since v1:
- Dropped phy-is-integrated documentation patch since another same patch was already merged
- Moved phy-is-integrated from SoC dtsi to final board DT.
Corentin Labbe (5):
net: stmmac: Handle possible fixed-link with need_mdio_ids
ARM: dts: sunxi: h3/h5: represent the mdio switch used by
sun8i-h3-emac
dt-bindings: net: dwmac-sun8i: update documentation about integrated
PHY
net: stmmac: dwmac-sun8i: choose internal PHY via phy-is-integrated
net: stmmac: Register parent MDIO in case of fake mdio-mux
.../devicetree/bindings/net/dwmac-sun8i.txt | 117 ++++++++++++++++++++-
arch/arm/boot/dts/sunxi-h3-h5.dtsi | 22 +++-
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 16 +--
.../net/ethernet/stmicro/stmmac/stmmac_platform.c | 13 ++-
4 files changed, 152 insertions(+), 16 deletions(-)
--
2.13.5
This patch add documentation about the MDIO switch used on sun8i-h3-emac
for integrated PHY.
Signed-off-by: Corentin Labbe <[email protected]>
---
.../devicetree/bindings/net/dwmac-sun8i.txt | 117 ++++++++++++++++++++-
1 file changed, 112 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
index 725f3b187886..5751f7afc5dd 100644
--- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
+++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
@@ -39,7 +39,7 @@ Optional properties for the following compatibles:
- allwinner,leds-active-low: EPHY LEDs are active low
Required child node of emac:
-- mdio bus node: should be named mdio
+- mdio bus node: should be labelled mdio
Required properties of the mdio node:
- #address-cells: shall be 1
@@ -48,14 +48,25 @@ Required properties of the mdio node:
The device node referenced by "phy" or "phy-handle" should be a child node
of the mdio node. See phy.txt for the generic PHY bindings.
-Required properties of the phy node with the following compatibles:
+The following compatibles require an mdio-mux node:
+ - "allwinner,sun8i-h3-emac"
+Required properties for the mdio-mux node:
+ - compatible = "mdio-mux"
+ - two child mdio, one for the integrated mdio, one for the external mdio
+ - mdio-parent-bus: a phandle to the emac's MDIO node
+
+The following compatibles require a PHY node representing the integrated
+PHY, under the integrated MDIO bus node if an mdio-mux node is used:
- "allwinner,sun8i-h3-emac",
- "allwinner,sun8i-v3s-emac":
+
+Required properties of the integrated phy node:
- clocks: a phandle to the reference clock for the EPHY
- resets: a phandle to the reset control for the EPHY
+- phy-is-integrated
+- Should be a child of the integrated mdio
-Example:
-
+Example with integrated PHY:
emac: ethernet@1c0b000 {
compatible = "allwinner,sun8i-h3-emac";
syscon = <&syscon>;
@@ -72,13 +83,109 @@ emac: ethernet@1c0b000 {
phy-handle = <&int_mii_phy>;
phy-mode = "mii";
allwinner,leds-active-low;
- mdio: mdio {
+
+ mdio0: mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "snps,dwmac-mdio";
+ };
+
+};
+eth-phy-mux {
+ compatible = "mdio-mux";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ mdio-parent-bus = <&mdio0>;
+
+ int_mdio: mdio@1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ int_mii_phy: ethernet-phy@1 {
+ reg = <1>;
+ clocks = <&ccu CLK_BUS_EPHY>;
+ resets = <&ccu RST_BUS_EPHY>;
+ phy-is-integrated
+ };
+ };
+ ext_mdio: mdio@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+};
+
+Example with external PHY:
+emac: ethernet@1c0b000 {
+ compatible = "allwinner,sun8i-h3-emac";
+ syscon = <&syscon>;
+ reg = <0x01c0b000 0x104>;
+ interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "macirq";
+ resets = <&ccu RST_BUS_EMAC>;
+ reset-names = "stmmaceth";
+ clocks = <&ccu CLK_BUS_EMAC>;
+ clock-names = "stmmaceth";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ phy-handle = <&ext_rgmii_phy>;
+ phy-mode = "rgmii";
+ allwinner,leds-active-low;
+
+ mdio0: mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "snps,dwmac-mdio";
+ };
+
+};
+eth-phy-mux {
+ compatible = "mdio-mux";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ mdio-parent-bus = <&mdio0>;
+
+ int_mdio: mdio@1 {
#address-cells = <1>;
#size-cells = <0>;
int_mii_phy: ethernet-phy@1 {
reg = <1>;
clocks = <&ccu CLK_BUS_EPHY>;
resets = <&ccu RST_BUS_EPHY>;
+ phy-is-integrated
+ };
+ };
+ ext_mdio: mdio@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ ext_rgmii_phy: ethernet-phy@1 {
+ reg = <1>;
+ };
+ };
+};
+
+Example with SoC without integrated PHY
+
+emac: ethernet@1c0b000 {
+ compatible = "allwinner,sun8i-a83t-emac";
+ syscon = <&syscon>;
+ reg = <0x01c0b000 0x104>;
+ interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "macirq";
+ resets = <&ccu RST_BUS_EMAC>;
+ reset-names = "stmmaceth";
+ clocks = <&ccu CLK_BUS_EMAC>;
+ clock-names = "stmmaceth";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ phy-handle = <&ext_rgmii_phy>;
+ phy-mode = "rgmii";
+
+ mdio: mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ ext_rgmii_phy: ethernet-phy@1 {
+ reg = <1>;
};
};
};
--
2.13.5
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 <[email protected]>
---
arch/arm/boot/dts/sunxi-h3-h5.dtsi | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
index 4b599b5d26f6..5ffb940a44bb 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
@@ -417,7 +417,22 @@
#size-cells = <0>;
status = "disabled";
- mdio: mdio {
+ mdio0: mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "snps,dwmac-mdio";
+ };
+
+ };
+
+ eth-phy-mux {
+ compatible = "mdio-mux";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ mdio-parent-bus = <&mdio0>;
+
+ /* 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 +440,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 {
--
2.13.5
In case of a fake MDIO switch/mux (like Allwinner H3),
the registered MDIO node should be the parent of the PHY.
Otherwise of_phy_connect will fail.
Signed-off-by: Corentin Labbe <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index e1be5735365b..4d5f3cc82476 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -312,10 +312,12 @@ static int stmmac_dt_phy(struct plat_stmmacenet_data *plat,
static const struct of_device_id need_mdio_ids[] = {
{ .compatible = "snps,dwc-qos-ethernet-4.10" },
{ .compatible = "allwinner,sun8i-a83t-emac" },
- { .compatible = "allwinner,sun8i-h3-emac" },
{ .compatible = "allwinner,sun8i-v3s-emac" },
{ .compatible = "allwinner,sun50i-a64-emac" },
};
+ static const struct of_device_id register_parent_mdio_ids[] = {
+ { .compatible = "allwinner,sun8i-h3-emac" },
+ };
/* If phy-handle property is passed from DT, use it as the PHY */
plat->phy_node = of_parse_phandle(np, "phy-handle", 0);
@@ -332,7 +334,14 @@ static int stmmac_dt_phy(struct plat_stmmacenet_data *plat,
mdio = false;
}
- if (of_match_node(need_mdio_ids, np) && !of_phy_is_fixed_link(np)) {
+ /*
+ * In case of a fake MDIO switch/mux (like Allwinner H3),
+ * the registered MDIO node should be the parent of the PHY.
+ * Otherwise of_phy_connect will fail.
+ */
+ if (of_match_node(register_parent_mdio_ids, np) && !of_phy_is_fixed_link(np)) {
+ plat->mdio_node = of_get_parent(plat->phy_node);
+ } else if (of_match_node(need_mdio_ids, np) && !of_phy_is_fixed_link(np)) {
plat->mdio_node = of_get_child_by_name(np, "mdio");
} else {
/**
--
2.13.5
The current way to find if the phy is internal is to compare DT phy-mode
and emac_variant/internal_phy.
But it will negate a possible future SoC where an external PHY use the
same phy mode than the internal one.
This patch adds a new way to find if the PHY is internal, via
the phy-is-integrated property.
Since the internal_phy variable does not need anymore to contain the xMII mode
used by the internal PHY, it is still used for knowing the presence of an
internal PHY, so it is modified to a boolean soc_has_internal_phy.
Signed-off-by: Corentin Labbe <[email protected]>
Acked-by: Chen-Yu Tsai <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 675a09629d85..c353e5bcb3c1 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -41,14 +41,14 @@
* This value is used for disabling properly EMAC
* and used as a good starting value in case of the
* boot process(uboot) leave some stuff.
- * @internal_phy: Does the MAC embed an internal PHY
+ * @soc_has_internal_phy: Does the MAC embed an internal PHY
* @support_mii: Does the MAC handle MII
* @support_rmii: Does the MAC handle RMII
* @support_rgmii: Does the MAC handle RGMII
*/
struct emac_variant {
u32 default_syscon_value;
- int internal_phy;
+ bool soc_has_internal_phy;
bool support_mii;
bool support_rmii;
bool support_rgmii;
@@ -75,7 +75,7 @@ struct sunxi_priv_data {
static const struct emac_variant emac_variant_h3 = {
.default_syscon_value = 0x58000,
- .internal_phy = PHY_INTERFACE_MODE_MII,
+ .soc_has_internal_phy = true,
.support_mii = true,
.support_rmii = true,
.support_rgmii = true
@@ -83,20 +83,20 @@ static const struct emac_variant emac_variant_h3 = {
static const struct emac_variant emac_variant_v3s = {
.default_syscon_value = 0x38000,
- .internal_phy = PHY_INTERFACE_MODE_MII,
+ .soc_has_internal_phy = true,
.support_mii = true
};
static const struct emac_variant emac_variant_a83t = {
.default_syscon_value = 0,
- .internal_phy = 0,
+ .soc_has_internal_phy = false,
.support_mii = true,
.support_rgmii = true
};
static const struct emac_variant emac_variant_a64 = {
.default_syscon_value = 0,
- .internal_phy = 0,
+ .soc_has_internal_phy = false,
.support_mii = true,
.support_rmii = true,
.support_rgmii = true
@@ -648,7 +648,7 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv)
"Current syscon value is not the default %x (expect %x)\n",
val, reg);
- if (gmac->variant->internal_phy) {
+ if (gmac->variant->soc_has_internal_phy) {
if (!gmac->use_internal_phy) {
/* switch to external PHY interface */
reg &= ~H3_EPHY_SELECT;
@@ -933,7 +933,7 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
}
plat_dat->interface = of_get_phy_mode(dev->of_node);
- if (plat_dat->interface == gmac->variant->internal_phy) {
+ if (of_property_read_bool(plat_dat->phy_node, "phy-is-integrated")) {
dev_info(&pdev->dev, "Will use internal PHY\n");
gmac->use_internal_phy = true;
gmac->ephy_clk = of_clk_get(plat_dat->phy_node, 0);
--
2.13.5
In case of fixed link, there are no mdio node.
This patch add a test for fixed-link for bypassing MDIO node register
that match need_mdio_ids.
Note that this do not change behaviour for MDIO snps,dwmac-mdio nodes.
Signed-off-by: Corentin Labbe <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index a366b3747eeb..e1be5735365b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -332,7 +332,7 @@ static int stmmac_dt_phy(struct plat_stmmacenet_data *plat,
mdio = false;
}
- if (of_match_node(need_mdio_ids, np)) {
+ if (of_match_node(need_mdio_ids, np) && !of_phy_is_fixed_link(np)) {
plat->mdio_node = of_get_child_by_name(np, "mdio");
} else {
/**
--
2.13.5
Hi Corentin
I think we have now all agreed this is an mdio-mux, plus it is also an
MII mux. We should represent that in device tree. This patchset does
this. However, as it is now, the mux structure in DT is ignored. All
it does is search for the phy-is-integrated flags and goes on that.
I made the comment that the device tree representation cannot be
implemented using an MDIO mux driver, because of driver loading
issues. However, the core of the MDIO mux code is just a library,
symbols exported as GPL, free for anything to use.
What i think should happen is the mdio-mux is implemented inside the
MAC driver, using the mux-core as a library. The device tree structure
of a mix is then reflected within Linux. The mux switch callback is
implemented within the MAC driver. So it can reset the MAC when the
mux is switched. The 'phy-is-integrated' property is then no longer
needed.
I would suggest a binding something like:
emac: ethernet@1c0b000 {
compatible = "allwinner,sun8i-h3-emac";
syscon = <&syscon>;
reg = <0x01c0b000 0x104>;
interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
interrupt-names = "macirq";
resets = <&ccu RST_BUS_EMAC>;
reset-names = "stmmaceth";
clocks = <&ccu CLK_BUS_EMAC>;
clock-names = "stmmaceth";
#address-cells = <1>;
#size-cells = <0>;
phy-handle = <&int_mii_phy>;
phy-mode = "mii";
allwinner,leds-active-low;
mdio: mdio {
#address-cells = <1>;
#size-cells = <0>;
}
mdio-mux {
#address-cells = <1>;
#size-cells = <0>;
mdio@0 {
reg = <0>;
#address-cells = <1>;
#size-cells = <0>;
int_mii_phy: ethernet-phy@1 {
reg = <1>;
clocks = <&ccu CLK_BUS_EPHY>;
resets = <&ccu RST_BUS_EPHY>;
};
};
ext_mdio: mdio@0 {
#address-cells = <1>;
#size-cells = <0>;
ext_rgmii_phy: ethernet-phy@1 {
reg = <1>;
};
};
};
};
Andrew
On Sat, Aug 26, 2017 at 11:20:51PM +0200, Andrew Lunn wrote:
> Hi Corentin
>
> I think we have now all agreed this is an mdio-mux, plus it is also an
> MII mux. We should represent that in device tree. This patchset does
> this. However, as it is now, the mux structure in DT is ignored. All
> it does is search for the phy-is-integrated flags and goes on that.
>
> I made the comment that the device tree representation cannot be
> implemented using an MDIO mux driver, because of driver loading
> issues. However, the core of the MDIO mux code is just a library,
> symbols exported as GPL, free for anything to use.
>
> What i think should happen is the mdio-mux is implemented inside the
> MAC driver, using the mux-core as a library. The device tree structure
> of a mix is then reflected within Linux. The mux switch callback is
> implemented within the MAC driver. So it can reset the MAC when the
> mux is switched. The 'phy-is-integrated' property is then no longer
> needed.
It is stilll needed because some settings (allwinner,leds-active-low for example) are only for integrated phy.
>
> I would suggest a binding something like:
>
> emac: ethernet@1c0b000 {
> compatible = "allwinner,sun8i-h3-emac";
> syscon = <&syscon>;
> reg = <0x01c0b000 0x104>;
> interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
> interrupt-names = "macirq";
> resets = <&ccu RST_BUS_EMAC>;
> reset-names = "stmmaceth";
> clocks = <&ccu CLK_BUS_EMAC>;
> clock-names = "stmmaceth";
> #address-cells = <1>;
> #size-cells = <0>;
>
> phy-handle = <&int_mii_phy>;
> phy-mode = "mii";
> allwinner,leds-active-low;
>
> mdio: mdio {
> #address-cells = <1>;
> #size-cells = <0>;
> }
>
> mdio-mux {
> #address-cells = <1>;
> #size-cells = <0>;
>
> mdio@0 {
> reg = <0>;
> #address-cells = <1>;
> #size-cells = <0>;
>
> int_mii_phy: ethernet-phy@1 {
> reg = <1>;
> clocks = <&ccu CLK_BUS_EPHY>;
> resets = <&ccu RST_BUS_EPHY>;
> };
> };
> ext_mdio: mdio@0 {
> #address-cells = <1>;
> #size-cells = <0>;
>
> ext_rgmii_phy: ethernet-phy@1 {
> reg = <1>;
> };
> };
> };
> };
>
I am trying to do that but I get:
dwmac-sun8i 1c30000.ethernet: Error: Failed to find reg for child /soc/ethernet@1c30000/mdio
dwmac-sun8i 1c30000.ethernet: Error: Failed to find reg for child /soc/ethernet@1c30000/mdio-mux
dwmac-sun8i 1c30000.ethernet: Error: No acceptable child buses found
So it seems that mdio_mux_init() must be run on mdio-mux and not on emac node.
But in the current state it cannot be done.
Do you agree that another mdio_mux_init() must be written ? (taking a of_node (in our case: mdio-mux) instead of a device)
Or do I miss something ?
Regards
> Do you agree that another mdio_mux_init() must be written ? (taking
> a of_node (in our case: mdio-mux) instead of a device)
Yes, you are correct, it is using dev->of_node. So you need to
refactor the code a little, to allow an of_node to be passed.
Andrew
On Sat, Aug 26, 2017 at 11:20:51PM +0200, Andrew Lunn wrote:
> Hi Corentin
>
> I think we have now all agreed this is an mdio-mux, plus it is also an
> MII mux. We should represent that in device tree. This patchset does
> this. However, as it is now, the mux structure in DT is ignored. All
> it does is search for the phy-is-integrated flags and goes on that.
>
> I made the comment that the device tree representation cannot be
> implemented using an MDIO mux driver, because of driver loading
> issues. However, the core of the MDIO mux code is just a library,
> symbols exported as GPL, free for anything to use.
>
> What i think should happen is the mdio-mux is implemented inside the
> MAC driver, using the mux-core as a library. The device tree structure
> of a mix is then reflected within Linux. The mux switch callback is
> implemented within the MAC driver. So it can reset the MAC when the
> mux is switched. The 'phy-is-integrated' property is then no longer
> needed.
>
> I would suggest a binding something like:
This is looks better to me, but...
> emac: ethernet@1c0b000 {
> compatible = "allwinner,sun8i-h3-emac";
> syscon = <&syscon>;
> reg = <0x01c0b000 0x104>;
> interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
> interrupt-names = "macirq";
> resets = <&ccu RST_BUS_EMAC>;
> reset-names = "stmmaceth";
> clocks = <&ccu CLK_BUS_EMAC>;
> clock-names = "stmmaceth";
> #address-cells = <1>;
> #size-cells = <0>;
>
> phy-handle = <&int_mii_phy>;
> phy-mode = "mii";
> allwinner,leds-active-low;
>
> mdio: mdio {
> #address-cells = <1>;
> #size-cells = <0>;
> }
Why do you need this node still?
>
> mdio-mux {
> #address-cells = <1>;
> #size-cells = <0>;
>
> mdio@0 {
> reg = <0>;
> #address-cells = <1>;
> #size-cells = <0>;
>
> int_mii_phy: ethernet-phy@1 {
> reg = <1>;
> clocks = <&ccu CLK_BUS_EPHY>;
> resets = <&ccu RST_BUS_EPHY>;
> };
> };
> ext_mdio: mdio@0 {
> #address-cells = <1>;
> #size-cells = <0>;
>
> ext_rgmii_phy: ethernet-phy@1 {
> reg = <1>;
> };
> };
> };
> };
>
> Andrew
On Thu, Aug 31, 2017 at 03:18:03PM -0500, Rob Herring wrote:
> On Sat, Aug 26, 2017 at 11:20:51PM +0200, Andrew Lunn wrote:
> > Hi Corentin
> >
> > I think we have now all agreed this is an mdio-mux, plus it is also an
> > MII mux. We should represent that in device tree. This patchset does
> > this. However, as it is now, the mux structure in DT is ignored. All
> > it does is search for the phy-is-integrated flags and goes on that.
> >
> > I made the comment that the device tree representation cannot be
> > implemented using an MDIO mux driver, because of driver loading
> > issues. However, the core of the MDIO mux code is just a library,
> > symbols exported as GPL, free for anything to use.
> >
> > What i think should happen is the mdio-mux is implemented inside the
> > MAC driver, using the mux-core as a library. The device tree structure
> > of a mix is then reflected within Linux. The mux switch callback is
> > implemented within the MAC driver. So it can reset the MAC when the
> > mux is switched. The 'phy-is-integrated' property is then no longer
> > needed.
> >
> > I would suggest a binding something like:
>
> This is looks better to me, but...
>
> > emac: ethernet@1c0b000 {
> > compatible = "allwinner,sun8i-h3-emac";
> > syscon = <&syscon>;
> > reg = <0x01c0b000 0x104>;
> > interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
> > interrupt-names = "macirq";
> > resets = <&ccu RST_BUS_EMAC>;
> > reset-names = "stmmaceth";
> > clocks = <&ccu CLK_BUS_EMAC>;
> > clock-names = "stmmaceth";
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > phy-handle = <&int_mii_phy>;
> > phy-mode = "mii";
> > allwinner,leds-active-low;
> >
> > mdio: mdio {
> > #address-cells = <1>;
> > #size-cells = <0>;
> > }
>
> Why do you need this node still?
Hi Rob
It might not be needed, depending on how it is implemented. But:
Documentation/devicetree/bindings/net/mdio-mux.txt
It is normal for an mdio bus mux to have a phandle back to the parent
mdio bus. Also, i think the stmmac driver will only instantiate the
mdio bus if there is a node for it in the device tree.
Andrew
On Thu, Aug 31, 2017 at 3:59 PM, Andrew Lunn <[email protected]> wrote:
> On Thu, Aug 31, 2017 at 03:18:03PM -0500, Rob Herring wrote:
>> On Sat, Aug 26, 2017 at 11:20:51PM +0200, Andrew Lunn wrote:
>> > Hi Corentin
>> >
>> > I think we have now all agreed this is an mdio-mux, plus it is also an
>> > MII mux. We should represent that in device tree. This patchset does
>> > this. However, as it is now, the mux structure in DT is ignored. All
>> > it does is search for the phy-is-integrated flags and goes on that.
>> >
>> > I made the comment that the device tree representation cannot be
>> > implemented using an MDIO mux driver, because of driver loading
>> > issues. However, the core of the MDIO mux code is just a library,
>> > symbols exported as GPL, free for anything to use.
>> >
>> > What i think should happen is the mdio-mux is implemented inside the
>> > MAC driver, using the mux-core as a library. The device tree structure
>> > of a mix is then reflected within Linux. The mux switch callback is
>> > implemented within the MAC driver. So it can reset the MAC when the
>> > mux is switched. The 'phy-is-integrated' property is then no longer
>> > needed.
>> >
>> > I would suggest a binding something like:
>>
>> This is looks better to me, but...
>>
>> > emac: ethernet@1c0b000 {
>> > compatible = "allwinner,sun8i-h3-emac";
>> > syscon = <&syscon>;
>> > reg = <0x01c0b000 0x104>;
>> > interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
>> > interrupt-names = "macirq";
>> > resets = <&ccu RST_BUS_EMAC>;
>> > reset-names = "stmmaceth";
>> > clocks = <&ccu CLK_BUS_EMAC>;
>> > clock-names = "stmmaceth";
>> > #address-cells = <1>;
>> > #size-cells = <0>;
>> >
>> > phy-handle = <&int_mii_phy>;
>> > phy-mode = "mii";
>> > allwinner,leds-active-low;
>> >
>> > mdio: mdio {
>> > #address-cells = <1>;
>> > #size-cells = <0>;
>> > }
>>
>> Why do you need this node still?
>
> Hi Rob
>
> It might not be needed, depending on how it is implemented. But:
>
> Documentation/devicetree/bindings/net/mdio-mux.txt
>
> It is normal for an mdio bus mux to have a phandle back to the parent
> mdio bus. Also, i think the stmmac driver will only instantiate the
> mdio bus if there is a node for it in the device tree.
You don't have a phandle to the parent mdio bus though.
I think we should allow for both case. The mux could be within the bus
hierarchy. Depends where the controls are. Another alternative someone
will try sooner or later is using the new mux control binding.
Rob