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

[Michal Simek]

On 10.1.2017 15:46, Stefan.Herbrechtsmeier at weidmueller.com wrote:
>> -----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?

It would be a solution but I am not sure if fixed clock was designed for
this. I would expect that fixed clock only provided clock which you
can't change. It means we need to have something as transition when
u-boot has real clk driver for zynqmp.

> 
> Additionally the patch is missing an enable function call.

correct.

> 
> 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.

Please correct me if I am wrong. Are you saying that with these patches
especially 8/8 clock for emio will handled automatically?

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP SoCs


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170110/574fafc1/attachment.sig>