[PATCH v2 0/4] net: qcom/emac: add shared mdio bus support

The emac include MDIO controller, and the motherboard has more than one
PHY connected to an MDIO bus. So share the shared mii_bus for others MAC
device that not has MDIO bus connected. 

Tested: QDF2400 (ACPI), buildin/insmod/rmmod

V2:
 - Separate patch.
 - bindings: s/Since QDF2400 emac/Since emac/

Wang Dongsheng (4):
  net: qcom/emac: split phy_config to mdio bus create and get phy device
  dt-bindings: net: qcom: Add binding for shared mdio bus
  net: qcom/emac: add of shared mdio bus support
  net: qcom/emac: add acpi shared mdio bus support

 .../devicetree/bindings/net/qcom-emac.txt     |   4 +
 drivers/net/ethernet/qualcomm/emac/emac-phy.c | 207 ++++++++++++++----
 drivers/net/ethernet/qualcomm/emac/emac.c     |   8 +-
 3 files changed, 175 insertions(+), 44 deletions(-)

-- 
2.18.0

This property copy from "ibm,emac.txt" to describe a shared MIDO bus.
Since emac include MDIO, so If the motherboard has more than one PHY
connected to an MDIO bus, this property will point to the MAC device
that has the MDIO bus.

Signed-off-by: Wang Dongsheng <redacted>
---
V2: s/Since QDF2400 emac/Since emac/
---
 Documentation/devicetree/bindings/net/qcom-emac.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/qcom-emac.txt b/Documentation/devicetree/bindings/net/qcom-emac.txt
index 346e6c7f47b7..50db71771358 100644
--- a/Documentation/devicetree/bindings/net/qcom-emac.txt
+++ b/Documentation/devicetree/bindings/net/qcom-emac.txt

@@ -24,6 +24,9 @@ Internal PHY node:
 The external phy child node:
 - reg : The phy address
 
+Optional properties:
+- mdio-device : Shared MIDO bus.
+
 Example:
 
 FSM9900:

@@ -86,6 +89,7 @@ soc {
 		reg = <0x0 0x38800000 0x0 0x10000>,
 		      <0x0 0x38816000 0x0 0x1000>;
 		interrupts = <0 256 4>;
+		mdio-device = <&emac1>;
 
 		clocks = <&gcc 0>, <&gcc 1>, <&gcc 3>, <&gcc 4>, <&gcc 5>,
 			 <&gcc 6>, <&gcc 7>;

-- 
2.18.0

This patch separate emac_mdio_bus_create and emac_get_phydev from
emac_phy_config, and do some codes clean.

Signed-off-by: Wang Dongsheng <redacted>
---
 drivers/net/ethernet/qualcomm/emac/emac-phy.c | 99 +++++++++++--------
 1 file changed, 57 insertions(+), 42 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/emac/emac-phy.c b/drivers/net/ethernet/qualcomm/emac/emac-phy.c
index 53dbf1e163a8..2d16c6b9ef49 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-phy.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac-phy.c

@@ -96,15 +96,14 @@ static int emac_mdio_write(struct mii_bus *bus, int addr, int regnum, u16 val)
 	return 0;
 }
 
-/* Configure the MDIO bus and connect the external PHY */
-int emac_phy_config(struct platform_device *pdev, struct emac_adapter *adpt)
+static int emac_mdio_bus_create(struct platform_device *pdev,
+				struct emac_adapter *adpt)
 {
 	struct device_node *np = pdev->dev.of_node;
 	struct mii_bus *mii_bus;
 	int ret;
 
-	/* Create the mii_bus object for talking to the MDIO bus */
-	adpt->mii_bus = mii_bus = devm_mdiobus_alloc(&pdev->dev);
+	mii_bus = devm_mdiobus_alloc(&pdev->dev);
 	if (!mii_bus)
 		return -ENOMEM;
 

@@ -115,50 +114,66 @@ int emac_phy_config(struct platform_device *pdev, struct emac_adapter *adpt)
 	mii_bus->parent = &pdev->dev;
 	mii_bus->priv = adpt;
 
-	if (has_acpi_companion(&pdev->dev)) {
-		u32 phy_addr;
-
-		ret = mdiobus_register(mii_bus);
-		if (ret) {
-			dev_err(&pdev->dev, "could not register mdio bus\n");
-			return ret;
-		}
-		ret = device_property_read_u32(&pdev->dev, "phy-channel",
-					       &phy_addr);
-		if (ret)
-			/* If we can't read a valid phy address, then assume
-			 * that there is only one phy on this mdio bus.
-			 */
-			adpt->phydev = phy_find_first(mii_bus);
-		else
-			adpt->phydev = mdiobus_get_phy(mii_bus, phy_addr);
-
-		/* of_phy_find_device() claims a reference to the phydev,
-		 * so we do that here manually as well. When the driver
-		 * later unloads, it can unilaterally drop the reference
-		 * without worrying about ACPI vs DT.
-		 */
-		if (adpt->phydev)
-			get_device(&adpt->phydev->mdio.dev);
-	} else {
-		struct device_node *phy_np;
+	ret = of_mdiobus_register(mii_bus, has_acpi_companion(&pdev->dev) ?
+				  NULL : np);
+	if (ret) {
+		dev_err(&pdev->dev, "Could not register mdio bus\n");
+		return ret;
+	}
 
-		ret = of_mdiobus_register(mii_bus, np);
-		if (ret) {
-			dev_err(&pdev->dev, "could not register mdio bus\n");
-			return ret;
-		}
+	adpt->mii_bus = mii_bus;
+	return 0;
+}
 
+static void emac_get_phydev(struct platform_device *pdev,
+			    struct emac_adapter *adpt)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct mii_bus *bus = adpt->mii_bus;
+	struct device_node *phy_np;
+	u32 phy_addr;
+	int ret;
+
+	if (!has_acpi_companion(&pdev->dev)) {
 		phy_np = of_parse_phandle(np, "phy-handle", 0);
 		adpt->phydev = of_phy_find_device(phy_np);
 		of_node_put(phy_np);
+		return;
 	}
 
-	if (!adpt->phydev) {
-		dev_err(&pdev->dev, "could not find external phy\n");
-		mdiobus_unregister(mii_bus);
-		return -ENODEV;
-	}
+	ret = device_property_read_u32(&pdev->dev, "phy-channel",
+				       &phy_addr);
+	if (ret)
+		/* If we can't read a valid phy address, then assume
+		 * that there is only one phy on this mdio bus.
+		 */
+		adpt->phydev = phy_find_first(bus);
+	else
+		adpt->phydev = mdiobus_get_phy(bus, phy_addr);
+
+	/* of_phy_find_device() claims a reference to the phydev,
+	 * so we do that here manually as well. When the driver
+	 * later unloads, it can unilaterally drop the reference
+	 * without worrying about ACPI vs DT.
+	 */
+	if (adpt->phydev)
+		get_device(&adpt->phydev->mdio.dev);
+}
 
-	return 0;
+/* Configure the MDIO bus and connect the external PHY */
+int emac_phy_config(struct platform_device *pdev, struct emac_adapter *adpt)
+{
+	int ret;
+
+	ret = emac_mdio_bus_create(pdev, adpt);
+	if (ret)
+		return ret;
+
+	emac_get_phydev(pdev, adpt);
+	if (adpt->phydev)
+		return 0;
+
+	dev_err(&pdev->dev, "Could not find external phy\n");
+	mdiobus_unregister(adpt->mii_bus);
+	return -ENODEV;
 }

-- 
2.18.0

On Mon, Sep 17, 2018 at 04:53:29PM +0800, Wang Dongsheng wrote:

Hi Dongsheng

I don't see why you need this property. The ethernet interface has a
phy-handle which points to a PHY. That is all you need to find the PHY.

        emac0: ethernet@feb20000 {
                compatible = "qcom,fsm9900-emac";
                reg = <0xfeb20000 0x10000>,
                      <0xfeb36000 0x1000>;
                interrupts = <76>;

                clocks = <&gcc 0>, <&gcc 1>, <&gcc 3>, <&gcc 4>, <&gcc 5>,
                        <&gcc 6>, <&gcc 7>;
                clock-names = "axi_clk", "cfg_ahb_clk", "high_speed_clk",
                        "mdio_clk", "tx_clk", "rx_clk", "sys_clk";

                internal-phy = <&emac_sgmii>;

                phy-handle = <&phy0>;

                #address-cells = <1>;
                #size-cells = <0>;

                phy0: ethernet-phy@0 {
                        reg = <0>;
                };

                phy1: ethernet-phy@1 {
                        reg = <1>;
                };

                pinctrl-names = "default";
                pinctrl-0 = <&mdio_pins_a>;
        };

        emac1: ethernet@38900000 {
                compatible = "qcom,fsm9900-emac";
		...
		...

                phy-handle = <&phy1>;
	};

	Andrew

