All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [ANN] U-Boot v2015.10-rc4 released
@ 2015-09-28 21:09 Tom Rini
  2015-09-29  6:41 ` Wolfgang Denk
                   ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: Tom Rini @ 2015-09-28 21:09 UTC (permalink / raw)
  To: u-boot

Hey all,

I've pushed v2015.10-rc4 out to the repository and tarballs should exist
soon.

We're coming up on the release date and I think things are in reasonable
shape overall.  Please speak up if there's some fixes I'm still
overlooking, I thought I grabbed them all by now but I may have still
missed something.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150928/283668fd/attachment.sig>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] [ANN] U-Boot v2015.10-rc4 released
  2015-09-28 21:09 [U-Boot] [ANN] U-Boot v2015.10-rc4 released Tom Rini
@ 2015-09-29  6:41 ` Wolfgang Denk
  2015-09-29 15:35 ` Lukasz Majewski
  2015-09-30 16:47 ` Sinan Akman
  2 siblings, 0 replies; 23+ messages in thread
From: Wolfgang Denk @ 2015-09-29  6:41 UTC (permalink / raw)
  To: u-boot

Dear Tom,

In message <20150928210901.GH22966@bill-the-cat> you wrote:
> 
> I've pushed v2015.10-rc4 out to the repository and tarballs should exist
> soon.

Tarballs are both on the FTP server [1] and the ACD [2].

[1] ftp://ftp.denx.de/pub/u-boot/
[2] https://www.amazon.com/clouddrive/share/9JvNre5ZS9KVI3kRyTjEQNYO5x9R5ai-CRXFuHdnWgs?ref_=cd_share_link_copy

> We're coming up on the release date and I think things are in reasonable
> shape overall.  Please speak up if there's some fixes I'm still
> overlooking, I thought I grabbed them all by now but I may have still
> missed something.

Thanks a lot!

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Yes, it's a technical challenge, and  you  have  to  kind  of  admire
people  who go to the lengths of actually implementing it, but at the
same time you wonder about their IQ...
         --  Linus Torvalds in <5phda5$ml6$1@palladium.transmeta.com>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] [ANN] U-Boot v2015.10-rc4 released
  2015-09-28 21:09 [U-Boot] [ANN] U-Boot v2015.10-rc4 released Tom Rini
  2015-09-29  6:41 ` Wolfgang Denk
@ 2015-09-29 15:35 ` Lukasz Majewski
  2015-09-30 16:47 ` Sinan Akman
  2 siblings, 0 replies; 23+ messages in thread
From: Lukasz Majewski @ 2015-09-29 15:35 UTC (permalink / raw)
  To: u-boot

Hi Tom,

> Hey all,
> 
> I've pushed v2015.10-rc4 out to the repository and tarballs should
> exist soon.
> 
> We're coming up on the release date and I think things are in
> reasonable shape overall.  Please speak up if there's some fixes I'm
> still overlooking, I thought I grabbed them all by now but I may have
> still missed something.
> 

I've found more issues with Exynos4 (Trats) regarding changes done in
the following commit:

"fdt: add new fdt address parsing functions"
SHA1: 02464e386bb5f0a022c121f95ae75cf583759d95

I'm strive to fix this issue ASAP.

-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] [ANN] U-Boot v2015.10-rc4 released
  2015-09-28 21:09 [U-Boot] [ANN] U-Boot v2015.10-rc4 released Tom Rini
  2015-09-29  6:41 ` Wolfgang Denk
  2015-09-29 15:35 ` Lukasz Majewski
@ 2015-09-30 16:47 ` Sinan Akman
  2015-09-30 17:16   ` York Sun
  2 siblings, 1 reply; 23+ messages in thread
From: Sinan Akman @ 2015-09-30 16:47 UTC (permalink / raw)
  To: u-boot



On 28/09/15 05:09 PM, Tom Rini wrote:
> Hey all,
>
> I've pushed v2015.10-rc4 out to the repository and tarballs should exist
> soon.
>
> We're coming up on the release date and I think things are in reasonable
> shape overall.  Please speak up if there's some fixes I'm still
> overlooking, I thought I grabbed them all by now but I may have still
> missed something.

   On ls1021atwr board (LS1021E, Version: 1.0, (0x87081110) with
ls1021atwr_sdcard_defconfig reset hangs on my board :

=> ver 

 

U-Boot 2015.10-rc4 (Sep 30 2015 - 12:29:15 -0400) 

arm-linux-gnueabi-gcc (GCC) 4.8.1 

GNU ld (GNU Binutils) 2.23.2

=> reset 

resetting ...


   Is anyone else seeing this ?

   Regards
   Sinan Akman

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] [ANN] U-Boot v2015.10-rc4 released
  2015-09-30 16:47 ` Sinan Akman
