[net-next,PATCH 0/4] net: mdio-ipq4019: add Clause 45 and clock support

[net-next,PATCH 0/4] net: mdio-ipq4019: add Clause 45 and clock support

[Robert Marko]

This patch series adds support for Clause 45 and clock handling support to
the driver.

While at it also change some defines to upper case to match rest of the driver.

Robert Marko (4):
  net: mdio-ipq4019: change defines to upper case
  net: mdio-ipq4019: add clock support
  net: mdio-ipq4019: add Clause 45 support
  dt-bindings: mdio-ipq4019: add clock support

 .../bindings/net/qcom,ipq4019-mdio.yaml       |  11 ++
 drivers/net/phy/mdio-ipq4019.c                | 141 +++++++++++++++---
 2 files changed, 132 insertions(+), 20 deletions(-)

-- 
2.26.2

[net-next,PATCH 1/4] net: mdio-ipq4019: change defines to upper case

[Robert Marko]

In the commit adding the IPQ4019 MDIO driver, defines for timeout and sleep partially used lower case.
Lets change it to upper case in line with the rest of driver defines.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
 drivers/net/phy/mdio-ipq4019.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/mdio-ipq4019.c b/drivers/net/phy/mdio-ipq4019.c
index f1f25489e134..0e78830c070b 100644
--- a/drivers/net/phy/mdio-ipq4019.c
+++ b/drivers/net/phy/mdio-ipq4019.c
@@ -21,8 +21,8 @@
 #define MDIO_CMD_ACCESS_CODE_READ	0
 #define MDIO_CMD_ACCESS_CODE_WRITE	1
 
-#define ipq4019_MDIO_TIMEOUT	10000
-#define ipq4019_MDIO_SLEEP		10
+#define IPQ4019_MDIO_TIMEOUT	10000
+#define IPQ4019_MDIO_SLEEP		10
 
 struct ipq4019_mdio_data {
 	void __iomem	*membase;
@@ -35,7 +35,7 @@ static int ipq4019_mdio_wait_busy(struct mii_bus *bus)
 
 	return readl_poll_timeout(priv->membase + MDIO_CMD_REG, busy,
 				  (busy & MDIO_CMD_ACCESS_BUSY) == 0, 
-				  ipq4019_MDIO_SLEEP, ipq4019_MDIO_TIMEOUT);
+				  IPQ4019_MDIO_SLEEP, IPQ4019_MDIO_TIMEOUT);
 }
 
 static int ipq4019_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
-- 
2.26.2

[net-next,PATCH 2/4] net: mdio-ipq4019: add clock support

[Robert Marko]

Some newer SoC-s have a separate MDIO clock that needs to be enabled.
So lets add support for handling the clocks to the driver.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
 drivers/net/phy/mdio-ipq4019.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/mdio-ipq4019.c b/drivers/net/phy/mdio-ipq4019.c
index 0e78830c070b..7660bf006da0 100644
--- a/drivers/net/phy/mdio-ipq4019.c
+++ b/drivers/net/phy/mdio-ipq4019.c
@@ -9,6 +9,7 @@
 #include <linux/iopoll.h>
 #include <linux/of_address.h>
 #include <linux/of_mdio.h>
+#include <linux/clk.h>
 #include <linux/phy.h>
 #include <linux/platform_device.h>
 
@@ -24,8 +25,12 @@
 #define IPQ4019_MDIO_TIMEOUT	10000
 #define IPQ4019_MDIO_SLEEP		10
 
+#define QCA_MDIO_CLK_DEFAULT_RATE	100000000
+
 struct ipq4019_mdio_data {
-	void __iomem	*membase;
+	void __iomem		*membase;
+	struct clk			*mdio_clk;
+	u32					clk_freq;
 };
 
 static int ipq4019_mdio_wait_busy(struct mii_bus *bus)
@@ -100,6 +105,7 @@ static int ipq4019_mdio_probe(struct platform_device *pdev)
 {
 	struct ipq4019_mdio_data *priv;
 	struct mii_bus *bus;
+	struct device_node *np = pdev->dev.of_node;
 	int ret;
 
 	bus = devm_mdiobus_alloc_size(&pdev->dev, sizeof(*priv));
@@ -112,6 +118,26 @@ static int ipq4019_mdio_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->membase))
 		return PTR_ERR(priv->membase);
 
