[PATCH v5 00/10] net: stmmac: dwmac-sun8i: Handle integrated PHY

[PATCH v5 00/10] net: stmmac: dwmac-sun8i: Handle integrated PHY

[Corentin Labbe]

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 and let the MAC handle all things.
Note that phy-is-integrated is still needed (even if we use a MDIO mux) since
some properties apply only on integrated PHY and we need to know the final MDIO
bus in mdio_mux_syscon_switch_fn().

Since DT bits was reverted in 4.13, this patch series include the revert of the revert.
So
- the first four patchs bring back DT/stmmac stuff that was in 4.13 (and reverted)
- fifth patch document how DT MDIO mux is implemented
- patch 6 and 7 modify DT
- patch 8, 9, 10 Modify stmmac according to the new bindings

I have let patch splited for easy review. (for seeing what's new)
But the final serie could have some patch squashed if someone want.
Like squashing patch and 2 and 5 (documentation)

Since DT worked well in 4.13, could it be targeted for 4.14 ?
If necessary I could split this serie in two:
- bring back A64/A83T (patchs 1, 2, 4, 7, 9)
- add MXIO-mux and H3 (patchs 3, 4, 5, 6, 8, 10)

Regards

Changes since v4:
- Update documentation for new bindings
- Added 4 patchs for bring back reverted stuff of 4.13
- dwmac-sun8i now handle mdio-mux
- MDIO use now compatible = "snps,dwmac-mdio";

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 (10):
  arm64: dts: allwinner: Restore EMAC changes
  dt-bindings: net: Restore sun8i dwmac binding
  arm: dts: sunxi: Restore EMAC changes
  net: stmmac: sun8i: Restore the compatibles
  dt-bindings: net: dwmac-sun8i: update documentation about integrated
    PHY
  ARM: dts: sunxi: h3/h5: represent the mdio switch used by
    sun8i-h3-emac
  arm64: dts: allwinner: add snps,dwmac-mdio compatible to emac/mdio
  net: stmmac: dwmac-sun8i: choose internal PHY via phy-is-integrated
  net: stmmac: snps,dwmac-mdio MDIOs are automatically registered
  net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs

 .../devicetree/bindings/net/dwmac-sun8i.txt        | 197 +++++++++++++++++++++
 arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts  |   9 +
 arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts    |  19 ++
 arch/arm/boot/dts/sun8i-h3-nanopi-neo.dts          |   7 +
 arch/arm/boot/dts/sun8i-h3-orangepi-2.dts          |   8 +
 arch/arm/boot/dts/sun8i-h3-orangepi-one.dts        |   8 +
 arch/arm/boot/dts/sun8i-h3-orangepi-pc-plus.dts    |   5 +
 arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts         |   8 +
 arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts       |  22 +++
 arch/arm/boot/dts/sun8i-h3-orangepi-plus2e.dts     |  16 ++
 arch/arm/boot/dts/sunxi-h3-h5.dtsi                 |  46 +++++
 .../boot/dts/allwinner/sun50i-a64-bananapi-m64.dts |  16 ++
 .../boot/dts/allwinner/sun50i-a64-pine64-plus.dts  |  15 ++
 .../arm64/boot/dts/allwinner/sun50i-a64-pine64.dts |  17 ++
 .../dts/allwinner/sun50i-a64-sopine-baseboard.dts  |  16 ++
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi      |  21 +++
 .../boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts   |  17 ++
 .../boot/dts/allwinner/sun50i-h5-orangepi-pc2.dts  |  17 ++
 .../dts/allwinner/sun50i-h5-orangepi-prime.dts     |  17 ++
 drivers/net/ethernet/stmicro/stmmac/Kconfig        |   1 +
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c  | 140 ++++++++++++---
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |   4 -
 22 files changed, 601 insertions(+), 25 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/dwmac-sun8i.txt

-- 
2.13.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 01/10] arm64: dts: allwinner: Restore EMAC changes

[Corentin Labbe]

This patch restore arm64 DT about dwmac-sun8i
This reverts commit 87e1f5e8bb4b ("arm64: dts: allwinner: Revert EMAC changes")

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 .../boot/dts/allwinner/sun50i-a64-bananapi-m64.dts   | 16 ++++++++++++++++
 .../boot/dts/allwinner/sun50i-a64-pine64-plus.dts    | 15 +++++++++++++++
 arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts  | 17 +++++++++++++++++
 .../dts/allwinner/sun50i-a64-sopine-baseboard.dts    | 16 ++++++++++++++++
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi        | 20 ++++++++++++++++++++
 .../boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts     | 17 +++++++++++++++++
 .../boot/dts/allwinner/sun50i-h5-orangepi-pc2.dts    | 17 +++++++++++++++++
 .../boot/dts/allwinner/sun50i-h5-orangepi-prime.dts  | 17 +++++++++++++++++
 8 files changed, 135 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
index d347f52e27f6..45bdbfb96126 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
@@ -51,6 +51,7 @@
 	compatible = "sinovoip,bananapi-m64", "allwinner,sun50i-a64";
 
 	aliases {
+		ethernet0 = &emac;
 		serial0 = &uart0;
 		serial1 = &uart1;
 	};
@@ -69,6 +70,14 @@
 	status = "okay";
 };
 
+&emac {
+	pinctrl-names = "default";
+	pinctrl-0 = <&rgmii_pins>;
+	phy-mode = "rgmii";
+	phy-handle = <&ext_rgmii_phy>;
+	status = "okay";
+};
+
 &i2c1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&i2c1_pins>;
@@ -79,6 +88,13 @@
 	bias-pull-up;
 };
 
+&mdio {
+	ext_rgmii_phy: ethernet-phy@1 {
+		compatible = "ethernet-phy-ieee802.3-c22";
+		reg = <1>;
+	};
+};
+
 &mmc0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&mmc0_pins>;
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dts
index f82ccf332c0f..24f1aac366d6 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dts
@@ -48,3 +48,18 @@
 
 	/* TODO: Camera, touchscreen, etc. */
 };
+
+&emac {
+	pinctrl-names = "default";
+	pinctrl-0 = <&rgmii_pins>;
+	phy-mode = "rgmii";
+	phy-handle = <&ext_rgmii_phy>;
+	status = "okay";
+};
+
+&mdio {
+	ext_rgmii_phy: ethernet-phy@1 {
+		compatible = "ethernet-phy-ieee802.3-c22";
+		reg = <1>;
+	};
+};
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
index caf8b6fbe5e3..6f209bb10a2f 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
@@ -51,6 +51,7 @@
 	compatible = "pine64,pine64", "allwinner,sun50i-a64";
 
 	aliases {
+		ethernet0 = &emac;
 		serial0 = &uart0;
 		serial1 = &uart1;
 		serial2 = &uart2;
@@ -78,6 +79,15 @@
 	status = "okay";
 };
 
+&emac {
+	pinctrl-names = "default";
+	pinctrl-0 = <&rmii_pins>;
+	phy-mode = "rmii";
+	phy-handle = <&ext_rmii_phy1>;
+	status = "okay";
+
+};
+
 &i2c1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&i2c1_pins>;
@@ -88,6 +98,13 @@
 	bias-pull-up;
 };
 
+&mdio {
+	ext_rmii_phy1: ethernet-phy@1 {
+		compatible = "ethernet-phy-ieee802.3-c22";
+		reg = <1>;
+	};
+};
+
 &mmc0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&mmc0_pins>;
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
index 17ccc12b58df..0eb2acedf8c3 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
@@ -53,6 +53,7 @@
 		     "allwinner,sun50i-a64";
 
 	aliases {
+		ethernet0 = &emac;
 		serial0 = &uart0;
 	};
 
@@ -76,6 +77,21 @@
 	status = "okay";
 };
 
+&emac {
+	pinctrl-names = "default";
+	pinctrl-0 = <&rgmii_pins>;
+	phy-mode = "rgmii";
+	phy-handle = <&ext_rgmii_phy>;
+	status = "okay";
+};
+
+&mdio {
+	ext_rgmii_phy: ethernet-phy@1 {
+		compatible = "ethernet-phy-ieee802.3-c22";
+		reg = <1>;
+	};
+};
+
 &mmc2 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&mmc2_pins>;
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 75e8d3182535..4dd9ffef0d80 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -461,6 +461,26 @@
 			#size-cells = <0>;
 		};
 
+		emac: ethernet@1c30000 {
+			compatible = "allwinner,sun50i-a64-emac";
+			syscon = <&syscon>;
+			reg = <0x01c30000 0x10000>;
+			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";
+			status = "disabled";
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			mdio: mdio {
+				#address-cells = <1>;
+				#size-cells = <0>;
+			};
+		};
+
 		gic: interrupt-controller@1c81000 {
 			compatible = "arm,gic-400";
 			reg = <0x01c81000 0x1000>,
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts b/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts
index 1c2387bd5df6..968908761194 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts
@@ -50,6 +50,7 @@
 	compatible = "friendlyarm,nanopi-neo2", "allwinner,sun50i-h5";
 
 	aliases {
+		ethernet0 = &emac;
 		serial0 = &uart0;
 	};
 
@@ -108,6 +109,22 @@
 	status = "okay";
 };
 
+&emac {
+	pinctrl-names = "default";
+	pinctrl-0 = <&emac_rgmii_pins>;
+	phy-supply = <&reg_gmac_3v3>;
+	phy-handle = <&ext_rgmii_phy>;
+	phy-mode = "rgmii";
+	status = "okay";
+};
+
+&mdio {
+	ext_rgmii_phy: ethernet-phy@7 {
+		compatible = "ethernet-phy-ieee802.3-c22";
+		reg = <7>;
+	};
+};
+
 &mmc0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&mmc0_pins_a>, <&mmc0_cd_pin>;
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-pc2.dts b/arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-pc2.dts
index 4f77c8470f6c..a8296feee884 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-pc2.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-pc2.dts
@@ -59,6 +59,7 @@
 	};
 
 	aliases {
+		ethernet0 = &emac;
 		serial0 = &uart0;
 	};
 
@@ -136,12 +137,28 @@
 	status = "okay";
 };
 
+&emac {
+	pinctrl-names = "default";
+	pinctrl-0 = <&emac_rgmii_pins>;
+	phy-supply = <&reg_gmac_3v3>;
+	phy-handle = <&ext_rgmii_phy>;
+	phy-mode = "rgmii";
+	status = "okay";
+};
+
 &ir {
 	pinctrl-names = "default";
 	pinctrl-0 = <&ir_pins_a>;
 	status = "okay";
 };
 
+&mdio {
+	ext_rgmii_phy: ethernet-phy@1 {
+		compatible = "ethernet-phy-ieee802.3-c22";
+		reg = <1>;
+	};
+};
+
 &mmc0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&mmc0_pins_a>, <&mmc0_cd_pin>;
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-prime.dts b/arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-prime.dts
index 6be06873e5af..d906b302cbcd 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-prime.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-prime.dts
@@ -54,6 +54,7 @@
 	compatible = "xunlong,orangepi-prime", "allwinner,sun50i-h5";
 
 	aliases {
+		ethernet0 = &emac;
 		serial0 = &uart0;
 	};
 
