[PATCH v1 0/2] net: mdio: Add IPQ MDIO reset related function

[PATCH v1 0/2] net: mdio: Add IPQ MDIO reset related function

[Luo Jie]

This patch series add the MDIO reset features, which includes
configuring MDIO clock source frequency and indicating CMN_PLL that
ethernet LDO has been ready, this ethernet LDO is dedicated in the
IPQ5018 platform.

Specify more chipset IPQ40xx, IPQ807x, IPQ60xx and IPQ50xx supported by
this MDIO driver.

The PHY reset with GPIO and PHY reset with reset controller are covered
by the phylib code, so remove the PHY reset related code from the
initial patch series. 

Luo Jie (2):
  net: mdio: Add the reset function for IPQ MDIO driver
  MDIO: Kconfig: Specify more IPQ chipset supported

 drivers/net/mdio/Kconfig        |  3 ++-
 drivers/net/mdio/mdio-ipq4019.c | 48 +++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 1 deletion(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v1 1/2] net: mdio: Add the reset function for IPQ MDIO driver

[Luo Jie]

1. configure the MDIO clock source frequency.
2. the LDO resource is needed to indicate the ethernet LDO ready for CMN_PLL.

Signed-off-by: Luo Jie <luoj@codeaurora.org>
---
 drivers/net/mdio/Kconfig        |  1 +
 drivers/net/mdio/mdio-ipq4019.c | 47 +++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/drivers/net/mdio/Kconfig b/drivers/net/mdio/Kconfig
index 99a6c13a11af..a94d34cc7dc1 100644
--- a/drivers/net/mdio/Kconfig
+++ b/drivers/net/mdio/Kconfig
@@ -169,6 +169,7 @@ config MDIO_OCTEON
 config MDIO_IPQ4019
 	tristate "Qualcomm IPQ4019 MDIO interface support"
 	depends on HAS_IOMEM && OF_MDIO
+	depends on COMMON_CLK
 	help
 	  This driver supports the MDIO interface found in Qualcomm
 	  IPQ40xx series Soc-s.
diff --git a/drivers/net/mdio/mdio-ipq4019.c b/drivers/net/mdio/mdio-ipq4019.c
index 9cd71d896963..b365f13c92ca 100644
--- a/drivers/net/mdio/mdio-ipq4019.c
+++ b/drivers/net/mdio/mdio-ipq4019.c
@@ -11,6 +11,7 @@
 #include <linux/of_mdio.h>
 #include <linux/phy.h>
 #include <linux/platform_device.h>
+#include <linux/clk.h>
 
 #define MDIO_MODE_REG				0x40
 #define MDIO_ADDR_REG				0x44
@@ -31,8 +32,15 @@
 #define IPQ4019_MDIO_TIMEOUT	10000
 #define IPQ4019_MDIO_SLEEP		10
 
+/* MDIO clock source frequency is fixed to 100M */
+#define IPQ_MDIO_CLK_RATE	100000000
+
+#define IPQ_PHY_SET_DELAY_US	100000
+
 struct ipq4019_mdio_data {
 	void __iomem	*membase;
+	void __iomem *eth_ldo_rdy;
+	struct clk *mdio_clk;
 };
 
 static int ipq4019_mdio_wait_busy(struct mii_bus *bus)
@@ -171,10 +179,41 @@ static int ipq4019_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
 	return 0;
 }
 
