[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