@ 2015-09-30 17:16   ` York Sun
  2015-09-30 17:22     ` Sinan Akman
  0 siblings, 1 reply; 23+ messages in thread
From: York Sun @ 2015-09-30 17:16 UTC (permalink / raw)
  To: u-boot



On 09/30/2015 09:47 AM, Sinan Akman wrote:
> 
> 
> On 28/09/15 05:09 PM, Tom Rini wrote:
>> Hey all,
>>
>> I've pushed v2015.10-rc4 out to the repository and tarballs should exist
>> soon.
>>
>> We're coming up on the release date and I think things are in reasonable
>> shape overall.  Please speak up if there's some fixes I'm still
>> overlooking, I thought I grabbed them all by now but I may have still
>> missed something.
> 
>    On ls1021atwr board (LS1021E, Version: 1.0, (0x87081110) with
> ls1021atwr_sdcard_defconfig reset hangs on my board :
> 
> => ver 
> 
>  
> 
> U-Boot 2015.10-rc4 (Sep 30 2015 - 12:29:15 -0400) 
> 
> arm-linux-gnueabi-gcc (GCC) 4.8.1 
> 
> GNU ld (GNU Binutils) 2.23.2
> 
> => reset 
> 
> resetting ...
> 
> 
>    Is anyone else seeing this ?
> 

The board maintainer will take a look. Unfortunately she is on holiday vacation.
Let me try to find someone else familiar with this board.

York

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] [ANN] U-Boot v2015.10-rc4 released
  2015-09-30 17:16   ` York Sun
@ 2015-09-30 17:22     ` Sinan Akman
  2015-09-30 18:02       ` Sinan Akman
  0 siblings, 1 reply; 23+ messages in thread
From: Sinan Akman @ 2015-09-30 17:22 UTC (permalink / raw)
  To: u-boot


   Hi York

> The board maintainer will take a look. Unfortunately she is on holiday vacation.
> Let me try to find someone else familiar with this board.

   No worries, I will also bisect and see what's going on.
Just wanted to make sure I don't work on it if someone is
already taking care. I'll take a look at in the coming
days.

   Regards
   Sinan Akman

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] [ANN] U-Boot v2015.10-rc4 released
  2015-09-30 17:22     ` Sinan Akman
@ 2015-09-30 18:02       ` Sinan Akman
  2015-09-30 18:16         ` Fabio Estevam
  0 siblings, 1 reply; 23+ messages in thread
From: Sinan Akman @ 2015-09-30 18:02 UTC (permalink / raw)
  To: u-boot



On 30/09/15 01:22 PM, Sinan Akman wrote:
>
>    Hi York
>
>> The board maintainer will take a look. Unfortunately she is on holiday
>> vacation.
>> Let me try to find someone else familiar with this board.
>
>    No worries, I will also bisect and see what's going on.
> Just wanted to make sure I don't work on it if someone is
> already taking care. I'll take a look at in the coming
> days.

   FYI,

   git revert 623d96e89aca64c2762150087f4e872c55481f13

   fixes the reset issue. I don't know yet why. I am cc'ing
to Peng as well.

   There are some other i2c warnings with this built,
