* [PATCH v2 0/2] rtc: maxim: add support for Maxim max31329 RTC
@ 2022-09-12 19:07 Jagath Jog J
2022-09-12 19:07 ` [PATCH v2 1/2] dt-bindings: rtc: add Maxim max31329 rtc device tree bindings Jagath Jog J
2022-09-12 19:07 ` [PATCH v2 2/2] rtc: maxim: Add Maxim max31329 real time clock Jagath Jog J
0 siblings, 2 replies; 8+ messages in thread
From: Jagath Jog J @ 2022-09-12 19:07 UTC (permalink / raw)
To: alexandre.belloni, a.zummo, robh+dt, krzysztof.kozlowski+dt
Cc: linux-rtc, linux-kernel, devicetree, jagathjog1996
Add driver and device tree bindings support for Maxim max31329 real
time clock.
Changes since v1.
1. Move 'allOf' after 'maintainers' in DT.
2. Add 'wakeup-source' into device tree bindings.
3. Add wakeup support depending on the 'wakeup-source' parameter from DT.
4. Remove unused 'dev' member from device private data structure.
5. Use '__maybe_unused' attribute for of_device_id table.
6. Initialize local variable 'events' to 0 in interrupt handler.
Reported-by: kernel test robot <lkp@intel.com>
7. Add Reviewed-by tag for device tree bindings.
Jagath Jog J (2):
dt-bindings: rtc: add Maxim max31329 rtc device tree bindings
rtc: maxim: Add Maxim max31329 real time clock
.../bindings/rtc/maxim,max31329.yaml | 61 ++
MAINTAINERS | 7 +
drivers/rtc/Kconfig | 10 +
drivers/rtc/Makefile | 1 +
drivers/rtc/rtc-max31329.c | 535 ++++++++++++++++++
5 files changed, 614 insertions(+)
create mode 100644 Documentation/devicetree/bindings/rtc/maxim,max31329.yaml
create mode 100644 drivers/rtc/rtc-max31329.c
--
2.17.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/2] dt-bindings: rtc: add Maxim max31329 rtc device tree bindings
2022-09-12 19:07 [PATCH v2 0/2] rtc: maxim: add support for Maxim max31329 RTC Jagath Jog J
@ 2022-09-12 19:07 ` Jagath Jog J
2022-09-12 19:07 ` [PATCH v2 2/2] rtc: maxim: Add Maxim max31329 real time clock Jagath Jog J
1 sibling, 0 replies; 8+ messages in thread
From: Jagath Jog J @ 2022-09-12 19:07 UTC (permalink / raw)
To: alexandre.belloni, a.zummo, robh+dt, krzysztof.kozlowski+dt
Cc: linux-rtc, linux-kernel, devicetree, jagathjog1996
Document devicetree bindings for the Maxim max31329 real time clock.
Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
.../bindings/rtc/maxim,max31329.yaml | 61 +++++++++++++++++++
1 file changed, 61 insertions(+)
create mode 100644 Documentation/devicetree/bindings/rtc/maxim,max31329.yaml
diff --git a/Documentation/devicetree/bindings/rtc/maxim,max31329.yaml b/Documentation/devicetree/bindings/rtc/maxim,max31329.yaml
new file mode 100644
index 000000000000..fc99f1854847
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/maxim,max31329.yaml
@@ -0,0 +1,61 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/maxim,max31329.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Maxim MAX31329 Real Time Clock
+
+maintainers:
+ - Jagath Jog J <jagathjog1996@gmail.com>
+
+allOf:
+ - $ref: rtc.yaml#
+
+properties:
+ compatible:
+ const: maxim,max31329
+
+ reg:
+ maxItems: 1
+
+ "#clock-cells":
+ const: 0
+
+ clock-output-names:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ trickle-resistor-ohms:
+ enum:
+ - 3000
+ - 6000
+ - 11000
+
+ wakeup-source: true
+
+ start-year: true
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ rtc@68 {
+ compatible = "maxim,max31329";
+ reg = <0x68>;
+ trickle-resistor-ohms = <6000>;
+ #clock-cells = <0>;
+ };
+ };
+...
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] rtc: maxim: Add Maxim max31329 real time clock
2022-09-12 19:07 [PATCH v2 0/2] rtc: maxim: add support for Maxim max31329 RTC Jagath Jog J
2022-09-12 19:07 ` [PATCH v2 1/2] dt-bindings: rtc: add Maxim max31329 rtc device tree bindings Jagath Jog J
@ 2022-09-12 19:07 ` Jagath Jog J
2022-09-14 11:55 ` Alexandre Belloni
1 sibling, 1 reply; 8+ messages in thread
From: Jagath Jog J @ 2022-09-12 19:07 UTC (permalink / raw)
To: alexandre.belloni, a.zummo, robh+dt, krzysztof.kozlowski+dt
Cc: linux-rtc, linux-kernel, devicetree, jagathjog1996
Add support for I2C based Maxim max31329 real time clock.
Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
---
MAINTAINERS | 7 +
drivers/rtc/Kconfig | 10 +
drivers/rtc/Makefile | 1 +
drivers/rtc/rtc-max31329.c | 535 +++++++++++++++++++++++++++++++++++++
4 files changed, 553 insertions(+)
create mode 100644 drivers/rtc/rtc-max31329.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 7032fcb0fd0f..d92ddab0958d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12478,6 +12478,13 @@ F: include/linux/mfd/max14577*.h
F: include/linux/mfd/max77686*.h
F: include/linux/mfd/max77693*.h
+MAXIM MAX31329 RTC DRIVER
+M: Jagath Jog J <jagathjog1996@gmail.com>
+L: linux-rtc@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/rtc/maxim,max31329.yaml
+F: drivers/rtc/max31329.c
+
MAXIRADIO FM RADIO RECEIVER DRIVER
M: Hans Verkuil <hverkuil@xs4all.nl>
L: linux-media@vger.kernel.org
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index b8de25118ad0..d02cba94e121 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -393,6 +393,16 @@ config RTC_DRV_NCT3018Y
This driver can also be built as a module, if so, the module will be
called "rtc-nct3018y".
+config RTC_DRV_MAX31329
+ tristate "Maxim MAX31329"
+ select REGMAP_I2C
+ help
+ If you say yes here you will get support for the
+ RTC of Maxim MAX31329.
+
+ This driver can also be build as a module. If so, the module
+ will be called rtc-max31329.
+
config RTC_DRV_RK808
tristate "Rockchip RK805/RK808/RK809/RK817/RK818 RTC"
depends on MFD_RK808
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index aab22bc63432..741216d2f9ca 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -97,6 +97,7 @@ obj-$(CONFIG_RTC_DRV_MAX8907) += rtc-max8907.o
obj-$(CONFIG_RTC_DRV_MAX8925) += rtc-max8925.o
obj-$(CONFIG_RTC_DRV_MAX8997) += rtc-max8997.o
obj-$(CONFIG_RTC_DRV_MAX8998) += rtc-max8998.o
+obj-$(CONFIG_RTC_DRV_MAX31329) += rtc-max31329.o
obj-$(CONFIG_RTC_DRV_MESON_VRTC)+= rtc-meson-vrtc.o
obj-$(CONFIG_RTC_DRV_MC13XXX) += rtc-mc13xxx.o
obj-$(CONFIG_RTC_DRV_MCP795) += rtc-mcp795.o
diff --git a/drivers/rtc/rtc-max31329.c b/drivers/rtc/rtc-max31329.c
new file mode 100644
index 000000000000..9cee221c7576
--- /dev/null
+++ b/drivers/rtc/rtc-max31329.c
@@ -0,0 +1,535 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * RTC driver for the Maxim MAX31329 Real-Time Clock
+ * Copyright (c) 2022 Jagath Jog J
+ *
+ * Datasheet: https://datasheets.maximintegrated.com/en/ds/MAX31329.pdf
+ *
+ */
+
+#include <linux/bcd.h>
+#include <linux/bitfield.h>
+#include <linux/clk-provider.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/rtc.h>
+
+/* Register map */
+/* Config registers */
+#define MAX31329_STATUS_REG 0x00
+#define MAX31329_STATUS_A1F BIT(0)
+#define MAX31329_STATUS_OSF BIT(6)
+#define MAX31329_INT_EN_REG 0x01
+#define MAX31329_INT_EN_A1IE BIT(0)
+#define MAX31329_RTC_RESET_REG 0x02
+#define MAX31329_RTC_CONFIG1_REG 0x03
+#define MAX31329_RTC_CONFIG2_REG 0x04
+#define MAX31329_RTC_CONFIG2_ENCLKIN BIT(2)
+#define MAX31329_RTC_CONFIG2_ENCLKO BIT(7)
+#define MAX31329_RTC_CFG2_CLKOHZ_MSK GENMASK(6, 5)
+#define MAX31329_TIMER_CONFIG_REG 0x05
+
+/* Watch registers */
+#define MAX31329_SECONDS_REG 0x06
+#define MAX31329_MINUTES_REG 0x07
+#define MAX31329_HOURS_REG 0x08
+#define MAX31329_HOURS_F24_12 BIT(6)
+#define MAX31329_HOURS_AM_PM BIT(5)
+#define MAX31329_DAY_REG 0x09
+#define MAX31329_DATE_REG 0x0A
+#define MAX31329_MONTH_REG 0x0B
+#define MAX31329_MONTH_CENTURY BIT(7)
+#define MAX31329_YEAR_REG 0x0C
+#define MAX31329_WATCH_SEC_LEN 0x07
+#define REG_TO_OFFSET(_REG) ((_REG) - MAX31329_SECONDS_REG)
+
+/* Alarm registers */
+#define MAX31329_ALM1_SEC_REG 0x0D
+#define MAX31329_ALM1_MIN_REG 0x0E
+#define MAX31329_ALM1_HRS_REG 0x0F
+#define MAX31329_ALM1_DAY_DATE_REG 0x10
+#define MAX31329_ALM1_MON_REG 0x11
+#define MAX31329_ALM1_YEAR_REG 0x12
+#define MAX31329_ALM1_SEC_LEN 0x06
+
+#define MAX31329_PWR_MGMT_REG 0x18
+#define MAX31329_TRICKLE_REG 0x19
+#define MAX31329_TRICKLE_EN BIT(7)
+#define MAX31329_TRICKLE_DIODE_EN BIT(2)
+#define MAX31329_D_TRICKLE_OHMS GENMASK(3, 0)
+
+/* Ram registers */
+#define MAX31329_RAM0_START_REG 0x22
+#define MAX31329_RAM0_END_REG 0x61
+
+struct max31329_data {
+ struct regmap *regmap;
+ struct rtc_device *rtc;
+ int irq;
+#ifdef CONFIG_COMMON_CLK
+ struct clk_hw clkout_hw;
+#endif
+};
+
+/* resistance in kohms */
+static u32 max31329_trickle_ohms[] = {
+ 3000,
+ 6000,
+ 11000
+};
+
+static const struct regmap_config config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = MAX31329_RAM0_END_REG,
+};
+
+static int max31329_get_osc_status(struct device *dev)
+{
+ struct max31329_data *max31329 = dev_get_drvdata(dev);
+ unsigned int status;
+ int ret;
+
+ ret = regmap_read(max31329->regmap, MAX31329_STATUS_REG, &status);
+ if (ret)
+ return ret;
+
+ if (status & MAX31329_STATUS_OSF) {
+ dev_err(dev, "Oscillator has stopped\n");
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int max31329_alarm_irq_enable(struct device *dev, unsigned int enable)
+{
+ struct max31329_data *max31329 = dev_get_drvdata(dev);
+
+ return regmap_update_bits(max31329->regmap, MAX31329_INT_EN_REG,
+ MAX31329_INT_EN_A1IE,
+ enable ? MAX31329_INT_EN_A1IE : 0);
+}
+
+static int max31329_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
+{
+ struct max31329_data *max31329 = dev_get_drvdata(dev);
+ struct rtc_time *const tm = &alarm->time;
+ unsigned int aie_en, aie_flag;
+ int ret;
+ u8 regs[6];
+
+ ret = max31329_get_osc_status(dev);
+ if (ret)
+ return ret;
+
+ ret = regmap_bulk_read(max31329->regmap, MAX31329_ALM1_SEC_REG, regs,
+ MAX31329_ALM1_SEC_LEN);
+ if (ret)
+ return ret;
+
+ ret = regmap_read(max31329->regmap, MAX31329_INT_EN_REG, &aie_en);
+ if (ret)
+ return ret;
+
+ ret = regmap_read(max31329->regmap, MAX31329_STATUS_REG, &aie_flag);
+ if (ret)
+ return ret;
+
+ tm->tm_sec = bcd2bin(regs[REG_TO_OFFSET(MAX31329_SECONDS_REG)] & 0x7f);
+ tm->tm_min = bcd2bin(regs[REG_TO_OFFSET(MAX31329_MINUTES_REG)] & 0x7f);
+ tm->tm_hour = bcd2bin(regs[REG_TO_OFFSET(MAX31329_HOURS_REG)] & 0x3f);
+ tm->tm_mday = bcd2bin(regs[REG_TO_OFFSET(MAX31329_DATE_REG) - 1] & 0x3f);
+ tm->tm_mon = bcd2bin(regs[REG_TO_OFFSET(MAX31329_MONTH_REG) - 1] &
+ 0x1f) - 1;
+ tm->tm_year = bcd2bin(regs[REG_TO_OFFSET(MAX31329_YEAR_REG) - 1]) + 200;
+
+ alarm->enabled = FIELD_GET(MAX31329_INT_EN_A1IE, aie_en);
+ alarm->pending = FIELD_GET(MAX31329_STATUS_A1F, aie_flag) &&
+ alarm->enabled;
+
+ return 0;
+}
+
+static int max31329_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+ struct max31329_data *max31329 = dev_get_drvdata(dev);
+ const struct rtc_time *tm = &alrm->time;
+ u8 regs[6], year;
+ int ret;
+
+ ret = max31329_get_osc_status(dev);
+ if (ret)
+ return ret;
+
+ regs[REG_TO_OFFSET(MAX31329_SECONDS_REG)] = bin2bcd(tm->tm_sec) & 0x7F;
+ regs[REG_TO_OFFSET(MAX31329_MINUTES_REG)] = bin2bcd(tm->tm_min) & 0x7f;
+ regs[REG_TO_OFFSET(MAX31329_HOURS_REG)] = bin2bcd(tm->tm_hour) & 0x3f;
+ regs[REG_TO_OFFSET(MAX31329_DATE_REG) - 1] = bin2bcd(tm->tm_mday) & 0x3f;
+ regs[REG_TO_OFFSET(MAX31329_MONTH_REG) - 1] = bin2bcd(tm->tm_mon + 1) & 0x1f;
+
+ if (tm->tm_year >= 200)
+ year = bin2bcd(tm->tm_year - 200);
+ else
+ year = bin2bcd(tm->tm_year - 100);
+ regs[REG_TO_OFFSET(MAX31329_YEAR_REG) - 1] = year;
+
+ ret = regmap_bulk_write(max31329->regmap, MAX31329_ALM1_SEC_REG, regs,
+ MAX31329_ALM1_SEC_LEN);
+ if (ret)
+ return ret;
+
+ return max31329_alarm_irq_enable(dev, alrm->enabled);
+}
+
+static int max31329_read_time(struct device *dev, struct rtc_time *tm)
+{
+ struct max31329_data *max31329 = dev_get_drvdata(dev);
+ u8 data[7], century = 0;
+ int ret;
+
+ ret = max31329_get_osc_status(dev);
+ if (ret)
+ return ret;
+
+ ret = regmap_bulk_read(max31329->regmap, MAX31329_SECONDS_REG, data,
+ sizeof(data));
+ if (ret)
+ return ret;
+
+ tm->tm_sec = bcd2bin(data[REG_TO_OFFSET(MAX31329_SECONDS_REG)] & 0x7f);
+ tm->tm_min = bcd2bin(data[REG_TO_OFFSET(MAX31329_MINUTES_REG)] & 0x7f);
+ tm->tm_hour = bcd2bin(data[REG_TO_OFFSET(MAX31329_HOURS_REG)] & 0x3f);
+ /* Day of the week in linux range is 0~6 while 1~7 in RTC chip */
+ tm->tm_wday = bcd2bin(data[REG_TO_OFFSET(MAX31329_DAY_REG)] & 0x07) - 1;
+ tm->tm_mday = bcd2bin(data[REG_TO_OFFSET(MAX31329_DATE_REG)] & 0x3f);
+ /* linux tm_mon range:0~11, while month range is 1~12 in RTC chip */
+ tm->tm_mon = bcd2bin(data[REG_TO_OFFSET(MAX31329_MONTH_REG)] & 0x1f) - 1;
+
+ century = data[REG_TO_OFFSET(MAX31329_MONTH_REG)] & MAX31329_MONTH_CENTURY;
+ tm->tm_year = bcd2bin(data[REG_TO_OFFSET(MAX31329_YEAR_REG)]) +
+ (century ? 200 : 100);
+
+ return 0;
+}
+
+static int max31329_set_time(struct device *dev, struct rtc_time *tm)
+{
+ struct max31329_data *max31329 = dev_get_drvdata(dev);
+ u8 regs[7];
+ int ret;
+
+ ret = max31329_get_osc_status(dev);
+ if (ret)
+ return ret;
+
+ regs[REG_TO_OFFSET(MAX31329_SECONDS_REG)] = bin2bcd(tm->tm_sec);
+ regs[REG_TO_OFFSET(MAX31329_MINUTES_REG)] = bin2bcd(tm->tm_min);
+ regs[REG_TO_OFFSET(MAX31329_HOURS_REG)] = bin2bcd(tm->tm_hour);
+ regs[REG_TO_OFFSET(MAX31329_DAY_REG)] = bin2bcd(tm->tm_wday + 1);
+ regs[REG_TO_OFFSET(MAX31329_DATE_REG)] = bin2bcd(tm->tm_mday);
+ regs[REG_TO_OFFSET(MAX31329_MONTH_REG)] = bin2bcd(tm->tm_mon + 1);
+
+ if (tm->tm_year >= 200)
+ regs[REG_TO_OFFSET(MAX31329_MONTH_REG)] |= MAX31329_MONTH_CENTURY;
+ regs[REG_TO_OFFSET(MAX31329_YEAR_REG)] = bin2bcd(tm->tm_year % 100);
+
+ return regmap_bulk_write(max31329->regmap, MAX31329_SECONDS_REG, regs,
+ MAX31329_WATCH_SEC_LEN);
+}
+
+static const struct rtc_class_ops max31329_rtc_ops = {
+ .read_time = max31329_read_time,
+ .set_time = max31329_set_time,
+ .read_alarm = max31329_read_alarm,
+ .set_alarm = max31329_set_alarm,
+ .alarm_irq_enable = max31329_alarm_irq_enable,
+};
+
+static irqreturn_t max31329_irq_handler(int irq, void *dev_id)
+{
+ struct device *dev = dev_id;
+ struct max31329_data *max31329 = dev_get_drvdata(dev);
+ unsigned int flags, controls;
+ unsigned long events = 0;
+ int ret;
+
+ ret = regmap_read(max31329->regmap, MAX31329_INT_EN_REG, &controls);
+ if (ret) {
+ dev_warn(dev, "Read IRQ control register error %d\n", ret);
+ return IRQ_NONE;
+ }
+
+ ret = regmap_read(max31329->regmap, MAX31329_STATUS_REG, &flags);
+ if (ret) {
+ dev_warn(dev, "Read IRQ flags register error %d\n", ret);
+ return IRQ_NONE;
+ }
+
+ if (flags & MAX31329_STATUS_A1F) {
+ flags &= ~MAX31329_STATUS_A1F;
+ controls &= ~MAX31329_INT_EN_A1IE;
+ events = RTC_AF | RTC_IRQF;
+ }
+
+ if (events) {
+ rtc_update_irq(max31329->rtc, 1, events);
+ regmap_write(max31329->regmap, MAX31329_STATUS_REG, flags);
+ regmap_write(max31329->regmap, MAX31329_INT_EN_REG, controls);
+ return IRQ_HANDLED;
+ }
+
+ return IRQ_NONE;
+}
+
+static void max31329_trickle_config(struct device *dev)
+{
+ struct max31329_data *max31329 = dev_get_drvdata(dev);
+ u8 trickle_reg;
+ int ret, i;
+ u32 ohms;
+
+ /* Configure the trickle charger. */
+ ret = device_property_read_u32(dev, "trickle-resistor-ohms", &ohms);
+ if (ret)
+ return;
+
+ trickle_reg = MAX31329_TRICKLE_EN;
+ for (i = 1; i <= ARRAY_SIZE(max31329_trickle_ohms); i++) {
+ if (max31329_trickle_ohms[i - 1] == ohms) {
+ trickle_reg |= i;
+ regmap_write(max31329->regmap, MAX31329_TRICKLE_REG,
+ trickle_reg);
+ }
+ }
+}
+
+static int max31329_nvram_write(void *priv, unsigned int offset, void *val,
+ size_t bytes)
+{
+ struct regmap *max31329_regmap = (struct regmap *)priv;
+
+ return regmap_bulk_write(max31329_regmap,
+ MAX31329_RAM0_START_REG + offset,
+ val, bytes);
+}
+
+static int max31329_nvram_read(void *priv, unsigned int offset, void *val,
+ size_t bytes)
+{
+ struct regmap *max31329_regmap = (struct regmap *)priv;
+
+ return regmap_bulk_read(max31329_regmap,
+ MAX31329_RAM0_START_REG + offset,
+ val, bytes);
+}
+
+#ifdef CONFIG_COMMON_CLK
+#define clkout_hw_to_max31329(hw) container_of(hw, struct max31329_data, clkout_hw)
+
+static int clkout_rates[] = {
+ 1,
+ 4096,
+ 8192,
+ 32768
+};
+
+static unsigned long max31329_clkout_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct max31329_data *max31329 = clkout_hw_to_max31329(hw);
+ int clkout, ret;
+
+ ret = regmap_read(max31329->regmap, MAX31329_RTC_CONFIG2_REG, &clkout);
+ if (ret)
+ return 0;
+
+ return clkout_rates[FIELD_GET(MAX31329_RTC_CFG2_CLKOHZ_MSK, clkout)];
+}
+
+static long max31329_clkout_round_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long *prate)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(clkout_rates); i++)
+ if (clkout_rates[i] >= rate)
+ return clkout_rates[i];
+
+ return 0;
+}
+
+static int max31329_clkout_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ struct max31329_data *max31329 = clkout_hw_to_max31329(hw);
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(clkout_rates); i++)
+ if (rate == clkout_rates[i])
+ return regmap_update_bits(max31329->regmap,
+ MAX31329_RTC_CONFIG2_REG,
+ MAX31329_RTC_CFG2_CLKOHZ_MSK,
+ FIELD_PREP(MAX31329_RTC_CFG2_CLKOHZ_MSK,
+ i));
+ return -EINVAL;
+}
+
+static int max31329_clkout_prepare(struct clk_hw *hw)
+{
+ struct max31329_data *max31329 = clkout_hw_to_max31329(hw);
+
+ return regmap_update_bits(max31329->regmap, MAX31329_RTC_CONFIG2_REG,
+ MAX31329_RTC_CONFIG2_ENCLKO,
+ MAX31329_RTC_CONFIG2_ENCLKO);
+}
+
+static void max31329_clkout_unprepare(struct clk_hw *hw)
+{
+ struct max31329_data *max31329 = clkout_hw_to_max31329(hw);
+
+ regmap_update_bits(max31329->regmap, MAX31329_RTC_CONFIG2_REG,
+ MAX31329_RTC_CONFIG2_ENCLKO,
+ FIELD_PREP(MAX31329_RTC_CONFIG2_ENCLKO, 0));
+}
+
+static int max31329_clkout_is_prepared(struct clk_hw *hw)
+{
+ struct max31329_data *max31329 = clkout_hw_to_max31329(hw);
+ int clkout, ret;
+
+ ret = regmap_read(max31329->regmap, MAX31329_RTC_CONFIG2_REG, &clkout);
+ if (ret)
+ return ret;
+
+ return !!(clkout & MAX31329_RTC_CONFIG2_ENCLKO);
+}
+
+static const struct clk_ops max31329_clkout_ops = {
+ .prepare = max31329_clkout_prepare,
+ .unprepare = max31329_clkout_unprepare,
+ .is_prepared = max31329_clkout_is_prepared,
+ .recalc_rate = max31329_clkout_recalc_rate,
+ .round_rate = max31329_clkout_round_rate,
+ .set_rate = max31329_clkout_set_rate,
+};
+
+static int max31329_clkout_register_clk(struct max31329_data *max31329,
+ struct i2c_client *client)
+{
+ struct device_node *node = client->dev.of_node;
+ struct clk_init_data init;
+ struct clk *clk;
+
+ init.name = "max31329-clkout";
+ init.ops = &max31329_clkout_ops;
+ init.flags = 0;
+ init.parent_names = NULL;
+ init.num_parents = 0;
+ max31329->clkout_hw.init = &init;
+
+ /* optional override of the clockname */
+ of_property_read_string(node, "clock-output-names", &init.name);
+
+ clk = devm_clk_register(&client->dev, &max31329->clkout_hw);
+ if (!IS_ERR(clk))
+ of_clk_add_provider(node, of_clk_src_simple_get, clk);
+
+ return PTR_ERR(clk);
+}
+#endif
+
+static int max31329_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct max31329_data *max31329;
+ int ret;
+ struct nvmem_config nvmem_cfg = {
+ .name = "max31329_nvram",
+ .word_size = 1,
+ .stride = 1,
+ .size = 64,
+ .type = NVMEM_TYPE_BATTERY_BACKED,
+ .reg_read = max31329_nvram_read,
+ .reg_write = max31329_nvram_write,
+ };
+
+ max31329 = devm_kzalloc(&client->dev, sizeof(*max31329), GFP_KERNEL);
+ if (!max31329)
+ return -ENOMEM;
+
+ max31329->regmap = devm_regmap_init_i2c(client, &config);
+ if (IS_ERR(max31329->regmap)) {
+ dev_err(&client->dev, "regmap init failed\n");
+ return PTR_ERR(max31329->regmap);
+ }
+
+ dev_set_drvdata(&client->dev, max31329);
+
+ max31329->rtc = devm_rtc_allocate_device(&client->dev);
+ if (IS_ERR(max31329->rtc))
+ return PTR_ERR(max31329->rtc);
+
+ max31329->rtc->ops = &max31329_rtc_ops;
+ max31329->irq = client->irq;
+ max31329->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
+ max31329->rtc->range_max = RTC_TIMESTAMP_END_2199;
+
+ if (max31329->irq) {
+ ret = devm_request_threaded_irq(&client->dev, max31329->irq,
+ NULL, max31329_irq_handler,
+ IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+ "max31329", &client->dev);
+ if (ret) {
+ dev_err(&client->dev, "unable to request IRQ\n");
+ return ret;
+ }
+
+ ret = regmap_write(max31329->regmap, MAX31329_RTC_CONFIG2_REG,
+ MAX31329_RTC_CONFIG2_ENCLKO);
+ if (ret) {
+ dev_err(&client->dev, "unable to configure INT pin");
+ return ret;
+ }
+
+ if (device_property_read_bool(&client->dev, "wakeup-source"))
+ device_init_wakeup(&client->dev, true);
+ } else {
+ clear_bit(RTC_FEATURE_ALARM, max31329->rtc->features);
+ clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, max31329->rtc->features);
+ }
+
+ ret = devm_rtc_register_device(max31329->rtc);
+ if (ret)
+ return ret;
+
+ max31329_trickle_config(&client->dev);
+
+ nvmem_cfg.priv = max31329->regmap;
+ devm_rtc_nvmem_register(max31329->rtc, &nvmem_cfg);
+
+#ifdef CONFIG_COMMON_CLK
+ max31329_clkout_register_clk(max31329, client);
+#endif
+
+ return 0;
+}
+
+static const __maybe_unused struct of_device_id max31329_of_match[] = {
+ { .compatible = "maxim,max31329", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, max31329_of_match);
+
+static struct i2c_driver max31329_driver = {
+ .driver = {
+ .name = "rtc-max31329",
+ .of_match_table = of_match_ptr(max31329_of_match),
+ },
+ .probe = max31329_probe,
+};
+module_i2c_driver(max31329_driver);
+
+MODULE_AUTHOR("Jagath Jog J <jagathjog1996@gmail.com>");
+MODULE_DESCRIPTION("Maxim MAX31329 RTC driver");
+MODULE_LICENSE("GPL");
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] rtc: maxim: Add Maxim max31329 real time clock
2022-09-12 19:07 ` [PATCH v2 2/2] rtc: maxim: Add Maxim max31329 real time clock Jagath Jog J
@ 2022-09-14 11:55 ` Alexandre Belloni
2022-09-17 10:39 ` Jagath Jog J
0 siblings, 1 reply; 8+ messages in thread
From: Alexandre Belloni @ 2022-09-14 11:55 UTC (permalink / raw)
To: Jagath Jog J
Cc: a.zummo, robh+dt, krzysztof.kozlowski+dt, linux-rtc,
linux-kernel, devicetree
Hello,
On 13/09/2022 00:37:53+0530, Jagath Jog J wrote:
> +static int max31329_get_osc_status(struct device *dev)
> +{
> + struct max31329_data *max31329 = dev_get_drvdata(dev);
> + unsigned int status;
> + int ret;
> +
> + ret = regmap_read(max31329->regmap, MAX31329_STATUS_REG, &status);
> + if (ret)
> + return ret;
> +
> + if (status & MAX31329_STATUS_OSF) {
> + dev_err(dev, "Oscillator has stopped\n");
I would drop this message there is probably nothing specific the user
can do once it appears in the kernel log.
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> +static int max31329_alarm_irq_enable(struct device *dev, unsigned int enable)
> +{
> + struct max31329_data *max31329 = dev_get_drvdata(dev);
> +
> + return regmap_update_bits(max31329->regmap, MAX31329_INT_EN_REG,
> + MAX31329_INT_EN_A1IE,
> + enable ? MAX31329_INT_EN_A1IE : 0);
> +}
> +
> +static int max31329_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
> +{
> + struct max31329_data *max31329 = dev_get_drvdata(dev);
> + struct rtc_time *const tm = &alarm->time;
> + unsigned int aie_en, aie_flag;
> + int ret;
> + u8 regs[6];
> +
> + ret = max31329_get_osc_status(dev);
> + if (ret)
> + return ret;
> +
The oscillator being stopped doesn't mean the alarm is not correct so
you don't need to check here.
> + ret = regmap_bulk_read(max31329->regmap, MAX31329_ALM1_SEC_REG, regs,
> + MAX31329_ALM1_SEC_LEN);
> + if (ret)
> + return ret;
> +
> + ret = regmap_read(max31329->regmap, MAX31329_INT_EN_REG, &aie_en);
> + if (ret)
> + return ret;
> +
> + ret = regmap_read(max31329->regmap, MAX31329_STATUS_REG, &aie_flag);
> + if (ret)
> + return ret;
> +
> + tm->tm_sec = bcd2bin(regs[REG_TO_OFFSET(MAX31329_SECONDS_REG)] & 0x7f);
> + tm->tm_min = bcd2bin(regs[REG_TO_OFFSET(MAX31329_MINUTES_REG)] & 0x7f);
> + tm->tm_hour = bcd2bin(regs[REG_TO_OFFSET(MAX31329_HOURS_REG)] & 0x3f);
> + tm->tm_mday = bcd2bin(regs[REG_TO_OFFSET(MAX31329_DATE_REG) - 1] & 0x3f);
> + tm->tm_mon = bcd2bin(regs[REG_TO_OFFSET(MAX31329_MONTH_REG) - 1] &
> + 0x1f) - 1;
> + tm->tm_year = bcd2bin(regs[REG_TO_OFFSET(MAX31329_YEAR_REG) - 1]) + 200;
> +
> + alarm->enabled = FIELD_GET(MAX31329_INT_EN_A1IE, aie_en);
> + alarm->pending = FIELD_GET(MAX31329_STATUS_A1F, aie_flag) &&
> + alarm->enabled;
> +
> + return 0;
> +}
> +
> +static int max31329_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
> +{
> + struct max31329_data *max31329 = dev_get_drvdata(dev);
> + const struct rtc_time *tm = &alrm->time;
> + u8 regs[6], year;
> + int ret;
> +
> + ret = max31329_get_osc_status(dev);
> + if (ret)
> + return ret;
It is fine to set the alarm while the oscillator is stopped. The usual
userspace tools will anyway prevent you from doing that because they
first read the time and that call will check.
> +
> + regs[REG_TO_OFFSET(MAX31329_SECONDS_REG)] = bin2bcd(tm->tm_sec) & 0x7F;
> + regs[REG_TO_OFFSET(MAX31329_MINUTES_REG)] = bin2bcd(tm->tm_min) & 0x7f;
> + regs[REG_TO_OFFSET(MAX31329_HOURS_REG)] = bin2bcd(tm->tm_hour) & 0x3f;
> + regs[REG_TO_OFFSET(MAX31329_DATE_REG) - 1] = bin2bcd(tm->tm_mday) & 0x3f;
> + regs[REG_TO_OFFSET(MAX31329_MONTH_REG) - 1] = bin2bcd(tm->tm_mon + 1) & 0x1f;
> +
> + if (tm->tm_year >= 200)
> + year = bin2bcd(tm->tm_year - 200);
> + else
> + year = bin2bcd(tm->tm_year - 100);
This doesn't feel right, doesn't that break start-year?
What is the actual time range supported by this RTC? Shouldn't you set
the century?
> + regs[REG_TO_OFFSET(MAX31329_YEAR_REG) - 1] = year;
> +
> + ret = regmap_bulk_write(max31329->regmap, MAX31329_ALM1_SEC_REG, regs,
> + MAX31329_ALM1_SEC_LEN);
> + if (ret)
> + return ret;
> +
> + return max31329_alarm_irq_enable(dev, alrm->enabled);
> +}
> +
> +static int max31329_read_time(struct device *dev, struct rtc_time *tm)
> +{
> + struct max31329_data *max31329 = dev_get_drvdata(dev);
> + u8 data[7], century = 0;
> + int ret;
> +
> + ret = max31329_get_osc_status(dev);
> + if (ret)
> + return ret;
> +
> + ret = regmap_bulk_read(max31329->regmap, MAX31329_SECONDS_REG, data,
> + sizeof(data));
> + if (ret)
> + return ret;
> +
> + tm->tm_sec = bcd2bin(data[REG_TO_OFFSET(MAX31329_SECONDS_REG)] & 0x7f);
> + tm->tm_min = bcd2bin(data[REG_TO_OFFSET(MAX31329_MINUTES_REG)] & 0x7f);
> + tm->tm_hour = bcd2bin(data[REG_TO_OFFSET(MAX31329_HOURS_REG)] & 0x3f);
> + /* Day of the week in linux range is 0~6 while 1~7 in RTC chip */
> + tm->tm_wday = bcd2bin(data[REG_TO_OFFSET(MAX31329_DAY_REG)] & 0x07) - 1;
> + tm->tm_mday = bcd2bin(data[REG_TO_OFFSET(MAX31329_DATE_REG)] & 0x3f);
> + /* linux tm_mon range:0~11, while month range is 1~12 in RTC chip */
> + tm->tm_mon = bcd2bin(data[REG_TO_OFFSET(MAX31329_MONTH_REG)] & 0x1f) - 1;
> +
> + century = data[REG_TO_OFFSET(MAX31329_MONTH_REG)] & MAX31329_MONTH_CENTURY;
> + tm->tm_year = bcd2bin(data[REG_TO_OFFSET(MAX31329_YEAR_REG)]) +
> + (century ? 200 : 100);
> +
> + return 0;
> +}
> +
> +static int max31329_set_time(struct device *dev, struct rtc_time *tm)
> +{
> + struct max31329_data *max31329 = dev_get_drvdata(dev);
> + u8 regs[7];
> + int ret;
> +
> + ret = max31329_get_osc_status(dev);
> + if (ret)
> + return ret;
> +
Checking the oscillator is not needed here but resetting the status is.
> + regs[REG_TO_OFFSET(MAX31329_SECONDS_REG)] = bin2bcd(tm->tm_sec);
> + regs[REG_TO_OFFSET(MAX31329_MINUTES_REG)] = bin2bcd(tm->tm_min);
> + regs[REG_TO_OFFSET(MAX31329_HOURS_REG)] = bin2bcd(tm->tm_hour);
> + regs[REG_TO_OFFSET(MAX31329_DAY_REG)] = bin2bcd(tm->tm_wday + 1);
> + regs[REG_TO_OFFSET(MAX31329_DATE_REG)] = bin2bcd(tm->tm_mday);
> + regs[REG_TO_OFFSET(MAX31329_MONTH_REG)] = bin2bcd(tm->tm_mon + 1);
> +
> + if (tm->tm_year >= 200)
> + regs[REG_TO_OFFSET(MAX31329_MONTH_REG)] |= MAX31329_MONTH_CENTURY;
> + regs[REG_TO_OFFSET(MAX31329_YEAR_REG)] = bin2bcd(tm->tm_year % 100);
> +
> + return regmap_bulk_write(max31329->regmap, MAX31329_SECONDS_REG, regs,
> + MAX31329_WATCH_SEC_LEN);
> +}
> +
> +static const struct rtc_class_ops max31329_rtc_ops = {
> + .read_time = max31329_read_time,
> + .set_time = max31329_set_time,
> + .read_alarm = max31329_read_alarm,
> + .set_alarm = max31329_set_alarm,
> + .alarm_irq_enable = max31329_alarm_irq_enable,
> +};
> +
> +static irqreturn_t max31329_irq_handler(int irq, void *dev_id)
> +{
> + struct device *dev = dev_id;
> + struct max31329_data *max31329 = dev_get_drvdata(dev);
> + unsigned int flags, controls;
> + unsigned long events = 0;
> + int ret;
> +
> + ret = regmap_read(max31329->regmap, MAX31329_INT_EN_REG, &controls);
> + if (ret) {
> + dev_warn(dev, "Read IRQ control register error %d\n", ret);
Drop all of those messages please.
> + return IRQ_NONE;
> + }
> +
> + ret = regmap_read(max31329->regmap, MAX31329_STATUS_REG, &flags);
> + if (ret) {
> + dev_warn(dev, "Read IRQ flags register error %d\n", ret);
> + return IRQ_NONE;
> + }
> +
> + if (flags & MAX31329_STATUS_A1F) {
> + flags &= ~MAX31329_STATUS_A1F;
> + controls &= ~MAX31329_INT_EN_A1IE;
> + events = RTC_AF | RTC_IRQF;
> + }
> +
> + if (events) {
> + rtc_update_irq(max31329->rtc, 1, events);
> + regmap_write(max31329->regmap, MAX31329_STATUS_REG, flags);
> + regmap_write(max31329->regmap, MAX31329_INT_EN_REG, controls);
> + return IRQ_HANDLED;
> + }
> +
> + return IRQ_NONE;
> +}
> +
> +static void max31329_trickle_config(struct device *dev)
> +{
> + struct max31329_data *max31329 = dev_get_drvdata(dev);
> + u8 trickle_reg;
> + int ret, i;
> + u32 ohms;
> +
> + /* Configure the trickle charger. */
> + ret = device_property_read_u32(dev, "trickle-resistor-ohms", &ohms);
> + if (ret)
> + return;
> +
> + trickle_reg = MAX31329_TRICKLE_EN;
> + for (i = 1; i <= ARRAY_SIZE(max31329_trickle_ohms); i++) {
> + if (max31329_trickle_ohms[i - 1] == ohms) {
> + trickle_reg |= i;
> + regmap_write(max31329->regmap, MAX31329_TRICKLE_REG,
> + trickle_reg);
> + }
> + }
> +}
> +
> +static int max31329_nvram_write(void *priv, unsigned int offset, void *val,
> + size_t bytes)
> +{
> + struct regmap *max31329_regmap = (struct regmap *)priv;
> +
> + return regmap_bulk_write(max31329_regmap,
> + MAX31329_RAM0_START_REG + offset,
> + val, bytes);
> +}
> +
> +static int max31329_nvram_read(void *priv, unsigned int offset, void *val,
> + size_t bytes)
> +{
> + struct regmap *max31329_regmap = (struct regmap *)priv;
> +
> + return regmap_bulk_read(max31329_regmap,
> + MAX31329_RAM0_START_REG + offset,
> + val, bytes);
> +}
> +
> +#ifdef CONFIG_COMMON_CLK
> +#define clkout_hw_to_max31329(hw) container_of(hw, struct max31329_data, clkout_hw)
> +
> +static int clkout_rates[] = {
> + 1,
> + 4096,
> + 8192,
> + 32768
> +};
> +
> +static unsigned long max31329_clkout_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct max31329_data *max31329 = clkout_hw_to_max31329(hw);
> + int clkout, ret;
> +
> + ret = regmap_read(max31329->regmap, MAX31329_RTC_CONFIG2_REG, &clkout);
> + if (ret)
> + return 0;
> +
> + return clkout_rates[FIELD_GET(MAX31329_RTC_CFG2_CLKOHZ_MSK, clkout)];
> +}
> +
> +static long max31329_clkout_round_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long *prate)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(clkout_rates); i++)
> + if (clkout_rates[i] >= rate)
> + return clkout_rates[i];
> +
> + return 0;
> +}
> +
> +static int max31329_clkout_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + struct max31329_data *max31329 = clkout_hw_to_max31329(hw);
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(clkout_rates); i++)
> + if (rate == clkout_rates[i])
> + return regmap_update_bits(max31329->regmap,
> + MAX31329_RTC_CONFIG2_REG,
> + MAX31329_RTC_CFG2_CLKOHZ_MSK,
> + FIELD_PREP(MAX31329_RTC_CFG2_CLKOHZ_MSK,
> + i));
> + return -EINVAL;
> +}
> +
> +static int max31329_clkout_prepare(struct clk_hw *hw)
> +{
> + struct max31329_data *max31329 = clkout_hw_to_max31329(hw);
> +
> + return regmap_update_bits(max31329->regmap, MAX31329_RTC_CONFIG2_REG,
> + MAX31329_RTC_CONFIG2_ENCLKO,
> + MAX31329_RTC_CONFIG2_ENCLKO);
> +}
> +
> +static void max31329_clkout_unprepare(struct clk_hw *hw)
> +{
> + struct max31329_data *max31329 = clkout_hw_to_max31329(hw);
> +
> + regmap_update_bits(max31329->regmap, MAX31329_RTC_CONFIG2_REG,
> + MAX31329_RTC_CONFIG2_ENCLKO,
> + FIELD_PREP(MAX31329_RTC_CONFIG2_ENCLKO, 0));
> +}
> +
> +static int max31329_clkout_is_prepared(struct clk_hw *hw)
> +{
> + struct max31329_data *max31329 = clkout_hw_to_max31329(hw);
> + int clkout, ret;
> +
> + ret = regmap_read(max31329->regmap, MAX31329_RTC_CONFIG2_REG, &clkout);
> + if (ret)
> + return ret;
> +
> + return !!(clkout & MAX31329_RTC_CONFIG2_ENCLKO);
> +}
> +
> +static const struct clk_ops max31329_clkout_ops = {
> + .prepare = max31329_clkout_prepare,
> + .unprepare = max31329_clkout_unprepare,
> + .is_prepared = max31329_clkout_is_prepared,
> + .recalc_rate = max31329_clkout_recalc_rate,
> + .round_rate = max31329_clkout_round_rate,
> + .set_rate = max31329_clkout_set_rate,
> +};
> +
> +static int max31329_clkout_register_clk(struct max31329_data *max31329,
> + struct i2c_client *client)
> +{
> + struct device_node *node = client->dev.of_node;
> + struct clk_init_data init;
> + struct clk *clk;
> +
> + init.name = "max31329-clkout";
> + init.ops = &max31329_clkout_ops;
> + init.flags = 0;
> + init.parent_names = NULL;
> + init.num_parents = 0;
> + max31329->clkout_hw.init = &init;
> +
> + /* optional override of the clockname */
> + of_property_read_string(node, "clock-output-names", &init.name);
> +
> + clk = devm_clk_register(&client->dev, &max31329->clkout_hw);
> + if (!IS_ERR(clk))
> + of_clk_add_provider(node, of_clk_src_simple_get, clk);
> +
> + return PTR_ERR(clk);
> +}
> +#endif
> +
> +static int max31329_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct max31329_data *max31329;
> + int ret;
> + struct nvmem_config nvmem_cfg = {
> + .name = "max31329_nvram",
> + .word_size = 1,
> + .stride = 1,
> + .size = 64,
> + .type = NVMEM_TYPE_BATTERY_BACKED,
> + .reg_read = max31329_nvram_read,
> + .reg_write = max31329_nvram_write,
> + };
> +
> + max31329 = devm_kzalloc(&client->dev, sizeof(*max31329), GFP_KERNEL);
> + if (!max31329)
> + return -ENOMEM;
> +
> + max31329->regmap = devm_regmap_init_i2c(client, &config);
> + if (IS_ERR(max31329->regmap)) {
> + dev_err(&client->dev, "regmap init failed\n");
> + return PTR_ERR(max31329->regmap);
> + }
> +
> + dev_set_drvdata(&client->dev, max31329);
> +
> + max31329->rtc = devm_rtc_allocate_device(&client->dev);
> + if (IS_ERR(max31329->rtc))
> + return PTR_ERR(max31329->rtc);
> +
> + max31329->rtc->ops = &max31329_rtc_ops;
> + max31329->irq = client->irq;
> + max31329->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
> + max31329->rtc->range_max = RTC_TIMESTAMP_END_2199;
> +
> + if (max31329->irq) {
> + ret = devm_request_threaded_irq(&client->dev, max31329->irq,
> + NULL, max31329_irq_handler,
> + IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> + "max31329", &client->dev);
> + if (ret) {
> + dev_err(&client->dev, "unable to request IRQ\n");
This message can be dropped.
> + return ret;
> + }
> +
> + ret = regmap_write(max31329->regmap, MAX31329_RTC_CONFIG2_REG,
> + MAX31329_RTC_CONFIG2_ENCLKO);
Can you explain what this does because if you are changing a default
register value, we will be stuck with that forever and I feel like this
is something that should be configurable.
> + if (ret) {
> + dev_err(&client->dev, "unable to configure INT pin");
> + return ret;
> + }
> +
> + if (device_property_read_bool(&client->dev, "wakeup-source"))
> + device_init_wakeup(&client->dev, true);
wakeup-source should be used when no interrupt is available. When there
is a interrupt, we assume the RTC can wake up the device.
> + } else {
> + clear_bit(RTC_FEATURE_ALARM, max31329->rtc->features);
> + clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, max31329->rtc->features);
> + }
> +
> + ret = devm_rtc_register_device(max31329->rtc);
> + if (ret)
> + return ret;
> +
> + max31329_trickle_config(&client->dev);
> +
> + nvmem_cfg.priv = max31329->regmap;
> + devm_rtc_nvmem_register(max31329->rtc, &nvmem_cfg);
> +
> +#ifdef CONFIG_COMMON_CLK
> + max31329_clkout_register_clk(max31329, client);
> +#endif
> +
> + return 0;
> +}
> +
> +static const __maybe_unused struct of_device_id max31329_of_match[] = {
> + { .compatible = "maxim,max31329", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, max31329_of_match);
> +
> +static struct i2c_driver max31329_driver = {
> + .driver = {
> + .name = "rtc-max31329",
> + .of_match_table = of_match_ptr(max31329_of_match),
> + },
> + .probe = max31329_probe,
> +};
> +module_i2c_driver(max31329_driver);
> +
> +MODULE_AUTHOR("Jagath Jog J <jagathjog1996@gmail.com>");
> +MODULE_DESCRIPTION("Maxim MAX31329 RTC driver");
> +MODULE_LICENSE("GPL");
> --
> 2.17.1
>
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] rtc: maxim: Add Maxim max31329 real time clock
2022-09-14 11:55 ` Alexandre Belloni
@ 2022-09-17 10:39 ` Jagath Jog J
2022-09-18 18:48 ` Alexandre Belloni
0 siblings, 1 reply; 8+ messages in thread
From: Jagath Jog J @ 2022-09-17 10:39 UTC (permalink / raw)
To: Alexandre Belloni
Cc: a.zummo, robh+dt, krzysztof.kozlowski+dt, linux-rtc,
linux-kernel, devicetree
Hello Alexandre,
Thank you for reviewing.
On Wed, Sep 14, 2022 at 5:25 PM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
>
> Hello,
>
> On 13/09/2022 00:37:53+0530, Jagath Jog J wrote:
> > +static int max31329_get_osc_status(struct device *dev)
> > +{
> > + struct max31329_data *max31329 = dev_get_drvdata(dev);
> > + unsigned int status;
> > + int ret;
> > +
> > + ret = regmap_read(max31329->regmap, MAX31329_STATUS_REG, &status);
> > + if (ret)
> > + return ret;
> > +
> > + if (status & MAX31329_STATUS_OSF) {
> > + dev_err(dev, "Oscillator has stopped\n");
>
> I would drop this message there is probably nothing specific the user
> can do once it appears in the kernel log.
>
> > + return -EINVAL;
> > + }
> > + return 0;
> > +}
> > +
> > +static int max31329_alarm_irq_enable(struct device *dev, unsigned int enable)
> > +{
> > + struct max31329_data *max31329 = dev_get_drvdata(dev);
> > +
> > + return regmap_update_bits(max31329->regmap, MAX31329_INT_EN_REG,
> > + MAX31329_INT_EN_A1IE,
> > + enable ? MAX31329_INT_EN_A1IE : 0);
> > +}
> > +
> > +static int max31329_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
> > +{
> > + struct max31329_data *max31329 = dev_get_drvdata(dev);
> > + struct rtc_time *const tm = &alarm->time;
> > + unsigned int aie_en, aie_flag;
> > + int ret;
> > + u8 regs[6];
> > +
> > + ret = max31329_get_osc_status(dev);
> > + if (ret)
> > + return ret;
> > +
>
> The oscillator being stopped doesn't mean the alarm is not correct so
> you don't need to check here.
Sure, I will remove this check.
>
> > + ret = regmap_bulk_read(max31329->regmap, MAX31329_ALM1_SEC_REG, regs,
> > + MAX31329_ALM1_SEC_LEN);
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_read(max31329->regmap, MAX31329_INT_EN_REG, &aie_en);
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_read(max31329->regmap, MAX31329_STATUS_REG, &aie_flag);
> > + if (ret)
> > + return ret;
> > +
> > + tm->tm_sec = bcd2bin(regs[REG_TO_OFFSET(MAX31329_SECONDS_REG)] & 0x7f);
> > + tm->tm_min = bcd2bin(regs[REG_TO_OFFSET(MAX31329_MINUTES_REG)] & 0x7f);
> > + tm->tm_hour = bcd2bin(regs[REG_TO_OFFSET(MAX31329_HOURS_REG)] & 0x3f);
> > + tm->tm_mday = bcd2bin(regs[REG_TO_OFFSET(MAX31329_DATE_REG) - 1] & 0x3f);
> > + tm->tm_mon = bcd2bin(regs[REG_TO_OFFSET(MAX31329_MONTH_REG) - 1] &
> > + 0x1f) - 1;
> > + tm->tm_year = bcd2bin(regs[REG_TO_OFFSET(MAX31329_YEAR_REG) - 1]) + 200;
> > +
> > + alarm->enabled = FIELD_GET(MAX31329_INT_EN_A1IE, aie_en);
> > + alarm->pending = FIELD_GET(MAX31329_STATUS_A1F, aie_flag) &&
> > + alarm->enabled;
> > +
> > + return 0;
> > +}
> > +
> > +static int max31329_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
> > +{
> > + struct max31329_data *max31329 = dev_get_drvdata(dev);
> > + const struct rtc_time *tm = &alrm->time;
> > + u8 regs[6], year;
> > + int ret;
> > +
> > + ret = max31329_get_osc_status(dev);
> > + if (ret)
> > + return ret;
>
> It is fine to set the alarm while the oscillator is stopped. The usual
> userspace tools will anyway prevent you from doing that because they
> first read the time and that call will check.
>
> > +
> > + regs[REG_TO_OFFSET(MAX31329_SECONDS_REG)] = bin2bcd(tm->tm_sec) & 0x7F;
> > + regs[REG_TO_OFFSET(MAX31329_MINUTES_REG)] = bin2bcd(tm->tm_min) & 0x7f;
> > + regs[REG_TO_OFFSET(MAX31329_HOURS_REG)] = bin2bcd(tm->tm_hour) & 0x3f;
> > + regs[REG_TO_OFFSET(MAX31329_DATE_REG) - 1] = bin2bcd(tm->tm_mday) & 0x3f;
> > + regs[REG_TO_OFFSET(MAX31329_MONTH_REG) - 1] = bin2bcd(tm->tm_mon + 1) & 0x1f;
> > +
> > + if (tm->tm_year >= 200)
> > + year = bin2bcd(tm->tm_year - 200);
> > + else
> > + year = bin2bcd(tm->tm_year - 100);
>
> This doesn't feel right, doesn't that break start-year?
>
> What is the actual time range supported by this RTC? Shouldn't you set
> the century?
The time range supported by RTC is 2000 to 2199.
The alarm registers don't have a century bit.
I have tested the alarm for
2122-09-17T01:22:00
2142-09-17T01:22:00
2160-02-29T00:00:00
2196-02-29T00:00:00 etc
I will add another condition such that if the century bit
from the time register is not set then configuring the
alarm for the next century is not allowed.
...
if (tm->tm_year >= 200) {
year = bin2bcd(tm->tm_year - 200);
ret = regmap_read(max31329->regmap, MAX31329_MONTH_REG, &month);
if (ret)
return ret;
if (!FIELD_GET(MAX31329_MONTH_CENTURY, month))
return -EINVAL;
} else {
year = bin2bcd(tm->tm_year - 100);
}
...
>
> > + regs[REG_TO_OFFSET(MAX31329_YEAR_REG) - 1] = year;
> > +
> > + ret = regmap_bulk_write(max31329->regmap, MAX31329_ALM1_SEC_REG, regs,
> > + MAX31329_ALM1_SEC_LEN);
> > + if (ret)
> > + return ret;
> > +
> > + return max31329_alarm_irq_enable(dev, alrm->enabled);
> > +}
> > +
> > +static int max31329_read_time(struct device *dev, struct rtc_time *tm)
> > +{
> > + struct max31329_data *max31329 = dev_get_drvdata(dev);
> > + u8 data[7], century = 0;
> > + int ret;
> > +
> > + ret = max31329_get_osc_status(dev);
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_bulk_read(max31329->regmap, MAX31329_SECONDS_REG, data,
> > + sizeof(data));
> > + if (ret)
> > + return ret;
> > +
> > + tm->tm_sec = bcd2bin(data[REG_TO_OFFSET(MAX31329_SECONDS_REG)] & 0x7f);
> > + tm->tm_min = bcd2bin(data[REG_TO_OFFSET(MAX31329_MINUTES_REG)] & 0x7f);
> > + tm->tm_hour = bcd2bin(data[REG_TO_OFFSET(MAX31329_HOURS_REG)] & 0x3f);
> > + /* Day of the week in linux range is 0~6 while 1~7 in RTC chip */
> > + tm->tm_wday = bcd2bin(data[REG_TO_OFFSET(MAX31329_DAY_REG)] & 0x07) - 1;
> > + tm->tm_mday = bcd2bin(data[REG_TO_OFFSET(MAX31329_DATE_REG)] & 0x3f);
> > + /* linux tm_mon range:0~11, while month range is 1~12 in RTC chip */
> > + tm->tm_mon = bcd2bin(data[REG_TO_OFFSET(MAX31329_MONTH_REG)] & 0x1f) - 1;
> > +
> > + century = data[REG_TO_OFFSET(MAX31329_MONTH_REG)] & MAX31329_MONTH_CENTURY;
> > + tm->tm_year = bcd2bin(data[REG_TO_OFFSET(MAX31329_YEAR_REG)]) +
> > + (century ? 200 : 100);
> > +
> > + return 0;
> > +}
> > +
> > +static int max31329_set_time(struct device *dev, struct rtc_time *tm)
> > +{
> > + struct max31329_data *max31329 = dev_get_drvdata(dev);
> > + u8 regs[7];
> > + int ret;
> > +
> > + ret = max31329_get_osc_status(dev);
> > + if (ret)
> > + return ret;
> > +
>
> Checking the oscillator is not needed here but resetting the status is.
Resetting the device will resets the digital block,
I2C-programmable registers and oscillator also,
The oscillator is taking some time around 80 milli sec
to be back as usual.
Is it required to reset every time during the time setting?
>
> > + regs[REG_TO_OFFSET(MAX31329_SECONDS_REG)] = bin2bcd(tm->tm_sec);
> > + regs[REG_TO_OFFSET(MAX31329_MINUTES_REG)] = bin2bcd(tm->tm_min);
> > + regs[REG_TO_OFFSET(MAX31329_HOURS_REG)] = bin2bcd(tm->tm_hour);
> > + regs[REG_TO_OFFSET(MAX31329_DAY_REG)] = bin2bcd(tm->tm_wday + 1);
> > + regs[REG_TO_OFFSET(MAX31329_DATE_REG)] = bin2bcd(tm->tm_mday);
> > + regs[REG_TO_OFFSET(MAX31329_MONTH_REG)] = bin2bcd(tm->tm_mon + 1);
> > +
> > + if (tm->tm_year >= 200)
> > + regs[REG_TO_OFFSET(MAX31329_MONTH_REG)] |= MAX31329_MONTH_CENTURY;
> > + regs[REG_TO_OFFSET(MAX31329_YEAR_REG)] = bin2bcd(tm->tm_year % 100);
> > +
> > + return regmap_bulk_write(max31329->regmap, MAX31329_SECONDS_REG, regs,
> > + MAX31329_WATCH_SEC_LEN);
> > +}
> > +
> > +static const struct rtc_class_ops max31329_rtc_ops = {
> > + .read_time = max31329_read_time,
> > + .set_time = max31329_set_time,
> > + .read_alarm = max31329_read_alarm,
> > + .set_alarm = max31329_set_alarm,
> > + .alarm_irq_enable = max31329_alarm_irq_enable,
> > +};
> > +
> > +static irqreturn_t max31329_irq_handler(int irq, void *dev_id)
> > +{
> > + struct device *dev = dev_id;
> > + struct max31329_data *max31329 = dev_get_drvdata(dev);
> > + unsigned int flags, controls;
> > + unsigned long events = 0;
> > + int ret;
> > +
> > + ret = regmap_read(max31329->regmap, MAX31329_INT_EN_REG, &controls);
> > + if (ret) {
> > + dev_warn(dev, "Read IRQ control register error %d\n", ret);
>
> Drop all of those messages please.
Sure I will drop all messages.
>
> > + return IRQ_NONE;
> > + }
> > +
> > + ret = regmap_read(max31329->regmap, MAX31329_STATUS_REG, &flags);
> > + if (ret) {
> > + dev_warn(dev, "Read IRQ flags register error %d\n", ret);
> > + return IRQ_NONE;
> > + }
> > +
> > + if (flags & MAX31329_STATUS_A1F) {
> > + flags &= ~MAX31329_STATUS_A1F;
> > + controls &= ~MAX31329_INT_EN_A1IE;
> > + events = RTC_AF | RTC_IRQF;
> > + }
> > +
> > + if (events) {
> > + rtc_update_irq(max31329->rtc, 1, events);
> > + regmap_write(max31329->regmap, MAX31329_STATUS_REG, flags);
> > + regmap_write(max31329->regmap, MAX31329_INT_EN_REG, controls);
> > + return IRQ_HANDLED;
> > + }
> > +
> > + return IRQ_NONE;
> > +}
> > +
> > +static void max31329_trickle_config(struct device *dev)
> > +{
> > + struct max31329_data *max31329 = dev_get_drvdata(dev);
> > + u8 trickle_reg;
> > + int ret, i;
> > + u32 ohms;
> > +
> > + /* Configure the trickle charger. */
> > + ret = device_property_read_u32(dev, "trickle-resistor-ohms", &ohms);
> > + if (ret)
> > + return;
> > +
> > + trickle_reg = MAX31329_TRICKLE_EN;
> > + for (i = 1; i <= ARRAY_SIZE(max31329_trickle_ohms); i++) {
> > + if (max31329_trickle_ohms[i - 1] == ohms) {
> > + trickle_reg |= i;
> > + regmap_write(max31329->regmap, MAX31329_TRICKLE_REG,
> > + trickle_reg);
> > + }
> > + }
> > +}
> > +
> > +static int max31329_nvram_write(void *priv, unsigned int offset, void *val,
> > + size_t bytes)
> > +{
> > + struct regmap *max31329_regmap = (struct regmap *)priv;
> > +
> > + return regmap_bulk_write(max31329_regmap,
> > + MAX31329_RAM0_START_REG + offset,
> > + val, bytes);
> > +}
> > +
> > +static int max31329_nvram_read(void *priv, unsigned int offset, void *val,
> > + size_t bytes)
> > +{
> > + struct regmap *max31329_regmap = (struct regmap *)priv;
> > +
> > + return regmap_bulk_read(max31329_regmap,
> > + MAX31329_RAM0_START_REG + offset,
> > + val, bytes);
> > +}
> > +
> > +#ifdef CONFIG_COMMON_CLK
> > +#define clkout_hw_to_max31329(hw) container_of(hw, struct max31329_data, clkout_hw)
> > +
> > +static int clkout_rates[] = {
> > + 1,
> > + 4096,
> > + 8192,
> > + 32768
> > +};
> > +
> > +static unsigned long max31329_clkout_recalc_rate(struct clk_hw *hw,
> > + unsigned long parent_rate)
> > +{
> > + struct max31329_data *max31329 = clkout_hw_to_max31329(hw);
> > + int clkout, ret;
> > +
> > + ret = regmap_read(max31329->regmap, MAX31329_RTC_CONFIG2_REG, &clkout);
> > + if (ret)
> > + return 0;
> > +
> > + return clkout_rates[FIELD_GET(MAX31329_RTC_CFG2_CLKOHZ_MSK, clkout)];
> > +}
> > +
> > +static long max31329_clkout_round_rate(struct clk_hw *hw, unsigned long rate,
> > + unsigned long *prate)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(clkout_rates); i++)
> > + if (clkout_rates[i] >= rate)
> > + return clkout_rates[i];
> > +
> > + return 0;
> > +}
> > +
> > +static int max31329_clkout_set_rate(struct clk_hw *hw, unsigned long rate,
> > + unsigned long parent_rate)
> > +{
> > + struct max31329_data *max31329 = clkout_hw_to_max31329(hw);
> > + int i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(clkout_rates); i++)
> > + if (rate == clkout_rates[i])
> > + return regmap_update_bits(max31329->regmap,
> > + MAX31329_RTC_CONFIG2_REG,
> > + MAX31329_RTC_CFG2_CLKOHZ_MSK,
> > + FIELD_PREP(MAX31329_RTC_CFG2_CLKOHZ_MSK,
> > + i));
> > + return -EINVAL;
> > +}
> > +
> > +static int max31329_clkout_prepare(struct clk_hw *hw)
> > +{
> > + struct max31329_data *max31329 = clkout_hw_to_max31329(hw);
> > +
> > + return regmap_update_bits(max31329->regmap, MAX31329_RTC_CONFIG2_REG,
> > + MAX31329_RTC_CONFIG2_ENCLKO,
> > + MAX31329_RTC_CONFIG2_ENCLKO);
> > +}
> > +
> > +static void max31329_clkout_unprepare(struct clk_hw *hw)
> > +{
> > + struct max31329_data *max31329 = clkout_hw_to_max31329(hw);
> > +
> > + regmap_update_bits(max31329->regmap, MAX31329_RTC_CONFIG2_REG,
> > + MAX31329_RTC_CONFIG2_ENCLKO,
> > + FIELD_PREP(MAX31329_RTC_CONFIG2_ENCLKO, 0));
> > +}
> > +
> > +static int max31329_clkout_is_prepared(struct clk_hw *hw)
> > +{
> > + struct max31329_data *max31329 = clkout_hw_to_max31329(hw);
> > + int clkout, ret;
> > +
> > + ret = regmap_read(max31329->regmap, MAX31329_RTC_CONFIG2_REG, &clkout);
> > + if (ret)
> > + return ret;
> > +
> > + return !!(clkout & MAX31329_RTC_CONFIG2_ENCLKO);
> > +}
> > +
> > +static const struct clk_ops max31329_clkout_ops = {
> > + .prepare = max31329_clkout_prepare,
> > + .unprepare = max31329_clkout_unprepare,
> > + .is_prepared = max31329_clkout_is_prepared,
> > + .recalc_rate = max31329_clkout_recalc_rate,
> > + .round_rate = max31329_clkout_round_rate,
> > + .set_rate = max31329_clkout_set_rate,
> > +};
> > +
> > +static int max31329_clkout_register_clk(struct max31329_data *max31329,
> > + struct i2c_client *client)
> > +{
> > + struct device_node *node = client->dev.of_node;
> > + struct clk_init_data init;
> > + struct clk *clk;
> > +
> > + init.name = "max31329-clkout";
> > + init.ops = &max31329_clkout_ops;
> > + init.flags = 0;
> > + init.parent_names = NULL;
> > + init.num_parents = 0;
> > + max31329->clkout_hw.init = &init;
> > +
> > + /* optional override of the clockname */
> > + of_property_read_string(node, "clock-output-names", &init.name);
> > +
> > + clk = devm_clk_register(&client->dev, &max31329->clkout_hw);
> > + if (!IS_ERR(clk))
> > + of_clk_add_provider(node, of_clk_src_simple_get, clk);
> > +
> > + return PTR_ERR(clk);
> > +}
> > +#endif
> > +
> > +static int max31329_probe(struct i2c_client *client,
> > + const struct i2c_device_id *id)
> > +{
> > + struct max31329_data *max31329;
> > + int ret;
> > + struct nvmem_config nvmem_cfg = {
> > + .name = "max31329_nvram",
> > + .word_size = 1,
> > + .stride = 1,
> > + .size = 64,
> > + .type = NVMEM_TYPE_BATTERY_BACKED,
> > + .reg_read = max31329_nvram_read,
> > + .reg_write = max31329_nvram_write,
> > + };
> > +
> > + max31329 = devm_kzalloc(&client->dev, sizeof(*max31329), GFP_KERNEL);
> > + if (!max31329)
> > + return -ENOMEM;
> > +
> > + max31329->regmap = devm_regmap_init_i2c(client, &config);
> > + if (IS_ERR(max31329->regmap)) {
> > + dev_err(&client->dev, "regmap init failed\n");
> > + return PTR_ERR(max31329->regmap);
> > + }
> > +
> > + dev_set_drvdata(&client->dev, max31329);
> > +
> > + max31329->rtc = devm_rtc_allocate_device(&client->dev);
> > + if (IS_ERR(max31329->rtc))
> > + return PTR_ERR(max31329->rtc);
> > +
> > + max31329->rtc->ops = &max31329_rtc_ops;
> > + max31329->irq = client->irq;
> > + max31329->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
> > + max31329->rtc->range_max = RTC_TIMESTAMP_END_2199;
> > +
> > + if (max31329->irq) {
> > + ret = devm_request_threaded_irq(&client->dev, max31329->irq,
> > + NULL, max31329_irq_handler,
> > + IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> > + "max31329", &client->dev);
> > + if (ret) {
> > + dev_err(&client->dev, "unable to request IRQ\n");
>
> This message can be dropped.
>
> > + return ret;
> > + }
> > +
> > + ret = regmap_write(max31329->regmap, MAX31329_RTC_CONFIG2_REG,
> > + MAX31329_RTC_CONFIG2_ENCLKO);
>
> Can you explain what this does because if you are changing a default
> register value, we will be stuck with that forever and I feel like this
> is something that should be configurable.
Yeah, I missed this, I shouldn't have set this as default.
I will remove this since it is already configurable in _clkout_prepare and
_clkout_unprepare.
>
> > + if (ret) {
> > + dev_err(&client->dev, "unable to configure INT pin");
> > + return ret;
> > + }
> > +
> > + if (device_property_read_bool(&client->dev, "wakeup-source"))
> > + device_init_wakeup(&client->dev, true);
>
> wakeup-source should be used when no interrupt is available. When there
> is a interrupt, we assume the RTC can wake up the device.
>
> > + } else {
> > + clear_bit(RTC_FEATURE_ALARM, max31329->rtc->features);
> > + clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, max31329->rtc->features);
> > + }
> > +
> > + ret = devm_rtc_register_device(max31329->rtc);
> > + if (ret)
> > + return ret;
> > +
> > + max31329_trickle_config(&client->dev);
> > +
> > + nvmem_cfg.priv = max31329->regmap;
> > + devm_rtc_nvmem_register(max31329->rtc, &nvmem_cfg);
> > +
> > +#ifdef CONFIG_COMMON_CLK
> > + max31329_clkout_register_clk(max31329, client);
> > +#endif
> > +
> > + return 0;
> > +}
> > +
> > +static const __maybe_unused struct of_device_id max31329_of_match[] = {
> > + { .compatible = "maxim,max31329", },
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(of, max31329_of_match);
> > +
> > +static struct i2c_driver max31329_driver = {
> > + .driver = {
> > + .name = "rtc-max31329",
> > + .of_match_table = of_match_ptr(max31329_of_match),
> > + },
> > + .probe = max31329_probe,
> > +};
> > +module_i2c_driver(max31329_driver);
> > +
> > +MODULE_AUTHOR("Jagath Jog J <jagathjog1996@gmail.com>");
> > +MODULE_DESCRIPTION("Maxim MAX31329 RTC driver");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.17.1
> >
>
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] rtc: maxim: Add Maxim max31329 real time clock
2022-09-17 10:39 ` Jagath Jog J
@ 2022-09-18 18:48 ` Alexandre Belloni
2022-09-19 19:39 ` Jagath Jog J
0 siblings, 1 reply; 8+ messages in thread
From: Alexandre Belloni @ 2022-09-18 18:48 UTC (permalink / raw)
To: Jagath Jog J
Cc: a.zummo, robh+dt, krzysztof.kozlowski+dt, linux-rtc,
linux-kernel, devicetree
On 17/09/2022 16:09:54+0530, Jagath Jog J wrote:
> > This doesn't feel right, doesn't that break start-year?
> >
> > What is the actual time range supported by this RTC? Shouldn't you set
> > the century?
>
> The time range supported by RTC is 2000 to 2199.
> The alarm registers don't have a century bit.
> I have tested the alarm for
> 2122-09-17T01:22:00
> 2142-09-17T01:22:00
> 2160-02-29T00:00:00
> 2196-02-29T00:00:00 etc
>
> I will add another condition such that if the century bit
> from the time register is not set then configuring the
> alarm for the next century is not allowed.
The actual check should be for the alarm to not exceed 100 years in the
future then. Else, this wouldn't work well with datetime offsetting.
> > > +static int max31329_set_time(struct device *dev, struct rtc_time *tm)
> > > +{
> > > + struct max31329_data *max31329 = dev_get_drvdata(dev);
> > > + u8 regs[7];
> > > + int ret;
> > > +
> > > + ret = max31329_get_osc_status(dev);
> > > + if (ret)
> > > + return ret;
> > > +
> >
> > Checking the oscillator is not needed here but resetting the status is.
>
> Resetting the device will resets the digital block,
> I2C-programmable registers and oscillator also,
> The oscillator is taking some time around 80 milli sec
> to be back as usual.
>
> Is it required to reset every time during the time setting?
>
Not but resetting the osc status is.
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] rtc: maxim: Add Maxim max31329 real time clock
2022-09-18 18:48 ` Alexandre Belloni
@ 2022-09-19 19:39 ` Jagath Jog J
2022-09-23 11:04 ` Alexandre Belloni
0 siblings, 1 reply; 8+ messages in thread
From: Jagath Jog J @ 2022-09-19 19:39 UTC (permalink / raw)
To: Alexandre Belloni
Cc: a.zummo, robh+dt, krzysztof.kozlowski+dt, linux-rtc,
linux-kernel, devicetree
Hi Alexandre,
Before sending v3 I have one comment,
Please see below.
On Mon, Sep 19, 2022 at 12:18 AM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
>
> On 17/09/2022 16:09:54+0530, Jagath Jog J wrote:
> > > This doesn't feel right, doesn't that break start-year?
> > >
> > > What is the actual time range supported by this RTC? Shouldn't you set
> > > the century?
> >
> > The time range supported by RTC is 2000 to 2199.
> > The alarm registers don't have a century bit.
> > I have tested the alarm for
> > 2122-09-17T01:22:00
> > 2142-09-17T01:22:00
> > 2160-02-29T00:00:00
> > 2196-02-29T00:00:00 etc
> >
> > I will add another condition such that if the century bit
> > from the time register is not set then configuring the
> > alarm for the next century is not allowed.
>
> The actual check should be for the alarm to not exceed 100 years in the
> future then. Else, this wouldn't work well with datetime offsetting.
Sure, I will add this check.
>
> > > > +static int max31329_set_time(struct device *dev, struct rtc_time *tm)
> > > > +{
> > > > + struct max31329_data *max31329 = dev_get_drvdata(dev);
> > > > + u8 regs[7];
> > > > + int ret;
> > > > +
> > > > + ret = max31329_get_osc_status(dev);
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > >
> > > Checking the oscillator is not needed here but resetting the status is.
> >
> > Resetting the device will resets the digital block,
> > I2C-programmable registers and oscillator also,
> > The oscillator is taking some time around 80 milli sec
> > to be back as usual.
> >
> > Is it required to reset every time during the time setting?
> >
>
> Not but resetting the osc status is.
Actually, the STATUS register which contains the Oscillator Stop
Flag (OSF) bit is a read-only register. If the OSF bit is set, then
reading the status register will not clear the OSF bit.
Based on the oscillator disable and enable testing, I observed
that the OSF bit is getting cleared automatically once the clock
settles, which is taking around 80msec. The manual resetting
option is not there for the OSC status bit.
Can I set the time without resetting the OSC status?
Thank you,
Jagath
>
>
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] rtc: maxim: Add Maxim max31329 real time clock
2022-09-19 19:39 ` Jagath Jog J
@ 2022-09-23 11:04 ` Alexandre Belloni
0 siblings, 0 replies; 8+ messages in thread
From: Alexandre Belloni @ 2022-09-23 11:04 UTC (permalink / raw)
To: Jagath Jog J
Cc: a.zummo, robh+dt, krzysztof.kozlowski+dt, linux-rtc,
linux-kernel, devicetree
On 20/09/2022 01:09:37+0530, Jagath Jog J wrote:
> Hi Alexandre,
>
> Before sending v3 I have one comment,
> Please see below.
>
> On Mon, Sep 19, 2022 at 12:18 AM Alexandre Belloni
> <alexandre.belloni@bootlin.com> wrote:
> >
> > On 17/09/2022 16:09:54+0530, Jagath Jog J wrote:
> > > > This doesn't feel right, doesn't that break start-year?
> > > >
> > > > What is the actual time range supported by this RTC? Shouldn't you set
> > > > the century?
> > >
> > > The time range supported by RTC is 2000 to 2199.
> > > The alarm registers don't have a century bit.
> > > I have tested the alarm for
> > > 2122-09-17T01:22:00
> > > 2142-09-17T01:22:00
> > > 2160-02-29T00:00:00
> > > 2196-02-29T00:00:00 etc
> > >
> > > I will add another condition such that if the century bit
> > > from the time register is not set then configuring the
> > > alarm for the next century is not allowed.
> >
> > The actual check should be for the alarm to not exceed 100 years in the
> > future then. Else, this wouldn't work well with datetime offsetting.
>
> Sure, I will add this check.
>
> >
> > > > > +static int max31329_set_time(struct device *dev, struct rtc_time *tm)
> > > > > +{
> > > > > + struct max31329_data *max31329 = dev_get_drvdata(dev);
> > > > > + u8 regs[7];
> > > > > + int ret;
> > > > > +
> > > > > + ret = max31329_get_osc_status(dev);
> > > > > + if (ret)
> > > > > + return ret;
> > > > > +
> > > >
> > > > Checking the oscillator is not needed here but resetting the status is.
> > >
> > > Resetting the device will resets the digital block,
> > > I2C-programmable registers and oscillator also,
> > > The oscillator is taking some time around 80 milli sec
> > > to be back as usual.
> > >
> > > Is it required to reset every time during the time setting?
> > >
> >
> > Not but resetting the osc status is.
>
> Actually, the STATUS register which contains the Oscillator Stop
> Flag (OSF) bit is a read-only register. If the OSF bit is set, then
> reading the status register will not clear the OSF bit.
>
> Based on the oscillator disable and enable testing, I observed
> that the OSF bit is getting cleared automatically once the clock
> settles, which is taking around 80msec. The manual resetting
> option is not there for the OSC status bit.
>
> Can I set the time without resetting the OSC status?
>
Sure but then it is not even useful to ever test OSF because we just
don't care if it fails while we are running, the most important info is
whether it fails when Linux is not running.
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-09-23 11:04 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-12 19:07 [PATCH v2 0/2] rtc: maxim: add support for Maxim max31329 RTC Jagath Jog J
2022-09-12 19:07 ` [PATCH v2 1/2] dt-bindings: rtc: add Maxim max31329 rtc device tree bindings Jagath Jog J
2022-09-12 19:07 ` [PATCH v2 2/2] rtc: maxim: Add Maxim max31329 real time clock Jagath Jog J
2022-09-14 11:55 ` Alexandre Belloni
2022-09-17 10:39 ` Jagath Jog J
2022-09-18 18:48 ` Alexandre Belloni
2022-09-19 19:39 ` Jagath Jog J
2022-09-23 11:04 ` Alexandre Belloni
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).