linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] Add MAX77714 PMIC minimal driver (RTC and watchdog only)
@ 2021-10-11 15:56 Luca Ceresoli
  2021-10-11 15:56 ` [PATCH 1/8] mfd: max77686: Correct tab-based alignment of register addresses Luca Ceresoli
                   ` (8 more replies)
  0 siblings, 9 replies; 41+ messages in thread
From: Luca Ceresoli @ 2021-10-11 15:56 UTC (permalink / raw)
  To: linux-kernel
  Cc: Luca Ceresoli, Lee Jones, Rob Herring, Alessandro Zummo,
	Alexandre Belloni, Chanwoo Choi, Krzysztof Kozlowski,
	Bartlomiej Zolnierkiewicz, Wim Van Sebroeck, Guenter Roeck,
	devicetree, linux-rtc, linux-watchdog, Chiwoong Byun,
	Laxman Dewangan

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.

Note! Something seems wrong in the interrupt management code. Due to the
fact that I'm not using interrupts on my hardware and since this is my
first addition of an MFD driver, I was unable to understand what is wrong
after studying the code for other MFD drivers. More details in reply to
patch 8. Advice would be greatly appreciated on this topic.

Except for that, all implemented functionality is tested and working: RTC
read/write, watchdog start/stop/ping/set_timeout.

The first 4 patches are trivial cleanups to the max77686 drivers and can
probably be applied easily.

Patches 5-8 add: dt bindings, mfd driver, watchdog driver and rtc driver.

Luca

Luca Ceresoli (8):
  mfd: max77686: Correct tab-based alignment of register addresses
  rtc: max77686: convert comments to kernel-doc format
  rtc: max77686: rename day-of-month defines
  rtc: max77686: remove useless variable
  dt-bindings: mfd: add Maxim MAX77714 PMIC
  mfd: max77714: Add driver for Maxim MAX77714 PMIC
  watchdog: max77714: add driver for the watchdog in the MAX77714 PMIC
  rtc: max77686: add MAX77714 support

 .../bindings/mfd/maxim,max77714.yaml          |  58 ++++++
 MAINTAINERS                                   |   8 +
 drivers/mfd/Kconfig                           |  14 ++
 drivers/mfd/Makefile                          |   1 +
 drivers/mfd/max77714.c                        | 151 ++++++++++++++++
 drivers/rtc/Kconfig                           |   2 +-
 drivers/rtc/rtc-max77686.c                    |  72 +++++---
 drivers/watchdog/Kconfig                      |   9 +
 drivers/watchdog/Makefile                     |   1 +
 drivers/watchdog/max77714_wdt.c               | 171 ++++++++++++++++++
 include/linux/mfd/max77686-private.h          |  28 +--
 include/linux/mfd/max77714.h                  |  68 +++++++
 12 files changed, 541 insertions(+), 42 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/maxim,max77714.yaml
 create mode 100644 drivers/mfd/max77714.c
 create mode 100644 drivers/watchdog/max77714_wdt.c
 create mode 100644 include/linux/mfd/max77714.h

-- 
2.25.1


^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH 1/8] mfd: max77686: Correct tab-based alignment of register addresses
  2021-10-11 15:56 [PATCH 0/8] Add MAX77714 PMIC minimal driver (RTC and watchdog only) Luca Ceresoli
@ 2021-10-11 15:56 ` Luca Ceresoli
  2021-10-12  8:00   ` Krzysztof Kozlowski
  2021-10-11 15:56 ` [PATCH 2/8] rtc: max77686: convert comments to kernel-doc format Luca Ceresoli
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 41+ messages in thread
From: Luca Ceresoli @ 2021-10-11 15:56 UTC (permalink / raw)
  To: linux-kernel
  Cc: Luca Ceresoli, Lee Jones, Rob Herring, Alessandro Zummo,
	Alexandre Belloni, Chanwoo Choi, Krzysztof Kozlowski,
	Bartlomiej Zolnierkiewicz, Wim Van Sebroeck, Guenter Roeck,
	devicetree, linux-rtc, linux-watchdog, Chiwoong Byun,
	Laxman Dewangan

Some lines have an extra tab, remove them for proper visual alignment as
present on the rest of this file.

Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
---
 include/linux/mfd/max77686-private.h | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/include/linux/mfd/max77686-private.h b/include/linux/mfd/max77686-private.h
index 833e578e051e..b1482b3cf353 100644
--- a/include/linux/mfd/max77686-private.h
+++ b/include/linux/mfd/max77686-private.h
@@ -133,35 +133,35 @@ enum max77686_pmic_reg {
 	/* Reserved: 0x7A-0x7D */
 
 	MAX77686_REG_BBAT_CHG		= 0x7E,
-	MAX77686_REG_32KHZ			= 0x7F,
+	MAX77686_REG_32KHZ		= 0x7F,
 
 	MAX77686_REG_PMIC_END		= 0x80,
 };
 
 enum max77686_rtc_reg {
-	MAX77686_RTC_INT			= 0x00,
-	MAX77686_RTC_INTM			= 0x01,
+	MAX77686_RTC_INT		= 0x00,
+	MAX77686_RTC_INTM		= 0x01,
 	MAX77686_RTC_CONTROLM		= 0x02,
 	MAX77686_RTC_CONTROL		= 0x03,
 	MAX77686_RTC_UPDATE0		= 0x04,
 	/* Reserved: 0x5 */
 	MAX77686_WTSR_SMPL_CNTL		= 0x06,
-	MAX77686_RTC_SEC			= 0x07,
-	MAX77686_RTC_MIN			= 0x08,
-	MAX77686_RTC_HOUR			= 0x09,
+	MAX77686_RTC_SEC		= 0x07,
+	MAX77686_RTC_MIN		= 0x08,
+	MAX77686_RTC_HOUR		= 0x09,
 	MAX77686_RTC_WEEKDAY		= 0x0A,
-	MAX77686_RTC_MONTH			= 0x0B,
-	MAX77686_RTC_YEAR			= 0x0C,
-	MAX77686_RTC_DATE			= 0x0D,
-	MAX77686_ALARM1_SEC			= 0x0E,
-	MAX77686_ALARM1_MIN			= 0x0F,
+	MAX77686_RTC_MONTH		= 0x0B,
+	MAX77686_RTC_YEAR		= 0x0C,
+	MAX77686_RTC_DATE		= 0x0D,
+	MAX77686_ALARM1_SEC		= 0x0E,
+	MAX77686_ALARM1_MIN		= 0x0F,
 	MAX77686_ALARM1_HOUR		= 0x10,
 	MAX77686_ALARM1_WEEKDAY		= 0x11,
 	MAX77686_ALARM1_MONTH		= 0x12,
 	MAX77686_ALARM1_YEAR		= 0x13,
 	MAX77686_ALARM1_DATE		= 0x14,
-	MAX77686_ALARM2_SEC			= 0x15,
-	MAX77686_ALARM2_MIN			= 0x16,
+	MAX77686_ALARM2_SEC		= 0x15,
+	MAX77686_ALARM2_MIN		= 0x16,
 	MAX77686_ALARM2_HOUR		= 0x17,
 	MAX77686_ALARM2_WEEKDAY		= 0x18,
 	MAX77686_ALARM2_MONTH		= 0x19,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 2/8] rtc: max77686: convert comments to kernel-doc format
  2021-10-11 15:56 [PATCH 0/8] Add MAX77714 PMIC minimal driver (RTC and watchdog only) Luca Ceresoli
  2021-10-11 15:56 ` [PATCH 1/8] mfd: max77686: Correct tab-based alignment of register addresses Luca Ceresoli
@ 2021-10-11 15:56 ` Luca Ceresoli
  2021-10-12  8:00   ` Krzysztof Kozlowski
  2021-10-15 17:29   ` Alexandre Belloni
  2021-10-11 15:56 ` [PATCH 3/8] rtc: max77686: rename day-of-month defines Luca Ceresoli
                   ` (6 subsequent siblings)
  8 siblings, 2 replies; 41+ messages in thread
From: Luca Ceresoli @ 2021-10-11 15:56 UTC (permalink / raw)
  To: linux-kernel
  Cc: Luca Ceresoli, Lee Jones, Rob Herring, Alessandro Zummo,
	Alexandre Belloni, Chanwoo Choi, Krzysztof Kozlowski,
	Bartlomiej Zolnierkiewicz, Wim Van Sebroeck, Guenter Roeck,
	devicetree, linux-rtc, linux-watchdog, Chiwoong Byun,
	Laxman Dewangan

Convert the comments documenting this struct to kernel-doc format for
standardization and readability.

Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
---
 drivers/rtc/rtc-max77686.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
index eae7cb9faf1e..bac52cdea97d 100644
--- a/drivers/rtc/rtc-max77686.c
+++ b/drivers/rtc/rtc-max77686.c
@@ -61,24 +61,27 @@ enum {
 	RTC_NR_TIME
 };
 
+/**
+ * struct max77686_rtc_driver_data - model-specific configuration
+ * @delay: Minimum usecs needed for a RTC update
+ * @mask: Mask used to read RTC registers value
+ * @map: Registers offset to I2C addresses map
+ * @alarm_enable_reg: Has a separate alarm enable register?
+ * @rtc_i2c_addr: I2C address for RTC block
+ * @rtc_irq_from_platform: RTC interrupt via platform resource
+ * @alarm_pending_status_reg: Pending alarm status register
+ * @rtc_irq_chip: RTC IRQ CHIP for regmap
+ * @regmap_config: regmap configuration for the chip
+ */
 struct max77686_rtc_driver_data {
-	/* Minimum usecs needed for a RTC update */
 	unsigned long		delay;
-	/* Mask used to read RTC registers value */
 	u8			mask;
-	/* Registers offset to I2C addresses map */
 	const unsigned int	*map;
-	/* Has a separate alarm enable register? */
 	bool			alarm_enable_reg;
-	/* I2C address for RTC block */
 	int			rtc_i2c_addr;
-	/* RTC interrupt via platform resource */
 	bool			rtc_irq_from_platform;
-	/* Pending alarm status register */
 	int			alarm_pending_status_reg;
-	/* RTC IRQ CHIP for regmap */
 	const struct regmap_irq_chip *rtc_irq_chip;
-	/* regmap configuration for the chip */
 	const struct regmap_config *regmap_config;
 };
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 3/8] rtc: max77686: rename day-of-month defines
  2021-10-11 15:56 [PATCH 0/8] Add MAX77714 PMIC minimal driver (RTC and watchdog only) Luca Ceresoli
  2021-10-11 15:56 ` [PATCH 1/8] mfd: max77686: Correct tab-based alignment of register addresses Luca Ceresoli
  2021-10-11 15:56 ` [PATCH 2/8] rtc: max77686: convert comments to kernel-doc format Luca Ceresoli
@ 2021-10-11 15:56 ` Luca Ceresoli
  2021-10-12  2:23   ` kernel test robot
                     ` (2 more replies)
  2021-10-11 15:56 ` [PATCH 4/8] rtc: max77686: remove useless variable Luca Ceresoli
                   ` (5 subsequent siblings)
  8 siblings, 3 replies; 41+ messages in thread
From: Luca Ceresoli @ 2021-10-11 15:56 UTC (permalink / raw)
  To: linux-kernel
  Cc: Luca Ceresoli, Lee Jones, Rob Herring, Alessandro Zummo,
	Alexandre Belloni, Chanwoo Choi, Krzysztof Kozlowski,
	Bartlomiej Zolnierkiewicz, Wim Van Sebroeck, Guenter Roeck,
	devicetree, linux-rtc, linux-watchdog, Chiwoong Byun,
	Laxman Dewangan

RTC_DATE and REG_RTC_DATE are used for the registers holding the day of
month. Rename these constants to mean what they mean.

Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
---
 drivers/rtc/rtc-max77686.c           | 16 ++++++++--------
 include/linux/mfd/max77686-private.h |  4 ++--
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
index bac52cdea97d..7e765207f28e 100644
--- a/drivers/rtc/rtc-max77686.c
+++ b/drivers/rtc/rtc-max77686.c
@@ -57,7 +57,7 @@ enum {
 	RTC_WEEKDAY,
 	RTC_MONTH,
 	RTC_YEAR,
-	RTC_DATE,
+	RTC_MONTHDAY,
 	RTC_NR_TIME
 };
 