+static int ipq_mdio_reset(struct mii_bus *bus)
+{
+	struct ipq4019_mdio_data *priv = bus->priv;
+	u32 val;
+	int ret;
+
+	/* To indicate CMN_PLL that ethernet_ldo has been ready if platform resource 1
+	 * is specified in the device tree.
+	 * */
+	if (!IS_ERR(priv->eth_ldo_rdy)) {
+		val = readl(priv->eth_ldo_rdy);
+		val |= BIT(0);
+		writel(val, priv->eth_ldo_rdy);
+		fsleep(IPQ_PHY_SET_DELAY_US);
+	}
+
+	/* Configure MDIO clock source frequency if clock is specified in the device tree */
+	if (!IS_ERR_OR_NULL(priv->mdio_clk)) {
+		ret = clk_set_rate(priv->mdio_clk, IPQ_MDIO_CLK_RATE);
+		if (ret)
+			return ret;
+
+		ret = clk_prepare_enable(priv->mdio_clk);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 static int ipq4019_mdio_probe(struct platform_device *pdev)
 {
 	struct ipq4019_mdio_data *priv;
 	struct mii_bus *bus;
+	struct resource *res;
 	int ret;
 
 	bus = devm_mdiobus_alloc_size(&pdev->dev, sizeof(*priv));
@@ -182,14 +221,22 @@ static int ipq4019_mdio_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	priv = bus->priv;
+	priv->eth_ldo_rdy = IOMEM_ERR_PTR(-EINVAL);
 
 	priv->membase = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(priv->membase))
 		return PTR_ERR(priv->membase);
 
+	priv->mdio_clk = devm_clk_get_optional(&pdev->dev, "gcc_mdio_ahb_clk");
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (res)
+		priv->eth_ldo_rdy = devm_ioremap_resource(&pdev->dev, res);
+
 	bus->name = "ipq4019_mdio";
 	bus->read = ipq4019_mdio_read;
 	bus->write = ipq4019_mdio_write;
+	bus->reset = ipq_mdio_reset;
 	bus->parent = &pdev->dev;
 	snprintf(bus->id, MII_BUS_ID_SIZE, "%s%d", pdev->name, pdev->id);
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v1 2/2] MDIO: Kconfig: Specify more IPQ chipset supported

[Luo Jie]

The IPQ MDIO driver currently supports IPQ40xx, IPQ807x, IPQ60xx and IPQ50xx.

Add the compatible 'qcom,ipq5018-mdio' because of ethernet LDO dedicated
to the IPQ5018 platform.

Signed-off-by: Luo Jie <luoj@codeaurora.org>
---
 drivers/net/mdio/Kconfig        | 2 +-
 drivers/net/mdio/mdio-ipq4019.c | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/mdio/Kconfig b/drivers/net/mdio/Kconfig
index a94d34cc7dc1..6da1fcb25847 100644
--- a/drivers/net/mdio/Kconfig
+++ b/drivers/net/mdio/Kconfig
@@ -172,7 +172,7 @@ config MDIO_IPQ4019
 	depends on COMMON_CLK
 	help
 	  This driver supports the MDIO interface found in Qualcomm
-	  IPQ40xx series Soc-s.
+	  IPQ40xx, IPQ60xx, IPQ807x and IPQ50xx series Soc-s.
 
 config MDIO_IPQ8064
 	tristate "Qualcomm IPQ8064 MDIO interface support"
diff --git a/drivers/net/mdio/mdio-ipq4019.c b/drivers/net/mdio/mdio-ipq4019.c
index b365f13c92ca..bc26b1459b12 100644
--- a/drivers/net/mdio/mdio-ipq4019.c
+++ b/drivers/net/mdio/mdio-ipq4019.c
@@ -262,6 +262,7 @@ static int ipq4019_mdio_remove(struct platform_device *pdev)
 
 static const struct of_device_id ipq4019_mdio_dt_ids[] = {
 	{ .compatible = "qcom,ipq4019-mdio" },
+	{ .compatible = "qcom,ipq5018-mdio" },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, ipq4019_mdio_dt_ids);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v1 1/2] net: mdio: Add the reset function for IPQ MDIO driver

[Andrew Lunn]

