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; };