@@ -119,7 +119,7 @@ enum max77686_rtc_reg_offset {
 	REG_RTC_WEEKDAY,
 	REG_RTC_MONTH,
 	REG_RTC_YEAR,
-	REG_RTC_DATE,
+	REG_RTC_MONTHDAY,
 	REG_ALARM1_SEC,
 	REG_ALARM1_MIN,
 	REG_ALARM1_HOUR,
@@ -150,7 +150,7 @@ static const unsigned int max77686_map[REG_RTC_END] = {
 	[REG_RTC_WEEKDAY]    = MAX77686_RTC_WEEKDAY,
 	[REG_RTC_MONTH]      = MAX77686_RTC_MONTH,
 	[REG_RTC_YEAR]       = MAX77686_RTC_YEAR,
-	[REG_RTC_DATE]       = MAX77686_RTC_DATE,
+	[REG_RTC_MONTHDAY]   = MAX77686_RTC_MONTHDAY,
 	[REG_ALARM1_SEC]     = MAX77686_ALARM1_SEC,
 	[REG_ALARM1_MIN]     = MAX77686_ALARM1_MIN,
 	[REG_ALARM1_HOUR]    = MAX77686_ALARM1_HOUR,
@@ -233,7 +233,7 @@ static const unsigned int max77802_map[REG_RTC_END] = {
 	[REG_RTC_WEEKDAY]    = MAX77802_RTC_WEEKDAY,
 	[REG_RTC_MONTH]      = MAX77802_RTC_MONTH,
 	[REG_RTC_YEAR]       = MAX77802_RTC_YEAR,
-	[REG_RTC_DATE]       = MAX77802_RTC_DATE,
+	[REG_RTC_MONTHDAY]   = MAX77802_RTC_MONTHDAY,
 	[REG_ALARM1_SEC]     = MAX77802_ALARM1_SEC,
 	[REG_ALARM1_MIN]     = MAX77802_ALARM1_MIN,
 	[REG_ALARM1_HOUR]    = MAX77802_ALARM1_HOUR,
@@ -288,7 +288,7 @@ static void max77686_rtc_data_to_tm(u8 *data, struct rtc_time *tm,
 
 	/* Only a single bit is set in data[], so fls() would be equivalent */
 	tm->tm_wday = ffs(data[RTC_WEEKDAY] & mask) - 1;
-	tm->tm_mday = data[RTC_DATE] & 0x1f;
+	tm->tm_mday = data[RTC_MONTHDAY] & 0x1f;
 	tm->tm_mon = (data[RTC_MONTH] & 0x0f) - 1;
 	tm->tm_year = data[RTC_YEAR] & mask;
 	tm->tm_yday = 0;
@@ -309,7 +309,7 @@ static int max77686_rtc_tm_to_data(struct rtc_time *tm, u8 *data,
 	data[RTC_MIN] = tm->tm_min;
 	data[RTC_HOUR] = tm->tm_hour;
 	data[RTC_WEEKDAY] = 1 << tm->tm_wday;
-	data[RTC_DATE] = tm->tm_mday;
+	data[RTC_MONTHDAY] = tm->tm_mday;
 	data[RTC_MONTH] = tm->tm_mon + 1;
 
 	if (info->drv_data->alarm_enable_reg) {
@@ -565,8 +565,8 @@ static int max77686_rtc_start_alarm(struct max77686_rtc_info *info)
 			data[RTC_MONTH] |= (1 << ALARM_ENABLE_SHIFT);
 		if (data[RTC_YEAR] & info->drv_data->mask)
 			data[RTC_YEAR] |= (1 << ALARM_ENABLE_SHIFT);
-		if (data[RTC_DATE] & 0x1f)
-			data[RTC_DATE] |= (1 << ALARM_ENABLE_SHIFT);
+		if (data[RTC_MONTHDAY] & 0x1f)
+			data[RTC_MONTHDAY] |= (1 << ALARM_ENABLE_SHIFT);
 
 		ret = regmap_bulk_write(info->rtc_regmap, map[REG_ALARM1_SEC],
 					data, ARRAY_SIZE(data));
diff --git a/include/linux/mfd/max77686-private.h b/include/linux/mfd/max77686-private.h
index b1482b3cf353..3acceeedbaba 100644
--- a/include/linux/mfd/max77686-private.h
+++ b/include/linux/mfd/max77686-private.h
@@ -152,7 +152,7 @@ enum max77686_rtc_reg {
 	MAX77686_RTC_WEEKDAY		= 0x0A,
 	MAX77686_RTC_MONTH		= 0x0B,
 	MAX77686_RTC_YEAR		= 0x0C,
-	MAX77686_RTC_DATE		= 0x0D,
+	MAX77686_RTC_MONTHDAY		= 0x0D,
 	MAX77686_ALARM1_SEC		= 0x0E,
 	MAX77686_ALARM1_MIN		= 0x0F,
 	MAX77686_ALARM1_HOUR		= 0x10,
@@ -352,7 +352,7 @@ enum max77802_rtc_reg {
 	MAX77802_RTC_WEEKDAY		= 0xCA,
 	MAX77802_RTC_MONTH		= 0xCB,
 	MAX77802_RTC_YEAR		= 0xCC,
-	MAX77802_RTC_DATE		= 0xCD,
+	MAX77802_RTC_MONTHDAY		= 0xCD,
 	MAX77802_RTC_AE1		= 0xCE,
 	MAX77802_ALARM1_SEC		= 0xCF,
 	MAX77802_ALARM1_MIN		= 0xD0,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 4/8] rtc: max77686: remove useless variable
  2021-10-11 15:56 [PATCH 0/8] Add MAX77714 PMIC minimal driver (RTC and watchdog only) Luca Ceresoli
                   ` (2 preceding siblings ...)
  2021-10-11 15:56 ` [PATCH 3/8] rtc: max77686: rename day-of-month defines Luca Ceresoli
@ 2021-10-11 15:56 ` Luca Ceresoli
  2021-10-12  8:01   ` Krzysztof Kozlowski
  2021-10-15 17:33   ` Alexandre Belloni
  2021-10-11 15:56 ` [PATCH 5/8] dt-bindings: mfd: add Maxim MAX77714 PMIC Luca Ceresoli
                   ` (4 subsequent siblings)
  8 siblings, 2 replies; 41+ messages in thread
From: Luca Ceresoli @ 2021-10-11 15:56 UTC (permalink / raw)
  To: linux-kernel
  Cc: Luca Ceresoli, Lee Jones, Rob Herring, Alessandro Zummo,
	Alexandre Belloni, Chanwoo Choi, Krzysztof Kozlowski,
	Bartlomiej Zolnierkiewicz, Wim Van Sebroeck, Guenter Roeck,
	devicetree, linux-rtc, linux-watchdog, Chiwoong Byun,
	Laxman Dewangan

rtc_24hr_mode is set to 1 in max77686_rtc_probe()->max77686_rtc_init_reg()
before being read and is never set back to 0 again. As such, it is de facto
a constant.

Remove the variable and the unreachable code.

Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
---
 drivers/rtc/rtc-max77686.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
index 7e765207f28e..9901c596998a 100644
--- a/drivers/rtc/rtc-max77686.c
+++ b/drivers/rtc/rtc-max77686.c
@@ -99,7 +99,6 @@ struct max77686_rtc_info {
 
 	int rtc_irq;
 	int virq;
-	int rtc_24hr_mode;
 };
 
 enum MAX77686_RTC_OP {
@@ -278,13 +277,7 @@ static void max77686_rtc_data_to_tm(u8 *data, struct rtc_time *tm,
 
 	tm->tm_sec = data[RTC_SEC] & mask;
 	tm->tm_min = data[RTC_MIN] & mask;
-	if (info->rtc_24hr_mode) {
-		tm->tm_hour = data[RTC_HOUR] & 0x1f;
-	} else {
-		tm->tm_hour = data[RTC_HOUR] & 0x0f;
-		if (data[RTC_HOUR] & HOUR_PM_MASK)
-			tm->tm_hour += 12;
-	}
+	tm->tm_hour = data[RTC_HOUR] & 0x1f;
 
 	/* Only a single bit is set in data[], so fls() would be equivalent */
 	tm->tm_wday = ffs(data[RTC_WEEKDAY] & mask) - 1;
@@ -662,8 +655,6 @@ static int max77686_rtc_init_reg(struct max77686_rtc_info *info)
 	data[0] = (1 << BCD_EN_SHIFT) | (1 << MODEL24_SHIFT);
 	data[1] = (0 << BCD_EN_SHIFT) | (1 << MODEL24_SHIFT);
 
-	info->rtc_24hr_mode = 1;
-
 	ret = regmap_bulk_write(info->rtc_regmap,
 				info->drv_data->map[REG_RTC_CONTROLM],
 				data, ARRAY_SIZE(data));
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 5/8] dt-bindings: mfd: add Maxim MAX77714 PMIC
  2021-10-11 15:56 [PATCH 0/8] Add MAX77714 PMIC minimal driver (RTC and watchdog only) Luca Ceresoli
                   ` (3 preceding siblings ...)
  2021-10-11 15:56 ` [PATCH 4/8] rtc: max77686: remove useless variable Luca Ceresoli
@ 2021-10-11 15:56 ` Luca Ceresoli
  2021-10-12  8:02   ` Krzysztof Kozlowski
  2021-10-11 15:56 ` [PATCH 6/8] mfd: max77714: Add driver for " Luca Ceresoli
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 41+ messages in thread
From: Luca Ceresoli @ 2021-10-11 15:56 UTC (permalink / raw)
  To: linux-kernel
  Cc: Luca Ceresoli, Lee Jones, Rob Herring, Alessandro Zummo,
	Alexandre Belloni, Chanwoo Choi, Krzysztof Kozlowski,
	Bartlomiej Zolnierkiewicz, Wim Van Sebroeck, Guenter Roeck,
	devicetree, linux-rtc, linux-watchdog, Chiwoong Byun,
	Laxman Dewangan

Add bindings for the MAX77714 PMIC with GPIO, RTC and watchdog.

Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
---
 .../bindings/mfd/maxim,max77714.yaml          | 58 +++++++++++++++++++
 MAINTAINERS                                   |  5 ++
 2 files changed, 63 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/maxim,max77714.yaml

diff --git a/Documentation/devicetree/bindings/mfd/maxim,max77714.yaml b/Documentation/devicetree/bindings/mfd/maxim,max77714.yaml
new file mode 100644
index 000000000000..2b0ce3b9bc92
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/maxim,max77714.yaml
@@ -0,0 +1,58 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/maxim,max77714.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MAX77714 PMIC with GPIO, RTC and watchdog from Maxim Integrated.
+
+maintainers:
+  - Luca Ceresoli <luca@lucaceresoli.net>
+
+description: |
+  MAX77714 is a Power Management IC with 4 buck regulators, 9
+  low-dropout regulators, 8 GPIOs, RTC and watchdog.
+
+properties:
+  compatible:
+    const: maxim,max77714
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 2
+    description:
+      The first cell is the IRQ number, the second cell is the trigger type.
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-controller
+  - "#interrupt-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        pmic@1c {
+            compatible = "maxim,max77714";
+            reg = <0x1c>;
+            interrupt-parent = <&gpio2>;
+            interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
+
+            interrupt-controller;
+            #interrupt-cells = <2>;
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index a4a0c2baaf27..4d0134752537 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11385,6 +11385,11 @@ F:	drivers/power/supply/max77650-charger.c
 F:	drivers/regulator/max77650-regulator.c
 F:	include/linux/mfd/max77650.h
 
+MAXIM MAX77714 PMIC MFD DRIVER
+M:	Luca Ceresoli <luca@lucaceresoli.net>
+S:	Maintained
+F:	Documentation/devicetree/bindings/mfd/maxim,max77714.yaml
+
 MAXIM MAX77802 PMIC REGULATOR DEVICE DRIVER
 M:	Javier Martinez Canillas <javier@dowhile0.org>
 L:	linux-kernel@vger.kernel.org
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 6/8] mfd: max77714: Add driver for Maxim MAX77714 PMIC
  2021-10-11 15:56 [PATCH 0/8] Add MAX77714 PMIC minimal driver (RTC and watchdog only) Luca Ceresoli
                   ` (4 preceding siblings ...)
  2021-10-11 15:56 ` [PATCH 5/8] dt-bindings: mfd: add Maxim MAX77714 PMIC Luca Ceresoli
@ 2021-10-11 15:56 ` Luca Ceresoli
  2021-10-11 17:15   ` Guenter Roeck
                     ` (3 more replies)
  2021-10-11 15:56 ` [PATCH 7/8] watchdog: max77714: add driver for the watchdog in the " Luca Ceresoli
                   ` (2 subsequent siblings)
  8 siblings, 4 replies; 41+ messages in thread
From: Luca Ceresoli @ 2021-10-11 15:56 UTC (permalink / raw)
  To: linux-kernel
  Cc: Luca Ceresoli, Lee Jones, Rob Herring, Alessandro Zummo,
	Alexandre Belloni, Chanwoo Choi, Krzysztof Kozlowski,
	Bartlomiej Zolnierkiewicz, Wim Van Sebroeck, Guenter Roeck,
	devicetree, linux-rtc, linux-watchdog, Chiwoong Byun,
	Laxman Dewangan

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"
+	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 2ba6646e874c..fe43f2fdd5cb 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -163,6 +163,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..5d6c88d4d6c0
--- /dev/null
+++ b/drivers/mfd/max77714.c
@@ -0,0 +1,151 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Maxim MAX77714 Watchdog Driver
+ *
+ * Copyright (C) 2021 Luca Ceresoli
+ * Author: Luca Ceresoli <luca@lucaceresoli.net>
+ */
+
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/max77714.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+
+static const struct regmap_range max77714_readable_ranges[] = {
+	regmap_reg_range(MAX77714_INT_TOP,     MAX77714_INT_TOP),
+	regmap_reg_range(MAX77714_INT_TOPM,    MAX77714_INT_TOPM),
+	regmap_reg_range(MAX77714_32K_STATUS,  MAX77714_32K_CONFIG),
+	regmap_reg_range(MAX77714_CNFG_GLBL2,  MAX77714_CNFG2_ONOFF),
+};
+
+static const struct regmap_range max77714_writable_ranges[] = {
+	regmap_reg_range(MAX77714_INT_TOPM,    MAX77714_INT_TOPM),
+	regmap_reg_range(MAX77714_32K_CONFIG,  MAX77714_32K_CONFIG),
+	regmap_reg_range(MAX77714_CNFG_GLBL2,  MAX77714_CNFG2_ONOFF),
+};
+
+static const struct regmap_access_table max77714_readable_table = {
+	.yes_ranges = max77714_readable_ranges,
+	.n_yes_ranges = ARRAY_SIZE(max77714_readable_ranges),
+};
+
+static const struct regmap_access_table max77714_writable_table = {
+	.yes_ranges = max77714_writable_ranges,
+	.n_yes_ranges = ARRAY_SIZE(max77714_writable_ranges),
+};
+
+static const struct regmap_config max77714_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = MAX77714_CNFG2_ONOFF,
+	.rd_table = &max77714_readable_table,
+	.wr_table = &max77714_writable_table,
+};
+
+static const struct regmap_irq max77714_top_irqs[] = {
+	REGMAP_IRQ_REG(MAX77714_IRQ_TOP_ONOFF,   0, MAX77714_INT_TOP_ONOFF),
+	REGMAP_IRQ_REG(MAX77714_IRQ_TOP_RTC,     0, MAX77714_INT_TOP_RTC),
+	REGMAP_IRQ_REG(MAX77714_IRQ_TOP_GPIO,    0, MAX77714_INT_TOP_GPIO),
+	REGMAP_IRQ_REG(MAX77714_IRQ_TOP_LDO,     0, MAX77714_INT_TOP_LDO),
+	REGMAP_IRQ_REG(MAX77714_IRQ_TOP_SD,      0, MAX77714_INT_TOP_SD),
+	REGMAP_IRQ_REG(MAX77714_IRQ_TOP_GLBL,    0, MAX77714_INT_TOP_GLBL),
+};
+
+static const struct regmap_irq_chip max77714_irq_chip = {
+	.name			= "max77714-pmic",
+	.status_base		= MAX77714_INT_TOP,
+	.mask_base		= MAX77714_INT_TOPM,
+	.num_regs		= 1,
+	.irqs			= max77714_top_irqs,
+	.num_irqs		= ARRAY_SIZE(max77714_top_irqs),
+};
+
+static const struct mfd_cell max77714_cells[] = {
+	{ .name = "max77714-watchdog" },
+	{ .name = "max77714-rtc" },
+};
+
+/*
+ * MAX77714 initially uses the internal, low precision oscillator. Enable
+ * the external oscillator by setting the XOSC_RETRY bit. If the external
+ * oscillator is not OK (probably not installed) this has no effect.
+ */
+static int max77714_setup_xosc(struct max77714 *chip)
+{
+	/* Internal Crystal Load Capacitance, indexed by value of 32KLOAD bits */
+	static const unsigned int load_cap[4] = {0, 10, 12, 22};
+	unsigned int load_cap_idx;
+	unsigned int status;
+	int err;
+
+	err = regmap_update_bits(chip->regmap, MAX77714_32K_CONFIG,
+				 MAX77714_32K_CONFIG_XOSC_RETRY,
+				 MAX77714_32K_CONFIG_XOSC_RETRY);
+	if (err)
+		return dev_err_probe(chip->dev, err, "cannot configure XOSC\n");
+
+	err = regmap_read(chip->regmap, MAX77714_32K_STATUS, &status);
+	if (err)
+		return dev_err_probe(chip->dev, err, "cannot read XOSC status\n");
+
+	load_cap_idx = (status >> MAX77714_32K_STATUS_32KLOAD_SHF)
+		& MAX77714_32K_STATUS_32KLOAD_MSK;
+
+	dev_info(chip->dev, "Using %s oscillator, %d pF load cap\n",
+		 status & MAX77714_32K_STATUS_32KSOURCE ? "internal" : "external",
+		 load_cap[load_cap_idx]);
+
+	return 0;
+}
+
+static int max77714_probe(struct i2c_client *client)
+{
+	struct max77714 *chip;
+	int err;
+
+	chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, chip);
+	chip->dev = &client->dev;
+
+	chip->regmap = devm_regmap_init_i2c(client, &max77714_regmap_config);
+	if (IS_ERR(chip->regmap))
+		return dev_err_probe(chip->dev, PTR_ERR(chip->regmap),
+				     "failed to initialise regmap\n");
+
+	err = max77714_setup_xosc(chip);
+	if (err)
+		return err;
+
+	err = devm_regmap_add_irq_chip(chip->dev, chip->regmap, client->irq,
+				       IRQF_ONESHOT | IRQF_SHARED, 0,
+				       &max77714_irq_chip, &chip->irq_data);
+	if (err)
+		return dev_err_probe(chip->dev, err, "failed to add PMIC irq chip\n");
+
+	err =  devm_mfd_add_devices(chip->dev, PLATFORM_DEVID_NONE,
+				    max77714_cells, ARRAY_SIZE(max77714_cells),
+				    NULL, 0, NULL);
+	if (err)
+		return dev_err_probe(chip->dev, err, "failed adding MFD children\n");
+
+	return 0;
+}
+
+static const struct of_device_id max77714_dt_match[] = {
+	{ .compatible = "maxim,max77714" },
+	{},
+};
+
+static struct i2c_driver max77714_driver = {
+	.driver = {
+		.name = "max77714",
+		.of_match_table = of_match_ptr(max77714_dt_match),
+	},
+	.probe_new = max77714_probe,
+};
+builtin_i2c_driver(max77714_driver);
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_
+
+#include <linux/bits.h>
+
+#define MAX77714_INT_TOP	0x00
+#define MAX77714_INT_TOPM	0x07 /* Datasheet says "read only", but it is RW */
+
+#define MAX77714_INT_TOP_ONOFF		BIT(1)
+#define MAX77714_INT_TOP_RTC		BIT(3)
+#define MAX77714_INT_TOP_GPIO		BIT(4)
+#define MAX77714_INT_TOP_LDO		BIT(5)
+#define MAX77714_INT_TOP_SD		BIT(6)
+#define MAX77714_INT_TOP_GLBL		BIT(7)
+
+#define MAX77714_32K_STATUS	0x30
+#define MAX77714_32K_STATUS_SIOSCOK	BIT(5)
+#define MAX77714_32K_STATUS_XOSCOK	BIT(4)
+#define MAX77714_32K_STATUS_32KSOURCE	BIT(3)
+#define MAX77714_32K_STATUS_32KLOAD_MSK	0x3
+#define MAX77714_32K_STATUS_32KLOAD_SHF	1
+#define MAX77714_32K_STATUS_CRYSTAL_CFG	BIT(0)
+
+#define MAX77714_32K_CONFIG	0x31
+#define MAX77714_32K_CONFIG_XOSC_RETRY	BIT(4)
+
+#define MAX77714_CNFG_GLBL2	0x91
+#define MAX77714_WDTEN			BIT(2)
+#define MAX77714_WDTSLPC		BIT(3)
+#define MAX77714_TWD_MASK		0x3
+#define MAX77714_TWD_2s			0x0
+#define MAX77714_TWD_16s		0x1
+#define MAX77714_TWD_64s		0x2
+#define MAX77714_TWD_128s		0x3
+
+#define MAX77714_CNFG_GLBL3	0x92
+#define MAX77714_WDTC			BIT(0)
+
+#define MAX77714_CNFG2_ONOFF	0x94
+#define MAX77714_WD_RST_WK		BIT(5)
+
+/* Interrupts */
+enum {
+	MAX77714_IRQ_TOP_ONOFF,
+	MAX77714_IRQ_TOP_RTC,		/* Real-time clock */
+	MAX77714_IRQ_TOP_GPIO,		/* GPIOs */
+	MAX77714_IRQ_TOP_LDO,		/* Low-dropout regulators */
+	MAX77714_IRQ_TOP_SD,		/* Step-down regulators */
+	MAX77714_IRQ_TOP_GLBL,		/* "Global resources": Low-Battery, overtemp... */
+};
+
+struct max77714 {
+	struct device *dev;
+	struct regmap *regmap;
+	struct regmap_irq_chip_data *irq_data;
+
+	int irq;
+};
+
+#endif /* _MFD_MAX77714_H_ */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 7/8] watchdog: max77714: add driver for the watchdog in the MAX77714 PMIC
  2021-10-11 15:56 [PATCH 0/8] Add MAX77714 PMIC minimal driver (RTC and watchdog only) Luca Ceresoli
                   ` (5 preceding siblings ...)
  2021-10-11 15:56 ` [PATCH 6/8] mfd: max77714: Add driver for " Luca Ceresoli
@ 2021-10-11 15:56 ` Luca Ceresoli
  2021-10-11 17:17   ` Guenter Roeck
  2021-10-12  1:18   ` Randy Dunlap
  2021-10-11 20:25 ` [PATCH 8/8] rtc: max77686: add MAX77714 support Luca Ceresoli
  2021-10-12  7:59 ` [PATCH 0/8] Add MAX77714 PMIC minimal driver (RTC and watchdog only) Krzysztof Kozlowski
  8 siblings, 2 replies; 41+ messages in thread
From: Luca Ceresoli @ 2021-10-11 15:56 UTC (permalink / raw)
  To: linux-kernel
  Cc: Luca Ceresoli, Lee Jones, Rob Herring, Alessandro Zummo,
	Alexandre Belloni, Chanwoo Choi, Krzysztof Kozlowski,
	Bartlomiej Zolnierkiewicz, Wim Van Sebroeck, Guenter Roeck,
	devicetree, linux-rtc, linux-watchdog, Chiwoong Byun,
	Laxman Dewangan

Add a simple driver to suppor the watchdog embedded in the Maxim MAX77714
PMIC.

Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
---
 MAINTAINERS                     |   1 +
 drivers/watchdog/Kconfig        |   9 ++
 drivers/watchdog/Makefile       |   1 +
 drivers/watchdog/max77714_wdt.c | 171 ++++++++++++++++++++++++++++++++
 4 files changed, 182 insertions(+)
 create mode 100644 drivers/watchdog/max77714_wdt.c

diff --git a/MAINTAINERS b/MAINTAINERS
index df394192f14e..08900b5729a5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11390,6 +11390,7 @@ M:	Luca Ceresoli <luca@lucaceresoli.net>
 S:	Maintained
 F:	Documentation/devicetree/bindings/mfd/maxim,max77714.yaml
 F:	drivers/mfd/max77714.c
+F:	drivers/watchdog/max77714_wdt.c
 F:	include/linux/mfd/max77714.h
 
 MAXIM MAX77802 PMIC REGULATOR DEVICE DRIVER
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index bf59faeb3de1..00bc3f932a6c 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -699,6 +699,15 @@ config MAX77620_WATCHDOG
 	 MAX77620 chips. To compile this driver as a module,
 	 choose M here: the module will be called max77620_wdt.
 
+config MAX77714_WATCHDOG
+	tristate "Maxim MAX77714 Watchdog Timer"
+	depends on MFD_MAX77714 || COMPILE_TEST
+	help
+	 This is the driver for watchdog timer in the MAX77714 PMIC.
+	 Say 'Y' here to enable the watchdog timer support for
+	 MAX77714 chips. To compile this driver as a module,
+	 choose M here: the module will be called max77714_wdt.
+
 config IMX2_WDT
 	tristate "IMX2+ Watchdog"
 	depends on ARCH_MXC || ARCH_LAYERSCAPE || COMPILE_TEST
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 1bd2d6f37c53..268a942311a0 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -215,6 +215,7 @@ obj-$(CONFIG_WM831X_WATCHDOG) += wm831x_wdt.o
 obj-$(CONFIG_WM8350_WATCHDOG) += wm8350_wdt.o
 obj-$(CONFIG_MAX63XX_WATCHDOG) += max63xx_wdt.o
 obj-$(CONFIG_MAX77620_WATCHDOG) += max77620_wdt.o
+obj-$(CONFIG_MAX77714_WATCHDOG) += max77714_wdt.o
 obj-$(CONFIG_ZIIRAVE_WATCHDOG) += ziirave_wdt.o
 obj-$(CONFIG_SOFT_WATCHDOG) += softdog.o
 obj-$(CONFIG_MENF21BMC_WATCHDOG) += menf21bmc_wdt.o
diff --git a/drivers/watchdog/max77714_wdt.c b/drivers/watchdog/max77714_wdt.c
new file mode 100644
index 000000000000..2d468db849f9
--- /dev/null
+++ b/drivers/watchdog/max77714_wdt.c
@@ -0,0 +1,171 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Maxim MAX77714 Watchdog Driver
+ *
+ * Copyright (C) 2021 Luca Ceresoli
+ * Author: Luca Ceresoli <luca@lucaceresoli.net>
+ */
+
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/mfd/max77714.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/watchdog.h>
+
+struct max77714_wdt {
+	struct device		*dev;
+	struct regmap		*rmap;
+	struct watchdog_device	wd_dev;
+};
+
+/* Timeout in seconds, indexed by TWD bits of CNFG_GLBL2 register */
+unsigned int max77714_margin_value[] = { 2, 16, 64, 128 };
+
+static int max77714_wdt_start(struct watchdog_device *wd_dev)
+{
+	struct max77714_wdt *wdt = watchdog_get_drvdata(wd_dev);
+
+	return regmap_update_bits(wdt->rmap, MAX77714_CNFG_GLBL2,
+				  MAX77714_WDTEN, MAX77714_WDTEN);
+}
+
+static int max77714_wdt_stop(struct watchdog_device *wd_dev)
+{
+	struct max77714_wdt *wdt = watchdog_get_drvdata(wd_dev);
+
+	return regmap_update_bits(wdt->rmap, MAX77714_CNFG_GLBL2,
+				  MAX77714_WDTEN, 0);
+}
+
+static int max77714_wdt_ping(struct watchdog_device *wd_dev)
+{
+	struct max77714_wdt *wdt = watchdog_get_drvdata(wd_dev);
+
+	return regmap_update_bits(wdt->rmap, MAX77714_CNFG_GLBL3,
+				  MAX77714_WDTC, 1);
+}
+
+static int max77714_wdt_set_timeout(struct watchdog_device *wd_dev,
+				    unsigned int timeout)
+{
+	struct max77714_wdt *wdt = watchdog_get_drvdata(wd_dev);
+	unsigned int new_timeout, new_twd;
+	int err;
+
+	for (new_twd = 0; new_twd < ARRAY_SIZE(max77714_margin_value) - 1; new_twd++)
+		if (timeout <= max77714_margin_value[new_twd])
+			break;
+
+	/* new_wdt is not out of bounds here due to the "- 1" in the for loop */
+	new_timeout = max77714_margin_value[new_twd];
+
+	/*
+	 * "If the value of TWD needs to be changed, clear the system
+	 * watchdog timer first [...], then change the value of TWD."
+	 * (MAX77714 datasheet)
+	 */
+	err = regmap_update_bits(wdt->rmap, MAX77714_CNFG_GLBL3,
+				 MAX77714_WDTC, 1);
+	if (err)
+		return err;
+
+	err = regmap_update_bits(wdt->rmap, MAX77714_CNFG_GLBL2,
+				 MAX77714_TWD_MASK, new_twd);
+	if (err)
+		return err;
+
+	wd_dev->timeout = new_timeout;
+
+	dev_dbg(wdt->dev, "New timeout = %u s (WDT = 0x%x)", new_timeout, new_twd);
+
+	return 0;
+}
+
+static const struct watchdog_info max77714_wdt_info = {
+	.identity = "max77714-watchdog",
+	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
+};
+
+static const struct watchdog_ops max77714_wdt_ops = {
+	.start		= max77714_wdt_start,
+	.stop		= max77714_wdt_stop,
+	.ping		= max77714_wdt_ping,
+	.set_timeout	= max77714_wdt_set_timeout,
+};
+
+static int max77714_wdt_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct max77714_wdt *wdt;
+	struct watchdog_device *wd_dev;
+	unsigned int regval;
+	int err;
+
+	wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL);
+	if (!wdt)
+		return -ENOMEM;
+
+	wdt->dev = dev;
+
+	wd_dev = &wdt->wd_dev;
+	wd_dev->info = &max77714_wdt_info;
+	wd_dev->ops = &max77714_wdt_ops;
+	wd_dev->min_timeout = 2;
+	wd_dev->max_timeout = 128;
+
+	platform_set_drvdata(pdev, wdt);
+	watchdog_set_drvdata(wd_dev, wdt);
+
+	wdt->rmap = dev_get_regmap(dev->parent, NULL);
+	if (!wdt->rmap)
+		return dev_err_probe(wdt->dev, -ENODEV, "Failed to get parent regmap\n");
+
+	/* WD_RST_WK: if 1 wdog restarts; if 0 wdog shuts down */
+	err = regmap_update_bits(wdt->rmap, MAX77714_CNFG2_ONOFF,
+				 MAX77714_WD_RST_WK, MAX77714_WD_RST_WK);
+	if (err)
+		return dev_err_probe(wdt->dev, err, "Error updating CNFG2_ONOFF\n");
+
+	err = regmap_read(wdt->rmap, MAX77714_CNFG_GLBL2, &regval);
+	if (err)
+		return dev_err_probe(wdt->dev, err, "Error reading CNFG_GLBL2\n");
+
+	/* enable watchdog | enable auto-clear in sleep state */
+	regval |= (MAX77714_WDTEN | MAX77714_WDTSLPC);
+
+	err = regmap_write(wdt->rmap, MAX77714_CNFG_GLBL2, regval);
+	if (err)
+		return dev_err_probe(wdt->dev, err, "Error writing CNFG_GLBL2\n");
+
+	wd_dev->timeout = max77714_margin_value[regval & MAX77714_TWD_MASK];
+
+	dev_dbg(wdt->dev, "Timeout = %u s (WDT = 0x%x)",
+		wd_dev->timeout, regval & MAX77714_TWD_MASK);
+
+	set_bit(WDOG_HW_RUNNING, &wd_dev->status);
+
+	watchdog_stop_on_unregister(wd_dev);
+
+	err = devm_watchdog_register_device(dev, wd_dev);
+	if (err)
+		return dev_err_probe(dev, err, "Cannot register watchdog device\n");
+
+	dev_info(dev, "registered as /dev/watchdog%d\n", wd_dev->id);
+
+	return 0;
+}
+
+static struct platform_driver max77714_wdt_driver = {
+	.driver	= {
+		.name	= "max77714-watchdog",
+	},
+	.probe	= max77714_wdt_probe,
+};
+
+module_platform_driver(max77714_wdt_driver);
+
+MODULE_DESCRIPTION("MAX77714 watchdog timer driver");
+MODULE_AUTHOR("Luca Ceresoli <luca@lucaceresoli.net>");
+MODULE_LICENSE("GPL v2");
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* Re: [PATCH 8/8] rtc: max77686: add MAX77714 support
  2021-10-11 20:25 ` [PATCH 8/8] rtc: max77686: add MAX77714 support Luca Ceresoli
@ 2021-10-11 16:12   ` Luca Ceresoli
  2021-10-12  8:20     ` Krzysztof Kozlowski
  2021-10-12  8:29   ` Krzysztof Kozlowski
  2021-10-15 17:36   ` Alexandre Belloni
  2 siblings, 1 reply; 41+ messages in thread
From: Luca Ceresoli @ 2021-10-11 16:12 UTC (permalink / raw)
  To: linux-kernel
  Cc: Lee Jones, Rob Herring, Alessandro Zummo, Alexandre Belloni,
	Chanwoo Choi, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz,
	Wim Van Sebroeck, Guenter Roeck, devicetree, linux-rtc,
	linux-watchdog, Chiwoong Byun, Laxman Dewangan

Hi,

see below for the issues with interrupt implementation that I mentioned
in the cover letter.

On 11/10/21 17:56, Luca Ceresoli wrote:
> The RTC included in the MAX77714 PMIC is very similar to the one in the
> MAX77686. Reuse the rtc-max77686.c driver with the minimum required changes
> for the MAX77714 RTC.
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> ---
>  drivers/rtc/Kconfig        |  2 +-
>  drivers/rtc/rtc-max77686.c | 24 ++++++++++++++++++++++++
>  2 files changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index e1bc5214494e..a73591ad292b 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -375,7 +375,7 @@ config RTC_DRV_MAX8997
>  
>  config RTC_DRV_MAX77686
>  	tristate "Maxim MAX77686"
> -	depends on MFD_MAX77686 || MFD_MAX77620 || COMPILE_TEST
> +	depends on MFD_MAX77686 || MFD_MAX77620 || MFD_MAX77714 || COMPILE_TEST
>  	help
>  	  If you say yes here you will get support for the
>  	  RTC of Maxim MAX77686/MAX77620/MAX77802 PMIC.
> diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
> index 9901c596998a..e6564bc2171e 100644
> --- a/drivers/rtc/rtc-max77686.c
> +++ b/drivers/rtc/rtc-max77686.c
> @@ -19,6 +19,7 @@
>  
>  #define MAX77686_I2C_ADDR_RTC		(0x0C >> 1)
>  #define MAX77620_I2C_ADDR_RTC		0x68
> +#define MAX77714_I2C_ADDR_RTC		0x48
>  #define MAX77686_INVALID_I2C_ADDR	(-1)
>  
>  /* Define non existing register */
> @@ -203,6 +204,28 @@ static const struct max77686_rtc_driver_data max77686_drv_data = {
>  	.regmap_config = &max77686_rtc_regmap_config,
>  };
>  
> +static const struct regmap_irq_chip max77714_rtc_irq_chip = {
> +	.name		= "max77714-rtc",
> +	.status_base	= MAX77686_RTC_INT,
> +	.mask_base	= MAX77686_RTC_INTM,
> +	.num_regs	= 1,
> +	.irqs		= max77686_rtc_irqs,
> +	.num_irqs	= ARRAY_SIZE(max77686_rtc_irqs) - 1, /* no WTSR on 77714 */
> +};
> +
> +static const struct max77686_rtc_driver_data max77714_drv_data = {
> +	.delay = 16000,
> +	.mask  = 0x7f,
> +	.map   = max77686_map,
> +	.alarm_enable_reg = false,
> +	.rtc_irq_from_platform = false,

As far as I could understand, rtc_irq_from_platform should be 'true'.
This would trigger the 'if' branch in function
max77686_init_rtc_regmap() [0]:

  if (info->drv_data->rtc_irq_from_platform) {
	struct platform_device *pdev = to_platform_device(info->dev);

	info->rtc_irq = platform_get_irq(pdev, 0);
	if (info->rtc_irq < 0)
		return info->rtc_irq;
  } else {
	info->rtc_irq =  parent_i2c->irq;
  }

Calling platform_get_irq() seems correct for the MAX77714, which can
generate various IRQ events, collecting them in a register, and raise a
single IRQ to the CPU via a physical pin.

However, if I set rtc_irq_from_platform = true, platform_get_irq()
returns IRQ number '1', which ends up in:

  dummy 0-0048: Failed to request IRQ 1 for max77714-rtc: -22
  max77686-rtc max77714-rtc: Failed to add RTC irq chip: -22
  max77686-rtc: probe of max77714-rtc failed with error -22

I compared my code with other MFD drivers and their cell drivers (but
their datasheets is not available so I had to add some guesswork), and
couldn't find out where my code is wrong.

Unfortunately I have no IRQ access on my board (and I don't need them
for my use case). For this reason I initially thought of disabling all
the IRQ code in rtc-max77686.c via a new flag, but it would be quite
invasive and I wouldn't even be able to test that existing hardware
still works. Implementing a new RTC driver for the MAX77714 does not
seem to be a sane option as the hardware is really 99% equal to the
MAX77686 RTC.

Any suggestions on how to move on? -- Thanks!

[0]
https://elixir.bootlin.com/linux/latest/source/drivers/rtc/rtc-max77686.c#L676

Regards,
-- 
Luca

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 6/8] mfd: max77714: Add driver for Maxim MAX77714 PMIC
  2021-10-11 15:56 ` [PATCH 6/8] mfd: max77714: Add driver for " Luca Ceresoli
@ 2021-10-11 17:15   ` Guenter Roeck
  2021-10-12  7:11   ` kernel test robot
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 41+ messages in thread
From: Guenter Roeck @ 2021-10-11 17:15 UTC (permalink / raw)
  To: Luca Ceresoli, linux-kernel
  Cc: Lee Jones, Rob Herring, Alessandro Zummo, Alexandre Belloni,
	Chanwoo Choi, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz,
	Wim Van Sebroeck, devicetree, linux-rtc, linux-watchdog,
	Chiwoong Byun, Laxman Dewangan

On 10/11/21 8:56 AM, 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>
> ---
>   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"
> +	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 2ba6646e874c..fe43f2fdd5cb 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -163,6 +163,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..5d6c88d4d6c0
> --- /dev/null
> +++ b/drivers/mfd/max77714.c
> @@ -0,0 +1,151 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Maxim MAX77714 Watchdog Driver

watchdog ?

> + *
> + * Copyright (C) 2021 Luca Ceresoli
> + * Author: Luca Ceresoli <luca@lucaceresoli.net>
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/max77714.h>
> +#include <linux/of.h>
> +#include <linux/regmap.h>
> +
> +static const struct regmap_range max77714_readable_ranges[] = {
> +	regmap_reg_range(MAX77714_INT_TOP,     MAX77714_INT_TOP),
> +	regmap_reg_range(MAX77714_INT_TOPM,    MAX77714_INT_TOPM),
> +	regmap_reg_range(MAX77714_32K_STATUS,  MAX77714_32K_CONFIG),
> +	regmap_reg_range(MAX77714_CNFG_GLBL2,  MAX77714_CNFG2_ONOFF),
> +};
> +
> +static const struct regmap_range max77714_writable_ranges[] = {
> +	regmap_reg_range(MAX77714_INT_TOPM,    MAX77714_INT_TOPM),
> +	regmap_reg_range(MAX77714_32K_CONFIG,  MAX77714_32K_CONFIG),
> +	regmap_reg_range(MAX77714_CNFG_GLBL2,  MAX77714_CNFG2_ONOFF),
> +};
> +
> +static const struct regmap_access_table max77714_readable_table = {
> +	.yes_ranges = max77714_readable_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(max77714_readable_ranges),
> +};
> +
> +static const struct regmap_access_table max77714_writable_table = {
> +	.yes_ranges = max77714_writable_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(max77714_writable_ranges),
> +};
> +
> +static const struct regmap_config max77714_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = MAX77714_CNFG2_ONOFF,
> +	.rd_table = &max77714_readable_table,
> +	.wr_table = &max77714_writable_table,
> +};
> +
> +static const struct regmap_irq max77714_top_irqs[] = {
> +	REGMAP_IRQ_REG(MAX77714_IRQ_TOP_ONOFF,   0, MAX77714_INT_TOP_ONOFF),
> +	REGMAP_IRQ_REG(MAX77714_IRQ_TOP_RTC,     0, MAX77714_INT_TOP_RTC),
> +	REGMAP_IRQ_REG(MAX77714_IRQ_TOP_GPIO,    0, MAX77714_INT_TOP_GPIO),
> +	REGMAP_IRQ_REG(MAX77714_IRQ_TOP_LDO,     0, MAX77714_INT_TOP_LDO),
> +	REGMAP_IRQ_REG(MAX77714_IRQ_TOP_SD,      0, MAX77714_INT_TOP_SD),
> +	REGMAP_IRQ_REG(MAX77714_IRQ_TOP_GLBL,    0, MAX77714_INT_TOP_GLBL),
> +};
> +
> +static const struct regmap_irq_chip max77714_irq_chip = {
> +	.name			= "max77714-pmic",
> +	.status_base		= MAX77714_INT_TOP,
> +	.mask_base		= MAX77714_INT_TOPM,
> +	.num_regs		= 1,
> +	.irqs			= max77714_top_irqs,
> +	.num_irqs		= ARRAY_SIZE(max77714_top_irqs),
> +};
> +
> +static const struct mfd_cell max77714_cells[] = {
> +	{ .name = "max77714-watchdog" },
> +	{ .name = "max77714-rtc" },
> +};
> +
> +/*
> + * MAX77714 initially uses the internal, low precision oscillator. Enable
> + * the external oscillator by setting the XOSC_RETRY bit. If the external
> + * oscillator is not OK (probably not installed) this has no effect.
> + */
> +static int max77714_setup_xosc(struct max77714 *chip)
> +{
> +	/* Internal Crystal Load Capacitance, indexed by value of 32KLOAD bits */
> +	static const unsigned int load_cap[4] = {0, 10, 12, 22};
> +	unsigned int load_cap_idx;
> +	unsigned int status;
> +	int err;
> +
> +	err = regmap_update_bits(chip->regmap, MAX77714_32K_CONFIG,
> +				 MAX77714_32K_CONFIG_XOSC_RETRY,
> +				 MAX77714_32K_CONFIG_XOSC_RETRY);
> +	if (err)
> +		return dev_err_probe(chip->dev, err, "cannot configure XOSC\n");
> +
> +	err = regmap_read(chip->regmap, MAX77714_32K_STATUS, &status);
> +	if (err)
> +		return dev_err_probe(chip->dev, err, "cannot read XOSC status\n");
> +
> +	load_cap_idx = (status >> MAX77714_32K_STATUS_32KLOAD_SHF)
> +		& MAX77714_32K_STATUS_32KLOAD_MSK;
> +
> +	dev_info(chip->dev, "Using %s oscillator, %d pF load cap\n",
> +		 status & MAX77714_32K_STATUS_32KSOURCE ? "internal" : "external",
> +		 load_cap[load_cap_idx]);
> +
> +	return 0;
> +}
> +
> +static int max77714_probe(struct i2c_client *client)
> +{
> +	struct max77714 *chip;
> +	int err;
> +
> +	chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
> +	if (!chip)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(client, chip);
> +	chip->dev = &client->dev;
> +
> +	chip->regmap = devm_regmap_init_i2c(client, &max77714_regmap_config);
> +	if (IS_ERR(chip->regmap))
> +		return dev_err_probe(chip->dev, PTR_ERR(chip->regmap),
> +				     "failed to initialise regmap\n");
> +
> +	err = max77714_setup_xosc(chip);
> +	if (err)
> +		return err;
> +
> +	err = devm_regmap_add_irq_chip(chip->dev, chip->regmap, client->irq,
> +				       IRQF_ONESHOT | IRQF_SHARED, 0,
> +				       &max77714_irq_chip, &chip->irq_data);
> +	if (err)
> +		return dev_err_probe(chip->dev, err, "failed to add PMIC irq chip\n");
> +
> +	err =  devm_mfd_add_devices(chip->dev, PLATFORM_DEVID_NONE,
> +				    max77714_cells, ARRAY_SIZE(max77714_cells),
> +				    NULL, 0, NULL);
> +	if (err)
> +		return dev_err_probe(chip->dev, err, "failed adding MFD children\n");
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id max77714_dt_match[] = {
> +	{ .compatible = "maxim,max77714" },
> +	{},
> +};
> +
> +static struct i2c_driver max77714_driver = {
> +	.driver = {
> +		.name = "max77714",
> +		.of_match_table = of_match_ptr(max77714_dt_match),
> +	},
> +	.probe_new = max77714_probe,
> +};
> +builtin_i2c_driver(max77714_driver);
> 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_
> +
> +#include <linux/bits.h>
> +
> +#define MAX77714_INT_TOP	0x00
> +#define MAX77714_INT_TOPM	0x07 /* Datasheet says "read only", but it is RW */
> +
> +#define MAX77714_INT_TOP_ONOFF		BIT(1)
> +#define MAX77714_INT_TOP_RTC		BIT(3)
> +#define MAX77714_INT_TOP_GPIO		BIT(4)
> +#define MAX77714_INT_TOP_LDO		BIT(5)
> +#define MAX77714_INT_TOP_SD		BIT(6)
> +#define MAX77714_INT_TOP_GLBL		BIT(7)
> +
> +#define MAX77714_32K_STATUS	0x30
> +#define MAX77714_32K_STATUS_SIOSCOK	BIT(5)
> +#define MAX77714_32K_STATUS_XOSCOK	BIT(4)
> +#define MAX77714_32K_STATUS_32KSOURCE	BIT(3)
> +#define MAX77714_32K_STATUS_32KLOAD_MSK	0x3
> +#define MAX77714_32K_STATUS_32KLOAD_SHF	1
> +#define MAX77714_32K_STATUS_CRYSTAL_CFG	BIT(0)
> +
> +#define MAX77714_32K_CONFIG	0x31
> +#define MAX77714_32K_CONFIG_XOSC_RETRY	BIT(4)
> +
> +#define MAX77714_CNFG_GLBL2	0x91
> +#define MAX77714_WDTEN			BIT(2)
> +#define MAX77714_WDTSLPC		BIT(3)
> +#define MAX77714_TWD_MASK		0x3
> +#define MAX77714_TWD_2s			0x0
> +#define MAX77714_TWD_16s		0x1
> +#define MAX77714_TWD_64s		0x2
> +#define MAX77714_TWD_128s		0x3
> +
> +#define MAX77714_CNFG_GLBL3	0x92
> +#define MAX77714_WDTC			BIT(0)
> +
> +#define MAX77714_CNFG2_ONOFF	0x94
> +#define MAX77714_WD_RST_WK		BIT(5)
> +
> +/* Interrupts */
> +enum {
> +	MAX77714_IRQ_TOP_ONOFF,
> +	MAX77714_IRQ_TOP_RTC,		/* Real-time clock */
> +	MAX77714_IRQ_TOP_GPIO,		/* GPIOs */
> +	MAX77714_IRQ_TOP_LDO,		/* Low-dropout regulators */
> +	MAX77714_IRQ_TOP_SD,		/* Step-down regulators */
> +	MAX77714_IRQ_TOP_GLBL,		/* "Global resources": Low-Battery, overtemp... */
> +};
> +
> +struct max77714 {
> +	struct device *dev;
> +	struct regmap *regmap;
> +	struct regmap_irq_chip_data *irq_data;
> +
> +	int irq;
> +};
> +
> +#endif /* _MFD_MAX77714_H_ */
> 


^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 7/8] watchdog: max77714: add driver for the watchdog in the MAX77714 PMIC
  2021-10-11 15:56 ` [PATCH 7/8] watchdog: max77714: add driver for the watchdog in the " Luca Ceresoli
@ 2021-10-11 17:17   ` Guenter Roeck
  2021-10-15 16:43     ` Luca Ceresoli
  2021-10-12  1:18   ` Randy Dunlap
  1 sibling, 1 reply; 41+ messages in thread
From: Guenter Roeck @ 2021-10-11 17:17 UTC (permalink / raw)
  To: Luca Ceresoli, linux-kernel
  Cc: Lee Jones, Rob Herring, Alessandro Zummo, Alexandre Belloni,
	Chanwoo Choi, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz,
	Wim Van Sebroeck, devicetree, linux-rtc, linux-watchdog,
	Chiwoong Byun, Laxman Dewangan

On 10/11/21 8:56 AM, Luca Ceresoli wrote:
> Add a simple driver to suppor the watchdog embedded in the Maxim MAX77714
> PMIC.
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> ---
>   MAINTAINERS                     |   1 +
>   drivers/watchdog/Kconfig        |   9 ++
>   drivers/watchdog/Makefile       |   1 +
>   drivers/watchdog/max77714_wdt.c | 171 ++++++++++++++++++++++++++++++++
>   4 files changed, 182 insertions(+)
>   create mode 100644 drivers/watchdog/max77714_wdt.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index df394192f14e..08900b5729a5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11390,6 +11390,7 @@ M:	Luca Ceresoli <luca@lucaceresoli.net>
>   S:	Maintained
>   F:	Documentation/devicetree/bindings/mfd/maxim,max77714.yaml
>   F:	drivers/mfd/max77714.c
> +F:	drivers/watchdog/max77714_wdt.c
>   F:	include/linux/mfd/max77714.h
>   
>   MAXIM MAX77802 PMIC REGULATOR DEVICE DRIVER
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index bf59faeb3de1..00bc3f932a6c 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -699,6 +699,15 @@ config MAX77620_WATCHDOG
>   	 MAX77620 chips. To compile this driver as a module,
>   	 choose M here: the module will be called max77620_wdt.
>   
> +config MAX77714_WATCHDOG
> +	tristate "Maxim MAX77714 Watchdog Timer"
> +	depends on MFD_MAX77714 || COMPILE_TEST
> +	help
> +	 This is the driver for watchdog timer in the MAX77714 PMIC.
> +	 Say 'Y' here to enable the watchdog timer support for
> +	 MAX77714 chips. To compile this driver as a module,
> +	 choose M here: the module will be called max77714_wdt.
> +
>   config IMX2_WDT
>   	tristate "IMX2+ Watchdog"
>   	depends on ARCH_MXC || ARCH_LAYERSCAPE || COMPILE_TEST
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 1bd2d6f37c53..268a942311a0 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -215,6 +215,7 @@ obj-$(CONFIG_WM831X_WATCHDOG) += wm831x_wdt.o
>   obj-$(CONFIG_WM8350_WATCHDOG) += wm8350_wdt.o
>   obj-$(CONFIG_MAX63XX_WATCHDOG) += max63xx_wdt.o
>   obj-$(CONFIG_MAX77620_WATCHDOG) += max77620_wdt.o
> +obj-$(CONFIG_MAX77714_WATCHDOG) += max77714_wdt.o
>   obj-$(CONFIG_ZIIRAVE_WATCHDOG) += ziirave_wdt.o
>   obj-$(CONFIG_SOFT_WATCHDOG) += softdog.o
>   obj-$(CONFIG_MENF21BMC_WATCHDOG) += menf21bmc_wdt.o
> diff --git a/drivers/watchdog/max77714_wdt.c b/drivers/watchdog/max77714_wdt.c
> new file mode 100644
> index 000000000000..2d468db849f9
> --- /dev/null
> +++ b/drivers/watchdog/max77714_wdt.c
> @@ -0,0 +1,171 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Maxim MAX77714 Watchdog Driver
> + *
> + * Copyright (C) 2021 Luca Ceresoli
> + * Author: Luca Ceresoli <luca@lucaceresoli.net>
> + */
> +
> +#include <linux/err.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/max77714.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/watchdog.h>
> +
> +struct max77714_wdt {
> +	struct device		*dev;
> +	struct regmap		*rmap;
> +	struct watchdog_device	wd_dev;
> +};
> +
> +/* Timeout in seconds, indexed by TWD bits of CNFG_GLBL2 register */
> +unsigned int max77714_margin_value[] = { 2, 16, 64, 128 };

static

> +
> +static int max77714_wdt_start(struct watchdog_device *wd_dev)
> +{
> +	struct max77714_wdt *wdt = watchdog_get_drvdata(wd_dev);
> +
> +	return regmap_update_bits(wdt->rmap, MAX77714_CNFG_GLBL2,
> +				  MAX77714_WDTEN, MAX77714_WDTEN);
> +}
> +
> +static int max77714_wdt_stop(struct watchdog_device *wd_dev)
> +{
> +	struct max77714_wdt *wdt = watchdog_get_drvdata(wd_dev);
> +
> +	return regmap_update_bits(wdt->rmap, MAX77714_CNFG_GLBL2,
> +				  MAX77714_WDTEN, 0);
> +}
> +
> +static int max77714_wdt_ping(struct watchdog_device *wd_dev)
> +{
> +	struct max77714_wdt *wdt = watchdog_get_drvdata(wd_dev);
> +
> +	return regmap_update_bits(wdt->rmap, MAX77714_CNFG_GLBL3,
> +				  MAX77714_WDTC, 1);
> +}
> +
> +static int max77714_wdt_set_timeout(struct watchdog_device *wd_dev,
> +				    unsigned int timeout)
> +{
> +	struct max77714_wdt *wdt = watchdog_get_drvdata(wd_dev);
> +	unsigned int new_timeout, new_twd;
> +	int err;
> +
> +	for (new_twd = 0; new_twd < ARRAY_SIZE(max77714_margin_value) - 1; new_twd++)
> +		if (timeout <= max77714_margin_value[new_twd])
> +			break;
> +
> +	/* new_wdt is not out of bounds here due to the "- 1" in the for loop */
> +	new_timeout = max77714_margin_value[new_twd];
> +
> +	/*
> +	 * "If the value of TWD needs to be changed, clear the system
> +	 * watchdog timer first [...], then change the value of TWD."
> +	 * (MAX77714 datasheet)
> +	 */
> +	err = regmap_update_bits(wdt->rmap, MAX77714_CNFG_GLBL3,
> +				 MAX77714_WDTC, 1);
> +	if (err)
> +		return err;
> +
> +	err = regmap_update_bits(wdt->rmap, MAX77714_CNFG_GLBL2,
> +				 MAX77714_TWD_MASK, new_twd);
> +	if (err)
> +		return err;
> +
> +	wd_dev->timeout = new_timeout;
> +
> +	dev_dbg(wdt->dev, "New timeout = %u s (WDT = 0x%x)", new_timeout, new_twd);
> +
> +	return 0;
> +}
> +
> +static const struct watchdog_info max77714_wdt_info = {
> +	.identity = "max77714-watchdog",
> +	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
> +};
> +
> +static const struct watchdog_ops max77714_wdt_ops = {
> +	.start		= max77714_wdt_start,
> +	.stop		= max77714_wdt_stop,
> +	.ping		= max77714_wdt_ping,
> +	.set_timeout	= max77714_wdt_set_timeout,
> +};
> +
> +static int max77714_wdt_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct max77714_wdt *wdt;
> +	struct watchdog_device *wd_dev;
> +	unsigned int regval;
> +	int err;
> +
> +	wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL);
> +	if (!wdt)
> +		return -ENOMEM;
> +
> +	wdt->dev = dev;
> +
> +	wd_dev = &wdt->wd_dev;
> +	wd_dev->info = &max77714_wdt_info;
> +	wd_dev->ops = &max77714_wdt_ops;
> +	wd_dev->min_timeout = 2;
> +	wd_dev->max_timeout = 128;
> +
> +	platform_set_drvdata(pdev, wdt);
> +	watchdog_set_drvdata(wd_dev, wdt);
> +
> +	wdt->rmap = dev_get_regmap(dev->parent, NULL);
> +	if (!wdt->rmap)
> +		return dev_err_probe(wdt->dev, -ENODEV, "Failed to get parent regmap\n");
> +
> +	/* WD_RST_WK: if 1 wdog restarts; if 0 wdog shuts down */
> +	err = regmap_update_bits(wdt->rmap, MAX77714_CNFG2_ONOFF,
> +				 MAX77714_WD_RST_WK, MAX77714_WD_RST_WK);
> +	if (err)
> +		return dev_err_probe(wdt->dev, err, "Error updating CNFG2_ONOFF\n");
> +
> +	err = regmap_read(wdt->rmap, MAX77714_CNFG_GLBL2, &regval);
> +	if (err)
> +		return dev_err_probe(wdt->dev, err, "Error reading CNFG_GLBL2\n");
> +
> +	/* enable watchdog | enable auto-clear in sleep state */
> +	regval |= (MAX77714_WDTEN | MAX77714_WDTSLPC);
> +
> +	err = regmap_write(wdt->rmap, MAX77714_CNFG_GLBL2, regval);
> +	if (err)
> +		return dev_err_probe(wdt->dev, err, "Error writing CNFG_GLBL2\n");
> +
> +	wd_dev->timeout = max77714_margin_value[regval & MAX77714_TWD_MASK];
> +
> +	dev_dbg(wdt->dev, "Timeout = %u s (WDT = 0x%x)",
> +		wd_dev->timeout, regval & MAX77714_TWD_MASK);
> +
> +	set_bit(WDOG_HW_RUNNING, &wd_dev->status);
> +
> +	watchdog_stop_on_unregister(wd_dev);
> +
> +	err = devm_watchdog_register_device(dev, wd_dev);
> +	if (err)
> +		return dev_err_probe(dev, err, "Cannot register watchdog device\n");
> +
> +	dev_info(dev, "registered as /dev/watchdog%d\n", wd_dev->id);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver max77714_wdt_driver = {
> +	.driver	= {
> +		.name	= "max77714-watchdog",
> +	},
> +	.probe	= max77714_wdt_probe,
> +};
> +
> +module_platform_driver(max77714_wdt_driver);
> +
> +MODULE_DESCRIPTION("MAX77714 watchdog timer driver");
> +MODULE_AUTHOR("Luca Ceresoli <luca@lucaceresoli.net>");
> +MODULE_LICENSE("GPL v2");
> 


^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH 8/8] rtc: max77686: add MAX77714 support
  2021-10-11 15:56 [PATCH 0/8] Add MAX77714 PMIC minimal driver (RTC and watchdog only) Luca Ceresoli
                   ` (6 preceding siblings ...)
  2021-10-11 15:56 ` [PATCH 7/8] watchdog: max77714: add driver for the watchdog in the " Luca Ceresoli
@ 2021-10-11 20:25 ` Luca Ceresoli
  2021-10-11 16:12   ` Luca Ceresoli
                     ` (2 more replies)
  2021-10-12  7:59 ` [PATCH 0/8] Add MAX77714 PMIC minimal driver (RTC and watchdog only) Krzysztof Kozlowski
  8 siblings, 3 replies; 41+ messages in thread
From: Luca Ceresoli @ 2021-10-11 20:25 UTC (permalink / raw)
  To: linux-kernel
  Cc: Luca Ceresoli, Lee Jones, Rob Herring, Alessandro Zummo,
	Alexandre Belloni, Chanwoo Choi, Krzysztof Kozlowski,
	Bartlomiej Zolnierkiewicz, Wim Van Sebroeck, Guenter Roeck,
	devicetree, linux-rtc, linux-watchdog, Chiwoong Byun,
	Laxman Dewangan

The RTC included in the MAX77714 PMIC is very similar to the one in the
MAX77686. Reuse the rtc-max77686.c driver with the minimum required changes
for the MAX77714 RTC.

Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
---

*** NOTE ***

This patch didn't reach most recipients having hit a limit in my service
provider (125 e-mails per hour). I'm resending it, as far as possible with
proper message-id etc. Apologies for any duplicate.

 drivers/rtc/Kconfig        |  2 +-
 drivers/rtc/rtc-max77686.c | 24 ++++++++++++++++++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index e1bc5214494e..a73591ad292b 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -375,7 +375,7 @@ config RTC_DRV_MAX8997

 config RTC_DRV_MAX77686
 	tristate "Maxim MAX77686"
-	depends on MFD_MAX77686 || MFD_MAX77620 || COMPILE_TEST
+	depends on MFD_MAX77686 || MFD_MAX77620 || MFD_MAX77714 || COMPILE_TEST
 	help
 	  If you say yes here you will get support for the
 	  RTC of Maxim MAX77686/MAX77620/MAX77802 PMIC.
diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
index 9901c596998a..e6564bc2171e 100644
--- a/drivers/rtc/rtc-max77686.c
+++ b/drivers/rtc/rtc-max77686.c
@@ -19,6 +19,7 @@

 #define MAX77686_I2C_ADDR_RTC		(0x0C >> 1)
 #define MAX77620_I2C_ADDR_RTC		0x68
+#define MAX77714_I2C_ADDR_RTC		0x48
 #define MAX77686_INVALID_I2C_ADDR	(-1)

 /* Define non existing register */
@@ -203,6 +204,28 @@ static const struct max77686_rtc_driver_data max77686_drv_data = {
 	.regmap_config = &max77686_rtc_regmap_config,
 };

+static const struct regmap_irq_chip max77714_rtc_irq_chip = {
+	.name		= "max77714-rtc",
+	.status_base	= MAX77686_RTC_INT,
+	.mask_base	= MAX77686_RTC_INTM,
+	.num_regs	= 1,
+	.irqs		= max77686_rtc_irqs,
+	.num_irqs	= ARRAY_SIZE(max77686_rtc_irqs) - 1, /* no WTSR on 77714 */
+};
+
+static const struct max77686_rtc_driver_data max77714_drv_data = {
+	.delay = 16000,
+	.mask  = 0x7f,
+	.map   = max77686_map,
+	.alarm_enable_reg = false,
+	.rtc_irq_from_platform = false,
+	/* On MAX77714 RTCA1 is BIT 1 of RTCINT (0x00). Not supported by this driver. */
+	.alarm_pending_status_reg = MAX77686_INVALID_REG,
+	.rtc_i2c_addr = MAX77714_I2C_ADDR_RTC,
+	.rtc_irq_chip = &max77714_rtc_irq_chip,
+	.regmap_config = &max77686_rtc_regmap_config,
+};
+
 static const struct regmap_config max77620_rtc_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,
@@ -846,6 +869,7 @@ static const struct platform_device_id rtc_id[] = {
 	{ "max77686-rtc", .driver_data = (kernel_ulong_t)&max77686_drv_data, },
 	{ "max77802-rtc", .driver_data = (kernel_ulong_t)&max77802_drv_data, },
 	{ "max77620-rtc", .driver_data = (kernel_ulong_t)&max77620_drv_data, },
+	{ "max77714-rtc", .driver_data = (kernel_ulong_t)&max77714_drv_data, },
 	{},
 };
 MODULE_DEVICE_TABLE(platform, rtc_id);
--
2.25.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* Re: [PATCH 7/8] watchdog: max77714: add driver for the watchdog in the MAX77714 PMIC
  2021-10-11 15:56 ` [PATCH 7/8] watchdog: max77714: add driver for the watchdog in the " Luca Ceresoli
  2021-10-11 17:17   ` Guenter Roeck
@ 2021-10-12  1:18   ` Randy Dunlap
  2021-10-15 16:42     ` Luca Ceresoli
  1 sibling, 1 reply; 41+ messages in thread
From: Randy Dunlap @ 2021-10-12  1:18 UTC (permalink / raw)
  To: Luca Ceresoli, linux-kernel
  Cc: Lee Jones, Rob Herring, Alessandro Zummo, Alexandre Belloni,
	Chanwoo Choi, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz,
	Wim Van Sebroeck, Guenter Roeck, devicetree, linux-rtc,
	linux-watchdog, Chiwoong Byun, Laxman Dewangan

Hi,

On 10/11/21 8:56 AM, Luca Ceresoli wrote:
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index bf59faeb3de1..00bc3f932a6c 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -699,6 +699,15 @@ config MAX77620_WATCHDOG
>   	 MAX77620 chips. To compile this driver as a module,
>   	 choose M here: the module will be called max77620_wdt.
>   
> +config MAX77714_WATCHDOG
> +	tristate "Maxim MAX77714 Watchdog Timer"
> +	depends on MFD_MAX77714 || COMPILE_TEST
> +	help
> +	 This is the driver for watchdog timer in the MAX77714 PMIC.
> +	 Say 'Y' here to enable the watchdog timer support for
> +	 MAX77714 chips. To compile this driver as a module,
> +	 choose M here: the module will be called max77714_wdt.

Please follow coding-style for Kconfig files:

(from Documentation/process/coding-style.rst, section 10):

For all of the Kconfig* configuration files throughout the source tree,
the indentation is somewhat different.  Lines under a ``config`` definition
are indented with one tab, while help text is indented an additional two
spaces.


-- 
~Randy

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 3/8] rtc: max77686: rename day-of-month defines
  2021-10-11 15:56 ` [PATCH 3/8] rtc: max77686: rename day-of-month defines Luca Ceresoli
@ 2021-10-12  2:23   ` kernel test robot
  2021-10-12  2:58   ` kernel test robot
  2021-10-12  8:13   ` Krzysztof Kozlowski
  2 siblings, 0 replies; 41+ messages in thread
