public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Heiko Schocher <hs@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] dfu: cmd: trigger watchdog before calling usb_gadget_handle_interrupts
Date: Wed, 18 Mar 2015 07:32:28 +0100	[thread overview]
Message-ID: <55091BFC.7060405@denx.de> (raw)
In-Reply-To: <20150317161614.5dbf495f@amdc2363>

Hello Lukasz,

Am 17.03.2015 16:16, schrieb Lukasz Majewski:
> Hi Heiko,
>
>> Hello Lukasz,
>>
>> Am 17.03.2015 13:56, schrieb Lukasz Majewski:
>>> Hi Heiko,
>>>
>>>> Hello Lukasz,
>>>>
>>>> Am 17.03.2015 10:52, schrieb Lukasz Majewski:
>>>>> Hi Heiko,
>>>>>
>>>>>> trigger watchdog before calling usb_gadget_handle_interrupts()
>>>>>> This prevents board resets when calling dfu command on boards
>>>>>> which have a watchdog.
>>>>>>
>>>>>> Signed-off-by: Heiko Schocher <hs@denx.de>
>>>>>> ---
>>>>>>
>>>>>>     common/cmd_dfu.c | 2 ++
>>>>>>     1 file changed, 2 insertions(+)
>>>>>>
>>>>>> diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c
>>>>>> index e975abe..46af4cf 100644
>>>>>> --- a/common/cmd_dfu.c
>>>>>> +++ b/common/cmd_dfu.c
>>>>>> @@ -9,6 +9,7 @@
>>>>>>      */
>>>>>>
>>>>>>     #include <common.h>
>>>>>> +#include <watchdog.h>
>>>>>>     #include <dfu.h>
>>>>>>     #include <g_dnl.h>
>>>>>>     #include <usb.h>
>>>>>> @@ -64,6 +65,7 @@ static int do_dfu(cmd_tbl_t *cmdtp, int flag,
>>>>>> int argc, char * const argv[]) if (ctrlc())
>>>>>>     			goto exit;
>>>>>>
>>>>>> +		WATCHDOG_RESET();
>>>>>>     		usb_gadget_handle_interrupts();
>>>>>>     	}
>>>>>>     exit:
>>>>>
>>>>> It seems strange for me, that we must reset watchdog when looping
>>>>> in the dfu.
>>>>
>>>> Hmm.. maybe I overlook something, but If you look into this
>>>> while(1) loop, there is no trigger of the watchdog ... and if I
>>>> start the dfu command without a USB cable on the board, what
>>>> triggers the boards watchdog?
>>>
>>> So the problem is when cable is not attached to the board and you
>>> call dfu command to execute?
>>
>> Yes.
>
> For UMS gadget there is defined g_dnl_board_usb_cable_connected()
> function.
>
> However, it is not yet supported in dfu and requires working MUIC
> block, which might be not available at your board.

Maybe, I must check this ...

>>>>> What is the WATCHDOG interval on the affected board?
>>>>
>>>> ~5 seconds
>>>>
>>>> Ah, this is on an at91 board .. and in the
>>>> drivers/serial/atmel_usart.c atmel_serial_tstc() is no
>>>> WATCHDOG_RESET...
>>>>
>>>> So ctrlc() does not trigger the watchdog
>>>
>>> Could you be a bit more specific here. Does your board expect
>>> ctrlc() to update watchdog, so it won't reset?
>>
>> I do not know, if ctrlc() must trigger the WDT ... I just looked into
>> the while(1) loop in common/cmd_dfu.c. There is only ctrlc() which
>> could trigger the WDT... and on at91 it does not trigger it ...
>
> By trigger you mean reset WDT core and don't reset the board?

I mean, the code must trigger the WDT somewhere in this while(1).
If not, the WDT resets the board.

>> If dfu transfer is started there is some output on the console, which
>> triggers the WDT too ... do you have a board with a running WDT?
>
> On trats/trats2 we disable WDT when we enter the u-boot.

Why?

So you can test this behaviour. Do not disable the WDT and start
the dfu command ... the board should reset when you start the dfu
command (without starting dfu-util on the host, and if ctrl() does not
call WATCHDOG_RESET for your hw) ... with my patch it should work ...

BTW: I could not disable the WDT on this board, once it is enabled.
So disabling the WDT is no option ... :-(

> I can imagine following situation when WDT is enabled when u-boot
> starts (its timeout is ~5sec) and then you start dfu transfer with not
> connected cable.
> Then 5sec pass since cable is not connected and no transfer is ongoing.
> This causes board reset by WDT.
>
> Am I right?

Yes, because the WDT gets not triggered in this while(1). And maybe
if you start the dfu command with connected cable, but not starting
dfu-util on the host also, but I have not yet running the usb gadget
on this at91 board (at91sam9g20 based) ...

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

  parent reply	other threads:[~2015-03-18  6:32 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-12 14:34 [U-Boot] [PATCH] dfu: cmd: trigger watchdog before calling usb_gadget_handle_interrupts Heiko Schocher
2015-03-17  9:52 ` Lukasz Majewski
2015-03-17 10:48   ` Heiko Schocher
2015-03-17 12:56     ` Lukasz Majewski
2015-03-17 13:27       ` Heiko Schocher
2015-03-17 15:16         ` Lukasz Majewski
2015-03-17 16:09           ` Tom Rini
2015-03-17 21:36             ` Lukasz Majewski
2015-03-18  6:32           ` Heiko Schocher [this message]
2015-03-18  7:44             ` Lukasz Majewski
2015-03-18  8:10               ` Heiko Schocher
2015-03-19 11:11 ` Lukasz Majewski

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=55091BFC.7060405@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox