linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/2] drivers: rtc: add max313xx series rtc driver
@ 2024-02-02  2:52 Chris Packham
  2024-02-02  2:52 ` [PATCH v6 1/2] " Chris Packham
  2024-02-02  2:52 ` [PATCH v6 2/2] dt-bindings: rtc: add max313xx RTCs Chris Packham
  0 siblings, 2 replies; 12+ messages in thread
From: Chris Packham @ 2024-02-02  2:52 UTC (permalink / raw)
  To: alexandre.belloni, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	jdelvare, linux, antoniu.miclaus, noname.nuno
  Cc: linux-rtc, devicetree, linux-kernel, linux-hwmon, Chris Packham

changelog:
since v5:
  - change of maintainer
  - use adi,ti-diode property
  - deal with oscillator fail and releasing SWRST

since v4:
  - dt-binding: add enum value "2" to aux-voltage-chargable
  - dt-binding: remove adi,trickle-diode-enable
  - dt-binding: change description of trickle-resistor-ohms
  - dt-binding: reorder as in example schema
  - parse "wakeup-source" when irq not requested
  - remove limitation on max31328 irq and clockout
  - remove error and warning messages during trickle charger setup

since v3:
  - dt-binding: remove interrupt names.
  - dt-binding: add description for "interrupts" property
  - dt-binding: replace deprecated property "trickle-diode-disable"
      by "aux-voltage-chargeable"
  - dt-binding: add new property "adi,trickle-diode-enable"
  - dt-binding: remove "wakeup-source"
  - use clear_bit instead of __clear_bit
  - use devm_of_clk_add_hw_provider instead of of_clk_add_provider
  - use chip_desc pointer as driver data instead of enum.

since v2:
  - add "break" to fix warning: unannotated fall-through
    Reported-by: kernel test robot <lkp@intel.com>

since v1:
  - dt-binding: update title and description
  - dt-binding: remove last example
  - drop watchdog support
  - support reading 12Hr format instead of forcing 24hr at probe time
  - use "tm_year % 100" instead of range check
  - refactor max313xx_init for readability

Ibrahim Tilki (2):
  drivers: rtc: add max313xx series rtc driver
  dt-bindings: rtc: add max313xx RTCs

 .../devicetree/bindings/rtc/adi,max313xx.yaml |  145 +++
 drivers/rtc/Kconfig                           |   11 +
 drivers/rtc/Makefile                          |    1 +
 drivers/rtc/rtc-max313xx.c                    | 1098 +++++++++++++++++
 4 files changed, 1255 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/adi,max313xx.yaml
 create mode 100644 drivers/rtc/rtc-max313xx.c

-- 
2.43.0


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

* [PATCH v6 1/2] drivers: rtc: add max313xx series rtc driver
  2024-02-02  2:52 [PATCH v6 0/2] drivers: rtc: add max313xx series rtc driver Chris Packham
@ 2024-02-02  2:52 ` Chris Packham
  2024-02-03  7:07   ` kernel test robot
  2024-02-04  2:25   ` kernel test robot
  2024-02-02  2:52 ` [PATCH v6 2/2] dt-bindings: rtc: add max313xx RTCs Chris Packham
  1 sibling, 2 replies; 12+ messages in thread
From: Chris Packham @ 2024-02-02  2:52 UTC (permalink / raw)
  To: alexandre.belloni, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	jdelvare, linux, antoniu.miclaus, noname.nuno
  Cc: linux-rtc, devicetree, linux-kernel, linux-hwmon, Ibrahim Tilki,
	Zeynep Arslanbenzer, Chris Packham

From: Ibrahim Tilki <Ibrahim.Tilki@analog.com>

Adding support for Analog Devices MAX313XX series RTCs.

Signed-off-by: Ibrahim Tilki <Ibrahim.Tilki@analog.com>
Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 drivers/rtc/Kconfig        |   11 +
 drivers/rtc/Makefile       |    1 +
 drivers/rtc/rtc-max313xx.c | 1098 ++++++++++++++++++++++++++++++++++++
 3 files changed, 1110 insertions(+)
 create mode 100644 drivers/rtc/rtc-max313xx.c

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index e37a4341f442..fc53cc3c6093 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -386,6 +386,17 @@ config RTC_DRV_MAX31335
 	  This driver can also be built as a module. If so, the module
 	  will be called rtc-max31335.
 
+config RTC_DRV_MAX313XX
+	tristate "Analog Devices MAX313XX RTC driver"
+	depends on I2C
+	select REGMAP_I2C
+	help
+	  If you say yes here you will get support for the
+	  Analog Devices MAX313XX series RTC family.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called rtc-max313xx.
+
 config RTC_DRV_MAX77686
 	tristate "Maxim MAX77686"
 	depends on MFD_MAX77686 || MFD_MAX77620 || MFD_MAX77714 || COMPILE_TEST
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 6efff381c484..8bf6c60c837c 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -89,6 +89,7 @@ obj-$(CONFIG_RTC_DRV_M48T35)	+= rtc-m48t35.o
 obj-$(CONFIG_RTC_DRV_M48T59)	+= rtc-m48t59.o
 obj-$(CONFIG_RTC_DRV_M48T86)	+= rtc-m48t86.o
 obj-$(CONFIG_RTC_DRV_MA35D1)	+= rtc-ma35d1.o
+obj-$(CONFIG_RTC_DRV_MAX313XX)	+= rtc-max313xx.o
 obj-$(CONFIG_RTC_DRV_MAX31335)	+= rtc-max31335.o
 obj-$(CONFIG_RTC_DRV_MAX6900)	+= rtc-max6900.o
 obj-$(CONFIG_RTC_DRV_MAX6902)	+= rtc-max6902.o
