| Message ID | 20211120155707.4019487-1-luca@lucaceresoli.net |
|---|---|
| Headers | show |
| Series | Add MAX77714 PMIC minimal driver (RTC and watchdog only) | expand |
On 20/11/2021 16:57, Luca Ceresoli wrote: > Add a simple driver for the Maxim MAX77714 PMIC, supporting RTC and > watchdog only. > > Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net> > > --- > > Changes in v4: none > > Changes in v3: > - Suggested by Lee Jones: > - move struct mfd_cell to top of file > - remove struct max77714 and its kmalloc, not used after probe > - reword error messages > - add "/* pF */" onto the end of the load_cap line > > Changes in v2: > - fix "watchdog" word in heading comment (Guenter Roeck) > - move struct max77714 to .c file (Krzysztof Kozlowski) > - change include guard format (Krzysztof Kozlowski) > - allow building as a module (Krzysztof Kozlowski) > - remove of_match_ptr usage (Krzysztof Kozlowski / lkp) > (Reported-by: kernel test robot <lkp@intel.com>) > --- > MAINTAINERS | 2 + > drivers/mfd/Kconfig | 14 ++++ > drivers/mfd/Makefile | 1 + > drivers/mfd/max77714.c | 152 +++++++++++++++++++++++++++++++++++ > include/linux/mfd/max77714.h | 60 ++++++++++++++ > 5 files changed, 229 insertions(+) > create mode 100644 drivers/mfd/max77714.c > create mode 100644 include/linux/mfd/max77714.h > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> Best regards, Krzysztof
Hi Guenter, On 29/11/21 17:04, Guenter Roeck wrote: > On Sat, Nov 20, 2021 at 04:57:06PM +0100, Luca Ceresoli wrote: >> Clarify why we need to ping the watchdog before changing the timeout by >> quoting the MAX77714 datasheet. >> > > Unless I am missing something, this adds confusion instead of clarifying > anything, and it is misleading. The added comment in the code makes it > sound like clearing the watchdog timer is only needed for MAX77614. > However, the code was in place for MAX77620, suggesting that it was needed > for that chip as well and is not MAX77614 specific. You're right, the comment comes from the max77714-only driver, but now that it is in a multi-chip driver the confusion started to exist. > Please either drop this patch or rephrase it to clarify that it applies > to both chips. What if I rephrase to: /* * "If the value of TWD needs to be changed, clear the system * watchdog timer first [...], then change the value of TWD." - * (MAX77714 datasheet) + * (MAX77714 datasheet but applies to MAX77620 too) */ ?
On 11/29/21 8:08 AM, Luca Ceresoli wrote: > Hi Guenter, > > On 29/11/21 17:04, Guenter Roeck wrote: >> On Sat, Nov 20, 2021 at 04:57:06PM +0100, Luca Ceresoli wrote: >>> Clarify why we need to ping the watchdog before changing the timeout by >>> quoting the MAX77714 datasheet. >>> >> >> Unless I am missing something, this adds confusion instead of clarifying >> anything, and it is misleading. The added comment in the code makes it >> sound like clearing the watchdog timer is only needed for MAX77614. >> However, the code was in place for MAX77620, suggesting that it was needed >> for that chip as well and is not MAX77614 specific. > > You're right, the comment comes from the max77714-only driver, but now > that it is in a multi-chip driver the confusion started to exist. > >> Please either drop this patch or rephrase it to clarify that it applies >> to both chips. > > What if I rephrase to: > > /* > * "If the value of TWD needs to be changed, clear the system > * watchdog timer first [...], then change the value of TWD." > - * (MAX77714 datasheet) > + * (MAX77714 datasheet but applies to MAX77620 too) > */ > Sounds good. Guenter
Hi, this series adds minimal drivers for the Maxim Semiconductor MAX77714 (https://www.maximintegrated.com/en/products/power/power-management-ics/MAX77714.html). Only RTC and watchdog are implemented by these patches. All implemented functionality is tested and working: RTC read/write, watchdog start/stop/ping/set_timeout. Patches 1-3 + 6 are trivial cleanups to the max77686 drivers and Kconfig indentation and can probably be applied easily. Patches 4, 5, 7, 8 and 9 add: dt bindings, mfd driver, watchdog driver and rtc driver. Changes in v4: - do not add a new wdog driver for MAX77714, extend the MAX77620 wdog driver; this means removing v3 patch 7, now replaced by patches 7+8 - added review tags Changes in v3: - fixed all issues reported on v1 patches - removed patch 1 of v2, already applied ("mfd: max77686: Correct tab-based alignment of register addresses") Changes in v2: - fixed all issues reported on v1 patches - added patch 7 ("watchdog: Kconfig: fix help text indentation") - additional minor improvements Luca Luca Ceresoli (9): rtc: max77686: convert comments to kernel-doc format rtc: max77686: rename day-of-month defines rtc: max77686: remove unused code to read in 12-hour mode dt-bindings: mfd: add Maxim MAX77714 PMIC mfd: max77714: Add driver for Maxim MAX77714 PMIC watchdog: Kconfig: fix help text indentation watchdog: max77620: add support for the max77714 variant watchdog: max77620: add comment to clarify set_timeout procedure rtc: max77686: add MAX77714 support .../bindings/mfd/maxim,max77714.yaml | 68 ++++++++ MAINTAINERS | 7 + drivers/mfd/Kconfig | 14 ++ drivers/mfd/Makefile | 1 + drivers/mfd/max77686.c | 2 +- drivers/mfd/max77714.c | 152 ++++++++++++++++++ drivers/rtc/Kconfig | 2 +- drivers/rtc/rtc-max77686.c | 75 +++++---- drivers/watchdog/Kconfig | 50 +++--- drivers/watchdog/max77620_wdt.c | 101 +++++++++--- include/linux/mfd/max77686-private.h | 4 +- include/linux/mfd/max77714.h | 60 +++++++ 12 files changed, 455 insertions(+), 81 deletions(-) create mode 100644 Documentation/devicetree/bindings/mfd/maxim,max77714.yaml create mode 100644 drivers/mfd/max77714.c create mode 100644 include/linux/mfd/max77714.h