> 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....@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;
> >   };
> >
> >
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to