+	priv->mdio_clk = devm_clk_get_optional(&pdev->dev, "mdio_ahb");
+	if (!IS_ERR(priv->mdio_clk)) {
+		if (of_property_read_u32(np, "clock-frequency", &priv->clk_freq)) {
+			dev_warn(&pdev->dev, "Cannot find MDIO clock frequency, using default!\n");
+			priv->clk_freq = QCA_MDIO_CLK_DEFAULT_RATE;
+		}
+
+		ret = clk_set_rate(priv->mdio_clk, priv->clk_freq);
+		if (ret) {
+			dev_err(&pdev->dev, "Cannot set MDIO clock rate!\n");
+			return ret;
+		}
+
+		ret = clk_prepare_enable(priv->mdio_clk);
+		if (ret) {
+			dev_err(&pdev->dev, "Cannot enable MDIO clock!\n");
+			return ret;
+		}
+	}
+
 	bus->name = "ipq4019_mdio";
 	bus->read = ipq4019_mdio_read;
 	bus->write = ipq4019_mdio_write;
-- 
2.26.2

[net-next,PATCH 3/4] net: mdio-ipq4019: add Clause 45 support

[Robert Marko]

While up-streaming the IPQ4019 driver it was thought that the controller had no Clause 45 support,
but it actually does and its activated by writing a bit to the mode register.

So lets add it as newer SoC-s use the same controller and Clause 45 compliant PHY-s.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
 drivers/net/phy/mdio-ipq4019.c | 109 ++++++++++++++++++++++++++++-----
 1 file changed, 92 insertions(+), 17 deletions(-)

diff --git a/drivers/net/phy/mdio-ipq4019.c b/drivers/net/phy/mdio-ipq4019.c
index 7660bf006da0..9143113d5a6b 100644
--- a/drivers/net/phy/mdio-ipq4019.c
+++ b/drivers/net/phy/mdio-ipq4019.c
@@ -13,6 +13,7 @@
 #include <linux/phy.h>
 #include <linux/platform_device.h>
 
+#define MDIO_MODE_REG				0x40
 #define MDIO_ADDR_REG				0x44
 #define MDIO_DATA_WRITE_REG			0x48
 #define MDIO_DATA_READ_REG			0x4c
@@ -21,6 +22,12 @@
 #define MDIO_CMD_ACCESS_START		BIT(8)
 #define MDIO_CMD_ACCESS_CODE_READ	0
 #define MDIO_CMD_ACCESS_CODE_WRITE	1
+#define MDIO_CMD_ACCESS_CODE_C45_ADDR	0
+#define MDIO_CMD_ACCESS_CODE_C45_WRITE	1
+#define MDIO_CMD_ACCESS_CODE_C45_READ	2
+
+/* 0 = Clause 22, 1 = Clause 45 */
+#define MDIO_MODE_BIT				BIT(8)
 
 #define IPQ4019_MDIO_TIMEOUT	10000
 #define IPQ4019_MDIO_SLEEP		10
@@ -39,7 +46,7 @@ static int ipq4019_mdio_wait_busy(struct mii_bus *bus)
 	unsigned int busy;
 
 	return readl_poll_timeout(priv->membase + MDIO_CMD_REG, busy,
-				  (busy & MDIO_CMD_ACCESS_BUSY) == 0, 
+				  (busy & MDIO_CMD_ACCESS_BUSY) == 0,
 				  IPQ4019_MDIO_SLEEP, IPQ4019_MDIO_TIMEOUT);
 }
 
