Re: [PATCH] watchdog: imx: error out on negative timeouts

[Masahiro Yamada <yamada.m@jp.panasonic.com>]

On Mon, 2 Feb 2015 12:04:32 +0100
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> On Mon, Feb 02, 2015 at 11:49:57AM +0100, Uwe Kleine-Konig wrote:
> > Hello,
> > 
> > On Tue, Jan 27, 2015 at 02:41:01PM +0100, Uwe Kleine-Konig wrote:
> > > I'm not sure where a negative timeout could come from but making the
> > > code more robust for no additional runtime cost is good nevertheless.
> > > 
> > > Signed-off-by: Uwe Kleine-Konig <u.kleine-koenig@pengutronix.de>
> > > ---
> > >  drivers/watchdog/imxwd.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/watchdog/imxwd.c b/drivers/watchdog/imxwd.c
> > > index 31c3d0d85353..66e9f6848f74 100644
> > > --- a/drivers/watchdog/imxwd.c
> > > +++ b/drivers/watchdog/imxwd.c
> > > @@ -87,7 +87,7 @@ static int imx21_watchdog_set_timeout(struct imx_wd *priv, int timeout)
> > >  
> > >  	dev_dbg(priv->dev, "%s: %d\n", __func__, timeout);
> > >  
> > > -	if (!timeout || timeout > 128)
> > > +	if (timeout <= 0 || timeout > 128)
> > >  		return -EINVAL;
> > This patch is broken because reset_cpu (in the same source file) calls
> > set_timeout with timeout=-1 to reset the cpu immediatly. The wd command
> > only parses non-negative values, so from there nothing strange should be
> > expected.
> > 
> > Returning -EINVAL on timeout=0 (which means "disable watchdog") is OK
> > because the imx21 watchdog cannot be stopped.
> > 
> > So in short: please drop this patch from next.
> 
> Did that.
> 


Forgive my newbie questions.

I have been studied barebox for one month and a half,
I think I could roughly understand the next branch policy in this community,
but could you help me to make it a little bit clearer?


I guess the barebox/next is similar to the linux-next repository.

Similar points are:
 [1] barebox/next is not fast-forwarded
     (Developers should work with "git rebase --onto" as we do in linux-next
 [2] barebox/next represents the source tree that is *probably* merged into the master branch
     after the next release.
     (Perhaps, barebox might not have what we call Merge Window,
      but I notice topic branches are merged right after every-month release.)


On the other hand, I notice some differences

[3] All the topic branches are locally maintained by Sascha, so they are never pushed
    to the public repository.
[4] Some commits in topic branches might be dropped rather than being git-reverted
    if they turned out to be bad.
    (i.e. commit ID becomes a fixed value when it is merge into the master branch.)


If [4] is true, we should not describe the commit ID in the following commits
until it is merged into the master branch.   (Or we should be very careful when we do so.)

We often write something like
"Since commit xxxxxxxxxxxx, the foo function has not been working. Blah Blah ..."
in bug-fix patches.
But xxxxxxxxxxxx may change if the preceding commit is dropped or modified.


Is this statement correct?


Best Regards
Masahiro Yamada


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox