From mboxrd@z Thu Jan  1 00:00:00 1970
From: Sinan Akman <sinan@writeme.com>
Date: Fri, 2 Oct 2015 09:04:26 -0400
Subject: [U-Boot] [PATCH] Revert "imx: wdog: correct wcr register
 settings"
In-Reply-To: <CAOMZO5CEA2k-5+7iexacvgCt1ogDHUs_=vuKWWN6b7tE=fX0Gw@mail.gmail.com>
References: <1443727970-10347-1-git-send-email-festevam@gmail.com>
 <20151001201143.865D8380905@gemini.denx.de>
 <CAOMZO5BEfS10XVztnigMejMVJYLvv+jqDLZYom9K8-G+Zi1TXA@mail.gmail.com>
 <20151001205008.732FB3822C0@gemini.denx.de>
 <CAOMZO5CAQXVzpX9mW7uHKB98JnMBeuT3ZjiUe4-V4=adjRk0zg@mail.gmail.com>
 <20151002043033.1486D3822C0@gemini.denx.de>
 <CAOMZO5B+EKkS6GE5+EXFpra9NtbvuTG5G+650gm4yHmQ4ioq+g@mail.gmail.com>
 <CAOMZO5CEA2k-5+7iexacvgCt1ogDHUs_=vuKWWN6b7tE=fX0Gw@mail.gmail.com>
Message-ID: <560E80DA.8030700@writeme.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de



On 02/10/15 07:39 AM, Fabio Estevam wrote:
> On Fri, Oct 2, 2015 at 8:10 AM, Fabio Estevam <festevam@gmail.com> wrote:
>> On Fri, Oct 2, 2015 at 1:30 AM, Wolfgang Denk <wd@denx.de> wrote:
>>
>>> In message <CAOMZO5CAQXVzpX9mW7uHKB98JnMBeuT3ZjiUe4-V4=adjRk0zg@mail.gmail.com> you
>>> But if we agree that both are LE accessors, and that the register is
>>> BE, then how does it work at all - we would be writing the wrong bit?
>> Watchdog on LS1021 works by accident rather than by design.
>>
>> What we are trying to do is to avoid the regression on LS1021 for the
>> 2015.10 release.
>>
>> Then a proper watchdog driver implementation is needed for 2016.01 so
>> that it takes care of the endianness.
>>
>> Is this approach acceptable?
> Or what about providing a reset_cpu() for LS102x that uses the proper
> endianness? Would this be a better approach?
>
> Sinan, does it work?
>
> --- a/arch/arm/cpu/armv7/ls102xa/cpu.c
> +++ b/arch/arm/cpu/armv7/ls102xa/cpu.c
> @@ -13,6 +13,7 @@
>   #include <tsec.h>
>   #include <netdev.h>
>   #include <fsl_esdhc.h>
> +#include <config.h>
>
>   #include "fsl_epu.h"
>
> @@ -354,3 +355,25 @@ void smp_kick_all_cpus(void)
>       asm volatile("sev");
>   }
>   #endif
> +
> +struct watchdog_regs {
> +    u16    wcr;    /* Control */
> +    u16    wsr;    /* Service */
> +    u16    wrsr;    /* Reset Status */
> +};
> +
> +#define WCR_WDE        0x04    /* WDOG enable */
> +void reset_cpu(ulong addr)
> +{
> +    struct watchdog_regs *wdog = (struct watchdog_regs *)WDOG1_BASE_ADDR;
> +
> +    out_be16(&wdog->wcr, WCR_WDE);

   I'll test this little later on when I am in the lab, but why are
we setting WCR_WDE anyways. We are not re-setting a new time
out value so this should be irrelevant.

   All we need is to clear the SRS bit, no need to set WCR_WDE
and no 5555/aaaa service sequence. I tested this earlier I know
it works. So a correct patch for reset_cpu() for LS102x could be
a single line SRS bit clear via _be32 which is all what we are intending
to.

   Regards
   Sinan Akman

> +
> +    out_be16(&wdog->wsr, 0x5555);
> +    out_be16(&wdog->wsr, 0xaaaa);    /* load minimum 1/2 second timeout */
> +    while (1) {
> +        /*
> +         * spin for .5 seconds before reset
> +         */
> +    }
> +}
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 9e9cb55..a007ae8 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -7,7 +7,7 @@
>
>   obj-$(CONFIG_AT91SAM9_WATCHDOG) += at91sam9_wdt.o
>   obj-$(CONFIG_FTWDT010_WATCHDOG) += ftwdt010_wdt.o
> -ifneq (,$(filter $(SOC), mx31 mx35 mx5 mx6 mx7 vf610 ls102xa))
> +ifneq (,$(filter $(SOC), mx31 mx35 mx5 mx6 mx7 vf610))
>   obj-y += imx_watchdog.o
>   endif
>   obj-$(CONFIG_S5P)               += s5p_wdt.o