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

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

[Wang Dongsheng]

Share the mii_bus for others MAC device because QDF2400 emac
include MDIO, and the motherboard has more than one PHY connected
to an MDIO bus.

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

Wang Dongsheng (2):
  dt-bindings: net: qcom: Add binding for shared mdio bus
  net: qcom/emac: add shared mdio bus support

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

-- 
2.18.0

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

[Wang Dongsheng]

Share the mii_bus for others MAC device because QDF2400 emac
include MDIO, and the motherboard has more than one PHY connected
to an MDIO bus.

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

Wang Dongsheng (2):
  dt-bindings: net: qcom: Add binding for shared mdio bus
  net: qcom/emac: add shared mdio bus support

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

-- 
2.18.0

[PATCH 1/2] dt-bindings: net: qcom: Add binding for shared mdio bus

[Wang Dongsheng]

This property copy from "ibm,emac.txt" to describe a shared MIDO bus.
Since QDF2400 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 <dongsheng.wang@hxt-semitech.com>
---
 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

[PATCH 2/2] net: qcom/emac: add shared mdio bus support

[Wang Dongsheng]

Share the mii_bus for others MAC device because QDF2400 emac
include MDIO, and the motherboard has more than one PHY connected
to an MDIO bus.

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

Signed-off-by: Wang Dongsheng <dongsheng.wang@hxt-semitech.com>
---
 drivers/net/ethernet/qualcomm/emac/emac-phy.c | 211 ++++++++++++++----
 drivers/net/ethernet/qualcomm/emac/emac.c     |   7 +-
 2 files changed, 174 insertions(+), 44 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/emac/emac-phy.c b/drivers/net/ethernet/qualcomm/emac/emac-phy.c
index 53dbf1e163a8..327362f6b673 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-phy.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac-phy.c
@@ -13,6 +13,7 @@
 /* Qualcomm Technologies, Inc. EMAC PHY Controller driver.
  */
 
+#include <linux/of_platform.h>
 #include <linux/of_mdio.h>
 #include <linux/phy.h>
 #include <linux/iopoll.h>
@@ -96,15 +97,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 __do_emac_mido_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,177 @@ 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 = of_mdiobus_register(mii_bus, has_acpi_companion(&pdev->dev) ?
+				  NULL : np);
+	if (!ret) {
+		adpt->mii_bus = mii_bus;
+		return 0;
+	}
 
-		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;
-		}
+	dev_err(&pdev->dev, "Could not register mdio bus\n");
+	return ret;
+}
 