@@ -143,12 +144,28 @@
 	status = "okay";
 };
 
+&emac {
+	pinctrl-names = "default";
+	pinctrl-0 = <&emac_rgmii_pins>;
+	phy-supply = <&reg_gmac_3v3>;
+	phy-handle = <&ext_rgmii_phy>;
+	phy-mode = "rgmii";
+	status = "okay";
+};
+
 &ir {
 	pinctrl-names = "default";
 	pinctrl-0 = <&ir_pins_a>;
 	status = "okay";
 };
 
+&mdio {
+	ext_rgmii_phy: ethernet-phy@1 {
+		compatible = "ethernet-phy-ieee802.3-c22";
+		reg = <1>;
+	};
+};
+
 &mmc0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&mmc0_pins_a>, <&mmc0_cd_pin>;
-- 
2.13.5

[PATCH v5 02/10] dt-bindings: net: Restore sun8i dwmac binding

[Corentin Labbe]

This patch restore dt-bindings documentation about dwmac-sun8i
This reverts commit 8aa33ec2f481 ("dt-bindings: net: Revert sun8i dwmac binding")

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 .../devicetree/bindings/net/dwmac-sun8i.txt        | 84 ++++++++++++++++++++++
 1 file changed, 84 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/dwmac-sun8i.txt

diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
new file mode 100644
index 000000000000..725f3b187886
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
@@ -0,0 +1,84 @@
+* Allwinner sun8i GMAC ethernet controller
+
+This device is a platform glue layer for stmmac.
+Please see stmmac.txt for the other unchanged properties.
+
+Required properties:
+- compatible: should be one of the following string:
+		"allwinner,sun8i-a83t-emac"
+		"allwinner,sun8i-h3-emac"
+		"allwinner,sun8i-v3s-emac"
+		"allwinner,sun50i-a64-emac"
+- reg: address and length of the register for the device.
+- interrupts: interrupt for the device
+- interrupt-names: should be "macirq"
+- clocks: A phandle to the reference clock for this device
+- clock-names: should be "stmmaceth"
+- resets: A phandle to the reset control for this device
+- reset-names: should be "stmmaceth"
+- phy-mode: See ethernet.txt
+- phy-handle: See ethernet.txt
+- #address-cells: shall be 1
+- #size-cells: shall be 0
+- syscon: A phandle to the syscon of the SoC with one of the following
+ compatible string:
+  - allwinner,sun8i-h3-system-controller
+  - allwinner,sun8i-v3s-system-controller
+  - allwinner,sun50i-a64-system-controller
+  - allwinner,sun8i-a83t-system-controller
+
+Optional properties:
+- allwinner,tx-delay-ps: TX clock delay chain value in ps. Range value is 0-700. Default is 0)
+- allwinner,rx-delay-ps: RX clock delay chain value in ps. Range value is 0-3100. Default is 0)
+Both delay properties need to be a multiple of 100. They control the delay for
+external PHY.
+
+Optional properties for the following compatibles:
+  - "allwinner,sun8i-h3-emac",
+  - "allwinner,sun8i-v3s-emac":
+- allwinner,leds-active-low: EPHY LEDs are active low
+
+Required child node of emac:
+- mdio bus node: should be named mdio
+
+Required properties of the mdio node:
+- #address-cells: shall be 1
+- #size-cells: shall be 0
+
+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:
+  - "allwinner,sun8i-h3-emac",
+  - "allwinner,sun8i-v3s-emac":
+- clocks: a phandle to the reference clock for the EPHY
+- resets: a phandle to the reset control for the EPHY
+
+Example:
+
+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>;
+		int_mii_phy: ethernet-phy@1 {
+			reg = <1>;
+			clocks = <&ccu CLK_BUS_EPHY>;
+			resets = <&ccu RST_BUS_EPHY>;
+		};
+	};
+};
-- 
2.13.5

[PATCH v5 03/10] arm: dts: sunxi: Restore EMAC changes

[Corentin Labbe]

This patch restore arm DT about dwmac-sun8i
This reverts commit fe45174b72ae ("arm: dts: sunxi: Revert EMAC changes")

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts |  9 ++++++++
 arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts   | 19 +++++++++++++++++
 arch/arm/boot/dts/sun8i-h3-nanopi-neo.dts         |  7 ++++++
 arch/arm/boot/dts/sun8i-h3-orangepi-2.dts         |  8 +++++++
 arch/arm/boot/dts/sun8i-h3-orangepi-one.dts       |  8 +++++++
 arch/arm/boot/dts/sun8i-h3-orangepi-pc-plus.dts   |  5 +++++
 arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts        |  8 +++++++
 arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts      | 22 +++++++++++++++++++
 arch/arm/boot/dts/sun8i-h3-orangepi-plus2e.dts    | 16 ++++++++++++++
 arch/arm/boot/dts/sunxi-h3-h5.dtsi                | 26 +++++++++++++++++++++++
 10 files changed, 128 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts b/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts
index b1502df7b509..6713d0f2b3f4 100644
--- a/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts
+++ b/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts
@@ -56,6 +56,8 @@
 
 	aliases {
 		serial0 = &uart0;
+		/* ethernet0 is the H3 emac, defined in sun8i-h3.dtsi */
+		ethernet0 = &emac;
 		ethernet1 = &xr819;
 	};
 
@@ -102,6 +104,13 @@
 	status = "okay";
 };
 
+&emac {
+	phy-handle = <&int_mii_phy>;
+	phy-mode = "mii";
+	allwinner,leds-active-low;
+	status = "okay";
+};
+
 &mmc0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&mmc0_pins_a>;
diff --git a/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts b/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts
index a337af1de322..d756ff825116 100644
--- a/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts
+++ b/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts
@@ -52,6 +52,7 @@
 	compatible = "sinovoip,bpi-m2-plus", "allwinner,sun8i-h3";
 
 	aliases {
+		ethernet0 = &emac;
 		serial0 = &uart0;
 		serial1 = &uart1;
 	};
@@ -114,12 +115,30 @@
 	status = "okay";
 };
 
+&emac {
+	pinctrl-names = "default";
+	pinctrl-0 = <&emac_rgmii_pins>;
+	phy-supply = <&reg_gmac_3v3>;
+	phy-handle = <&ext_rgmii_phy>;
+	phy-mode = "rgmii";
+
+	allwinner,leds-active-low;
+	status = "okay";
+};
+
 &ir {
 	pinctrl-names = "default";
 	pinctrl-0 = <&ir_pins_a>;
 	status = "okay";
 };
 
+&mdio {
+	ext_rgmii_phy: ethernet-phy@1 {
+		compatible = "ethernet-phy-ieee802.3-c22";
+		reg = <0>;
+	};
+};
+
 &mmc0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&mmc0_pins_a>, <&mmc0_cd_pin>;
diff --git a/arch/arm/boot/dts/sun8i-h3-nanopi-neo.dts b/arch/arm/boot/dts/sun8i-h3-nanopi-neo.dts
index 8d2cc6e9a03f..78f6c24952dd 100644
--- a/arch/arm/boot/dts/sun8i-h3-nanopi-neo.dts
+++ b/arch/arm/boot/dts/sun8i-h3-nanopi-neo.dts
@@ -46,3 +46,10 @@
 	model = "FriendlyARM NanoPi NEO";
 	compatible = "friendlyarm,nanopi-neo", "allwinner,sun8i-h3";
 };
+
+&emac {
+	phy-handle = <&int_mii_phy>;
+	phy-mode = "mii";
+	allwinner,leds-active-low;
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts
index 8ff71b1bb45b..17cdeae19c6f 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts
@@ -54,6 +54,7 @@
 	aliases {
 		serial0 = &uart0;
 		/* ethernet0 is the H3 emac, defined in sun8i-h3.dtsi */
+		ethernet0 = &emac;
 		ethernet1 = &rtl8189;
 	};
 
@@ -117,6 +118,13 @@
 	status = "okay";
 };
 
+&emac {
+	phy-handle = <&int_mii_phy>;
+	phy-mode = "mii";
+	allwinner,leds-active-low;
+	status = "okay";
+};
+
 &ir {
 	pinctrl-names = "default";
 	pinctrl-0 = <&ir_pins_a>;
diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-one.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-one.dts
index 5fea430e0eb1..6880268e8b87 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-one.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-one.dts
@@ -52,6 +52,7 @@
 	compatible = "xunlong,orangepi-one", "allwinner,sun8i-h3";
 
 	aliases {
+		ethernet0 = &emac;
 		serial0 = &uart0;
 	};
 
@@ -97,6 +98,13 @@
 	status = "okay";
 };
 
+&emac {
+	phy-handle = <&int_mii_phy>;
+	phy-mode = "mii";
+	allwinner,leds-active-low;
+	status = "okay";
+};
+
 &mmc0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&mmc0_pins_a>, <&mmc0_cd_pin>;
diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-pc-plus.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-pc-plus.dts
index 8b93f5c781a7..a10281b455f5 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-pc-plus.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-pc-plus.dts
@@ -53,6 +53,11 @@
 	};
 };
 
+&emac {
+	/* LEDs changed to active high on the plus */
+	/delete-property/ allwinner,leds-active-low;
+};
+
 &mmc1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&mmc1_pins_a>;
diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
index c88518b3f538..f5f0f15a2088 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
@@ -52,6 +52,7 @@
 	compatible = "xunlong,orangepi-pc", "allwinner,sun8i-h3";
 
 	aliases {
+		ethernet0 = &emac;
 		serial0 = &uart0;
 	};
 
@@ -117,6 +118,13 @@
 	status = "okay";
 };
 
+&emac {
+	phy-handle = <&int_mii_phy>;
+	phy-mode = "mii";
+	allwinner,leds-active-low;
+	status = "okay";
+};
+
 &ir {
 	pinctrl-names = "default";
 	pinctrl-0 = <&ir_pins_a>;
diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts
index 828ae7a526d9..331ed683ac62 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts
@@ -47,6 +47,10 @@
 	model = "Xunlong Orange Pi Plus / Plus 2";
 	compatible = "xunlong,orangepi-plus", "allwinner,sun8i-h3";
 
+	aliases {
+		ethernet0 = &emac;
+	};
+
 	reg_gmac_3v3: gmac-3v3 {
 		compatible = "regulator-fixed";
 		regulator-name = "gmac-3v3";
@@ -74,6 +78,24 @@
 	status = "okay";
 };
 
+&emac {
+	pinctrl-names = "default";
+	pinctrl-0 = <&emac_rgmii_pins>;
+	phy-supply = <&reg_gmac_3v3>;
+	phy-handle = <&ext_rgmii_phy>;
+	phy-mode = "rgmii";
+
+	allwinner,leds-active-low;
+	status = "okay";
+};
+
+&mdio {
+	ext_rgmii_phy: ethernet-phy@1 {
+		compatible = "ethernet-phy-ieee802.3-c22";
+		reg = <0>;
+	};
+};
+
 &mmc2 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&mmc2_8bit_pins>;
diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-plus2e.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-plus2e.dts
index 97920b12a944..80026f3caafc 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-plus2e.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-plus2e.dts
@@ -61,3 +61,19 @@
 		gpio = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */
 	};
 };