From: kernel test robot @ 2021-10-12  2:23 UTC (permalink / raw)
  To: Luca Ceresoli, linux-kernel
  Cc: kbuild-all, Luca Ceresoli, Lee Jones, Rob Herring,
	Alessandro Zummo, Alexandre Belloni, Chanwoo Choi,
	Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz, Wim Van Sebroeck

[-- Attachment #1: Type: text/plain, Size: 3451 bytes --]

Hi Luca,

I love your patch! Yet something to improve:

[auto build test ERROR on lee-mfd/for-mfd-next]
[also build test ERROR on abelloni/rtc-next linux/master linus/master v5.15-rc5 next-20211011]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Luca-Ceresoli/Add-MAX77714-PMIC-minimal-driver-RTC-and-watchdog-only/20211012-042754
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: h8300-randconfig-r015-20211011 (attached as .config)
compiler: h8300-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/6fc9e86fa2de7918c36cb31070f4d4eb7d48bff0
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Luca-Ceresoli/Add-MAX77714-PMIC-minimal-driver-RTC-and-watchdog-only/20211012-042754
        git checkout 6fc9e86fa2de7918c36cb31070f4d4eb7d48bff0
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=h8300 SHELL=/bin/bash drivers/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/mfd/max77686.c: In function 'max77802_rtc_is_volatile_reg':
>> drivers/mfd/max77686.c:90:24: error: 'MAX77802_RTC_DATE' undeclared (first use in this function); did you mean 'MAX77802_RTC_AE2'?
      90 |                 reg == MAX77802_RTC_DATE);
         |                        ^~~~~~~~~~~~~~~~~
         |                        MAX77802_RTC_AE2
   drivers/mfd/max77686.c:90:24: note: each undeclared identifier is reported only once for each function it appears in
   drivers/mfd/max77686.c:91:1: error: control reaches end of non-void function [-Werror=return-type]
      91 | }
         | ^
   cc1: some warnings being treated as errors


