[U-Boot,V1,5/6] net: fec_mxc: support i.MX8M with CLK_CCF
diff mbox series

Message ID 20191022034714.27781-6-peng.fan@nxp.com
State Awaiting Upstream
Delegated to: Stefano Babic
Headers show
Series
  • imx: imx8mm-evk: support eth
Related show

Commit Message

Peng Fan Oct. 22, 2019, 3:30 a.m. UTC
Add more clks for fec_mxc according to Linux Kernel 5.4.0-rc1
drivers/net/ethernet/freescale/fec_main.c.

Since i.MX8MQ not support CLK_CCF, so add a check to restrict
the code only effect when CONFIG_IMX8M and CONFIG_CLK_CCF both defined.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/net/fec_mxc.c | 74 ++++++++++++++++++++++++++++++++++++++++-----------
 drivers/net/fec_mxc.h |  4 +++
 2 files changed, 63 insertions(+), 15 deletions(-)

Comments

Schrempf Frieder Oct. 23, 2019, 4:31 p.m. UTC | #1
On 22.10.19 05:30, Peng Fan wrote:
> Add more clks for fec_mxc according to Linux Kernel 5.4.0-rc1
> drivers/net/ethernet/freescale/fec_main.c.
> 
> Since i.MX8MQ not support CLK_CCF, so add a check to restrict
> the code only effect when CONFIG_IMX8M and CONFIG_CLK_CCF both defined.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>   drivers/net/fec_mxc.c | 74 ++++++++++++++++++++++++++++++++++++++++-----------
>   drivers/net/fec_mxc.h |  4 +++
>   2 files changed, 63 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
> index 080dbcf7db..9362aa0d05 100644
> --- a/drivers/net/fec_mxc.c
> +++ b/drivers/net/fec_mxc.c
> @@ -125,28 +125,29 @@ static int fec_mdio_read(struct ethernet_regs *eth, uint8_t phyaddr,
>   
>   static int fec_get_clk_rate(void *udev, int idx)
>   {
> -#if IS_ENABLED(CONFIG_IMX8)
>   	struct fec_priv *fec;
>   	struct udevice *dev;
>   	int ret;
>   
> -	dev = udev;
> -	if (!dev) {
> -		ret = uclass_get_device(UCLASS_ETH, idx, &dev);
> -		if (ret < 0) {
> -			debug("Can't get FEC udev: %d\n", ret);
> -			return ret;
> +	if (IS_ENABLED(CONFIG_IMX8) ||
> +	    (IS_ENABLED(CONFIG_IMX8M) && IS_ENABLED(CONFIG_CLK_CCF))) {

Can't we just drop the IS_ENABLED(CONFIG_IMX8M)? Otherwise we always 
need to touch this code when other SoCs will start using CCF.

Also can you use CONFIG_IS_ENABLED(CLK_CCF) instead of 
IS_ENABLED(CONFIG_CLK_CCF), so we can detect the config options for SPL 
and non-SPL separately?

> +		dev = udev;
> +		if (!dev) {
> +			ret = uclass_get_device(UCLASS_ETH, idx, &dev);
> +			if (ret < 0) {
> +				debug("Can't get FEC udev: %d\n", ret);
> +				return ret;
> +			}
>   		}
> -	}
>   
> -	fec = dev_get_priv(dev);
> -	if (fec)
> -		return fec->clk_rate;
> +		fec = dev_get_priv(dev);
> +		if (fec)
> +			return fec->clk_rate;
>   
> -	return -EINVAL;
> -#else
> -	return imx_get_fecclk();
> -#endif
> +		return -EINVAL;
> +	} else {
> +		return imx_get_fecclk();
> +	}
>   }
>   
>   static void fec_mii_setspeed(struct ethernet_regs *eth)
> @@ -1335,6 +1336,49 @@ static int fecmxc_probe(struct udevice *dev)
>   			return ret;
>   		}
>   
> +		priv->clk_rate = clk_get_rate(&priv->ipg_clk);
> +	} else if (IS_ENABLED(CONFIG_IMX8M) && IS_ENABLED(CONFIG_CLK_CCF)) {

Same questions here as above.

> +		ret = clk_get_by_name(dev, "ipg", &priv->ipg_clk);
> +		if (ret < 0) {
> +			debug("Can't get FEC ipg clk: %d\n", ret);
> +			return ret;
> +		} else {

You can drop the else branches here and below as the code returns before 
it will be evaluated.

> +			ret = clk_enable(&priv->ipg_clk);
> +			if(ret)
> +				return ret;
> +		}
> +
> +		ret = clk_get_by_name(dev, "ipg", &priv->ahb_clk);

This should be "ahb", not "ipg".

> +		if (ret < 0) {
> +			debug("Can't get FEC ahb clk: %d\n", ret);
> +			return ret;
> +		} else {
> +			ret = clk_enable(&priv->ahb_clk);
> +			if (ret)
> +				return ret;
> +		}
> +
> +		ret = clk_get_by_name(dev, "enet_out", &priv->clk_enet_out);
> +		if (!ret) {
> +			ret = clk_enable(&priv->clk_enet_out);
> +			if (ret)
> +				return ret;
> +		}
> +
> +		ret = clk_get_by_name(dev, "enet_clk_ref", &priv->clk_ref);
> +		if (!ret) {
> +			ret = clk_enable(&priv->clk_ref);
> +			if (ret)
> +				return ret;
> +		}
> +
> +		ret = clk_get_by_name(dev, "ptp", &priv->clk_ptp);
> +		if (!ret) {
> +			ret = clk_enable(&priv->clk_ptp);
> +			if (ret)
> +				return ret;
> +		}
> +
>   		priv->clk_rate = clk_get_rate(&priv->ipg_clk);
>   	}
>   
> diff --git a/drivers/net/fec_mxc.h b/drivers/net/fec_mxc.h
> index e5f2dd75c5..723b06a651 100644
> --- a/drivers/net/fec_mxc.h
> +++ b/drivers/net/fec_mxc.h
> @@ -264,6 +264,10 @@ struct fec_priv {
>   	u32 interface;
>   #endif
>   	struct clk ipg_clk;
> +	struct clk ahb_clk;
> +	struct clk clk_enet_out;
> +	struct clk clk_ref;
> +	struct clk clk_ptp;
>   	u32 clk_rate;
>   };
>   
>
Peng Fan Oct. 24, 2019, 1:09 a.m. UTC | #2
> Subject: Re: [U-Boot] [PATCH V1 5/6] net: fec_mxc: support i.MX8M with
> CLK_CCF
> 
> On 22.10.19 05:30, Peng Fan wrote:
> > Add more clks for fec_mxc according to Linux Kernel 5.4.0-rc1
> > drivers/net/ethernet/freescale/fec_main.c.
> >
> > Since i.MX8MQ not support CLK_CCF, so add a check to restrict the code
> > only effect when CONFIG_IMX8M and CONFIG_CLK_CCF both defined.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >   drivers/net/fec_mxc.c | 74
> ++++++++++++++++++++++++++++++++++++++++-----------
> >   drivers/net/fec_mxc.h |  4 +++
> >   2 files changed, 63 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c index
> > 080dbcf7db..9362aa0d05 100644
> > --- a/drivers/net/fec_mxc.c
> > +++ b/drivers/net/fec_mxc.c
> > @@ -125,28 +125,29 @@ static int fec_mdio_read(struct ethernet_regs
> > *eth, uint8_t phyaddr,
> >
> >   static int fec_get_clk_rate(void *udev, int idx)
> >   {
> > -#if IS_ENABLED(CONFIG_IMX8)
> >   	struct fec_priv *fec;
> >   	struct udevice *dev;
> >   	int ret;
> >
> > -	dev = udev;
> > -	if (!dev) {
> > -		ret = uclass_get_device(UCLASS_ETH, idx, &dev);
> > -		if (ret < 0) {
> > -			debug("Can't get FEC udev: %d\n", ret);
> > -			return ret;
> > +	if (IS_ENABLED(CONFIG_IMX8) ||
> > +	    (IS_ENABLED(CONFIG_IMX8M) &&
> IS_ENABLED(CONFIG_CLK_CCF))) {
> 
> Can't we just drop the IS_ENABLED(CONFIG_IMX8M)? Otherwise we always
> need to touch this code when other SoCs will start using CCF.

ok. 

> 
> Also can you use CONFIG_IS_ENABLED(CLK_CCF) instead of
> IS_ENABLED(CONFIG_CLK_CCF), so we can detect the config options for SPL
> and non-SPL separately?

FEC will not be used in SPL stage, so I not use CONFIG_IS_ENABLED.
But it does not hurt using IS_ENABLED, change in v2.

Regards,
Peng.

> 
> > +		dev = udev;
> > +		if (!dev) {
> > +			ret = uclass_get_device(UCLASS_ETH, idx, &dev);
> > +			if (ret < 0) {
> > +				debug("Can't get FEC udev: %d\n", ret);
> > +				return ret;
> > +			}
> >   		}
> > -	}
> >
> > -	fec = dev_get_priv(dev);
> > -	if (fec)
> > -		return fec->clk_rate;
> > +		fec = dev_get_priv(dev);
> > +		if (fec)
> > +			return fec->clk_rate;
> >
> > -	return -EINVAL;
> > -#else
> > -	return imx_get_fecclk();
> > -#endif
> > +		return -EINVAL;
> > +	} else {
> > +		return imx_get_fecclk();
> > +	}
> >   }
> >
> >   static void fec_mii_setspeed(struct ethernet_regs *eth) @@ -1335,6
> > +1336,49 @@ static int fecmxc_probe(struct udevice *dev)
> >   			return ret;
> >   		}
> >
> > +		priv->clk_rate = clk_get_rate(&priv->ipg_clk);
> > +	} else if (IS_ENABLED(CONFIG_IMX8M) &&
> IS_ENABLED(CONFIG_CLK_CCF)) {
> 
> Same questions here as above.
> 
> > +		ret = clk_get_by_name(dev, "ipg", &priv->ipg_clk);
> > +		if (ret < 0) {
> > +			debug("Can't get FEC ipg clk: %d\n", ret);
> > +			return ret;
> > +		} else {
> 
> You can drop the else branches here and below as the code returns before it
> will be evaluated.
> 
> > +			ret = clk_enable(&priv->ipg_clk);
> > +			if(ret)
> > +				return ret;
> > +		}
> > +
> > +		ret = clk_get_by_name(dev, "ipg", &priv->ahb_clk);
> 
> This should be "ahb", not "ipg".
> 
> > +		if (ret < 0) {
> > +			debug("Can't get FEC ahb clk: %d\n", ret);
> > +			return ret;
> > +		} else {
> > +			ret = clk_enable(&priv->ahb_clk);
> > +			if (ret)
> > +				return ret;
> > +		}
> > +
> > +		ret = clk_get_by_name(dev, "enet_out", &priv->clk_enet_out);
> > +		if (!ret) {
> > +			ret = clk_enable(&priv->clk_enet_out);
> > +			if (ret)
> > +				return ret;
> > +		}
> > +
> > +		ret = clk_get_by_name(dev, "enet_clk_ref", &priv->clk_ref);
> > +		if (!ret) {
> > +			ret = clk_enable(&priv->clk_ref);
> > +			if (ret)
> > +				return ret;
> > +		}
> > +
> > +		ret = clk_get_by_name(dev, "ptp", &priv->clk_ptp);
> > +		if (!ret) {
> > +			ret = clk_enable(&priv->clk_ptp);
> > +			if (ret)
> > +				return ret;
> > +		}
> > +
> >   		priv->clk_rate = clk_get_rate(&priv->ipg_clk);
> >   	}
> >
> > diff --git a/drivers/net/fec_mxc.h b/drivers/net/fec_mxc.h index
> > e5f2dd75c5..723b06a651 100644
> > --- a/drivers/net/fec_mxc.h
> > +++ b/drivers/net/fec_mxc.h
> > @@ -264,6 +264,10 @@ struct fec_priv {
> >   	u32 interface;
> >   #endif
> >   	struct clk ipg_clk;
> > +	struct clk ahb_clk;
> > +	struct clk clk_enet_out;
> > +	struct clk clk_ref;
> > +	struct clk clk_ptp;
> >   	u32 clk_rate;
> >   };
> >
> >
Schrempf Frieder Oct. 24, 2019, 7:53 a.m. UTC | #3
On 24.10.19 03:09, Peng Fan wrote:
>> Subject: Re: [U-Boot] [PATCH V1 5/6] net: fec_mxc: support i.MX8M with
>> CLK_CCF
>>
>> On 22.10.19 05:30, Peng Fan wrote:
>>> Add more clks for fec_mxc according to Linux Kernel 5.4.0-rc1
>>> drivers/net/ethernet/freescale/fec_main.c.
>>>
>>> Since i.MX8MQ not support CLK_CCF, so add a check to restrict the code
>>> only effect when CONFIG_IMX8M and CONFIG_CLK_CCF both defined.
>>>
>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>>> ---
>>>    drivers/net/fec_mxc.c | 74
>> ++++++++++++++++++++++++++++++++++++++++-----------
>>>    drivers/net/fec_mxc.h |  4 +++
>>>    2 files changed, 63 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c index
>>> 080dbcf7db..9362aa0d05 100644
>>> --- a/drivers/net/fec_mxc.c
>>> +++ b/drivers/net/fec_mxc.c
>>> @@ -125,28 +125,29 @@ static int fec_mdio_read(struct ethernet_regs
>>> *eth, uint8_t phyaddr,
>>>
>>>    static int fec_get_clk_rate(void *udev, int idx)
>>>    {
>>> -#if IS_ENABLED(CONFIG_IMX8)
>>>    	struct fec_priv *fec;
>>>    	struct udevice *dev;
>>>    	int ret;
>>>
>>> -	dev = udev;
>>> -	if (!dev) {
>>> -		ret = uclass_get_device(UCLASS_ETH, idx, &dev);
>>> -		if (ret < 0) {
>>> -			debug("Can't get FEC udev: %d\n", ret);
>>> -			return ret;
>>> +	if (IS_ENABLED(CONFIG_IMX8) ||
>>> +	    (IS_ENABLED(CONFIG_IMX8M) &&
>> IS_ENABLED(CONFIG_CLK_CCF))) {
>>
>> Can't we just drop the IS_ENABLED(CONFIG_IMX8M)? Otherwise we always
>> need to touch this code when other SoCs will start using CCF.
> 
> ok.
> 
>>
>> Also can you use CONFIG_IS_ENABLED(CLK_CCF) instead of
>> IS_ENABLED(CONFIG_CLK_CCF), so we can detect the config options for SPL
>> and non-SPL separately?
> 
> FEC will not be used in SPL stage, so I not use CONFIG_IS_ENABLED.
> But it does not hurt using IS_ENABLED, change in v2.
> 
> Regards,
> Peng.
> 
>>
>>> +		dev = udev;
>>> +		if (!dev) {
>>> +			ret = uclass_get_device(UCLASS_ETH, idx, &dev);
>>> +			if (ret < 0) {
>>> +				debug("Can't get FEC udev: %d\n", ret);
>>> +				return ret;
>>> +			}
>>>    		}
>>> -	}
>>>
>>> -	fec = dev_get_priv(dev);
>>> -	if (fec)
>>> -		return fec->clk_rate;
>>> +		fec = dev_get_priv(dev);
>>> +		if (fec)
>>> +			return fec->clk_rate;
>>>
>>> -	return -EINVAL;
>>> -#else
>>> -	return imx_get_fecclk();
>>> -#endif
>>> +		return -EINVAL;
>>> +	} else {
>>> +		return imx_get_fecclk();
>>> +	}
>>>    }
>>>
>>>    static void fec_mii_setspeed(struct ethernet_regs *eth) @@ -1335,6
>>> +1336,49 @@ static int fecmxc_probe(struct udevice *dev)
>>>    			return ret;
>>>    		}
>>>
>>> +		priv->clk_rate = clk_get_rate(&priv->ipg_clk);
>>> +	} else if (IS_ENABLED(CONFIG_IMX8M) &&
>> IS_ENABLED(CONFIG_CLK_CCF)) {
>>
>> Same questions here as above.
>>
>>> +		ret = clk_get_by_name(dev, "ipg", &priv->ipg_clk);
>>> +		if (ret < 0) {
>>> +			debug("Can't get FEC ipg clk: %d\n", ret);
>>> +			return ret;
>>> +		} else {
>>
>> You can drop the else branches here and below as the code returns before it
>> will be evaluated.

I think you missed this comment for your v2.

>>
>>> +			ret = clk_enable(&priv->ipg_clk);
>>> +			if(ret)
>>> +				return ret;
>>> +		}
>>> +
>>> +		ret = clk_get_by_name(dev, "ipg", &priv->ahb_clk);
>>
>> This should be "ahb", not "ipg".

And this one, too.

>>
>>> +		if (ret < 0) {
>>> +			debug("Can't get FEC ahb clk: %d\n", ret);
>>> +			return ret;
>>> +		} else {
>>> +			ret = clk_enable(&priv->ahb_clk);
>>> +			if (ret)
>>> +				return ret;
>>> +		}
>>> +
>>> +		ret = clk_get_by_name(dev, "enet_out", &priv->clk_enet_out);
>>> +		if (!ret) {
>>> +			ret = clk_enable(&priv->clk_enet_out);
>>> +			if (ret)
>>> +				return ret;
>>> +		}
>>> +
>>> +		ret = clk_get_by_name(dev, "enet_clk_ref", &priv->clk_ref);
>>> +		if (!ret) {
>>> +			ret = clk_enable(&priv->clk_ref);
>>> +			if (ret)
>>> +				return ret;
>>> +		}
>>> +
>>> +		ret = clk_get_by_name(dev, "ptp", &priv->clk_ptp);
>>> +		if (!ret) {
>>> +			ret = clk_enable(&priv->clk_ptp);
>>> +			if (ret)
>>> +				return ret;
>>> +		}
>>> +
>>>    		priv->clk_rate = clk_get_rate(&priv->ipg_clk);
>>>    	}
>>>
>>> diff --git a/drivers/net/fec_mxc.h b/drivers/net/fec_mxc.h index
>>> e5f2dd75c5..723b06a651 100644
>>> --- a/drivers/net/fec_mxc.h
>>> +++ b/drivers/net/fec_mxc.h
>>> @@ -264,6 +264,10 @@ struct fec_priv {
>>>    	u32 interface;
>>>    #endif
>>>    	struct clk ipg_clk;
>>> +	struct clk ahb_clk;
>>> +	struct clk clk_enet_out;
>>> +	struct clk clk_ref;
>>> +	struct clk clk_ptp;
>>>    	u32 clk_rate;
>>>    };
>>>
>>>