diff --git a/drivers/rtc/rtc-max313xx.c b/drivers/rtc/rtc-max313xx.c
new file mode 100644
index 000000000000..50194f162e13
--- /dev/null
+++ b/drivers/rtc/rtc-max313xx.c
@@ -0,0 +1,1098 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Analog Devices MAX313XX series I2C RTC driver
+ *
+ * Copyright 2023 Analog Devices Inc.
+ */
+#include <asm-generic/unaligned.h>
+#include <linux/bcd.h>
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/hwmon.h>
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+#include <linux/rtc.h>
+#include <linux/util_macros.h>
+
+/* common registers */
+#define MAX313XX_INT_ALARM1		BIT(0)
+#define MAX313XX_HRS_F_AM_PM		BIT(5)
+#define MAX313XX_HRS_F_12_24		BIT(6)
+#define MAX313XX_MONTH_CENTURY		BIT(7)
+
+#define MAX313XX_TIME_SIZE		0x07
+
+/* device specific registers */
+#define MAX3134X_CFG2_REG		0x01
+#define MAX3134X_CFG2_SET_RTC		BIT(1)
+
+#define MAX31341_TRICKLE_RES_MASK	GENMASK(1, 0)
+#define MAX31341_TRICKLE_DIODE_EN	BIT(2)
+#define MAX31341_TRICKLE_ENABLE_BIT	BIT(3)
+#define MAX31341_POWER_MGMT_REG		0x56
+#define MAX31341_POWER_MGMT_TRICKLE_BIT	BIT(0)
+
+#define MAX3133X_TRICKLE_RES_MASK	GENMASK(2, 1)
+#define MAX3133X_TRICKLE_DIODE_EN	BIT(3)
+#define MAX3133X_TRICKLE_ENABLE_BIT	BIT(0)
+
+#define MAX31329_TRICKLE_ENABLE_BIT	BIT(7)
+#define MAX31343_TRICKLE_ENABLE_MASK	GENMASK(7, 4)
+#define MAX31343_TRICKLE_ENABLE_CODE	5
+#define MAX31329_43_TRICKLE_RES_MASK	GENMASK(1, 0)
+#define MAX31329_43_TRICKLE_DIODE_EN	BIT(2)
+
+#define MAX31329_CONFIG2_REG		0x04
+#define MAX31329_CONFIG2_CLKIN_EN	BIT(2)
+#define MAX31329_CONFIG2_CLKIN_FREQ	GENMASK(1, 0)
+
+#define MAX31341_42_CONFIG1_REG		0x00
+#define MAX31341_42_CONFIG1_CLKIN_EN	BIT(7)
+#define MAX31341_42_CONFIG1_CLKIN_FREQ	GENMASK(5, 4)
+#define MAX31341_42_CONFIG1_OSC_DISABLE	BIT(3)
+#define MAX31341_42_CONFIG1_SWRST	BIT(0)
+
+enum max313xx_ids {
+	ID_MAX31328,
+	ID_MAX31329,
+	ID_MAX31331,
+	ID_MAX31334,
+	ID_MAX31341,
+	ID_MAX31342,
+	ID_MAX31343,
+	MAX313XX_ID_NR
+};
+
+struct clkout_cfg {
+	const int *freq_avail;
+	u8 freq_size;
+	u8 freq_pos;
+	u8 reg;
+	u8 en_bit;
+	bool en_invert;
+};
+
+struct chip_desc {
+	struct clkout_cfg *clkout;
+	const char *clkout_name;
+	u8 sec_reg;
+	u8 alarm1_sec_reg;
+
+	u8 int_en_reg;
+	u8 int_status_reg;
+	u8 osf_bit;
+
+	u8 ram_reg;
+	u8 ram_size;
+
+	u8 temp_reg;
+
+	u8 trickle_reg;
+
+	u8 rst_reg;
+	u8 rst_bit;
+};
+
+#define clk_hw_to_max313xx(_hw) container_of(_hw, struct max313xx, clkout)
+
+struct max313xx {
+	enum max313xx_ids id;
+	struct regmap *regmap;
+	struct rtc_device *rtc;
+	struct clk_hw clkout;
+	struct clk *clkin;
+	const struct chip_desc *chip;
+	int irq;
+};
+
+static const int max313xx_clkin_freq[] = { 1, 50, 60, 32768 };
+
+static const int max31328_clkout_freq[] = { 1, 1024, 4096, 8192 };
+static const int max31329_clkout_freq[] = { 1, 4096, 8192, 32768 };
+static const int max3133x_clkout_freq[] = { 1, 64, 1024, 32768 };
+static const int max31341_42_clkout_freq[] = { 1, 50, 60, 32768 };
+static const int max31343_clkout_freq[] = { 1, 2, 4, 8, 16, 32, 64, 128, 32875 };
+
+static struct clkout_cfg max31328_clkout = {
+	.freq_avail = max31328_clkout_freq,
+	.freq_size = ARRAY_SIZE(max31328_clkout_freq),
+	.freq_pos = 3,
+	.reg = 0x0E,
+	.en_bit = BIT(3),
+	.en_invert = true,
+};
+
+static struct clkout_cfg max31329_clkout = {
+	.freq_avail = max31329_clkout_freq,
+	.freq_size = ARRAY_SIZE(max31329_clkout_freq),
+	.freq_pos = 5,
+	.reg = 0x04,
+	.en_bit = BIT(7),
+};
+
+static struct clkout_cfg max3133x_clkout = {
+	.freq_avail = max3133x_clkout_freq,
+	.freq_size = ARRAY_SIZE(max3133x_clkout_freq),
+	.freq_pos = 0,
+	.reg = 0x04,
+	.en_bit = BIT(2),
+};
+
+static struct clkout_cfg max31341_42_clkout = {
+	.freq_avail = max31341_42_clkout_freq,
+	.freq_size = ARRAY_SIZE(max31341_42_clkout_freq),
+	.freq_pos = 1,
+	.reg = 0x00,
+	.en_bit = BIT(6),
+	.en_invert = true,
+};
+
+static struct clkout_cfg max31343_clkout = {
+	.freq_avail = max31343_clkout_freq,
+	.freq_size = ARRAY_SIZE(max31343_clkout_freq),
+	.freq_pos = 3,
+	.reg = 0x04,
+	.en_bit = BIT(7),
+};
+
+static const struct chip_desc chip[MAX313XX_ID_NR] = {
+	[ID_MAX31328] = {
+		.int_en_reg = 0x0E,
+		.int_status_reg = 0x0F,
+		.osf_bit = BIT(7),
+		.sec_reg = 0x00,
+		.alarm1_sec_reg = 0x07,
+		.temp_reg = 0x11,
+		.clkout = &max31328_clkout,
+		.clkout_name = "max31328-sqw",
+	},
+	[ID_MAX31329] = {
+		.int_en_reg = 0x01,
+		.int_status_reg = 0x00,
+		.osf_bit = BIT(6),
+		.sec_reg = 0x06,
+		.alarm1_sec_reg = 0x0D,
+		.ram_reg = 0x22,
+		.ram_size = 64,
+		.trickle_reg = 0x19,
+		.clkout = &max31329_clkout,
+		.clkout_name = "max31329-clkout",
+		.rst_reg = 0x02,
+		.rst_bit = BIT(0),
+	},
+	[ID_MAX31331] = {
+		.int_en_reg = 0x01,
+		.int_status_reg = 0x00,
+		.osf_bit = BIT(6),
+		.sec_reg = 0x08,
+		.alarm1_sec_reg = 0x0F,
+		.ram_reg = 0x20,
+		.ram_size = 32,
+		.trickle_reg = 0x1B,
+		.clkout = &max3133x_clkout,
+		.clkout_name = "max31331-clkout",
+		.rst_reg = 0x02,
+		.rst_bit = BIT(0),
+	},
+	[ID_MAX31334] = {
+		.int_en_reg = 0x01,
+		.int_status_reg = 0x00,
+		.osf_bit = BIT(6),
+		.sec_reg = 0x09,
+		.alarm1_sec_reg = 0x10,
+		.ram_reg = 0x30,
+		.ram_size = 32,
+		.trickle_reg = 0x1E,
+		.clkout = &max3133x_clkout,
+		.clkout_name = "max31334-clkout",
+		.rst_reg = 0x02,
+		.rst_bit = BIT(0),
+	},
+	[ID_MAX31341] = {
+		.int_en_reg = 0x04,
+		.int_status_reg = 0x05,
+		.osf_bit = BIT(6),
+		.sec_reg = 0x06,
+		.alarm1_sec_reg = 0x0D,
+		.ram_reg = 0x16,
+		.ram_size = 64,
+		.trickle_reg = 0x57,
+		.clkout = &max31341_42_clkout,
+		.clkout_name = "max31341-clkout",
+		.rst_reg = 0x00,
+		.rst_bit = BIT(0),
+	},
+	[ID_MAX31342] = {
+		.int_en_reg = 0x04,
+		.int_status_reg = 0x05,
+		.osf_bit = BIT(6),
+		.sec_reg = 0x06,
+		.alarm1_sec_reg = 0x0D,
+		.clkout = &max31341_42_clkout,
+		.clkout_name = "max31342-clkout",
+		.rst_reg = 0x00,
+		.rst_bit = BIT(0),
+	},
+	[ID_MAX31343] = {
+		.int_en_reg = 0x01,
+		.int_status_reg = 0x00,
+		.osf_bit = BIT(6),
+		.sec_reg = 0x06,
+		.alarm1_sec_reg = 0x0D,
+		.ram_reg = 0x22,
+		.ram_size = 64,
+		.temp_reg = 0x1A,
+		.trickle_reg = 0x19,
+		.clkout = &max31343_clkout,
+		.clkout_name = "max31343-clko",
+		.rst_reg = 0x02,
+		.rst_bit = BIT(0),
+	},
+};
+
+static const u32 max313xx_trickle_ohms[] = { 3000, 6000, 11000 };
+
+static bool max313xx_volatile_reg(struct device *dev, unsigned int reg)
+{
+	struct max313xx *rtc = dev_get_drvdata(dev);
+	const struct chip_desc *chip = rtc->chip;
+
+	/* time keeping registers */
+	if (reg >= chip->sec_reg && reg < chip->sec_reg + MAX313XX_TIME_SIZE)
+		return true;
+
+	/* interrupt status register */
+	if (reg == chip->int_status_reg)
+		return true;
+
+	/* temperature registers */
+	return chip->temp_reg &&
+		(reg == chip->temp_reg || reg == chip->temp_reg + 1);
+}
+
+static const struct regmap_config regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.cache_type = REGCACHE_RBTREE,
+	.volatile_reg = max313xx_volatile_reg,
+};
+
+static int max313xx_get_hour(u8 hour_reg)
+{
+	int hour;
+
+	/* 24Hr mode */
+	if (!FIELD_GET(MAX313XX_HRS_F_12_24, hour_reg))
+		return bcd2bin(hour_reg & 0x3f);
+
+	/* 12Hr mode */
+	hour = bcd2bin(hour_reg & 0x1f);
+	if (hour == 12)
+		hour = 0;
+
+	if (FIELD_GET(MAX313XX_HRS_F_AM_PM, hour_reg))
+		hour += 12;
+
+	return hour;
+}
+
+static int max313xx_read_time(struct device *dev, struct rtc_time *t)
+{
+	struct max313xx *rtc = dev_get_drvdata(dev);
+	u8 regs[7];
+	int ret;
+	unsigned int status;
+
+	ret = regmap_read(rtc->regmap, rtc->chip->int_status_reg, &status);
+	if (ret)
+		return ret;
+
+	if (status & rtc->chip->osf_bit)
+		return -EINVAL;
+
+	ret = regmap_bulk_read(rtc->regmap, rtc->chip->sec_reg, regs, 7);
+	if (ret)
+		return ret;
+
+	t->tm_sec = bcd2bin(regs[0] & 0x7f);
+	t->tm_min = bcd2bin(regs[1] & 0x7f);
+	t->tm_hour = max313xx_get_hour(regs[2]);
+	t->tm_wday = bcd2bin(regs[3] & 0x07) - 1;
+	t->tm_mday = bcd2bin(regs[4] & 0x3f);
+	t->tm_mon = bcd2bin(regs[5] & 0x1f) - 1;
+	t->tm_year = bcd2bin(regs[6]) + 100;
+
+	if (FIELD_GET(MAX313XX_MONTH_CENTURY, regs[5]))
+		t->tm_year += 100;
+
+	return 0;
+}
+
+static int max313xx_pre_set_time(struct max313xx *rtc)
+{
+	if (!rtc->clkin)
+		return 0;
+
+	/* Clkin needs to be disabled before setting time. */
+	switch (rtc->id) {
+	case ID_MAX31341:
+	case ID_MAX31342:
+		return regmap_clear_bits(rtc->regmap,
+					 MAX31341_42_CONFIG1_REG,
+					 MAX31341_42_CONFIG1_CLKIN_EN);
+	default:
+		return 0;
+	}
+}
+
+static int max313xx_post_set_time(struct max313xx *rtc)
+{
+	int ret;
+
+	switch (rtc->id) {
+	case ID_MAX31341:
+	case ID_MAX31342:
+		ret = regmap_set_bits(rtc->regmap, MAX3134X_CFG2_REG,
+				      MAX3134X_CFG2_SET_RTC);
+		if (ret)
+			return ret;
+
+		fsleep(10000);
+
+		ret = regmap_clear_bits(rtc->regmap, MAX3134X_CFG2_REG,
+					MAX3134X_CFG2_SET_RTC);
+		if (ret)
+			return ret;
+
+		if (rtc->clkin)
+			ret = regmap_set_bits(rtc->regmap,
+					      MAX31341_42_CONFIG1_REG,
+					      MAX31341_42_CONFIG1_CLKIN_EN);
+
+		break;
+	default:
+		return 0;
+	}
+
+	return ret;
+}
+
+static int max313xx_set_time(struct device *dev, struct rtc_time *t)
+{
+	struct max313xx *rtc = dev_get_drvdata(dev);
+	u8 regs[7];
+	int ret;
+
+	regs[0] = bin2bcd(t->tm_sec);
+	regs[1] = bin2bcd(t->tm_min);
+	regs[2] = bin2bcd(t->tm_hour);
+	regs[3] = bin2bcd(t->tm_wday + 1);
+	regs[4] = bin2bcd(t->tm_mday);
+	regs[5] = bin2bcd(t->tm_mon + 1);
+	regs[6] = bin2bcd(t->tm_year % 100);
+
+	if (t->tm_year >= 200)
+		regs[5] |= FIELD_PREP(MAX313XX_MONTH_CENTURY, 1);
+
+	ret = max313xx_pre_set_time(rtc);
+	if (ret)
+		return ret;
+
+	if (rtc->chip->rst_bit) {
+		ret = regmap_clear_bits(rtc->regmap, rtc->chip->rst_reg, rtc->chip->rst_bit);
+		if (ret)
+			return ret;
+	}
+
+	ret = regmap_bulk_write(rtc->regmap, rtc->chip->sec_reg, regs, 7);
+	if (ret)
+		return ret;
+
+	return max313xx_post_set_time(rtc);
+}
+
+static int max313xx_read_alarm(struct device *dev, struct rtc_wkalrm *t)
+{
+	struct max313xx *rtc = dev_get_drvdata(dev);
+	unsigned int status, int_en;
+	struct rtc_time time;
+	u8 regs[6];
+	int ret;
+
+	ret = regmap_bulk_read(rtc->regmap, rtc->chip->alarm1_sec_reg, regs,
+			       sizeof(regs));
+	if (ret)
+		return ret;
+
+	t->time.tm_sec = bcd2bin(regs[0] & 0x7f);
+	t->time.tm_min = bcd2bin(regs[1] & 0x7f);
+	t->time.tm_hour = bcd2bin(regs[2] & 0x3f);
+	t->time.tm_mday = bcd2bin(regs[3] & 0x3f);
+	t->time.tm_mon = bcd2bin(regs[4] & 0x1f) - 1;
+	t->time.tm_year = bcd2bin(regs[5]) + 100;
+
+	ret = max313xx_read_time(dev, &time);
+	if (ret)
+		return ret;
+
+	if (time.tm_year >= 200)
+		t->time.tm_year += 100;
+
+	ret = regmap_read(rtc->regmap, rtc->chip->int_status_reg, &status);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(rtc->regmap, rtc->chip->int_en_reg, &int_en);
+	if (ret)
+		return ret;
+
+	t->enabled = FIELD_GET(MAX313XX_INT_ALARM1, int_en);
+	t->pending = FIELD_GET(MAX313XX_INT_ALARM1, status);
+
+	return 0;
+}
+
+static int max313xx_set_alarm(struct device *dev, struct rtc_wkalrm *t)
+{
+	struct max313xx *rtc = dev_get_drvdata(dev);
+	unsigned int reg;
+	u8 regs[6];
+	int ret;
+
+	regs[0] = bin2bcd(t->time.tm_sec);
+	regs[1] = bin2bcd(t->time.tm_min);
+	regs[2] = bin2bcd(t->time.tm_hour);
+	regs[3] = bin2bcd(t->time.tm_mday);
+	regs[4] = bin2bcd(t->time.tm_mon + 1);
+	regs[5] = bin2bcd(t->time.tm_year % 100);
+
+	ret = regmap_bulk_write(rtc->regmap, rtc->chip->alarm1_sec_reg, regs,
+				sizeof(regs));
+	if (ret)
+		return ret;
+
+	reg = FIELD_PREP(MAX313XX_INT_ALARM1, t->enabled);
+	ret = regmap_update_bits(rtc->regmap, rtc->chip->int_en_reg,
+				 MAX313XX_INT_ALARM1, reg);
+	if (ret)
+		return ret;
+
+	/* Clear status register */
+	return regmap_read(rtc->regmap, rtc->chip->int_status_reg, &reg);
+}
+
+static int max313xx_alarm_irq_enable(struct device *dev, unsigned int enabled)
+{
+	struct max313xx *rtc = dev_get_drvdata(dev);
+
+	return regmap_update_bits(rtc->regmap, rtc->chip->int_en_reg,
+				  MAX313XX_INT_ALARM1,
+				  FIELD_PREP(MAX313XX_INT_ALARM1, enabled));
+}
+
+static const struct rtc_class_ops max3133x_rtc_ops = {
+	.read_time	= max313xx_read_time,
+	.set_time	= max313xx_set_time,
+	.read_alarm	= max313xx_read_alarm,
+	.set_alarm	= max313xx_set_alarm,
+	.alarm_irq_enable = max313xx_alarm_irq_enable,
+};
+
+static irqreturn_t max313xx_irq(int irq, void *dev_id)
+{
+	struct max313xx	*rtc = dev_id;
+	struct mutex *lock = &rtc->rtc->ops_lock;
+	int stat, ret;
+
+	mutex_lock(lock);
+	ret = regmap_read(rtc->regmap, rtc->chip->int_status_reg, &stat);
+	if (ret)
+		goto out;
+
+	if (FIELD_GET(MAX313XX_INT_ALARM1, stat)) {
+		ret = regmap_update_bits(rtc->regmap, rtc->chip->int_en_reg,
+					 MAX313XX_INT_ALARM1, 0);
+		if (ret)
+			goto out;
+
+		rtc_update_irq(rtc->rtc, 1, RTC_AF | RTC_IRQF);
+	}
+
+out:
+	mutex_unlock(lock);
+
+	return IRQ_HANDLED;
+}
+
+static int max313xx_nvmem_reg_read(void *priv, unsigned int offset,
+				   void *val, size_t bytes)
+{
+	struct max313xx *rtc = priv;
+	unsigned int reg = rtc->chip->ram_reg + offset;
+
+	return regmap_bulk_read(rtc->regmap, reg, val, bytes);
+}
+
+static int max313xx_nvmem_reg_write(void *priv, unsigned int offset,
+				    void *val, size_t bytes)
+{
+	struct max313xx *rtc = priv;
+	unsigned int reg = rtc->chip->ram_reg + offset;
+
+	return regmap_bulk_write(rtc->regmap, reg, val, bytes);
+}
+
+struct nvmem_config max313xx_nvmem_cfg = {
+	.reg_read = max313xx_nvmem_reg_read,
+	.reg_write = max313xx_nvmem_reg_write,
+	.word_size = 8,
+};
+
+static unsigned long max313xx_clkout_recalc_rate(struct clk_hw *hw,
+						 unsigned long parent_rate)
+{
+	struct max313xx *rtc = clk_hw_to_max313xx(hw);
+	const struct clkout_cfg *clkout = rtc->chip->clkout;
+	unsigned int freq_mask;
+	unsigned int reg;
+	int ret;
+
+	ret = regmap_read(rtc->regmap, clkout->reg, &reg);
+	if (ret)
+		return 0;
+
+	freq_mask = __roundup_pow_of_two(clkout->freq_size) - 1;
+
+	return clkout->freq_avail[(reg >> clkout->freq_pos) & freq_mask];
+}
+
+static long max313xx_clkout_round_rate(struct clk_hw *hw, unsigned long rate,
+				       unsigned long *prate)
+{
+	struct max313xx *rtc = clk_hw_to_max313xx(hw);
+	struct clkout_cfg *clkout = rtc->chip->clkout;
+	int index;
+
+	index = find_closest(rate, clkout->freq_avail, clkout->freq_size);
+	return clkout->freq_avail[index];
+}
+
+static int max313xx_clkout_set_rate(struct clk_hw *hw, unsigned long rate,
+				    unsigned long parent_rate)
+{
+	struct max313xx *rtc = clk_hw_to_max313xx(hw);
+	struct clkout_cfg *clkout = rtc->chip->clkout;
+	unsigned int freq_mask;
+	int index;
+
+	index = find_closest(rate, clkout->freq_avail, clkout->freq_size);
+	freq_mask = __roundup_pow_of_two(clkout->freq_size) - 1;
+
+	return regmap_update_bits(rtc->regmap, clkout->reg,
+				  freq_mask << clkout->freq_pos,
+				  index << clkout->freq_pos);
+}
+
+static int max313xx_clkout_enable(struct clk_hw *hw)
+{
+	struct max313xx *rtc = clk_hw_to_max313xx(hw);
+	struct clkout_cfg *clkout = rtc->chip->clkout;
+
+	if (clkout->en_invert)
+		return regmap_clear_bits(rtc->regmap, clkout->reg,
+					 clkout->en_bit);
+
+	return regmap_set_bits(rtc->regmap, clkout->reg,  clkout->en_bit);
+}
+
+static void max313xx_clkout_disable(struct clk_hw *hw)
+{
+	struct max313xx *rtc = clk_hw_to_max313xx(hw);
+	struct clkout_cfg *clkout = rtc->chip->clkout;
+
+	switch (rtc->id) {
+	case ID_MAX31331:
+	case ID_MAX31334:
+		if (rtc->irq > 0) {
+			dev_err(rtc->rtc->dev.parent,
+				"clkout cannot be disabled when IRQ is requested");
+			return;
+		}
+		break;
+	default:
+		break;
+	}
+
+	if (clkout->en_invert)
+		regmap_set_bits(rtc->regmap, clkout->reg, clkout->en_bit);
+	else
+		regmap_clear_bits(rtc->regmap, clkout->reg,  clkout->en_bit);
+}
+
+static int max313xx_clkout_is_enabled(struct clk_hw *hw)
+{
+	struct max313xx *rtc = clk_hw_to_max313xx(hw);
+	struct clkout_cfg *clkout = rtc->chip->clkout;
+	unsigned int reg;
+	int ret;
+
+	ret = regmap_read(rtc->regmap, clkout->reg, &reg);
+	if (ret)
+		return ret;
+
+	return !!(reg & clkout->en_bit) ^ clkout->en_invert;
+}
+
+static const struct clk_ops max313xx_clkout_ops = {
+	.recalc_rate = max313xx_clkout_recalc_rate,
+	.round_rate = max313xx_clkout_round_rate,
+	.set_rate = max313xx_clkout_set_rate,
+	.enable = max313xx_clkout_enable,
+	.disable = max313xx_clkout_disable,
+	.is_enabled = max313xx_clkout_is_enabled,
+};
+
+struct clk_init_data max313xx_clk_init = {
+	.name = "max313xx-clkout",
+	.ops = &max313xx_clkout_ops,
+};
+
+#if IS_REACHABLE(HWMON)
+static int max313xx_read_temp(struct device *dev, enum hwmon_sensor_types type,
+			      u32 attr, int channel, long *val)
+{
+	struct max313xx *rtc = dev_get_drvdata(dev);
+	u8 reg[2];
+	s16 temp;
+	int ret;
+
+	if (type != hwmon_temp || attr != hwmon_temp_input)
+		return -EOPNOTSUPP;
+
+	ret = regmap_bulk_read(rtc->regmap, rtc->chip->temp_reg, reg, 2);
+	if (ret)
+		return ret;
+
+	temp = get_unaligned_be16(reg);
+
+	*val = (temp / 64) * 250;
+
+	return 0;
+}
+
+static umode_t max313xx_is_visible(const void *data,
+				   enum hwmon_sensor_types type,
+				   u32 attr, int channel)
+{
+	if (type == hwmon_temp && attr == hwmon_temp_input)
+		return 0444;
+
+	return 0;
+}
+
+static const struct hwmon_channel_info *max313xx_info[] = {
+	HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT),
+	NULL
+};
+
+static const struct hwmon_ops max313xx_hwmon_ops = {
+	.is_visible = max313xx_is_visible,
+	.read = max313xx_read_temp,
+};
+
+static const struct hwmon_chip_info max313xx_chip_info = {
+	.ops = &max313xx_hwmon_ops,
+	.info = max313xx_info,
+};
+#endif
+
+static int max313xx_init(struct max313xx *rtc)
+{
+	int ret;
+
+	switch (rtc->id) {
+	case ID_MAX31341:
+	case ID_MAX31342:
+		ret = regmap_clear_bits(rtc->regmap, MAX31341_42_CONFIG1_REG,
+					MAX31341_42_CONFIG1_OSC_DISABLE);
+		if (ret)
+			return ret;
+
+		return regmap_set_bits(rtc->regmap, MAX31341_42_CONFIG1_REG,
+				       MAX31341_42_CONFIG1_SWRST);
+	default:
+		return 0;
+	}
+}
+
+static int max313xx_clkout_register(struct device *dev)
+{
+	struct max313xx *rtc = dev_get_drvdata(dev);
+	int ret;
+
+	if (!device_property_present(dev, "#clock-cells"))
+		return 0;
+
+	max313xx_clk_init.name = rtc->chip->clkout_name;
+	device_property_read_string(dev, "clock-output-names",
+				    &max313xx_clk_init.name);
+	rtc->clkout.init = &max313xx_clk_init;
+
+	ret = devm_clk_hw_register(dev, &rtc->clkout);
+	if (ret)
+		return dev_err_probe(dev, ret, "cannot register clock\n");
+
+	return devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get,
+					   &rtc->clkout);
+}
+
+static int max313xx_trickle_charger_setup(struct device *dev)
+{
+	struct max313xx *rtc = dev_get_drvdata(dev);
+	bool trickle_diode_en;
+	int index, reg;
+	u32 ohms, chargeable;
+	int ret;
+	const char *diode;
+
+	if (device_property_read_u32(dev, "aux-voltage-chargeable",
+				     &chargeable))
+		return 0;
+
+	if (device_property_read_u32(dev, "trickle-resistor-ohms", &ohms))
+		return 0;
+
+	if (device_property_read_string(dev, "adi,tc-diode", &diode))
+		return 0;
+
+	if (!strcmp(diode, "schottky"))
+		trickle_diode_en = 0;
+	else if (!strcmp(diode, "standard+schottky"))
+		trickle_diode_en = 1;
+	else
+		return dev_err_probe(dev, -EINVAL,
+				     "Invalid tc-diode value: %s\n", diode);
+
+	if (!rtc->chip->trickle_reg)
+		return 0;
+
+	index = find_closest(ohms, max313xx_trickle_ohms,
+			     ARRAY_SIZE(max313xx_trickle_ohms)) + 1;
+
+	switch (rtc->id) {
+	case ID_MAX31329:
+		reg = FIELD_PREP(MAX31329_TRICKLE_ENABLE_BIT, chargeable) |
+		      FIELD_PREP(MAX31329_43_TRICKLE_RES_MASK, index) |
+		      FIELD_PREP(MAX31329_43_TRICKLE_DIODE_EN, trickle_diode_en);
+		break;
+	case ID_MAX31331:
+	case ID_MAX31334:
+		reg = FIELD_PREP(MAX3133X_TRICKLE_ENABLE_BIT, chargeable) |
+		      FIELD_PREP(MAX3133X_TRICKLE_RES_MASK, index) |
+		      FIELD_PREP(MAX3133X_TRICKLE_DIODE_EN, trickle_diode_en);
+		break;
+	case ID_MAX31341:
+		if (index == 1)
+			index = 0;
+
+		reg = FIELD_PREP(MAX31341_TRICKLE_ENABLE_BIT, chargeable) |
+		      FIELD_PREP(MAX31341_TRICKLE_RES_MASK, index) |
+		      FIELD_PREP(MAX31341_TRICKLE_DIODE_EN, trickle_diode_en);
+
+		ret = regmap_set_bits(rtc->regmap, MAX31341_POWER_MGMT_REG,
+				      MAX31341_POWER_MGMT_TRICKLE_BIT);
+		if (ret)
+			return ret;
+
+		break;
+	case ID_MAX31343:
+		reg = FIELD_PREP(MAX31329_43_TRICKLE_RES_MASK, index) |
+		      FIELD_PREP(MAX31329_43_TRICKLE_DIODE_EN, trickle_diode_en) |
+		      FIELD_PREP(MAX31343_TRICKLE_ENABLE_MASK,
+				 MAX31343_TRICKLE_ENABLE_CODE);
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return regmap_write(rtc->regmap, rtc->chip->trickle_reg, reg);
+}
+
+static int max313xx_find_clkin_freq_index(struct clk *clk)
+{
+	unsigned long rate = clk_get_rate(clk);
+	int freq;
+	int i;
+
+	i = find_closest(rate, max313xx_clkin_freq,
+			 ARRAY_SIZE(max313xx_clkin_freq));
+	if (max313xx_clkin_freq[i] == rate)
+		return i;
+
+	for (i = ARRAY_SIZE(max313xx_clkin_freq) - 1; i >= 0; i--) {
+		freq = max313xx_clkin_freq[i];
+		if (freq == clk_round_rate(clk, freq))
+			return i;
+	}
+
+	/* supplied clock cannot produce one of desired frequency rate */
+	return -ENODEV;
+}
+
+static int max313xx_clkin_init(struct device *dev)
+{
+	struct max313xx *rtc = dev_get_drvdata(dev);
+	int rate;
+	int ret;
+
+	rtc->clkin = devm_clk_get_enabled(dev, NULL);
+	if (IS_ERR(rtc->clkin)) {
+		if (PTR_ERR(rtc->clkin) == -ENOENT)
+			rtc->clkin = NULL;
+		else
+			return dev_err_probe(dev, PTR_ERR(rtc->clkin),
+					     "error while clkin setup\n");
+	}
+
+	if (!rtc->clkin) {
+		switch (rtc->id) {
+		case ID_MAX31329:
+			return regmap_clear_bits(rtc->regmap,
+						 MAX31329_CONFIG2_REG,
+						 MAX31329_CONFIG2_CLKIN_EN);
+		case ID_MAX31341:
+		case ID_MAX31342:
+			return regmap_clear_bits(rtc->regmap,
+						 MAX31341_42_CONFIG1_REG,
+						 MAX31341_42_CONFIG1_CLKIN_EN);
+		default:
+			return 0;
+		}
+	}
+
+	rate = max313xx_find_clkin_freq_index(rtc->clkin);
+	if (rate < 0)
+		return dev_err_probe(dev, rate,
+				     "clkin cannot produce required frequency\n");
+
+	ret = clk_set_rate(rtc->clkin, max313xx_clkin_freq[rate]);
+	if (ret)
+		return ret;
+
+	switch (rtc->id) {
+	case ID_MAX31329:
+		ret = regmap_update_bits(rtc->regmap, MAX31329_CONFIG2_REG,
+					 MAX31329_CONFIG2_CLKIN_FREQ, rate);
+		if (ret)
+			return ret;
+
+		return regmap_set_bits(rtc->regmap, MAX31329_CONFIG2_REG,
+				       MAX31329_CONFIG2_CLKIN_EN);
+	case ID_MAX31341:
+	case ID_MAX31342:
+		ret = regmap_update_bits(rtc->regmap, MAX31341_42_CONFIG1_REG,
+					 MAX31341_42_CONFIG1_CLKIN_FREQ,
+					 FIELD_PREP(MAX31341_42_CONFIG1_CLKIN_FREQ, rate));
+		if (ret)
+			return ret;
+
+		return regmap_set_bits(rtc->regmap, MAX31341_42_CONFIG1_REG,
+				       MAX31341_42_CONFIG1_CLKIN_EN);
+	default:
+		rtc->clkin = NULL;
+		dev_warn(dev, "device does not have clock input\n");
+		return 0;
+	}
+}
+
+static int max313xx_irq_init(struct device *dev, const char *devname)
+{
+	struct max313xx *rtc = dev_get_drvdata(dev);
+	int ret;
+
+	switch (rtc->id) {
+	case ID_MAX31328:
+		break;
+	case ID_MAX31331:
+	case ID_MAX31334:
+		if (rtc->clkout.clk) {
+			/*
+			 * interrupt muxing depends on clkout so enable clkout
+			 * if configured before requesting interrupt
+			 */
+			ret = clk_prepare_enable(rtc->clkout.clk);
+			if (ret)
+				return dev_err_probe(dev, ret,
+						     "cannot enable clkout\n");
+		}
+		break;
+	default:
+		if (rtc->clkin) {
+			if (rtc->clkout.clk && rtc->irq > 0)
+				return dev_err_probe(dev, -EOPNOTSUPP,
+						     "irq not possible when both clkin and clkout are configured\n");
+
+			if (rtc->irq <= 0)
+				break;
+
+			/* clkout needs to be disabled for interrupt */
+			if (rtc->chip->clkout->en_invert)
+				ret = regmap_set_bits(rtc->regmap,
+						      rtc->chip->clkout->reg,
+						      rtc->chip->clkout->en_bit);
+			else
+				ret = regmap_clear_bits(rtc->regmap,
+							rtc->chip->clkout->reg,
+							rtc->chip->clkout->en_bit);
+
+			if (ret)
+				return ret;
+		}
+		break;
+	}
+
+	if (rtc->irq > 0) {
+		return devm_request_threaded_irq(dev, rtc->irq, NULL,
+						 &max313xx_irq, IRQF_ONESHOT,
+						 devname, rtc);
+	} else if (device_property_read_bool(dev, "wakeup-source")) {
+		clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->rtc->features);
+		return device_init_wakeup(dev, true);
+	}
+
+	clear_bit(RTC_FEATURE_ALARM, rtc->rtc->features);
+
+	return 0;
+}
+
+static int max313xx_probe(struct i2c_client *client)
+{
+	const struct i2c_device_id *id = i2c_client_get_device_id(client);
+	struct device *dev = &client->dev;
+	struct max313xx *max313xx;
+#if IS_REACHABLE(HWMON)
+	struct device *hwmon;
+#endif
+	const void *match;
+	int ret;
+
+	max313xx = devm_kzalloc(&client->dev, sizeof(*max313xx), GFP_KERNEL);
+	if (!max313xx)
+		return -ENOMEM;
+
+	dev_set_drvdata(&client->dev, max313xx);
+
+	max313xx->regmap = devm_regmap_init_i2c(client, &regmap_config);
+	if (IS_ERR(max313xx->regmap)) {
+		return dev_err_probe(dev, PTR_ERR(max313xx->regmap),
+				     "regmap init failed\n");
+	}
+
+	i2c_set_clientdata(client, max313xx);
+
+	match = device_get_match_data(dev);
+	if (match)
+		max313xx->chip = match;
+	else if (id)
+		max313xx->chip = (struct chip_desc *)id->driver_data;
+	else
+		return -ENODEV;
+
+	max313xx->id = max313xx->chip - chip;
+
+	ret = max313xx_init(max313xx);
+	if (ret)
+		return ret;
+
+	max313xx->rtc = devm_rtc_allocate_device(dev);
+	if (IS_ERR(max313xx->rtc))
+		return PTR_ERR(max313xx->rtc);
+
+	max313xx->rtc->ops = &max3133x_rtc_ops;
+	max313xx->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
+	max313xx->rtc->range_max = RTC_TIMESTAMP_END_2199;
+
+	ret = devm_rtc_register_device(max313xx->rtc);
+	if (ret)
+		return ret;
+
+	max313xx->irq = client->irq;
+
+	ret = max313xx_clkout_register(dev);
+	if (ret)
+		return ret;
+
+	ret = max313xx_clkin_init(dev);
+	if (ret)
+		return ret;
+
+	/* IRQ wiring depends on the clock configuration so parse them first */
+	ret = max313xx_irq_init(dev, client->name);
+	if (ret)
+		return ret;
+
+	if (max313xx->chip->ram_size) {
+		max313xx_nvmem_cfg.size = max313xx->chip->ram_size;
+		max313xx_nvmem_cfg.priv = max313xx;
+
+		ret = devm_rtc_nvmem_register(max313xx->rtc, &max313xx_nvmem_cfg);
+		if (ret)
+			dev_warn(dev, "cannot register rtc nvmem\n");
+	}
+
+#if IS_REACHABLE(HWMON)
+	if (max313xx->chip->temp_reg) {
+		hwmon = devm_hwmon_device_register_with_info(dev, client->name,
+							     max313xx,
+							     &max313xx_chip_info,
+							     NULL);
+		if (IS_ERR(hwmon))
+			dev_warn(dev, "cannot register hwmon device: %li\n",
+				 PTR_ERR(hwmon));
+	}
+#endif
+
+	return max313xx_trickle_charger_setup(dev);
+}
+
+static const struct of_device_id max313xx_of_id[] = {
+	{ .compatible = "adi,max31328", .data = &chip[ID_MAX31328] },
+	{ .compatible = "adi,max31329", .data = &chip[ID_MAX31329] },
+	{ .compatible = "adi,max31331", .data = &chip[ID_MAX31331] },
+	{ .compatible = "adi,max31334", .data = &chip[ID_MAX31334] },
+	{ .compatible = "adi,max31341", .data = &chip[ID_MAX31341] },
+	{ .compatible = "adi,max31342", .data = &chip[ID_MAX31342] },
+	{ .compatible = "adi,max31343", .data = &chip[ID_MAX31343] },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, max313xx_of_id);
+
+static const struct i2c_device_id max313xx_id[] = {
+	{ "max31328", (kernel_ulong_t)&chip[ID_MAX31328] },
+	{ "max31329", (kernel_ulong_t)&chip[ID_MAX31329] },
+	{ "max31331", (kernel_ulong_t)&chip[ID_MAX31331] },
+	{ "max31334", (kernel_ulong_t)&chip[ID_MAX31334] },
+	{ "max31341", (kernel_ulong_t)&chip[ID_MAX31341] },
+	{ "max31342", (kernel_ulong_t)&chip[ID_MAX31342] },
+	{ "max31343", (kernel_ulong_t)&chip[ID_MAX31343] },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, max313xx_id);
+
+static struct i2c_driver max313xx_driver = {
+	.driver = {
+		.name	= "rtc-max313xx",
+		.of_match_table = max313xx_of_id,
+	},
+	.probe		= max313xx_probe,
+	.id_table	= max313xx_id,
+};
+module_i2c_driver(max313xx_driver);
+
+MODULE_DESCRIPTION("Analog Devices MAX313XX RTCs");
+MODULE_AUTHOR("Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>");
+MODULE_AUTHOR("Ibrahim Tilki <Ibrahim.Tilki@analog.com>");
+MODULE_SOFTDEP("pre: regmap-i2c");
+MODULE_LICENSE("GPL");
+MODULE_VERSION("1.0");
-- 
2.43.0


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