On 9/17/2018 10:50 PM, Andrew Lunn wrote:

On Mon, Sep 17, 2018 at 04:53:29PM +0800, Wang Dongsheng wrote:

quoted

This property copy from "ibm,emac.txt" to describe a shared MIDO bus.
Since emac include MDIO, so If the motherboard has more than one PHY
connected to an MDIO bus, this property will point to the MAC device
that has the MDIO bus.

Signed-off-by: Wang Dongsheng <redacted>
---
V2: s/Since QDF2400 emac/Since emac/
---
 Documentation/devicetree/bindings/net/qcom-emac.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/qcom-emac.txt b/Documentation/devicetree/bindings/net/qcom-emac.txt
index 346e6c7f47b7..50db71771358 100644
--- a/Documentation/devicetree/bindings/net/qcom-emac.txt
+++ b/Documentation/devicetree/bindings/net/qcom-emac.txt

@@ -24,6 +24,9 @@ Internal PHY node:
 The external phy child node:
 - reg : The phy address
 
+Optional properties:
+- mdio-device : Shared MIDO bus.

Hi Dongsheng

I don't see why you need this property. The ethernet interface has a
phy-handle which points to a PHY. That is all you need to find the PHY.

phy-handle is description PHY address. This property is describing an
MDIO controller.
Each QCOM emac include an MDIO controller, normally each emac only
connect one
PHY device, but when all of the PHY devices mdio lines connect one MDIO
controller
that is included in EMAC, we need to share this MDIO controller for
others EMAC.

Normally:

                                (MDIO)
MAC0 ---------------------------------------PHY0
     |                                                               |
     |                          (DATA)                         |
     ---------------------------------------------


                                (MDIO)
MAC1 ---------------------------------------PHY1
     |                                                               |
     |                          (DATA)                         |
     ---------------------------------------------



Shared MDIO bus: "mdio-device" = &MAC0, MAC1 will get MAC0's MDIO bus
and also get the corresponding PHY device.

                                (DATA)
MAC0 ---------------------------------------PHY0
     |                                                               |
     |                          (MDIO)                         |
     --------------------------------------------
                                                                     |
MAC1                                                        PHY0
     |                                                               |
     |                          (DATA)                         |
     --------------------------------------------

Cheers,
Dongsheng

        emac0: ethernet@feb20000 {
                compatible = "qcom,fsm9900-emac";
                reg = <0xfeb20000 0x10000>,
                      <0xfeb36000 0x1000>;
                interrupts = <76>;

                clocks = <&gcc 0>, <&gcc 1>, <&gcc 3>, <&gcc 4>, <&gcc 5>,
                        <&gcc 6>, <&gcc 7>;
                clock-names = "axi_clk", "cfg_ahb_clk", "high_speed_clk",
                        "mdio_clk", "tx_clk", "rx_clk", "sys_clk";

                internal-phy = <&emac_sgmii>;

                phy-handle = <&phy0>;

                #address-cells = <1>;
                #size-cells = <0>;

                phy0: ethernet-phy@0 {
                        reg = <0>;
                };

                phy1: ethernet-phy@1 {
                        reg = <1>;
                };

                pinctrl-names = "default";
                pinctrl-0 = <&mdio_pins_a>;
        };

        emac1: ethernet@38900000 {
                compatible = "qcom,fsm9900-emac";
		...
		...

                phy-handle = <&phy1>;
	};

	Andrew

On 09/17/2018 09:47 AM, Wang, Dongsheng wrote:

On 9/17/2018 10:50 PM, Andrew Lunn wrote:

quoted

On Mon, Sep 17, 2018 at 04:53:29PM +0800, Wang Dongsheng wrote:

quoted

This property copy from "ibm,emac.txt" to describe a shared MIDO bus.
Since emac include MDIO, so If the motherboard has more than one PHY
connected to an MDIO bus, this property will point to the MAC device
that has the MDIO bus.

Signed-off-by: Wang Dongsheng <redacted>
---
V2: s/Since QDF2400 emac/Since emac/
---
 Documentation/devicetree/bindings/net/qcom-emac.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/qcom-emac.txt b/Documentation/devicetree/bindings/net/qcom-emac.txt
index 346e6c7f47b7..50db71771358 100644
--- a/Documentation/devicetree/bindings/net/qcom-emac.txt
+++ b/Documentation/devicetree/bindings/net/qcom-emac.txt

@@ -24,6 +24,9 @@ Internal PHY node:
 The external phy child node:
 - reg : The phy address
 
+Optional properties:
+- mdio-device : Shared MIDO bus.

Hi Dongsheng

I don't see why you need this property. The ethernet interface has a
phy-handle which points to a PHY. That is all you need to find the PHY.

phy-handle is description PHY address. This property is describing an
MDIO controller.
Each QCOM emac include an MDIO controller, normally each emac only
connect one
PHY device, but when all of the PHY devices mdio lines connect one MDIO
controller
that is included in EMAC, we need to share this MDIO controller for
others EMAC.

If you want to describe the MDIO controller, then you embed a mdio
subnode into your Ethernet MAC node:

 emac0: ethernet@feb20000 {
	mdio {
		#address-cells = <1>;
		#size-cells = <0>;

		phy0: ethernet-phy@0 {
			reg = <0>;
		};
	};
};

And then each Ethernet MAC controller refers to their appropriate PHY
device tree node using a phy-handle property to point to either their
own MDIO controller, or another MAC's MDIO controller.

The IBM Emac is a old (not to say bad) example and it does not use the
PHY library and the standard Device Tree node property, please don't use
it as a reference.
-- 
Florian

On 2018/9/18 0:54, Florian Fainelli wrote:

On 09/17/2018 09:47 AM, Wang, Dongsheng wrote:

quoted

On 9/17/2018 10:50 PM, Andrew Lunn wrote:

quoted

On Mon, Sep 17, 2018 at 04:53:29PM +0800, Wang Dongsheng wrote:

quoted

This property copy from "ibm,emac.txt" to describe a shared MIDO bus.
Since emac include MDIO, so If the motherboard has more than one PHY
connected to an MDIO bus, this property will point to the MAC device
that has the MDIO bus.

Signed-off-by: Wang Dongsheng <redacted>
---
V2: s/Since QDF2400 emac/Since emac/
---
 Documentation/devicetree/bindings/net/qcom-emac.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/qcom-emac.txt b/Documentation/devicetree/bindings/net/qcom-emac.txt
index 346e6c7f47b7..50db71771358 100644
--- a/Documentation/devicetree/bindings/net/qcom-emac.txt
+++ b/Documentation/devicetree/bindings/net/qcom-emac.txt

@@ -24,6 +24,9 @@ Internal PHY node:
 The external phy child node:
 - reg : The phy address
 
+Optional properties:
+- mdio-device : Shared MIDO bus.

Hi Dongsheng

I don't see why you need this property. The ethernet interface has a
phy-handle which points to a PHY. That is all you need to find the PHY.

phy-handle is description PHY address. This property is describing an
MDIO controller.
Each QCOM emac include an MDIO controller, normally each emac only
connect one
PHY device, but when all of the PHY devices mdio lines connect one MDIO
controller
that is included in EMAC, we need to share this MDIO controller for
others EMAC.

If you want to describe the MDIO controller, then you embed a mdio
subnode into your Ethernet MAC node:

 emac0: ethernet@feb20000 {
	mdio {
		#address-cells = <1>;
		#size-cells = <0>;

		phy0: ethernet-phy@0 {
			reg = <0>;
		};
	};
};

And then each Ethernet MAC controller refers to their appropriate PHY
device tree node using a phy-handle property to point to either their
own MDIO controller, or another MAC's MDIO controller.

Sorry, I do not understand how phy-handle point to MDIO controller,
because phy-handle is defined to point to a phy.
I suppose you mean:
mdio_node = of_get_parent(phy_node);

emac0: ethernet@feb20000 {
    phy-handle = <&phy0>;

    mdio {
        #address-cells = <1>;
        #size-cells = <0>;

        phy0: ethernet-phy@0 {
            reg = <0>;
        };

        phy1: ethernet-phy@1 {
            reg = <1>;
        };
    };
};

emac1: ethernet@fexxxxxx {
    phy-handle = <&phy1>;
};

emac2: ethernet@fexxxxxx {
    phy-handle = <&phy2>;
    mdio {
        #address-cells = <1>;
        #size-cells = <0>;

        phy2: ethernet-phy@2 {
            reg = <0>;
        };
    };
};

