[U-Boot] [PATCH 2/8] net: zynq: Add clk framework support to zynq ethernet driver

[Stefan.Herbrechtsmeier at weidmueller.com]

> -----Ursprüngliche Nachricht-----
> Von: Michal Simek [mailto:monstr at monstr.eu]
> Gesendet: Dienstag, 10. Januar 2017 14:59
> An: Herbrechtsmeier, Stefan; u-boot at lists.denx.de
> Cc: Herbrechtsmeier, Stefan; Albert Aribaud; Michal Simek; Joe
> Hershberger
> Betreff: Re: [PATCH 2/8] net: zynq: Add clk framework support to zynq
> ethernet driver
> 
> On 4.1.2017 13:27, stefan.herbrechtsmeier at weidmueller.com wrote:
> > From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier at weidmueller.de>
> >
> > If available use the clock framework to set the tx clock rate of the
> > zynq ethernet controller.
> >
> > Signed-off-by: Stefan Herbrechtsmeier
> > <stefan.herbrechtsmeier at weidmueller.com>
> > ---
> >
> >  arch/arm/include/asm/arch-zynqmp/sys_proto.h |  5 -----
> >  drivers/net/zynq_gem.c                       | 31
> ++++++++++++++++++++++++++++
> >  2 files changed, 31 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/arm/include/asm/arch-zynqmp/sys_proto.h
> > b/arch/arm/include/asm/arch-zynqmp/sys_proto.h
> > index 1db2bd6..bd633a6 100644
> > --- a/arch/arm/include/asm/arch-zynqmp/sys_proto.h
> > +++ b/arch/arm/include/asm/arch-zynqmp/sys_proto.h
> > @@ -8,11 +8,6 @@
> >  #ifndef _ASM_ARCH_SYS_PROTO_H
> >  #define _ASM_ARCH_SYS_PROTO_H
> >
> > -/* Setup clk for network */
> > -static inline void zynq_slcr_gem_clk_setup(u32 gem_id, unsigned long
> > clk_rate) -{ -}
> > -
> >  int zynq_slcr_get_mio_pin_status(const char *periph);
> >
> >  unsigned int zynqmp_get_silicon_version(void); diff --git
> > a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index
> > 9bfb89f..9118e49 100644
> > --- a/drivers/net/zynq_gem.c
> > +++ b/drivers/net/zynq_gem.c
> > @@ -9,6 +9,7 @@
> >   * SPDX-License-Identifier:	GPL-2.0+
> >   */
> >
> > +#include <clk.h>
> >  #include <common.h>
> >  #include <dm.h>
> >  #include <net.h>
> > @@ -180,6 +181,9 @@ struct zynq_gem_priv {
> >  	struct phy_device *phydev;
> >  	int phy_of_handle;
> >  	struct mii_dev *bus;
> > +#if defined(CONFIG_CLK) || defined(CONFIG_SPL_CLK)
> > +	struct clk tx_clk;
> > +#endif
> >  };
> >
> >  static inline int mdio_wait(struct zynq_gem_regs *regs) @@ -364,6
> > +368,9 @@ static int zynq_gem_init(struct udevice *dev)
> >  	u32 i, nwconfig;
> >  	int ret;
> >  	unsigned long clk_rate = 0;
> > +#if defined(CONFIG_CLK) || defined(CONFIG_SPL_CLK)
> > +	unsigned long clk_rate_rounded;
> > +#endif
> >  	struct zynq_gem_priv *priv = dev_get_priv(dev);
> >  	struct zynq_gem_regs *regs = priv->iobase;
> >  	struct emac_bd *dummy_tx_bd = &priv->tx_bd[TX_FREE_DESC]; @@ -
> 466,8
> > +473,22 @@ static int zynq_gem_init(struct udevice *dev)
> >  		break;
> >  	}
> >
> > +#if defined(CONFIG_CLK) || defined(CONFIG_SPL_CLK)
> > +	clk_rate_rounded = clk_set_rate(&priv->tx_clk, clk_rate);
> > +	if (IS_ERR_VALUE(clk_rate_rounded)) {
> > +		dev_err(dev, "failed to set tx clock rate\n");
> > +		return clk_rate_rounded;
> > +	}
> > +
> > +	ret = clk_enable(&priv->tx_clk);
> > +	if (ret && ret != -ENOSYS) {
> > +		dev_err(dev, "failed to enable tx clock\n");
> > +		return ret;
> > +	}
> > +#else
> >  	zynq_slcr_gem_clk_setup((ulong)priv->iobase !=
> >  				ZYNQ_GEM_BASEADDR0, clk_rate);
> > +#endif
> >
> >  	setbits_le32(&regs->nwctrl, ZYNQ_GEM_NWCTRL_RXEN_MASK |
> >  					ZYNQ_GEM_NWCTRL_TXEN_MASK);
> > @@ -678,6 +699,9 @@ static int zynq_gem_ofdata_to_platdata(struct
> udevice *dev)
> >  	struct eth_pdata *pdata = dev_get_platdata(dev);
> >  	struct zynq_gem_priv *priv = dev_get_priv(dev);
> >  	const char *phy_mode;
> > +#if defined(CONFIG_CLK) || defined(CONFIG_SPL_CLK)
> > +	int ret;
> > +#endif
> >
> >  	pdata->iobase = (phys_addr_t)dev_get_addr(dev);
> >  	priv->iobase = (struct zynq_gem_regs *)pdata->iobase; @@ -699,6
> > +723,13 @@ static int zynq_gem_ofdata_to_platdata(struct udevice
> *dev)
> >  	}
> >  	priv->interface = pdata->phy_interface;
> >
> > +#if defined(CONFIG_CLK) || defined(CONFIG_SPL_CLK)
> > +	ret = clk_get_by_index(dev, 2, &priv->tx_clk);
> > +	if (ret < 0) {
> > +		dev_err(dev, "failed to get clock\n");
> > +		return ret;
> > +	}
> > +#endif
> >
> >  	printf("ZYNQ GEM: %lx, phyaddr %x, interface %s\n", (ulong)priv-
> >iobase,
> >  	       priv->phyaddr, phy_string_for_interface(priv->interface));
> >
> 
> Can you please rebase it on the top of this?
> https://patchwork.ozlabs.org/patch/706419/
> 
> The reason why this patch was written in this way is that we don't have
> full zynqmp clk driver yet but we are using fixed clock where you can't
> run set_rate which fails.
> 
The patch assumes that the EMIO_ENET_TX_CLK is always fixed. Is this true?

Why don't you add a set_rate() function to the fixed clock?

Additionally the patch is missing an enable function call.

I don't think the emio configuration should be keep in the Ethernet driver as it is part of the clock configuration and doesn't influent the network interface.

Regards,
  Stefan



Kommanditgesellschaft - Sitz: Detmold - Amtsgericht Lemgo HRA 2790 - 
Komplementärin: Weidmüller Interface Führungsgesellschaft mbH - 
Sitz: Detmold - Amtsgericht Lemgo HRB 3924; 
Geschäftsführer: José Carlos Álvarez Tobar, Elke Eckstein, Dr. Peter Köhler, Jörg Timmermann;
USt-ID-Nr. DE124599660