From: Heiko Schocher <hs@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [RFC] arm926ejs, at91: add common phy_reset function
Date: Tue, 12 Nov 2013 14:50:07 +0100 [thread overview]
Message-ID: <5282320F.4010205@denx.de> (raw)
In-Reply-To: <52822564.3010905@gmail.com>
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@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.
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@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
next prev parent reply other threads:[~2013-11-12 13:50 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-12 10:21 [U-Boot] [RFC] arm926ejs, at91: add common phy_reset function Heiko Schocher
2013-11-12 12:56 ` Andreas Bießmann
2013-11-12 13:50 ` Heiko Schocher [this message]
2013-11-13 1:35 ` Bo Shen
2013-11-13 5:04 ` Heiko Schocher
2013-11-13 8:15 ` Bo Shen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5282320F.4010205@denx.de \
--to=hs@denx.de \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.