On Wed, Aug 27, 2014 at 12:01:35PM -0700, Guenter Roeck wrote:
[...]
> > +#include <linux/device.h>
> > +#include <linux/mfd/rn5t618.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/watchdog.h>
> > +
> > +#define DRIVER_NAME "rn5t618-wdt"
> > +
> > +static bool nowayout = WATCHDOG_NOWAYOUT;
> > +static unsigned int heartbeat = -1;
>
> I'd be surprised if this doesn't cause a compiler warning.
> Why not just initialize the variable with 0 ?
The idea was to initialize the variable with an invalid value that has
no effect when watchdog_init_timeout() is called during probe, thus
leaving the watchdog timeout to the maximum value.
You are right, the same effect can be achieved in a cleaner way
leaving the variable to zero (by the way, the above assignment doesn't
seem to generate warnings).
>
> > +
> > +module_param(heartbeat, int, 0);
> > +MODULE_PARM_DESC(heartbeat, "Initial watchdog heartbeat in seconds");
> > +
> > +module_param(nowayout, bool, 0);
> > +MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
> > + __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
> > +
> > +struct rn5t618_wdt {
> > + struct watchdog_device wdt_dev;
> > + struct rn5t618 *rn5t618;
> > +};
[..]
> > +static int rn5t618_wdt_set_timeout(struct watchdog_device *wdt_dev,
> > + unsigned int timeout)
> > +{
> > + struct rn5t618_wdt *wdt = watchdog_get_drvdata(wdt_dev);
> > + int ret, i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(rn5t618_wdt_map); i++) {
> > + if (rn5t618_wdt_map[i].time + 1 >= timeout)
> > + break;
> > + }
> > +
> > + if (i == ARRAY_SIZE(rn5t618_wdt_map))
> > + ret = -EINVAL;
>
> Can you simplify this a bit ? If you use
>
> if (i == ARRAY_SIZE(rn5t618_wdt_map))
> return -EINVAL;
>
> > + else
>
> You can drop this else statement.
Will do, thanks.
Beniamino
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
