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

[Lukasz Majewski <l.majewski@majess.pl>]

On Tue, 17 Mar 2015 12:09:17 -0400
Tom Rini <trini@konsulko.com> wrote:

> 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@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, I agree with the above. However in trats/trats2 case it is not
necessary.

Best regards,
Lukasz
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150317/d7c29047/attachment.sig>