"The regulator (MC34VR500) does not exist" .. etc. but
they are probably unrelated, I will take a look at them later.

   Regards
   Sinan Akman

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] [ANN] U-Boot v2015.10-rc4 released
  2015-09-30 18:02       ` Sinan Akman
@ 2015-09-30 18:16         ` Fabio Estevam
  2015-09-30 18:33           ` Sinan Akman
  0 siblings, 1 reply; 23+ messages in thread
From: Fabio Estevam @ 2015-09-30 18:16 UTC (permalink / raw)
  To: u-boot

Hi Sinan,

On Wed, Sep 30, 2015 at 3:02 PM, Sinan Akman <sinan@writeme.com> wrote:
>
>
> On 30/09/15 01:22 PM, Sinan Akman wrote:
>>
>>
>>    Hi York
>>
>>> The board maintainer will take a look. Unfortunately she is on holiday
>>> vacation.
>>> Let me try to find someone else familiar with this board.
>>
>>
>>    No worries, I will also bisect and see what's going on.
>> Just wanted to make sure I don't work on it if someone is
>> already taking care. I'll take a look at in the coming
>> days.
>
>
>   FYI,
>
>   git revert 623d96e89aca64c2762150087f4e872c55481f13
>
>   fixes the reset issue. I don't know yet why. I am cc'ing
> to Peng as well.

Does this fix the reset issue?

--- a/drivers/watchdog/imx_watchdog.c
+++ b/drivers/watchdog/imx_watchdog.c
@@ -55,7 +55,7 @@ void reset_cpu(ulong addr)
 {
        struct watchdog_regs *wdog = (struct watchdog_regs *)WDOG1_BASE_ADDR;

-       clrsetbits_le16(&wdog->wcr, 0, WCR_WDE);
+       setbits_le16(&wdog->wcr, WCR_WDE);

        writew(0x5555, &wdog->wsr);
        writew(0xaaaa, &wdog->wsr);     /* load minimum 1/2 second timeout */

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] [ANN] U-Boot v2015.10-rc4 released
  2015-09-30 18:16         ` Fabio Estevam
@ 2015-09-30 18:33           ` Sinan Akman
  2015-09-30 18:56             ` Fabio Estevam
  0 siblings, 1 reply; 23+ messages in thread
From: Sinan Akman @ 2015-09-30 18:33 UTC (permalink / raw)
  To: u-boot


   Hi Fabio

Fabio Estevam wrote:
> Hi Sinan,
> [...]
> 
> Does this fix the reset issue?
> 
> --- a/drivers/watchdog/imx_watchdog.c
> +++ b/drivers/watchdog/imx_watchdog.c
> @@ -55,7 +55,7 @@ void reset_cpu(ulong addr)
>  {
>         struct watchdog_regs *wdog = (struct watchdog_regs *)WDOG1_BASE_ADDR;
> 
> -       clrsetbits_le16(&wdog->wcr, 0, WCR_WDE);
> +       setbits_le16(&wdog->wcr, WCR_WDE);
> 
>         writew(0x5555, &wdog->wsr);
>         writew(0xaaaa, &wdog->wsr);     /* load minimum 1/2 second timeout */
> 

   Nope, AFAICS it doesn't make any difference.

   Regards
   Sinan Akman

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] [ANN] U-Boot v2015.10-rc4 released
  2015-09-30 18:33           ` Sinan Akman
@ 2015-09-30 18:56             ` Fabio Estevam
  2015-09-30 19:03               ` Fabio Estevam
  2015-10-01 14:22               ` [U-Boot] [ANN] U-Boot v2015.10-rc4 released Wolfgang Denk
  0 siblings, 2 replies; 23+ messages in thread
From: Fabio Estevam @ 2015-09-30 18:56 UTC (permalink / raw)
  To: u-boot

On Wed, Sep 30, 2015 at 3:33 PM, Sinan Akman <sinan@writeme.com> wrote:

>   Nope, AFAICS it doesn't make any difference.

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;

