Re: [PATCH v3 4/4] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY
From: Chen-Yu Tsai
Date: Fri Aug 18 2017 - 12:57:42 EST
On Fri, Aug 18, 2017 at 8:21 PM, Corentin Labbe
<clabbe.montjoie@xxxxxxxxx> wrote:
> This patch add documentation about the MDIO switch used on sun8i-h3-emac
> for integrated PHY.
>
> Signed-off-by: Corentin Labbe <clabbe.montjoie@xxxxxxxxx>
> ---
> .../devicetree/bindings/net/dwmac-sun8i.txt | 112 +++++++++++++++++++--
> 1 file changed, 105 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> index 725f3b187886..631084122532 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 named mdio (or mdio_parent in case of H3)
The node would still be named "mdio". You are confusing the node name
with the label. But you don't really need the mdio node for the H3.
You could say here that it is not needed for the H3. See below.
>
> Required properties of the mdio node:
> - #address-cells: shall be 1
> @@ -48,14 +48,21 @@ 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:
> +Required mdio-mux nodes for the following compatibles:
The following compatibles require an mdio-mux node:
> + - "allwinner,sun8i-h3-emac",
Drop the comma. It's already a list.
> +- a mdio-mux node with compatible = "allwinner,sun8i-h3-mdio-switch"
Required properties for the mdio-mux node:
- compatible: "allwinner,sun8i-h3-mdio-switch"
> + - mdio-parent-bus: The parent bus is "mdio_parent"
This is optional in the mdio-mux binding. Since you have the mdio-mux under
the EMAC node, it should be obvious that it is connected to the EMAC. The
parent phandle is more useful for muxes that are separate from the controller,
such as an external GPIO-controlled mux. So you could just drop it.
> + - two child mdio, one for the integrated mdio, one for the external mdio
> +
> +Required properties of the integrated phy node with the following compatibles:
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>;
> @@ -75,10 +82,101 @@ emac: ethernet@1c0b000 {
> mdio: mdio {
> #address-cells = <1>;
> #size-cells = <0>;
> - int_mii_phy: ethernet-phy@1 {
> + };
> + mdio-mux {
Drop the mdio node as mentioned above. And also have spaces between
blocks to be consistent in styling. It's also easier to read.
> + compatible = "allwinner,sun8i-h3-mdio-switch";
> + mdio-parent-bus = <&mdio>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + int_mdio: mdio@1 {
> + reg = <1>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + int_mii_phy: ethernet-phy@1 {
> + reg = <1>;
> + clocks = <&ccu CLK_BUS_EPHY>;
> + resets = <&ccu RST_BUS_EPHY>;
> + phy-is-integrated
> + };
> + };
> + ext_mdio: mdio@0 {
> + reg = <0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> +};
> +
> +Example with external PHY:
> +emac: ethernet@1c0b000 {
> + compatible = "allwinner,sun8i-h3-emac";
> + syscon = <&syscon>;
> + reg = <0x01c0b000 0x104>;
> + interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "macirq";
> + resets = <&ccu RST_BUS_EMAC>;
> + reset-names = "stmmaceth";
> + clocks = <&ccu CLK_BUS_EMAC>;
> + clock-names = "stmmaceth";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + phy-handle = <&ext_rgmii_phy>;
> + phy-mode = "rgmii";
> + allwinner,leds-active-low;
You don't need this when using the external PHY.
Regards
ChenYu
> + mdio: mdio {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + mdio-mux {
> + compatible = "allwinner,sun8i-h3-mdio-switch";
> + mdio-parent-bus = <&mdio>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + int_mdio: mdio@1 {
> + reg = <1>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + int_mii_phy: ethernet-phy@1 {
> + reg = <1>;
> + clocks = <&ccu CLK_BUS_EPHY>;
> + resets = <&ccu RST_BUS_EPHY>;
> + phy-is-integrated
> + };
> + };
> + ext_mdio: mdio@0 {
> + reg = <0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + ext_rgmii_phy: ethernet-phy@1 {
> + reg = <1>;
> + };
> + };
> +};
> +
> +Example with SoC without integrated PHY
> +
> +emac: ethernet@1c0b000 {
> + compatible = "allwinner,sun8i-a83t-emac";
> + syscon = <&syscon>;
> + reg = <0x01c0b000 0x104>;
> + interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "macirq";
> + resets = <&ccu RST_BUS_EMAC>;
> + reset-names = "stmmaceth";
> + clocks = <&ccu CLK_BUS_EMAC>;
> + clock-names = "stmmaceth";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + phy-handle = <&ext_rgmii_phy>;
> + phy-mode = "rgmii";
> + mdio: mdio {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + ext_rgmii_phy: ethernet-phy@1 {
> reg = <1>;
> - clocks = <&ccu CLK_BUS_EPHY>;
> - resets = <&ccu RST_BUS_EPHY>;
> };
> };
> };
> --
> 2.13.0
>