* [PATCH v6 2/2] dt-bindings: rtc: add max313xx RTCs
  2024-02-02  2:52 [PATCH v6 0/2] drivers: rtc: add max313xx series rtc driver Chris Packham
  2024-02-02  2:52 ` [PATCH v6 1/2] " Chris Packham
@ 2024-02-02  2:52 ` Chris Packham
  2024-02-02  8:28   ` Krzysztof Kozlowski
  1 sibling, 1 reply; 12+ messages in thread
From: Chris Packham @ 2024-02-02  2:52 UTC (permalink / raw)
  To: alexandre.belloni, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	jdelvare, linux, antoniu.miclaus, noname.nuno
  Cc: linux-rtc, devicetree, linux-kernel, linux-hwmon, Ibrahim Tilki,
	Zeynep Arslanbenzer, Chris Packham

From: Ibrahim Tilki <Ibrahim.Tilki@analog.com>

Devicetree binding documentation for Analog Devices MAX313XX RTCs

Signed-off-by: Ibrahim Tilki <Ibrahim.Tilki@analog.com>
Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 .../devicetree/bindings/rtc/adi,max313xx.yaml | 145 ++++++++++++++++++
 1 file changed, 145 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/adi,max313xx.yaml

diff --git a/Documentation/devicetree/bindings/rtc/adi,max313xx.yaml b/Documentation/devicetree/bindings/rtc/adi,max313xx.yaml
new file mode 100644
index 000000000000..ccfb0cbfb045
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/adi,max313xx.yaml
@@ -0,0 +1,145 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2022 Analog Devices Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/adi,max313xx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices MAX313XX series I2C RTCs
+
+maintainers:
+  - Chris Packham <chris.packham@alliedtelesis.co.nz>
+
+description: Analog Devices MAX313XX series I2C RTCs.
+
+properties:
+  compatible:
+    enum:
+      - adi,max31328
+      - adi,max31329
+      - adi,max31331
+      - adi,max31334
+      - adi,max31341
+      - adi,max31342
+      - adi,max31343
+
+  reg:
+    description: I2C address of the RTC
+    items:
+      - enum: [0x68, 0x69]
+
+  interrupts:
+    description:
+      Alarm1 interrupt line of the RTC. Some of the RTCs have two interrupt
+      lines and alarm1 interrupt muxing depends on the clockin/clockout
+      configuration.
+    maxItems: 1
+
+  "#clock-cells":
+    description:
+      RTC can be used as a clock source through its clock output pin when
+      supplied.
+    const: 0
+
+  clocks:
+    description:
+      RTC uses this clock for clock input when supplied. Clock has to provide
+      one of these four frequencies - 1Hz, 50Hz, 60Hz or 32.768kHz.
+    maxItems: 1
+
+  adi,tc-diode:
+    description:
+      Select the diode configuration for the trickle charger.
+      schottky - Schottky diode in series.
+      standard+schottky - standard diode + Schottky diode in series.
+    enum: [schottky, standard+schottky]
+
+  trickle-resistor-ohms:
+    description: Selected resistor for trickle charger.
+    enum: [3000, 6000, 11000]
+
+required:
+  - compatible
+  - reg
+
+allOf:
+  - $ref: rtc.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - adi,max31328
+              - adi,max31342
+
+    then:
+      properties:
+        aux-voltage-chargeable: false
+        trickle-resistor-ohms: false
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - adi,max31328
+              - adi,max31331
+              - adi,max31334
+              - adi,max31343
+
+    then:
+      properties:
+        clocks: false
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - adi,max31341
+              - adi,max31342
+
+    then:
+      properties:
+        reg:
+          items:
+            - const: 0x69
+
+    else:
+      properties:
+        reg:
+          items:
+            - const: 0x68
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        rtc@68 {
+            reg = <0x68>;
+            compatible = "adi,max31329";
+            clocks = <&clkin>;
+            interrupt-parent = <&gpio>;
+            interrupts = <26 IRQ_TYPE_EDGE_FALLING>;
+            aux-voltage-chargeable = <1>;
+            trickle-resistor-ohms = <6000>;
+            adi,tc-diode = "schottky";
+        };
+    };
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        rtc@68 {
+            reg = <0x68>;
+            compatible = "adi,max31331";
+            #clock-cells = <0>;
+        };
+    };
-- 
2.43.0


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

* Re: [PATCH v6 2/2] dt-bindings: rtc: add max313xx RTCs
  2024-02-02  2:52 ` [PATCH v6 2/2] dt-bindings: rtc: add max313xx RTCs Chris Packham