The IBM Emac is a old (not to say bad) example and it does not use the
PHY library and the standard Device Tree node property, please don't use
it as a reference.

Ok, thanks.

Cheers,
Dongsheng

quoted

If you want to describe the MDIO controller, then you embed a mdio
subnode into your Ethernet MAC node:

 emac0: ethernet@feb20000 {
	mdio {
		#address-cells = <1>;
		#size-cells = <0>;

		phy0: ethernet-phy@0 {
			reg = <0>;
		};
	};
};

And then each Ethernet MAC controller refers to their appropriate PHY
device tree node using a phy-handle property to point to either their
own MDIO controller, or another MAC's MDIO controller.

Sorry, I do not understand how phy-handle point to MDIO controller,
because phy-handle is defined to point to a phy.

The MAC driver does not care what MDIO controller a PHY is on. All you
need to do to register the PHY is:

	phy_node = of_parse_phandle(np, "phy-handle", 0);
	phy_interface = of_get_phy_mode(np);
	phydev = of_phy_connect(dev, phy_node,
                                &handle_link_change, 0,
                                phy_interface);

	Andrew

On 2018/9/18 20:35, Andrew Lunn wrote:

quoted

quoted

If you want to describe the MDIO controller, then you embed a mdio
subnode into your Ethernet MAC node:

 emac0: ethernet@feb20000 {
	mdio {
		#address-cells = <1>;
		#size-cells = <0>;

		phy0: ethernet-phy@0 {
			reg = <0>;
		};
	};
};

And then each Ethernet MAC controller refers to their appropriate PHY
device tree node using a phy-handle property to point to either their
own MDIO controller, or another MAC's MDIO controller.

Sorry, I do not understand how phy-handle point to MDIO controller,
because phy-handle is defined to point to a phy.

The MAC driver does not care what MDIO controller a PHY is on. All you
need to do to register the PHY is:

Yes, these are all things that must be done, and emac driver will
connect phy when mac up.
If we had a separate MDIO controller, the MAC would not care about MDIO
bus. But MDIO is integrated within the EMAC, and emac driver maintains
the mdio.

Each EMAC do their mdio register/unregister. But in the shared scenario,
the EMACs that use the shared bus do not need to create an MDIO and
cannot release the Shared bus.

In device tree environment as you and Florian said. Just use phy-handle
get the phy_node.
The EMAC would not care about the phy come from which MDIO bus because
the phy device gets from the device_node match(phy-handle). And if the
phy_dev cannot get through phy_node means, the mdio bus is not ready.

But ACPI environment my understand is this:
First method. EMAC driver gets the shared MDIO bus, and maintain it.
The second way, EMAC match the phy_dev from the name.
These patch series try to use the FIRST way. Now I prefer to use the
second way to do the shared function.

I will rework this patchset and maybe patches will be a delay for a few
days.

Cheers,
Dongsheng

	phy_node = of_parse_phandle(np, "phy-handle", 0);
	phy_interface = of_get_phy_mode(np);
	phydev = of_phy_connect(dev, phy_node,
                                &handle_link_change, 0,
                                phy_interface);

	Andrew

On Wed, Sep 19, 2018 at 09:19:19AM +0000, Wang, Dongsheng wrote:

On 2018/9/18 20:35, Andrew Lunn wrote:

quoted

quoted

quoted

If you want to describe the MDIO controller, then you embed a mdio
subnode into your Ethernet MAC node:

 emac0: ethernet@feb20000 {
	mdio {
		#address-cells = <1>;
		#size-cells = <0>;

		phy0: ethernet-phy@0 {
			reg = <0>;
		};
	};
};

And then each Ethernet MAC controller refers to their appropriate PHY
device tree node using a phy-handle property to point to either their
own MDIO controller, or another MAC's MDIO controller.

Sorry, I do not understand how phy-handle point to MDIO controller,
because phy-handle is defined to point to a phy.

The MAC driver does not care what MDIO controller a PHY is on. All you
need to do to register the PHY is:

Yes, these are all things that must be done, and emac driver will
connect phy when mac up.
If we had a separate MDIO controller, the MAC would not care about MDIO
bus. But MDIO is integrated within the EMAC, and emac driver maintains
the mdio.

Each EMAC do their mdio register/unregister. But in the shared scenario,
the EMACs that use the shared bus do not need to create an MDIO and
cannot release the Shared bus.

Hi Dongsheng

There is nothing new here. Many Ethernet drivers export an MDIO bus
which is then used by some other device, often an Ethernet
switch. Ordering should not be a problem, you just need to handle
EPROBE_DEFER, which will happen if the MDIO bus has not yet been
probed when you try to lookup the phy-handle. And once the phy has
been connected, the MDIO bus will be locked, preventing it from being
removed.

But ACPI environment my understand is this:

ACPI is completely separate and should not affect the DT binding.
I've not yet looked at the ACPI changes you added.

I will rework this patchset and maybe patches will be a delay for a few
days.

Thanks

	Andrew

On 9/19/18 7:25 AM, Andrew Lunn wrote:

ACPI is completely separate and should not affect the DT binding.
I've not yet looked at the ACPI changes you added.

Just FYI, there is no device tree platform on which the upstream EMAC 
driver works.  All of the DT code in the driver is theoretical.  It 
worked once on a prototype platform, when I originally wrote the code, 
but since then DT support is mostly a guess.

The focus of any patches for the EMAC should be ACPI, not DT.  If 
anything, ACPI support should come first.  No one should be writing or 
reviewing DT code before ACPI code.

The upstream EMAC driver is only known to work on the QDF2400, which is 
an ACPI-only chip.  I feel like I've been repeating this too often.

The focus of any patches for the EMAC should be ACPI, not DT.  If anything,
ACPI support should come first.  No one should be writing or reviewing DT
code before ACPI code.

I suspect that is not going to be easy. Last time i looked, the ACPI
standard had nothing about MDIO busses or PHYs. Marcin Wojtas did some
work in this area a while back for the mvpp2, but if i remember
correctly, he worked around this by simply not having a PHY when using
ACPI, and making use of a MAC interrupt which indicated when there was
link.

Whoever implements this first needs to be an ACPI expert and probably
needs to write it up and submit it as an amendment to the ACPI
standard.

      Andrew

On 2018/9/19 22:15, Timur Tabi wrote:

On 9/19/18 7:25 AM, Andrew Lunn wrote:

quoted

ACPI is completely separate and should not affect the DT binding.
I've not yet looked at the ACPI changes you added.

Just FYI, there is no device tree platform on which the upstream EMAC 
driver works.  All of the DT code in the driver is theoretical.  It 
worked once on a prototype platform, when I originally wrote the code, 
but since then DT support is mostly a guess.

The focus of any patches for the EMAC should be ACPI, not DT.  If 
anything, ACPI support should come first.  No one should be writing or 
reviewing DT code before ACPI code.

The upstream EMAC driver is only known to work on the QDF2400, which is 
an ACPI-only chip.  I feel like I've been repeating this too often.

Ok, I just focus on ACPI, and keep DT code no changes.

Cheers,
Dongsheng

On 9/19/18 10:20 AM, Andrew Lunn wrote:

I suspect that is not going to be easy. Last time i looked, the ACPI
standard had nothing about MDIO busses or PHYs. Marcin Wojtas did some
work in this area a while back for the mvpp2, but if i remember
correctly, he worked around this by simply not having a PHY when using
ACPI, and making use of a MAC interrupt which indicated when there was
link.

Whoever implements this first needs to be an ACPI expert and probably
needs to write it up and submit it as an amendment to the ACPI
standard.

If that's what it takes, then so be it.  But adding DT support for a 
device that is only used on ACPI platforms is not a worthwhile endeavor.

After ACPI support is merged, Dongsheng can choose to add DT support to 
maintain parity, if he wants.  Maybe one day the MSM developers will use 
the upstream driver on one of their SOCs.

The emac include MDIO controller, and the motherboard has more than one
PHY connected to an MDIO bus. So share the shared mii_bus for others MAC
device that not has MDIO bus connected.

Based on ACPI, since "phy-handle" cannot directly point to a _DSD
sub-package, so we use "phy-handle" to point an internal MDIO device port.
The port describes the phy address.

Tested: QDF2400 (ACPI), buildin/insmod/rmmod

V3:
 - Add "phy-handle" support.
 - Remove all of DT changes.

V2:
 - Separate patch.

