[U-Boot] [RFC] arm926ejs, at91: add common phy_reset function

["Andreas Bießmann" <andreas.devel@googlemail.com>]

Hello Heiko,

On 11/12/2013 11:21 AM, Heiko Schocher wrote:
> add common phy reset code into a common function.
> 
> Signed-off-by: Heiko Schocher <hs@denx.de>
> Cc: Andreas Bie?mann <andreas.devel@googlemail.com>
> Cc: Bo Shen <voice.shen@atmel.com>
> Cc: Jens Scharsig <esw@bus-elektronik.de>
> Cc: Sergey Lapin <slapin@ossfans.org>
> Cc: Stelian Pop <stelian@popies.net>
> Cc: Albin Tonnerre <albin.tonnerre@free-electrons.com>
> Cc: Eric Benard <eric@eukrea.com>
> Cc: Markus Hubig <mhubig@imko.de>
> 
> ---
> Patch based on the spl patchset from Bo Shen (as I want to
> collect this function in at91-common directory), see:
> http://lists.denx.de/pipermail/u-boot/2013-November/166272.html
> (reworked this against newest Kconfig Makefile changes ...
>  @Bo: Do you plan an update for this patchset for the Kconfig changes?

@Bo: I'll review the patches also these days.

> 
> Maybe my change in arch/arm/cpu/at91-common/Makefile
> could be done better... Do we have a common define for
> all this variants?

I think not, but how about defining a new one?

> 
> ---
>  arch/arm/cpu/Makefile                           |  1 +
>  arch/arm/cpu/at91-common/Makefile               |  5 +++
>  arch/arm/cpu/at91-common/phy.c                  | 48 +++++++++++++++++++++++++
>  arch/arm/include/asm/arch-at91/at91_common.h    |  1 +
>  board/BuS/vl_ma2sc/vl_ma2sc.c                   | 18 ++--------
>  board/afeb9260/afeb9260.c                       | 18 +---------
>  board/atmel/at91sam9260ek/at91sam9260ek.c       | 19 +---------
>  board/atmel/at91sam9263ek/at91sam9263ek.c       | 19 ++--------
>  board/atmel/at91sam9m10g45ek/at91sam9m10g45ek.c | 19 +---------
>  board/bluewater/snapper9260/snapper9260.c       | 16 +--------
>  board/calao/sbc35_a9g20/sbc35_a9g20.c           | 19 +---------
>  board/eukrea/cpu9260/cpu9260.c                  | 18 +---------
>  board/taskit/stamp9g20/stamp9g20.c              | 31 +---------------
>  spl/Makefile                                    |  4 ---
>  14 files changed, 66 insertions(+), 170 deletions(-)
>  create mode 100644 arch/arm/cpu/at91-common/phy.c
> 
> diff --git a/arch/arm/cpu/Makefile b/arch/arm/cpu/Makefile
> index fd0da53..886347d 100644
> --- a/arch/arm/cpu/Makefile
> +++ b/arch/arm/cpu/Makefile
> @@ -1,2 +1,3 @@
>  obj-$(CONFIG_TEGRA) += $(SOC)-common/
>  obj-$(CONFIG_TEGRA) += tegra-common/
> +obj-$(CONFIG_AT91FAMILY) += at91-common/
> diff --git a/arch/arm/cpu/at91-common/Makefile b/arch/arm/cpu/at91-common/Makefile
> index 040b956..255c7b9 100644
> --- a/arch/arm/cpu/at91-common/Makefile
> +++ b/arch/arm/cpu/at91-common/Makefile
> @@ -8,5 +8,10 @@
>  # SPDX-License-Identifier:	GPL-2.0+
>  #
>  
> +obj-$(CONFIG_AT91SAM9260) += phy.o
> +obj-$(CONFIG_AT91SAM9G20) += phy.o
> +obj-$(CONFIG_AT91SAM9263) += phy.o
> +obj-$(CONFIG_AT91SAM9XE) += phy.o
> +obj-$(CONFIG_AT91SAM9M10G45) += phy.o

How about defining some CONFIG_AT91_WANTS_PHY_RESET or
CONFIG_AT91_WANTS_COMMON_PHY?

>  obj-$(CONFIG_SPL_BUILD) += mpddrc.o
>  obj-$(CONFIG_SPL_BUILD) += spl.o
> diff --git a/arch/arm/cpu/at91-common/phy.c b/arch/arm/cpu/at91-common/phy.c
> new file mode 100644
> index 0000000..4706635
> --- /dev/null
> +++ b/arch/arm/cpu/at91-common/phy.c
> @@ -0,0 +1,48 @@
> +/*
> + * (C) Copyright 2007-2008
> + * Stelian Pop <stelian@popies.net>
> + * Lead Tech Design <www.leadtechdesign.com>
> + *
> + * Copyright (C) 2013 DENX Software Engineering, hs at denx.de
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <asm/io.h>
> +#include <asm/sizes.h>
> +#include <asm/arch/at91_pmc.h>
> +#include <asm/arch/at91_rstc.h>
> +#include <watchdog.h>
> +
> +void at91_phy_reset(void)
> +{
> +	unsigned long erstl;
> +	unsigned long start = get_timer(0);
> +	unsigned long timeout = 1000; /* 1000ms */

I'd constify timeout, it could give a hint to the compiler and it
doesn't hurt here.

> +	at91_rstc_t *rstc = (at91_rstc_t *)ATMEL_BASE_RSTC;
> +
> +	erstl = readl(&rstc->mr) & AT91_RSTC_MR_ERSTL_MASK;
> +
> +	/* Need to reset PHY -> 500ms reset */

As there where discussion about this trick here could you please add
some comments:

---8<---
Reset PHY by pulling the NRST line for 500ms to low. To do so disable
user reset for low level on NRST pin and poll the NRST level in reset
status register.
--->8---

> +	writel(AT91_RSTC_KEY | AT91_RSTC_MR_ERSTL(0x0D) |
> +		AT91_RSTC_MR_URSTEN, &rstc->mr);
> +
> +	writel(AT91_RSTC_KEY | AT91_RSTC_CR_EXTRST, &rstc->cr);
> +
> +	/* Wait for end of hardware reset */
> +	while (!(readl(&rstc->sr) & AT91_RSTC_SR_NRSTL)) {
> +		/* avoid shutdown by watchdog */
> +		WATCHDOG_RESET();
> +		mdelay(10);
> +
> +		/* timeout for not getting stuck in an endless loop */
> +		if (get_timer(start) >= timeout) {
> +			puts("*** ERROR: Timeout waiting for PHY reset!\n");
> +			break;
> +		}
> +	};
> +
> +	/* Restore NRST value */
> +	writel(AT91_RSTC_KEY | erstl | AT91_RSTC_MR_URSTEN, &rstc->mr);
> +}
> diff --git a/arch/arm/include/asm/arch-at91/at91_common.h b/arch/arm/include/asm/arch-at91/at91_common.h
> index 3ca4d5b..59e2f43 100644
> --- a/arch/arm/include/asm/arch-at91/at91_common.h
> +++ b/arch/arm/include/asm/arch-at91/at91_common.h
> @@ -26,5 +26,6 @@ void at91_plla_init(u32 pllar);
>  void at91_mck_init(u32 mckr);
>  void at91_pmc_init(void);
>  void mem_init(void);
> +void at91_phy_reset(void);
>  
>  #endif /* AT91_COMMON_H */

<snip>

> diff --git a/board/taskit/stamp9g20/stamp9g20.c b/board/taskit/stamp9g20/stamp9g20.c
> index 704a63b..27cdf77 100644
> --- a/board/taskit/stamp9g20/stamp9g20.c
> +++ b/board/taskit/stamp9g20/stamp9g20.c
> @@ -19,7 +19,6 @@
>  #include <asm/arch/at91sam9_smc.h>
>  #include <asm/arch/at91_common.h>
>  #include <asm/arch/at91_pmc.h>
> -#include <asm/arch/at91_rstc.h>
>  #include <asm/arch/gpio.h>
>  #include <watchdog.h>
>  
> @@ -67,8 +66,6 @@ static void stamp9G20_nand_hw_init(void)
>  static void stamp9G20_macb_hw_init(void)
>  {
>  	struct at91_port *pioa = (struct at91_port *)ATMEL_BASE_PIOA;
> -	struct at91_rstc *rstc = (struct at91_rstc *)ATMEL_BASE_RSTC;
> -	unsigned long erstl;
>  
>  	/* Enable the PHY Chip via PA26 on the Stamp 2 Adaptor */
>  	at91_set_gpio_output(AT91_PIN_PA26, 0);
> @@ -91,33 +88,7 @@ static void stamp9G20_macb_hw_init(void)
>  		pin_to_mask(AT91_PIN_PA28),
>  		&pioa->pudr);
>  
> -	erstl = readl(&rstc->mr) & AT91_RSTC_MR_ERSTL_MASK;
> -
> -	/* Need to reset PHY -> 500ms reset */
> -	writel(AT91_RSTC_KEY | (AT91_RSTC_MR_ERSTL(13) &
> -				~AT91_RSTC_MR_URSTEN), &rstc->mr);
> -	writel(AT91_RSTC_KEY | AT91_RSTC_CR_EXTRST, &rstc->cr);
> -
> -	/* Wait for end of hardware reset */
> -	unsigned long start = get_timer(0);
> -	unsigned long timeout = 1000; /* 1000ms */
> -
> -	while (!(readl(&rstc->sr) & AT91_RSTC_SR_NRSTL)) {
> -
> -		/* avoid shutdown by watchdog */
> -		WATCHDOG_RESET();
> -		mdelay(10);
> -
> -		/* timeout for not getting stuck in an endless loop */
> -		if (get_timer(start) >= timeout) {
> -			puts("*** ERROR: Timeout waiting for PHY reset!\n");
> -			break;

Your code looks like this one, you should add Markus Hubig to your
Copyright list.

> -		};
> -	};
> -
> -	/* Restore NRST value */
> -	writel(AT91_RSTC_KEY | erstl | AT91_RSTC_MR_URSTEN,
> -		&rstc->mr);
> +	at91_phy_reset();
>  
>  	/* Re-enable pull-up */
>  	writel(pin_to_mask(AT91_PIN_PA14) |
> diff --git a/spl/Makefile b/spl/Makefile
> index 736c6ca..cbd3d27 100644
> --- a/spl/Makefile
> +++ b/spl/Makefile
> @@ -111,10 +111,6 @@ ifneq ($(CONFIG_MX23)$(CONFIG_MX35),)
>  LIBS-y += arch/$(ARCH)/imx-common/libimx-common.o
>  endif
>  
> -ifeq ($(SOC),at91)
> -LIBS-y += arch/$(ARCH)/cpu/at91-common/libat91-common.o
> -endif
> -

That should not be removed here.

Looks good to me. Hopefully some board maintainers send their tested-by ...

Best regards

Andreas Bie?mann