vim +90 drivers/mfd/max77686.c

a259f3896a39ec Javier Martinez Canillas 2014-07-24  80  
a259f3896a39ec Javier Martinez Canillas 2014-07-24  81  static bool max77802_rtc_is_volatile_reg(struct device *dev, unsigned int reg)
a259f3896a39ec Javier Martinez Canillas 2014-07-24  82  {
a259f3896a39ec Javier Martinez Canillas 2014-07-24  83  	return (max77802_rtc_is_precious_reg(dev, reg) ||
a259f3896a39ec Javier Martinez Canillas 2014-07-24  84  		reg == MAX77802_RTC_SEC ||
a259f3896a39ec Javier Martinez Canillas 2014-07-24  85  		reg == MAX77802_RTC_MIN ||
a259f3896a39ec Javier Martinez Canillas 2014-07-24  86  		reg == MAX77802_RTC_HOUR ||
a259f3896a39ec Javier Martinez Canillas 2014-07-24  87  		reg == MAX77802_RTC_WEEKDAY ||
a259f3896a39ec Javier Martinez Canillas 2014-07-24  88  		reg == MAX77802_RTC_MONTH ||
a259f3896a39ec Javier Martinez Canillas 2014-07-24  89  		reg == MAX77802_RTC_YEAR ||
a259f3896a39ec Javier Martinez Canillas 2014-07-24 @90  		reg == MAX77802_RTC_DATE);
a259f3896a39ec Javier Martinez Canillas 2014-07-24  91  }
a259f3896a39ec Javier Martinez Canillas 2014-07-24  92  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 32518 bytes --]

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 3/8] rtc: max77686: rename day-of-month defines
  2021-10-11 15:56 ` [PATCH 3/8] rtc: max77686: rename day-of-month defines Luca Ceresoli
  2021-10-12  2:23   ` kernel test robot
