public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Lukasz Majewski <l.majewski@samsung.com>
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 08:44:16 +0100	[thread overview]
Message-ID: <20150318084416.73b39d3c@amdc2363> (raw)
In-Reply-To: <55091BFC.7060405@denx.de>

Hi Heiko,

> 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 ... :-(

Ok, I see. Then I will apply the proposed patch.

> 
> > 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) ...

Thanks for explanation. I just wanted to have bigger picture of the
problem.

> 
> bye,
> Heiko


-- 
Best regards,

Lukasz Majewski

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

  reply	other threads:[~2015-03-18  7:44 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
2015-03-18  7:44             ` Lukasz Majewski [this message]
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=20150318084416.73b39d3c@amdc2363 \
    --to=l.majewski@samsung.com \
    --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