2021-10-19 15:04:38

by Luca Ceresoli

[permalink] [raw]
Subject: [PATCH v2 0/9] Add MAX77714 PMIC minimal driver (RTC and watchdog only)

Hi,

this series adds minimal drivers for the Maxim Semiconductor MAX77714
(https://www.maximintegrated.com/en/products/power/power-management-ics/MAX77714.html).
Only RTC and watchdog are implemented by these patches.

All implemented functionality is tested and working: RTC read/write,
watchdog start/stop/ping/set_timeout.

Patches 1-4 + 7 are trivial cleanups to the max77686 drivers and can
probably be applied easily.

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

Changes in v2:
- fixed all issues reported on v1 patches
- added patch 7 ("watchdog: Kconfig: fix help text indentation")
- additional minor improvements

Luca

Luca Ceresoli (9):
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 unused code to read in 12-hour mode
dt-bindings: mfd: add Maxim MAX77714 PMIC
mfd: max77714: Add driver for Maxim MAX77714 PMIC
watchdog: Kconfig: fix help text indentation
watchdog: 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/max77686.c | 2 +-
drivers/mfd/max77714.c | 165 ++++++++++++++++
drivers/rtc/Kconfig | 2 +-
drivers/rtc/rtc-max77686.c | 75 +++++---
drivers/watchdog/Kconfig | 57 +++---
drivers/watchdog/Makefile | 1 +
drivers/watchdog/max77714_wdt.c | 179 ++++++++++++++++++
include/linux/mfd/max77686-private.h | 28 +--
include/linux/mfd/max77714.h | 60 ++++++
13 files changed, 580 insertions(+), 70 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


2021-10-19 15:05:48

by Luca Ceresoli

[permalink] [raw]
Subject: [PATCH v2 8/9] watchdog: max77714: add driver for the watchdog in the MAX77714 PMIC

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

Signed-off-by: Luca Ceresoli <[email protected]>

---

Changes in v2:
- fix Kconfig help indentation (Randy Dunlap)
- make max77714_margin_value static const (Guenter Roeck)
- fix platform module instantiation
---
MAINTAINERS | 1 +
drivers/watchdog/Kconfig | 9 ++
drivers/watchdog/Makefile | 1 +
drivers/watchdog/max77714_wdt.c | 179 ++++++++++++++++++++++++++++++++
4 files changed, 190 insertions(+)
create mode 100644 drivers/watchdog/max77714_wdt.c

diff --git a/MAINTAINERS b/MAINTAINERS
index abd9de8a9d99..71c3d8513ba0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11391,6 +11391,7 @@ M: Luca Ceresoli <[email protected]>
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 a24385099a91..b9b575acd690 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..cce6c13d76eb
--- /dev/null
+++ b/drivers/watchdog/max77714_wdt.c
@@ -0,0 +1,179 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Maxim MAX77714 Watchdog Driver
+ *
+ * Copyright (C) 2021 Luca Ceresoli
+ * Author: Luca Ceresoli <[email protected]>
+ */
+
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/mfd/max77714.h>
+#include <linux/mod_devicetable.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 */
+static const 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 const struct platform_device_id max77714_wdt_platform_id[] = {
+ { .name = "max77714-watchdog", },
+ { },
+};
+MODULE_DEVICE_TABLE(platform, max77714_wdt_platform_id);
+
+static struct platform_driver max77714_wdt_driver = {
+ .driver = {
+ .name = "max77714-watchdog",
+ },
+ .probe = max77714_wdt_probe,
+ .id_table = max77714_wdt_platform_id,
+};
+
+module_platform_driver(max77714_wdt_driver);
+
+MODULE_DESCRIPTION("MAX77714 watchdog timer driver");
+MODULE_AUTHOR("Luca Ceresoli <[email protected]>");
+MODULE_LICENSE("GPL v2");
--
2.25.1

2021-10-19 15:05:49

by Luca Ceresoli

[permalink] [raw]
Subject: [PATCH v2 3/9] rtc: max77686: rename day-of-month defines

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 <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>

---

Changes in v2:
- fix drivers/mfd/max77686.c build failure due to missing rename
(Reported-by: kernel test robot <[email protected]>)
---
drivers/mfd/max77686.c | 2 +-
drivers/rtc/rtc-max77686.c | 16 ++++++++--------
include/linux/mfd/max77686-private.h | 4 ++--
3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/mfd/max77686.c b/drivers/mfd/max77686.c
index 2ad554b921d9..b1048ab25120 100644
--- a/drivers/mfd/max77686.c
+++ b/drivers/mfd/max77686.c
@@ -87,7 +87,7 @@ static bool max77802_rtc_is_volatile_reg(struct device *dev, unsigned int reg)
reg == MAX77802_RTC_WEEKDAY ||
reg == MAX77802_RTC_MONTH ||
reg == MAX77802_RTC_YEAR ||
- reg == MAX77802_RTC_DATE);
+ reg == MAX77802_RTC_MONTHDAY);
}