+
+&emac {
+	pinctrl-names = "default";
+	pinctrl-0 = <&emac_rgmii_pins>;
+	phy-supply = <&reg_gmac_3v3>;
+	phy-handle = <&ext_rgmii_phy>;
+	phy-mode = "rgmii";
+	status = "okay";
+};
+
+&mdio {
+	ext_rgmii_phy: ethernet-phy@1 {
+		compatible = "ethernet-phy-ieee802.3-c22";
+		reg = <1>;
+	};
+};
diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
index 34fe2b8b3f33..4b599b5d26f6 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
@@ -403,6 +403,32 @@
 			clocks = <&osc24M>;
 		};
 
+		emac: ethernet@1c30000 {
+			compatible = "allwinner,sun8i-h3-emac";
+			syscon = <&syscon>;
+			reg = <0x01c30000 0x10000>;
+			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>;
+			status = "disabled";
+
+			mdio: 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>;
+				};
+			};
+		};
+
 		spi0: spi@01c68000 {
 			compatible = "allwinner,sun8i-h3-spi";
 			reg = <0x01c68000 0x1000>;
-- 
2.13.5

[PATCH v5 04/10] net: stmmac: sun8i: Restore the compatibles

[Corentin Labbe]

This patch restore compatibles about dwmac-sun8i
This reverts commit ad4540cc5aa3 ("net: stmmac: sun8i: Remove the compatibles")

Signed-off-by: Corentin Labbe <clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 39c2122a4f26..fffd6d5fc907 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -979,6 +979,14 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
 }
 
 static const struct of_device_id sun8i_dwmac_match[] = {
+	{ .compatible = "allwinner,sun8i-h3-emac",
+		.data = &emac_variant_h3 },
+	{ .compatible = "allwinner,sun8i-v3s-emac",
+		.data = &emac_variant_v3s },
+	{ .compatible = "allwinner,sun8i-a83t-emac",
+		.data = &emac_variant_a83t },
+	{ .compatible = "allwinner,sun50i-a64-emac",
+		.data = &emac_variant_a64 },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, sun8i_dwmac_match);
-- 
2.13.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 05/10] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY

[Corentin Labbe]

This patch add documentation about the MDIO switch used on sun8i-h3-emac
for integrated PHY.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 .../devicetree/bindings/net/dwmac-sun8i.txt        | 127 +++++++++++++++++++--
 1 file changed, 120 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
index 725f3b187886..3fa0e54825ea 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,28 @@ 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 called "mdio-mux":
+  - "allwinner,sun8i-h3-emac"
+  - "allwinner,sun8i-v3s-emac":
+Required properties for the mdio-mux node:
+  - compatible = "mdio-mux"
+  - one child mdio for the integrated mdio
+  - one child mdio for the external mdio if present (V3s have none)
+Required properties for the mdio-mux children node:
+  - reg: 0 for internal MDIO bus, 1 for external MDIO bus
+
+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 +86,112 @@ emac: ethernet@1c0b000 {
 	phy-handle = <&int_mii_phy>;
 	phy-mode = "mii";
 	allwinner,leds-active-low;
+
+	mdio0: mdio {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "snps,dwmac-mdio";
+	};
+
+	mdio-mux {
+		compatible = "mdio-mux";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		int_mdio: mdio@1 {
+			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>;
+				phy-is-integrated
+			};
+		};
+		ext_mdio: mdio@0 {
+			reg = <1>;
+			#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";
+	};
+
+	mdio-mux {
+		compatible = "mdio-mux";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		int_mdio: mdio@1 {
+			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>;
+				phy-is-integrated
+			};
+		};
+		ext_mdio: mdio@0 {
+			reg = <1>;
+			#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 {
+		compatible = "snps,dwmac-mdio";
 		#address-cells = <1>;
 		#size-cells = <0>;
-		int_mii_phy: ethernet-phy@1 {
+		ext_rgmii_phy: ethernet-phy@1 {
 			reg = <1>;
-			clocks = <&ccu CLK_BUS_EPHY>;
-			resets = <&ccu RST_BUS_EPHY>;
 		};
 	};
 };
-- 
2.13.5

[PATCH v5 06/10] ARM: dts: sunxi: h3/h5: represent the mdio switch used by sun8i-h3-emac

[Corentin Labbe]

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 | 30 +++++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
index 4b599b5d26f6..e137377b312d 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
@@ -417,14 +417,34 @@
 			#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";
+				compatible = "snps,dwmac-mdio";
+			};
+
+			mdio-mux {
+				compatible = "mdio-mux";
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				/* Only one MDIO is usable at the time */
+				internal_mdio: mdio@1 {
+					reg = <0>;
+					#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 = <1>;
-					clocks = <&ccu CLK_BUS_EPHY>;
-					resets = <&ccu RST_BUS_EPHY>;
+					#address-cells = <1>;
+					#size-cells = <0>;
 				};
 			};
 		};
-- 
2.13.5

[PATCH v5 07/10] arm64: dts: allwinner: add snps,dwmac-mdio compatible to emac/mdio

[Corentin Labbe]

stmmac bindings docs said that its mdio node must have
compatible = "snps,dwmac-mdio";
Since dwmac-sun8i does not have any good reasons to not doing it, all
their MDIO node must have it.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 4dd9ffef0d80..5dceebd81f09 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -476,6 +476,7 @@
 			#size-cells = <0>;
 
 			mdio: mdio {
+				compatible = "snps,dwmac-mdio";
 				#address-cells = <1>;
 				#size-cells = <0>;
 			};
-- 
2.13.5

[PATCH v5 08/10] net: stmmac: dwmac-sun8i: choose internal PHY via phy-is-integrated

[Corentin Labbe]

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 <clabbe.montjoie@gmail.com>
Acked-by: Chen-Yu Tsai <wens@csie.org>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 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 fffd6d5fc907..672553b652bd 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;
@@ -932,7 +932,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

[PATCH v5 09/10] net: stmmac: snps,dwmac-mdio MDIOs are automatically registered

[Corentin Labbe]

stmmac bindings docs said that its mdio node must have
compatible = "snps,dwmac-mdio";
Since dwmac-sun8i does not have any good reasons to not doing it, all
their MDIO node must have it.

Since these compatible is automatically registered, dwmac-sun8i compatible
does not need to be in need_mdio_ids.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index a366b3747eeb..3de5501e34fe 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -311,10 +311,6 @@ static int stmmac_dt_phy(struct plat_stmmacenet_data *plat,
 	bool mdio = true;
 	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" },
 	};
 
 	/* If phy-handle property is passed from DT, use it as the PHY */
-- 
2.13.5

[PATCH v5 10/10] net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs

[Corentin Labbe]

The Allwinner H3 SoC have two distinct MDIO bus, only one could be
active at the same time.
The selection of the active MDIO bus are done via some bits in the EMAC
register of the system controller.

This patch implement this MDIO switch via a custom MDIO-mux.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/Kconfig       |   1 +
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 116 +++++++++++++++++++---
 2 files changed, 104 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index 97035766c291..e28c0d2c58e9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -159,6 +159,7 @@ config DWMAC_SUN8I
 	tristate "Allwinner sun8i GMAC support"
 	default ARCH_SUNXI
 	depends on OF && (ARCH_SUNXI || COMPILE_TEST)
+	select MDIO_BUS_MUX
 	---help---
 	  Support for Allwinner H3 A83T A64 EMAC ethernet controllers.
 
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 672553b652bd..ddd5695886ac 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -17,6 +17,7 @@
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/iopoll.h>
+#include <linux/mdio-mux.h>
 #include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
