On 13/10/2021 23:49, Luca Ceresoli wrote:

Hi,

On 12/10/21 10:09, Krzysztof Kozlowski wrote:

quoted

On 11/10/2021 17:56, Luca Ceresoli wrote:

quoted

Add a simple driver for the Maxim MAX77714 PMIC, supporting RTC and
watchdog only.

Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
---
 MAINTAINERS                  |   2 +
 drivers/mfd/Kconfig          |  14 ++++
 drivers/mfd/Makefile         |   1 +
 drivers/mfd/max77714.c       | 151 +++++++++++++++++++++++++++++++++++
 include/linux/mfd/max77714.h |  68 ++++++++++++++++
 5 files changed, 236 insertions(+)
 create mode 100644 drivers/mfd/max77714.c
 create mode 100644 include/linux/mfd/max77714.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 4d0134752537..df394192f14e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS

@@ -11389,6 +11389,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 ca0edab91aeb..b5f6e6174508 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig

@@ -853,6 +853,20 @@ config MFD_MAX77693
 	  additional drivers must be enabled in order to use the functionality
 	  of the device.
 
+config MFD_MAX77714
+	bool "Maxim Semiconductor MAX77714 PMIC Support"

Why it cannot be a tristate (module)?

Because it's not done in the driver I initially copied from, I guess. :)

And also because I thought it's appropriate for a PMIC driver since
regulators tend to be always instantiated. But I understand there are
valid use cases for that -- will do in v2 unless a good reason pops up
for not doing it.

Main PMIC as a module sometimes requires additional effort (like initrd
with the PMIC driver) to make system booting. Still for non-SoC
components we choose to allow modules (e.g. max77686).

It seems in your case it can be used as module easily because you did
not implement regulators, which are needed early for storage devices.

quoted

quoted

diff --git a/include/linux/mfd/max77714.h b/include/linux/mfd/max77714.h
new file mode 100644
index 000000000000..ca6b747b73c2
--- /dev/null
+++ b/include/linux/mfd/max77714.h

@@ -0,0 +1,68 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Maxim MAX77714 Register and data structures definition.
+ *
+ * Copyright (C) 2021 Luca Ceresoli
+ * Author: Luca Ceresoli <luca@lucaceresoli.net>
+ */
+
+#ifndef _MFD_MAX77714_H_
+#define _MFD_MAX77714_H_

Header guard:
__LINUX_MFD_MAX77714_H_

OK.

quoted

quoted

+
+struct max77714 {
+	struct device *dev;
+	struct regmap *regmap;
+	struct regmap_irq_chip_data *irq_data;
+
+	int irq;
+};

Do you have to make it a public structure? If not, please put it in the
max77714.c

Good point. Will fix.

Best regards,
Krzysztof