@ 2021-10-12  2:58   ` kernel test robot
  2021-10-12  8:13   ` Krzysztof Kozlowski
  2 siblings, 0 replies; 41+ messages in thread
From: kernel test robot @ 2021-10-12  2:58 UTC (permalink / raw)
  To: Luca Ceresoli, linux-kernel
  Cc: llvm, kbuild-all, Luca Ceresoli, Lee Jones, Rob Herring,
	Alessandro Zummo, Alexandre Belloni, Chanwoo Choi,
	Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz, Wim Van Sebroeck

[-- Attachment #1: Type: text/plain, Size: 3079 bytes --]

Hi Luca,

I love your patch! Yet something to improve:

[auto build test ERROR on lee-mfd/for-mfd-next]
[also build test ERROR on abelloni/rtc-next linux/master linus/master v5.15-rc5 next-20211011]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Luca-Ceresoli/Add-MAX77714-PMIC-minimal-driver-RTC-and-watchdog-only/20211012-042754
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: hexagon-randconfig-r012-20211011 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 7ae8f392a1610992c9a925c867fd7238c70d3ce0)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/6fc9e86fa2de7918c36cb31070f4d4eb7d48bff0
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Luca-Ceresoli/Add-MAX77714-PMIC-minimal-driver-RTC-and-watchdog-only/20211012-042754
        git checkout 6fc9e86fa2de7918c36cb31070f4d4eb7d48bff0
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/mfd/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/mfd/max77686.c:90:10: error: use of undeclared identifier 'MAX77802_RTC_DATE'
                   reg == MAX77802_RTC_DATE);
                          ^
   1 error generated.


vim +/MAX77802_RTC_DATE +90 drivers/mfd/max77686.c

a259f3896a39ec Javier Martinez Canillas 2014-07-24  80  
a259f3896a39ec Javier Martinez Canillas 2014-07-24  81  static bool max77802_rtc_is_volatile_reg(struct device *dev, unsigned int reg)
a259f3896a39ec Javier Martinez Canillas 2014-07-24  82  {
a259f3896a39ec Javier Martinez Canillas 2014-07-24  83  	return (max77802_rtc_is_precious_reg(dev, reg) ||
a259f3896a39ec Javier Martinez Canillas 2014-07-24  84  		reg == MAX77802_RTC_SEC ||
a259f3896a39ec Javier Martinez Canillas 2014-07-24  85  		reg == MAX77802_RTC_MIN ||
a259f3896a39ec Javier Martinez Canillas 2014-07-24  86  		reg == MAX77802_RTC_HOUR ||
a259f3896a39ec Javier Martinez Canillas 2014-07-24  87  		reg == MAX77802_RTC_WEEKDAY ||
a259f3896a39ec Javier Martinez Canillas 2014-07-24  88  		reg == MAX77802_RTC_MONTH ||
a259f3896a39ec Javier Martinez Canillas 2014-07-24  89  		reg == MAX77802_RTC_YEAR ||
a259f3896a39ec Javier Martinez Canillas 2014-07-24 @90  		reg == MAX77802_RTC_DATE);
a259f3896a39ec Javier Martinez Canillas 2014-07-24  91  }
a259f3896a39ec Javier Martinez Canillas 2014-07-24  92  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25757 bytes --]

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 6/8] mfd: max77714: Add driver for Maxim MAX77714 PMIC
  2021-10-11 15:56 ` [PATCH 6/8] mfd: max77714: Add driver for " Luca Ceresoli
  2021-10-11 17:15   ` Guenter Roeck
@ 2021-10-12  7:11   ` kernel test robot
  2021-10-12  8:09   ` Krzysztof Kozlowski
  2021-10-12  8:32   ` Krzysztof Kozlowski
  3 siblings, 0 replies; 41+ messages in thread
From: kernel test robot @ 2021-10-12  7:11 UTC (permalink / raw)
  To: Luca Ceresoli, linux-kernel
  Cc: llvm, kbuild-all, Luca Ceresoli, Lee Jones, Rob Herring,
	Alessandro Zummo, Alexandre Belloni, Chanwoo Choi,
	Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz, Wim Van Sebroeck

[-- Attachment #1: Type: text/plain, Size: 2139 bytes --]

Hi Luca,

I love your patch! Perhaps something to improve:

[auto build test WARNING on lee-mfd/for-mfd-next]
[also build test WARNING on abelloni/rtc-next linux/master linus/master v5.15-rc5 next-20211011]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Luca-Ceresoli/Add-MAX77714-PMIC-minimal-driver-RTC-and-watchdog-only/20211012-042754
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: hexagon-randconfig-r035-20211012 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project c3dcf39554dbea780d6cb7e12239451ba47a2668)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/56e2a213fef77c0aac171f8178abf3f20519f0b5
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Luca-Ceresoli/Add-MAX77714-PMIC-minimal-driver-RTC-and-watchdog-only/20211012-042754
        git checkout 56e2a213fef77c0aac171f8178abf3f20519f0b5
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=hexagon 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/mfd/max77714.c:139:34: warning: unused variable 'max77714_dt_match' [-Wunused-const-variable]
   static const struct of_device_id max77714_dt_match[] = {
                                    ^
   1 warning generated.


vim +/max77714_dt_match +139 drivers/mfd/max77714.c

   138	
 > 139	static const struct of_device_id max77714_dt_match[] = {
   140		{ .compatible = "maxim,max77714" },
   141		{},
   142	};
   143	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 16201 bytes --]

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 0/8] Add MAX77714 PMIC minimal driver (RTC and watchdog only)
  2021-10-11 15:56 [PATCH 0/8] Add MAX77714 PMIC minimal driver (RTC and watchdog only) Luca Ceresoli
                   ` (7 preceding siblings ...)
  2021-10-11 20:25 ` [PATCH 8/8] rtc: max77686: add MAX77714 support Luca Ceresoli
@ 2021-10-12  7:59 ` Krzysztof Kozlowski
  8 siblings, 0 replies; 41+ messages in thread
From: Krzysztof Kozlowski @ 2021-10-12  7:59 UTC (permalink / raw)
  To: Luca Ceresoli, linux-kernel
  Cc: Lee Jones, Rob Herring, Alessandro Zummo, Alexandre Belloni,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Wim Van Sebroeck,
	Guenter Roeck, devicetree, linux-rtc, linux-watchdog,
	Chiwoong Byun, Laxman Dewangan

On 11/10/2021 17:56, 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.
> 
> Note! Something seems wrong in the interrupt management code. Due to the
> fact that I'm not using interrupts on my hardware and since this is my
> first addition of an MFD driver, I was unable to understand what is wrong
> after studying the code for other MFD drivers. More details in reply to
> patch 8. Advice would be greatly appreciated on this topic.
> 
> Except for that, all implemented functionality is tested and working: RTC
> read/write, watchdog start/stop/ping/set_timeout.
> 
> The first 4 patches are trivial cleanups to the max77686 drivers and can
> probably be applied easily.
> 
> Patches 5-8 add: dt bindings, mfd driver, watchdog driver and rtc driver.
> 
> Luca
> 
> Luca Ceresoli (8):
>   mfd: max77686: Correct tab-based alignment of register addresses
>   rtc: max77686: convert comments to kernel-doc format
>   rtc: max77686: rename day-of-month defines
>   rtc: max77686: remove useless variable
>   dt-bindings: mfd: add Maxim MAX77714 PMIC
>   mfd: max77714: Add driver for Maxim MAX77714 PMIC
>   watchdog: max77714: add driver for the watchdog in the MAX77714 PMIC
>   rtc: max77686: add MAX77714 support
> 
>  .../bindings/mfd/maxim,max77714.yaml          |  58 ++++++
>  MAINTAINERS                                   |   8 +
>  drivers/mfd/Kconfig                           |  14 ++
>  drivers/mfd/Makefile                          |   1 +
>  drivers/mfd/max77714.c                        | 151 ++++++++++++++++
>  drivers/rtc/Kconfig                           |   2 +-
>  drivers/rtc/rtc-max77686.c                    |  72 +++++---
>  drivers/watchdog/Kconfig                      |   9 +
>  drivers/watchdog/Makefile                     |   1 +
>  drivers/watchdog/max77714_wdt.c               | 171 ++++++++++++++++++
>  include/linux/mfd/max77686-private.h          |  28 +--
>  include/linux/mfd/max77714.h                  |  68 +++++++
>  12 files changed, 541 insertions(+), 42 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mfd/maxim,max77714.yaml
>  create mode 100644 drivers/mfd/max77714.c
>  create mode 100644 drivers/watchdog/max77714_wdt.c
>  create mode 100644 include/linux/mfd/max77714.h
> 

Thanks for the patches.

It's awesome to see extension of existing drivers - max77686 family.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 1/8] mfd: max77686: Correct tab-based alignment of register addresses
  2021-10-11 15:56 ` [PATCH 1/8] mfd: max77686: Correct tab-based alignment of register addresses Luca Ceresoli
@ 2021-10-12  8:00   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 41+ messages in thread
From: Krzysztof Kozlowski @ 2021-10-12  8:00 UTC (permalink / raw)
  To: Luca Ceresoli, linux-kernel
  Cc: Lee Jones, Rob Herring, Alessandro Zummo, Alexandre Belloni,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Wim Van Sebroeck,
	Guenter Roeck, devicetree, linux-rtc, linux-watchdog,
	Chiwoong Byun, Laxman Dewangan

On 11/10/2021 17:56, Luca Ceresoli wrote:
> Some lines have an extra tab, remove them for proper visual alignment as
> present on the rest of this file.
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> ---
>  include/linux/mfd/max77686-private.h | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 2/8] rtc: max77686: convert comments to kernel-doc format
  2021-10-11 15:56 ` [PATCH 2/8] rtc: max77686: convert comments to kernel-doc format Luca Ceresoli
@ 2021-10-12  8:00   ` Krzysztof Kozlowski
  2021-10-15 17:29   ` Alexandre Belloni
  1 sibling, 0 replies; 41+ messages in thread
From: Krzysztof Kozlowski @ 2021-10-12  8:00 UTC (permalink / raw)
  To: Luca Ceresoli, linux-kernel
  Cc: Lee Jones, Rob Herring, Alessandro Zummo, Alexandre Belloni,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Wim Van Sebroeck,
	Guenter Roeck, devicetree, linux-rtc, linux-watchdog,
	Chiwoong Byun, Laxman Dewangan

On 11/10/2021 17:56, Luca Ceresoli wrote:
> Convert the comments documenting this struct to kernel-doc format for
> standardization and readability.
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> ---
>  drivers/rtc/rtc-max77686.c | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
> index eae7cb9faf1e..bac52cdea97d 100644
> --- a/drivers/rtc/rtc-max77686.c
> +++ b/drivers/rtc/rtc-max77686.c
> @@ -61,24 +61,27 @@ enum {
>  	RTC_NR_TIME
>  };
>  


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 4/8] rtc: max77686: remove useless variable
  2021-10-11 15:56 ` [PATCH 4/8] rtc: max77686: remove useless variable Luca Ceresoli
@ 2021-10-12  8:01   ` Krzysztof Kozlowski
  2021-10-15 17:33   ` Alexandre Belloni
  1 sibling, 0 replies; 41+ messages in thread
From: Krzysztof Kozlowski @ 2021-10-12  8:01 UTC (permalink / raw)
  To: Luca Ceresoli, linux-kernel
  Cc: Lee Jones, Rob Herring, Alessandro Zummo, Alexandre Belloni,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Wim Van Sebroeck,
	Guenter Roeck, devicetree, linux-rtc, linux-watchdog,
	Chiwoong Byun, Laxman Dewangan

On 11/10/2021 17:56, Luca Ceresoli wrote:
> rtc_24hr_mode is set to 1 in max77686_rtc_probe()->max77686_rtc_init_reg()
> before being read and is never set back to 0 again. As such, it is de facto
> a constant.
> 
> Remove the variable and the unreachable code.
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> ---
>  drivers/rtc/rtc-max77686.c | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
> 


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 5/8] dt-bindings: mfd: add Maxim MAX77714 PMIC
  2021-10-11 15:56 ` [PATCH 5/8] dt-bindings: mfd: add Maxim MAX77714 PMIC Luca Ceresoli
@ 2021-10-12  8:02   ` Krzysztof Kozlowski
  2021-10-13 21:28     ` Luca Ceresoli
  0 siblings, 1 reply; 41+ messages in thread
From: Krzysztof Kozlowski @ 2021-10-12  8:02 UTC (permalink / raw)
  To: Luca Ceresoli, linux-kernel
  Cc: Lee Jones, Rob Herring, Alessandro Zummo, Alexandre Belloni,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Wim Van Sebroeck,
	Guenter Roeck, devicetree, linux-rtc, linux-watchdog,
	Chiwoong Byun, Laxman Dewangan

On 11/10/2021 17:56, Luca Ceresoli wrote:
> Add bindings for the MAX77714 PMIC with GPIO, RTC and watchdog.
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> ---
>  .../bindings/mfd/maxim,max77714.yaml          | 58 +++++++++++++++++++
>  MAINTAINERS                                   |  5 ++
>  2 files changed, 63 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/maxim,max77714.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/maxim,max77714.yaml b/Documentation/devicetree/bindings/mfd/maxim,max77714.yaml
> new file mode 100644
> index 000000000000..2b0ce3b9bc92
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/maxim,max77714.yaml
> @@ -0,0 +1,58 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/maxim,max77714.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MAX77714 PMIC with GPIO, RTC and watchdog from Maxim Integrated.
> +
> +maintainers:
> +  - Luca Ceresoli <luca@lucaceresoli.net>
> +
> +description: |
> +  MAX77714 is a Power Management IC with 4 buck regulators, 9
> +  low-dropout regulators, 8 GPIOs, RTC and watchdog.
> +
> +properties:
> +  compatible:
> +    const: maxim,max77714
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  interrupt-controller: true
> +
> +  "#interrupt-cells":
> +    const: 2
> +    description:
> +      The first cell is the IRQ number, the second cell is the trigger type.
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - interrupt-controller
> +  - "#interrupt-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        pmic@1c {
> +            compatible = "maxim,max77714";
> +            reg = <0x1c>;
> +            interrupt-parent = <&gpio2>;
> +            interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
> +
> +            interrupt-controller;
> +            #interrupt-cells = <2>;
> +        };
> +    };

Looks good to me, but what about regulators and other properties? Are
you planning to add them later?


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 6/8] mfd: max77714: Add driver for Maxim MAX77714 PMIC
  2021-10-11 15:56 ` [PATCH 6/8] mfd: max77714: Add driver for " Luca Ceresoli
  2021-10-11 17:15   ` Guenter Roeck
  2021-10-12  7:11   ` kernel test robot
@ 2021-10-12  8:09   ` Krzysztof Kozlowski
  2021-10-13 21:49     ` Luca Ceresoli
  2021-10-12  8:32   ` Krzysztof Kozlowski
  3 siblings, 1 reply; 41+ messages in thread
From: Krzysztof Kozlowski @ 2021-10-12  8:09 UTC (permalink / raw)
  To: Luca Ceresoli, linux-kernel
  Cc: Lee Jones, Rob Herring, Alessandro Zummo, Alexandre Belloni,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Wim Van Sebroeck,
	Guenter Roeck, devicetree, linux-rtc, linux-watchdog,
	Chiwoong Byun, Laxman Dewangan

On 11/10/2021 17:56, 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>
> ---
>  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)?

