Messages in this thread | | | Date | Mon, 29 Apr 2019 15:19:07 +0200 | From | Uwe Kleine-König <> | Subject | Re: [PATCH v3 08/16] watchdog: imx_sc_wdt: drop warning after calling watchdog_init_timeout |
| |
Hello,
On Mon, Apr 29, 2019 at 05:37:12AM -0700, Guenter Roeck wrote: > On 4/29/19 3:15 AM, Uwe Kleine-König wrote: > > On Fri, Apr 19, 2019 at 08:15:53PM +0200, Wolfram Sang wrote: > > > The core will print out details now. > > > > > > Reviewed-by: Guenter Roeck <linux@roeck-us.net> > > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > > > --- > > > drivers/watchdog/imx_sc_wdt.c | 5 +---- > > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > > > > diff --git a/drivers/watchdog/imx_sc_wdt.c b/drivers/watchdog/imx_sc_wdt.c > > > index 86c2722f2a09..6dc24ceb1b2c 100644 > > > --- a/drivers/watchdog/imx_sc_wdt.c > > > +++ b/drivers/watchdog/imx_sc_wdt.c > > > > This driver isn't in next, and I don't know where to look for it. > > > > Branch watchdog-next of > git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git
Thanks, found it now.
> > > @@ -117,10 +117,7 @@ static int imx_sc_wdt_probe(struct platform_device *pdev) > > > imx_sc_wdd->parent = &pdev->dev; > > > imx_sc_wdd->timeout = DEFAULT_TIMEOUT; > > > - ret = watchdog_init_timeout(imx_sc_wdd, 0, &pdev->dev); > > > - if (ret) > > > - dev_warn(&pdev->dev, "Failed to set timeout value, using default\n"); > > > - > > > + watchdog_init_timeout(imx_sc_wdd, 0, &pdev->dev); > > > > One side effect is however that ret isn't set any more. So I wonder if a > > failure in watchdog_init_timeout() really makes the core print the > > details as expected. > > > > Sorry, I don't understand. The warning is printed in watchdog_init_timeout(). > What does that have to do with setting ret here or not ?
Ah, I thought the warning is done in the caller of the modified function. Maybe this means the commit log can be improved to for example:
The function watchdog_init_timeout() itself already emits a more informative warning on failure. So drop the driver specific one.
Otherwise my concerns are eliminated.
Thanks Uwe
-- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ |
| |