> +static int ipq_mdio_reset(struct mii_bus *bus)
> +{
> +	struct ipq4019_mdio_data *priv = bus->priv;
> +	u32 val;
> +	int ret;
> +
> +	/* To indicate CMN_PLL that ethernet_ldo has been ready if platform resource 1
> +	 * is specified in the device tree.
> +	 * */
> +	if (!IS_ERR(priv->eth_ldo_rdy)) {
> +		val = readl(priv->eth_ldo_rdy);
> +		val |= BIT(0);
> +		writel(val, priv->eth_ldo_rdy);
> +		fsleep(IPQ_PHY_SET_DELAY_US);
> +	}
> +
> +	/* Configure MDIO clock source frequency if clock is specified in the device tree */
> +	if (!IS_ERR_OR_NULL(priv->mdio_clk)) {
> +		ret = clk_set_rate(priv->mdio_clk, IPQ_MDIO_CLK_RATE);
> +		if (ret)
> +			return ret;
> +
> +		ret = clk_prepare_enable(priv->mdio_clk);
> +		if (ret)
> +			return ret;
> +	}

These !IS_ERR() are pretty ugly. So

> @@ -182,14 +221,22 @@ static int ipq4019_mdio_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	priv = bus->priv;
> +	priv->eth_ldo_rdy = IOMEM_ERR_PTR(-EINVAL);
>  
>  	priv->membase = devm_platform_ioremap_resource(pdev, 0);
>  	if (IS_ERR(priv->membase))
>  		return PTR_ERR(priv->membase);
>  
> +	priv->mdio_clk = devm_clk_get_optional(&pdev->dev, "gcc_mdio_ahb_clk");

If this returns an error, it is a real error. You should not ignore
it. Fail the probe returning the error. That then means when the reset
function is called priv->mdio_clk contains either a clock, or NULL,
which the clk API is happy to take. No need for an if.


> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	if (res)
> +		priv->eth_ldo_rdy = devm_ioremap_resource(&pdev->dev, res);

platform_get_resource() returns a pointer or NULL. There is no error
code. So

> +	if (!IS_ERR(priv->eth_ldo_rdy)) {

is actually wrong, should simply become

> +	if (priv->eth_ldo_rdy) {

  Andrew

Re: [PATCH v1 1/2] net: mdio: Add the reset function for IPQ MDIO driver

[Andrew Lunn]

> +	/* Configure MDIO clock source frequency if clock is specified in the device tree */
> +	if (!IS_ERR_OR_NULL(priv->mdio_clk)) {

Please document the clock in the binding. Make it clear which devices
require the clock, and which don't.

	Andrew

Re: [PATCH v1 1/2] net: mdio: Add the reset function for IPQ MDIO driver

[Jie Luo]

On 8/8/2021 11:27 PM, Andrew Lunn wrote:
>> +static int ipq_mdio_reset(struct mii_bus *bus)
>> +{
>> +	struct ipq4019_mdio_data *priv = bus->priv;
>> +	u32 val;
>> +	int ret;
>> +
>> +	/* To indicate CMN_PLL that ethernet_ldo has been ready if platform resource 1
>> +	 * is specified in the device tree.
>> +	 * */
>> +	if (!IS_ERR(priv->eth_ldo_rdy)) {
>> +		val = readl(priv->eth_ldo_rdy);
>> +		val |= BIT(0);
>> +		writel(val, priv->eth_ldo_rdy);
>> +		fsleep(IPQ_PHY_SET_DELAY_US);
>> +	}
>> +
>> +	/* Configure MDIO clock source frequency if clock is specified in the device tree */
>> +	if (!IS_ERR_OR_NULL(priv->mdio_clk)) {
>> +		ret = clk_set_rate(priv->mdio_clk, IPQ_MDIO_CLK_RATE);
>> +		if (ret)
>> +			return ret;
>> +
>> +		ret = clk_prepare_enable(priv->mdio_clk);
>> +		if (ret)
>> +			return ret;
>> +	}
> These !IS_ERR() are pretty ugly. So
>
>> @@ -182,14 +221,22 @@ static int ipq4019_mdio_probe(struct platform_device *pdev)
>>   		return -ENOMEM;
>>   
>>   	priv = bus->priv;
>> +	priv->eth_ldo_rdy = IOMEM_ERR_PTR(-EINVAL);
>>   
>>   	priv->membase = devm_platform_ioremap_resource(pdev, 0);
>>   	if (IS_ERR(priv->membase))
>>   		return PTR_ERR(priv->membase);
>>   
>> +	priv->mdio_clk = devm_clk_get_optional(&pdev->dev, "gcc_mdio_ahb_clk");
> If this returns an error, it is a real error. You should not ignore
> it. Fail the probe returning the error. That then means when the reset
> function is called priv->mdio_clk contains either a clock, or NULL,
> which the clk API is happy to take. No need for an if.
>
>
>> +
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>> +	if (res)
>> +		priv->eth_ldo_rdy = devm_ioremap_resource(&pdev->dev, res);
> platform_get_resource() returns a pointer or NULL. There is no error
> code. So
>
>> +	if (!IS_ERR(priv->eth_ldo_rdy)) {
> is actually wrong, should simply become
>
>> +	if (priv->eth_ldo_rdy) {
>    Andrew

Hi Andrew,

Thanks for the kindly review and the comments, will follow it in the 
next patch set.

Re: [PATCH v1 1/2] net: mdio: Add the reset function for IPQ MDIO driver

[Jie Luo]

On 8/8/2021 11:29 PM, Andrew Lunn wrote:
>> +	/* Configure MDIO clock source frequency if clock is specified in the device tree */
>> +	if (!IS_ERR_OR_NULL(priv->mdio_clk)) {
> Please document the clock in the binding. Make it clear which devices
> require the clock, and which don't.
>
> 	Andrew
sure, will document it in the next patch set, thanks Andrew.