@@ -71,6 +72,7 @@ struct sunxi_priv_data {
 	const struct emac_variant *variant;
 	struct regmap *regmap;
 	bool use_internal_phy;
+	void *mux_handle;
 };
 
 static const struct emac_variant emac_variant_h3 = {
@@ -195,6 +197,9 @@ static const struct emac_variant emac_variant_a64 = {
 #define H3_EPHY_LED_POL		BIT(17) /* 1: active low, 0: active high */
 #define H3_EPHY_SHUTDOWN	BIT(16) /* 1: shutdown, 0: power up */
 #define H3_EPHY_SELECT		BIT(15) /* 1: internal PHY, 0: external PHY */
+#define H3_EPHY_MUX_MASK	(H3_EPHY_SHUTDOWN | H3_EPHY_SELECT)
+#define DWMAC_sUN8I_MDIO_MUX_INTERNAL_ID	0
+#define DWMAC_sUN8I_MDIO_MUX_EXTERNAL_ID	1
 
 /* H3/A64 specific bits */
 #define SYSCON_RMII_EN		BIT(13) /* 1: enable RMII (overrides EPIT) */
@@ -634,6 +639,76 @@ static int sun8i_dwmac_reset(struct stmmac_priv *priv)
 	return 0;
 }
 
+/* MDIO multiplexing switch function
+ * This function is called by the mdio-mux layer when it thinks the mdio bus
+ * multiplexer needs to switch.
+ * 'current_child' is the current value of the mux register
+ * 'desired_child' is the value of the 'reg' property of the target child MDIO
+ * node.
+ * The first time this function is called, current_child == -1.
+ * If current_child == desired_child, then the mux is already set to the
+ * correct bus.
+ *
+ * Note that we do not use reg/mask like mdio-mux-mmioreg because we need to
+ * know easily which bus is used (reset must be done only for desired bus).
+ */
+static int mdio_mux_syscon_switch_fn(int current_child, int desired_child,
+				     void *data)
+{
+	struct stmmac_priv *priv = data;
+	struct sunxi_priv_data *gmac = priv->plat->bsp_priv;
+	u32 reg, val;
+	int ret = 0;
+	bool need_reset = false;
+
+	if (current_child ^ desired_child) {
+		regmap_read(gmac->regmap, SYSCON_EMAC_REG, &reg);
+		switch (desired_child) {
+		case DWMAC_sUN8I_MDIO_MUX_INTERNAL_ID:
+			dev_info(priv->device, "Switch mux to internal PHY");
+			val = (reg & ~H3_EPHY_MUX_MASK) | H3_EPHY_SELECT;
+			if (gmac->use_internal_phy)
+				need_reset = true;
+			break;
+		case DWMAC_sUN8I_MDIO_MUX_EXTERNAL_ID:
+			dev_info(priv->device, "Switch mux to external PHY");
+			val = (reg & ~H3_EPHY_MUX_MASK) | H3_EPHY_SHUTDOWN;
+			if (!gmac->use_internal_phy)
+				need_reset = true;
+			break;
+		default:
+			dev_err(priv->device, "Invalid child id %x\n", desired_child);
+			return -EINVAL;
+		}
+		regmap_write(gmac->regmap, SYSCON_EMAC_REG, val);
+		/* After changing syscon value, the MAC need reset or it will use
+		 * the last value (and so the last PHY set).
+		 * Reset is necessary only when we reach the needed MDIO,
+		 * it timeout in other case.
+		 */
+		if (need_reset)
+			ret = sun8i_dwmac_reset(priv);
+		else
+			dev_dbg(priv->device, "skipped reset\n");
+	}
+	return ret;
+}
+
+static int sun8i_dwmac_register_mdio_mux(struct stmmac_priv *priv)
+{
+	int ret;
+	struct device_node *mdio_mux;
+	struct sunxi_priv_data *gmac = priv->plat->bsp_priv;
+
+	mdio_mux = of_get_child_by_name(priv->device->of_node, "mdio-mux");
+	if (!mdio_mux)
+		return -ENODEV;
+
+	ret = mdio_mux_init(priv->device, mdio_mux, mdio_mux_syscon_switch_fn,
+			    &gmac->mux_handle, priv, priv->mii);
+	return ret;
+}
+
 static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv)
 {
 	struct sunxi_priv_data *gmac = priv->plat->bsp_priv;
@@ -649,12 +724,7 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv)
 			 val, reg);
 
 	if (gmac->variant->soc_has_internal_phy) {
-		if (!gmac->use_internal_phy) {
-			/* switch to external PHY interface */
-			reg &= ~H3_EPHY_SELECT;
-		} else {
-			reg |= H3_EPHY_SELECT;
-			reg &= ~H3_EPHY_SHUTDOWN;
+		if (gmac->use_internal_phy) {
 			dev_dbg(priv->device, "Select internal_phy %x\n", reg);
 
 			if (of_property_read_bool(priv->plat->phy_node,
@@ -743,6 +813,8 @@ static void sun8i_dwmac_unset_syscon(struct sunxi_priv_data *gmac)
 {
 	u32 reg = gmac->variant->default_syscon_value;
 
+	if (gmac->variant->soc_has_internal_phy)
+		mdio_mux_uninit(gmac->mux_handle);
 	regmap_write(gmac->regmap, SYSCON_EMAC_REG, reg);
 }
 
@@ -801,12 +873,6 @@ static int sun8i_power_phy(struct stmmac_priv *priv)
 	if (ret)
 		return ret;
 
-	/* After changing syscon value, the MAC need reset or it will use
-	 * the last value (and so the last PHY set.
-	 */
-	ret = sun8i_dwmac_reset(priv);
-	if (ret)
-		return ret;
 	return 0;
 }
 
@@ -889,6 +955,8 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
 	struct sunxi_priv_data *gmac;
 	struct device *dev = &pdev->dev;
 	int ret;
+	struct stmmac_priv *priv;
+	struct net_device *ndev;
 
 	ret = stmmac_get_platform_resources(pdev, &stmmac_res);
 	if (ret)
@@ -973,9 +1041,31 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
 
 	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
 	if (ret)
-		sun8i_dwmac_exit(pdev, plat_dat->bsp_priv);
+		goto dwmac_exit;
+
+	ndev = dev_get_drvdata(&pdev->dev);
+	priv = netdev_priv(ndev);
+	/* The mux must be registered after parent MDIO
+	 * so after stmmac_dvr_probe()
+	 */
+	if (gmac->variant->soc_has_internal_phy) {
+		ret = sun8i_dwmac_register_mdio_mux(priv);
+		if (ret) {
+			dev_err(&pdev->dev, "Failed to register mux\n");
+			goto dwmac_mux;
+		}
+	} else {
+		ret = sun8i_dwmac_reset(priv);
+		if (ret)
+			goto dwmac_exit;
+	}
 
 	return ret;
+dwmac_mux:
+	sun8i_dwmac_unset_syscon(gmac);
+dwmac_exit:
+	sun8i_dwmac_exit(pdev, plat_dat->bsp_priv);
+return ret;
 }
 
 static const struct of_device_id sun8i_dwmac_match[] = {
-- 
2.13.5

Re: [PATCH v5 01/10] arm64: dts: allwinner: Restore EMAC changes

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1121 bytes --]

On Fri, Sep 08, 2017 at 09:11:47AM +0200, Corentin Labbe wrote:
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts b/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts
> index 1c2387bd5df6..968908761194 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts
> @@ -50,6 +50,7 @@
>  	compatible = "friendlyarm,nanopi-neo2", "allwinner,sun50i-h5";
>  
>  	aliases {
> +		ethernet0 = &emac;
>  		serial0 = &uart0;
>  	};
>  
> @@ -108,6 +109,22 @@
>  	status = "okay";
>  };
>  
> +&emac {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&emac_rgmii_pins>;
> +	phy-supply = <&reg_gmac_3v3>;
> +	phy-handle = <&ext_rgmii_phy>;
> +	phy-mode = "rgmii";
> +	status = "okay";
> +};
> +
> +&mdio {
> +	ext_rgmii_phy: ethernet-phy@7 {
> +		compatible = "ethernet-phy-ieee802.3-c22";
> +		reg = <7>;
> +	};
> +};
> +

This won't compile, you don't have that node in the H5 DTSI.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH v5 05/10] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 5736 bytes --]

On Fri, Sep 08, 2017 at 09:11:51AM +0200, Corentin Labbe wrote:
> This patch add documentation about the MDIO switch used on sun8i-h3-emac
> for integrated PHY.
> 
> Signed-off-by: Corentin Labbe <clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  .../devicetree/bindings/net/dwmac-sun8i.txt        | 127 +++++++++++++++++++--
>  1 file changed, 120 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> index 725f3b187886..3fa0e54825ea 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

labels do not end up in the final DT (while the names do) so why are
you making this change?

>  
>  Required properties of the mdio node:
>  - #address-cells: shall be 1
> @@ -48,14 +48,28 @@ 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 called "mdio-mux":
> +  - "allwinner,sun8i-h3-emac"
> +  - "allwinner,sun8i-v3s-emac":
> +Required properties for the mdio-mux node:
> +  - compatible = "mdio-mux"
> +  - one child mdio for the integrated mdio
> +  - one child mdio for the external mdio if present (V3s have none)
> +Required properties for the mdio-mux children node:
> +  - reg: 0 for internal MDIO bus, 1 for external MDIO bus
> +
> +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

I'm not sure what you mean by that, you ask that it should (so not
required?) be a child of the integrated mdio...

>  
> -Example:
> -
> +Example with integrated PHY:
>  emac: ethernet@1c0b000 {
>  	compatible = "allwinner,sun8i-h3-emac";
>  	syscon = <&syscon>;
> @@ -72,13 +86,112 @@ emac: ethernet@1c0b000 {
>  	phy-handle = <&int_mii_phy>;
>  	phy-mode = "mii";
>  	allwinner,leds-active-low;
> +
> +	mdio0: mdio {

(You don't label it mdio here, unlike what was asked before)

> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		compatible = "snps,dwmac-mdio";
> +	};

I think Rob wanted that node gone?

> +	mdio-mux {
> +		compatible = "mdio-mux";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		int_mdio: mdio@1 {
> +			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>;
> +				phy-is-integrated
> +			};
> +		};

... And in your example it's a child of the mdio mux?

> +		ext_mdio: mdio@0 {
> +			reg = <1>;
> +			#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";
> +	};
> +
> +	mdio-mux {
> +		compatible = "mdio-mux";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		int_mdio: mdio@1 {
> +			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>;
> +				phy-is-integrated
> +			};
> +		};
> +		ext_mdio: mdio@0 {
> +			reg = <1>;
> +			#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 {
> +		compatible = "snps,dwmac-mdio";
>  		#address-cells = <1>;
>  		#size-cells = <0>;
> -		int_mii_phy: ethernet-phy@1 {
> +		ext_rgmii_phy: ethernet-phy@1 {
>  			reg = <1>;
> -			clocks = <&ccu CLK_BUS_EPHY>;
> -			resets = <&ccu RST_BUS_EPHY>;
>  		};
>  	};
>  };
> -- 
> 2.13.5
> 

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH v5 01/10] arm64: dts: allwinner: Restore EMAC changes

[Corentin Labbe]

On Fri, Sep 08, 2017 at 09:19:54AM +0200, Maxime Ripard wrote:
> On Fri, Sep 08, 2017 at 09:11:47AM +0200, Corentin Labbe wrote:
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts b/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts
> > index 1c2387bd5df6..968908761194 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts
> > @@ -50,6 +50,7 @@
> >  	compatible = "friendlyarm,nanopi-neo2", "allwinner,sun50i-h5";
> >  
> >  	aliases {
> > +		ethernet0 = &emac;
> >  		serial0 = &uart0;
> >  	};
> >  
> > @@ -108,6 +109,22 @@
> >  	status = "okay";
> >  };
> >  
> > +&emac {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&emac_rgmii_pins>;
> > +	phy-supply = <&reg_gmac_3v3>;
> > +	phy-handle = <&ext_rgmii_phy>;
> > +	phy-mode = "rgmii";
> > +	status = "okay";
> > +};
> > +
> > +&mdio {
> > +	ext_rgmii_phy: ethernet-phy@7 {
> > +		compatible = "ethernet-phy-ieee802.3-c22";
> > +		reg = <7>;
> > +	};
> > +};
> > +
> 
> This won't compile, you don't have that node in the H5 DTSI.
> 

Since H5 DTSI include arm/sunxi-h3-h5.dtsi it compiles.
Furthermore, I restested just now and confirm, it compiles fine.

Regards
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 01/10] arm64: dts: allwinner: Restore EMAC changes

[Chen-Yu Tsai]

On Fri, Sep 8, 2017 at 3:36 PM, Corentin Labbe
<clabbe.montjoie@gmail.com> wrote:
> On Fri, Sep 08, 2017 at 09:19:54AM +0200, Maxime Ripard wrote:
>> On Fri, Sep 08, 2017 at 09:11:47AM +0200, Corentin Labbe wrote:
>> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts b/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts
>> > index 1c2387bd5df6..968908761194 100644
>> > --- a/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts
>> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts
>> > @@ -50,6 +50,7 @@
>> >     compatible = "friendlyarm,nanopi-neo2", "allwinner,sun50i-h5";
>> >
>> >     aliases {
>> > +           ethernet0 = &emac;
>> >             serial0 = &uart0;
>> >     };
>> >
>> > @@ -108,6 +109,22 @@
>> >     status = "okay";
>> >  };
>> >
>> > +&emac {
>> > +   pinctrl-names = "default";
>> > +   pinctrl-0 = <&emac_rgmii_pins>;
>> > +   phy-supply = <&reg_gmac_3v3>;
>> > +   phy-handle = <&ext_rgmii_phy>;
>> > +   phy-mode = "rgmii";
>> > +   status = "okay";
>> > +};
>> > +
>> > +&mdio {
>> > +   ext_rgmii_phy: ethernet-phy@7 {
>> > +           compatible = "ethernet-phy-ieee802.3-c22";
>> > +           reg = <7>;
>> > +   };
>> > +};
>> > +
>>
>> This won't compile, you don't have that node in the H5 DTSI.
>>
>
> Since H5 DTSI include arm/sunxi-h3-h5.dtsi it compiles.
> Furthermore, I restested just now and confirm, it compiles fine.