-       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 */

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] [ANN] U-Boot v2015.10-rc4 released
  2015-09-30 18:56             ` Fabio Estevam
@ 2015-09-30 19:03               ` Fabio Estevam
  2015-09-30 19:09                 ` Sinan Akman
  2015-10-01 15:19                 ` [U-Boot] ls1021atwr reset issue (was [ANN] U-Boot v2015.10-rc4 released) Sinan Akman
  2015-10-01 14:22               ` [U-Boot] [ANN] U-Boot v2015.10-rc4 released Wolfgang Denk
  1 sibling, 2 replies; 23+ messages in thread
From: Fabio Estevam @ 2015-09-30 19:03 UTC (permalink / raw)
  To: u-boot

On Wed, Sep 30, 2015 at 3:56 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Wed, Sep 30, 2015 at 3:33 PM, Sinan Akman <sinan@writeme.com> wrote:
>
>>   Nope, AFAICS it doesn't make any difference.
>
> 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 */

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] [ANN] U-Boot v2015.10-rc4 released
  2015-09-30 19:03               ` Fabio Estevam
@ 2015-09-30 19:09                 ` Sinan Akman
  2015-09-30 19:13                   ` Fabio Estevam
  2015-10-01 15:19                 ` [U-Boot] ls1021atwr reset issue (was [ANN] U-Boot v2015.10-rc4 released) Sinan Akman
  1 sibling, 1 reply; 23+ messages in thread
From: Sinan Akman @ 2015-09-30 19:09 UTC (permalink / raw)
  To: u-boot

Fabio Estevam wrote:
> On Wed, Sep 30, 2015 at 3:56 PM, Fabio Estevam <festevam@gmail.com> wrote:
>> On Wed, Sep 30, 2015 at 3:33 PM, Sinan Akman <sinan@writeme.com> wrote:
>>
>>>   Nope, AFAICS it doesn't make any difference.
>> 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 */
> 

   OK, this will probably work, as I tried writew earlier and it worked,
I agree it seems an endianness issue, do we not want to address
all common accesses in a way that takes endianness into account ?

   I'll test the above as well (with u16 reg) to make sure.

   Regards
   Sinan Akman

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] [ANN] U-Boot v2015.10-rc4 released
  2015-09-30 19:09                 ` Sinan Akman
@ 2015-09-30 19:13                   ` Fabio Estevam
  0 siblings, 0 replies; 23+ messages in thread
From: Fabio Estevam @ 2015-09-30 19:13 UTC (permalink / raw)
  To: u-boot

On Wed, Sep 30, 2015 at 4:09 PM, Sinan Akman <sinan@writeme.com> wrote:

>   OK, this will probably work, as I tried writew earlier and it worked,
> I agree it seems an endianness issue, do we not want to address
> all common accesses in a way that takes endianness into account ?

Yes, that would be nice. I don't have access to ls1021 hardware, so
can't do it myself.

>   I'll test the above as well (with u16 reg) to make sure.

Ok, great. Please let us know how it goes.

Thanks

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] [ANN] U-Boot v2015.10-rc4 released
  2015-09-30 18:56             ` Fabio Estevam
  2015-09-30 19:03               ` Fabio Estevam
@ 2015-10-01 14:22               ` Wolfgang Denk
  1 sibling, 0 replies; 23+ messages in thread
From: Wolfgang Denk @ 2015-10-01 14:22 UTC (permalink / raw)
  To: u-boot

Dear Fabio,

In message <CAOMZO5ATtUjk_TVS94N6evTKndozVRH9nXyE0UHPJ3sCQ906Zg@mail.gmail.com> you 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().
...
> +       int reg;
> 
> -       clrsetbits_le16(&wdog->wcr, 0, WCR_WDE);
> +       reg = readw(&wdog->wcr);
> +       reg |= WCR_WDE;
> +       writew(reg, &wdog->wcr);

But this is broken.  We have the clrsetbits() [*] macros exactly to
avoid such code.