-		phy_np = of_parse_phandle(np, "phy-handle", 0);
-		adpt->phydev = of_phy_find_device(phy_np);
-		of_node_put(phy_np);
+static int acpi_device_match(struct device *dev, void *fwnode)
+{
+	return dev->fwnode == fwnode;
+}
+
+static int emac_acpi_get_shared_bus(struct platform_device *pdev,
+				    struct mii_bus **bus)
+{
+	acpi_handle shared_handle;
+	struct acpi_device *adev;
+	const union acpi_object *obj;
+	union acpi_object *obj_e;
+	struct device *shared_dev;
+	struct net_device *shared_netdev;
+	struct emac_adapter *shared_adpt;
+	int ret;
+
+	adev = ACPI_COMPANION(&pdev->dev);
+	if (!adev)
+		return -ENODEV;
+
+	ret = acpi_dev_get_property(adev, "mdio-device", ACPI_TYPE_ANY, &obj);
+	if (ret) {
+		dev_err(&pdev->dev, "Missing mdio-device property\n");
+		return -ENODEV;
 	}
 
-	if (!adpt->phydev) {
-		dev_err(&pdev->dev, "could not find external phy\n");
-		mdiobus_unregister(mii_bus);
+	if (obj->package.count != 1)
+		return -ENODEV;
+
+	obj_e = &obj->package.elements[0];
+	if (obj_e->type != ACPI_TYPE_LOCAL_REFERENCE)
+		return -ENODEV;
+
+	if (obj_e->reference.actual_type != ACPI_TYPE_DEVICE)
+		return -ENODEV;
+
+	shared_handle = obj_e->reference.handle;
+	if (!shared_handle || acpi_bus_get_device(shared_handle, &adev))
+		return -ENODEV;
+
+	shared_dev = bus_find_device(&platform_bus_type, NULL,
+				     acpi_fwnode_handle(adev),
+				     acpi_device_match);
+	if (!shared_dev)
+		return -EPROBE_DEFER;
+
+	shared_netdev = dev_get_drvdata(shared_dev);
+	if (!shared_netdev)
+		return -EPROBE_DEFER;
+
+	shared_adpt = netdev_priv(shared_netdev);
+	if (!shared_adpt->mii_bus)
+		return -EPROBE_DEFER;
+
+	*bus = shared_adpt->mii_bus;
+	return 0;
+}
+
+static int emac_of_get_shared_bus(struct platform_device *pdev,
+				  struct mii_bus **bus)
+{
+	struct device_node *shared_node;
+	struct platform_device *shared_pdev;
+	struct net_device *shared_netdev;
+	struct emac_adapter *shared_adpt;
+	struct device_node *np = pdev->dev.of_node;
+
+	const phandle *prop;
+
+	prop = of_get_property(np, "mdio-device", NULL);
+	if (!prop) {
+		dev_err(&pdev->dev, "Missing mdio-device property\n");
 		return -ENODEV;
 	}
 
+	shared_node = of_find_node_by_phandle(*prop);
+	if (!shared_node)
+		return -ENODEV;
+
+	shared_pdev = of_find_device_by_node(shared_node);
+	if (!shared_pdev)
+		return -ENODEV;
+
+	shared_netdev = dev_get_drvdata(&shared_pdev->dev);
+	if (!shared_netdev)
+		return -EPROBE_DEFER;
+
+	shared_adpt = netdev_priv(shared_netdev);
+	if (!shared_adpt->mii_bus)
+		return -EPROBE_DEFER;
+
+	*bus = shared_adpt->mii_bus;
 	return 0;
 }
+
+static int __do_get_emac_mido_shared_bus(struct platform_device *pdev,
+					 struct emac_adapter *adpt)
+{
+	int ret = -ENODEV;
+
+	if (IS_ENABLED(CONFIG_ACPI)) {
+		ret = emac_acpi_get_shared_bus(pdev, &adpt->mii_bus);
+		if (adpt->mii_bus || ret == -EPROBE_DEFER)
+			return ret;
+	}
+
+	if (IS_ENABLED(CONFIG_OF)) {
+		ret = emac_of_get_shared_bus(pdev, &adpt->mii_bus);
+		if (adpt->mii_bus || ret == -EPROBE_DEFER)
+			return ret;
+	}
+
+	return ret;
+}
+
+static int emac_mdio_bus_create(struct platform_device *pdev,
+				struct emac_adapter *adpt)
+{
+	bool shared_mdio;
+
+	shared_mdio = device_property_read_bool(&pdev->dev, "mdio-device");
+	if (shared_mdio)
+		return __do_get_emac_mido_shared_bus(pdev, adpt);
+
+	return __do_emac_mido_bus_create(pdev, adpt);
+}
+
+/* Configure the MDIO bus and connect the external PHY */
+int emac_phy_config(struct platform_device *pdev, struct emac_adapter *adpt)
+{
+	struct device *dev = &pdev->dev;
+	u32 phy_addr = PHY_MAX_ADDR;
+	int ret;
+
+	ret = emac_mdio_bus_create(pdev, adpt);
+	if (ret)
+		return ret;
+
+	ret = device_property_read_u32(dev,
+				       has_acpi_companion(dev) ?
+				       "phy-channel" : "phy-handle",
+				       &phy_addr);
+	if (ret || phy_addr == PHY_MAX_ADDR)
+		/* 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(adpt->mii_bus);
+	else
+		adpt->phydev = mdiobus_get_phy(adpt->mii_bus, phy_addr);
+
+	if (adpt->phydev) {
+		get_device(&adpt->phydev->mdio.dev);
+		return 0;
+	}
+
+	dev_err(dev, "Could not find external phy\n");
+	/* Only the bus creator can unregister mdio bus */
+	if (dev == adpt->mii_bus->parent)
+		mdiobus_unregister(adpt->mii_bus);
+
+	return -ENODEV;
+}
diff --git a/drivers/net/ethernet/qualcomm/emac/emac.c b/drivers/net/ethernet/qualcomm/emac/emac.c
index 2a0cbc535a2e..11d0fe795616 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac.c
@@ -738,7 +738,8 @@ static int emac_probe(struct platform_device *pdev)
 
 static int emac_remove(struct platform_device *pdev)
 {
-	struct net_device *netdev = dev_get_drvdata(&pdev->dev);
+	struct device *dev = &pdev->dev;
+	struct net_device *netdev = dev_get_drvdata(dev);
 	struct emac_adapter *adpt = netdev_priv(netdev);
 
 	unregister_netdev(netdev);
@@ -747,7 +748,9 @@ static int emac_remove(struct platform_device *pdev)
 	emac_clks_teardown(adpt);
 
 	put_device(&adpt->phydev->mdio.dev);
-	mdiobus_unregister(adpt->mii_bus);
+	/* Only the bus creator can unregister mdio bus */
+	if (dev == adpt->mii_bus->parent)
+		mdiobus_unregister(adpt->mii_bus);
 	free_netdev(netdev);
 
 	if (adpt->phy.digital)
-- 
2.18.0

Re: [PATCH 0/2] net: qcom/emac: add shared mdio bus support

[Andrew Lunn]

On Thu, Sep 13, 2018 at 05:04:51PM +0800, Wang Dongsheng wrote:
> Share the mii_bus for others MAC device because QDF2400 emac
> include MDIO, and the motherboard has more than one PHY connected
> to an MDIO bus.
> 
> Tested: QDF2400 (ACPI), buildin/insmod/rmmod
> 
> Wang Dongsheng (2):
>   dt-bindings: net: qcom: Add binding for shared mdio bus

Hi Wang

Patch #1 did not seem to make it to netdev.

I probably have some comments on the binding...

  Andrew

Re: [PATCH 2/2] net: qcom/emac: add shared mdio bus support

[Andrew Lunn]

On Thu, Sep 13, 2018 at 05:04:53PM +0800, Wang Dongsheng wrote:
> Share the mii_bus for others MAC device because QDF2400 emac
> include MDIO, and the motherboard has more than one PHY connected
> to an MDIO bus.
> 
> Tested: QDF2400 (ACPI), buildin/insmod/rmmod
> 
> Signed-off-by: Wang Dongsheng <dongsheng.wang@hxt-semitech.com>

Hi Wang

This is a pretty big patch, and is hard to review. Could you try to
break it up into a number of smaller patches. You could for example
first refactor emacs_phy_config(), without making any functional
changes. Then add the sharing. Maybe do OF an ACPI in different
patches?

Thanks
	 Andrew

Re: [PATCH 1/2] dt-bindings: net: qcom: Add binding for shared mdio bus

[Timur Tabi]

On 9/13/18 4:04 AM, Wang Dongsheng wrote:
> This property copy from "ibm,emac.txt" to describe a shared MIDO bus.
> Since QDF2400 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<dongsheng.wang@hxt-semitech.com>

The QDF2400 does not support device trees, so please don't talk about DT 
support in the context of the QDF2400.

Re: [PATCH 2/2] net: qcom/emac: add shared mdio bus support

[Timur Tabi]

On 9/13/18 7:42 AM, Andrew Lunn wrote:
> This is a pretty big patch, and is hard to review. Could you try to
> break it up into a number of smaller patches. You could for example
> first refactor emacs_phy_config(), without making any functional
> changes. Then add the sharing. Maybe do OF an ACPI in different
> patches?

Yes, please.

Re: [PATCH 1/2] dt-bindings: net: qcom: Add binding for shared mdio bus

[Wang, Dongsheng]

On 9/13/2018 11:02 PM, Timur Tabi wrote:
> On 9/13/18 4:04 AM, Wang Dongsheng wrote:
>> This property copy from "ibm,emac.txt" to describe a shared MIDO bus.
>> Since QDF2400 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<dongsheng.wang@hxt-semitech.com>
> The QDF2400 does not support device trees, so please don't talk about DT 
> support in the context of the QDF2400.
>
Ok, remove QDF2400.

s/QDF2400 emac/emac/


Cheers,

-Dongsheng

Re: [PATCH 2/2] net: qcom/emac: add shared mdio bus support

[Wang, Dongsheng]

On 9/13/2018 8:42 PM, Andrew Lunn wrote:
> On Thu, Sep 13, 2018 at 05:04:53PM +0800, Wang Dongsheng wrote:
>> Share the mii_bus for others MAC device because QDF2400 emac
>> include MDIO, and the motherboard has more than one PHY connected
>> to an MDIO bus.
>>
>> Tested: QDF2400 (ACPI), buildin/insmod/rmmod
>>
>> Signed-off-by: Wang Dongsheng <dongsheng.wang@hxt-semitech.com>
> Hi Wang
>
> This is a pretty big patch, and is hard to review. Could you try to
> break it up into a number of smaller patches. You could for example
> first refactor emacs_phy_config(), without making any functional
> changes. Then add the sharing. Maybe do OF an ACPI in different
> patches?
>
> Thanks
> 	 Andrew
>
Ok, thanks.


Cheers

Dongsheng