The order of your patches are wrong. No individual patch should
introduce build failures, not just the whole patch series.

ChenYu

Re: [PATCH v5 05/10] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY

[Corentin Labbe]

On Fri, Sep 08, 2017 at 09:25:38AM +0200, Maxime Ripard wrote:
> On Fri, Sep 08, 2017 at 09:11:51AM +0200, Corentin Labbe wrote:
> > This patch add documentation about the MDIO switch used on sun8i-h3-emac
> > for integrated PHY.
> > 
> > Signed-off-by: Corentin Labbe <clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > ---
> >  .../devicetree/bindings/net/dwmac-sun8i.txt        | 127 +++++++++++++++++++--
> >  1 file changed, 120 insertions(+), 7 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> > index 725f3b187886..3fa0e54825ea 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
> 
> labels do not end up in the final DT (while the names do) so why are
> you making this change?
> 

I misunderstood label/name.
Anyway, this contrainst should leave due to "snps,dwmac-mdio MDIOs are automatically registered"

> >  
> >  Required properties of the mdio node:
> >  - #address-cells: shall be 1
> > @@ -48,14 +48,28 @@ 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 called "mdio-mux":
> > +  - "allwinner,sun8i-h3-emac"
> > +  - "allwinner,sun8i-v3s-emac":
> > +Required properties for the mdio-mux node:
> > +  - compatible = "mdio-mux"
> > +  - one child mdio for the integrated mdio
> > +  - one child mdio for the external mdio if present (V3s have none)
> > +Required properties for the mdio-mux children node:
> > +  - reg: 0 for internal MDIO bus, 1 for external MDIO bus
> > +
> > +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
> 
> I'm not sure what you mean by that, you ask that it should (so not
> required?) be a child of the integrated mdio...
> 

I will change words to "must"