[*] resp. setbits() - using clrsetbits*() with 0 as clear mask is
a stupid thing to do anyway.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
It is wrong always, everywhere and for everyone to  believe  anything
upon  insufficient  evidence.  - W. K. Clifford, British philosopher,
circa 1876

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] ls1021atwr reset issue (was [ANN] U-Boot v2015.10-rc4 released)
  2015-09-30 19:03               ` Fabio Estevam
  2015-09-30 19:09                 ` Sinan Akman
@ 2015-10-01 15:19                 ` Sinan Akman
  2015-10-01 19:09                   ` Fabio Estevam
  1 sibling, 1 reply; 23+ messages in thread
From: Sinan Akman @ 2015-10-01 15:19 UTC (permalink / raw)
  To: u-boot


   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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] ls1021atwr reset issue (was [ANN] U-Boot v2015.10-rc4 released)
  2015-10-01 15:19                 ` [U-Boot] ls1021atwr reset issue (was [ANN] U-Boot v2015.10-rc4 released) Sinan Akman
@ 2015-10-01 19:09                   ` Fabio Estevam
  2015-10-01 19:14                     ` Tom Rini
  0 siblings, 1 reply; 23+ messages in thread
From: Fabio Estevam @ 2015-10-01 19:09 UTC (permalink / raw)
  To: u-boot

On Thu, Oct 1, 2015 at 12:19 PM, Sinan Akman <sinan@writeme.com> wrote:
>   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.

The imx platforms do not use dts in U-boot yet.

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

Looks good. 2015.10 release is only a few days away: do you think you
could implement your proposal for the upcoming release?

Regards,

Fabio Estevam

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] ls1021atwr reset issue (was [ANN] U-Boot v2015.10-rc4 released)
  2015-10-01 19:09                   ` Fabio Estevam
@ 2015-10-01 19:14                     ` Tom Rini
  2015-10-01 19:17                       ` Otavio Salvador
  2015-10-01 19:28                       ` Sinan Akman
  0 siblings, 2 replies; 23+ messages in thread
From: Tom Rini @ 2015-10-01 19:14 UTC (permalink / raw)
  To: u-boot

On Thu, Oct 01, 2015 at 04:09:46PM -0300, Fabio Estevam wrote:
> On Thu, Oct 1, 2015 at 12:19 PM, Sinan Akman <sinan@writeme.com> wrote:
> >   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.
> 
> The imx platforms do not use dts in U-boot yet.
> 
> >
> >   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 ?
> 
> Looks good. 2015.10 release is only a few days away: do you think you
> could implement your proposal for the upcoming release?

That seems rather large.  Is there something we can revert today so
that:
a) All boards in v2015.07 continue to work in v2015.10 as well as before
b) Any new boards are not introduced in a totally broken state?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151001/d26c4ff9/attachment.sig>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] ls1021atwr reset issue (was [ANN] U-Boot v2015.10-rc4 released)
  2015-10-01 19:14                     ` Tom Rini