> +	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 2ba6646e874c..fe43f2fdd5cb 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -163,6 +163,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..5d6c88d4d6c0
> --- /dev/null
> +++ b/drivers/mfd/max77714.c
> @@ -0,0 +1,151 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Maxim MAX77714 Watchdog Driver
> + *
> + * Copyright (C) 2021 Luca Ceresoli
> + * Author: Luca Ceresoli <luca@lucaceresoli.net>
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/max77714.h>
> +#include <linux/of.h>
> +#include <linux/regmap.h>
> +
> +static const struct regmap_range max77714_readable_ranges[] = {
> +	regmap_reg_range(MAX77714_INT_TOP,     MAX77714_INT_TOP),
> +	regmap_reg_range(MAX77714_INT_TOPM,    MAX77714_INT_TOPM),
> +	regmap_reg_range(MAX77714_32K_STATUS,  MAX77714_32K_CONFIG),
> +	regmap_reg_range(MAX77714_CNFG_GLBL2,  MAX77714_CNFG2_ONOFF),
> +};
> +
> +static const struct regmap_range max77714_writable_ranges[] = {
> +	regmap_reg_range(MAX77714_INT_TOPM,    MAX77714_INT_TOPM),
> +	regmap_reg_range(MAX77714_32K_CONFIG,  MAX77714_32K_CONFIG),
> +	regmap_reg_range(MAX77714_CNFG_GLBL2,  MAX77714_CNFG2_ONOFF),
> +};
> +
> +static const struct regmap_access_table max77714_readable_table = {
> +	.yes_ranges = max77714_readable_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(max77714_readable_ranges),
> +};
> +
> +static const struct regmap_access_table max77714_writable_table = {
> +	.yes_ranges = max77714_writable_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(max77714_writable_ranges),
> +};
> +
> +static const struct regmap_config max77714_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = MAX77714_CNFG2_ONOFF,
> +	.rd_table = &max77714_readable_table,
> +	.wr_table = &max77714_writable_table,
> +};
> +
> +static const struct regmap_irq max77714_top_irqs[] = {
> +	REGMAP_IRQ_REG(MAX77714_IRQ_TOP_ONOFF,   0, MAX77714_INT_TOP_ONOFF),
> +	REGMAP_IRQ_REG(MAX77714_IRQ_TOP_RTC,     0, MAX77714_INT_TOP_RTC),
> +	REGMAP_IRQ_REG(MAX77714_IRQ_TOP_GPIO,    0, MAX77714_INT_TOP_GPIO),
> +	REGMAP_IRQ_REG(MAX77714_IRQ_TOP_LDO,     0, MAX77714_INT_TOP_LDO),
> +	REGMAP_IRQ_REG(MAX77714_IRQ_TOP_SD,      0, MAX77714_INT_TOP_SD),
> +	REGMAP_IRQ_REG(MAX77714_IRQ_TOP_GLBL,    0, MAX77714_INT_TOP_GLBL),
> +};
> +
> +static const struct regmap_irq_chip max77714_irq_chip = {
> +	.name			= "max77714-pmic",
> +	.status_base		= MAX77714_INT_TOP,
> +	.mask_base		= MAX77714_INT_TOPM,
> +	.num_regs		= 1,
> +	.irqs			= max77714_top_irqs,
> +	.num_irqs		= ARRAY_SIZE(max77714_top_irqs),
> +};
> +
> +static const struct mfd_cell max77714_cells[] = {
> +	{ .name = "max77714-watchdog" },
> +	{ .name = "max77714-rtc" },
> +};
> +
> +/*
> + * MAX77714 initially uses the internal, low precision oscillator. Enable
> + * the external oscillator by setting the XOSC_RETRY bit. If the external
> + * oscillator is not OK (probably not installed) this has no effect.
> + */
> +static int max77714_setup_xosc(struct max77714 *chip)
> +{
> +	/* Internal Crystal Load Capacitance, indexed by value of 32KLOAD bits */
> +	static const unsigned int load_cap[4] = {0, 10, 12, 22};
> +	unsigned int load_cap_idx;
> +	unsigned int status;
> +	int err;
> +
> +	err = regmap_update_bits(chip->regmap, MAX77714_32K_CONFIG,
> +				 MAX77714_32K_CONFIG_XOSC_RETRY,
> +				 MAX77714_32K_CONFIG_XOSC_RETRY);
> +	if (err)
> +		return dev_err_probe(chip->dev, err, "cannot configure XOSC\n");
> +
> +	err = regmap_read(chip->regmap, MAX77714_32K_STATUS, &status);
> +	if (err)
> +		return dev_err_probe(chip->dev, err, "cannot read XOSC status\n");
> +
> +	load_cap_idx = (status >> MAX77714_32K_STATUS_32KLOAD_SHF)
> +		& MAX77714_32K_STATUS_32KLOAD_MSK;
> +
> +	dev_info(chip->dev, "Using %s oscillator, %d pF load cap\n",
> +		 status & MAX77714_32K_STATUS_32KSOURCE ? "internal" : "external",
> +		 load_cap[load_cap_idx]);
> +
> +	return 0;
> +}
> +
> +static int max77714_probe(struct i2c_client *client)
> +{
> +	struct max77714 *chip;
> +	int err;
> +
> +	chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
> +	if (!chip)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(client, chip);
> +	chip->dev = &client->dev;
> +
> +	chip->regmap = devm_regmap_init_i2c(client, &max77714_regmap_config);
> +	if (IS_ERR(chip->regmap))
> +		return dev_err_probe(chip->dev, PTR_ERR(chip->regmap),
> +				     "failed to initialise regmap\n");
> +
> +	err = max77714_setup_xosc(chip);
> +	if (err)
> +		return err;
> +
> +	err = devm_regmap_add_irq_chip(chip->dev, chip->regmap, client->irq,
> +				       IRQF_ONESHOT | IRQF_SHARED, 0,
> +				       &max77714_irq_chip, &chip->irq_data);
> +	if (err)
> +		return dev_err_probe(chip->dev, err, "failed to add PMIC irq chip\n");
> +
> +	err =  devm_mfd_add_devices(chip->dev, PLATFORM_DEVID_NONE,
> +				    max77714_cells, ARRAY_SIZE(max77714_cells),
> +				    NULL, 0, NULL);
> +	if (err)
> +		return dev_err_probe(chip->dev, err, "failed adding MFD children\n");
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id max77714_dt_match[] = {
> +	{ .compatible = "maxim,max77714" },
> +	{},
> +};
> +
> +static struct i2c_driver max77714_driver = {
> +	.driver = {
> +		.name = "max77714",
> +		.of_match_table = of_match_ptr(max77714_dt_match),
> +	},
> +	.probe_new = max77714_probe,
> +};
> +builtin_i2c_driver(max77714_driver);

Try to make it a module, so: module_i2c_driver

> 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_

> +
> +#include <linux/bits.h>
> +
> +#define MAX77714_INT_TOP	0x00
> +#define MAX77714_INT_TOPM	0x07 /* Datasheet says "read only", but it is RW */
> +
> +#define MAX77714_INT_TOP_ONOFF		BIT(1)
> +#define MAX77714_INT_TOP_RTC		BIT(3)
> +#define MAX77714_INT_TOP_GPIO		BIT(4)
> +#define MAX77714_INT_TOP_LDO		BIT(5)
> +#define MAX77714_INT_TOP_SD		BIT(6)
> +#define MAX77714_INT_TOP_GLBL		BIT(7)
> +
> +#define MAX77714_32K_STATUS	0x30
> +#define MAX77714_32K_STATUS_SIOSCOK	BIT(5)
> +#define MAX77714_32K_STATUS_XOSCOK	BIT(4)
> +#define MAX77714_32K_STATUS_32KSOURCE	BIT(3)
> +#define MAX77714_32K_STATUS_32KLOAD_MSK	0x3
> +#define MAX77714_32K_STATUS_32KLOAD_SHF	1
> +#define MAX77714_32K_STATUS_CRYSTAL_CFG	BIT(0)
> +
> +#define MAX77714_32K_CONFIG	0x31
> +#define MAX77714_32K_CONFIG_XOSC_RETRY	BIT(4)
> +
> +#define MAX77714_CNFG_GLBL2	0x91
> +#define MAX77714_WDTEN			BIT(2)
> +#define MAX77714_WDTSLPC		BIT(3)
> +#define MAX77714_TWD_MASK		0x3
> +#define MAX77714_TWD_2s			0x0
> +#define MAX77714_TWD_16s		0x1
> +#define MAX77714_TWD_64s		0x2
> +#define MAX77714_TWD_128s		0x3
> +
> +#define MAX77714_CNFG_GLBL3	0x92
> +#define MAX77714_WDTC			BIT(0)
> +
> +#define MAX77714_CNFG2_ONOFF	0x94
> +#define MAX77714_WD_RST_WK		BIT(5)
> +
> +/* Interrupts */
> +enum {
> +	MAX77714_IRQ_TOP_ONOFF,
> +	MAX77714_IRQ_TOP_RTC,		/* Real-time clock */
> +	MAX77714_IRQ_TOP_GPIO,		/* GPIOs */
> +	MAX77714_IRQ_TOP_LDO,		/* Low-dropout regulators */
> +	MAX77714_IRQ_TOP_SD,		/* Step-down regulators */
> +	MAX77714_IRQ_TOP_GLBL,		/* "Global resources": Low-Battery, overtemp... */
> +};
> +
> +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



> +
> +#endif /* _MFD_MAX77714_H_ */
> 


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 3/8] rtc: max77686: rename day-of-month defines
  2021-10-11 15:56 ` [PATCH 3/8] rtc: max77686: rename day-of-month defines Luca Ceresoli
  2021-10-12  2:23   ` kernel test robot
  2021-10-12  2:58   ` kernel test robot
@ 2021-10-12  8:13   ` Krzysztof Kozlowski
  2 siblings, 0 replies; 41+ messages in thread
From: Krzysztof Kozlowski @ 2021-10-12  8:13 UTC (permalink / raw)
  To: Luca Ceresoli, linux-kernel
  Cc: Lee Jones, Rob Herring, Alessandro Zummo, Alexandre Belloni,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Wim Van Sebroeck,
	Guenter Roeck, devicetree, linux-rtc, linux-watchdog,
	Chiwoong Byun, Laxman Dewangan

On 11/10/2021 17:56, Luca Ceresoli wrote:
> RTC_DATE and REG_RTC_DATE are used for the registers holding the day of
> month. Rename these constants to mean what they mean.
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> ---
>  drivers/rtc/rtc-max77686.c           | 16 ++++++++--------
>  include/linux/mfd/max77686-private.h |  4 ++--
>  2 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
> index bac52cdea97d..7e765207f28e 100644
> --- a/drivers/rtc/rtc-max77686.c
> +++ b/drivers/rtc/rtc-max77686.c
> @@ -57,7 +57,7 @@ enum {
>  	RTC_WEEKDAY,
>  	RTC_MONTH,
>  	RTC_YEAR,
> -	RTC_DATE,
> +	RTC_MONTHDAY,
>  	RTC_NR_TIME
>  };



Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 8/8] rtc: max77686: add MAX77714 support
  2021-10-11 16:12   ` Luca Ceresoli
@ 2021-10-12  8:20     ` Krzysztof Kozlowski
  2021-10-15 16:46       ` Luca Ceresoli
  0 siblings, 1 reply; 41+ messages in thread
From: Krzysztof Kozlowski @ 2021-10-12  8:20 UTC (permalink / raw)
  To: Luca Ceresoli, linux-kernel
  Cc: Lee Jones, Rob Herring, Alessandro Zummo, Alexandre Belloni,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Wim Van Sebroeck,
	Guenter Roeck, devicetree, linux-rtc, linux-watchdog,
	Chiwoong Byun, Laxman Dewangan

On 11/10/2021 18:12, Luca Ceresoli wrote:
> Hi,
> 
> see below for the issues with interrupt implementation that I mentioned
> in the cover letter.
> 
> On 11/10/21 17:56, Luca Ceresoli wrote:
>> The RTC included in the MAX77714 PMIC is very similar to the one in the
>> MAX77686. Reuse the rtc-max77686.c driver with the minimum required changes
>> for the MAX77714 RTC.
>>
>> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
>> ---
>>  drivers/rtc/Kconfig        |  2 +-
>>  drivers/rtc/rtc-max77686.c | 24 ++++++++++++++++++++++++
>>  2 files changed, 25 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
>> index e1bc5214494e..a73591ad292b 100644
>> --- a/drivers/rtc/Kconfig
>> +++ b/drivers/rtc/Kconfig
>> @@ -375,7 +375,7 @@ config RTC_DRV_MAX8997
>>  
>>  config RTC_DRV_MAX77686
>>  	tristate "Maxim MAX77686"
>> -	depends on MFD_MAX77686 || MFD_MAX77620 || COMPILE_TEST
>> +	depends on MFD_MAX77686 || MFD_MAX77620 || MFD_MAX77714 || COMPILE_TEST
>>  	help
>>  	  If you say yes here you will get support for the
>>  	  RTC of Maxim MAX77686/MAX77620/MAX77802 PMIC.
>> diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
>> index 9901c596998a..e6564bc2171e 100644
>> --- a/drivers/rtc/rtc-max77686.c
>> +++ b/drivers/rtc/rtc-max77686.c
>> @@ -19,6 +19,7 @@
>>  
>>  #define MAX77686_I2C_ADDR_RTC		(0x0C >> 1)
>>  #define MAX77620_I2C_ADDR_RTC		0x68
>> +#define MAX77714_I2C_ADDR_RTC		0x48
>>  #define MAX77686_INVALID_I2C_ADDR	(-1)
>>  
>>  /* Define non existing register */
>> @@ -203,6 +204,28 @@ static const struct max77686_rtc_driver_data max77686_drv_data = {
>>  	.regmap_config = &max77686_rtc_regmap_config,
>>  };
>>  
>> +static const struct regmap_irq_chip max77714_rtc_irq_chip = {
>> +	.name		= "max77714-rtc",
>> +	.status_base	= MAX77686_RTC_INT,
>> +	.mask_base	= MAX77686_RTC_INTM,
>> +	.num_regs	= 1,
>> +	.irqs		= max77686_rtc_irqs,
>> +	.num_irqs	= ARRAY_SIZE(max77686_rtc_irqs) - 1, /* no WTSR on 77714 */
>> +};
>> +
>> +static const struct max77686_rtc_driver_data max77714_drv_data = {
>> +	.delay = 16000,
>> +	.mask  = 0x7f,
>> +	.map   = max77686_map,
>> +	.alarm_enable_reg = false,
>> +	.rtc_irq_from_platform = false,
> 
> As far as I could understand, rtc_irq_from_platform should be 'true'.
> This would trigger the 'if' branch in function
> max77686_init_rtc_regmap() [0]:
> 
>   if (info->drv_data->rtc_irq_from_platform) {
> 	struct platform_device *pdev = to_platform_device(info->dev);
> 
> 	info->rtc_irq = platform_get_irq(pdev, 0);
> 	if (info->rtc_irq < 0)
> 		return info->rtc_irq;
>   } else {
> 	info->rtc_irq =  parent_i2c->irq;
>   }
> 
> Calling platform_get_irq() seems correct for the MAX77714, which can
> generate various IRQ events, collecting them in a register, and raise a
> single IRQ to the CPU via a physical pin.
> 
> However, if I set rtc_irq_from_platform = true, platform_get_irq()
> returns IRQ number '1', which ends up in:
> 
>   dummy 0-0048: Failed to request IRQ 1 for max77714-rtc: -22
>   max77686-rtc max77714-rtc: Failed to add RTC irq chip: -22
>   max77686-rtc: probe of max77714-rtc failed with error -22
> 
> I compared my code with other MFD drivers and their cell drivers (but
> their datasheets is not available so I had to add some guesswork), and
> couldn't find out where my code is wrong.
> 
> Unfortunately I have no IRQ access on my board (and I don't need them
> for my use case). For this reason I initially thought of disabling all
> the IRQ code in rtc-max77686.c via a new flag, but it would be quite
> invasive and I wouldn't even be able to test that existing hardware
> still works. Implementing a new RTC driver for the MAX77714 does not
> seem to be a sane option as the hardware is really 99% equal to the
> MAX77686 RTC.
> 

I think the flag should be false, not true. The true means you have RTC
device with its own interrupt. For example in DT it could look like:

  pmic@1c {
      compatible = "maxim,max77714";
      reg = <0x1c>;
      interrupt-parent = <&gpio2>;
      interrupts = <3 IRQ_TYPE_LEVEL_LOW>;

      interrupt-controller;
      #interrupt-cells = <2>;
   };

   rtc@48 {
      compatible = "maxim,max77714-rtc";
      reg = <0x48>;
      interrupt-parent = <&gpio2>;
      interrupts = <4 IRQ_TYPE_LEVEL_LOW>;
   };

In your case, the RTC device will not have its own devicetree node and
will be instantiated as MFD child device. The only interrupt line
available is the parents interrupt line - the same as in max77686 and
max77802 setups.

Have in mind that this does not necessarily reflect real HW, but how we
represent it in devicetree and driver model.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 8/8] rtc: max77686: add MAX77714 support
  2021-10-11 20:25 ` [PATCH 8/8] rtc: max77686: add MAX77714 support Luca Ceresoli
  2021-10-11 16:12   ` Luca Ceresoli
@ 2021-10-12  8:29   ` Krzysztof Kozlowski
  2021-10-15 17:36   ` Alexandre Belloni
  2 siblings, 0 replies; 41+ messages in thread
From: Krzysztof Kozlowski @ 2021-10-12  8:29 UTC (permalink / raw)
  To: Luca Ceresoli, linux-kernel
  Cc: Lee Jones, Rob Herring, Alessandro Zummo, Alexandre Belloni,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Wim Van Sebroeck,
	Guenter Roeck, devicetree, linux-rtc, linux-watchdog,
	Chiwoong Byun, Laxman Dewangan

On 11/10/2021 22:25, Luca Ceresoli wrote:
> The RTC included in the MAX77714 PMIC is very similar to the one in the
> MAX77686. Reuse the rtc-max77686.c driver with the minimum required changes
> for the MAX77714 RTC.
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> ---
> 
> *** NOTE ***
> 
> This patch didn't reach most recipients having hit a limit in my service
> provider (125 e-mails per hour). I'm resending it, as far as possible with
> proper message-id etc. Apologies for any duplicate.
> 
>  drivers/rtc/Kconfig        |  2 +-
>  drivers/rtc/rtc-max77686.c | 24 ++++++++++++++++++++++++
>  2 files changed, 25 insertions(+), 1 deletion(-)
> 


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 6/8] mfd: max77714: Add driver for Maxim MAX77714 PMIC
  2021-10-11 15:56 ` [PATCH 6/8] mfd: max77714: Add driver for " Luca Ceresoli
                     ` (2 preceding siblings ...)
  2021-10-12  8:09   ` Krzysztof Kozlowski
@ 2021-10-12  8:32   ` Krzysztof Kozlowski
  2021-10-13 21:39     ` Luca Ceresoli
  3 siblings, 1 reply; 41+ messages in thread
From: Krzysztof Kozlowski @ 2021-10-12  8:32 UTC (permalink / raw)
  To: Luca Ceresoli, linux-kernel
  Cc: Lee Jones, Rob Herring, Alessandro Zummo, Alexandre Belloni,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Wim Van Sebroeck,
	Guenter Roeck, devicetree, linux-rtc, linux-watchdog,
	Chiwoong Byun, Laxman Dewangan

On 11/10/2021 17:56, 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>
> ---
>  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
> 

(...)

> +
> +static const struct of_device_id max77714_dt_match[] = {
> +	{ .compatible = "maxim,max77714" },
> +	{},
> +};

When converting to module - don't forget the MODULE_DEVICE_TABLE

> +
> +static struct i2c_driver max77714_driver = {
> +	.driver = {
> +		.name = "max77714",
> +		.of_match_table = of_match_ptr(max77714_dt_match),

Kbuild robot pointed it out - of_matc_ptr should not be needed, even for
compile testing without OF.

> +	},
> +	.probe_new = max77714_probe,
> +};

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 5/8] dt-bindings: mfd: add Maxim MAX77714 PMIC
  2021-10-12  8:02   ` Krzysztof Kozlowski
@ 2021-10-13 21:28     ` Luca Ceresoli
  2021-10-14  7:38       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 41+ messages in thread
From: Luca Ceresoli @ 2021-10-13 21:28 UTC (permalink / raw)
  To: Krzysztof Kozlowski, linux-kernel
  Cc: Lee Jones, Rob Herring, Alessandro Zummo, Alexandre Belloni,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Wim Van Sebroeck,
	Guenter Roeck, devicetree, linux-rtc, linux-watchdog,
	Chiwoong Byun, Laxman Dewangan

Hi Krzysztof,

thanks for reviewing.

On 12/10/21 10:02, Krzysztof Kozlowski wrote:
> On 11/10/2021 17:56, Luca Ceresoli wrote:
>> Add bindings for the MAX77714 PMIC with GPIO, RTC and watchdog.
>>
>> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
>> ---
>>  .../bindings/mfd/maxim,max77714.yaml          | 58 +++++++++++++++++++
>>  MAINTAINERS                                   |  5 ++
>>  2 files changed, 63 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mfd/maxim,max77714.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/maxim,max77714.yaml b/Documentation/devicetree/bindings/mfd/maxim,max77714.yaml
>> new file mode 100644
>> index 000000000000..2b0ce3b9bc92
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/maxim,max77714.yaml
>> @@ -0,0 +1,58 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/mfd/maxim,max77714.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: MAX77714 PMIC with GPIO, RTC and watchdog from Maxim Integrated.
>> +
>> +maintainers:
>> +  - Luca Ceresoli <luca@lucaceresoli.net>
>> +
>> +description: |
>> +  MAX77714 is a Power Management IC with 4 buck regulators, 9
>> +  low-dropout regulators, 8 GPIOs, RTC and watchdog.
>> +
>> +properties:
>> +  compatible:
>> +    const: maxim,max77714
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  interrupt-controller: true
>> +
>> +  "#interrupt-cells":
>> +    const: 2
>> +    description:
>> +      The first cell is the IRQ number, the second cell is the trigger type.
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +  - interrupt-controller
>> +  - "#interrupt-cells"
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +    i2c {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +
>> +        pmic@1c {
>> +            compatible = "maxim,max77714";
>> +            reg = <0x1c>;
>> +            interrupt-parent = <&gpio2>;
>> +            interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
>> +
>> +            interrupt-controller;
>> +            #interrupt-cells = <2>;
>> +        };
>> +    };
> 
> Looks good to me, but what about regulators and other properties? Are
> you planning to add them later?

No plan to add them, sorry.

I know, complete bindings are better than incomplete bindings. But in
the foreseeable future I don't need to do anything on the regulators
(even though it might happen at some point). And since their setting is
possibly non trivial, I'm not going to study them to write a complete
bindings document and then make no use of it.