Patch
diff mbox series

diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
index 080dbcf7db..9362aa0d05 100644
--- a/drivers/net/fec_mxc.c
+++ b/drivers/net/fec_mxc.c
@@ -125,28 +125,29 @@  static int fec_mdio_read(struct ethernet_regs *eth, uint8_t phyaddr,
 
 static int fec_get_clk_rate(void *udev, int idx)
 {
-#if IS_ENABLED(CONFIG_IMX8)
 	struct fec_priv *fec;
 	struct udevice *dev;
 	int ret;
 
-	dev = udev;
-	if (!dev) {
-		ret = uclass_get_device(UCLASS_ETH, idx, &dev);
-		if (ret < 0) {
-			debug("Can't get FEC udev: %d\n", ret);
-			return ret;
+	if (IS_ENABLED(CONFIG_IMX8) ||
+	    (IS_ENABLED(CONFIG_IMX8M) && IS_ENABLED(CONFIG_CLK_CCF))) {
+		dev = udev;
+		if (!dev) {
+			ret = uclass_get_device(UCLASS_ETH, idx, &dev);
+			if (ret < 0) {
+				debug("Can't get FEC udev: %d\n", ret);
+				return ret;
+			}
 		}
-	}
 
-	fec = dev_get_priv(dev);
-	if (fec)
-		return fec->clk_rate;
+		fec = dev_get_priv(dev);
+		if (fec)
+			return fec->clk_rate;
 
-	return -EINVAL;
-#else
-	return imx_get_fecclk();
-#endif
+		return -EINVAL;
+	} else {
+		return imx_get_fecclk();
+	}
 }
 
 static void fec_mii_setspeed(struct ethernet_regs *eth)
@@ -1335,6 +1336,49 @@  static int fecmxc_probe(struct udevice *dev)
 			return ret;
 		}
 
+		priv->clk_rate = clk_get_rate(&priv->ipg_clk);
+	} else if (IS_ENABLED(CONFIG_IMX8M) && IS_ENABLED(CONFIG_CLK_CCF)) {
+		ret = clk_get_by_name(dev, "ipg", &priv->ipg_clk);
+		if (ret < 0) {
+			debug("Can't get FEC ipg clk: %d\n", ret);
+			return ret;
+		} else {
+			ret = clk_enable(&priv->ipg_clk);
+			if(ret)
+				return ret;
+		}
+
+		ret = clk_get_by_name(dev, "ipg", &priv->ahb_clk);
+		if (ret < 0) {
+			debug("Can't get FEC ahb clk: %d\n", ret);
+			return ret;
+		} else {
+			ret = clk_enable(&priv->ahb_clk);
+			if (ret)
+				return ret;
+		}
+
+		ret = clk_get_by_name(dev, "enet_out", &priv->clk_enet_out);
+		if (!ret) {
+			ret = clk_enable(&priv->clk_enet_out);
+			if (ret)
+				return ret;
+		}
+
+		ret = clk_get_by_name(dev, "enet_clk_ref", &priv->clk_ref);
+		if (!ret) {
+			ret = clk_enable(&priv->clk_ref);
+			if (ret)
+				return ret;
+		}
+
+		ret = clk_get_by_name(dev, "ptp", &priv->clk_ptp);
+		if (!ret) {
+			ret = clk_enable(&priv->clk_ptp);
+			if (ret)
+				return ret;
+		}
+
 		priv->clk_rate = clk_get_rate(&priv->ipg_clk);
 	}
 
diff --git a/drivers/net/fec_mxc.h b/drivers/net/fec_mxc.h
index e5f2dd75c5..723b06a651 100644
--- a/drivers/net/fec_mxc.h
+++ b/drivers/net/fec_mxc.h
@@ -264,6 +264,10 @@  struct fec_priv {
 	u32 interface;
 #endif
 	struct clk ipg_clk;
+	struct clk ahb_clk;
+	struct clk clk_enet_out;
+	struct clk clk_ref;
+	struct clk clk_ptp;
 	u32 clk_rate;
 };