| Message ID | 20211120155707.4019487-8-luca@lucaceresoli.net |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add MAX77714 PMIC minimal driver (RTC and watchdog only) | expand |
On Sat, Nov 20, 2021 at 04:57:05PM +0100, Luca Ceresoli wrote: > The MAX77714 is a MFD chip whose watchdog has the same programming > procedures as the MAX77620 watchdog, but most register offsets and bit > masks are different, as well as some features. > > Support the MAX77714 watchdog by adding a variant description table holding > the differences. > > All the features implemented by this driver are available on the MAX77714 > except for the lack of a WDTOFFC bit. Instead of using a "HAS_*" flag we > handle this by holding in the cnfg_glbl2_cfg_bits struct field the bits > (i.e. the features) to enable in the CNFG_GLBL2 register. These bits differ > among the two models. This implementation allows to avoid any conditional > code, keeping the execution flow unchanged. > > Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net> > --- > > This patch is new in v4. It replaces v3 patch 7 ("watchdog: max77714: add > driver for the watchdog in the MAX77714 PMIC") by adding MAX77714 wdog > support to the existing MAX77620 wdog driver instead of adding a new > driver. Suggested by Guenter Roeck and Krzysztof Kozlowski. > --- > drivers/watchdog/Kconfig | 2 +- > drivers/watchdog/max77620_wdt.c | 96 +++++++++++++++++++++++++-------- > 2 files changed, 75 insertions(+), 23 deletions(-) > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > index a6d97f30325a..f920ad271dde 100644 > --- a/drivers/watchdog/Kconfig > +++ b/drivers/watchdog/Kconfig > @@ -677,7 +677,7 @@ config MAX63XX_WATCHDOG > > config MAX77620_WATCHDOG > tristate "Maxim Max77620 Watchdog Timer" > - depends on MFD_MAX77620 || COMPILE_TEST > + depends on MFD_MAX77620 || MFD_MAX77714 || COMPILE_TEST > select WATCHDOG_CORE > help > This is the driver for the Max77620 watchdog timer. > diff --git a/drivers/watchdog/max77620_wdt.c b/drivers/watchdog/max77620_wdt.c > index be6a53c30002..06b48295fab6 100644 > --- a/drivers/watchdog/max77620_wdt.c > +++ b/drivers/watchdog/max77620_wdt.c > @@ -3,8 +3,10 @@ > * Maxim MAX77620 Watchdog Driver > * > * Copyright (C) 2016 NVIDIA CORPORATION. All rights reserved. > + * Copyright (C) 2021 Luca Ceresoli > * > * Author: Laxman Dewangan <ldewangan@nvidia.com> > + * Author: Luca Ceresoli <luca@lucaceresoli.net> > */ > > #include <linux/err.h> > @@ -13,6 +15,7 @@ > #include <linux/module.h> > #include <linux/mod_devicetable.h> > #include <linux/mfd/max77620.h> > +#include <linux/mfd/max77714.h> > #include <linux/platform_device.h> > #include <linux/regmap.h> > #include <linux/slab.h> > @@ -20,17 +23,66 @@ > > static bool nowayout = WATCHDOG_NOWAYOUT; > > +/** > + * struct max77620_variant - Data specific to a chip variant > + * @wdt_info: watchdog descriptor > + * @reg_onoff_cnfg2: ONOFF_CNFG2 register offset > + * @reg_cnfg_glbl2: CNFG_GLBL2 register offset > + * @reg_cnfg_glbl3: CNFG_GLBL3 register offset > + * @wdtc_mask: WDTC bit mask in CNFG_GLBL3 (=bits to update to ping the watchdog) > + * @bit_wd_rst_wk: WD_RST_WK bit offset within ONOFF_CNFG2 > + * @cnfg_glbl2_cfg_bits: configuration bits to enable in CNFG_GLBL2 register > + */ > +struct max77620_variant { > + const struct watchdog_info wdt_info; > + u8 reg_onoff_cnfg2; > + u8 reg_cnfg_glbl2; > + u8 reg_cnfg_glbl3; > + u8 wdtc_mask; > + u8 bit_wd_rst_wk; > + u8 cnfg_glbl2_cfg_bits; > +}; > + > struct max77620_wdt { > struct device *dev; > struct regmap *rmap; > + const struct max77620_variant *drv_data; > struct watchdog_device wdt_dev; > }; > > +static const struct max77620_variant max77620_wdt_data = { > + .wdt_info = { > + .identity = "max77620-watchdog", > + .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE, > + }, That does not have to be, and should not be, part of device specific data, just because of the identity string. Either keep the current identity string, mark max77620_wdt_info as __ro_after_init and overwrite the identity string there during probe, or add the structure to max77620_wdt and fill it out there. Guenter > + .reg_onoff_cnfg2 = MAX77620_REG_ONOFFCNFG2, > + .reg_cnfg_glbl2 = MAX77620_REG_CNFGGLBL2, > + .reg_cnfg_glbl3 = MAX77620_REG_CNFGGLBL3, > + .wdtc_mask = MAX77620_WDTC_MASK, > + .bit_wd_rst_wk = MAX77620_ONOFFCNFG2_WD_RST_WK, > + /* Set WDT clear in OFF and sleep mode */ > + .cnfg_glbl2_cfg_bits = MAX77620_WDTSLPC | MAX77620_WDTOFFC, > +}; > + > +static const struct max77620_variant max77714_wdt_data = { > + .wdt_info = { > + .identity = "max77714-watchdog", > + .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE, > + }, > + .reg_onoff_cnfg2 = MAX77714_CNFG2_ONOFF, > + .reg_cnfg_glbl2 = MAX77714_CNFG_GLBL2, > + .reg_cnfg_glbl3 = MAX77714_CNFG_GLBL3, > + .wdtc_mask = MAX77714_WDTC, > + .bit_wd_rst_wk = MAX77714_WD_RST_WK, > + /* Set WDT clear in sleep mode (there is no WDTOFFC on MAX77714) */ > + .cnfg_glbl2_cfg_bits = MAX77714_WDTSLPC, > +}; > + > static int max77620_wdt_start(struct watchdog_device *wdt_dev) > { > struct max77620_wdt *wdt = watchdog_get_drvdata(wdt_dev); > > - return regmap_update_bits(wdt->rmap, MAX77620_REG_CNFGGLBL2, > + return regmap_update_bits(wdt->rmap, wdt->drv_data->reg_cnfg_glbl2, > MAX77620_WDTEN, MAX77620_WDTEN); > } > > @@ -38,7 +90,7 @@ static int max77620_wdt_stop(struct watchdog_device *wdt_dev) > { > struct max77620_wdt *wdt = watchdog_get_drvdata(wdt_dev); > > - return regmap_update_bits(wdt->rmap, MAX77620_REG_CNFGGLBL2, > + return regmap_update_bits(wdt->rmap, wdt->drv_data->reg_cnfg_glbl2, > MAX77620_WDTEN, 0); > } > > @@ -46,8 +98,8 @@ static int max77620_wdt_ping(struct watchdog_device *wdt_dev) > { > struct max77620_wdt *wdt = watchdog_get_drvdata(wdt_dev); > > - return regmap_update_bits(wdt->rmap, MAX77620_REG_CNFGGLBL3, > - MAX77620_WDTC_MASK, 0x1); > + return regmap_update_bits(wdt->rmap, wdt->drv_data->reg_cnfg_glbl3, > + wdt->drv_data->wdtc_mask, 0x1); > } > > static int max77620_wdt_set_timeout(struct watchdog_device *wdt_dev, > @@ -80,12 +132,12 @@ static int max77620_wdt_set_timeout(struct watchdog_device *wdt_dev, > break; > } > > - ret = regmap_update_bits(wdt->rmap, MAX77620_REG_CNFGGLBL3, > - MAX77620_WDTC_MASK, 0x1); > + ret = regmap_update_bits(wdt->rmap, wdt->drv_data->reg_cnfg_glbl3, > + wdt->drv_data->wdtc_mask, 0x1); > if (ret < 0) > return ret; > > - ret = regmap_update_bits(wdt->rmap, MAX77620_REG_CNFGGLBL2, > + ret = regmap_update_bits(wdt->rmap, wdt->drv_data->reg_cnfg_glbl2, > MAX77620_TWD_MASK, regval); > if (ret < 0) > return ret; > @@ -95,11 +147,6 @@ static int max77620_wdt_set_timeout(struct watchdog_device *wdt_dev, > return 0; > } > > -static const struct watchdog_info max77620_wdt_info = { > - .identity = "max77620-watchdog", > - .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE, > -}; > - > static const struct watchdog_ops max77620_wdt_ops = { > .start = max77620_wdt_start, > .stop = max77620_wdt_stop, > @@ -109,6 +156,7 @@ static const struct watchdog_ops max77620_wdt_ops = { > > static int max77620_wdt_probe(struct platform_device *pdev) > { > + const struct platform_device_id *id = platform_get_device_id(pdev); > struct device *dev = &pdev->dev; > struct max77620_wdt *wdt; > struct watchdog_device *wdt_dev; > @@ -120,6 +168,8 @@ static int max77620_wdt_probe(struct platform_device *pdev) > return -ENOMEM; > > wdt->dev = dev; > + wdt->drv_data = (const struct max77620_variant *) id->driver_data; > + > wdt->rmap = dev_get_regmap(dev->parent, NULL); > if (!wdt->rmap) { > dev_err(wdt->dev, "Failed to get parent regmap\n"); > @@ -127,7 +177,7 @@ static int max77620_wdt_probe(struct platform_device *pdev) > } > > wdt_dev = &wdt->wdt_dev; > - wdt_dev->info = &max77620_wdt_info; > + wdt_dev->info = &wdt->drv_data->wdt_info; > wdt_dev->ops = &max77620_wdt_ops; > wdt_dev->min_timeout = 2; > wdt_dev->max_timeout = 128; > @@ -136,25 +186,25 @@ static int max77620_wdt_probe(struct platform_device *pdev) > platform_set_drvdata(pdev, wdt); > > /* Enable WD_RST_WK - WDT expire results in a restart */ > - ret = regmap_update_bits(wdt->rmap, MAX77620_REG_ONOFFCNFG2, > - MAX77620_ONOFFCNFG2_WD_RST_WK, > - MAX77620_ONOFFCNFG2_WD_RST_WK); > + ret = regmap_update_bits(wdt->rmap, wdt->drv_data->reg_onoff_cnfg2, > + wdt->drv_data->bit_wd_rst_wk, > + wdt->drv_data->bit_wd_rst_wk); > if (ret < 0) { > dev_err(wdt->dev, "Failed to set WD_RST_WK: %d\n", ret); > return ret; > } > > - /* Set WDT clear in OFF and sleep mode */ > - ret = regmap_update_bits(wdt->rmap, MAX77620_REG_CNFGGLBL2, > - MAX77620_WDTOFFC | MAX77620_WDTSLPC, > - MAX77620_WDTOFFC | MAX77620_WDTSLPC); > + /* Set the "auto WDT clear" bits available on the chip */ > + ret = regmap_update_bits(wdt->rmap, wdt->drv_data->reg_cnfg_glbl2, > + wdt->drv_data->cnfg_glbl2_cfg_bits, > + wdt->drv_data->cnfg_glbl2_cfg_bits); > if (ret < 0) { > dev_err(wdt->dev, "Failed to set WDT OFF mode: %d\n", ret); > return ret; > } > > /* Check if WDT running and if yes then set flags properly */ > - ret = regmap_read(wdt->rmap, MAX77620_REG_CNFGGLBL2, ®val); > + ret = regmap_read(wdt->rmap, wdt->drv_data->reg_cnfg_glbl2, ®val); > if (ret < 0) { > dev_err(wdt->dev, "Failed to read WDT CFG register: %d\n", ret); > return ret; > @@ -186,7 +236,8 @@ static int max77620_wdt_probe(struct platform_device *pdev) > } > > static const struct platform_device_id max77620_wdt_devtype[] = { > - { .name = "max77620-watchdog", }, > + { "max77620-watchdog", (kernel_ulong_t)&max77620_wdt_data }, > + { "max77714-watchdog", (kernel_ulong_t)&max77714_wdt_data }, > { }, > }; > MODULE_DEVICE_TABLE(platform, max77620_wdt_devtype); > @@ -208,4 +259,5 @@ MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started " > "(default=" __MODULE_STRING(WATCHDOG_NOWAYOUT) ")"); > > MODULE_AUTHOR("Laxman Dewangan <ldewangan@nvidia.com>"); > +MODULE_AUTHOR("Luca Ceresoli <luca@lucaceresoli.net>"); > MODULE_LICENSE("GPL v2");
Hi Guenter, On 29/11/21 22:56, Guenter Roeck wrote: > On 11/29/21 1:24 PM, Luca Ceresoli wrote: > [ ... ] >>>> +static const struct max77620_variant max77620_wdt_data = { >>>> + .wdt_info = { >>>> + .identity = "max77620-watchdog", >>>> + .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | >>>> WDIOF_MAGICCLOSE, >>>> + }, >>> >>> That does not have to be, and should not be, part of device specific >>> data, >>> just because of the identity string. >> >> Ok, no problem, will fix, but I have two questions. >> >> First, what's the reason? Coding style or a functional difference? >> Usually const data is preferred to runtime assignment. >> > > wdt_info is not chip specific variant information as nothing but the > identity > string is different, and there is no technical need for that difference. > >> Second: it's not clear how you expect it to be done. Looking into the > > I gave you three options to pick from. Perhaps it's because I'm not a native English speaker, but I thought "either" introduces an alternative between two options (wiktionary confirms, FWIW). Reading your sentence in that perspective gave it a different meaning. >> kernel it looks like almost all drivers set a constant string. I could >> find only one exception, f71808e_wdt: >> https://elixir.bootlin.com/linux/v5.16-rc3/source/drivers/watchdog/f71808e_wdt.c#L471 >> >> >>> Either keep the current identity string, >>> mark max77620_wdt_info as __ro_after_init and overwrite the identity >>> string >>> there during probe >> >> And also remove 'static' I guess. Hm, I don't love this, as above I tend >> to prefer static const when possible for file-scoped data. >> > > Where did I suggest to remove 'static', and what would be the benefit of > making > the variable global ? You didn't. But since max77620_wdt_info is currently file-scoped, should it be modified during probe it would generate a weird situation in case one has both a max77714 and a max77620 on the same board (unlikely but possible), as two instances of the same driver would modify the same (static) data. But all of this discussion is getting quite theoretical as... >>> or add the structure to max77620_wdt and fill it out there. >> >> Do you mean like the following, untested, kind-of-pseudocode? >> >> struct max77620_wdt { >> struct device *dev; >> struct regmap *rmap; >> const struct max77620_variant *drv_data; >> + struct watchdog_info info; /* not a pointer! */ >> struct watchdog_device wdt_dev; >> }; >> >> and then, in probe: >> >> wdt->dev = dev; >> wdt->drv_data = (const struct max77620_variant *)id->driver_data; >> /* ... assign other wdt fields ... */ >> + strlcpy(wdt_dev->info.identity, id->name, \ >> + sizeof(wdt_dev->info.identity)); >> + wdt_dev->info.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | \ >> + WDIOF_MAGICCLOSE; >> > For example, yes. > >> Finally, what about simply: >> >> static const struct max77620_variant max77620_wdt_data = { >> .wdt_info = { >> - .identity = "max77620-watchdog", >> + .identity = "max77xxx-watchdog", >> .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | ... >> }, >> >> and always use that struct unconditionally? The max63xx_wdt.c driver >> seems to do that. Or, if this is an issue for backward compatibility (is >> it?), just leave max77620_wdt_data and the .identity field will always >> be "max77620-watchdog" even when using a MAX77714. ...I'll go for this last, simplest option: same struct, same string, always.
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index a6d97f30325a..f920ad271dde 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -677,7 +677,7 @@ config MAX63XX_WATCHDOG config MAX77620_WATCHDOG tristate "Maxim Max77620 Watchdog Timer" - depends on MFD_MAX77620 || COMPILE_TEST + depends on MFD_MAX77620 || MFD_MAX77714 || COMPILE_TEST select WATCHDOG_CORE help This is the driver for the Max77620 watchdog timer. diff --git a/drivers/watchdog/max77620_wdt.c b/drivers/watchdog/max77620_wdt.c index be6a53c30002..06b48295fab6 100644 --- a/drivers/watchdog/max77620_wdt.c +++ b/drivers/watchdog/max77620_wdt.c @@ -3,8 +3,10 @@ * Maxim MAX77620 Watchdog Driver * * Copyright (C) 2016 NVIDIA CORPORATION. All rights reserved. + * Copyright (C) 2021 Luca Ceresoli * * Author: Laxman Dewangan <ldewangan@nvidia.com> + * Author: Luca Ceresoli <luca@lucaceresoli.net> */ #include <linux/err.h> @@ -13,6 +15,7 @@ #include <linux/module.h> #include <linux/mod_devicetable.h> #include <linux/mfd/max77620.h> +#include <linux/mfd/max77714.h> #include <linux/platform_device.h> #include <linux/regmap.h> #include <linux/slab.h> @@ -20,17 +23,66 @@ static bool nowayout = WATCHDOG_NOWAYOUT; +/** + * struct max77620_variant - Data specific to a chip variant + * @wdt_info: watchdog descriptor + * @reg_onoff_cnfg2: ONOFF_CNFG2 register offset + * @reg_cnfg_glbl2: CNFG_GLBL2 register offset + * @reg_cnfg_glbl3: CNFG_GLBL3 register offset + * @wdtc_mask: WDTC bit mask in CNFG_GLBL3 (=bits to update to ping the watchdog) + * @bit_wd_rst_wk: WD_RST_WK bit offset within ONOFF_CNFG2 + * @cnfg_glbl2_cfg_bits: configuration bits to enable in CNFG_GLBL2 register + */ +struct max77620_variant { + const struct watchdog_info wdt_info; + u8 reg_onoff_cnfg2; + u8 reg_cnfg_glbl2; + u8 reg_cnfg_glbl3; + u8 wdtc_mask; + u8 bit_wd_rst_wk; + u8 cnfg_glbl2_cfg_bits; +}; + struct max77620_wdt { struct device *dev; struct regmap *rmap; + const struct max77620_variant *drv_data; struct watchdog_device wdt_dev; }; +static const struct max77620_variant max77620_wdt_data = { + .wdt_info = { + .identity = "max77620-watchdog", + .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE, + }, + .reg_onoff_cnfg2 = MAX77620_REG_ONOFFCNFG2, + .reg_cnfg_glbl2 = MAX77620_REG_CNFGGLBL2, + .reg_cnfg_glbl3 = MAX77620_REG_CNFGGLBL3, + .wdtc_mask = MAX77620_WDTC_MASK, + .bit_wd_rst_wk = MAX77620_ONOFFCNFG2_WD_RST_WK, + /* Set WDT clear in OFF and sleep mode */ + .cnfg_glbl2_cfg_bits = MAX77620_WDTSLPC | MAX77620_WDTOFFC, +}; + +static const struct max77620_variant max77714_wdt_data = { + .wdt_info = { + .identity = "max77714-watchdog", + .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE, + }, + .reg_onoff_cnfg2 = MAX77714_CNFG2_ONOFF, + .reg_cnfg_glbl2 = MAX77714_CNFG_GLBL2, + .reg_cnfg_glbl3 = MAX77714_CNFG_GLBL3, + .wdtc_mask = MAX77714_WDTC, + .bit_wd_rst_wk = MAX77714_WD_RST_WK, + /* Set WDT clear in sleep mode (there is no WDTOFFC on MAX77714) */ + .cnfg_glbl2_cfg_bits = MAX77714_WDTSLPC, +}; + static int max77620_wdt_start(struct watchdog_device *wdt_dev) { struct max77620_wdt *wdt = watchdog_get_drvdata(wdt_dev); - return regmap_update_bits(wdt->rmap, MAX77620_REG_CNFGGLBL2, + return regmap_update_bits(wdt->rmap, wdt->drv_data->reg_cnfg_glbl2, MAX77620_WDTEN, MAX77620_WDTEN); } @@ -38,7 +90,7 @@ static int max77620_wdt_stop(struct watchdog_device *wdt_dev) { struct max77620_wdt *wdt = watchdog_get_drvdata(wdt_dev); - return regmap_update_bits(wdt->rmap, MAX77620_REG_CNFGGLBL2, + return regmap_update_bits(wdt->rmap, wdt->drv_data->reg_cnfg_glbl2, MAX77620_WDTEN, 0); } @@ -46,8 +98,8 @@ static int max77620_wdt_ping(struct watchdog_device *wdt_dev) { struct max77620_wdt *wdt = watchdog_get_drvdata(wdt_dev); - return regmap_update_bits(wdt->rmap, MAX77620_REG_CNFGGLBL3, - MAX77620_WDTC_MASK, 0x1); + return regmap_update_bits(wdt->rmap, wdt->drv_data->reg_cnfg_glbl3, + wdt->drv_data->wdtc_mask, 0x1); } static int max77620_wdt_set_timeout(struct watchdog_device *wdt_dev, @@ -80,12 +132,12 @@ static int max77620_wdt_set_timeout(struct watchdog_device *wdt_dev, break; } - ret = regmap_update_bits(wdt->rmap, MAX77620_REG_CNFGGLBL3, - MAX77620_WDTC_MASK, 0x1); + ret = regmap_update_bits(wdt->rmap, wdt->drv_data->reg_cnfg_glbl3, + wdt->drv_data->wdtc_mask, 0x1); if (ret < 0) return ret; - ret = regmap_update_bits(wdt->rmap, MAX77620_REG_CNFGGLBL2, + ret = regmap_update_bits(wdt->rmap, wdt->drv_data->reg_cnfg_glbl2, MAX77620_TWD_MASK, regval); if (ret < 0) return ret; @@ -95,11 +147,6 @@ static int max77620_wdt_set_timeout(struct watchdog_device *wdt_dev, return 0; } -static const struct watchdog_info max77620_wdt_info = { - .identity = "max77620-watchdog", - .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE, -}; - static const struct watchdog_ops max77620_wdt_ops = { .start = max77620_wdt_start, .stop = max77620_wdt_stop, @@ -109,6 +156,7 @@ static const struct watchdog_ops max77620_wdt_ops = { static int max77620_wdt_probe(struct platform_device *pdev) { + const struct platform_device_id *id = platform_get_device_id(pdev); struct device *dev = &pdev->dev; struct max77620_wdt *wdt; struct watchdog_device *wdt_dev; @@ -120,6 +168,8 @@ static int max77620_wdt_probe(struct platform_device *pdev) return -ENOMEM; wdt->dev = dev; + wdt->drv_data = (const struct max77620_variant *) id->driver_data; + wdt->rmap = dev_get_regmap(dev->parent, NULL); if (!wdt->rmap) { dev_err(wdt->dev, "Failed to get parent regmap\n"); @@ -127,7 +177,7 @@ static int max77620_wdt_probe(struct platform_device *pdev) } wdt_dev = &wdt->wdt_dev; - wdt_dev->info = &max77620_wdt_info; + wdt_dev->info = &wdt->drv_data->wdt_info; wdt_dev->ops = &max77620_wdt_ops; wdt_dev->min_timeout = 2; wdt_dev->max_timeout = 128; @@ -136,25 +186,25 @@ static int max77620_wdt_probe(struct platform_device *pdev) platform_set_drvdata(pdev, wdt); /* Enable WD_RST_WK - WDT expire results in a restart */ - ret = regmap_update_bits(wdt->rmap, MAX77620_REG_ONOFFCNFG2, - MAX77620_ONOFFCNFG2_WD_RST_WK, - MAX77620_ONOFFCNFG2_WD_RST_WK); + ret = regmap_update_bits(wdt->rmap, wdt->drv_data->reg_onoff_cnfg2, + wdt->drv_data->bit_wd_rst_wk, + wdt->drv_data->bit_wd_rst_wk); if (ret < 0) { dev_err(wdt->dev, "Failed to set WD_RST_WK: %d\n", ret); return ret; } - /* Set WDT clear in OFF and sleep mode */ - ret = regmap_update_bits(wdt->rmap, MAX77620_REG_CNFGGLBL2, - MAX77620_WDTOFFC | MAX77620_WDTSLPC, - MAX77620_WDTOFFC | MAX77620_WDTSLPC); + /* Set the "auto WDT clear" bits available on the chip */ + ret = regmap_update_bits(wdt->rmap, wdt->drv_data->reg_cnfg_glbl2, + wdt->drv_data->cnfg_glbl2_cfg_bits, + wdt->drv_data->cnfg_glbl2_cfg_bits); if (ret < 0) { dev_err(wdt->dev, "Failed to set WDT OFF mode: %d\n", ret); return ret; } /* Check if WDT running and if yes then set flags properly */ - ret = regmap_read(wdt->rmap, MAX77620_REG_CNFGGLBL2, ®val); + ret = regmap_read(wdt->rmap, wdt->drv_data->reg_cnfg_glbl2, ®val); if (ret < 0) { dev_err(wdt->dev, "Failed to read WDT CFG register: %d\n", ret); return ret; @@ -186,7 +236,8 @@ static int max77620_wdt_probe(struct platform_device *pdev) } static const struct platform_device_id max77620_wdt_devtype[] = { - { .name = "max77620-watchdog", }, + { "max77620-watchdog", (kernel_ulong_t)&max77620_wdt_data }, + { "max77714-watchdog", (kernel_ulong_t)&max77714_wdt_data }, { }, }; MODULE_DEVICE_TABLE(platform, max77620_wdt_devtype); @@ -208,4 +259,5 @@ MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started " "(default=" __MODULE_STRING(WATCHDOG_NOWAYOUT) ")"); MODULE_AUTHOR("Laxman Dewangan <ldewangan@nvidia.com>"); +MODULE_AUTHOR("Luca Ceresoli <luca@lucaceresoli.net>"); MODULE_LICENSE("GPL v2");
The MAX77714 is a MFD chip whose watchdog has the same programming procedures as the MAX77620 watchdog, but most register offsets and bit masks are different, as well as some features. Support the MAX77714 watchdog by adding a variant description table holding the differences. All the features implemented by this driver are available on the MAX77714 except for the lack of a WDTOFFC bit. Instead of using a "HAS_*" flag we handle this by holding in the cnfg_glbl2_cfg_bits struct field the bits (i.e. the features) to enable in the CNFG_GLBL2 register. These bits differ among the two models. This implementation allows to avoid any conditional code, keeping the execution flow unchanged. Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net> --- This patch is new in v4. It replaces v3 patch 7 ("watchdog: max77714: add driver for the watchdog in the MAX77714 PMIC") by adding MAX77714 wdog support to the existing MAX77620 wdog driver instead of adding a new driver. Suggested by Guenter Roeck and Krzysztof Kozlowski. --- drivers/watchdog/Kconfig | 2 +- drivers/watchdog/max77620_wdt.c | 96 +++++++++++++++++++++++++-------- 2 files changed, 75 insertions(+), 23 deletions(-)