@@ -47,18 +54,43 @@ static int ipq4019_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
 {
 	struct ipq4019_mdio_data *priv = bus->priv;
 	unsigned int cmd;
-
-	/* Reject clause 45 */
-	if (regnum & MII_ADDR_C45)
-		return -EOPNOTSUPP;
+	unsigned int data;
 
 	if (ipq4019_mdio_wait_busy(bus))
 		return -ETIMEDOUT;
 
-	/* issue the phy address and reg */
-	writel((mii_id << 8) | regnum, priv->membase + MDIO_ADDR_REG);
+	/* Clause 45 support */
+	if (regnum & MII_ADDR_C45) {
+		unsigned int mmd = (regnum >> 16) & 0x1F;
+		unsigned int reg = regnum & 0xFFFF;
+
+		/* Enter Clause 45 mode */
+		data = readl(priv->membase + MDIO_MODE_REG);
+
+		data |= MDIO_MODE_BIT;
+
+		writel(data, priv->membase + MDIO_MODE_REG);
+
+		/* issue the phy address and mmd */
+		writel((mii_id << 8) | mmd, priv->membase + MDIO_ADDR_REG);
+
+		/* issue reg */
+		writel(reg, priv->membase + MDIO_DATA_WRITE_REG);
+
+		cmd = MDIO_CMD_ACCESS_START | MDIO_CMD_ACCESS_CODE_C45_ADDR;
+	} else {
+		/* Enter Clause 22 mode */
+		data = readl(priv->membase + MDIO_MODE_REG);
 
-	cmd = MDIO_CMD_ACCESS_START | MDIO_CMD_ACCESS_CODE_READ;
+		data &= ~MDIO_MODE_BIT;
+
+		writel(data, priv->membase + MDIO_MODE_REG);
+
+		/* issue the phy address and reg */
+		writel((mii_id << 8) | regnum, priv->membase + MDIO_ADDR_REG);
+
+		cmd = MDIO_CMD_ACCESS_START | MDIO_CMD_ACCESS_CODE_READ;
+	}
 
 	/* issue read command */
 	writel(cmd, priv->membase + MDIO_CMD_REG);
@@ -67,6 +99,15 @@ static int ipq4019_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
 	if (ipq4019_mdio_wait_busy(bus))
 		return -ETIMEDOUT;
 
+	if (regnum & MII_ADDR_C45) {
+		cmd = MDIO_CMD_ACCESS_START | MDIO_CMD_ACCESS_CODE_C45_READ;
+
+		writel(cmd, priv->membase + MDIO_CMD_REG);
+
+		if (ipq4019_mdio_wait_busy(bus))
+			return -ETIMEDOUT;
+	}
+
 	/* Read and return data */
 	return readl(priv->membase + MDIO_DATA_READ_REG);
 }
