On 01/22/2015 09:21 AM, Lee Jones wrote:
On Thu, 22 Jan 2015, Guenter Roeck wrote:
On 01/22/2015 03:56 AM, Lee Jones wrote:
Signed-off-by: David Paris <david.paris@xxxxxx>
Signed-off-by: Lee Jones <lee.jones@xxxxxxxxxx>
---
drivers/watchdog/Kconfig | 13 ++
drivers/watchdog/Makefile | 1 +
drivers/watchdog/st_lpc_wdt.c | 335 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 349 insertions(+)
create mode 100644 drivers/watchdog/st_lpc_wdt.c
[...]
+struct st_wdog_syscfg {
+ struct regmap *regmap;
Hi David & Lee,
Why did you add the regmap pointer to this structure and not to st_wdog ?
It is not a configuration value, after all, and it is always dereferenced
through wt_wdog->syscfg. So all it does is to make the code more complicated.
Am I missing something ?
I guess it was just to group it all together, as it will all be used
at the same time:
regmap_update_bits(st_wdog->syscfg->regmap,
st_wdog->syscfg->reset_type_reg,
st_wdog->syscfg->reset_type_mask,
st_wdog->warm_reset);
It really doesn't matter to me either way.
Having it in syscfg means it is stored in the static configuration data instead
of the allocated data, which is at least conceptually wrong. So I think it should
be in st_wdog.
Thanks,
Guenter
--
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
