From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from metis.ext.pengutronix.de ([2001:6f8:1178:4:290:27ff:fe1d:cc33]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1YIFw3-0001Wq-7p for barebox@lists.infradead.org; Mon, 02 Feb 2015 12:17:00 +0000 Date: Mon, 2 Feb 2015 13:16:32 +0100 From: Sascha Hauer Message-ID: <20150202121632.GC12209@pengutronix.de> References: <20150202104957.GA10842@pengutronix.de> <20150202110432.GA12209@pengutronix.de> <20150202205038.451E.AA925319@jp.panasonic.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20150202205038.451E.AA925319@jp.panasonic.com> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "barebox" Errors-To: barebox-bounces+u.kleine-koenig=pengutronix.de@lists.infradead.org Subject: Re: [PATCH] watchdog: imx: error out on negative timeouts To: Masahiro Yamada Cc: barebox@lists.infradead.org, Uwe Kleine-Konig On Mon, Feb 02, 2015 at 08:50:38PM +0900, Masahiro Yamada wrote: > > On Mon, 2 Feb 2015 12:04:32 +0100 > Sascha Hauer 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 > > > > --- > > > > 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 If possible please base your patches on the master branch. I'll pick a suitable for-next/* branch merge the for-next/* branches together and handle the merge conflicts. If you have direct dependencies on some patches in -next you can base your patches on -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.) yes. > > > 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. Right. > [4] Some commits in topic branches might be dropped rather than being git-reverted > if they turned out to be bad. Right. Also you can always send a patch committed with "--fixup=" to the list, then I can just squash the changes into the original commit. I routinely build every commit in -next with every defconfig in the tree. It regularly happens that patches do not build in every defconfig because of missing ifdefs or dependencies. Most of the times I fix that up quietly. > (i.e. commit ID becomes a fixed value when it is merge into the master branch.) Yes. > > > 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.) Yes. > > 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. As long xxxxxxxxxxxx hasn't hit master you can just ask me to fix the offending commit directly, preferably using --fixup= to git-commit. Once it hits master the commit IDs are stable, so you can refer to them in bug fix patches. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox