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

Heiko Schocher hs at denx.de
Tue Nov 12 14:50:07 CET 2013


Hello Andreas,

Am 12.11.2013 13:56, schrieb Andreas Bießmann:
> 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 at denx.de>
>> Cc: Andreas Bießmann<andreas.devel at googlemail.com>
>> Cc: Bo Shen<voice.shen at atmel.com>
>> Cc: Jens Scharsig<esw at bus-elektronik.de>
>> Cc: Sergey Lapin<slapin at ossfans.org>
>> Cc: Stelian Pop<stelian at popies.net>
>> Cc: Albin Tonnerre<albin.tonnerre at free-electrons.com>
>> Cc: Eric Benard<eric at eukrea.com>
>> Cc: Markus Hubig<mhubig at 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.

Perfect!

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

I am fine with this too...

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

I vote for 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 at 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.

Ok.

>> +	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:

Ok.

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

Ok.

>> -		};
>> -	};
>> -
>> -	/* 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.

See my change in arch/arm/cpu/Makefile

With this change, this in the spl/Makefile is not needed ...
I did this, because arch/arm/cpu/at91-common/ contains not only
spl code. But maybe this should be changed in the spl patchset
from bo?

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

I am too...

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


More information about the U-Boot mailing list