static bool max77802_is_volatile_reg(struct device *dev, unsigned int reg)
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

2021-10-19 15:06:01

by Luca Ceresoli

[permalink] [raw]
Subject: [PATCH v2 4/9] rtc: max77686: remove unused code to read in 12-hour mode

The MAX77714 RTC chip is explicitly set to 24-hour mode in
max77686_rtc_probe() -> max77686_rtc_init_reg() and never changed back to
12-hour mode. Accordingly info->rtc_24hr_mode is set to 1 in the same place
and never modified later, so it is de facto a constant. Yet there is code
to read 12-hour time, which is unreachable.

Remove the unused variable, the unreachable code to manage 12-hour mode and
the defines that become unused due to the above changes.

Signed-off-by: Luca Ceresoli <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>

---

Changes in v2:
- remove the now-unused defines too (Alexandre Belloni)
- improve the commit message
---
drivers/rtc/rtc-max77686.c | 14 +-------------
1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
index 7e765207f28e..5c64d08c0732 100644
--- a/drivers/rtc/rtc-max77686.c
+++ b/drivers/rtc/rtc-max77686.c
@@ -34,9 +34,6 @@
#define RTC_UDR_MASK BIT(RTC_UDR_SHIFT)
#define RTC_RBUDR_SHIFT 4
#define RTC_RBUDR_MASK BIT(RTC_RBUDR_SHIFT)
-/* RTC Hour register */
-#define HOUR_PM_SHIFT 6
-#define HOUR_PM_MASK BIT(HOUR_PM_SHIFT)
/* RTC Alarm Enable */
#define ALARM_ENABLE_SHIFT 7
#define ALARM_ENABLE_MASK BIT(ALARM_ENABLE_SHIFT)
@@ -99,7 +96,6 @@ struct max77686_rtc_info {

int rtc_irq;
int virq;
- int rtc_24hr_mode;
};

enum MAX77686_RTC_OP {
@@ -278,13 +274,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 +652,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

2021-10-19 15:06:43

by Luca Ceresoli

[permalink] [raw]
Subject: [PATCH v2 5/9] dt-bindings: mfd: add Maxim MAX77714 PMIC

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

Signed-off-by: Luca Ceresoli <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>

---

Changes in v2: none
---
.../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 <[email protected]>
+
+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 8d118d7957d2..514ff4a735e5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11386,6 +11386,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 <[email protected]>
+S: Maintained
+F: Documentation/devicetree/bindings/mfd/maxim,max77714.yaml
+
MAXIM MAX77802 PMIC REGULATOR DEVICE DRIVER
M: Javier Martinez Canillas <[email protected]>
L: [email protected]
--
2.25.1

2021-10-19 15:08:26

by Luca Ceresoli

[permalink] [raw]
Subject: [PATCH v2 9/9] rtc: max77686: add MAX77714 support

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 <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Acked-by: Alexandre Belloni <[email protected]>

---

Changes in v2: none
---
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 5c64d08c0732..b0250d91fb00 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 */
@@ -200,6 +201,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,
@@ -843,6 +866,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

2021-10-19 15:12:42

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH v2 8/9] watchdog: max77714: add driver for the watchdog in the MAX77714 PMIC

