On 08/28/2014 12:19 AM, Lee Jones wrote:
On Wed, 27 Aug 2014, Guenter Roeck wrote:
On Wed, Aug 27, 2014 at 12:13:56AM +0200, Beniamino Galvani wrote:
This adds a driver for the watchdog timer available in Ricoh RN5T618
PMIC. The device supports a programmable expiration time of 1, 8, 32
or 128 seconds.
Signed-off-by: Beniamino Galvani <b.galvani@xxxxxxxxx>
---
drivers/watchdog/Kconfig | 11 +++
drivers/watchdog/Makefile | 1 +
drivers/watchdog/rn5t618_wdt.c | 196 ++++++++++++++++++++++++++++++++++++++++
include/linux/mfd/rn5t618.h | 4 +
4 files changed, 212 insertions(+)
create mode 100644 drivers/watchdog/rn5t618_wdt.c
[...]
+++ b/drivers/watchdog/rn5t618_wdt.c
@@ -0,0 +1,196 @@
[...]
+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;
This changes the semantics.
How so ? If ret is set to -EINVAL, the rest of the function won't do anything
but eventually return -EINVAL. I don't see why returning -EINVAL immediately
would change that.
Guenter
+ else
You can drop this else statement.
+ ret = regmap_update_bits(wdt->rn5t618->regmap, RN5T618_WATCHDOG,
+ RN5T618_WATCHDOG_WDOGTIM_M,
+ rn5t618_wdt_map[i].reg_val);
+ if (!ret)
+ wdt_dev->timeout = rn5t618_wdt_map[i].time;
... Isn't this important?
+ return ret;
+}
--
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
