mbox series

[v5,0/9] Add MAX77714 PMIC minimal driver (RTC and watchdog only)

Message ID 20211211175951.30763-1-luca@lucaceresoli.net
Headers show
Series Add MAX77714 PMIC minimal driver (RTC and watchdog only) | expand

Message

Luca Ceresoli Dec. 11, 2021, 5:59 p.m. UTC
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 v5:
 - patch 7: fix (and simplify) watchdog_info code
 - patch 8: remove amibguity in comment

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               |  85 ++++++++--
 include/linux/mfd/max77686-private.h          |   4 +-
 include/linux/mfd/max77714.h                  |  60 +++++++
 12 files changed, 445 insertions(+), 75 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

Comments

Lee Jones Dec. 21, 2021, 9:29 a.m. UTC | #1
On Sat, 11 Dec 2021, 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>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> 
> ---
> 
> Changes in v5: none
> 
> 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
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a08f5167dfe0..ef3ffba828af 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11586,6 +11586,8 @@ MAXIM MAX77714 PMIC MFD DRIVER
>  M:	Luca Ceresoli <luca@lucaceresoli.net>
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/mfd/maxim,max77714.yaml
> +F:	drivers/mfd/max77714.c
> +F:	include/linux/mfd/max77714.h
>  
>  MAXIM MAX77802 PMIC REGULATOR DEVICE DRIVER
>  M:	Javier Martinez Canillas <javier@dowhile0.org>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 3fb480818599..1b9d772bdae6 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -855,6 +855,20 @@ config MFD_MAX77693
>  	  additional drivers must be enabled in order to use the functionality
>  	  of the device.
>  
> +config MFD_MAX77714
> +	tristate "Maxim Semiconductor MAX77714 PMIC Support"
> +	depends on I2C
> +	depends on OF || COMPILE_TEST
> +	select MFD_CORE
> +	select REGMAP_I2C
> +	help
> +	  Say yes here to add support for Maxim Semiconductor MAX77714.
> +	  This is a Power Management IC with 4 buck regulators, 9
> +	  low-dropout regulators, 8 GPIOs, RTC, watchdog etc. This driver
> +	  provides common support for accessing the device; additional
> +	  drivers must be enabled in order to use each functionality of the
> +	  device.
> +
>  config MFD_MAX77843
>  	bool "Maxim Semiconductor MAX77843 PMIC Support"
>  	depends on I2C=y
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 0b1b629aef3e..03115cf1336b 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -162,6 +162,7 @@ obj-$(CONFIG_MFD_MAX77620)	+= max77620.o
>  obj-$(CONFIG_MFD_MAX77650)	+= max77650.o
>  obj-$(CONFIG_MFD_MAX77686)	+= max77686.o
>  obj-$(CONFIG_MFD_MAX77693)	+= max77693.o
> +obj-$(CONFIG_MFD_MAX77714)	+= max77714.o
>  obj-$(CONFIG_MFD_MAX77843)	+= max77843.o
>  obj-$(CONFIG_MFD_MAX8907)	+= max8907.o
>  max8925-objs			:= max8925-core.o max8925-i2c.o
> diff --git a/drivers/mfd/max77714.c b/drivers/mfd/max77714.c
> new file mode 100644
> index 000000000000..08dfb69bc6e8
> --- /dev/null
> +++ b/drivers/mfd/max77714.c
> @@ -0,0 +1,152 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Maxim MAX77714 MFD Driver

I'm only mentioning this because you are still missing some reviews.

But I'd prefer for drivers not to describe themselves as MFD Drivers.

The term Parent or Core driver is usually better.

If you have to respin the set, please fix it.

If not, please sent a subsequent fix-up.

Once fixed:

Acked-by: Lee Jones <lee.jones@linaro.org>
Luca Ceresoli Dec. 21, 2021, 9:38 a.m. UTC | #2
Hi Lee,

