[U-Boot] [PATCH] dfu: cmd: trigger watchdog before calling usb_gadget_handle_interrupts

[Tom Rini]

On Tue, Mar 17, 2015 at 04:16:14PM +0100, Lukasz Majewski wrote:
> 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 at 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.
> 
> > 
> > >>> 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?
> 
> > 
> > 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.

This seems wrong.  We should enable the WDT and then be "petting" or
whatever the right term is for saying "Hey WDT, I'm alive!" which is
what WATCHDOG_RESET() is really about.

-- 
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/20150317/7d58126a/attachment.sig>