@ 2024-02-02  8:28   ` Krzysztof Kozlowski
  2024-02-06 20:21     ` Chris Packham
       [not found]     ` <5d4b7fa1-5cc2-4a4a-8fa4-d2c7a8d070b7@alliedtelesis.co.nz>
  0 siblings, 2 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2024-02-02  8:28 UTC (permalink / raw)
  To: Chris Packham, alexandre.belloni, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, jdelvare, linux,
	antoniu.miclaus, noname.nuno
  Cc: linux-rtc, devicetree, linux-kernel, linux-hwmon, Ibrahim Tilki,
	Zeynep Arslanbenzer

On 02/02/2024 03:52, Chris Packham wrote:
> From: Ibrahim Tilki <Ibrahim.Tilki@analog.com>
> 
> Devicetree binding documentation for Analog Devices MAX313XX RTCs
> 
> Signed-off-by: Ibrahim Tilki <Ibrahim.Tilki@analog.com>
> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---

...

> +  clocks:
> +    description:
> +      RTC uses this clock for clock input when supplied. Clock has to provide
> +      one of these four frequencies - 1Hz, 50Hz, 60Hz or 32.768kHz.
> +    maxItems: 1
> +
> +  adi,tc-diode:
> +    description:
> +      Select the diode configuration for the trickle charger.
> +      schottky - Schottky diode in series.
> +      standard+schottky - standard diode + Schottky diode in series.
> +    enum: [schottky, standard+schottky]
> +
> +  trickle-resistor-ohms:
> +    description: Selected resistor for trickle charger.
> +    enum: [3000, 6000, 11000]

Please remind us and document in commit msg, why this cannot be part of
max31335 binding? Looks exactly the same.

> +
> +required:
> +  - compatible
> +  - reg



Best regards,
Krzysztof


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

* Re: [PATCH v6 1/2] drivers: rtc: add max313xx series rtc driver
  2024-02-02  2:52 ` [PATCH v6 1/2] " Chris Packham