On 21/12/21 10:29, Lee Jones wrote:
> On Sat, 11 Dec 2021, 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>
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
>>
>> ---
>>
>> Changes in v5: none
>>
>> 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
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index a08f5167dfe0..ef3ffba828af 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -11586,6 +11586,8 @@ MAXIM MAX77714 PMIC MFD DRIVER
>>  M:	Luca Ceresoli <luca@lucaceresoli.net>
>>  S:	Maintained
>>  F:	Documentation/devicetree/bindings/mfd/maxim,max77714.yaml
>> +F:	drivers/mfd/max77714.c
>> +F:	include/linux/mfd/max77714.h
>>  
>>  MAXIM MAX77802 PMIC REGULATOR DEVICE DRIVER
>>  M:	Javier Martinez Canillas <javier@dowhile0.org>
>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>> index 3fb480818599..1b9d772bdae6 100644
>> --- a/drivers/mfd/Kconfig
>> +++ b/drivers/mfd/Kconfig
>> @@ -855,6 +855,20 @@ config MFD_MAX77693
>>  	  additional drivers must be enabled in order to use the functionality
>>  	  of the device.
>>  
>> +config MFD_MAX77714
>> +	tristate "Maxim Semiconductor MAX77714 PMIC Support"
>> +	depends on I2C
>> +	depends on OF || COMPILE_TEST
>> +	select MFD_CORE
>> +	select REGMAP_I2C
>> +	help
>> +	  Say yes here to add support for Maxim Semiconductor MAX77714.
>> +	  This is a Power Management IC with 4 buck regulators, 9
>> +	  low-dropout regulators, 8 GPIOs, RTC, watchdog etc. This driver
>> +	  provides common support for accessing the device; additional
>> +	  drivers must be enabled in order to use each functionality of the
>> +	  device.
>> +
>>  config MFD_MAX77843
>>  	bool "Maxim Semiconductor MAX77843 PMIC Support"
>>  	depends on I2C=y
>> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
>> index 0b1b629aef3e..03115cf1336b 100644
>> --- a/drivers/mfd/Makefile
>> +++ b/drivers/mfd/Makefile
>> @@ -162,6 +162,7 @@ obj-$(CONFIG_MFD_MAX77620)	+= max77620.o
>>  obj-$(CONFIG_MFD_MAX77650)	+= max77650.o
>>  obj-$(CONFIG_MFD_MAX77686)	+= max77686.o
>>  obj-$(CONFIG_MFD_MAX77693)	+= max77693.o
>> +obj-$(CONFIG_MFD_MAX77714)	+= max77714.o
>>  obj-$(CONFIG_MFD_MAX77843)	+= max77843.o
>>  obj-$(CONFIG_MFD_MAX8907)	+= max8907.o
>>  max8925-objs			:= max8925-core.o max8925-i2c.o
>> diff --git a/drivers/mfd/max77714.c b/drivers/mfd/max77714.c
>> new file mode 100644
>> index 000000000000..08dfb69bc6e8
>> --- /dev/null
>> +++ b/drivers/mfd/max77714.c
>> @@ -0,0 +1,152 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Maxim MAX77714 MFD Driver
> 
> I'm only mentioning this because you are still missing some reviews.
> 
> But I'd prefer for drivers not to describe themselves as MFD Drivers.
> 
> The term Parent or Core driver is usually better.
> 
> If you have to respin the set, please fix it.

OK. However No plan for a new iteration, but the wdog patches still need
a review so it's possible. I don't expect changes for the other patches,
they are acked and unchanged since a couple iterations.

> If not, please sent a subsequent fix-up.

Sure. Maybe I'll find some good will and reword all drivers which
currently self-describe as "MFD drivers".

> Acked-by: Lee Jones <lee.jones@linaro.org>

Thanks.
Guenter Roeck Dec. 21, 2021, 3:06 p.m. UTC | #3
On 12/11/21 12:34 PM, 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>
> 


Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Guenter Roeck Dec. 21, 2021, 3:06 p.m. UTC | #4
On 12/11/21 12:34 PM, Luca Ceresoli wrote:
> Clarify why we need to ping the watchdog before changing the timeout by
> quoting the MAX77714 datasheet.
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> 

Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Luca Ceresoli Jan. 11, 2022, 10:10 a.m. UTC | #5
Hi All,

On 11/12/21 18:59, Luca Ceresoli wrote:
> 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.

A gentle ping about this series. It's at v5, all patches have at least
one ack/review tag and most patches are unchanged since ~v2. It applies
cleanly on current master.

Is there anything I should do to help making progress?

Regards,
Luca Ceresoli Jan. 29, 2022, 8:40 a.m. UTC | #6
Hi Lee, all,

On 11/01/22 11:10, Luca Ceresoli wrote:
> Hi All,
> 
> On 11/12/21 18:59, Luca Ceresoli wrote:
>> 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.
> 
> A gentle ping about this series. It's at v5, all patches have at least
> one ack/review tag and most patches are unchanged since ~v2. It applies
> cleanly on current master.
> 
> Is there anything I should do to help making progress?

Apologies for pinging again... but as I got no further comments about
these patches I guess I can really do nothing at the moment.

