diff mbox series

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

Message ID 20200702103001.233961-3-robert.marko@sartura.hr (mailing list archive)
State New, archived
Headers show
Series net: mdio-ipq4019: add Clause 45 and clock support | expand

Commit Message

Robert Marko July 2, 2020, 10:29 a.m. UTC
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(-)

Comments

Andrew Lunn July 2, 2020, 1:29 p.m. UTC | #1
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
Florian Fainelli July 2, 2020, 7:59 p.m. UTC | #2
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?
Robert Marko July 3, 2020, 11:37 a.m. UTC | #3
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
Andrew Lunn July 3, 2020, 1:35 p.m. UTC | #4
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
diff mbox series

Patch

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;