On 10/19/21 7:59 AM, Luca Ceresoli wrote:
> Add a simple driver to support the watchdog embedded in the Maxim MAX77714
> PMIC.
>
> Signed-off-by: Luca Ceresoli <[email protected]>

Acked-by: Guenter Roeck <[email protected]>

The driver needs the include file introduced with the mfd driver,
so I assume it will be submitted through the mfd branch.

Thanks,
Guenter

>
> ---
>
> Changes in v2:
> - fix Kconfig help indentation (Randy Dunlap)
> - make max77714_margin_value static const (Guenter Roeck)
> - fix platform module instantiation
> ---
> MAINTAINERS | 1 +
> drivers/watchdog/Kconfig | 9 ++
> drivers/watchdog/Makefile | 1 +
> drivers/watchdog/max77714_wdt.c | 179 ++++++++++++++++++++++++++++++++
> 4 files changed, 190 insertions(+)
> create mode 100644 drivers/watchdog/max77714_wdt.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index abd9de8a9d99..71c3d8513ba0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11391,6 +11391,7 @@ M: Luca Ceresoli <[email protected]>
> 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 a24385099a91..b9b575acd690 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..cce6c13d76eb
> --- /dev/null
> +++ b/drivers/watchdog/max77714_wdt.c
> @@ -0,0 +1,179 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Maxim MAX77714 Watchdog Driver
> + *
> + * Copyright (C) 2021 Luca Ceresoli
> + * Author: Luca Ceresoli <[email protected]>
> + */
> +
> +#include <linux/err.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/max77714.h>
> +#include <linux/mod_devicetable.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 */
> +static const 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 const struct platform_device_id max77714_wdt_platform_id[] = {
> + { .name = "max77714-watchdog", },
> + { },
> +};
> +MODULE_DEVICE_TABLE(platform, max77714_wdt_platform_id);
> +
> +static struct platform_driver max77714_wdt_driver = {
> + .driver = {
> + .name = "max77714-watchdog",
> + },
> + .probe = max77714_wdt_probe,
> + .id_table = max77714_wdt_platform_id,
> +};
> +
> +module_platform_driver(max77714_wdt_driver);
> +
> +MODULE_DESCRIPTION("MAX77714 watchdog timer driver");
> +MODULE_AUTHOR("Luca Ceresoli <[email protected]>");
> +MODULE_LICENSE("GPL v2");
>

2021-10-21 09:45:28

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [PATCH v2 3/9] rtc: max77686: rename day-of-month defines

On 19/10/2021 16:59:13+0200, 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 <[email protected]>
> Reviewed-by: Krzysztof Kozlowski <[email protected]>
Acked-by: Alexandre Belloni <[email protected]>

>
> ---
>
> Changes in v2:
> - fix drivers/mfd/max77686.c build failure due to missing rename
> (Reported-by: kernel test robot <[email protected]>)
> ---
> drivers/mfd/max77686.c | 2 +-
> drivers/rtc/rtc-max77686.c | 16 ++++++++--------
> include/linux/mfd/max77686-private.h | 4 ++--
> 3 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/mfd/max77686.c b/drivers/mfd/max77686.c
> index 2ad554b921d9..b1048ab25120 100644
> --- a/drivers/mfd/max77686.c
> +++ b/drivers/mfd/max77686.c
> @@ -87,7 +87,7 @@ static bool max77802_rtc_is_volatile_reg(struct device *dev, unsigned int reg)
> reg == MAX77802_RTC_WEEKDAY ||
> reg == MAX77802_RTC_MONTH ||
> reg == MAX77802_RTC_YEAR ||
> - reg == MAX77802_RTC_DATE);
> + reg == MAX77802_RTC_MONTHDAY);
> }
>
> static bool max77802_is_volatile_reg(struct device *dev, unsigned int reg)
> 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
>

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

2021-10-21 09:48:50

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [PATCH v2 4/9] rtc: max77686: remove unused code to read in 12-hour mode

