On 01/22/2015 09:21 AM, Lee Jones wrote:

On Thu, 22 Jan 2015, Guenter Roeck wrote:

quoted

On 01/22/2015 03:56 AM, Lee Jones wrote:

quoted

Signed-off-by: David Paris <redacted>
Signed-off-by: Lee Jones <redacted>
---
  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

[...]

quoted

quoted

+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