@ 2015-10-01 19:17                       ` Otavio Salvador
  2015-10-01 19:33                         ` Sinan Akman
  2015-10-01 19:28                       ` Sinan Akman
  1 sibling, 1 reply; 23+ messages in thread
From: Otavio Salvador @ 2015-10-01 19:17 UTC (permalink / raw)
  To: u-boot

On Thu, Oct 1, 2015 at 4:14 PM, Tom Rini <trini@konsulko.com> wrote:
> On Thu, Oct 01, 2015 at 04:09:46PM -0300, Fabio Estevam wrote:
>> On Thu, Oct 1, 2015 at 12:19 PM, Sinan Akman <sinan@writeme.com> wrote:
>> >   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.
>>
>> The imx platforms do not use dts in U-boot yet.
>>
>> >
>> >   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 ?
>>
>> Looks good. 2015.10 release is only a few days away: do you think you
>> could implement your proposal for the upcoming release?
>
> That seems rather large.  Is there something we can revert today so
> that:
> a) All boards in v2015.07 continue to work in v2015.10 as well as before
> b) Any new boards are not introduced in a totally broken state?

Agreed; I prefer to revert the watchdog changes and break ls102x than
introduce such big change.

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] ls1021atwr reset issue (was [ANN] U-Boot v2015.10-rc4 released)
  2015-10-01 19:14                     ` Tom Rini
  2015-10-01 19:17                       ` Otavio Salvador
@ 2015-10-01 19:28                       ` Sinan Akman
  2015-10-01 19:36                         ` Fabio Estevam
  1 sibling, 1 reply; 23+ messages in thread
From: Sinan Akman @ 2015-10-01 19:28 UTC (permalink / raw)
  To: u-boot



On 01/10/15 03:14 PM, Tom Rini wrote:
> On Thu, Oct 01, 2015 at 04:09:46PM -0300, Fabio Estevam wrote:
>> On Thu, Oct 1, 2015 at 12:19 PM, Sinan Akman <sinan@writeme.com> wrote:
>>>    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.
>> The imx platforms do not use dts in U-boot yet.
>>
>>>    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 ?
>> Looks good. 2015.10 release is only a few days away: do you think you
>> could implement your proposal for the upcoming release?
> That seems rather large.  Is there something we can revert today so
> that:
> a) All boards in v2015.07 continue to work in v2015.10 as well as before
> b) Any new boards are not introduced in a totally broken state?
   I agree, this is something needs to be done in steps and not in rush.
   For coming out of rc4 and making sure ls1021atwr reset works, all
we need is to make sure that SRS bit is set and that this bit is written
properly with big endian. We could add that specifically for ls1021atw.
Still there is something not nice about the current implementation that
is kinda assumes hw_watchdog is enabled and _init is called but it doesn't
check for this. I will come up with something to have ls1021atwr working
and see if it feels clean enough.

   Regards
   Sinan Akman

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] ls1021atwr reset issue (was [ANN] U-Boot v2015.10-rc4 released)
  2015-10-01 19:17                       ` Otavio Salvador
@ 2015-10-01 19:33                         ` Sinan Akman
  2015-10-01 19:38                           ` Fabio Estevam
  0 siblings, 1 reply; 23+ messages in thread
From: Sinan Akman @ 2015-10-01 19:33 UTC (permalink / raw)
  To: u-boot



On 01/10/15 03:17 PM, Otavio Salvador wrote:
> On Thu, Oct 1, 2015 at 4:14 PM, Tom Rini <trini@konsulko.com> wrote:
>> On Thu, Oct 01, 2015 at 04:09:46PM -0300, Fabio Estevam wrote:
>>> On Thu, Oct 1, 2015 at 12:19 PM, Sinan Akman <sinan@writeme.com> wrote:
>>>>    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.
>>> The imx platforms do not use dts in U-boot yet.
>>>
>>>>    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 ?
>>> Looks good. 2015.10 release is only a few days away: do you think you
>>> could implement your proposal for the upcoming release?
>> That seems rather large.  Is there something we can revert today so
>> that:
>> a) All boards in v2015.07 continue to work in v2015.10 as well as before
>> b) Any new boards are not introduced in a totally broken state?
> Agreed; I prefer to revert the watchdog changes and break ls102x than
> introduce such big change.
   Otavio, please note that the watchdog changes that are committed that 
we are referring
to broke ls102x. So if you revert those as you suggest here you would 
have ls102x working
again but it would break imx6 which the commit was for.

   Perhaps what you meant is, let's leave ls1021atwr broken and not 
change anything for now.
I think this is something Tom will probably have to decide. I would 
prefer we don't have anything
known broken before the main release.

   Regards
   Sinan Akman

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] ls1021atwr reset issue (was [ANN] U-Boot v2015.10-rc4 released)
  2015-10-01 19:28                       ` Sinan Akman
@ 2015-10-01 19:36                         ` Fabio Estevam
  0 siblings, 0 replies; 23+ messages in thread