On 19/10/2021 16:59:14+0200, Luca Ceresoli wrote:
> The MAX77714 RTC chip is explicitly set to 24-hour mode in
> max77686_rtc_probe() -> max77686_rtc_init_reg() and never changed back to
> 12-hour mode. Accordingly info->rtc_24hr_mode is set to 1 in the same place
> and never modified later, so it is de facto a constant. Yet there is code
> to read 12-hour time, which is unreachable.
>
> Remove the unused variable, the unreachable code to manage 12-hour mode and
> the defines that become unused due to the above changes.
>
> Signed-off-by: Luca Ceresoli <[email protected]>
> Reviewed-by: Krzysztof Kozlowski <[email protected]>
Acked-by: Alexandre Belloni <[email protected]>

>
> ---
>
> Changes in v2:
> - remove the now-unused defines too (Alexandre Belloni)
> - improve the commit message
> ---
> drivers/rtc/rtc-max77686.c | 14 +-------------
> 1 file changed, 1 insertion(+), 13 deletions(-)
>
> diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
> index 7e765207f28e..5c64d08c0732 100644
> --- a/drivers/rtc/rtc-max77686.c
> +++ b/drivers/rtc/rtc-max77686.c
> @@ -34,9 +34,6 @@
> #define RTC_UDR_MASK BIT(RTC_UDR_SHIFT)
> #define RTC_RBUDR_SHIFT 4
> #define RTC_RBUDR_MASK BIT(RTC_RBUDR_SHIFT)
> -/* RTC Hour register */
> -#define HOUR_PM_SHIFT 6
> -#define HOUR_PM_MASK BIT(HOUR_PM_SHIFT)
> /* RTC Alarm Enable */
> #define ALARM_ENABLE_SHIFT 7
> #define ALARM_ENABLE_MASK BIT(ALARM_ENABLE_SHIFT)
> @@ -99,7 +96,6 @@ struct max77686_rtc_info {
>
> int rtc_irq;
> int virq;
> - int rtc_24hr_mode;
> };
>
> enum MAX77686_RTC_OP {
> @@ -278,13 +274,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 +652,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

2021-10-27 16:48:25

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 5/9] dt-bindings: mfd: add Maxim MAX77714 PMIC

On Tue, Oct 19, 2021 at 04:59:15PM +0200, Luca Ceresoli wrote:
> Add bindings for the MAX77714 PMIC with GPIO, RTC and watchdog.
>
> Signed-off-by: Luca Ceresoli <[email protected]>
> Reviewed-by: Krzysztof Kozlowski <[email protected]>
>
> ---
>
> Changes in v2: none
> ---
> .../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 <[email protected]>
> +
> +description: |
> + MAX77714 is a Power Management IC with 4 buck regulators, 9
> + low-dropout regulators, 8 GPIOs, RTC and watchdog.

Where's the regulators nodes and binding?

> +
> +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 8d118d7957d2..514ff4a735e5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11386,6 +11386,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 <[email protected]>
> +S: Maintained
> +F: Documentation/devicetree/bindings/mfd/maxim,max77714.yaml
> +
> MAXIM MAX77802 PMIC REGULATOR DEVICE DRIVER
> M: Javier Martinez Canillas <[email protected]>
> L: [email protected]
> --
> 2.25.1
>
>

2021-10-29 15:52:58

by Luca Ceresoli

[permalink] [raw]
Subject: Re: [PATCH v2 5/9] dt-bindings: mfd: add Maxim MAX77714 PMIC

Hi Rob,

On 27/10/21 05:17, Rob Herring wrote:
> On Tue, Oct 19, 2021 at 04:59:15PM +0200, Luca Ceresoli wrote:
>> Add bindings for the MAX77714 PMIC with GPIO, RTC and watchdog.
>>
>> Signed-off-by: Luca Ceresoli <[email protected]>
>> Reviewed-by: Krzysztof Kozlowski <[email protected]>
>>
>> ---
>>
>> Changes in v2: none
>> ---
>> .../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 <[email protected]>
>> +
>> +description: |
>> + MAX77714 is a Power Management IC with 4 buck regulators, 9
>> + low-dropout regulators, 8 GPIOs, RTC and watchdog.
>
> Where's the regulators nodes and binding?

As discussed for the v1 patches [0]:

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?

[0]
https://lore.kernel.org/lkml/[email protected]/

--
Luca