@@ -76,22 +117,56 @@ static int ipq4019_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
 {
 	struct ipq4019_mdio_data *priv = bus->priv;
 	unsigned int cmd;
-
-	/* Reject clause 45 */
-	if (regnum & MII_ADDR_C45)
-		return -EOPNOTSUPP;
+	unsigned int data;
 
 	if (ipq4019_mdio_wait_busy(bus))
 		return -ETIMEDOUT;
 
-	/* issue the phy address and reg */
-	writel((mii_id << 8) | regnum, priv->membase + MDIO_ADDR_REG);
+	/* Clause 45 support */
+	if (regnum & MII_ADDR_C45) {
+		unsigned int mmd = (regnum >> 16) & 0x1F;
+		unsigned int reg = regnum & 0xFFFF;
+
+		/* Enter Clause 45 mode */
+		data = readl(priv->membase + MDIO_MODE_REG);
+
+		data |= MDIO_MODE_BIT;
+
+		writel(data, priv->membase + MDIO_MODE_REG);
+
+		/* issue the phy address and mmd */
+		writel((mii_id << 8) | mmd, priv->membase + MDIO_ADDR_REG);
+
+		/* issue reg */
+		writel(reg, priv->membase + MDIO_DATA_WRITE_REG);
+
+		cmd = MDIO_CMD_ACCESS_START | MDIO_CMD_ACCESS_CODE_C45_ADDR;
+
+		writel(cmd, priv->membase + MDIO_CMD_REG);
+
+		if (ipq4019_mdio_wait_busy(bus))
+			return -ETIMEDOUT;
+	} else {
+		/* Enter Clause 22 mode */
+		data = readl(priv->membase + MDIO_MODE_REG);
+
+		data &= ~MDIO_MODE_BIT;
+
+		writel(data, priv->membase + MDIO_MODE_REG);
+
+		/* issue the phy address and reg */
+		writel((mii_id << 8) | regnum, priv->membase + MDIO_ADDR_REG);
+	}
 
 	/* issue write data */
 	writel(value, priv->membase + MDIO_DATA_WRITE_REG);
-
-	cmd = MDIO_CMD_ACCESS_START | MDIO_CMD_ACCESS_CODE_WRITE;
+
 	/* issue write command */
+	if (regnum & MII_ADDR_C45)
+		cmd = MDIO_CMD_ACCESS_START | MDIO_CMD_ACCESS_CODE_C45_WRITE;
+	else
+		cmd = MDIO_CMD_ACCESS_START | MDIO_CMD_ACCESS_CODE_WRITE;
+
 	writel(cmd, priv->membase + MDIO_CMD_REG);
 
 	/* Wait write complete */
@@ -105,7 +180,7 @@ static int ipq4019_mdio_probe(struct platform_device *pdev)
 {
 	struct ipq4019_mdio_data *priv;
 	struct mii_bus *bus;
-	struct device_node *np = pdev->dev.of_node; 
+	struct device_node *np = pdev->dev.of_node;
 	int ret;
 
 	bus = devm_mdiobus_alloc_size(&pdev->dev, sizeof(*priv));
-- 
2.26.2

[net-next,PATCH 4/4] dt-bindings: mdio-ipq4019: add clock support

[Robert Marko]

This adds the necessary bindings for SoC-s that have a separate MDIO clock.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
 .../devicetree/bindings/net/qcom,ipq4019-mdio.yaml    | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml b/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
index 13555a89975f..06b4eedb4370 100644
--- a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
+++ b/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
@@ -25,6 +25,17 @@ properties:
   reg:
     maxItems: 1
 
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    items:
+      - const: mdio_ahb
+    maxItems: 1
+
+  clock-frequency:
+    default: 100000000
+
 required:
   - compatible
   - reg
-- 
2.26.2

Re: [net-next,PATCH 1/4] net: mdio-ipq4019: change defines to upper case

[Andrew Lunn]

On Thu, Jul 02, 2020 at 12:29:58PM +0200, Robert Marko wrote:
> In the commit adding the IPQ4019 MDIO driver, defines for timeout and sleep partially used lower case.
> Lets change it to upper case in line with the rest of driver defines.
> 
> Signed-off-by: Robert Marko <robert.marko@sartura.hr>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [net-next,PATCH 2/4] net: mdio-ipq4019: add clock support

[Andrew Lunn]

On Thu, Jul 02, 2020 at 12:29:59PM +0200, Robert Marko wrote:
> Some newer SoC-s have a separate MDIO clock that needs to be enabled.
> So lets add support for handling the clocks to the driver.
> 
> Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> ---
>  drivers/net/phy/mdio-ipq4019.c | 28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/phy/mdio-ipq4019.c b/drivers/net/phy/mdio-ipq4019.c
> index 0e78830c070b..7660bf006da0 100644
> --- a/drivers/net/phy/mdio-ipq4019.c
> +++ b/drivers/net/phy/mdio-ipq4019.c
> @@ -9,6 +9,7 @@
>  #include <linux/iopoll.h>
>  #include <linux/of_address.h>
>  #include <linux/of_mdio.h>
> +#include <linux/clk.h>
>  #include <linux/phy.h>
>  #include <linux/platform_device.h>
>  
> @@ -24,8 +25,12 @@
>  #define IPQ4019_MDIO_TIMEOUT	10000
>  #define IPQ4019_MDIO_SLEEP		10
>  
> +#define QCA_MDIO_CLK_DEFAULT_RATE	100000000
> +
>  struct ipq4019_mdio_data {
> -	void __iomem	*membase;
> +	void __iomem		*membase;
> +	struct clk			*mdio_clk;
> +	u32					clk_freq;

Hi Robert

Some sort of tab/space issue here.

>  };
>  
>  static int ipq4019_mdio_wait_busy(struct mii_bus *bus)
> @@ -100,6 +105,7 @@ static int ipq4019_mdio_probe(struct platform_device *pdev)
>  {
>  	struct ipq4019_mdio_data *priv;
>  	struct mii_bus *bus;
> +	struct device_node *np = pdev->dev.of_node;
>  	int ret;

Reverse Christmas tree.

	Andrew

Re: [net-next,PATCH 3/4] net: mdio-ipq4019: add Clause 45 support

[Andrew Lunn]

On Thu, Jul 02, 2020 at 12:30:00PM +0200, Robert Marko wrote:
> While up-streaming the IPQ4019 driver it was thought that the controller had no Clause 45 support,
> but it actually does and its activated by writing a bit to the mode register.
> 
> So lets add it as newer SoC-s use the same controller and Clause 45 compliant PHY-s.
> 
> Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> ---
>  drivers/net/phy/mdio-ipq4019.c | 109 ++++++++++++++++++++++++++++-----
>  1 file changed, 92 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/net/phy/mdio-ipq4019.c b/drivers/net/phy/mdio-ipq4019.c
> index 7660bf006da0..9143113d5a6b 100644
> --- a/drivers/net/phy/mdio-ipq4019.c
> +++ b/drivers/net/phy/mdio-ipq4019.c
> @@ -13,6 +13,7 @@
>  #include <linux/phy.h>
>  #include <linux/platform_device.h>
>  
> +#define MDIO_MODE_REG				0x40
>  #define MDIO_ADDR_REG				0x44
>  #define MDIO_DATA_WRITE_REG			0x48
>  #define MDIO_DATA_READ_REG			0x4c
> @@ -21,6 +22,12 @@
>  #define MDIO_CMD_ACCESS_START		BIT(8)
>  #define MDIO_CMD_ACCESS_CODE_READ	0
>  #define MDIO_CMD_ACCESS_CODE_WRITE	1
> +#define MDIO_CMD_ACCESS_CODE_C45_ADDR	0
> +#define MDIO_CMD_ACCESS_CODE_C45_WRITE	1
> +#define MDIO_CMD_ACCESS_CODE_C45_READ	2
> +
> +/* 0 = Clause 22, 1 = Clause 45 */
> +#define MDIO_MODE_BIT				BIT(8)
>  
>  #define IPQ4019_MDIO_TIMEOUT	10000
>  #define IPQ4019_MDIO_SLEEP		10
> @@ -39,7 +46,7 @@ static int ipq4019_mdio_wait_busy(struct mii_bus *bus)
>  	unsigned int busy;
>  
>  	return readl_poll_timeout(priv->membase + MDIO_CMD_REG, busy,
> -				  (busy & MDIO_CMD_ACCESS_BUSY) == 0, 
> +				  (busy & MDIO_CMD_ACCESS_BUSY) == 0,
>  				  IPQ4019_MDIO_SLEEP, IPQ4019_MDIO_TIMEOUT);
>  }

Please put white space changes into a separate patch.


>  
> @@ -47,18 +54,43 @@ static int ipq4019_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
>  {
>  	struct ipq4019_mdio_data *priv = bus->priv;
>  	unsigned int cmd;
> -
> -	/* Reject clause 45 */
> -	if (regnum & MII_ADDR_C45)
> -		return -EOPNOTSUPP;
> +	unsigned int data;

Reverse Christmas tree please.

>  
>  	if (ipq4019_mdio_wait_busy(bus))
>  		return -ETIMEDOUT;
>  
> -	/* issue the phy address and reg */
> -	writel((mii_id << 8) | regnum, priv->membase + MDIO_ADDR_REG);
> +	/* Clause 45 support */
> +	if (regnum & MII_ADDR_C45) {
> +		unsigned int mmd = (regnum >> 16) & 0x1F;
> +		unsigned int reg = regnum & 0xFFFF;
> +
> +		/* Enter Clause 45 mode */
> +		data = readl(priv->membase + MDIO_MODE_REG);
> +
> +		data |= MDIO_MODE_BIT;
> +
> +		writel(data, priv->membase + MDIO_MODE_REG);
> +
> +		/* issue the phy address and mmd */
> +		writel((mii_id << 8) | mmd, priv->membase + MDIO_ADDR_REG);
> +
> +		/* issue reg */
> +		writel(reg, priv->membase + MDIO_DATA_WRITE_REG);
> +
> +		cmd = MDIO_CMD_ACCESS_START | MDIO_CMD_ACCESS_CODE_C45_ADDR;
> +	} else {
> +		/* Enter Clause 22 mode */
> +		data = readl(priv->membase + MDIO_MODE_REG);
>  
> -	cmd = MDIO_CMD_ACCESS_START | MDIO_CMD_ACCESS_CODE_READ;
> +		data &= ~MDIO_MODE_BIT;
> +
> +		writel(data, priv->membase + MDIO_MODE_REG);
> +
> +		/* issue the phy address and reg */
> +		writel((mii_id << 8) | regnum, priv->membase + MDIO_ADDR_REG);
> +
> +		cmd = MDIO_CMD_ACCESS_START | MDIO_CMD_ACCESS_CODE_READ;
> +	}
>  
>  	/* issue read command */
>  	writel(cmd, priv->membase + MDIO_CMD_REG);
> @@ -67,6 +99,15 @@ static int ipq4019_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
>  	if (ipq4019_mdio_wait_busy(bus))
>  		return -ETIMEDOUT;
>  
> +	if (regnum & MII_ADDR_C45) {
> +		cmd = MDIO_CMD_ACCESS_START | MDIO_CMD_ACCESS_CODE_C45_READ;
> +
> +		writel(cmd, priv->membase + MDIO_CMD_REG);
> +
> +		if (ipq4019_mdio_wait_busy(bus))
> +			return -ETIMEDOUT;
> +	}
> +
>  	/* Read and return data */
>  	return readl(priv->membase + MDIO_DATA_READ_REG);
>  }
> @@ -76,22 +117,56 @@ static int ipq4019_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
>  {
>  	struct ipq4019_mdio_data *priv = bus->priv;
>  	unsigned int cmd;
> -
> -	/* Reject clause 45 */
> -	if (regnum & MII_ADDR_C45)
> -		return -EOPNOTSUPP;
> +	unsigned int data;
>  
>  	if (ipq4019_mdio_wait_busy(bus))
>  		return -ETIMEDOUT;
>  
> -	/* issue the phy address and reg */
> -	writel((mii_id << 8) | regnum, priv->membase + MDIO_ADDR_REG);
> +	/* Clause 45 support */
> +	if (regnum & MII_ADDR_C45) {
> +		unsigned int mmd = (regnum >> 16) & 0x1F;
> +		unsigned int reg = regnum & 0xFFFF;
> +
> +		/* Enter Clause 45 mode */
> +		data = readl(priv->membase + MDIO_MODE_REG);
> +
> +		data |= MDIO_MODE_BIT;
> +
> +		writel(data, priv->membase + MDIO_MODE_REG);
> +
> +		/* issue the phy address and mmd */
> +		writel((mii_id << 8) | mmd, priv->membase + MDIO_ADDR_REG);
> +
> +		/* issue reg */
> +		writel(reg, priv->membase + MDIO_DATA_WRITE_REG);
> +
> +		cmd = MDIO_CMD_ACCESS_START | MDIO_CMD_ACCESS_CODE_C45_ADDR;
> +
> +		writel(cmd, priv->membase + MDIO_CMD_REG);
> +
> +		if (ipq4019_mdio_wait_busy(bus))
> +			return -ETIMEDOUT;
> +	} else {
> +		/* Enter Clause 22 mode */
> +		data = readl(priv->membase + MDIO_MODE_REG);
> +
> +		data &= ~MDIO_MODE_BIT;
> +
> +		writel(data, priv->membase + MDIO_MODE_REG);
> +
> +		/* issue the phy address and reg */
> +		writel((mii_id << 8) | regnum, priv->membase + MDIO_ADDR_REG);
> +	}
>  
>  	/* issue write data */
>  	writel(value, priv->membase + MDIO_DATA_WRITE_REG);
> -
> -	cmd = MDIO_CMD_ACCESS_START | MDIO_CMD_ACCESS_CODE_WRITE;
> +
>  	/* issue write command */
> +	if (regnum & MII_ADDR_C45)
> +		cmd = MDIO_CMD_ACCESS_START | MDIO_CMD_ACCESS_CODE_C45_WRITE;
> +	else
> +		cmd = MDIO_CMD_ACCESS_START | MDIO_CMD_ACCESS_CODE_WRITE;
> +
>  	writel(cmd, priv->membase + MDIO_CMD_REG);
>  
>  	/* Wait write complete */
> @@ -105,7 +180,7 @@ static int ipq4019_mdio_probe(struct platform_device *pdev)
>  {
>  	struct ipq4019_mdio_data *priv;
>  	struct mii_bus *bus;
> -	struct device_node *np = pdev->dev.of_node; 
> +	struct device_node *np = pdev->dev.of_node;
>  	int ret;

Another white space change.

	Andrew

Re: [net-next,PATCH 4/4] dt-bindings: mdio-ipq4019: add clock support

[Andrew Lunn]

> +  clock-frequency:
> +    default: 100000000

IEEE 802.3 says the default should be 2.5MHz. Some PHYs will go
faster, but 100MHz seems unlikely!

     Andrew

Re: [net-next,PATCH 4/4] dt-bindings: mdio-ipq4019: add clock support

[Robert Marko]

On Thu, Jul 2, 2020 at 3:38 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > +  clock-frequency:
> > +    default: 100000000
>
> IEEE 802.3 says the default should be 2.5MHz. Some PHYs will go
> faster, but 100MHz seems unlikely!
This MDIO controller has an internal divider, by default its set for
100MHz clock.
In IPQ4019 MDIO clock is not controllable but in IPQ6018 etc it's controllable.
That is the only combination I have currently seen used by Qualcomm.
>
>      Andrew

Re: [net-next,PATCH 1/4] net: mdio-ipq4019: change defines to upper case

[Florian Fainelli]

On 7/2/2020 3:29 AM, Robert Marko wrote:
> In the commit adding the IPQ4019 MDIO driver, defines for timeout and sleep partially used lower case.
> Lets change it to upper case in line with the rest of driver defines.
> 
> Signed-off-by: Robert Marko <robert.marko@sartura.hr>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [net-next,PATCH 2/4] net: mdio-ipq4019: add clock support

[Florian Fainelli]

On 7/2/2020 3:29 AM, Robert Marko wrote:
> Some newer SoC-s have a separate MDIO clock that needs to be enabled.
> So lets add support for handling the clocks to the driver.
> 
> Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> ---
>  drivers/net/phy/mdio-ipq4019.c | 28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/phy/mdio-ipq4019.c b/drivers/net/phy/mdio-ipq4019.c
> index 0e78830c070b..7660bf006da0 100644
> --- a/drivers/net/phy/mdio-ipq4019.c
> +++ b/drivers/net/phy/mdio-ipq4019.c
> @@ -9,6 +9,7 @@
>  #include <linux/iopoll.h>
>  #include <linux/of_address.h>
>  #include <linux/of_mdio.h>
> +#include <linux/clk.h>
>  #include <linux/phy.h>
>  #include <linux/platform_device.h>
>  
> @@ -24,8 +25,12 @@
>  #define IPQ4019_MDIO_TIMEOUT	10000
>  #define IPQ4019_MDIO_SLEEP		10
>  
> +#define QCA_MDIO_CLK_DEFAULT_RATE	100000000

100MHz? Is not that going to be a tad too much for most MDIO devices out
there?
-- 
Florian

Re: [net-next,PATCH 4/4] dt-bindings: mdio-ipq4019: add clock support

[Florian Fainelli]

On 7/2/2020 12:18 PM, Robert Marko wrote:
> On Thu, Jul 2, 2020 at 3:38 PM Andrew Lunn <andrew@lunn.ch> wrote:
>>
>>> +  clock-frequency:
>>> +    default: 100000000
>>
>> IEEE 802.3 says the default should be 2.5MHz. Some PHYs will go
>> faster, but 100MHz seems unlikely!
> This MDIO controller has an internal divider, by default its set for
> 100MHz clock.
> In IPQ4019 MDIO clock is not controllable but in IPQ6018 etc it's controllable.
> That is the only combination I have currently seen used by Qualcomm.

Not sure I understand here, the 'clock-frequency' is supposed to denote
the MDIO bus output clock frequency, that is the frequency at which all
MDIO devices are going to operate at. Is this 100MHz a clock that feeds
into the MDIO block and get internally divided by a programmable
register to obtain an output MDIO clock?
-- 
Florian

Re: [net-next,PATCH 4/4] dt-bindings: mdio-ipq4019: add clock support

[Robert Marko]

On Thu, Jul 2, 2020 at 10:04 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>
>
>
> On 7/2/2020 12:18 PM, Robert Marko wrote:
> > On Thu, Jul 2, 2020 at 3:38 PM Andrew Lunn <andrew@lunn.ch> wrote:
> >>
> >>> +  clock-frequency:
> >>> +    default: 100000000
> >>
> >> IEEE 802.3 says the default should be 2.5MHz. Some PHYs will go
> >> faster, but 100MHz seems unlikely!
> > This MDIO controller has an internal divider, by default its set for
> > 100MHz clock.
> > In IPQ4019 MDIO clock is not controllable but in IPQ6018 etc it's controllable.
> > That is the only combination I have currently seen used by Qualcomm.
>
> Not sure I understand here, the 'clock-frequency' is supposed to denote
> the MDIO bus output clock frequency, that is the frequency at which all
> MDIO devices are going to operate at. Is this 100MHz a clock that feeds
> into the MDIO block and get internally divided by a programmable
> register to obtain an output MDIO clock?
Yes, in this case that 100MHz comes from the GCC clock controller and
is then internally divided by the MDIO.
I do not know what is the actual output MDIO bus frequency as
datasheet only denotes that MDC divide
bits in the mode register are set for 100MHz incoming clock.
> --
> Florian

Re: [net-next,PATCH 2/4] net: mdio-ipq4019: add clock support

[Robert Marko]

On Thu, Jul 2, 2020 at 9:59 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>
>
>
> On 7/2/2020 3:29 AM, Robert Marko wrote:
> > Some newer SoC-s have a separate MDIO clock that needs to be enabled.
> > So lets add support for handling the clocks to the driver.
> >
> > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> > ---
> >  drivers/net/phy/mdio-ipq4019.c | 28 +++++++++++++++++++++++++++-
> >  1 file changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/phy/mdio-ipq4019.c b/drivers/net/phy/mdio-ipq4019.c
> > index 0e78830c070b..7660bf006da0 100644
> > --- a/drivers/net/phy/mdio-ipq4019.c
> > +++ b/drivers/net/phy/mdio-ipq4019.c
> > @@ -9,6 +9,7 @@
> >  #include <linux/iopoll.h>
> >  #include <linux/of_address.h>
> >  #include <linux/of_mdio.h>
> > +#include <linux/clk.h>
> >  #include <linux/phy.h>
> >  #include <linux/platform_device.h>
> >
> > @@ -24,8 +25,12 @@
> >  #define IPQ4019_MDIO_TIMEOUT 10000
> >  #define IPQ4019_MDIO_SLEEP           10
> >
> > +#define QCA_MDIO_CLK_DEFAULT_RATE    100000000
>
> 100MHz? Is not that going to be a tad too much for most MDIO devices out
> there?
This is not the actual MDIO bus clock, that is the clock frequency
that SoC clock generator produces.
MDIO controller has an internal divider set up for that 100MHz, I
don't know the actual MDIO bus clock
frequency as it's not listed anywhere.
> --
> Florian

Re: [net-next,PATCH 2/4] net: mdio-ipq4019: add clock support

[Andrew Lunn]

On Fri, Jul 03, 2020 at 01:37:48PM +0200, Robert Marko wrote:
> This is not the actual MDIO bus clock, that is the clock frequency
> that SoC clock generator produces.
> MDIO controller has an internal divider set up for that 100MHz, I
> don't know the actual MDIO bus clock
> frequency as it's not listed anywhere.

Hi Robert

From Documentation/devicetree/bindings/net/mdio.yaml 

  clock-frequency:
    description:
      Desired MDIO bus clock frequency in Hz. Values greater than IEEE 802.3
      defined 2.5MHz should only be used when all devices on the bus support
      the given clock speed.

You have to use that definition for clock-frequency. It means the MDIO
bus frequency. It would be good if you can get an oscilloscope onto
the bus and measure it. Otherwise, we have to assume the divider is
40, in order to give a standards compliment 2.5MHz. You can then work
out what value to pass to the clk_ API to get the correct input clock
frequency for the MDIO block.

	  Andrew