Lee, is this series completely in charge to you or should it be applied
by the respective subsystem maintainers?

Thanks.
Guenter Roeck Jan. 30, 2022, 1:48 a.m. UTC | #7
On 1/29/22 00:40, Luca Ceresoli wrote:
> Hi Lee, all,
> 
> On 11/01/22 11:10, Luca Ceresoli wrote:
>> Hi All,
>>
>> On 11/12/21 18:59, Luca Ceresoli wrote:
>>> 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.
>>
>> A gentle ping about this series. It's at v5, all patches have at least
>> one ack/review tag and most patches are unchanged since ~v2. It applies
>> cleanly on current master.
>>
>> Is there anything I should do to help making progress?
> 
> Apologies for pinging again... but as I got no further comments about
> these patches I guess I can really do nothing at the moment.
> 
> Lee, is this series completely in charge to you or should it be applied
> by the respective subsystem maintainers?
> 

I hesitated to take the watchdog patches because an earlier patch of the series
introduces MFD_MAX77714 and the watchdog Kconfig entry lists it as dependency.
I now added patch 7/9 and 8/9 to my watchdog-next tree anyway. If the mfd part
doesn't make it we can still decide to take it out at some point.

Note that patch 6/9 has already been applied.

Guenter
Luca Ceresoli Jan. 30, 2022, 12:45 p.m. UTC | #8
Hi Guenter,

On 30/01/22 02:48, Guenter Roeck wrote:
> On 1/29/22 00:40, Luca Ceresoli wrote:
>> Hi Lee, all,
>>
>> On 11/01/22 11:10, Luca Ceresoli wrote:
>>> Hi All,
>>>
>>> On 11/12/21 18:59, Luca Ceresoli wrote:
>>>> 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.
>>>
>>> A gentle ping about this series. It's at v5, all patches have at least
>>> one ack/review tag and most patches are unchanged since ~v2. It applies
>>> cleanly on current master.
>>>
>>> Is there anything I should do to help making progress?
>>
>> Apologies for pinging again... but as I got no further comments about
>> these patches I guess I can really do nothing at the moment.
>>
>> Lee, is this series completely in charge to you or should it be applied
>> by the respective subsystem maintainers?
>>
> 
> I hesitated to take the watchdog patches because an earlier patch of the
> series
> introduces MFD_MAX77714 and the watchdog Kconfig entry lists it as
> dependency.
> I now added patch 7/9 and 8/9 to my watchdog-next tree anyway. If the
> mfd part
> doesn't make it we can still decide to take it out at some point.

OK, thank you! In the meanwhile the kernel test robot also reported a
build failure due to missing max77714.h file, which is added by patch 5.

> Note that patch 6/9 has already been applied.

Indeed, it's in Linus' master already.

Regards.
Guenter Roeck Jan. 30, 2022, 4 p.m. UTC | #9
On 1/30/22 04:45, Luca Ceresoli wrote:
> Hi Guenter,
> 
> On 30/01/22 02:48, Guenter Roeck wrote:
>> On 1/29/22 00:40, Luca Ceresoli wrote:
>>> Hi Lee, all,
>>>
>>> On 11/01/22 11:10, Luca Ceresoli wrote:
>>>> Hi All,
>>>>
>>>> On 11/12/21 18:59, Luca Ceresoli wrote:
>>>>> 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.
>>>>
>>>> A gentle ping about this series. It's at v5, all patches have at least
>>>> one ack/review tag and most patches are unchanged since ~v2. It applies
>>>> cleanly on current master.
>>>>
>>>> Is there anything I should do to help making progress?
>>>
>>> Apologies for pinging again... but as I got no further comments about
>>> these patches I guess I can really do nothing at the moment.
>>>
>>> Lee, is this series completely in charge to you or should it be applied
>>> by the respective subsystem maintainers?
>>>
>>
>> I hesitated to take the watchdog patches because an earlier patch of the
>> series
>> introduces MFD_MAX77714 and the watchdog Kconfig entry lists it as
>> dependency.
>> I now added patch 7/9 and 8/9 to my watchdog-next tree anyway. If the
>> mfd part
>> doesn't make it we can still decide to take it out at some point.
> 
> OK, thank you! In the meanwhile the kernel test robot also reported a
> build failure due to missing max77714.h file, which is added by patch 5.
> 

Yes, I noticed. Which means I'll have to drop the patch again, and we'll have
to wait for the mfd patch to land, sorry.

Guenter

>> Note that patch 6/9 has already been applied.
> 
> Indeed, it's in Linus' master already.
> 
> Regards.