| Message ID | 20191022034714.27781-6-peng.fan@nxp.com |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Stefano Babic |
| Headers | show |
| Series | imx: imx8mm-evk: support eth | expand |
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; > }; > >
> 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; > > }; > > > >
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; >>> }; >>> >>>
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; };
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(-)