> >  
> > -Example:
> > -
> > +Example with integrated PHY:
> >  emac: ethernet@1c0b000 {
> >  	compatible = "allwinner,sun8i-h3-emac";
> >  	syscon = <&syscon>;
> > @@ -72,13 +86,112 @@ emac: ethernet@1c0b000 {
> >  	phy-handle = <&int_mii_phy>;
> >  	phy-mode = "mii";
> >  	allwinner,leds-active-low;
> > +
> > +	mdio0: mdio {
> 
> (You don't label it mdio here, unlike what was asked before)
> 
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +		compatible = "snps,dwmac-mdio";
> > +	};
> 
> I think Rob wanted that node gone?
> 

MDIO mux does not work without a parent MDIO, either gived by "parent-bus" or directly via mdio_mux_init() (like it is the case in dwmac-sun8i)

> > +	mdio-mux {
> > +		compatible = "mdio-mux";
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		int_mdio: mdio@1 {
> > +			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>;
> > +				phy-is-integrated
> > +			};
> > +		};
> 
> ... And in your example it's a child of the mdio mux?
> 

So I confirm, integrated PHY must be a child of integrated MDIO (that must be a child of mdio-mux).
The example is good.

> > +		ext_mdio: mdio@0 {
> > +			reg = <1>;
> > +			#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";
> > +	};
> > +
> > +	mdio-mux {
> > +		compatible = "mdio-mux";
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		int_mdio: mdio@1 {
> > +			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>;
> > +				phy-is-integrated
> > +			};
> > +		};
> > +		ext_mdio: mdio@0 {
> > +			reg = <1>;
> > +			#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 {
> > +		compatible = "snps,dwmac-mdio";
> >  		#address-cells = <1>;
> >  		#size-cells = <0>;
> > -		int_mii_phy: ethernet-phy@1 {
> > +		ext_rgmii_phy: ethernet-phy@1 {
> >  			reg = <1>;
> > -			clocks = <&ccu CLK_BUS_EPHY>;
> > -			resets = <&ccu RST_BUS_EPHY>;
> >  		};
> >  	};
> >  };
> > -- 
> > 2.13.5
> > 
> 
> -- 
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

Thanks for the review, I will fix all reported problem in next version.

Regards
Corentin Labbe
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 10/10] net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs

[Andrew Lunn]

> +#define DWMAC_sUN8I_MDIO_MUX_INTERNAL_ID	0
> +#define DWMAC_sUN8I_MDIO_MUX_EXTERNAL_ID	1
>  
>  /* H3/A64 specific bits */
>  #define SYSCON_RMII_EN		BIT(13) /* 1: enable RMII (overrides EPIT) */
> @@ -634,6 +639,76 @@ static int sun8i_dwmac_reset(struct stmmac_priv *priv)
>  	return 0;
>  }
>  
> +/* MDIO multiplexing switch function
> + * This function is called by the mdio-mux layer when it thinks the mdio bus
> + * multiplexer needs to switch.
> + * 'current_child' is the current value of the mux register
> + * 'desired_child' is the value of the 'reg' property of the target child MDIO
> + * node.
> + * The first time this function is called, current_child == -1.
> + * If current_child == desired_child, then the mux is already set to the
> + * correct bus.
> + *
> + * Note that we do not use reg/mask like mdio-mux-mmioreg because we need to
> + * know easily which bus is used (reset must be done only for desired bus).
> + */
> +static int mdio_mux_syscon_switch_fn(int current_child, int desired_child,
> +				     void *data)
> +{
> +	struct stmmac_priv *priv = data;
> +	struct sunxi_priv_data *gmac = priv->plat->bsp_priv;
> +	u32 reg, val;
> +	int ret = 0;
> +	bool need_reset = false;
> +
> +	if (current_child ^ desired_child) {
> +		regmap_read(gmac->regmap, SYSCON_EMAC_REG, &reg);
> +		switch (desired_child) {
> +		case DWMAC_sUN8I_MDIO_MUX_INTERNAL_ID:
> +			dev_info(priv->device, "Switch mux to internal PHY");
> +			val = (reg & ~H3_EPHY_MUX_MASK) | H3_EPHY_SELECT;
> +			if (gmac->use_internal_phy)
> +				need_reset = true;
> +			break;

This i don't get. Why do you need use_internal_phy? Isn't that
implicit from DWMAC_sUN8I_MDIO_MUX_INTERNAL_ID? Is it even possible to
use an external PHY on the internal MDIO bus?

> +		case DWMAC_sUN8I_MDIO_MUX_EXTERNAL_ID:
> +			dev_info(priv->device, "Switch mux to external PHY");
> +			val = (reg & ~H3_EPHY_MUX_MASK) | H3_EPHY_SHUTDOWN;
> +			if (!gmac->use_internal_phy)
> +				need_reset = true;
> +			break;

And is it possible to use the internal PHY on the external bus?

    Andrew

Re: [PATCH v5 10/10] net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs

[Corentin Labbe]

On Fri, Sep 08, 2017 at 03:05:20PM +0200, Andrew Lunn wrote:
> > +#define DWMAC_sUN8I_MDIO_MUX_INTERNAL_ID	0
> > +#define DWMAC_sUN8I_MDIO_MUX_EXTERNAL_ID	1
> >  
> >  /* H3/A64 specific bits */
> >  #define SYSCON_RMII_EN		BIT(13) /* 1: enable RMII (overrides EPIT) */
> > @@ -634,6 +639,76 @@ static int sun8i_dwmac_reset(struct stmmac_priv *priv)
> >  	return 0;
> >  }
> >  
> > +/* MDIO multiplexing switch function
> > + * This function is called by the mdio-mux layer when it thinks the mdio bus
> > + * multiplexer needs to switch.
> > + * 'current_child' is the current value of the mux register
> > + * 'desired_child' is the value of the 'reg' property of the target child MDIO
> > + * node.
> > + * The first time this function is called, current_child == -1.
> > + * If current_child == desired_child, then the mux is already set to the
> > + * correct bus.
> > + *
> > + * Note that we do not use reg/mask like mdio-mux-mmioreg because we need to
> > + * know easily which bus is used (reset must be done only for desired bus).
> > + */
> > +static int mdio_mux_syscon_switch_fn(int current_child, int desired_child,
> > +				     void *data)
> > +{
> > +	struct stmmac_priv *priv = data;
> > +	struct sunxi_priv_data *gmac = priv->plat->bsp_priv;
> > +	u32 reg, val;
> > +	int ret = 0;
> > +	bool need_reset = false;
> > +
> > +	if (current_child ^ desired_child) {
> > +		regmap_read(gmac->regmap, SYSCON_EMAC_REG, &reg);
> > +		switch (desired_child) {
> > +		case DWMAC_sUN8I_MDIO_MUX_INTERNAL_ID:
> > +			dev_info(priv->device, "Switch mux to internal PHY");
> > +			val = (reg & ~H3_EPHY_MUX_MASK) | H3_EPHY_SELECT;
> > +			if (gmac->use_internal_phy)
> > +				need_reset = true;
> > +			break;
> 
> This i don't get. Why do you need use_internal_phy? Isn't that
> implicit from DWMAC_sUN8I_MDIO_MUX_INTERNAL_ID? Is it even possible to
> use an external PHY on the internal MDIO bus?
> 

On my H3 box with external PHY, the MDIO mux library first select (for scan ?) the internal MDIO.
Without use_internal_phy usage, this board will launch a reset to use the internal MDIO... and this reset timeout/fail.
After the MDIO mux select the external MDIO.

> > +		case DWMAC_sUN8I_MDIO_MUX_EXTERNAL_ID:
> > +			dev_info(priv->device, "Switch mux to external PHY");
> > +			val = (reg & ~H3_EPHY_MUX_MASK) | H3_EPHY_SHUTDOWN;
> > +			if (!gmac->use_internal_phy)
> > +				need_reset = true;
> > +			break;
> 
> And is it possible to use the internal PHY on the external bus?
> 

I need to check that.

Regards

Re: [PATCH v5 10/10] net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs

[Andrew Lunn]

> > > +static int mdio_mux_syscon_switch_fn(int current_child, int desired_child,
> > > +				     void *data)
> > > +{
> > > +	struct stmmac_priv *priv = data;
> > > +	struct sunxi_priv_data *gmac = priv->plat->bsp_priv;
> > > +	u32 reg, val;
> > > +	int ret = 0;
> > > +	bool need_reset = false;
> > > +
> > > +	if (current_child ^ desired_child) {
> > > +		regmap_read(gmac->regmap, SYSCON_EMAC_REG, &reg);
> > > +		switch (desired_child) {
> > > +		case DWMAC_sUN8I_MDIO_MUX_INTERNAL_ID:
> > > +			dev_info(priv->device, "Switch mux to internal PHY");
> > > +			val = (reg & ~H3_EPHY_MUX_MASK) | H3_EPHY_SELECT;
> > > +			if (gmac->use_internal_phy)
> > > +				need_reset = true;
> > > +			break;
> > 
> > This i don't get. Why do you need use_internal_phy? Isn't that
> > implicit from DWMAC_sUN8I_MDIO_MUX_INTERNAL_ID? Is it even possible to
> > use an external PHY on the internal MDIO bus?
> > 
> 
> On my H3 box with external PHY, the MDIO mux library first select (for scan ?) the internal MDIO.
> Without use_internal_phy usage, this board will launch a reset to use the internal MDIO... and this reset timeout/fail.

Do you know why the reset times out/fails?

   Andrew

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 10/10] net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs

[Corentin Labbe]

On Fri, Sep 08, 2017 at 04:00:20PM +0200, Andrew Lunn wrote:
> > > > +static int mdio_mux_syscon_switch_fn(int current_child, int desired_child,
> > > > +				     void *data)
> > > > +{
> > > > +	struct stmmac_priv *priv = data;
> > > > +	struct sunxi_priv_data *gmac = priv->plat->bsp_priv;
> > > > +	u32 reg, val;
> > > > +	int ret = 0;
> > > > +	bool need_reset = false;
> > > > +
> > > > +	if (current_child ^ desired_child) {
> > > > +		regmap_read(gmac->regmap, SYSCON_EMAC_REG, &reg);
> > > > +		switch (desired_child) {
> > > > +		case DWMAC_sUN8I_MDIO_MUX_INTERNAL_ID:
> > > > +			dev_info(priv->device, "Switch mux to internal PHY");
> > > > +			val = (reg & ~H3_EPHY_MUX_MASK) | H3_EPHY_SELECT;
> > > > +			if (gmac->use_internal_phy)
> > > > +				need_reset = true;
> > > > +			break;
> > > 
> > > This i don't get. Why do you need use_internal_phy? Isn't that
> > > implicit from DWMAC_sUN8I_MDIO_MUX_INTERNAL_ID? Is it even possible to
> > > use an external PHY on the internal MDIO bus?
> > > 
> > 
> > On my H3 box with external PHY, the MDIO mux library first select (for scan ?) the internal MDIO.
> > Without use_internal_phy usage, this board will launch a reset to use the internal MDIO... and this reset timeout/fail.
> 
> Do you know why the reset times out/fails?
> 

Because there are nothing connected to it.
I got also reset timeout on integrated MDIO when the integrated PHY is not powered.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 10/10] net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs

[Andrew Lunn]

> > Do you know why the reset times out/fails?
> > 
> 
> Because there are nothing connected to it.

That should not be an issue. A read should just return 0xffff.  And it
should return 0xffff fast. The timing of the MDIO protocol is fixed. A
read or a write takes a fixed number of cycles, independent of if
there is a device there or not. The bus data line has a pullup, so if
you try to access a missing device, you automatically read 0xffff.

       Andrew

Re: [PATCH v5 10/10] net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs

[Corentin Labbe]

On Fri, Sep 08, 2017 at 04:17:36PM +0200, Andrew Lunn wrote:
> > > Do you know why the reset times out/fails?
> > > 
> > 
> > Because there are nothing connected to it.
> 
> That should not be an issue. A read should just return 0xffff.  And it
> should return 0xffff fast. The timing of the MDIO protocol is fixed. A
> read or a write takes a fixed number of cycles, independent of if
> there is a device there or not. The bus data line has a pullup, so if
> you try to access a missing device, you automatically read 0xffff.
> 

Perhaps, but the reality is that with nothing connected to it, the reset of the MAC timeout.
Certainly, the MAC does not support finding no PHY.

So, to prevent an error message, and a "freeze" of the net process, the need_reset trick is necessary.

Regards
Corentin Labbe

Re: [PATCH v5 01/10] arm64: dts: allwinner: Restore EMAC changes

[Corentin Labbe]

On Fri, Sep 08, 2017 at 03:39:04PM +0800, Chen-Yu Tsai wrote:
> On Fri, Sep 8, 2017 at 3:36 PM, Corentin Labbe
> <clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > On Fri, Sep 08, 2017 at 09:19:54AM +0200, Maxime Ripard wrote:
> >> On Fri, Sep 08, 2017 at 09:11:47AM +0200, Corentin Labbe wrote:
> >> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts b/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts
> >> > index 1c2387bd5df6..968908761194 100644
> >> > --- a/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts
> >> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts
> >> > @@ -50,6 +50,7 @@
> >> >     compatible = "friendlyarm,nanopi-neo2", "allwinner,sun50i-h5";
> >> >
> >> >     aliases {
> >> > +           ethernet0 = &emac;
> >> >             serial0 = &uart0;
> >> >     };
> >> >
> >> > @@ -108,6 +109,22 @@
> >> >     status = "okay";
> >> >  };
> >> >
> >> > +&emac {
> >> > +   pinctrl-names = "default";
> >> > +   pinctrl-0 = <&emac_rgmii_pins>;
> >> > +   phy-supply = <&reg_gmac_3v3>;
> >> > +   phy-handle = <&ext_rgmii_phy>;
> >> > +   phy-mode = "rgmii";
> >> > +   status = "okay";
> >> > +};
> >> > +
> >> > +&mdio {
> >> > +   ext_rgmii_phy: ethernet-phy@7 {
> >> > +           compatible = "ethernet-phy-ieee802.3-c22";
> >> > +           reg = <7>;
> >> > +   };
> >> > +};
> >> > +
> >>
> >> This won't compile, you don't have that node in the H5 DTSI.
> >>
> >
> > Since H5 DTSI include arm/sunxi-h3-h5.dtsi it compiles.
> > Furthermore, I restested just now and confirm, it compiles fine.
> 
> The order of your patches are wrong. No individual patch should
> introduce build failures, not just the whole patch series.
> 

Yes, I just miss-understood the reason of build failure.
I will fix the order in the next serie.

Thanks
Corentin Labbe
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 10/10] net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs

[Andrew Lunn]

On Fri, Sep 08, 2017 at 04:28:25PM +0200, Corentin Labbe wrote:
> On Fri, Sep 08, 2017 at 04:17:36PM +0200, Andrew Lunn wrote:
> > > > Do you know why the reset times out/fails?
> > > > 
> > > 
> > > Because there are nothing connected to it.
> > 
> > That should not be an issue. A read should just return 0xffff.  And it
> > should return 0xffff fast. The timing of the MDIO protocol is fixed. A
> > read or a write takes a fixed number of cycles, independent of if
> > there is a device there or not. The bus data line has a pullup, so if
> > you try to access a missing device, you automatically read 0xffff.
> > 
> 
> Perhaps, but the reality is that with nothing connected to it, the reset of the MAC timeout.
> Certainly, the MAC does not support finding no PHY.

Are you sure this is not because of the clock and reset?

+                               #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>;

The way you describe it here, the clock and reset are for the PHY. But
maybe it is actually for the bus? I can understand a bus timing out if
it has no clock, or it is held in reset. Try enabling the clock and
reset when the internal bus is selected, not when the PHY on the bus
is selected.

	Andrew

Re: [PATCH v5 10/10] net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs

[Corentin Labbe]

On Mon, Sep 11, 2017 at 06:11:24PM +0200, Andrew Lunn wrote:
> On Fri, Sep 08, 2017 at 04:28:25PM +0200, Corentin Labbe wrote:
> > On Fri, Sep 08, 2017 at 04:17:36PM +0200, Andrew Lunn wrote:
> > > > > Do you know why the reset times out/fails?
> > > > > 
> > > > 
> > > > Because there are nothing connected to it.
> > > 
> > > That should not be an issue. A read should just return 0xffff.  And it
> > > should return 0xffff fast. The timing of the MDIO protocol is fixed. A
> > > read or a write takes a fixed number of cycles, independent of if
> > > there is a device there or not. The bus data line has a pullup, so if
> > > you try to access a missing device, you automatically read 0xffff.
> > > 
> > 
> > Perhaps, but the reality is that with nothing connected to it, the reset of the MAC timeout.
> > Certainly, the MAC does not support finding no PHY.
> 
> Are you sure this is not because of the clock and reset?
> 
> +                               #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>;
> 
> The way you describe it here, the clock and reset are for the PHY. But
> maybe it is actually for the bus? I can understand a bus timing out if
> it has no clock, or it is held in reset. Try enabling the clock and
> reset when the internal bus is selected, not when the PHY on the bus
> is selected.
> 

Even with CLK_BUS_EPHY/RST_BUS_EPHY enabled, the MAC reset timeout.
So no the CLK/RST are really for the PHY.

Regards

PS: patch and result with "integrated CLK/RST always on"
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -659,7 +659,7 @@ static int mdio_mux_syscon_switch_fn(int current_child, int desired_child,
        struct sunxi_priv_data *gmac = priv->plat->bsp_priv;
        u32 reg, val;
        int ret = 0;
-       bool need_reset = false;
+       bool need_reset = true;
 
        if (current_child ^ desired_child) {
                regmap_read(gmac->regmap, SYSCON_EMAC_REG, &reg);
@@ -824,7 +824,7 @@ static int sun8i_dwmac_power_internal_phy(struct stmmac_priv *priv)
        int ret;
 
        if (!gmac->use_internal_phy)
-               return 0;
+               dev_info(priv->device, "IPHY BYPASS\n");
 
        ret = clk_prepare_enable(gmac->ephy_clk);
        if (ret) {

[   18.057162] dwmac-sun8i 1c30000.ethernet: Will use external PHY
[   18.183789] dwmac-sun8i 1c30000.ethernet: IPHY BYPASS
[   18.184136] dwmac-sun8i 1c30000.ethernet: Chain mode enabled
[   18.184158] dwmac-sun8i 1c30000.ethernet: No HW DMA feature register supported
[   18.184175] dwmac-sun8i 1c30000.ethernet: Normal descriptors
[   18.184192] dwmac-sun8i 1c30000.ethernet: RX Checksum Offload Engine supported
[   18.184214] dwmac-sun8i 1c30000.ethernet: COE Type 2
[   18.184231] dwmac-sun8i 1c30000.ethernet: TX Checksum insertion supported
[   18.185491] libphy: stmmac: probed
[   18.188481] libphy: mdio_mux: probed
[   18.188831] dwmac-sun8i 1c30000.ethernet: Switch mux to internal PHY
[   18.288981] dwmac-sun8i 1c30000.ethernet: EMAC reset timeout
[   18.289559] libphy: mdio_mux: probed
[   18.289629] dwmac-sun8i 1c30000.ethernet: Switch mux to external PHY
[   20.578316] EXT4-fs (mmcblk0p1): re-mounted. Opts: (null)
[   31.240650] RTL8211E Gigabit Ethernet 0.1:00: attached PHY driver [RTL8211E Gigabit Ethernet] (mii_bus:phy_addr=0.1:00, irq=POLL)

Re: [PATCH v5 10/10] net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs

[Andrew Lunn]

> Even with CLK_BUS_EPHY/RST_BUS_EPHY enabled, the MAC reset timeout.
> So no the CLK/RST are really for the PHY.

Thanks for trying that.

You said it was probably during scanning of the bus it times out. What
address is causing the timeout? 0 or 1? If the internal bus can only
have one PHY on it, maybe we need to set bus->phy_mask to 0x1?

   Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 10/10] net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs

[Corentin Labbe]

On Mon, Sep 11, 2017 at 10:19:20PM +0200, Andrew Lunn wrote:
> > Even with CLK_BUS_EPHY/RST_BUS_EPHY enabled, the MAC reset timeout.
> > So no the CLK/RST are really for the PHY.
> 
> Thanks for trying that.
> 
> You said it was probably during scanning of the bus it times out. What
> address is causing the timeout? 0 or 1? If the internal bus can only
> have one PHY on it, maybe we need to set bus->phy_mask to 0x1?
> 

I have added a trace in begin and end of stmmac_mdio_read()

[   18.145451] libphy: stmmac: probed
[   18.148398] libphy: mdio_mux: probed
[   18.148650] dwmac-sun8i 1c30000.ethernet: Switch mux to internal PHY
[   18.248751] dwmac-sun8i 1c30000.ethernet: EMAC reset timeout
[   18.249297] libphy: mdio_mux: probed
[   18.249362] dwmac-sun8i 1c30000.ethernet: Switch mux to external PHY
[   18.249391] stmmac_mdio_read 0 2
[   18.249598] stmmac_mdio_read 0 2 1c
[   18.249623] stmmac_mdio_read 0 3
[   18.249811] stmmac_mdio_read 0 3 c915
[   20.737271] EXT4-fs (mmcblk0p1): re-mounted. Opts: (null)
[   31.294868] stmmac_mdio_read 0 0
[   31.295311] stmmac_mdio_read 0 0 1140

It seems that the timeout is unrelated to MDIO bus.

Regards

Re: [PATCH v5 02/10] dt-bindings: net: Restore sun8i dwmac binding

[Rob Herring]

On Fri, Sep 08, 2017 at 09:11:48AM +0200, Corentin Labbe wrote:
> This patch restore dt-bindings documentation about dwmac-sun8i
> This reverts commit 8aa33ec2f481 ("dt-bindings: net: Revert sun8i dwmac binding")

Why?

> 
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> ---
>  .../devicetree/bindings/net/dwmac-sun8i.txt        | 84 ++++++++++++++++++++++
>  1 file changed, 84 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/dwmac-sun8i.txt

Otherwise,

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH v5 05/10] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY

[Rob Herring]

On Fri, Sep 08, 2017 at 09:43:25AM +0200, Corentin Labbe wrote:
> On Fri, Sep 08, 2017 at 09:25:38AM +0200, Maxime Ripard wrote:
> > On Fri, Sep 08, 2017 at 09:11:51AM +0200, Corentin Labbe wrote:
> > > This patch add documentation about the MDIO switch used on sun8i-h3-emac
> > > for integrated PHY.
> > > 
> > > Signed-off-by: Corentin Labbe <clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > ---
> > >  .../devicetree/bindings/net/dwmac-sun8i.txt        | 127 +++++++++++++++++++--
> > >  1 file changed, 120 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> > > index 725f3b187886..3fa0e54825ea 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
> > 
> > labels do not end up in the final DT (while the names do) so why are
> > you making this change?
> > 
> 
> I misunderstood label/name.
> Anyway, this contrainst should leave due to "snps,dwmac-mdio MDIOs are automatically registered"
> 
> > >  
> > >  Required properties of the mdio node:
> > >  - #address-cells: shall be 1
> > > @@ -48,14 +48,28 @@ 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 called "mdio-mux":
> > > +  - "allwinner,sun8i-h3-emac"
> > > +  - "allwinner,sun8i-v3s-emac":
> > > +Required properties for the mdio-mux node:
> > > +  - compatible = "mdio-mux"
> > > +  - one child mdio for the integrated mdio
> > > +  - one child mdio for the external mdio if present (V3s have none)
> > > +Required properties for the mdio-mux children node:
> > > +  - reg: 0 for internal MDIO bus, 1 for external MDIO bus
> > > +
> > > +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
> > 
> > I'm not sure what you mean by that, you ask that it should (so not
> > required?) be a child of the integrated mdio...
> > 
> 
> I will change words to "must"
> 
> > >  
> > > -Example:
> > > -
> > > +Example with integrated PHY:
> > >  emac: ethernet@1c0b000 {
> > >  	compatible = "allwinner,sun8i-h3-emac";
> > >  	syscon = <&syscon>;
> > > @@ -72,13 +86,112 @@ emac: ethernet@1c0b000 {
> > >  	phy-handle = <&int_mii_phy>;
> > >  	phy-mode = "mii";
> > >  	allwinner,leds-active-low;
> > > +
> > > +	mdio0: mdio {
> > 
> > (You don't label it mdio here, unlike what was asked before)
> > 
> > > +		#address-cells = <1>;
> > > +		#size-cells = <0>;
> > > +		compatible = "snps,dwmac-mdio";
> > > +	};
> > 
> > I think Rob wanted that node gone?
> > 
> 
> MDIO mux does not work without a parent MDIO, either gived by "parent-bus" or directly via mdio_mux_init() (like it is the case in dwmac-sun8i)

Is the MDIO controller "allwinner,sun8i-h3-emac" or "snps,dwmac-mdio"? 
If the latter, then I think the node is fine, but then the mux should be 
a child node of it. IOW, the child of an MDIO controller should either 
be a mux node or slave devices.

> 
> > > +	mdio-mux {
> > > +		compatible = "mdio-mux";
> > > +		#address-cells = <1>;
> > > +		#size-cells = <0>;
> > > +
> > > +		int_mdio: mdio@1 {
> > > +			reg = <0>;

unit address of 1 and reg prop of 0 don't match. Build your dtb with 
W=2.

> > > +			#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

Missing ;

> > > +			};
> > > +		};
> > 
> > ... And in your example it's a child of the mdio mux?
> > 
> 
> So I confirm, integrated PHY must be a child of integrated MDIO (that must be a child of mdio-mux).
> The example is good.
> 
> > > +		ext_mdio: mdio@0 {
> > > +			reg = <1>;

Another unit address mismatch.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 02/10] dt-bindings: net: Restore sun8i dwmac binding

[Corentin Labbe]

On Wed, Sep 13, 2017 at 01:07:34PM -0500, Rob Herring wrote:
> On Fri, Sep 08, 2017 at 09:11:48AM +0200, Corentin Labbe wrote:
> > This patch restore dt-bindings documentation about dwmac-sun8i
> > This reverts commit 8aa33ec2f481 ("dt-bindings: net: Revert sun8i dwmac binding")
> 
> Why?
> 

I have put the reason in cover-letter and forget to put it in here also.
I fix that in next series.

Thanks

Re: [PATCH v5 05/10] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY

[Corentin Labbe]

On Wed, Sep 13, 2017 at 01:20:04PM -0500, Rob Herring wrote:
> On Fri, Sep 08, 2017 at 09:43:25AM +0200, Corentin Labbe wrote:
> > On Fri, Sep 08, 2017 at 09:25:38AM +0200, Maxime Ripard wrote:
> > > On Fri, Sep 08, 2017 at 09:11:51AM +0200, Corentin Labbe wrote:
> > > > This patch add documentation about the MDIO switch used on sun8i-h3-emac
> > > > for integrated PHY.
> > > > 
> > > > Signed-off-by: Corentin Labbe <clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > > ---
> > > >  .../devicetree/bindings/net/dwmac-sun8i.txt        | 127 +++++++++++++++++++--
> > > >  1 file changed, 120 insertions(+), 7 deletions(-)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> > > > index 725f3b187886..3fa0e54825ea 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
> > > 
> > > labels do not end up in the final DT (while the names do) so why are
> > > you making this change?
> > > 
> > 
> > I misunderstood label/name.
> > Anyway, this contrainst should leave due to "snps,dwmac-mdio MDIOs are automatically registered"
> > 
> > > >  
> > > >  Required properties of the mdio node:
> > > >  - #address-cells: shall be 1
> > > > @@ -48,14 +48,28 @@ 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 called "mdio-mux":
> > > > +  - "allwinner,sun8i-h3-emac"
> > > > +  - "allwinner,sun8i-v3s-emac":
> > > > +Required properties for the mdio-mux node:
> > > > +  - compatible = "mdio-mux"
> > > > +  - one child mdio for the integrated mdio
> > > > +  - one child mdio for the external mdio if present (V3s have none)
> > > > +Required properties for the mdio-mux children node:
> > > > +  - reg: 0 for internal MDIO bus, 1 for external MDIO bus
> > > > +
> > > > +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
> > > 
> > > I'm not sure what you mean by that, you ask that it should (so not
> > > required?) be a child of the integrated mdio...
> > > 
> > 
> > I will change words to "must"
> > 
> > > >  
> > > > -Example:
> > > > -
> > > > +Example with integrated PHY:
> > > >  emac: ethernet@1c0b000 {
> > > >  	compatible = "allwinner,sun8i-h3-emac";
> > > >  	syscon = <&syscon>;
> > > > @@ -72,13 +86,112 @@ emac: ethernet@1c0b000 {
> > > >  	phy-handle = <&int_mii_phy>;
> > > >  	phy-mode = "mii";
> > > >  	allwinner,leds-active-low;
> > > > +
> > > > +	mdio0: mdio {
> > > 
> > > (You don't label it mdio here, unlike what was asked before)
> > > 
> > > > +		#address-cells = <1>;
> > > > +		#size-cells = <0>;
> > > > +		compatible = "snps,dwmac-mdio";
> > > > +	};
> > > 
> > > I think Rob wanted that node gone?
> > > 
> > 
> > MDIO mux does not work without a parent MDIO, either gived by "parent-bus" or directly via mdio_mux_init() (like it is the case in dwmac-sun8i)
> 
> Is the MDIO controller "allwinner,sun8i-h3-emac" or "snps,dwmac-mdio"? 
> If the latter, then I think the node is fine, but then the mux should be 
> a child node of it. IOW, the child of an MDIO controller should either 
> be a mux node or slave devices.
> 

It will be snps,dwmac-mdio but putting mdio-mux as a child of it (the mdio node) give me:
[   18.175338] libphy: stmmac: probed
[   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.175482] mdio_bus stmmac-0: scan phy mdio-mux at address 2
[   18.175513] mdio_bus stmmac-0: scan phy mdio-mux at address 3
[   18.175544] mdio_bus stmmac-0: scan phy mdio-mux at address 4
[   18.175575] mdio_bus stmmac-0: scan phy mdio-mux at address 5
[   18.175607] mdio_bus stmmac-0: scan phy mdio-mux at address 6
[   18.175638] mdio_bus stmmac-0: scan phy mdio-mux at address 7
[   18.175669] mdio_bus stmmac-0: scan phy mdio-mux at address 8
[   18.175700] mdio_bus stmmac-0: scan phy mdio-mux at address 9
[   18.175731] mdio_bus stmmac-0: scan phy mdio-mux at address 10
[   18.175762] mdio_bus stmmac-0: scan phy mdio-mux at address 11
[   18.175795] mdio_bus stmmac-0: scan phy mdio-mux at address 12
[   18.175827] mdio_bus stmmac-0: scan phy mdio-mux at address 13
[   18.175858] mdio_bus stmmac-0: scan phy mdio-mux at address 14
[   18.175889] mdio_bus stmmac-0: scan phy mdio-mux at address 15
[   18.175919] mdio_bus stmmac-0: scan phy mdio-mux at address 16
[   18.175951] mdio_bus stmmac-0: scan phy mdio-mux at address 17
[   18.175982] mdio_bus stmmac-0: scan phy mdio-mux at address 18
[   18.176014] mdio_bus stmmac-0: scan phy mdio-mux at address 19
[   18.176045] mdio_bus stmmac-0: scan phy mdio-mux at address 20
[   18.176076] mdio_bus stmmac-0: scan phy mdio-mux at address 21
[   18.176107] mdio_bus stmmac-0: scan phy mdio-mux at address 22
[   18.176139] mdio_bus stmmac-0: scan phy mdio-mux at address 23
[   18.176170] mdio_bus stmmac-0: scan phy mdio-mux at address 24
[   18.176202] mdio_bus stmmac-0: scan phy mdio-mux at address 25
[   18.176233] mdio_bus stmmac-0: scan phy mdio-mux at address 26
[   18.176271] mdio_bus stmmac-0: scan phy mdio-mux at address 27
[   18.176320] mdio_bus stmmac-0: scan phy mdio-mux at address 28
[   18.176371] mdio_bus stmmac-0: scan phy mdio-mux at address 29
[   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

Adding a fake <reg> to mdio-mux remove it.
Does it is acceptable ? or perhaps patching of_mdiobus_register() to not scan node with compatible "mdio-mux".

> > 
> > > > +	mdio-mux {
> > > > +		compatible = "mdio-mux";
> > > > +		#address-cells = <1>;
> > > > +		#size-cells = <0>;
> > > > +
> > > > +		int_mdio: mdio@1 {
> > > > +			reg = <0>;
> 
> unit address of 1 and reg prop of 0 don't match. Build your dtb with 
> W=2.
> 

reg are arbitrary value (like mdio-mux-mmioreg), but in our case it is easy to fix this warning.

Thanks
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 05/10] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY

[Andrew Lunn]

> > Is the MDIO controller "allwinner,sun8i-h3-emac" or "snps,dwmac-mdio"? 
> > If the latter, then I think the node is fine, but then the mux should be 
> > a child node of it. IOW, the child of an MDIO controller should either 
> > be a mux node or slave devices.

Hi Rob

Up until now, children of an MDIO bus have been MDIO devices. Those
MDIO devices are either Ethernet PHYs, Ethernet Switches, or the
oddball devices that Broadcom iProc has, like generic PHYs.

We have never had MDIO-muxes as MDIO children. A Mux is not an MDIO
device, and does not have the properties of an MDIO device. It is not
addressable on the MDIO bus. The current MUXes are addressed via GPIOs
or MMIO.

There other similar cases. i2c-mux-gpio is not a child of an i2c bus,
nor i2c-mux-reg or gpio-mux. nxp,pca9548 is however a child of the i2c
bus, because it is an i2c device itself...

If the MDIO mux was an MDIO device, i would agree with you. Bit it is
not, so lets not make it a child.

	    Andrew

Re: [PATCH v5 05/10] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY

[Corentin Labbe]

On Thu, Sep 14, 2017 at 09:19:49PM +0200, Andrew Lunn wrote:
> > > Is the MDIO controller "allwinner,sun8i-h3-emac" or "snps,dwmac-mdio"? 
> > > If the latter, then I think the node is fine, but then the mux should be 
> > > a child node of it. IOW, the child of an MDIO controller should either 
> > > be a mux node or slave devices.
> 
> Hi Rob
> 
> Up until now, children of an MDIO bus have been MDIO devices. Those
> MDIO devices are either Ethernet PHYs, Ethernet Switches, or the
> oddball devices that Broadcom iProc has, like generic PHYs.
> 
> We have never had MDIO-muxes as MDIO children. A Mux is not an MDIO
> device, and does not have the properties of an MDIO device. It is not
> addressable on the MDIO bus. The current MUXes are addressed via GPIOs
> or MMIO.
> 
> There other similar cases. i2c-mux-gpio is not a child of an i2c bus,
> nor i2c-mux-reg or gpio-mux. nxp,pca9548 is however a child of the i2c
> bus, because it is an i2c device itself...
> 
> If the MDIO mux was an MDIO device, i would agree with you. Bit it is
> not, so lets not make it a child.
> 
> 	    Andrew

Hello Rob, could you anwser/confirm please.
I wait on this for sending the next version.

Thanks
Regards
Corentin Labbe
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 05/10] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY

[Rob Herring]

On Thu, Sep 14, 2017 at 2:19 PM, Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> wrote:
>> > Is the MDIO controller "allwinner,sun8i-h3-emac" or "snps,dwmac-mdio"?
>> > If the latter, then I think the node is fine, but then the mux should be
>> > a child node of it. IOW, the child of an MDIO controller should either
>> > be a mux node or slave devices.
>
> Hi Rob
>
> Up until now, children of an MDIO bus have been MDIO devices. Those
> MDIO devices are either Ethernet PHYs, Ethernet Switches, or the
> oddball devices that Broadcom iProc has, like generic PHYs.
>
> We have never had MDIO-muxes as MDIO children. A Mux is not an MDIO
> device, and does not have the properties of an MDIO device. It is not
> addressable on the MDIO bus. The current MUXes are addressed via GPIOs
> or MMIO.

The DT parent/child relationship defines the bus topology. We describe
MDIO buses in that way and if a mux is sitting between the controller
and the devices, then the DT hierarchy should reflect that. Now
sometimes we have 2 options for what interface has the parent/child
relationship (e.g. an I2C controlled USB hub chip), but in this case
we don't.

> There other similar cases. i2c-mux-gpio is not a child of an i2c bus,
> nor i2c-mux-reg or gpio-mux. nxp,pca9548 is however a child of the i2c
> bus, because it is an i2c device itself...

Some are i2c controlled mux devices, but some can be GPIO controlled.

>
> If the MDIO mux was an MDIO device, i would agree with you. Bit it is
> not, so lets not make it a child.
>
>             Andrew
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 05/10] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY

[Corentin Labbe]

On Tue, Sep 19, 2017 at 09:49:52PM -0500, Rob Herring wrote:
> On Thu, Sep 14, 2017 at 2:19 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> >> > Is the MDIO controller "allwinner,sun8i-h3-emac" or "snps,dwmac-mdio"?
> >> > If the latter, then I think the node is fine, but then the mux should be
> >> > a child node of it. IOW, the child of an MDIO controller should either
> >> > be a mux node or slave devices.
> >
> > Hi Rob
> >
> > Up until now, children of an MDIO bus have been MDIO devices. Those
> > MDIO devices are either Ethernet PHYs, Ethernet Switches, or the
> > oddball devices that Broadcom iProc has, like generic PHYs.
> >
> > We have never had MDIO-muxes as MDIO children. A Mux is not an MDIO
> > device, and does not have the properties of an MDIO device. It is not
> > addressable on the MDIO bus. The current MUXes are addressed via GPIOs
> > or MMIO.
> 
> The DT parent/child relationship defines the bus topology. We describe
> MDIO buses in that way and if a mux is sitting between the controller
> and the devices, then the DT hierarchy should reflect that. Now
> sometimes we have 2 options for what interface has the parent/child
> relationship (e.g. an I2C controlled USB hub chip), but in this case
> we don't.
> 

Putting mdio-mux as a child of it (the mdio node) give me:
[   18.175338] libphy: stmmac: probed
[   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.175482] mdio_bus stmmac-0: scan phy mdio-mux at address 2
[   18.175513] mdio_bus stmmac-0: scan phy mdio-mux at address 3
[   18.175544] mdio_bus stmmac-0: scan phy mdio-mux at address 4
[   18.175575] mdio_bus stmmac-0: scan phy mdio-mux at address 5
[   18.175607] mdio_bus stmmac-0: scan phy mdio-mux at address 6
[   18.175638] mdio_bus stmmac-0: scan phy mdio-mux at address 7
[   18.175669] mdio_bus stmmac-0: scan phy mdio-mux at address 8
[   18.175700] mdio_bus stmmac-0: scan phy mdio-mux at address 9
[   18.175731] mdio_bus stmmac-0: scan phy mdio-mux at address 10
[   18.175762] mdio_bus stmmac-0: scan phy mdio-mux at address 11
[   18.175795] mdio_bus stmmac-0: scan phy mdio-mux at address 12
[   18.175827] mdio_bus stmmac-0: scan phy mdio-mux at address 13
[   18.175858] mdio_bus stmmac-0: scan phy mdio-mux at address 14
[   18.175889] mdio_bus stmmac-0: scan phy mdio-mux at address 15
[   18.175919] mdio_bus stmmac-0: scan phy mdio-mux at address 16
[   18.175951] mdio_bus stmmac-0: scan phy mdio-mux at address 17
[   18.175982] mdio_bus stmmac-0: scan phy mdio-mux at address 18
[   18.176014] mdio_bus stmmac-0: scan phy mdio-mux at address 19
[   18.176045] mdio_bus stmmac-0: scan phy mdio-mux at address 20
[   18.176076] mdio_bus stmmac-0: scan phy mdio-mux at address 21
[   18.176107] mdio_bus stmmac-0: scan phy mdio-mux at address 22
[   18.176139] mdio_bus stmmac-0: scan phy mdio-mux at address 23
[   18.176170] mdio_bus stmmac-0: scan phy mdio-mux at address 24
[   18.176202] mdio_bus stmmac-0: scan phy mdio-mux at address 25
[   18.176233] mdio_bus stmmac-0: scan phy mdio-mux at address 26
[   18.176271] mdio_bus stmmac-0: scan phy mdio-mux at address 27
[   18.176320] mdio_bus stmmac-0: scan phy mdio-mux at address 28
[   18.176371] mdio_bus stmmac-0: scan phy mdio-mux at address 29
[   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

Adding a fake <reg> to mdio-mux remove it, but I found that a bit ugly.
Or perhaps patching of_mdiobus_register() to not scan node with compatible "mdio-mux".

What do you think ?

Regards