@ 2024-02-03  7:07   ` kernel test robot
  2024-02-04  2:25   ` kernel test robot
  1 sibling, 0 replies; 12+ messages in thread
From: kernel test robot @ 2024-02-03  7:07 UTC (permalink / raw)
  To: Chris Packham, alexandre.belloni, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, jdelvare, linux,
	antoniu.miclaus, noname.nuno
  Cc: oe-kbuild-all, linux-rtc, devicetree, linux-kernel, linux-hwmon,
	Ibrahim Tilki, Zeynep Arslanbenzer, Chris Packham

Hi Chris,

kernel test robot noticed the following build warnings:

[auto build test WARNING on abelloni/rtc-next]
[also build test WARNING on robh/for-next linus/master v6.8-rc2 next-20240202]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Chris-Packham/drivers-rtc-add-max313xx-series-rtc-driver/20240202-105538
base:   https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git rtc-next
patch link:    https://lore.kernel.org/r/20240202025241.834283-2-chris.packham%40alliedtelesis.co.nz
patch subject: [PATCH v6 1/2] drivers: rtc: add max313xx series rtc driver
config: i386-randconfig-063-20240203 (https://download.01.org/0day-ci/archive/20240203/202402031414.zapkL312-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240203/202402031414.zapkL312-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202402031414.zapkL312-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/rtc/rtc-max313xx.c:546:21: sparse: sparse: symbol 'max313xx_nvmem_cfg' was not declared. Should it be static?
>> drivers/rtc/rtc-max313xx.c:656:22: sparse: sparse: symbol 'max313xx_clk_init' was not declared. Should it be static?

vim +/max313xx_nvmem_cfg +546 drivers/rtc/rtc-max313xx.c

   545	
 > 546	struct nvmem_config max313xx_nvmem_cfg = {
   547		.reg_read = max313xx_nvmem_reg_read,
   548		.reg_write = max313xx_nvmem_reg_write,
   549		.word_size = 8,
   550	};
   551	
   552	static unsigned long max313xx_clkout_recalc_rate(struct clk_hw *hw,
   553							 unsigned long parent_rate)
   554	{
   555		struct max313xx *rtc = clk_hw_to_max313xx(hw);
   556		const struct clkout_cfg *clkout = rtc->chip->clkout;
   557		unsigned int freq_mask;
   558		unsigned int reg;
   559		int ret;
   560	
   561		ret = regmap_read(rtc->regmap, clkout->reg, &reg);
   562		if (ret)
   563			return 0;
   564	
   565		freq_mask = __roundup_pow_of_two(clkout->freq_size) - 1;
   566	
   567		return clkout->freq_avail[(reg >> clkout->freq_pos) & freq_mask];
   568	}
   569	
   570	static long max313xx_clkout_round_rate(struct clk_hw *hw, unsigned long rate,
   571					       unsigned long *prate)
   572	{
   573		struct max313xx *rtc = clk_hw_to_max313xx(hw);
   574		struct clkout_cfg *clkout = rtc->chip->clkout;
   575		int index;
   576	
   577		index = find_closest(rate, clkout->freq_avail, clkout->freq_size);
   578		return clkout->freq_avail[index];
   579	}
   580	
   581	static int max313xx_clkout_set_rate(struct clk_hw *hw, unsigned long rate,
   582					    unsigned long parent_rate)
   583	{
   584		struct max313xx *rtc = clk_hw_to_max313xx(hw);
   585		struct clkout_cfg *clkout = rtc->chip->clkout;
   586		unsigned int freq_mask;
   587		int index;
   588	
   589		index = find_closest(rate, clkout->freq_avail, clkout->freq_size);
   590		freq_mask = __roundup_pow_of_two(clkout->freq_size) - 1;
   591	
   592		return regmap_update_bits(rtc->regmap, clkout->reg,
   593					  freq_mask << clkout->freq_pos,
   594					  index << clkout->freq_pos);
   595	}
   596	
   597	static int max313xx_clkout_enable(struct clk_hw *hw)
   598	{
   599		struct max313xx *rtc = clk_hw_to_max313xx(hw);
   600		struct clkout_cfg *clkout = rtc->chip->clkout;
   601	
   602		if (clkout->en_invert)
   603			return regmap_clear_bits(rtc->regmap, clkout->reg,
   604						 clkout->en_bit);
   605	
   606		return regmap_set_bits(rtc->regmap, clkout->reg,  clkout->en_bit);
   607	}
   608	
   609	static void max313xx_clkout_disable(struct clk_hw *hw)
   610	{
   611		struct max313xx *rtc = clk_hw_to_max313xx(hw);
   612		struct clkout_cfg *clkout = rtc->chip->clkout;
   613	
   614		switch (rtc->id) {
   615		case ID_MAX31331:
   616		case ID_MAX31334:
   617			if (rtc->irq > 0) {
   618				dev_err(rtc->rtc->dev.parent,
   619					"clkout cannot be disabled when IRQ is requested");
   620				return;
   621			}
   622			break;
   623		default:
   624			break;
   625		}
   626	
   627		if (clkout->en_invert)
   628			regmap_set_bits(rtc->regmap, clkout->reg, clkout->en_bit);
   629		else
   630			regmap_clear_bits(rtc->regmap, clkout->reg,  clkout->en_bit);
   631	}
   632	
   633	static int max313xx_clkout_is_enabled(struct clk_hw *hw)
   634	{
   635		struct max313xx *rtc = clk_hw_to_max313xx(hw);
   636		struct clkout_cfg *clkout = rtc->chip->clkout;
   637		unsigned int reg;
   638		int ret;
   639	
   640		ret = regmap_read(rtc->regmap, clkout->reg, &reg);
   641		if (ret)
   642			return ret;
   643	
   644		return !!(reg & clkout->en_bit) ^ clkout->en_invert;
   645	}
   646	
   647	static const struct clk_ops max313xx_clkout_ops = {
   648		.recalc_rate = max313xx_clkout_recalc_rate,
   649		.round_rate = max313xx_clkout_round_rate,
   650		.set_rate = max313xx_clkout_set_rate,
   651		.enable = max313xx_clkout_enable,
   652		.disable = max313xx_clkout_disable,
   653		.is_enabled = max313xx_clkout_is_enabled,
   654	};
   655	
 > 656	struct clk_init_data max313xx_clk_init = {
   657		.name = "max313xx-clkout",
   658		.ops = &max313xx_clkout_ops,
   659	};
   660	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* Re: [PATCH v6 1/2] drivers: rtc: add max313xx series rtc driver
  2024-02-02  2:52 ` [PATCH v6 1/2] " Chris Packham
  2024-02-03  7:07   ` kernel test robot
@ 2024-02-04  2:25   ` kernel test robot
  1 sibling, 0 replies; 12+ messages in thread
From: kernel test robot @ 2024-02-04  2:25 UTC (permalink / raw)
  To: Chris Packham, alexandre.belloni, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, jdelvare, linux,
	antoniu.miclaus, noname.nuno
  Cc: oe-kbuild-all, linux-rtc, devicetree, linux-kernel, linux-hwmon,
	Ibrahim Tilki, Zeynep Arslanbenzer, Chris Packham

Hi Chris,

kernel test robot noticed the following build errors:

[auto build test ERROR on abelloni/rtc-next]
[also build test ERROR on robh/for-next linus/master v6.8-rc2 next-20240202]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Chris-Packham/drivers-rtc-add-max313xx-series-rtc-driver/20240202-105538
base:   https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git rtc-next
patch link:    https://lore.kernel.org/r/20240202025241.834283-2-chris.packham%40alliedtelesis.co.nz
patch subject: [PATCH v6 1/2] drivers: rtc: add max313xx series rtc driver
config: parisc-randconfig-r051-20240204 (https://download.01.org/0day-ci/archive/20240204/202402041027.jyioXP7e-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240204/202402041027.jyioXP7e-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202402041027.jyioXP7e-lkp@intel.com/

All errors (new ones prefixed by >>):

   hppa-linux-ld: drivers/rtc/rtc-max313xx.o: in function `max313xx_probe':
>> (.text+0x16b0): undefined reference to `devm_clk_hw_register'
>> hppa-linux-ld: (.text+0x16c8): undefined reference to `devm_of_clk_add_hw_provider'
   hppa-linux-ld: drivers/rtc/rtc-max313xx.o: in function `.LC19':
>> (.rodata.cst4+0x4): undefined reference to `of_clk_hw_simple_get'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* Re: [PATCH v6 2/2] dt-bindings: rtc: add max313xx RTCs
  2024-02-02  8:28   ` Krzysztof Kozlowski
@ 2024-02-06 20:21     ` Chris Packham
       [not found]     ` <5d4b7fa1-5cc2-4a4a-8fa4-d2c7a8d070b7@alliedtelesis.co.nz>
  1 sibling, 0 replies; 12+ messages in thread
From: Chris Packham @ 2024-02-06 20:21 UTC (permalink / raw)
  To: Krzysztof Kozlowski, alexandre.belloni, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, jdelvare, linux,
	antoniu.miclaus, noname.nuno
  Cc: linux-rtc, devicetree, linux-kernel, linux-hwmon, Ibrahim Tilki,
	Zeynep Arslanbenzer

(resend as plain text, sorry for the noise)


On 2/02/24 21:28, Krzysztof Kozlowski wrote:
> On 02/02/2024 03:52, Chris Packham wrote:
>> From: Ibrahim Tilki<Ibrahim.Tilki@analog.com>
>>
>> Devicetree binding documentation for Analog Devices MAX313XX RTCs
>>
>> Signed-off-by: Ibrahim Tilki<Ibrahim.Tilki@analog.com>
>> Signed-off-by: Zeynep Arslanbenzer<Zeynep.Arslanbenzer@analog.com>
>> Signed-off-by: Chris Packham<chris.packham@alliedtelesis.co.nz>
>> ---
> ...
>
>> +  clocks:
>> +    description:
>> +      RTC uses this clock for clock input when supplied. Clock has to provide
>> +      one of these four frequencies - 1Hz, 50Hz, 60Hz or 32.768kHz.
>> +    maxItems: 1
>> +
>> +  adi,tc-diode:
>> +    description:
>> +      Select the diode configuration for the trickle charger.
>> +      schottky - Schottky diode in series.
>> +      standard+schottky - standard diode + Schottky diode in series.
>> +    enum: [schottky, standard+schottky]
>> +
>> +  trickle-resistor-ohms:
>> +    description: Selected resistor for trickle charger.
>> +    enum: [3000, 6000, 11000]
> Please remind us and document in commit msg, why this cannot be part of
> max31335 binding? Looks exactly the same.

That is an incredibly good point. The max31335 binding covers one 
specific chip. This binding covers more and with that there are a few 
more properties that the max31335 on it's own doesn't have (e.g. the 
clock consumer, the ability to have different i2c addresses). Binding 
wise I could probably roll all of the max31335 into this max313xx binding.

Driver wise things are a bit trickier. I've only got access to one of 
the variants so I am hoping to leverage the work Ibrahim had already 
done. I could attempt to incorporate max31335 support into the max313xx 
driver but I wouldn't really be able to test it properly and there is a 
reasonably high chance of regressing something.

>> +
>> +required:
>> +  - compatible
>> +  - reg
> Best regards,
> Krzysztof
>

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

* Re: [PATCH v6 2/2] dt-bindings: rtc: add max313xx RTCs
       [not found]     ` <5d4b7fa1-5cc2-4a4a-8fa4-d2c7a8d070b7@alliedtelesis.co.nz>
@ 2024-02-06 21:12       ` Alexandre Belloni
  2024-02-06 21:41         ` Chris Packham
  0 siblings, 1 reply; 12+ messages in thread
From: Alexandre Belloni @ 2024-02-06 21:12 UTC (permalink / raw)
  To: Chris Packham
  Cc: Krzysztof Kozlowski, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	jdelvare, linux, antoniu.miclaus, noname.nuno, linux-rtc,
	devicetree, linux-kernel, linux-hwmon, Ibrahim Tilki,
	Zeynep Arslanbenzer

On 06/02/2024 20:19:20+0000, Chris Packham wrote:
> That is an incredibly good point. The max31335 binding covers one specific chip. This binding covers more and with that there are a few more properties that the max31335 on it's own doesn't have (e.g. the clock consumer, the ability to have different i2c addresses). Binding wise I could probably roll all of the max31335 into this max313xx binding.
> 
> Driver wise things are a bit trickier. I've only got access to one of
> the variants so I am hoping to leverage the work Ibrahim had already
> done. I could attempt to incorporate max31335 support into the
> max313xx driver but I wouldn't really be able to test it properly and
> there is a reasonably high chance of regressing something.

But I won't take a separate driver. Everything would be better if Analog
was sharing the datasheets...


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

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

* Re: [PATCH v6 2/2] dt-bindings: rtc: add max313xx RTCs
  2024-02-06 21:12       ` Alexandre Belloni
@ 2024-02-06 21:41         ` Chris Packham
  2024-02-06 22:16           ` Alexandre Belloni
  0 siblings, 1 reply; 12+ messages in thread
From: Chris Packham @ 2024-02-06 21:41 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: Krzysztof Kozlowski, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	jdelvare, linux, antoniu.miclaus, noname.nuno, linux-rtc,
	devicetree, linux-kernel, linux-hwmon, Ibrahim Tilki,
	Zeynep Arslanbenzer


On 7/02/24 10:12, Alexandre Belloni wrote:
> On 06/02/2024 20:19:20+0000, Chris Packham wrote:
>> That is an incredibly good point. The max31335 binding covers one specific chip. This binding covers more and with that there are a few more properties that the max31335 on it's own doesn't have (e.g. the clock consumer, the ability to have different i2c addresses). Binding wise I could probably roll all of the max31335 into this max313xx binding.
>>
>> Driver wise things are a bit trickier. I've only got access to one of
>> the variants so I am hoping to leverage the work Ibrahim had already
>> done. I could attempt to incorporate max31335 support into the
>> max313xx driver but I wouldn't really be able to test it properly and
>> there is a reasonably high chance of regressing something.
> But I won't take a separate driver. Everything would be better if Analog
> was sharing the datasheets...

The datasheets are pretty accessible so I'd give Analog a pass on that 
(they're certainly better than some vendors). I'll include some links on 
the next update.

I'll attempt to roll the max31335 into the max313xx driver.

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

* Re: [PATCH v6 2/2] dt-bindings: rtc: add max313xx RTCs
  2024-02-06 21:41         ` Chris Packham
@ 2024-02-06 22:16           ` Alexandre Belloni
  2024-02-07  9:54             ` Miclaus, Antoniu
  0 siblings, 1 reply; 12+ messages in thread
From: Alexandre Belloni @ 2024-02-06 22:16 UTC (permalink / raw)
  To: Chris Packham
  Cc: Krzysztof Kozlowski, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	jdelvare, linux, antoniu.miclaus, noname.nuno, linux-rtc,
	devicetree, linux-kernel, linux-hwmon, Ibrahim Tilki,
	Zeynep Arslanbenzer

On 06/02/2024 21:41:10+0000, Chris Packham wrote:
> 
> On 7/02/24 10:12, Alexandre Belloni wrote:
> > On 06/02/2024 20:19:20+0000, Chris Packham wrote:
> >> That is an incredibly good point. The max31335 binding covers one specific chip. This binding covers more and with that there are a few more properties that the max31335 on it's own doesn't have (e.g. the clock consumer, the ability to have different i2c addresses). Binding wise I could probably roll all of the max31335 into this max313xx binding.
> >>
> >> Driver wise things are a bit trickier. I've only got access to one of
> >> the variants so I am hoping to leverage the work Ibrahim had already
> >> done. I could attempt to incorporate max31335 support into the
> >> max313xx driver but I wouldn't really be able to test it properly and
> >> there is a reasonably high chance of regressing something.
> > But I won't take a separate driver. Everything would be better if Analog
> > was sharing the datasheets...
> 
> The datasheets are pretty accessible so I'd give Analog a pass on that 
> (they're certainly better than some vendors). I'll include some links on 
> the next update.
> 

The max31335 is not available

> I'll attempt to roll the max31335 into the max313xx driver.

I guess this would be the opposite. Renaming a driver breaks existing
kernel configurations.

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

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

* RE: [PATCH v6 2/2] dt-bindings: rtc: add max313xx RTCs
  2024-02-06 22:16           ` Alexandre Belloni
@ 2024-02-07  9:54             ` Miclaus, Antoniu
  2024-02-07 15:00               ` Guenter Roeck
  0 siblings, 1 reply; 12+ messages in thread
From: Miclaus, Antoniu @ 2024-02-07  9:54 UTC (permalink / raw)
  To: Alexandre Belloni, Chris Packham
  Cc: Krzysztof Kozlowski, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	jdelvare, linux, noname.nuno, linux-rtc, devicetree,
	linux-kernel, linux-hwmon, Ibrahim Tilki, Arslanbenzer, Zeynep

> On 06/02/2024 21:41:10+0000, Chris Packham wrote:
> >
> > On 7/02/24 10:12, Alexandre Belloni wrote:
> > > On 06/02/2024 20:19:20+0000, Chris Packham wrote:
> > >> That is an incredibly good point. The max31335 binding covers one
> specific chip. This binding covers more and with that there are a few more
> properties that the max31335 on it's own doesn't have (e.g. the clock
> consumer, the ability to have different i2c addresses). Binding wise I could
> probably roll all of the max31335 into this max313xx binding.
> > >>
> > >> Driver wise things are a bit trickier. I've only got access to one of
> > >> the variants so I am hoping to leverage the work Ibrahim had already
> > >> done. I could attempt to incorporate max31335 support into the
> > >> max313xx driver but I wouldn't really be able to test it properly and
> > >> there is a reasonably high chance of regressing something.
> > > But I won't take a separate driver. Everything would be better if Analog
> > > was sharing the datasheets...
> >
> > The datasheets are pretty accessible so I'd give Analog a pass on that
> > (they're certainly better than some vendors). I'll include some links on
> > the next update.
> >
> 
> The max31335 is not available
>
 
Indeed, the max31355 datasheet is not available yet. This is also stated in the
cover letter of the patch series for max31335.

It was an urgent request from a customer to have it mainline as soon as possible.

If there are questions about the part functionality, I can definitely help.

> > I'll attempt to roll the max31335 into the max313xx driver.
> 
> I guess this would be the opposite. Renaming a driver breaks existing
> kernel configurations.
> 
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://urldefense.com/v3/__https://bootlin.com__;!!A3Ni8CS0y2Y!7A0VM
> Nj1SLsNWZ8SnPrFwG6KX2TqjeLQi38EYIV164_5MWlQ7yCFvp8yOjaswIRNLC
> 0EK-_bhHEfy6xAXEJimVEW8BZXdn_r$

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

* Re: [PATCH v6 2/2] dt-bindings: rtc: add max313xx RTCs
  2024-02-07  9:54             ` Miclaus, Antoniu
@ 2024-02-07 15:00               ` Guenter Roeck
  0 siblings, 0 replies; 12+ messages in thread
From: Guenter Roeck @ 2024-02-07 15:00 UTC (permalink / raw)
  To: Miclaus, Antoniu, Alexandre Belloni, Chris Packham
  Cc: Krzysztof Kozlowski, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	jdelvare, noname.nuno, linux-rtc, devicetree, linux-kernel,
	linux-hwmon, Ibrahim Tilki, Arslanbenzer, Zeynep

On 2/7/24 01:54, Miclaus, Antoniu wrote:
[ ... ]
>>
>> The max31335 is not available
>>
>   
> Indeed, the max31355 datasheet is not available yet. This is also stated in the
> cover letter of the patch series for max31335.
> 
> It was an urgent request from a customer to have it mainline as soon as possible.
> 
> If there are questions about the part functionality, I can definitely help.
> 

It seems to be quite common for automotive chips, though, that they are held
tightly under wrap, making it all but impossible to properly review their drivers.
I have observed several times now that information not available to reviewers
resulted in bad or buggy drivers. This isn't about willingness to help, it is
about the ability to understand chip capabilities and requirements.

Guenter


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

end of thread, other threads:[~2024-02-07 15:00 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-02  2:52 [PATCH v6 0/2] drivers: rtc: add max313xx series rtc driver Chris Packham
2024-02-02  2:52 ` [PATCH v6 1/2] " Chris Packham
2024-02-03  7:07   ` kernel test robot
2024-02-04  2:25   ` kernel test robot
2024-02-02  2:52 ` [PATCH v6 2/2] dt-bindings: rtc: add max313xx RTCs Chris Packham
2024-02-02  8:28   ` Krzysztof Kozlowski
2024-02-06 20:21     ` Chris Packham
     [not found]     ` <5d4b7fa1-5cc2-4a4a-8fa4-d2c7a8d070b7@alliedtelesis.co.nz>
2024-02-06 21:12       ` Alexandre Belloni
2024-02-06 21:41         ` Chris Packham
2024-02-06 22:16           ` Alexandre Belloni
2024-02-07  9:54             ` Miclaus, Antoniu
2024-02-07 15:00               ` Guenter Roeck

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