Is it a problem for you?

-- 
Luca

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 6/8] mfd: max77714: Add driver for Maxim MAX77714 PMIC
  2021-10-12  8:32   ` Krzysztof Kozlowski
@ 2021-10-13 21:39     ` Luca Ceresoli
  2021-10-14  7:40       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 41+ messages in thread
From: Luca Ceresoli @ 2021-10-13 21:39 UTC (permalink / raw)
  To: Krzysztof Kozlowski, linux-kernel
  Cc: Lee Jones, Rob Herring, Alessandro Zummo, Alexandre Belloni,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Wim Van Sebroeck,
	Guenter Roeck, devicetree, linux-rtc, linux-watchdog,
	Chiwoong Byun, Laxman Dewangan

Hi,

On 12/10/21 10:32, Krzysztof Kozlowski wrote:
> On 11/10/2021 17:56, 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>
>> ---
>>  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
>>
> 
> (...)
> 
>> +
>> +static const struct of_device_id max77714_dt_match[] = {
>> +	{ .compatible = "maxim,max77714" },
>> +	{},
>> +};
> 
> When converting to module - don't forget the MODULE_DEVICE_TABLE
> 
>> +
>> +static struct i2c_driver max77714_driver = {
>> +	.driver = {
>> +		.name = "max77714",
>> +		.of_match_table = of_match_ptr(max77714_dt_match),
> 
> Kbuild robot pointed it out - of_matc_ptr should not be needed, even for
> compile testing without OF.

I wonder whether it's better to add '#ifdef CONFIG_OF / #endif' around
the struct of_device_id declaration. I think it's what most drivers do,
even though I tend to prefer not adding #ifdefs making code less clean
only for COMPILE_TESTING.

-- 
Luca

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 6/8] mfd: max77714: Add driver for Maxim MAX77714 PMIC
  2021-10-12  8:09   ` Krzysztof Kozlowski
@ 2021-10-13 21:49     ` Luca Ceresoli
  2021-10-14  7:44       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 41+ messages in thread
From: Luca Ceresoli @ 2021-10-13 21:49 UTC (permalink / raw)
  To: Krzysztof Kozlowski, linux-kernel
  Cc: Lee Jones, Rob Herring, Alessandro Zummo, Alexandre Belloni,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Wim Van Sebroeck,
	Guenter Roeck, devicetree, linux-rtc, linux-watchdog,
	Chiwoong Byun, Laxman Dewangan

Hi,

On 12/10/21 10:09, Krzysztof Kozlowski wrote:
> On 11/10/2021 17:56, 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>
>> ---
>>  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.

>> 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.

>> +
>> +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.

-- 
Luca

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 5/8] dt-bindings: mfd: add Maxim MAX77714 PMIC
  2021-10-13 21:28     ` Luca Ceresoli
@ 2021-10-14  7:38       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 41+ messages in thread
From: Krzysztof Kozlowski @ 2021-10-14  7:38 UTC (permalink / raw)
  To: Luca Ceresoli, linux-kernel
  Cc: Lee Jones, Rob Herring, Alessandro Zummo, Alexandre Belloni,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Wim Van Sebroeck,
	Guenter Roeck, devicetree, linux-rtc, linux-watchdog,
	Chiwoong Byun, Laxman Dewangan

On 13/10/2021 23:28, Luca Ceresoli wrote:
> Hi Krzysztof,
> 
> thanks for reviewing.
> 
> On 12/10/21 10:02, Krzysztof Kozlowski wrote:
>> On 11/10/2021 17:56, Luca Ceresoli wrote:
>>> Add bindings for the MAX77714 PMIC with GPIO, RTC and watchdog.
>>>
>>> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
>>> ---
>>>  .../bindings/mfd/maxim,max77714.yaml          | 58 +++++++++++++++++++
>>>  MAINTAINERS                                   |  5 ++
>>>  2 files changed, 63 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/mfd/maxim,max77714.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/mfd/maxim,max77714.yaml b/Documentation/devicetree/bindings/mfd/maxim,max77714.yaml
>>> new file mode 100644
>>> index 000000000000..2b0ce3b9bc92
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mfd/maxim,max77714.yaml
>>> @@ -0,0 +1,58 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/mfd/maxim,max77714.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: MAX77714 PMIC with GPIO, RTC and watchdog from Maxim Integrated.
>>> +
>>> +maintainers:
>>> +  - Luca Ceresoli <luca@lucaceresoli.net>
>>> +
>>> +description: |
>>> +  MAX77714 is a Power Management IC with 4 buck regulators, 9
>>> +  low-dropout regulators, 8 GPIOs, RTC and watchdog.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: maxim,max77714
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  interrupts:
>>> +    maxItems: 1
>>> +
>>> +  interrupt-controller: true
>>> +
>>> +  "#interrupt-cells":
>>> +    const: 2
>>> +    description:
>>> +      The first cell is the IRQ number, the second cell is the trigger type.
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +  - interrupts
>>> +  - interrupt-controller
>>> +  - "#interrupt-cells"
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/interrupt-controller/irq.h>
>>> +    i2c {
>>> +        #address-cells = <1>;
>>> +        #size-cells = <0>;
>>> +
>>> +        pmic@1c {
>>> +            compatible = "maxim,max77714";
>>> +            reg = <0x1c>;
>>> +            interrupt-parent = <&gpio2>;
>>> +            interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
>>> +
>>> +            interrupt-controller;
>>> +            #interrupt-cells = <2>;
>>> +        };
>>> +    };
>>
>> Looks good to me, but what about regulators and other properties? Are
>> you planning to add them later?
> 
> No plan to add them, sorry.
> 
> I know, complete bindings are better than incomplete bindings. But in
> the foreseeable future I don't need to do anything on the regulators
> (even though it might happen at some point). And since their setting is
> possibly non trivial, I'm not going to study them to write a complete
> bindings document and then make no use of it.
> 
> Is it a problem for you?

It's OK.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 6/8] mfd: max77714: Add driver for Maxim MAX77714 PMIC
  2021-10-13 21:39     ` Luca Ceresoli
@ 2021-10-14  7:40       ` Krzysztof Kozlowski
  2021-10-14  8:25         ` Luca Ceresoli
  0 siblings, 1 reply; 41+ messages in thread
From: Krzysztof Kozlowski @ 2021-10-14  7:40 UTC (permalink / raw)
  To: Luca Ceresoli, linux-kernel
  Cc: Lee Jones, Rob Herring, Alessandro Zummo, Alexandre Belloni,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Wim Van Sebroeck,
	Guenter Roeck, devicetree, linux-rtc, linux-watchdog,
	Chiwoong Byun, Laxman Dewangan

On 13/10/2021 23:39, Luca Ceresoli wrote:
> Hi,
> 
> On 12/10/21 10:32, Krzysztof Kozlowski wrote:
>> On 11/10/2021 17:56, 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>
>>> ---
>>>  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
>>>
>>
>> (...)
>>
>>> +
>>> +static const struct of_device_id max77714_dt_match[] = {
>>> +	{ .compatible = "maxim,max77714" },
>>> +	{},
>>> +};
>>
>> When converting to module - don't forget the MODULE_DEVICE_TABLE
>>
>>> +
>>> +static struct i2c_driver max77714_driver = {
>>> +	.driver = {
>>> +		.name = "max77714",
>>> +		.of_match_table = of_match_ptr(max77714_dt_match),
>>
>> Kbuild robot pointed it out - of_matc_ptr should not be needed, even for
>> compile testing without OF.
> 
> I wonder whether it's better to add '#ifdef CONFIG_OF / #endif' around
> the struct of_device_id declaration. I think it's what most drivers do,
> even though I tend to prefer not adding #ifdefs making code less clean
> only for COMPILE_TESTING.

No, most drivers added it long time ago before we switched it to a new
way - either __maybe_unused or without anything even. The point is that
OF driver can be reused for ACPI platforms. If you limit it with ifdef
or of_match_ptr, the ACPI platform won't have any table to use for binding.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 6/8] mfd: max77714: Add driver for Maxim MAX77714 PMIC
  2021-10-13 21:49     ` Luca Ceresoli
@ 2021-10-14  7:44       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 41+ messages in thread
From: Krzysztof Kozlowski @ 2021-10-14  7:44 UTC (permalink / raw)
  To: Luca Ceresoli, linux-kernel
  Cc: Lee Jones, Rob Herring, Alessandro Zummo, Alexandre Belloni,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Wim Van Sebroeck,
	Guenter Roeck, devicetree, linux-rtc, linux-watchdog,
	Chiwoong Byun, Laxman Dewangan

On 13/10/2021 23:49, Luca Ceresoli wrote:
> Hi,
> 
> On 12/10/21 10:09, Krzysztof Kozlowski wrote:
>> On 11/10/2021 17:56, 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>
>>> ---
>>>  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.

> 
>>> 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.
> 
>>> +
>>> +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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 6/8] mfd: max77714: Add driver for Maxim MAX77714 PMIC
  2021-10-14  7:40       ` Krzysztof Kozlowski
@ 2021-10-14  8:25         ` Luca Ceresoli
  0 siblings, 0 replies; 41+ messages in thread
From: Luca Ceresoli @ 2021-10-14  8:25 UTC (permalink / raw)
  To: Krzysztof Kozlowski, linux-kernel
  Cc: Lee Jones, Rob Herring, Alessandro Zummo, Alexandre Belloni,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Wim Van Sebroeck,
	Guenter Roeck, devicetree, linux-rtc, linux-watchdog,
	Chiwoong Byun, Laxman Dewangan

Hi,

On 14/10/21 09:40, Krzysztof Kozlowski wrote:
[...]
>>>> +
>>>> +static const struct of_device_id max77714_dt_match[] = {
>>>> +	{ .compatible = "maxim,max77714" },
>>>> +	{},
>>>> +};
>>>
>>> When converting to module - don't forget the MODULE_DEVICE_TABLE
>>>
>>>> +
>>>> +static struct i2c_driver max77714_driver = {
>>>> +	.driver = {
>>>> +		.name = "max77714",
>>>> +		.of_match_table = of_match_ptr(max77714_dt_match),
>>>
>>> Kbuild robot pointed it out - of_matc_ptr should not be needed, even for
>>> compile testing without OF.
>>
>> I wonder whether it's better to add '#ifdef CONFIG_OF / #endif' around
>> the struct of_device_id declaration. I think it's what most drivers do,
>> even though I tend to prefer not adding #ifdefs making code less clean
>> only for COMPILE_TESTING.
> 
> No, most drivers added it long time ago before we switched it to a new
> way - either __maybe_unused or without anything even. The point is that
> OF driver can be reused for ACPI platforms. If you limit it with ifdef
> or of_match_ptr, the ACPI platform won't have any table to use for binding.

Oh, I see, thanks for the clarification.

I wonder if it makes sense to mass-remove all of them and remove the macro.

-- 
Luca

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 7/8] watchdog: max77714: add driver for the watchdog in the MAX77714 PMIC
  2021-10-12  1:18   ` Randy Dunlap
@ 2021-10-15 16:42     ` Luca Ceresoli
  2021-10-15 17:07       ` Randy Dunlap
  0 siblings, 1 reply; 41+ messages in thread
From: Luca Ceresoli @ 2021-10-15 16:42 UTC (permalink / raw)
  To: Randy Dunlap, linux-kernel
  Cc: Lee Jones, Rob Herring, Alessandro Zummo, Alexandre Belloni,
	Chanwoo Choi, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz,
	Wim Van Sebroeck, Guenter Roeck, devicetree, linux-rtc,
	linux-watchdog, Chiwoong Byun, Laxman Dewangan

Hi,

On 12/10/21 03:18, Randy Dunlap wrote:
> Hi,
> 
> On 10/11/21 8:56 AM, Luca Ceresoli wrote:
>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>> index bf59faeb3de1..00bc3f932a6c 100644
>> --- a/drivers/watchdog/Kconfig
>> +++ b/drivers/watchdog/Kconfig
>> @@ -699,6 +699,15 @@ config MAX77620_WATCHDOG
>>        MAX77620 chips. To compile this driver as a module,
>>        choose M here: the module will be called max77620_wdt.
>>   +config MAX77714_WATCHDOG
>> +    tristate "Maxim MAX77714 Watchdog Timer"
>> +    depends on MFD_MAX77714 || COMPILE_TEST
>> +    help
>> +     This is the driver for watchdog timer in the MAX77714 PMIC.
>> +     Say 'Y' here to enable the watchdog timer support for
>> +     MAX77714 chips. To compile this driver as a module,
>> +     choose M here: the module will be called max77714_wdt.
> 
> Please follow coding-style for Kconfig files:
> 
> (from Documentation/process/coding-style.rst, section 10):
> 
> For all of the Kconfig* configuration files throughout the source tree,
> the indentation is somewhat different.  Lines under a ``config`` definition
> are indented with one tab, while help text is indented an additional two
> spaces.

Oh dear, I usually don't make such silly mistakes, apologies.

[...some fast typing later...]

Uhm, now I noticed many entries in that file have that same mistake.
Perhaps I copy-pasted and didn't check. I'll send a patch to fix them too.

-- 
Luca


^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 7/8] watchdog: max77714: add driver for the watchdog in the MAX77714 PMIC
  2021-10-11 17:17   ` Guenter Roeck
@ 2021-10-15 16:43     ` Luca Ceresoli
  0 siblings, 0 replies; 41+ messages in thread
From: Luca Ceresoli @ 2021-10-15 16:43 UTC (permalink / raw)
  To: Guenter Roeck, linux-kernel
  Cc: Lee Jones, Rob Herring, Alessandro Zummo, Alexandre Belloni,
	Chanwoo Choi, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz,
	Wim Van Sebroeck, devicetree, linux-rtc, linux-watchdog,
	Chiwoong Byun, Laxman Dewangan

Hi,

On 11/10/21 19:17, Guenter Roeck wrote:
> On 10/11/21 8:56 AM, Luca Ceresoli wrote:
[...]
>> diff --git a/drivers/watchdog/max77714_wdt.c
>> b/drivers/watchdog/max77714_wdt.c
>> new file mode 100644
>> index 000000000000..2d468db849f9
>> --- /dev/null
>> +++ b/drivers/watchdog/max77714_wdt.c
>> @@ -0,0 +1,171 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Maxim MAX77714 Watchdog Driver
>> + *
>> + * Copyright (C) 2021 Luca Ceresoli
>> + * Author: Luca Ceresoli <luca@lucaceresoli.net>
>> + */
>> +
>> +#include <linux/err.h>
>> +#include <linux/kernel.h>
>> +#include <linux/mfd/max77714.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/watchdog.h>
>> +
>> +struct max77714_wdt {
>> +    struct device        *dev;
>> +    struct regmap        *rmap;
>> +    struct watchdog_device    wd_dev;
>> +};
>> +
>> +/* Timeout in seconds, indexed by TWD bits of CNFG_GLBL2 register */
>> +unsigned int max77714_margin_value[] = { 2, 16, 64, 128 };
> 
> static

Absolutely. And const too. Will fix.

-- 
Luca


^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 8/8] rtc: max77686: add MAX77714 support
  2021-10-12  8:20     ` Krzysztof Kozlowski
@ 2021-10-15 16:46       ` Luca Ceresoli
  0 siblings, 0 replies; 41+ messages in thread
From: Luca Ceresoli @ 2021-10-15 16:46 UTC (permalink / raw)
  To: Krzysztof Kozlowski, linux-kernel
  Cc: Lee Jones, Rob Herring, Alessandro Zummo, Alexandre Belloni,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Wim Van Sebroeck,
	Guenter Roeck, devicetree, linux-rtc, linux-watchdog,
	Chiwoong Byun, Laxman Dewangan

Hi,

On 12/10/21 10:20, Krzysztof Kozlowski wrote:
> On 11/10/2021 18:12, Luca Ceresoli wrote:
>> Hi,
>>
>> see below for the issues with interrupt implementation that I mentioned
>> in the cover letter.
>>
>> On 11/10/21 17:56, Luca Ceresoli wrote:
>>> The RTC included in the MAX77714 PMIC is very similar to the one in the
>>> MAX77686. Reuse the rtc-max77686.c driver with the minimum required changes
>>> for the MAX77714 RTC.
>>>
>>> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
>>> ---
>>>  drivers/rtc/Kconfig        |  2 +-
>>>  drivers/rtc/rtc-max77686.c | 24 ++++++++++++++++++++++++
>>>  2 files changed, 25 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
>>> index e1bc5214494e..a73591ad292b 100644
>>> --- a/drivers/rtc/Kconfig
>>> +++ b/drivers/rtc/Kconfig
>>> @@ -375,7 +375,7 @@ config RTC_DRV_MAX8997
>>>  
>>>  config RTC_DRV_MAX77686
>>>  	tristate "Maxim MAX77686"
>>> -	depends on MFD_MAX77686 || MFD_MAX77620 || COMPILE_TEST
>>> +	depends on MFD_MAX77686 || MFD_MAX77620 || MFD_MAX77714 || COMPILE_TEST
>>>  	help
>>>  	  If you say yes here you will get support for the
>>>  	  RTC of Maxim MAX77686/MAX77620/MAX77802 PMIC.
>>> diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
>>> index 9901c596998a..e6564bc2171e 100644
>>> --- a/drivers/rtc/rtc-max77686.c
>>> +++ b/drivers/rtc/rtc-max77686.c
>>> @@ -19,6 +19,7 @@
>>>  
>>>  #define MAX77686_I2C_ADDR_RTC		(0x0C >> 1)
>>>  #define MAX77620_I2C_ADDR_RTC		0x68
>>> +#define MAX77714_I2C_ADDR_RTC		0x48
>>>  #define MAX77686_INVALID_I2C_ADDR	(-1)
>>>  
>>>  /* Define non existing register */
>>> @@ -203,6 +204,28 @@ static const struct max77686_rtc_driver_data max77686_drv_data = {
>>>  	.regmap_config = &max77686_rtc_regmap_config,
>>>  };
>>>  
>>> +static const struct regmap_irq_chip max77714_rtc_irq_chip = {
>>> +	.name		= "max77714-rtc",
>>> +	.status_base	= MAX77686_RTC_INT,
>>> +	.mask_base	= MAX77686_RTC_INTM,
>>> +	.num_regs	= 1,
>>> +	.irqs		= max77686_rtc_irqs,
>>> +	.num_irqs	= ARRAY_SIZE(max77686_rtc_irqs) - 1, /* no WTSR on 77714 */
>>> +};
>>> +
>>> +static const struct max77686_rtc_driver_data max77714_drv_data = {
>>> +	.delay = 16000,
>>> +	.mask  = 0x7f,
>>> +	.map   = max77686_map,
>>> +	.alarm_enable_reg = false,
>>> +	.rtc_irq_from_platform = false,
>>
>> As far as I could understand, rtc_irq_from_platform should be 'true'.
>> This would trigger the 'if' branch in function
>> max77686_init_rtc_regmap() [0]:
>>
>>   if (info->drv_data->rtc_irq_from_platform) {
>> 	struct platform_device *pdev = to_platform_device(info->dev);
>>
>> 	info->rtc_irq = platform_get_irq(pdev, 0);
>> 	if (info->rtc_irq < 0)
>> 		return info->rtc_irq;
>>   } else {
>> 	info->rtc_irq =  parent_i2c->irq;
>>   }
>>
>> Calling platform_get_irq() seems correct for the MAX77714, which can
>> generate various IRQ events, collecting them in a register, and raise a
>> single IRQ to the CPU via a physical pin.
>>
>> However, if I set rtc_irq_from_platform = true, platform_get_irq()
>> returns IRQ number '1', which ends up in:
>>
>>   dummy 0-0048: Failed to request IRQ 1 for max77714-rtc: -22
>>   max77686-rtc max77714-rtc: Failed to add RTC irq chip: -22
>>   max77686-rtc: probe of max77714-rtc failed with error -22
>>
>> I compared my code with other MFD drivers and their cell drivers (but
>> their datasheets is not available so I had to add some guesswork), and
>> couldn't find out where my code is wrong.
>>
>> Unfortunately I have no IRQ access on my board (and I don't need them
>> for my use case). For this reason I initially thought of disabling all
>> the IRQ code in rtc-max77686.c via a new flag, but it would be quite
>> invasive and I wouldn't even be able to test that existing hardware
>> still works. Implementing a new RTC driver for the MAX77714 does not
>> seem to be a sane option as the hardware is really 99% equal to the
>> MAX77686 RTC.
>>
> 
> I think the flag should be false, not true. The true means you have RTC
> device with its own interrupt. For example in DT it could look like:
> 
>   pmic@1c {
>       compatible = "maxim,max77714";
>       reg = <0x1c>;
>       interrupt-parent = <&gpio2>;
>       interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
> 
>       interrupt-controller;
>       #interrupt-cells = <2>;
>    };
> 
>    rtc@48 {
>       compatible = "maxim,max77714-rtc";
>       reg = <0x48>;
>       interrupt-parent = <&gpio2>;
>       interrupts = <4 IRQ_TYPE_LEVEL_LOW>;
>    };
> 
> In your case, the RTC device will not have its own devicetree node and
> will be instantiated as MFD child device. The only interrupt line
> available is the parents interrupt line - the same as in max77686 and
> max77802 setups.
> 
> Have in mind that this does not necessarily reflect real HW, but how we
> represent it in devicetree and driver model.

Good to know. Thank you for the explanation.

-- 
Luca

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 7/8] watchdog: max77714: add driver for the watchdog in the MAX77714 PMIC
  2021-10-15 16:42     ` Luca Ceresoli
@ 2021-10-15 17:07       ` Randy Dunlap
  0 siblings, 0 replies; 41+ messages in thread
