[U-Boot] ls1021atwr reset issue (was [ANN] U-Boot v2015.10-rc4 released)

[Sinan Akman]

   Hi Fabio

On 30/09/15 03:03 PM, Fabio Estevam wrote:
>> [...]
>> Ok, the issue is due to endianness: on LS1021 the watchdog is
>> big-endian, so that's why we can't use clrsetbits_le16().
>>
>> Please check:
>> http://git.freescale.com/git/cgit.cgi/layerscape/ls1021a/linux.git/commit/?id=b53a344d20f6ffdc383d990a9636efb53ce272a9
>>
>> What about this?
>>
>> --- a/drivers/watchdog/imx_watchdog.c
>> +++ b/drivers/watchdog/imx_watchdog.c
>> @@ -54,8 +54,11 @@ void hw_watchdog_init(void)
>>   void reset_cpu(ulong addr)
>>   {
>>          struct watchdog_regs *wdog = (struct watchdog_regs *)WDOG1_BASE_ADDR;
>> +       int reg;
> ops, this should be u16 instead.
>
>> -       clrsetbits_le16(&wdog->wcr, 0, WCR_WDE);
>> +       reg = readw(&wdog->wcr);
>> +       reg |= WCR_WDE;
>> +       writew(reg, &wdog->wcr);
>>
>>          writew(0x5555, &wdog->wsr);
>>          writew(0xaaaa, &wdog->wsr);     /* load minimum 1/2 second timeout */

   I took a further look at this. I believe this won't work as readw
will now return the data in the wrong endianness. I think Wolfgang
also already pointed out at this. We would need to use the macro with
correct endianness for different platforms (ls1021atwr would need
big endian) and control this based on the watchdog endian setting in dts.

   However, there seems to be other problems here. The original
fix was intending to preserve the current set bits in wcr and
enable DTE. But in the case of ls1021atwr, the watcdog is not
initialized and out of reset we have SRS bit is set. This would
disable wdog_rst and prevent reset from working. Before the
fix it was actually the accidental setting of SRS to zero made
the reset working because a writew(WCR_WDE, &wdog->wcr)
would clear that bit, setting WDE alone would not generate wdog_rst.

   So I believe a proper fix would have the following steps :
   - move watchdog driver to DM so that we can make use of endian type
   - use macros with the endian type for accessing the registers
   - enable hw_watchdog for the board so that it gets initialized to
a known value
   - reset SRS to enable generation of wdog_rst, this will reset immediately
with 0x00 default WT value.
   - or program the timeout and enable WDE so the board
resets after a longer timeout value if desired

   Does this make sense to everyone ?

   Regards
   Sinan Akman