Wang Dongsheng (2):
  net: qcom/emac: split phy_config to mdio bus create and get phy device
  net: qcom/emac: add phy-handle support for ACPI

 drivers/net/ethernet/qualcomm/emac/emac-phy.c | 183 ++++++++++++++----
 1 file changed, 142 insertions(+), 41 deletions(-)

-- 
2.18.0

This patch separate emac_mdio_bus_create and emac_get_phydev from
emac_phy_config, and do some codes clean.

Signed-off-by: Wang Dongsheng <redacted>
---
 drivers/net/ethernet/qualcomm/emac/emac-phy.c | 96 +++++++++++--------
 1 file changed, 56 insertions(+), 40 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/emac/emac-phy.c b/drivers/net/ethernet/qualcomm/emac/emac-phy.c
index 53dbf1e163a8..f2ed013ce5d5 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-phy.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac-phy.c

@@ -96,15 +96,15 @@ static int emac_mdio_write(struct mii_bus *bus, int addr, int regnum, u16 val)
 	return 0;
 }
 
-/* Configure the MDIO bus and connect the external PHY */
-int emac_phy_config(struct platform_device *pdev, struct emac_adapter *adpt)
+static int emac_mdio_bus_create(struct platform_device *pdev,
+				struct emac_adapter *adpt)
 {
 	struct device_node *np = pdev->dev.of_node;
 	struct mii_bus *mii_bus;
 	int ret;
 
 	/* Create the mii_bus object for talking to the MDIO bus */
-	adpt->mii_bus = mii_bus = devm_mdiobus_alloc(&pdev->dev);
+	mii_bus = devm_mdiobus_alloc(&pdev->dev);
 	if (!mii_bus)
 		return -ENOMEM;
 

@@ -115,50 +115,66 @@ int emac_phy_config(struct platform_device *pdev, struct emac_adapter *adpt)
 	mii_bus->parent = &pdev->dev;
 	mii_bus->priv = adpt;
 
-	if (has_acpi_companion(&pdev->dev)) {
-		u32 phy_addr;
-
-		ret = mdiobus_register(mii_bus);
-		if (ret) {
-			dev_err(&pdev->dev, "could not register mdio bus\n");
-			return ret;
-		}
-		ret = device_property_read_u32(&pdev->dev, "phy-channel",
-					       &phy_addr);
-		if (ret)
-			/* If we can't read a valid phy address, then assume
-			 * that there is only one phy on this mdio bus.
-			 */
-			adpt->phydev = phy_find_first(mii_bus);
-		else
-			adpt->phydev = mdiobus_get_phy(mii_bus, phy_addr);
-
-		/* of_phy_find_device() claims a reference to the phydev,
-		 * so we do that here manually as well. When the driver
-		 * later unloads, it can unilaterally drop the reference
-		 * without worrying about ACPI vs DT.
-		 */
-		if (adpt->phydev)
-			get_device(&adpt->phydev->mdio.dev);
-	} else {
-		struct device_node *phy_np;
-
-		ret = of_mdiobus_register(mii_bus, np);
-		if (ret) {
-			dev_err(&pdev->dev, "could not register mdio bus\n");
-			return ret;
-		}
+	ret = of_mdiobus_register(mii_bus, has_acpi_companion(&pdev->dev) ?
+				  NULL : np);
+	if (ret)
+		dev_err(&pdev->dev, "Could not register mdio bus\n");
+
+	adpt->mii_bus = ret ? NULL : mii_bus;
+	return ret;
+}
+
+static int emac_get_phydev(struct platform_device *pdev,
+			   struct emac_adapter *adpt)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct mii_bus *bus = adpt->mii_bus;
+	struct device_node *phy_np;
+	struct phy_device *phydev;
 
+	u32 phy_addr;
+	int ret;
+
+	if (!has_acpi_companion(&pdev->dev)) {
 		phy_np = of_parse_phandle(np, "phy-handle", 0);
 		adpt->phydev = of_phy_find_device(phy_np);
 		of_node_put(phy_np);
+		return adpt->phydev ? 0 : -ENODEV;
 	}
 
-	if (!adpt->phydev) {
-		dev_err(&pdev->dev, "could not find external phy\n");
-		mdiobus_unregister(mii_bus);
+	ret = device_property_read_u32(&pdev->dev, "phy-channel",
+				       &phy_addr);
+	/* If we can't read a valid phy address, then assume
+	 * that there is only one phy on this mdio bus.
+	 */
+	phydev = ret ? phy_find_first(bus) : mdiobus_get_phy(bus, phy_addr);
+	if (!phydev)
 		return -ENODEV;
-	}
 
+	/* of_phy_find_device() claims a reference to the phydev,
+	 * so we do that here manually as well. When the driver
+	 * later unloads, it can unilaterally drop the reference
+	 * without worrying about ACPI vs DT.
+	 */
+	get_device(&phydev->mdio.dev);
+	adpt->phydev = phydev;
 	return 0;
 }
+
+/* Configure the MDIO bus and connect the external PHY */
+int emac_phy_config(struct platform_device *pdev, struct emac_adapter *adpt)
+{
+	int ret;
+
+	ret = emac_mdio_bus_create(pdev, adpt);
+	if (ret)
+		return ret;
+
+	ret = emac_get_phydev(pdev, adpt);
+	if (ret) {
+		dev_err(&pdev->dev, "Could not find external phy\n");
+		mdiobus_unregister(adpt->mii_bus);
+	}
+
+	return ret;
+}

-- 
2.18.0

Use "phy-handle" to porint an internal MDIO device port.

Signed-off-by: Wang Dongsheng <redacted>
---
 drivers/net/ethernet/qualcomm/emac/emac-phy.c | 115 +++++++++++++++---
 1 file changed, 100 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/emac/emac-phy.c b/drivers/net/ethernet/qualcomm/emac/emac-phy.c
index f2ed013ce5d5..3dc3ae55e5bb 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-phy.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac-phy.c

@@ -96,6 +96,96 @@ static int emac_mdio_write(struct mii_bus *bus, int addr, int regnum, u16 val)
 	return 0;
 }
 