From: Fabio Estevam @ 2015-10-01 19:36 UTC (permalink / raw)
  To: u-boot

On Thu, Oct 1, 2015 at 4:28 PM, Sinan Akman <sinan@writeme.com> wrote:

>   I agree, this is something needs to be done in steps and not in rush.
>   For coming out of rc4 and making sure ls1021atwr reset works, all
> we need is to make sure that SRS bit is set and that this bit is written
> properly with big endian. We could add that specifically for ls1021atw.

Safest approach would be to revert 623d96e89, right?

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] ls1021atwr reset issue (was [ANN] U-Boot v2015.10-rc4 released)
  2015-10-01 19:33                         ` Sinan Akman
@ 2015-10-01 19:38                           ` Fabio Estevam
  2015-10-01 19:41                             ` Sinan Akman
  0 siblings, 1 reply; 23+ messages in thread
From: Fabio Estevam @ 2015-10-01 19:38 UTC (permalink / raw)
  To: u-boot

On Thu, Oct 1, 2015 at 4:33 PM, Sinan Akman <sinan@writeme.com> wrote:

>   Otavio, please note that the watchdog changes that are committed that we
> are referring
> to broke ls102x. So if you revert those as you suggest here you would have
> ls102x working
> again but it would break imx6 which the commit was for.

Just to clarify: reverting 623d96e89aca does not break i.MX.

623d96e89aca has been introduced in v2015.10-rc4. It is safe to revert it.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [U-Boot] ls1021atwr reset issue (was [ANN] U-Boot v2015.10-rc4 released)
  2015-10-01 19:38                           ` Fabio Estevam
@ 2015-10-01 19:41                             ` Sinan Akman
  0 siblings, 0 replies; 23+ messages in thread
From: Sinan Akman @ 2015-10-01 19:41 UTC (permalink / raw)
  To: u-boot



On 01/10/15 03:38 PM, Fabio Estevam wrote:
> On Thu, Oct 1, 2015 at 4:33 PM, Sinan Akman <sinan@writeme.com> wrote:
>
>>    Otavio, please note that the watchdog changes that are committed that we
>> are referring
>> to broke ls102x. So if you revert those as you suggest here you would have
>> ls102x working
>> again but it would break imx6 which the commit was for.
> Just to clarify: reverting 623d96e89aca does not break i.MX.
>
> 623d96e89aca has been introduced in v2015.10-rc4. It is safe to revert it.

     OK

     Regards
     Sinan Akman

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2015-10-01 19:41 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-28 21:09 [U-Boot] [ANN] U-Boot v2015.10-rc4 released Tom Rini
2015-09-29  6:41 ` Wolfgang Denk
2015-09-29 15:35 ` Lukasz Majewski
2015-09-30 16:47 ` Sinan Akman
2015-09-30 17:16   ` York Sun
2015-09-30 17:22     ` Sinan Akman
2015-09-30 18:02       ` Sinan Akman
2015-09-30 18:16         ` Fabio Estevam
2015-09-30 18:33           ` Sinan Akman
2015-09-30 18:56             ` Fabio Estevam
2015-09-30 19:03               ` Fabio Estevam
2015-09-30 19:09                 ` Sinan Akman
2015-09-30 19:13                   ` Fabio Estevam
2015-10-01 15:19                 ` [U-Boot] ls1021atwr reset issue (was [ANN] U-Boot v2015.10-rc4 released) Sinan Akman
2015-10-01 19:09                   ` Fabio Estevam
2015-10-01 19:14                     ` Tom Rini
2015-10-01 19:17                       ` Otavio Salvador
2015-10-01 19:33                         ` Sinan Akman
2015-10-01 19:38                           ` Fabio Estevam
2015-10-01 19:41                             ` Sinan Akman
2015-10-01 19:28                       ` Sinan Akman
2015-10-01 19:36                         ` Fabio Estevam
2015-10-01 14:22               ` [U-Boot] [ANN] U-Boot v2015.10-rc4 released Wolfgang Denk

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.