From: Randy Dunlap @ 2021-10-15 17:07 UTC (permalink / raw)
  To: Luca Ceresoli, linux-kernel
  Cc: Lee Jones, Rob Herring, Alessandro Zummo, Alexandre Belloni,
	Chanwoo Choi, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz,
	Wim Van Sebroeck, Guenter Roeck, devicetree, linux-rtc,
	linux-watchdog, Chiwoong Byun, Laxman Dewangan

On 10/15/21 9:42 AM, Luca Ceresoli wrote:
> Hi,
> 
> On 12/10/21 03:18, Randy Dunlap wrote:
>> Hi,
>>
>> On 10/11/21 8:56 AM, Luca Ceresoli wrote:
>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>>> index bf59faeb3de1..00bc3f932a6c 100644
>>> --- a/drivers/watchdog/Kconfig
>>> +++ b/drivers/watchdog/Kconfig
>>> @@ -699,6 +699,15 @@ config MAX77620_WATCHDOG
>>>         MAX77620 chips. To compile this driver as a module,
>>>         choose M here: the module will be called max77620_wdt.
>>>    +config MAX77714_WATCHDOG
>>> +    tristate "Maxim MAX77714 Watchdog Timer"
>>> +    depends on MFD_MAX77714 || COMPILE_TEST
>>> +    help
>>> +     This is the driver for watchdog timer in the MAX77714 PMIC.
>>> +     Say 'Y' here to enable the watchdog timer support for
>>> +     MAX77714 chips. To compile this driver as a module,
>>> +     choose M here: the module will be called max77714_wdt.
>>
>> Please follow coding-style for Kconfig files:
>>
>> (from Documentation/process/coding-style.rst, section 10):
>>
>> For all of the Kconfig* configuration files throughout the source tree,
>> the indentation is somewhat different.  Lines under a ``config`` definition
>> are indented with one tab, while help text is indented an additional two
>> spaces.
> 
> Oh dear, I usually don't make such silly mistakes, apologies.
> 
> [...some fast typing later...]
> 
> Uhm, now I noticed many entries in that file have that same mistake.
> Perhaps I copy-pasted and didn't check. I'll send a patch to fix them too.
> 

Thanks. :)

-- 
~Randy

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 2/8] rtc: max77686: convert comments to kernel-doc format
  2021-10-11 15:56 ` [PATCH 2/8] rtc: max77686: convert comments to kernel-doc format Luca Ceresoli
  2021-10-12  8:00   ` Krzysztof Kozlowski
@ 2021-10-15 17:29   ` Alexandre Belloni
  1 sibling, 0 replies; 41+ messages in thread
From: Alexandre Belloni @ 2021-10-15 17:29 UTC (permalink / raw)
  To: Luca Ceresoli
  Cc: linux-kernel, Lee Jones, Rob Herring, Alessandro Zummo,
	Chanwoo Choi, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz,
	Wim Van Sebroeck, Guenter Roeck, devicetree, linux-rtc,
	linux-watchdog, Chiwoong Byun, Laxman Dewangan

On 11/10/2021 17:56:09+0200, Luca Ceresoli wrote:
> Convert the comments documenting this struct to kernel-doc format for
> standardization and readability.
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

> ---
>  drivers/rtc/rtc-max77686.c | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
> index eae7cb9faf1e..bac52cdea97d 100644
> --- a/drivers/rtc/rtc-max77686.c
> +++ b/drivers/rtc/rtc-max77686.c
> @@ -61,24 +61,27 @@ enum {
>  	RTC_NR_TIME
>  };
>  
> +/**
> + * struct max77686_rtc_driver_data - model-specific configuration
> + * @delay: Minimum usecs needed for a RTC update
> + * @mask: Mask used to read RTC registers value
> + * @map: Registers offset to I2C addresses map
> + * @alarm_enable_reg: Has a separate alarm enable register?
> + * @rtc_i2c_addr: I2C address for RTC block
> + * @rtc_irq_from_platform: RTC interrupt via platform resource
> + * @alarm_pending_status_reg: Pending alarm status register
> + * @rtc_irq_chip: RTC IRQ CHIP for regmap
> + * @regmap_config: regmap configuration for the chip
> + */
>  struct max77686_rtc_driver_data {
> -	/* Minimum usecs needed for a RTC update */
>  	unsigned long		delay;
> -	/* Mask used to read RTC registers value */
>  	u8			mask;
> -	/* Registers offset to I2C addresses map */
>  	const unsigned int	*map;
> -	/* Has a separate alarm enable register? */
>  	bool			alarm_enable_reg;
> -	/* I2C address for RTC block */
>  	int			rtc_i2c_addr;
> -	/* RTC interrupt via platform resource */
>  	bool			rtc_irq_from_platform;
> -	/* Pending alarm status register */
>  	int			alarm_pending_status_reg;
> -	/* RTC IRQ CHIP for regmap */
>  	const struct regmap_irq_chip *rtc_irq_chip;
> -	/* regmap configuration for the chip */
>  	const struct regmap_config *regmap_config;
>  };
>  
> -- 
> 2.25.1
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 4/8] rtc: max77686: remove useless variable
  2021-10-11 15:56 ` [PATCH 4/8] rtc: max77686: remove useless variable Luca Ceresoli
  2021-10-12  8:01   ` Krzysztof Kozlowski
@ 2021-10-15 17:33   ` Alexandre Belloni
  2021-10-15 20:59     ` Luca Ceresoli
  1 sibling, 1 reply; 41+ messages in thread
From: Alexandre Belloni @ 2021-10-15 17:33 UTC (permalink / raw)
  To: Luca Ceresoli
  Cc: linux-kernel, Lee Jones, Rob Herring, Alessandro Zummo,
	Chanwoo Choi, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz,
	Wim Van Sebroeck, Guenter Roeck, devicetree, linux-rtc,
	linux-watchdog, Chiwoong Byun, Laxman Dewangan

On 11/10/2021 17:56:11+0200, Luca Ceresoli wrote:
>> rtc_24hr_mode is set to 1 in max77686_rtc_probe()->max77686_rtc_init_reg()
> before being read and is never set back to 0 again. As such, it is de facto
> a constant.
> 
> Remove the variable and the unreachable code.
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> ---
>  drivers/rtc/rtc-max77686.c | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
> index 7e765207f28e..9901c596998a 100644
> --- a/drivers/rtc/rtc-max77686.c
> +++ b/drivers/rtc/rtc-max77686.c
> @@ -99,7 +99,6 @@ struct max77686_rtc_info {
>  
>  	int rtc_irq;
>  	int virq;
> -	int rtc_24hr_mode;
>  };
>  
>  enum MAX77686_RTC_OP {
> @@ -278,13 +277,7 @@ static void max77686_rtc_data_to_tm(u8 *data, struct rtc_time *tm,
>  
>  	tm->tm_sec = data[RTC_SEC] & mask;
>  	tm->tm_min = data[RTC_MIN] & mask;
> -	if (info->rtc_24hr_mode) {
> -		tm->tm_hour = data[RTC_HOUR] & 0x1f;
> -	} else {
> -		tm->tm_hour = data[RTC_HOUR] & 0x0f;
> -		if (data[RTC_HOUR] & HOUR_PM_MASK)

So I guess HOUR_PM_SHIFT and HOUR_PM_MASK can also be removed

> -			tm->tm_hour += 12;
> -	}
> +	tm->tm_hour = data[RTC_HOUR] & 0x1f;
>  
>  	/* Only a single bit is set in data[], so fls() would be equivalent */
>  	tm->tm_wday = ffs(data[RTC_WEEKDAY] & mask) - 1;
> @@ -662,8 +655,6 @@ static int max77686_rtc_init_reg(struct max77686_rtc_info *info)
>  	data[0] = (1 << BCD_EN_SHIFT) | (1 << MODEL24_SHIFT);
>  	data[1] = (0 << BCD_EN_SHIFT) | (1 << MODEL24_SHIFT);
>  
> -	info->rtc_24hr_mode = 1;
> -
>  	ret = regmap_bulk_write(info->rtc_regmap,
>  				info->drv_data->map[REG_RTC_CONTROLM],
>  				data, ARRAY_SIZE(data));
> -- 
> 2.25.1
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 8/8] rtc: max77686: add MAX77714 support
  2021-10-11 20:25 ` [PATCH 8/8] rtc: max77686: add MAX77714 support Luca Ceresoli
  2021-10-11 16:12   ` Luca Ceresoli
  2021-10-12  8:29   ` Krzysztof Kozlowski
@ 2021-10-15 17:36   ` Alexandre Belloni
  2 siblings, 0 replies; 41+ messages in thread
From: Alexandre Belloni @ 2021-10-15 17:36 UTC (permalink / raw)
  To: Luca Ceresoli
  Cc: linux-kernel, Lee Jones, Rob Herring, Alessandro Zummo,
	Chanwoo Choi, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz,
	Wim Van Sebroeck, Guenter Roeck, devicetree, linux-rtc,
	linux-watchdog, Chiwoong Byun, Laxman Dewangan

On 11/10/2021 22:25:50+0200, Luca Ceresoli wrote:
> The RTC included in the MAX77714 PMIC is very similar to the one in the
> MAX77686. Reuse the rtc-max77686.c driver with the minimum required changes
> for the MAX77714 RTC.
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

> ---
> 
> *** NOTE ***
> 
> This patch didn't reach most recipients having hit a limit in my service
> provider (125 e-mails per hour). I'm resending it, as far as possible with
> proper message-id etc. Apologies for any duplicate.
> 
>  drivers/rtc/Kconfig        |  2 +-
>  drivers/rtc/rtc-max77686.c | 24 ++++++++++++++++++++++++
>  2 files changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index e1bc5214494e..a73591ad292b 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -375,7 +375,7 @@ config RTC_DRV_MAX8997
> 
>  config RTC_DRV_MAX77686
>  	tristate "Maxim MAX77686"
> -	depends on MFD_MAX77686 || MFD_MAX77620 || COMPILE_TEST
> +	depends on MFD_MAX77686 || MFD_MAX77620 || MFD_MAX77714 || COMPILE_TEST
>  	help
>  	  If you say yes here you will get support for the
>  	  RTC of Maxim MAX77686/MAX77620/MAX77802 PMIC.
> diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
> index 9901c596998a..e6564bc2171e 100644
> --- a/drivers/rtc/rtc-max77686.c
> +++ b/drivers/rtc/rtc-max77686.c
> @@ -19,6 +19,7 @@
> 
>  #define MAX77686_I2C_ADDR_RTC		(0x0C >> 1)
>  #define MAX77620_I2C_ADDR_RTC		0x68
> +#define MAX77714_I2C_ADDR_RTC		0x48
>  #define MAX77686_INVALID_I2C_ADDR	(-1)
> 
>  /* Define non existing register */
> @@ -203,6 +204,28 @@ static const struct max77686_rtc_driver_data max77686_drv_data = {
>  	.regmap_config = &max77686_rtc_regmap_config,
>  };
> 
> +static const struct regmap_irq_chip max77714_rtc_irq_chip = {
> +	.name		= "max77714-rtc",
> +	.status_base	= MAX77686_RTC_INT,
> +	.mask_base	= MAX77686_RTC_INTM,
> +	.num_regs	= 1,
> +	.irqs		= max77686_rtc_irqs,
> +	.num_irqs	= ARRAY_SIZE(max77686_rtc_irqs) - 1, /* no WTSR on 77714 */
> +};
> +
> +static const struct max77686_rtc_driver_data max77714_drv_data = {
> +	.delay = 16000,
> +	.mask  = 0x7f,
> +	.map   = max77686_map,
> +	.alarm_enable_reg = false,
> +	.rtc_irq_from_platform = false,
> +	/* On MAX77714 RTCA1 is BIT 1 of RTCINT (0x00). Not supported by this driver. */
> +	.alarm_pending_status_reg = MAX77686_INVALID_REG,
> +	.rtc_i2c_addr = MAX77714_I2C_ADDR_RTC,
> +	.rtc_irq_chip = &max77714_rtc_irq_chip,
> +	.regmap_config = &max77686_rtc_regmap_config,
> +};
> +
>  static const struct regmap_config max77620_rtc_regmap_config = {
>  	.reg_bits = 8,
>  	.val_bits = 8,
> @@ -846,6 +869,7 @@ static const struct platform_device_id rtc_id[] = {
>  	{ "max77686-rtc", .driver_data = (kernel_ulong_t)&max77686_drv_data, },
>  	{ "max77802-rtc", .driver_data = (kernel_ulong_t)&max77802_drv_data, },
>  	{ "max77620-rtc", .driver_data = (kernel_ulong_t)&max77620_drv_data, },
> +	{ "max77714-rtc", .driver_data = (kernel_ulong_t)&max77714_drv_data, },
>  	{},
>  };
>  MODULE_DEVICE_TABLE(platform, rtc_id);
> --
> 2.25.1

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 4/8] rtc: max77686: remove useless variable
  2021-10-15 17:33   ` Alexandre Belloni
@ 2021-10-15 20:59     ` Luca Ceresoli
  0 siblings, 0 replies; 41+ messages in thread
From: Luca Ceresoli @ 2021-10-15 20:59 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: linux-kernel, Lee Jones, Rob Herring, Alessandro Zummo,
	Chanwoo Choi, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz,
	Wim Van Sebroeck, Guenter Roeck, devicetree, linux-rtc,
	linux-watchdog, Chiwoong Byun, Laxman Dewangan

Hi,

On 15/10/21 19:33, Alexandre Belloni wrote:
> On 11/10/2021 17:56:11+0200, Luca Ceresoli wrote:
>>> rtc_24hr_mode is set to 1 in max77686_rtc_probe()->max77686_rtc_init_reg()
>> before being read and is never set back to 0 again. As such, it is de facto
>> a constant.
>>
>> Remove the variable and the unreachable code.
>>
>> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
>> ---
>>  drivers/rtc/rtc-max77686.c | 11 +----------
>>  1 file changed, 1 insertion(+), 10 deletions(-)
>>
>> diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
>> index 7e765207f28e..9901c596998a 100644
>> --- a/drivers/rtc/rtc-max77686.c
>> +++ b/drivers/rtc/rtc-max77686.c
>> @@ -99,7 +99,6 @@ struct max77686_rtc_info {
>>  
>>  	int rtc_irq;
>>  	int virq;
>> -	int rtc_24hr_mode;
>>  };
>>  
>>  enum MAX77686_RTC_OP {
>> @@ -278,13 +277,7 @@ static void max77686_rtc_data_to_tm(u8 *data, struct rtc_time *tm,
>>  
>>  	tm->tm_sec = data[RTC_SEC] & mask;
>>  	tm->tm_min = data[RTC_MIN] & mask;
>> -	if (info->rtc_24hr_mode) {
>> -		tm->tm_hour = data[RTC_HOUR] & 0x1f;
>> -	} else {
>> -		tm->tm_hour = data[RTC_HOUR] & 0x0f;
>> -		if (data[RTC_HOUR] & HOUR_PM_MASK)
> 
> So I guess HOUR_PM_SHIFT and HOUR_PM_MASK can also be removed

Sure. Coming in v2.

Thanks.
-- 
Luca

^ permalink raw reply	[flat|nested] 41+ messages in thread

end of thread, other threads:[~2021-10-15 20:59 UTC | newest]

Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-11 15:56 [PATCH 0/8] Add MAX77714 PMIC minimal driver (RTC and watchdog only) Luca Ceresoli
2021-10-11 15:56 ` [PATCH 1/8] mfd: max77686: Correct tab-based alignment of register addresses Luca Ceresoli
2021-10-12  8:00   ` Krzysztof Kozlowski
2021-10-11 15:56 ` [PATCH 2/8] rtc: max77686: convert comments to kernel-doc format Luca Ceresoli
2021-10-12  8:00   ` Krzysztof Kozlowski
2021-10-15 17:29   ` Alexandre Belloni
2021-10-11 15:56 ` [PATCH 3/8] rtc: max77686: rename day-of-month defines Luca Ceresoli
2021-10-12  2:23   ` kernel test robot
2021-10-12  2:58   ` kernel test robot
2021-10-12  8:13   ` Krzysztof Kozlowski
2021-10-11 15:56 ` [PATCH 4/8] rtc: max77686: remove useless variable Luca Ceresoli
2021-10-12  8:01   ` Krzysztof Kozlowski
2021-10-15 17:33   ` Alexandre Belloni
2021-10-15 20:59     ` Luca Ceresoli
2021-10-11 15:56 ` [PATCH 5/8] dt-bindings: mfd: add Maxim MAX77714 PMIC Luca Ceresoli
2021-10-12  8:02   ` Krzysztof Kozlowski
2021-10-13 21:28     ` Luca Ceresoli
2021-10-14  7:38       ` Krzysztof Kozlowski
2021-10-11 15:56 ` [PATCH 6/8] mfd: max77714: Add driver for " Luca Ceresoli
2021-10-11 17:15   ` Guenter Roeck
2021-10-12  7:11   ` kernel test robot
2021-10-12  8:09   ` Krzysztof Kozlowski
2021-10-13 21:49     ` Luca Ceresoli
2021-10-14  7:44       ` Krzysztof Kozlowski
2021-10-12  8:32   ` Krzysztof Kozlowski
2021-10-13 21:39     ` Luca Ceresoli
2021-10-14  7:40       ` Krzysztof Kozlowski
2021-10-14  8:25         ` Luca Ceresoli
2021-10-11 15:56 ` [PATCH 7/8] watchdog: max77714: add driver for the watchdog in the " Luca Ceresoli
2021-10-11 17:17   ` Guenter Roeck
2021-10-15 16:43     ` Luca Ceresoli
2021-10-12  1:18   ` Randy Dunlap
2021-10-15 16:42     ` Luca Ceresoli
2021-10-15 17:07       ` Randy Dunlap
2021-10-11 20:25 ` [PATCH 8/8] rtc: max77686: add MAX77714 support Luca Ceresoli
2021-10-11 16:12   ` Luca Ceresoli
2021-10-12  8:20     ` Krzysztof Kozlowski
2021-10-15 16:46       ` Luca Ceresoli
2021-10-12  8:29   ` Krzysztof Kozlowski
2021-10-15 17:36   ` Alexandre Belloni
2021-10-12  7:59 ` [PATCH 0/8] Add MAX77714 PMIC minimal driver (RTC and watchdog only) Krzysztof Kozlowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).