+static int acpi_device_match(struct device *dev, void *fwnode)
+{
+	return dev->fwnode == fwnode;
+}
+
+static struct phy_device *
+emac_acpi_get_phydev_from_phy_handle(struct platform_device *pdev)
+{
+	struct fwnode_reference_args args;
+	struct fwnode_handle *fw_node;
+	struct acpi_device *adev;
+	acpi_handle handle;
+	struct device *dev;
+	struct phy_device *phydev;
+	struct net_device *netdev;
+	struct emac_adapter *adpt;
+	int phy_addr;
+	int ret;
+
+	/* Get PHY Port reference from phy-handle */
+	fw_node = acpi_fwnode_handle(ACPI_COMPANION(&pdev->dev));
+	ret = acpi_node_get_property_reference(fw_node, "phy-handle", 0,
+					       &args);
+	if (ACPI_FAILURE(ret) || !is_acpi_device_node(args.fwnode))
+		return ERR_PTR(-ENODEV);
+
+	/* Get PHY addr from the port node */
+	if (fwnode_property_read_u32(args.fwnode, "phy-channel", &phy_addr))
+		return ERR_PTR(-ENODEV);
+
+	/* Get the MDIO bus that included the port */
+	handle = ACPI_HANDLE_FWNODE(args.fwnode);
+	if (!handle || acpi_bus_get_device(handle, &adev))
+		return ERR_PTR(-ENODEV);
+
+	while (adev->parent) {
+		if (!strcmp(acpi_device_hid(adev), "QCOM8070"))
+			break;
+		adev = adev->parent;
+	}
+	if (!adev->parent)
+		return ERR_PTR(-ENODEV);
+
+	dev = bus_find_device(&platform_bus_type, NULL,
+			      &adev->fwnode,
+			      acpi_device_match);
+	if (!dev)
+		return ERR_PTR(-ENODEV);
+
+	netdev = dev_get_drvdata(dev);
+	if (!netdev)
+		return ERR_PTR(-EPROBE_DEFER);
+
+	adpt = netdev_priv(netdev);
+	if (!adpt->mii_bus)
+		return ERR_PTR(-EPROBE_DEFER);
+
+	phydev = mdiobus_get_phy(adpt->mii_bus, phy_addr);
+	return phydev ? phydev : ERR_PTR(-ENODEV);
+}
+
+static struct phy_device *
+emac_acpi_get_phydev(struct platform_device *pdev, struct emac_adapter *adpt)
+{
+	struct phy_device *phydev = NULL;
+	int phy_addr;
+	int ret;
+
+	/* Compatible with "phy-channel" */
+	ret = device_property_read_u32(&pdev->dev, "phy-channel",
+				       &phy_addr);
+	if (!ret)
+		phydev = mdiobus_get_phy(adpt->mii_bus, phy_addr);
+	if (phydev)
+		return phydev;
+
+	/* Get PHY Port reference from phy-handle */
+	phydev = emac_acpi_get_phydev_from_phy_handle(pdev);
+	if (!IS_ERR(phydev))
+		return phydev;
+	if (PTR_ERR(phydev) == -EPROBE_DEFER)
+		return ERR_PTR(-EPROBE_DEFER);
+
+	/* If we can't read a valid phy address from "phy-channel"/"phy-handle",
+	 * then assume that there is only one phy on local mdio bus.
+	 */
+	phydev = phy_find_first(adpt->mii_bus);
+	return phydev ? phydev : ERR_PTR(-ENODEV);
+}
+
 static int emac_mdio_bus_create(struct platform_device *pdev,
 				struct emac_adapter *adpt)
 {

@@ -128,13 +218,9 @@ static int emac_get_phydev(struct platform_device *pdev,
 			    struct emac_adapter *adpt)
 {
 	struct device_node *np = pdev->dev.of_node;
-	struct mii_bus *bus = adpt->mii_bus;
 	struct device_node *phy_np;
 	struct phy_device *phydev;
 
-	u32 phy_addr;
-	int ret;
-
 	if (!has_acpi_companion(&pdev->dev)) {
 		phy_np = of_parse_phandle(np, "phy-handle", 0);
 		adpt->phydev = of_phy_find_device(phy_np);

@@ -142,14 +228,9 @@ static int emac_get_phydev(struct platform_device *pdev,
 		return adpt->phydev ? 0 : -ENODEV;
 	}
 
-	ret = device_property_read_u32(&pdev->dev, "phy-channel",
-				       &phy_addr);
-	/* If we can't read a valid phy address, then assume
-	 * that there is only one phy on this mdio bus.
-	 */
-	phydev = ret ? phy_find_first(bus) : mdiobus_get_phy(bus, phy_addr);
-	if (!phydev)
-		return -ENODEV;
+	phydev = emac_acpi_get_phydev(pdev, adpt);
+	if (IS_ERR(phydev))
+		return PTR_ERR(phydev);
 
 	/* of_phy_find_device() claims a reference to the phydev,
 	 * so we do that here manually as well. When the driver

@@ -171,10 +252,14 @@ int emac_phy_config(struct platform_device *pdev, struct emac_adapter *adpt)
 		return ret;
 
 	ret = emac_get_phydev(pdev, adpt);
-	if (ret) {
+	if (!ret)
+		return 0;
+
+	if (ret != -EPROBE_DEFER)
 		dev_err(&pdev->dev, "Could not find external phy\n");
-		mdiobus_unregister(adpt->mii_bus);
-	}
+	else
+		dev_warn(&pdev->dev, "Phy is not available yet, deferred probing\n");
 
+	mdiobus_unregister(adpt->mii_bus);
 	return ret;
 }

-- 
2.18.0

On 2018/10/26 3:24, Andrew Lunn wrote:

On Thu, Oct 25, 2018 at 06:09:15PM +0800, Wang Dongsheng wrote:

quoted

Use "phy-handle" to porint an internal MDIO device port.

Hi Dongsheng

You are basically defining how all future ACPI based MAC drivers get
access to their PHY. This needs to become part of the ACPI standard,
etc.

This code should not be hidden away in the emac driver. It needs to be
placed somewhere public so other drivers can use it. And it needs good
documentation, including an example of what needs to go into the ACPI
tables, etc.

Hi Andrew

I saw AppliedMicro(apm) xgene has used "phy-handle" for ACPI method, so
I guess "phy-handle" has become part of the ACPI standard. But I cannot
make sure.
I tried to confirm the property is defined in the document(Like DT
binding). However, I did not find any documentation on the description
property definition on the UEFI/ACPICA website.
But when I was reading Documentation/acpi/DSD-properties-rules.txt, my
understanding is we should try to conform to DT bindings. So maybe ACPI
doesn't have such a document, just DT bindings.

Cheers,
Dongsheng

Thanks
	Andrew

On 10/25/18 9:18 PM, Wang, Dongsheng wrote:

But when I was reading Documentation/acpi/DSD-properties-rules.txt, my
understanding is we should try to conform to DT bindings. So maybe ACPI
doesn't have such a document, just DT bindings.

There was an attempt to document DSDs, but it was abandoned after a while.

https://github.com/ahs3/dsd

On 2018/10/26 10:37, Timur Tabi wrote:

On 10/25/18 9:18 PM, Wang, Dongsheng wrote:

quoted

But when I was reading Documentation/acpi/DSD-properties-rules.txt, my
understanding is we should try to conform to DT bindings. So maybe ACPI
doesn't have such a document, just DT bindings.

There was an attempt to document DSDs, but it was abandoned after a while.

https://github.com/ahs3/dsd

Yes, here's a database concept, and I asked some Intel guys, the answer
I got was there is no such database or document. :(


Cheers,

Dongsheng

On Thu, Oct 25, 2018 at 06:09:15PM +0800, Wang Dongsheng wrote:

Use "phy-handle" to porint an internal MDIO device port.

Hi Dongsheng

You are basically defining how all future ACPI based MAC drivers get
access to their PHY. This needs to become part of the ACPI standard,
etc.

This code should not be hidden away in the emac driver. It needs to be
placed somewhere public so other drivers can use it. And it needs good
documentation, including an example of what needs to go into the ACPI
tables, etc.

Thanks
	Andrew

On Fri, Oct 26, 2018 at 03:04:25AM +0000, Wang, Dongsheng wrote:

On 2018/10/26 10:37, Timur Tabi wrote:

quoted

On 10/25/18 9:18 PM, Wang, Dongsheng wrote:

quoted

But when I was reading Documentation/acpi/DSD-properties-rules.txt, my
understanding is we should try to conform to DT bindings. So maybe ACPI
doesn't have such a document, just DT bindings.

There was an attempt to document DSDs, but it was abandoned after a while.

https://github.com/ahs3/dsd

Yes, here's a database concept, and I asked some Intel guys, the answer
I got was there is no such database or document. :(

Hi Dongsheng

If there is no clear documentation for ACPI, it becomes even more
important that the xgene code is refactored into a central location,
and you make use of it. We really need to avoid every ACPI ethernet
driver doing its own thing.

       Thanks
	Andrew

On 2018/10/26 21:12, Andrew Lunn wrote:

On Fri, Oct 26, 2018 at 03:04:25AM +0000, Wang, Dongsheng wrote:

quoted

On 2018/10/26 10:37, Timur Tabi wrote:

quoted

On 10/25/18 9:18 PM, Wang, Dongsheng wrote:

quoted

But when I was reading Documentation/acpi/DSD-properties-rules.txt, my
understanding is we should try to conform to DT bindings. So maybe ACPI
doesn't have such a document, just DT bindings.

There was an attempt to document DSDs, but it was abandoned after a while.

https://github.com/ahs3/dsd

Yes, here's a database concept, and I asked some Intel guys, the answer
I got was there is no such database or document. :(

Hi Dongsheng

If there is no clear documentation for ACPI, it becomes even more
important that the xgene code is refactored into a central location,
and you make use of it. We really need to avoid every ACPI ethernet
driver doing its own thing.

However, without a document specifying MDIO and phy-handle, it is almost
difficult for us to do this. Because maybe the ACPI device or property
corresponding to each platform is different.
Just like APM looks different to us. APM's MDIO adev doesn't describe
the concept of port, and our platform does. Besides, I cannot get the
ACPI table of APM or other manufacturers.
The table of ACPI cannot be obtained from kernel source as easily as DT.
We can't know without a platform to do ACPI dump. Unless some of the
manufacturers have pushed the table to upstream.
So I think we might have a hard time doing this without a document. And
it's likely that this work involves code modifications by BIOS vendors.

Cheers,
Dongsheng

On Mon, Oct 29, 2018 at 02:39:36AM +0000, Wang, Dongsheng wrote:

On 2018/10/26 21:12, Andrew Lunn wrote:

quoted

On Fri, Oct 26, 2018 at 03:04:25AM +0000, Wang, Dongsheng wrote:

quoted

On 2018/10/26 10:37, Timur Tabi wrote:

quoted

On 10/25/18 9:18 PM, Wang, Dongsheng wrote:

quoted

But when I was reading Documentation/acpi/DSD-properties-rules.txt, my
understanding is we should try to conform to DT bindings. So maybe ACPI
doesn't have such a document, just DT bindings.

There was an attempt to document DSDs, but it was abandoned after a while.

https://github.com/ahs3/dsd

Yes, here's a database concept, and I asked some Intel guys, the answer
I got was there is no such database or document. :(

Hi Dongsheng

If there is no clear documentation for ACPI, it becomes even more
important that the xgene code is refactored into a central location,
and you make use of it. We really need to avoid every ACPI ethernet
driver doing its own thing.

However, without a document specifying MDIO and phy-handle, it is almost
difficult for us to do this. Because maybe the ACPI device or property
corresponding to each platform is different.
Just like APM looks different to us. APM's MDIO adev doesn't describe
the concept of port, and our platform does. Besides, I cannot get the
ACPI table of APM or other manufacturers.
The table of ACPI cannot be obtained from kernel source as easily as DT.
We can't know without a platform to do ACPI dump. Unless some of the
manufacturers have pushed the table to upstream.
So I think we might have a hard time doing this without a document. And
it's likely that this work involves code modifications by BIOS vendors.

Hi Dongsheng

There are two different options here.

1) Everybody does their own thing, ignoring what everybody else has
done, and invents their own wheel. There is no shared code, no shared
description, everybody has their own bugs, etc. ACPI as a standard is
pointless for Ethernet MDIOs and PHYs because it is not a standard,
everybody does something different.

2) Somebody takes the time to design a concept for Ethernet PHYs and
MDIO busses using ACPI. They implement the common code, try to modify
any existing users if possible, and submit the whole thing to become
part of ACPI 6.3.

I would really prefer we go the second route here. It is more initial
effort, but in the long run, everybody benefits.

	Andrew

On 2018/10/29 20:40, Andrew Lunn wrote:

On Mon, Oct 29, 2018 at 02:39:36AM +0000, Wang, Dongsheng wrote:

quoted

On 2018/10/26 21:12, Andrew Lunn wrote:

quoted

On Fri, Oct 26, 2018 at 03:04:25AM +0000, Wang, Dongsheng wrote:

quoted

On 2018/10/26 10:37, Timur Tabi wrote:

quoted

On 10/25/18 9:18 PM, Wang, Dongsheng wrote:

quoted

But when I was reading Documentation/acpi/DSD-properties-rules.txt, my
understanding is we should try to conform to DT bindings. So maybe ACPI
doesn't have such a document, just DT bindings.

There was an attempt to document DSDs, but it was abandoned after a while.

https://github.com/ahs3/dsd

Yes, here's a database concept, and I asked some Intel guys, the answer
I got was there is no such database or document. :(

Hi Dongsheng

If there is no clear documentation for ACPI, it becomes even more
important that the xgene code is refactored into a central location,
and you make use of it. We really need to avoid every ACPI ethernet
driver doing its own thing.

However, without a document specifying MDIO and phy-handle, it is almost
difficult for us to do this. Because maybe the ACPI device or property
corresponding to each platform is different.
Just like APM looks different to us. APM's MDIO adev doesn't describe
the concept of port, and our platform does. Besides, I cannot get the
ACPI table of APM or other manufacturers.
The table of ACPI cannot be obtained from kernel source as easily as DT.
We can't know without a platform to do ACPI dump. Unless some of the
manufacturers have pushed the table to upstream.
So I think we might have a hard time doing this without a document. And
it's likely that this work involves code modifications by BIOS vendors.

Hi Dongsheng

There are two different options here.

1) Everybody does their own thing, ignoring what everybody else has
done, and invents their own wheel. There is no shared code, no shared
description, everybody has their own bugs, etc. ACPI as a standard is
pointless for Ethernet MDIOs and PHYs because it is not a standard,
everybody does something different.

2) Somebody takes the time to design a concept for Ethernet PHYs and
MDIO busses using ACPI. They implement the common code, try to modify
any existing users if possible, and submit the whole thing to become
part of ACPI 6.3.

I would really prefer we go the second route here. It is more initial
effort, but in the long run, everybody benefits.

Yes, I also would like the second one. I will reply in a few days.

Cheers,
Dongsheng

	Andrew

Originally I just push "phy-handle" support for ACPI on the QCOM QDF2400
platform. After some discussion and following Andrew's advice, I send
out with a generic version of ACPI.

Current there is no clear documentation about MDIO/PHY for ACPI, so when
I reading some documents about ACPI [1], I think we just need to reuse the
DT binding in the ACPI.[2]. However, this series of patches are not
fully compatible with all contents specified in DT binding.

The most important thing about this iseries is link the phy device and
fwnode of acpi. Besides, we need to carry out bus scan at the mdio
register. Therefore, I am not compatible with more DT binding properties
in this series of patches. More support will be in the follow-up patches
support, or some people do the support.

Example:
Based on ACPI doc:
    Documentation/acpi/dsd/data-node-references.txt
    Documentation/acpi/dsd/graph.txt
With _DSD device properties we can finally do this:
    Device (MDIO) {
        Name (_DSD, Package () {
            ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
            Package () { Package () { "ethernet-phy@0", PHY0 }, }
        })
        Name (PHY0, Package() {
            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
            Package () { Package () { "reg", 0x0 }, }
        })
    }

    Device (MACO) {
        Name (_DSD, Package () {
            ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
            Package () { Package () { "phy-handle", \_SB.MDIO, "ethernet-phy@0" }, }
        })
    }

Tested: QDF2400 (ACPI), buildin/insmod/rmmod

[1]:
Documentation/acpi/dsd/data-node-references.txt
Documentation/acpi/dsd/graph.txt

[2]:
Documentation/devicetree/bindings/phy/phy-bindings.txt
https://lore.kernel.org/patchwork/patch/597296/

Wang Dongsheng (3):
  acpi: Add acpi mdio support code
  net: qcom/emac: split phy_config to mdio bus create and get phy device
  net: qcom/emac: add phy-handle support for ACPI

 drivers/acpi/Kconfig                          |   6 +
 drivers/acpi/Makefile                         |   1 +
 drivers/acpi/acpi_mdio.c                      | 167 ++++++++++++++++++
 drivers/net/ethernet/qualcomm/emac/emac-mac.c |  19 +-
 drivers/net/ethernet/qualcomm/emac/emac-phy.c | 142 ++++++++++-----
 drivers/net/phy/mdio_bus.c                    |   3 +
 include/linux/acpi_mdio.h                     |  82 +++++++++
 7 files changed, 369 insertions(+), 51 deletions(-)
 create mode 100644 drivers/acpi/acpi_mdio.c
 create mode 100644 include/linux/acpi_mdio.h

-- 
2.18.0

Add support for parsing the ACPI data node for PHY devices on an MDIO bus.
The current implementation depend on mdio bus scan.
With _DSD device properties we can finally do this:

    Device (MDIO) {
        Name (_DSD, Package () {
            ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
            Package () { Package () { "ethernet-phy@0", PHY0 }, }
        })
        Name (PHY0, Package() {
            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
            Package () { Package () { "reg", 0x0 }, }
        })
    }

    Device (MACO) {
        Name (_DSD, Package () {
            ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
            Package () { Package () { "phy-handle", \_SB.MDIO, "ethernet-phy@0" }, }
        })
    }

Documentations:
    The DT "phy-handle" binding that we reuse for ACPI is documented in
    Documentation/devicetree/bindings/phy/phy-bindings.txt

    Documentation/acpi/dsd/data-node-references.txt
    Documentation/acpi/dsd/graph.txt

Signed-off-by: Wang Dongsheng <redacted>
---
 drivers/acpi/Kconfig       |   6 ++
 drivers/acpi/Makefile      |   1 +
 drivers/acpi/acpi_mdio.c   | 167 +++++++++++++++++++++++++++++++++++++
 drivers/net/phy/mdio_bus.c |   3 +
 include/linux/acpi_mdio.h  |  82 ++++++++++++++++++
 5 files changed, 259 insertions(+)

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 9705fc986da9..0fefa3410ce9 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig

@@ -252,6 +252,12 @@ config ACPI_PROCESSOR_IDLE
 config ACPI_MCFG
 	bool
 
+config ACPI_MDIO
+	def_tristate PHYLIB
+	depends on PHYLIB
+	help
+	  ACPI MDIO bus (Ethernet PHY) accessors
+
 config ACPI_CPPC_LIB
 	bool
 	depends on ACPI_PROCESSOR

diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 6d59aa109a91..ec7461a064fc 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile

@@ -41,6 +41,7 @@ acpi-y				+= ec.o
 acpi-$(CONFIG_ACPI_DOCK)	+= dock.o
 acpi-y				+= pci_root.o pci_link.o pci_irq.o
 obj-$(CONFIG_ACPI_MCFG)		+= pci_mcfg.o
+acpi-$(CONFIG_ACPI_MDIO)	+= acpi_mdio.o
 acpi-y				+= acpi_lpss.o acpi_apd.o
 acpi-y				+= acpi_platform.o
 acpi-y				+= acpi_pnp.o

diff --git a/drivers/acpi/acpi_mdio.c b/drivers/acpi/acpi_mdio.c
new file mode 100644
index 000000000000..293bf9a63197
--- /dev/null
+++ b/drivers/acpi/acpi_mdio.c

@@ -0,0 +1,167 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Lots of code in this file is copy from drivers/of/of_mdio.c
+// Copyright (c) 2018 Huaxintong Semiconductor Technology Co., Ltd.
+
+#include <linux/acpi.h>
+#include <linux/acpi_mdio.h>
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <linux/netdevice.h>
+#include <linux/err.h>
+#include <linux/phy.h>
+#include <linux/phy_fixed.h>
+
+/* Helper function for acpi_phy_find_device */
+static int phy_match(struct device *dev, void *fwnode)
+{
+	return dev->fwnode == fwnode;
+}
+
+/**
+ * acpi_phy_find_device - Give a PHY fwnode, find the phy_device
+ * @fwnode: Pointer to the phy's acpi data node
+ *
+ * If successful, returns a pointer to the phy_device with the embedded
+ * struct device refcount incremented by one, or NULL on failure.
+ */
+struct phy_device *acpi_phy_find_device(struct fwnode_handle *fwnode)
+{
+	struct device *d;
+	struct mdio_device *mdiodev;
+
+	if (!fwnode)
+		return NULL;
+
+	d = bus_find_device(&mdio_bus_type, NULL, fwnode, phy_match);
+	if (d) {
+		mdiodev = to_mdio_device(d);
+		if (mdiodev->flags & MDIO_DEVICE_FLAG_PHY)
+			return to_phy_device(d);
+		put_device(d);
+	}
+	return NULL;
+}
+EXPORT_SYMBOL(acpi_phy_find_device);
+
+static int do_acpi_mdiodev_match(struct fwnode_handle *fwnode,
+				 struct mdio_device *mdiodev)
+{
+	struct device *dev = &mdiodev->dev;
+	struct fwnode_handle *child_node;
+	int addr;
+	int ret;
+
+	fwnode_for_each_child_node(fwnode, child_node) {
+		do {
+			addr = acpi_mdio_parse_addr(dev, child_node);
+			if (addr < 0)
+				break;
+
+			if (mdiodev->addr != addr)
+				break;
+
+			dev->fwnode = child_node;
+			return 0;
+		} while (0);
+
+		/* Walk hierarchical extension data nodes */
+		ret = do_acpi_mdiodev_match(child_node, mdiodev);
+		if (!ret)
+			return 0;
+	}
+
+	return -ENODEV;
+}
+
+/* Walk the list of subnodes of a mdio bus and look for a node that
+ * matches the mdio device's address with its 'reg' property. If
+ * found, set the fwnode pointer for the mdio device.
+ */
+void acpi_mdiobus_link_mdiodev(struct mii_bus *bus,
+			       struct mdio_device *mdiodev)
+{
+	struct device *dev = &mdiodev->dev;
+
+	if (dev->fwnode || !bus->dev.fwnode)
+		return;
+
+	if (!has_acpi_companion(&bus->dev))
+		return;
+
+	do_acpi_mdiodev_match(bus->dev.fwnode, mdiodev);
+}
+
+/**
+ * acpi_phy_connect - Connect to the phy
+ * @dev: pointer to net_device claiming the phy
+ * @fwnode: Pointer to ACPI data node for the PHY
+ * @hndlr: Link state callback for the network device
+ * @flags: flags to pass to the PHY
+ * @iface: PHY data interface type
+ *
+ * If successful, returns a pointer to the phy_device with the embedded
+ * struct device refcount incremented by one, or NULL on failure. The
+ * refcount must be dropped by calling phy_disconnect() or phy_detach().
+ */
+struct phy_device *acpi_phy_connect(struct net_device *dev,
+				    struct fwnode_handle *fwnode,
+				    void (*hndlr)(struct net_device *),
+				    u32 flags,
+				    phy_interface_t iface)
+{
+	struct phy_device *phy = acpi_phy_find_device(fwnode);
+	int ret;
+
+	if (!phy)
+		return NULL;
+
+	phy->dev_flags = flags;
+
+	ret = phy_connect_direct(dev, phy, hndlr, iface);
+
+	/* refcount is held by phy_connect_direct() on success */
+	put_device(&phy->mdio.dev);
+
+	return ret ? NULL : phy;
+}
+EXPORT_SYMBOL(acpi_phy_connect);
+
+static int acpi_mdio_node_verify(struct fwnode_handle *fwnode)
+{
+	return is_acpi_device_node(fwnode) ? 0 : -ENODEV;
+}
+
+static int fwnode_mdiobus_verify_node(struct fwnode_handle *fwnode)
+{
+	if (!is_acpi_node(fwnode))
+		return -ENODEV;
+	return acpi_mdio_node_verify(fwnode);
+}
+
+/**
+ * acpi_mdiobus_register - Register mii_bus and create PHYs
+ * @mdio: pointer to mii_bus structure
+ * @fwnode: pointer to fw_node of MDIO bus.
+ *
+ * This function registers the mii_bus structure and scan the phy_devices
+ * for each child node of @fwnode.
+ */
+int acpi_mdiobus_register(struct mii_bus *mdio, struct fwnode_handle *fwnode)
+{
+	int ret;
+
+	if (!fwnode)
+		return mdiobus_register(mdio);
+
+	ret = fwnode_mdiobus_verify_node(fwnode);
+	if (ret)
+		return ret;
+
+	/* Scan PHYs on MDIO bus */
+	mdio->phy_mask = 0;
+	mdio->dev.fwnode = fwnode;
+
+	/* Register the MDIO bus */
+	return mdiobus_register(mdio);
+}
+EXPORT_SYMBOL(acpi_mdiobus_register);

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 2e59a8419b17..d7bca2145d0f 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c

@@ -13,6 +13,7 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/acpi_mdio.h>
 #include <linux/kernel.h>
 #include <linux/string.h>
 #include <linux/errno.h>

@@ -516,6 +517,8 @@ struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr)
 	 * in the bus node, and set the of_node pointer in this case.
 	 */
 	of_mdiobus_link_mdiodev(bus, &phydev->mdio);
+	/* Link the phy device with ACPI phy fwnode. */
+	acpi_mdiobus_link_mdiodev(bus, &phydev->mdio);
 
 	err = phy_device_register(phydev);
 	if (err) {

diff --git a/include/linux/acpi_mdio.h b/include/linux/acpi_mdio.h
new file mode 100644
index 000000000000..1a4a30258ebc
--- /dev/null
+++ b/include/linux/acpi_mdio.h

@@ -0,0 +1,82 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+// Copyright (c) 2018 Huaxintong Semiconductor Technology Co., Ltd.
+
+#ifndef __LINUX_ACPI_MDIO_H
+#define __LINUX_ACPI_MDIO_H
+
+#include <linux/acpi.h>
+#include <linux/device.h>
+#include <linux/phy.h>
+#include <linux/property.h>
+
+#if IS_ENABLED(CONFIG_ACPI_MDIO)
+static inline int acpi_mdio_parse_addr(struct device *dev,
+				       const struct fwnode_handle *fwnode)
+{
+	u32 addr;
+
+	if (!is_acpi_data_node(fwnode))
+		return -ENODEV;
+
+	if (!fwnode_property_present(fwnode, "reg"))
+		return -ENODEV;
+
+	if (fwnode_property_read_u32(fwnode, "reg", &addr)) {
+		dev_err(dev, "Invalid PHY address\n");
+		return -ENODEV;
+	}
+
+	/* A PHY must have a reg property in the range [0-31] */
+	if (addr >= PHY_MAX_ADDR) {
+		dev_err(dev, "PHY address %i is too large\n", addr);
+		return -EINVAL;
+	}
+
+	return addr;
+}
+
+struct phy_device *acpi_phy_find_device(struct fwnode_handle *fwnode);
+struct phy_device *acpi_phy_connect(struct net_device *dev,
+				    struct fwnode_handle *fwnode,
+				    void (*hndlr)(struct net_device *),
+				    u32 flags, phy_interface_t iface);
+int acpi_mdiobus_register(struct mii_bus *mdio, struct fwnode_handle *fwnode);
+void acpi_mdiobus_link_mdiodev(struct mii_bus *bus,
+			       struct mdio_device *mdiodev);
+#else
+static inline int acpi_mdio_parse_addr(struct device *dev,
+				       const struct fwnode_handle *fwnode)
+{
+	return -EINVAL;
+}
+
+static inline struct phy_device *
+acpi_phy_find_device(struct fwnode_handle *fwnode)
+{
+	return NULL;
+}
+
+static inline struct phy_device *
+acpi_phy_connect(struct net_device *dev, struct fwnode_handle *fwnode,
+		 void (*hndlr)(struct net_device *), u32 flags,
+		 phy_interface_t iface)
+{
+	return NULL;
+}
+
+static inline int acpi_mdiobus_register(struct mii_bus *mdio,
+					struct fwnode_handle *fwnode)
+{
+	return -ENODEV;
+}
+
+static inline void
+acpi_mdiobus_link_mdiodev(struct mii_bus *bus, struct mdio_device *mdiodev) { }
+#endif
+
+static inline struct fwnode_handle *acpi_get_phy_node(struct phy_device *phydev)
+{
+	return !phydev ? NULL : phydev->mdio.dev.fwnode;
+}
+
+#endif /* __LINUX_ACPI_MDIO_H */

-- 
2.18.0

This patch separate emac_mdio_bus_create and emac_get_phydev from
emac_phy_config, and do some codes clean.

Signed-off-by: Wang Dongsheng <redacted>
---
 drivers/net/ethernet/qualcomm/emac/emac-phy.c | 96 +++++++++++--------
 1 file changed, 56 insertions(+), 40 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/emac/emac-phy.c b/drivers/net/ethernet/qualcomm/emac/emac-phy.c
index 53dbf1e163a8..8289fdda4be7 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-phy.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac-phy.c

@@ -96,15 +96,15 @@ static int emac_mdio_write(struct mii_bus *bus, int addr, int regnum, u16 val)
 	return 0;
 }
 
-/* Configure the MDIO bus and connect the external PHY */
-int emac_phy_config(struct platform_device *pdev, struct emac_adapter *adpt)
+static int emac_mdio_bus_create(struct platform_device *pdev,
+				struct emac_adapter *adpt)
 {
 	struct device_node *np = pdev->dev.of_node;
 	struct mii_bus *mii_bus;
 	int ret;
 
 	/* Create the mii_bus object for talking to the MDIO bus */
-	adpt->mii_bus = mii_bus = devm_mdiobus_alloc(&pdev->dev);
+	mii_bus = devm_mdiobus_alloc(&pdev->dev);
 	if (!mii_bus)
 		return -ENOMEM;
 

@@ -115,50 +115,66 @@ int emac_phy_config(struct platform_device *pdev, struct emac_adapter *adpt)
 	mii_bus->parent = &pdev->dev;
 	mii_bus->priv = adpt;
 
-	if (has_acpi_companion(&pdev->dev)) {
-		u32 phy_addr;
-
-		ret = mdiobus_register(mii_bus);
-		if (ret) {
-			dev_err(&pdev->dev, "could not register mdio bus\n");
-			return ret;
-		}
-		ret = device_property_read_u32(&pdev->dev, "phy-channel",
-					       &phy_addr);
-		if (ret)
-			/* If we can't read a valid phy address, then assume
-			 * that there is only one phy on this mdio bus.
-			 */
-			adpt->phydev = phy_find_first(mii_bus);
-		else
-			adpt->phydev = mdiobus_get_phy(mii_bus, phy_addr);
-
-		/* of_phy_find_device() claims a reference to the phydev,
-		 * so we do that here manually as well. When the driver
-		 * later unloads, it can unilaterally drop the reference
-		 * without worrying about ACPI vs DT.
-		 */
-		if (adpt->phydev)
-			get_device(&adpt->phydev->mdio.dev);
-	} else {
-		struct device_node *phy_np;
-
-		ret = of_mdiobus_register(mii_bus, np);
-		if (ret) {
-			dev_err(&pdev->dev, "could not register mdio bus\n");
-			return ret;
-		}
+	ret = of_mdiobus_register(mii_bus, has_acpi_companion(&pdev->dev) ?
+				  NULL : np);
+	if (ret)
+		dev_err(&pdev->dev, "Could not register mdio bus\n");
+
+	adpt->mii_bus = ret ? NULL : mii_bus;
+	return ret;
+}
+
+static int emac_get_phydev(struct platform_device *pdev,
+			   struct emac_adapter *adpt)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct mii_bus *bus = adpt->mii_bus;
+	struct device_node *phy_np;
+	struct phy_device *phydev;
 
+	u32 phy_addr;
+	int ret;
+
+	if (!has_acpi_companion(&pdev->dev)) {
 		phy_np = of_parse_phandle(np, "phy-handle", 0);
 		adpt->phydev = of_phy_find_device(phy_np);
 		of_node_put(phy_np);
+		return adpt->phydev ? 0 : -ENODEV;
 	}
 
-	if (!adpt->phydev) {
-		dev_err(&pdev->dev, "could not find external phy\n");
-		mdiobus_unregister(mii_bus);
+	ret = device_property_read_u32(&pdev->dev, "phy-channel",
+				       &phy_addr);
+	/* If we can't read a valid phy address, then assume
+	 * that there is only one phy on this mdio bus.
+	 */
+	phydev = ret ? phy_find_first(bus) : mdiobus_get_phy(bus, phy_addr);
+	if (!phydev)
 		return -ENODEV;
-	}
 
+	/* of_phy_find_device() claims a reference to the phydev,
+	 * so we do that here manually as well. When the driver
+	 * later unloads, it can unilaterally drop the reference
+	 * without worrying about ACPI vs DT.
+	 */
+	get_device(&phydev->mdio.dev);
+	adpt->phydev = phydev;
 	return 0;
 }
+
+/* Configure the MDIO bus and connect the external PHY */
+int emac_phy_config(struct platform_device *pdev, struct emac_adapter *adpt)
+{
+	int ret;
+
+	ret = emac_mdio_bus_create(pdev, adpt);
+	if (ret)
+		return ret;
+
+	ret = emac_get_phydev(pdev, adpt);
+	if (ret) {
+		dev_err(&pdev->dev, "Could not find external phy\n");
+		mdiobus_unregister(adpt->mii_bus);
+	}
+
+	return ret;
+}

-- 
2.18.0

Use "phy-handle" to point to an internal MDIO device port.

Signed-off-by: Wang Dongsheng <redacted>
---
 drivers/net/ethernet/qualcomm/emac/emac-mac.c | 19 ++---
 drivers/net/ethernet/qualcomm/emac/emac-phy.c | 78 ++++++++++++++-----
 2 files changed, 70 insertions(+), 27 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/emac/emac-mac.c b/drivers/net/ethernet/qualcomm/emac/emac-mac.c
index 031f6e6ee9c1..74cfe7b95bb3 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-mac.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac-mac.c

@@ -13,6 +13,7 @@
 /* Qualcomm Technologies, Inc. EMAC Ethernet Controller MAC layer support
  */
 
+#include <linux/acpi_mdio.h>
 #include <linux/tcp.h>
 #include <linux/ip.h>
 #include <linux/ipv6.h>

@@ -939,28 +940,28 @@ static void emac_adjust_link(struct net_device *netdev)
 int emac_mac_up(struct emac_adapter *adpt)
 {
 	struct net_device *netdev = adpt->netdev;
-	int ret;
+	struct phy_device *phydev = adpt->phydev;
+	struct fwnode_handle *phy_np = acpi_get_phy_node(phydev);
 
 	emac_mac_rx_tx_ring_reset_all(adpt);
 	emac_mac_config(adpt);
 	emac_mac_rx_descs_refill(adpt, &adpt->rx_q);
 
-	adpt->phydev->irq = PHY_POLL;
-	ret = phy_connect_direct(netdev, adpt->phydev, emac_adjust_link,
-				 PHY_INTERFACE_MODE_SGMII);
-	if (ret) {
+	phydev->irq = PHY_POLL;
+	phydev = acpi_phy_connect(netdev, phy_np, emac_adjust_link,
+				  0, PHY_INTERFACE_MODE_SGMII);
+	if (!phydev) {
 		netdev_err(adpt->netdev, "could not connect phy\n");
-		return ret;
+		return -ENODEV;
 	}
 
-	phy_attached_print(adpt->phydev, NULL);
+	phy_attached_print(phydev, NULL);
 
 	/* enable mac irq */
 	writel((u32)~DIS_INT, adpt->base + EMAC_INT_STATUS);
 	writel(adpt->irq.mask, adpt->base + EMAC_INT_MASK);
 
-	phy_start(adpt->phydev);
-
+	phy_start(phydev);
 	napi_enable(&adpt->rx_q.napi);
 	netif_start_queue(netdev);