Skip to contentSkip to conversation

rtc: abx80x: add support for abx81x

PATCH Superseded Antoni Pokusinski 2 participants 22 messages Jul 31 – Aug 4

Subsystems (2)

v3 of 7 Superseded Latest is v7 →

lore
Share & tools
lore
#0 lore raw

v3 cover rtc: abx80x: add support for abx81x

Hello,
This patch series adds support for the ABX81X RTCs, which are essentially SPI versions of the ABX80X RTCs. The first patches (P2-P3) contain bug fixes (added after sashiko's review in v1 and v2), then (P4-P7) we have some cleanup and reorganization changes (converting the driver to use the regmap API was needed for this), followed by the actual ABX81X support.

Kind regards,
Antoni Pokusinski

---
Chnges since v2:
* dropped patch "rtc: abx80x: move part autodetection before RV1805 configuration"
* PATCH 2 ("rtc: abx80x: add mutex protection for register writes") New patch
* PATCH 5 ("rtc: abx80x: use regmap instead of I2C specific API")
- return PTR_ERR(regmap) on I2C regmap init error
* PATCH 8 ("rtc: abx80x: add support for ABX81X")
- return PTR_ERR(regmap) on SPI regmap init error
---
Changes since v1:
* PATCH 1 ("dt-bindings: rtc: abx80x: document ABX81X RTCs")
- added missing "spi-peripherals-props" reference
- added abx815 compatible strings to the trickle charger enum list
* PATCH 2 ("rtc: abx80x: move part autodetection before RV1805 configuration") New patch
* PATCH 3 ("rtc: abx80x: properly handle shared IRQs")
New patch
* PATCH 5 ("rtc: abx80x: use regmap instead of I2C specific API")
- changed regmap_read() argument types from int to unsigned int
---

Antoni Pokusinski (8):
dt-bindings: rtc: abx80x: document ABX81X RTCs
rtc: abx80x: add mutex protection for register writes
rtc: abx80x: properly handle shared IRQs
rtc: abx80x: add irq to struct abx80x_priv
rtc: abx80x: use regmap instead of I2C specific API
rtc: abx80x: replace read-modify-write pattern with regmap helpers
rtc: abx80x: create abx80x_i2c_probe()
rtc: abx80x: add support for ABX81X

 .../bindings/rtc/abracon,abx80x.yaml          |  18 +-
 drivers/rtc/Kconfig                           |  25 +-
 drivers/rtc/rtc-abx80x.c                      | 639 +++++++++++-------
 3 files changed, 427 insertions(+), 255 deletions(-)
Signature · 1 line
2.55.0
#1 lore raw

PATCH v3 1/8 dt-bindings: rtc: abx80x: document ABX81X RTCs

Abracon AB0815 and AB1815 are RTC modules with SPI interface.

---
 .../bindings/rtc/abracon,abx80x.yaml           | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)
Documentation/devicetree/bindings/rtc/abracon,abx80x.yaml +17 -1
index 355b0598411a..d105570bcd5a 100644
@@ -4,7 +4,7 @@
 $id: http://devicetree.org/schemas/rtc/abracon,abx80x.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Abracon ABX80X I2C ultra low power RTC/Alarm chip
+title: Abracon ABX80X (I2C) and ABX81X (SPI) ultra low power RTC/Alarm chips
 
 maintainers:
   - [email protected]
@@ -21,10 +21,12 @@ properties:
       - abracon,ab0803
       - abracon,ab0804
       - abracon,ab0805
+      - abracon,ab0815
       - abracon,ab1801
       - abracon,ab1803
       - abracon,ab1804
       - abracon,ab1805
+      - abracon,ab1815
       - microcrystal,rv1805
 
   reg:
@@ -61,6 +63,7 @@ required:
   - reg
 
 allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
   - $ref: rtc.yaml#
   - if:
       properties:
@@ -73,6 +76,8 @@ allOf:
                 - abracon,ab1804
                 - abracon,ab0805
                 - abracon,ab1805
+                - abracon,ab0815
+                - abracon,ab1815
     then:
       properties:
         abracon,tc-diode: false
@@ -96,3 +101,14 @@ examples:
             interrupts = <44 IRQ_TYPE_EDGE_FALLING>;
         };
     };
+
+  - |
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        rtc@0 {
+            compatible = "abracon,ab1815";
+            reg = <0>;
+        };
+    };
Signature · 1 line
2.55.0
#2 lore raw

Re: [PATCH v3 1/8] dt-bindings: rtc: abx80x: document ABX81X RTCs

Sashiko has reviewed this patch and found no issues. It looks great!
Quoted 4 lines

Abracon AB0815 and AB1815 are RTC modules with SPI interface.

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
#3 lore raw

PATCH v3 2/8 rtc: abx80x: add mutex protection for register writes

In reply to Antoni Pokusinski

The ABX80X RTC driver performs multi-step register operations such as NVMEM transfers or register writes preceded by a configuration key write. Add a mutex to serialize all the register writes to protect these sequences against race conditions.

---
 drivers/rtc/rtc-abx80x.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)
drivers/rtc/rtc-abx80x.c +29 -0
index 5486d9d0b1e5..8d791b9ec7d2 100644
@@ -15,6 +15,7 @@
 #include <linux/i2c.h>
 #include <linux/kstrtox.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/of.h>
 #include <linux/rtc.h>
 #include <linux/watchdog.h>
@@ -127,6 +128,7 @@ struct abx80x_priv {
 	struct rtc_device *rtc;
 	struct i2c_client *client;
 	struct watchdog_device wdog;
+	struct mutex lock;
 };
 
 static int abx80x_write_config_key(struct i2c_client *client, u8 key)
@@ -219,6 +221,7 @@ static int abx80x_rtc_read_time(struct device *dev, struct rtc_time *tm)
 static int abx80x_rtc_set_time(struct device *dev, struct rtc_time *tm)
 {
 	struct i2c_client *client = to_i2c_client(dev);
+	struct abx80x_priv *priv = i2c_get_clientdata(client);
 	unsigned char buf[8];
 	int err, flags;
 
@@ -234,6 +237,8 @@ static int abx80x_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	buf[ABX8XX_REG_YR] = bin2bcd(tm->tm_year - 100);
 	buf[ABX8XX_REG_WD] = tm->tm_wday;
 
+	guard(mutex)(&priv->lock);
+
 	err = i2c_smbus_write_i2c_block_data(client, ABX8XX_REG_HTH,
 					     sizeof(buf), buf);
 	if (err < 0) {
@@ -263,6 +268,8 @@ static irqreturn_t abx80x_handle_irq(int irq, void *dev_id)
 	struct rtc_device *rtc = priv->rtc;
 	int status;
 
+	guard(mutex)(&priv->lock);
+
 	status = i2c_smbus_read_byte_data(client, ABX8XX_REG_STATUS);
 	if (status < 0)
 		return IRQ_NONE;
@@ -317,6 +324,7 @@ static int abx80x_read_alarm(struct device *dev, struct rtc_wkalrm *t)
 static int abx80x_set_alarm(struct device *dev, struct rtc_wkalrm *t)
 {
 	struct i2c_client *client = to_i2c_client(dev);
+	struct abx80x_priv *priv = i2c_get_clientdata(client);
 	u8 alarm[6];
 	int err;
 
@@ -330,6 +338,8 @@ static int abx80x_set_alarm(struct device *dev, struct rtc_wkalrm *t)
 	alarm[4] = bin2bcd(t->time.tm_mday);
 	alarm[5] = bin2bcd(t->time.tm_mon + 1);
 
+	guard(mutex)(&priv->lock);
+
 	err = i2c_smbus_write_i2c_block_data(client, ABX8XX_REG_AHTH,
 					     sizeof(alarm), alarm);
 	if (err < 0) {
@@ -352,6 +362,7 @@ static int abx80x_rtc_set_autocalibration(struct device *dev,
 					  int autocalibration)
 {
 	struct i2c_client *client = to_i2c_client(dev);
+	struct abx80x_priv *priv = i2c_get_clientdata(client);
 	int retval, flags = 0;
 
 	if ((autocalibration != 0) && (autocalibration != 1024) &&
@@ -360,6 +371,8 @@ static int abx80x_rtc_set_autocalibration(struct device *dev,
 		return -EINVAL;
 	}
 
+	guard(mutex)(&priv->lock);
+
 	flags = i2c_smbus_read_byte_data(client, ABX8XX_REG_OSC);
 	if (flags < 0)
 		return flags;
@@ -443,6 +456,7 @@ static ssize_t oscillator_store(struct device *dev,
 				const char *buf, size_t count)
 {
 	struct i2c_client *client = to_i2c_client(dev->parent);
+	struct abx80x_priv *priv = i2c_get_clientdata(client);
 	int retval, flags, rc_mode = 0;
 
 	if (strncmp(buf, "rc", 2) == 0) {
@@ -454,6 +468,8 @@ static ssize_t oscillator_store(struct device *dev,
 		return -EINVAL;
 	}
 
+	guard(mutex)(&priv->lock);
+
 	flags =  i2c_smbus_read_byte_data(client, ABX8XX_REG_OSC);
 	if (flags < 0)
 		return flags;
@@ -511,8 +527,11 @@ static const struct attribute_group rtc_calib_attr_group = {
 static int abx80x_alarm_irq_enable(struct device *dev, unsigned int enabled)
 {
 	struct i2c_client *client = to_i2c_client(dev);
+	struct abx80x_priv *priv = i2c_get_clientdata(client);
 	int err;
 
+	guard(mutex)(&priv->lock);
+
 	if (enabled)
 		err = i2c_smbus_write_byte_data(client, ABX8XX_REG_IRQ,
 						(ABX8XX_IRQ_IM_1_4 |
@@ -526,6 +545,7 @@ static int abx80x_alarm_irq_enable(struct device *dev, unsigned int enabled)
 static int abx80x_ioctl(struct device *dev, unsigned int cmd, unsigned long arg)
 {
 	struct i2c_client *client = to_i2c_client(dev);
+	struct abx80x_priv *priv = i2c_get_clientdata(client);
 	int status, tmp;
 
 	switch (cmd) {
@@ -539,6 +559,8 @@ static int abx80x_ioctl(struct device *dev, unsigned int cmd, unsigned long arg)
 		return put_user(tmp, (unsigned int __user *)arg);
 
 	case RTC_VL_CLR:
+		guard(mutex)(&priv->lock);
+
 		status = i2c_smbus_read_byte_data(client, ABX8XX_REG_STATUS);
 		if (status < 0)
 			return status;
@@ -617,6 +639,8 @@ static int __abx80x_wdog_set_timeout(struct watchdog_device *wdog,
 	struct abx80x_priv *priv = watchdog_get_drvdata(wdog);
 	u8 val = ABX8XX_WDT_WDS | timeout_bits(timeout);
 
+	guard(mutex)(&priv->lock);
+
 	/*
 	 * Writing any timeout to the WDT register resets the watchdog timer.
 	 * Writing 0 disables it.
@@ -701,6 +725,8 @@ static int abx80x_nvmem_xfer(struct abx80x_priv *priv, unsigned int offset,
 		len = min(lower + bytes, (size_t)ABX8XX_SRAM_WIN_SIZE) - lower;
 		len = min_t(u8, len, I2C_SMBUS_BLOCK_MAX);
 
+		guard(mutex)(&priv->lock);
+
 		ret = i2c_smbus_write_byte_data(priv->client, ABX8XX_REG_EXTRAM,
 						extram);
 		if (ret)
@@ -908,6 +934,9 @@ static int abx80x_probe(struct i2c_client *client)
 
 	priv->rtc->ops = &abx80x_rtc_ops;
 	priv->client = client;
+	err = devm_mutex_init(&client->dev, &priv->lock);
+	if (err)
+		return err;
 
 	i2c_set_clientdata(client, priv);
 
Signature · 1 line
2.55.0
#4 lore raw

Re: [PATCH v3 2/8] rtc: abx80x: add mutex protection for register writes

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Low] Build failure due to jumping over a __cleanup__ variable declaration in a switch statement.

Pre-existing issues:
- [High] Shared IRQ handler unconditionally returns IRQ_HANDLED and clears the status register, risking interrupt storms and breaking battery low detection.
- [High] Missing mutex protection on I2C reads allows them to interleave with and silently break multi-step configuration key write sequences. --

--- Patch [2]: [PATCH v3 2/8] rtc: abx80x: add mutex protection for register writes --- commit d3bc95e6c97e9f4f1dbb461f4986e984a4ccd25a
Author: Antoni Pokusinski <[email protected]>

rtc: abx80x: add mutex protection for register writes

The ABX80X RTC driver performs multi-step register operations such as NVMEM transfers or register writes preceded by a configuration key write. Add a mutex to serialize all the register writes to protect these sequences against race conditions.

Quoted 4 lines
drivers/rtc/rtc-abx80x.c +0 -0
index 5486d9d0b1e5d..8d791b9ec7d21 100644

[ ... ]

drivers/rtc/rtc-abx80x.c static int abx80x_rtc_read_time(struct device *dev, struct rtc_time *tm)

Quoted 1 line
drivers/rtc/rtc-abx80x.c +0 -0
@@ -219,6 +221,7 @@ static int abx80x_rtc_read_time(struct device *dev, struct rtc_time *tm)

[Severity: High]
This is a pre-existing issue, but should I2C reads like those performed in abx80x_rtc_read_time() also be protected by the new mutex?

If an unprotected I2C read interleaves with a protected multi-step write (such as abx80x_rtc_set_autocalibration()), it can reset the configuration key on the hardware. This would cause the target register write to be silently ignored.

Quoted 3 lines
drivers/rtc/rtc-abx80x.c +0 -0
 static int abx80x_rtc_set_time(struct device *dev, struct rtc_time *tm)
 {
 	struct i2c_client *client = to_i2c_client(dev);

[ ... ]

drivers/rtc/rtc-abx80x.c static irqreturn_t abx80x_handle_irq(int irq, void *dev_id) · line 275

Quoted 9 lines
drivers/rtc/rtc-abx80x.c +2 -0
@@ -263,6 +268,8 @@ static irqreturn_t abx80x_handle_irq(int irq, void *dev_id)
 	struct rtc_device *rtc = priv->rtc;
 	int status;
 
+	guard(mutex)(&priv->lock);
+
 	status = i2c_smbus_read_byte_data(client, ABX8XX_REG_STATUS);
 	if (status < 0)
 		return IRQ_NONE;

[Severity: High]
This isn't a bug introduced by this patch, but does this interrupt handler unconditionally clear the status register and return IRQ_HANDLED later in the function?

If a shared device or spurious interrupt triggers abx80x_handle_irq(), returning IRQ_HANDLED for unrelated interrupts prevents the core IRQ subsystem from disabling a stuck shared line.

Additionally, writing 0 unconditionally clears all flags, including ABX8XX_STATUS_BLF, which prevents userspace from detecting a low battery.

[ ... ]

drivers/rtc/rtc-abx80x.c static int abx80x_ioctl(struct device *dev, unsigned int cmd, unsigned long arg) · line 563

Quoted 6 lines
drivers/rtc/rtc-abx80x.c +2 -0
@@ -539,6 +559,8 @@ static int abx80x_ioctl(struct device *dev, unsigned int cmd, unsigned long arg)
 		return put_user(tmp, (unsigned int __user *)arg);
 
 	case RTC_VL_CLR:
+		guard(mutex)(&priv->lock);
+

[Severity: Low]
Does this cause a build failure?

The guard macro declares a cleanup variable inside the switch statement without a surrounding compound block. Any other ioctl command jumping to the default label will bypass the variable's initialization, which GCC usually flags as a hard error.

Quoted 3 lines
drivers/rtc/rtc-abx80x.c +0 -0
 		status = i2c_smbus_read_byte_data(client, ABX8XX_REG_STATUS);
 		if (status < 0)
 			return status;
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
#5 lore raw

Re: [PATCH v3 2/8] rtc: abx80x: add mutex protection for register writes

Quoted 32 lines · On Fri, Jul 31, 2026 at 07:02:55PM +0000, [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Low] Build failure due to jumping over a __cleanup__ variable declaration in a switch statement.

Pre-existing issues:
- [High] Shared IRQ handler unconditionally returns IRQ_HANDLED and clears the status register, risking interrupt storms and breaking battery low detection.
- [High] Missing mutex protection on I2C reads allows them to interleave with and silently break multi-step configuration key write sequences. --

--- Patch [2]: [PATCH v3 2/8] rtc: abx80x: add mutex protection for register writes --- commit d3bc95e6c97e9f4f1dbb461f4986e984a4ccd25a
Author: Antoni Pokusinski <[email protected]>

rtc: abx80x: add mutex protection for register writes

The ABX80X RTC driver performs multi-step register operations such as NVMEM transfers or register writes preceded by a configuration key write. Add a mutex to serialize all the register writes to protect these sequences against race conditions.

Quoted 4 lines
drivers/rtc/rtc-abx80x.c +0 -0
index 5486d9d0b1e5d..8d791b9ec7d21 100644

[ ... ]

drivers/rtc/rtc-abx80x.c static int abx80x_rtc_read_time(struct device *dev, struct rtc_time *tm)

Quoted 1 line
drivers/rtc/rtc-abx80x.c +0 -0
@@ -219,6 +221,7 @@ static int abx80x_rtc_read_time(struct device *dev, struct rtc_time *tm)

[Severity: High]
This is a pre-existing issue, but should I2C reads like those performed in abx80x_rtc_read_time() also be protected by the new mutex?

Quoted 5 lines

If an unprotected I2C read interleaves with a protected multi-step write (such as abx80x_rtc_set_autocalibration()), it can reset the configuration key on the hardware. This would cause the target register write to be silently ignored.

Not true. The reset of the configuration key happens on any register write but on on read.

Quoted 37 lines
Quoted 3 lines
drivers/rtc/rtc-abx80x.c +0 -0
 static int abx80x_rtc_set_time(struct device *dev, struct rtc_time *tm)
 {
 	struct i2c_client *client = to_i2c_client(dev);

[ ... ]

drivers/rtc/rtc-abx80x.c static irqreturn_t abx80x_handle_irq(int irq, void *dev_id) · line 275

Quoted 9 lines
drivers/rtc/rtc-abx80x.c +2 -0
@@ -263,6 +268,8 @@ static irqreturn_t abx80x_handle_irq(int irq, void *dev_id)
 	struct rtc_device *rtc = priv->rtc;
 	int status;
 
+	guard(mutex)(&priv->lock);
+
 	status = i2c_smbus_read_byte_data(client, ABX8XX_REG_STATUS);
 	if (status < 0)
 		return IRQ_NONE;

[Severity: High]
This isn't a bug introduced by this patch, but does this interrupt handler unconditionally clear the status register and return IRQ_HANDLED later in the function?

If a shared device or spurious interrupt triggers abx80x_handle_irq(), returning IRQ_HANDLED for unrelated interrupts prevents the core IRQ subsystem from disabling a stuck shared line.

Additionally, writing 0 unconditionally clears all flags, including ABX8XX_STATUS_BLF, which prevents userspace from detecting a low battery.

[ ... ]

drivers/rtc/rtc-abx80x.c static int abx80x_ioctl(struct device *dev, unsigned int cmd, unsigned long arg) · line 563

Quoted 6 lines
drivers/rtc/rtc-abx80x.c +2 -0
@@ -539,6 +559,8 @@ static int abx80x_ioctl(struct device *dev, unsigned int cmd, unsigned long arg)
 		return put_user(tmp, (unsigned int __user *)arg);
 
 	case RTC_VL_CLR:
+		guard(mutex)(&priv->lock);
+

[Severity: Low]
Does this cause a build failure?

Quoted 5 lines

The guard macro declares a cleanup variable inside the switch statement without a surrounding compound block. Any other ioctl command jumping to the default label will bypass the variable's initialization, which GCC usually flags as a hard error.

Even though I didn't experience any issue, this review seems valid to me. I'd change the guard() here to mutex_lock/unlock in v4

Quoted 6 lines
Quoted 3 lines
drivers/rtc/rtc-abx80x.c +0 -0
 		status = i2c_smbus_read_byte_data(client, ABX8XX_REG_STATUS);
 		if (status < 0)
 			return status;
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
#6 lore raw

PATCH v3 3/8 rtc: abx80x: properly handle shared IRQs

In reply to Antoni Pokusinski

The abx80x RTC driver registers its interrupt handler with IRQF_SHARED, but the handler always returned IRQ_HANDLED and cleared the status register even when no interrupt source from the RTC was active.

This could cause the driver to claim interrupts belonging to other devices sharing the same IRQ line and could also clear unrelated status bits in the RTC status register.

Only handle the interrupt when the alarm or watchdog flags are set, and clear only those interrupt status bits that were handled. Return IRQ_NONE when the interrupt was not generated by the RTC.

Fixes 718a820a303c ("rtc: abx80x: add alarm support")
---
 drivers/rtc/rtc-abx80x.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)
drivers/rtc/rtc-abx80x.c +12 -5
index 8d791b9ec7d2..b84fce0ef294 100644
@@ -266,27 +266,34 @@ static irqreturn_t abx80x_handle_irq(int irq, void *dev_id)
 	struct i2c_client *client = dev_id;
 	struct abx80x_priv *priv = i2c_get_clientdata(client);
 	struct rtc_device *rtc = priv->rtc;
+	irqreturn_t handled = IRQ_NONE;
 	int status;
 
 	guard(mutex)(&priv->lock);
 
 	status = i2c_smbus_read_byte_data(client, ABX8XX_REG_STATUS);
 	if (status < 0)
-		return IRQ_NONE;
+		return handled;
 
-	if (status & ABX8XX_STATUS_AF)
+	if (status & ABX8XX_STATUS_AF) {
 		rtc_update_irq(rtc, 1, RTC_AF | RTC_IRQF);
+		handled = IRQ_HANDLED;
+	}
 
 	/*
 	 * It is unclear if we'll get an interrupt before the external
 	 * reset kicks in.
 	 */
-	if (status & ABX8XX_STATUS_WDT)
+	if (status & ABX8XX_STATUS_WDT) {
 		dev_alert(&client->dev, "watchdog timeout interrupt.\n");
+		handled = IRQ_HANDLED;
+	}
 
-	i2c_smbus_write_byte_data(client, ABX8XX_REG_STATUS, 0);
+	if (handled == IRQ_HANDLED)
+		i2c_smbus_write_byte_data(client, ABX8XX_REG_STATUS,
+					  status & ~(ABX8XX_STATUS_AF | ABX8XX_STATUS_WDT));
 
-	return IRQ_HANDLED;
+	return handled;
 }
 
 static int abx80x_read_alarm(struct device *dev, struct rtc_wkalrm *t)
Signature · 1 line
2.55.0
#7 lore raw

Re: [PATCH v3 3/8] rtc: abx80x: properly handle shared IRQs

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Returning IRQ_NONE without masking or clearing unhandled RTC interrupts can cause an interrupt…

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Returning IRQ_NONE without masking or clearing unhandled RTC interrupts can cause an interrupt storm on the shared IRQ line. --

--- Patch [3]: [PATCH v3 3/8] rtc: abx80x: properly handle shared IRQs --- commit e06155ed4aeb662277df57a2324e881c6d065e84
Author: Antoni Pokusinski <[email protected]>

rtc: abx80x: properly handle shared IRQs

This commit modifies the abx80x interrupt handler to only claim the IRQ and clear the status register if the alarm or watchdog flags are explicitly set. It aims to prevent claiming interrupts from other devices sharing the same IRQ line.

Quoted 3 lines
drivers/rtc/rtc-abx80x.c +0 -0

[ ... ]

Quoted 1 line
drivers/rtc/rtc-abx80x.c +0 -0
@@ -266,27 +266,34 @@ static irqreturn_t abx80x_handle_irq(int irq, void *dev_id)

[ ... ]

drivers/rtc/rtc-abx80x.c static irqreturn_t abx80x_handle_irq(int irq, void *dev_id) · line 296

Quoted 13 lines
drivers/rtc/rtc-abx80x.c +7 -3
-	if (status & ABX8XX_STATUS_WDT)
+	if (status & ABX8XX_STATUS_WDT) {
 		dev_alert(&client->dev, "watchdog timeout interrupt.\n");
+		handled = IRQ_HANDLED;
+	}
 
-	i2c_smbus_write_byte_data(client, ABX8XX_REG_STATUS, 0);
+	if (handled == IRQ_HANDLED)
+		i2c_smbus_write_byte_data(client, ABX8XX_REG_STATUS,
+					  status & ~(ABX8XX_STATUS_AF | ABX8XX_STATUS_WDT));
 
-	return IRQ_HANDLED;
+	return handled;

[Severity: High]
If the RTC hardware asserts an interrupt for a reason other than AF or WDT (for example, a timer or century interrupt left enabled by a bootloader), will this cause an interrupt storm?

Because handled remains IRQ_NONE, the handler skips the i2c_smbus_write_byte_data() call and returns IRQ_NONE without clearing the unhandled RTC status bits.

Since the unhandled RTC interrupt is never cleared, the IRQ line will remain asserted. This causes the kernel to immediately re-enter the handler until the spurious IRQ detector steps in and completely disables the shared IRQ line.

Could we check if the status register indicates the interrupt originated from this RTC, clear those bits, and return IRQ_HANDLED?

Quoted 1 line

}

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
#8 lore raw

Re: [PATCH v3 3/8] rtc: abx80x: properly handle shared IRQs

Quoted 47 lines · On Fri, Jul 31, 2026 at 07:05:22PM +0000, [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Returning IRQ_NONE without masking or clearing unhandled RTC interrupts can cause an interrupt storm on the shared IRQ line. --

--- Patch [3]: [PATCH v3 3/8] rtc: abx80x: properly handle shared IRQs --- commit e06155ed4aeb662277df57a2324e881c6d065e84
Author: Antoni Pokusinski <[email protected]>

rtc: abx80x: properly handle shared IRQs

This commit modifies the abx80x interrupt handler to only claim the IRQ and clear the status register if the alarm or watchdog flags are explicitly set. It aims to prevent claiming interrupts from other devices sharing the same IRQ line.

Quoted 3 lines
drivers/rtc/rtc-abx80x.c +0 -0

[ ... ]

Quoted 1 line
drivers/rtc/rtc-abx80x.c +0 -0
@@ -266,27 +266,34 @@ static irqreturn_t abx80x_handle_irq(int irq, void *dev_id)

[ ... ]

drivers/rtc/rtc-abx80x.c static irqreturn_t abx80x_handle_irq(int irq, void *dev_id) · line 296

Quoted 13 lines
drivers/rtc/rtc-abx80x.c +7 -3
-	if (status & ABX8XX_STATUS_WDT)
+	if (status & ABX8XX_STATUS_WDT) {
 		dev_alert(&client->dev, "watchdog timeout interrupt.\n");
+		handled = IRQ_HANDLED;
+	}
 
-	i2c_smbus_write_byte_data(client, ABX8XX_REG_STATUS, 0);
+	if (handled == IRQ_HANDLED)
+		i2c_smbus_write_byte_data(client, ABX8XX_REG_STATUS,
+					  status & ~(ABX8XX_STATUS_AF | ABX8XX_STATUS_WDT));
 
-	return IRQ_HANDLED;
+	return handled;

[Severity: High]
If the RTC hardware asserts an interrupt for a reason other than AF or WDT (for example, a timer or century interrupt left enabled by a bootloader), will this cause an interrupt storm?

Because handled remains IRQ_NONE, the handler skips the i2c_smbus_write_byte_data() call and returns IRQ_NONE without clearing the unhandled RTC status bits.

Quoted 6 lines

Since the unhandled RTC interrupt is never cleared, the IRQ line will remain asserted. This causes the kernel to immediately re-enter the handler until the spurious IRQ detector steps in and completely disables the shared IRQ line.

Could we check if the status register indicates the interrupt originated from this RTC, clear those bits, and return IRQ_HANDLED?

Ok so it turns out that neither the current nor the newly proposed interrupt handling is correct.

The current approach is that we write 0 to REG_STATUS so we clear e.g. the BL flag which prevents the userspace from detecting a low battery. Also, IRQ_HANDLED is returned unconditionally which prevents handling a potential interrupt from another shared device.

The new approach that I've added in this v3 is not ok neither since (as sashiko points out) any interrupt other than AF or WDT may be enabled e.g. by the bootloader and in this case we'd get an interrupt storm.

In v4 I'll improve the new solution with explicitly disabling the interrupts that are not used in the probe function. That is:
* disable EX1, EX2, Timer, BL interrupts in REG_IRQ
* disable "oscillator fail" and "autocalibration fail" interrupts in REG_OSC

Quoted 5 lines
Quoted 2 lines

}

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
#9 lore raw

PATCH v3 4/8 rtc: abx80x: add irq to struct abx80x_priv

In reply to Antoni Pokusinski

This way the irq number is not tied to the i2c_client and can be easily accessed also in the case of ABX81X SPI clocks.

---
 drivers/rtc/rtc-abx80x.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)
drivers/rtc/rtc-abx80x.c +10 -7
index b84fce0ef294..4ac3ace687a1 100644
@@ -129,6 +129,7 @@ struct abx80x_priv {
 	struct i2c_client *client;
 	struct watchdog_device wdog;
 	struct mutex lock;
+	int irq;
 };
 
 static int abx80x_write_config_key(struct i2c_client *client, u8 key)
@@ -299,11 +300,12 @@ static irqreturn_t abx80x_handle_irq(int irq, void *dev_id)
 static int abx80x_read_alarm(struct device *dev, struct rtc_wkalrm *t)
 {
 	struct i2c_client *client = to_i2c_client(dev);
+	struct abx80x_priv *priv = i2c_get_clientdata(client);
 	unsigned char buf[7];
 
 	int irq_mask, err;
 
-	if (client->irq <= 0)
+	if (priv->irq <= 0)
 		return -EINVAL;
 
 	err = i2c_smbus_read_i2c_block_data(client, ABX8XX_REG_ASC,
@@ -335,7 +337,7 @@ static int abx80x_set_alarm(struct device *dev, struct rtc_wkalrm *t)
 	u8 alarm[6];
 	int err;
 
-	if (client->irq <= 0)
+	if (priv->irq <= 0)
 		return -EINVAL;
 
 	alarm[0] = 0x0;
@@ -941,6 +943,7 @@ static int abx80x_probe(struct i2c_client *client)
 
 	priv->rtc->ops = &abx80x_rtc_ops;
 	priv->client = client;
+	priv->irq = client->irq;
 	err = devm_mutex_init(&client->dev, &priv->lock);
 	if (err)
 		return err;
@@ -957,19 +960,19 @@ static int abx80x_probe(struct i2c_client *client)
 	if (err)
 		return err;
 
-	if (client->irq > 0) {
-		dev_info(&client->dev, "IRQ %d supplied\n", client->irq);
-		err = devm_request_threaded_irq(&client->dev, client->irq, NULL,
+	if (priv->irq > 0) {
+		dev_info(&client->dev, "IRQ %d supplied\n", priv->irq);
+		err = devm_request_threaded_irq(&client->dev, priv->irq, NULL,
 						abx80x_handle_irq,
 						IRQF_SHARED | IRQF_ONESHOT,
 						"abx8xx",
 						client);
 		if (err) {
 			dev_err(&client->dev, "unable to request IRQ, alarms disabled\n");
-			client->irq = 0;
+			priv->irq = 0;
 		}
 	}
-	if (client->irq <= 0)
+	if (priv->irq <= 0)
 		clear_bit(RTC_FEATURE_ALARM, priv->rtc->features);
 
 	err = rtc_add_group(priv->rtc, &rtc_calib_attr_group);
Signature · 1 line
2.55.0
#10 lore raw

Re: [PATCH v3 4/8] rtc: abx80x: add irq to struct abx80x_priv

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Read-modify-write of the status register introduces a race condition that loses interrupts.
- [High] Unconditional failure in abx80x_read_alarm due to incorrect handling of i2c_smbus_read_i2c_block_data return value. --

--- Patch [4]: [PATCH v3 4/8] rtc: abx80x: add irq to struct abx80x_priv --- commit be354d8ef94de08e1314876fad55888243ca019f
Author: Antoni Pokusinski <[email protected]>

rtc: abx80x: add irq to struct abx80x_priv

This adds the irq field to struct abx80x_priv so the interrupt number is not strictly tied to the i2c_client structure. This allows it to be more easily accessed for ABX81X SPI clocks.

drivers/rtc/rtc-abx80x.c static irqreturn_t abx80x_handle_irq(int irq, void *dev_id)

Quoted 4 lines
drivers/rtc/rtc-abx80x.c +0 -0
@@ -299,11 +300,12 @@ static irqreturn_t abx80x_handle_irq(int irq, void *dev_id)

[Severity: High]
This is a pre-existing issue, but I noticed a potential race condition with the status register.

In abx80x_handle_irq(), there is a read-modify-write sequence:

	status = i2c_smbus_read_byte_data(client, ABX8XX_REG_STATUS);
	...
	i2c_smbus_write_byte_data(client, ABX8XX_REG_STATUS,
				  status & ~(ABX8XX_STATUS_AF | ABX8XX_STATUS_WDT));

If a hardware interrupt (such as an RTC alarm or watchdog) sets a new flag in ABX8XX_REG_STATUS exactly between the read and write calls, wouldn't the write operation write back a 0 for the newly set bit and inadvertently clear it without handling it?

This same pattern appears to be present in abx80x_ioctl() as well:

	tmp = i2c_smbus_write_byte_data(client, ABX8XX_REG_STATUS,
					status);

Could this cause hardware interrupts or status flags to be silently dropped?

drivers/rtc/rtc-abx80x.c static irqreturn_t abx80x_handle_irq(int irq, void *dev_id) · line 311

Quoted 13 lines
drivers/rtc/rtc-abx80x.c +2 -1
 static int abx80x_read_alarm(struct device *dev, struct rtc_wkalrm *t)
 {
 	struct i2c_client *client = to_i2c_client(dev);
+	struct abx80x_priv *priv = i2c_get_clientdata(client);
 	unsigned char buf[7];
 
 	int irq_mask, err;
 
-	if (client->irq <= 0)
+	if (priv->irq <= 0)
 		return -EINVAL;
 
 	err = i2c_smbus_read_i2c_block_data(client, ABX8XX_REG_ASC,

[Severity: High]
This isn't a bug introduced by this patch, but does this error check work as intended?

i2c_smbus_read_i2c_block_data() returns the number of bytes read (which would be 7) on success.

Looking at the code just below this in abx80x_read_alarm():

	err = i2c_smbus_read_i2c_block_data(client, ABX8XX_REG_ASC,
					    sizeof(buf), buf);
	if (err)
		return err;

Wouldn't this evaluate to true on a successful read, causing the function to prematurely abort and pass the byte count back as an error code?

[ ... ]

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
#11 lore raw

Re: [PATCH v3 4/8] rtc: abx80x: add irq to struct abx80x_priv

Quoted 68 lines · On Fri, Jul 31, 2026 at 07:04:49PM +0000, [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Read-modify-write of the status register introduces a race condition that loses interrupts.
- [High] Unconditional failure in abx80x_read_alarm due to incorrect handling of i2c_smbus_read_i2c_block_data return value. --

--- Patch [4]: [PATCH v3 4/8] rtc: abx80x: add irq to struct abx80x_priv --- commit be354d8ef94de08e1314876fad55888243ca019f
Author: Antoni Pokusinski <[email protected]>

rtc: abx80x: add irq to struct abx80x_priv

This adds the irq field to struct abx80x_priv so the interrupt number is not strictly tied to the i2c_client structure. This allows it to be more easily accessed for ABX81X SPI clocks.

drivers/rtc/rtc-abx80x.c static irqreturn_t abx80x_handle_irq(int irq, void *dev_id)

Quoted 4 lines
drivers/rtc/rtc-abx80x.c +0 -0
@@ -299,11 +300,12 @@ static irqreturn_t abx80x_handle_irq(int irq, void *dev_id)

[Severity: High]
This is a pre-existing issue, but I noticed a potential race condition with the status register.

In abx80x_handle_irq(), there is a read-modify-write sequence:

	status = i2c_smbus_read_byte_data(client, ABX8XX_REG_STATUS);
	...
	i2c_smbus_write_byte_data(client, ABX8XX_REG_STATUS,
				  status & ~(ABX8XX_STATUS_AF | ABX8XX_STATUS_WDT));

If a hardware interrupt (such as an RTC alarm or watchdog) sets a new flag in ABX8XX_REG_STATUS exactly between the read and write calls, wouldn't the write operation write back a 0 for the newly set bit and inadvertently clear it without handling it?

This same pattern appears to be present in abx80x_ioctl() as well:

	tmp = i2c_smbus_write_byte_data(client, ABX8XX_REG_STATUS,
					status);

Could this cause hardware interrupts or status flags to be silently dropped?

drivers/rtc/rtc-abx80x.c static irqreturn_t abx80x_handle_irq(int irq, void *dev_id) · line 311

Quoted 13 lines
drivers/rtc/rtc-abx80x.c +2 -1
 static int abx80x_read_alarm(struct device *dev, struct rtc_wkalrm *t)
 {
 	struct i2c_client *client = to_i2c_client(dev);
+	struct abx80x_priv *priv = i2c_get_clientdata(client);
 	unsigned char buf[7];
 
 	int irq_mask, err;
 
-	if (client->irq <= 0)
+	if (priv->irq <= 0)
 		return -EINVAL;
 
 	err = i2c_smbus_read_i2c_block_data(client, ABX8XX_REG_ASC,

[Severity: High]
This isn't a bug introduced by this patch, but does this error check work as intended?

i2c_smbus_read_i2c_block_data() returns the number of bytes read (which would be 7) on success.

Looking at the code just below this in abx80x_read_alarm():

Quoted 7 lines
	err = i2c_smbus_read_i2c_block_data(client, ABX8XX_REG_ASC,
					    sizeof(buf), buf);
	if (err)
		return err;

Wouldn't this evaluate to true on a successful read, causing the function to prematurely abort and pass the byte count back as an error code?

Will add a separate patch to fix that in v4 as it may be useful to backport it to stable releases

Quoted 5 lines

[ ... ]

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
#12 lore raw

PATCH v3 5/8 rtc: abx80x: use regmap instead of I2C specific API

In reply to Antoni Pokusinski

Rework the driver so that it uses regmap_* functions for communication with the device instead of the i2c_smbus_* API. This way the support for ABX81X RTCs which use SPI can be added later.

---
 drivers/rtc/Kconfig      |   1 +
 drivers/rtc/rtc-abx80x.c | 354 +++++++++++++++++++++------------------
 2 files changed, 192 insertions(+), 163 deletions(-)
drivers/rtc/Kconfig +1 -0
index 01def8231873..4d14bcba616e 100644
@@ -215,6 +215,7 @@ config RTC_DRV_ABEOZ9
 config RTC_DRV_ABX80X
 	tristate "Abracon ABx80x"
 	select WATCHDOG_CORE if WATCHDOG
+	select REGMAP_I2C
 	help
 	  If you say yes here you get support for Abracon AB080X and AB180X
 	  families of ultra-low-power  battery- and capacitor-backed real-time
drivers/rtc/rtc-abx80x.c +191 -163

707 lines not on this page · raw message

Signature · 1 line
2.55.0
#13 lore raw

Re: [PATCH v3 5/8] rtc: abx80x: use regmap instead of I2C specific API

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] Variables declared as `int` are passed by reference to `regmap_read`, which expects an `unsigned…

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Variables declared as int are passed by reference to regmap_read, which expects an unsigned int *. --

--- Patch [5]: [PATCH v3 5/8] rtc: abx80x: use regmap instead of I2C specific API --- commit 5bae78c37282c7d7693255ad432126ac7e8c51f8
Author: Antoni Pokusinski <[email protected]>

rtc: abx80x: use regmap instead of I2C specific API

This commit reworks the abx80x RTC driver to use regmap API instead of i2c_smbus functions. This prepares the driver for future ABX81X SPI support.

Quoted 4 lines
drivers/rtc/rtc-abx80x.c +0 -0
index 4ac3ace687a1e..23823fc2f5458 100644

[ ... ]

Quoted 10 lines
drivers/rtc/rtc-abx80x.c +1 -2
@@ -221,8 +224,7 @@ static int abx80x_rtc_read_time(struct device *dev, struct rtc_time *tm)
 
 static int abx80x_rtc_set_time(struct device *dev, struct rtc_time *tm)
 {
-	struct i2c_client *client = to_i2c_client(dev);
-	struct abx80x_priv *priv = i2c_get_clientdata(client);
+	struct abx80x_priv *priv = dev_get_drvdata(dev);
 	unsigned char buf[8];
 	int err, flags;
 

[ ... ]

drivers/rtc/rtc-abx80x.c static int abx80x_rtc_set_time(struct device *dev, struct rtc_time *tm) · line 253

Quoted 5 lines
drivers/rtc/rtc-abx80x.c +1 -3
 	/* Clear the OF bit of Oscillator Status Register */
-	flags = i2c_smbus_read_byte_data(client, ABX8XX_REG_OSS);
-	if (flags < 0)
-		return flags;
+	err = regmap_read(priv->regmap, ABX8XX_REG_OSS, &flags);

[Severity: Low]
Will this cause a compiler error on GCC 14+ due to incompatible pointer types?

The regmap_read() function expects an "unsigned int *" for its third argument, but "flags" is declared as an "int" here.

It looks like this same issue with passing an "int *" to regmap_read() also happens with the "flags" variable in abx80x_rtc_set_autocalibration() and oscillator_store(), as well as with the "data" variable in abx80x_probe().

[ ... ]

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5
#14 lore raw

Re: [PATCH v3 5/8] rtc: abx80x: use regmap instead of I2C specific API

Quoted 42 lines · On Fri, Jul 31, 2026 at 07:05:58PM +0000, [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Variables declared as int are passed by reference to regmap_read, which expects an unsigned int *. --

--- Patch [5]: [PATCH v3 5/8] rtc: abx80x: use regmap instead of I2C specific API --- commit 5bae78c37282c7d7693255ad432126ac7e8c51f8
Author: Antoni Pokusinski <[email protected]>

rtc: abx80x: use regmap instead of I2C specific API

This commit reworks the abx80x RTC driver to use regmap API instead of i2c_smbus functions. This prepares the driver for future ABX81X SPI support.

Quoted 4 lines
drivers/rtc/rtc-abx80x.c +0 -0
index 4ac3ace687a1e..23823fc2f5458 100644

[ ... ]

Quoted 10 lines
drivers/rtc/rtc-abx80x.c +1 -2
@@ -221,8 +224,7 @@ static int abx80x_rtc_read_time(struct device *dev, struct rtc_time *tm)
 
 static int abx80x_rtc_set_time(struct device *dev, struct rtc_time *tm)
 {
-	struct i2c_client *client = to_i2c_client(dev);
-	struct abx80x_priv *priv = i2c_get_clientdata(client);
+	struct abx80x_priv *priv = dev_get_drvdata(dev);
 	unsigned char buf[8];
 	int err, flags;
 

[ ... ]

drivers/rtc/rtc-abx80x.c static int abx80x_rtc_set_time(struct device *dev, struct rtc_time *tm) · line 253

Quoted 5 lines
drivers/rtc/rtc-abx80x.c +1 -3
 	/* Clear the OF bit of Oscillator Status Register */
-	flags = i2c_smbus_read_byte_data(client, ABX8XX_REG_OSS);
-	if (flags < 0)
-		return flags;
+	err = regmap_read(priv->regmap, ABX8XX_REG_OSS, &flags);

[Severity: Low]
Will this cause a compiler error on GCC 14+ due to incompatible pointer types?

Quoted 7 lines

The regmap_read() function expects an "unsigned int *" for its third argument, but "flags" is declared as an "int" here.

It looks like this same issue with passing an "int *" to regmap_read() also happens with the "flags" variable in abx80x_rtc_set_autocalibration() and oscillator_store(), as well as with the "data" variable in abx80x_probe().

I compiled this with arm-linux-gnueabihf-gcc (GCC) 15.2.1 and did not experience any issues. But it's no big deal to have make the flags variable an uint, will change that in v4

Quoted 4 lines

[ ... ]

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5
#15 lore raw

PATCH v3 6/8 rtc: abx80x: replace read-modify-write pattern with regmap helpers

In reply to Antoni Pokusinski

Before introducing the regmap usage in the driver, updating specific bits of a register was performed using the read-modify-write pattern. Now, the functions regmap_update_bits() and regmap_write_bits() can be used to simplify the code.

---
 drivers/rtc/rtc-abx80x.c | 109 +++++++++++----------------------------
 1 file changed, 30 insertions(+), 79 deletions(-)
drivers/rtc/rtc-abx80x.c +30 -79
index 23823fc2f545..52f958e2aecd 100644
@@ -226,7 +226,7 @@ static int abx80x_rtc_set_time(struct device *dev, struct rtc_time *tm)
 {
 	struct abx80x_priv *priv = dev_get_drvdata(dev);
 	unsigned char buf[8];
-	int err, flags;
+	int err;
 
 	if (tm->tm_year < 100)
 		return -EINVAL;
@@ -250,18 +250,11 @@ static int abx80x_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	}
 
 	/* Clear the OF bit of Oscillator Status Register */
-	err = regmap_read(priv->regmap, ABX8XX_REG_OSS, &flags);
+	err = regmap_update_bits(priv->regmap, ABX8XX_REG_OSS, ABX8XX_OSS_OF, 0);
 	if (err < 0)
-		return err;
-
-	err = regmap_write(priv->regmap, ABX8XX_REG_OSS,
-			   flags & ~ABX8XX_OSS_OF);
-	if (err < 0) {
 		dev_err(dev, "Unable to write oscillator status register\n");
-		return err;
-	}
 
-	return 0;
+	return err;
 }
 
 static irqreturn_t abx80x_handle_irq(int irq, void *dev_id)
@@ -371,7 +364,7 @@ static int abx80x_rtc_set_autocalibration(struct device *dev,
 					  int autocalibration)
 {
 	struct abx80x_priv *priv = dev_get_drvdata(dev);
-	int retval, flags = 0;
+	int flags = 0;
 
 	if ((autocalibration != 0) && (autocalibration != 1024) &&
 	    (autocalibration != 512)) {
@@ -379,12 +372,6 @@ static int abx80x_rtc_set_autocalibration(struct device *dev,
 		return -EINVAL;
 	}
 
-	guard(mutex)(&priv->lock);
-
-	retval = regmap_read(priv->regmap, ABX8XX_REG_OSC, &flags);
-	if (retval < 0)
-		return retval;
-
 	if (autocalibration == 0) {
 		flags &= ~(ABX8XX_OSC_ACAL_512 | ABX8XX_OSC_ACAL_1024);
 	} else if (autocalibration == 1024) {
@@ -396,13 +383,15 @@ static int abx80x_rtc_set_autocalibration(struct device *dev,
 		flags |= (ABX8XX_OSC_ACAL_1024 | ABX8XX_OSC_ACAL_512);
 	}
 
+	guard(mutex)(&priv->lock);
+
 	/* Unlock write access to Oscillator Control Register */
 	if (abx80x_write_config_key(dev, ABX8XX_CFG_KEY_OSC) < 0)
 		return -EIO;
 
-	retval = regmap_write(priv->regmap, ABX8XX_REG_OSC, flags);
-
-	return retval;
+	return regmap_write_bits(priv->regmap, ABX8XX_REG_OSC,
+				 ABX8XX_OSC_ACAL_1024 | ABX8XX_OSC_ACAL_512,
+				 flags);
 }
 
 static int abx80x_rtc_get_autocalibration(struct device *dev)
@@ -465,7 +454,7 @@ static ssize_t oscillator_store(struct device *dev,
 				const char *buf, size_t count)
 {
 	struct abx80x_priv *priv = dev_get_drvdata(dev->parent);
-	int retval, flags, rc_mode = 0;
+	int retval, rc_mode = 0;
 
 	if (strncmp(buf, "rc", 2) == 0) {
 		rc_mode = 1;
@@ -478,24 +467,14 @@ static ssize_t oscillator_store(struct device *dev,
 
 	guard(mutex)(&priv->lock);
 
-	retval = regmap_read(priv->regmap, ABX8XX_REG_OSC, &flags);
-	if (retval < 0)
-		return retval;
-
-	if (rc_mode == 0)
-		flags &= ~(ABX8XX_OSC_OSEL);
-	else
-		flags |= (ABX8XX_OSC_OSEL);
-
 	/* Unlock write access on Oscillator Control register */
 	if (abx80x_write_config_key(dev->parent, ABX8XX_CFG_KEY_OSC) < 0)
 		return -EIO;
 
-	retval = regmap_write(priv->regmap, ABX8XX_REG_OSC, flags);
-	if (retval < 0) {
+	retval = regmap_write_bits(priv->regmap, ABX8XX_REG_OSC, ABX8XX_OSC_OSEL,
+				   rc_mode == 0 ? 0 : (ABX8XX_OSC_OSEL));
+	if (retval < 0)
 		dev_err(dev, "Failed to write Oscillator Control register\n");
-		return retval;
-	}
 
 	return retval ? retval : count;
 }
@@ -566,17 +545,8 @@ static int abx80x_ioctl(struct device *dev, unsigned int cmd, unsigned long arg)
 	case RTC_VL_CLR:
 		guard(mutex)(&priv->lock);
 
-		err = regmap_read(priv->regmap, ABX8XX_REG_STATUS, &status);
-		if (err < 0)
-			return err;
-
-		status &= ~ABX8XX_STATUS_BLF;
-
-		err = regmap_write(priv->regmap, ABX8XX_REG_STATUS, status);
-		if (err < 0)
-			return err;
-
-		return 0;
+		return regmap_update_bits(priv->regmap, ABX8XX_REG_STATUS,
+					  ABX8XX_STATUS_BLF, 0);
 
 	default:
 		return -ENOIOCTLCMD;
@@ -832,7 +802,7 @@ static int abx80x_probe(struct i2c_client *client)
 	struct regmap *regmap;
 	struct device_node *np = client->dev.of_node;
 	struct abx80x_priv *priv;
-	int i, data, err, trickle_cfg = -EINVAL;
+	int i, err, trickle_cfg = -EINVAL;
 	char buf[7];
 	unsigned int part = (uintptr_t)i2c_get_match_data(client);
 	unsigned int partnumber;
@@ -882,14 +852,9 @@ static int abx80x_probe(struct i2c_client *client)
 	dev_info(&client->dev, "model %04x, revision %u.%u, lot %x, wafer %x, uid %x\n",
 		 partnumber, majrev, minrev, lot, wafer, uid);
 
-	err = regmap_read(regmap, ABX8XX_REG_CTRL1, &data);
-	if (err < 0) {
-		dev_err(&client->dev, "Unable to read control register\n");
-		return -EIO;
-	}
-
-	err = regmap_write(regmap, ABX8XX_REG_CTRL1,
-			   (data & ~(ABX8XX_CTRL_12_24 | ABX8XX_CTRL_ARST)) | ABX8XX_CTRL_WRITE);
+	err = regmap_update_bits(regmap, ABX8XX_REG_CTRL1,
+				 ABX8XX_CTRL_12_24 | ABX8XX_CTRL_ARST | ABX8XX_CTRL_WRITE,
+				 ABX8XX_CTRL_WRITE);
 	if (err < 0) {
 		dev_err(&client->dev, "Unable to write control register\n");
 		return -EIO;
@@ -903,30 +868,10 @@ static int abx80x_probe(struct i2c_client *client)
 		 * register is set. RV-1805-C3 datasheet indicates that
 		 * the bit should be cleared in section 11h - Control2.
 		 */
-		err = regmap_read(regmap, ABX8XX_REG_CTRL2, &data);
-		if (err < 0) {
-			dev_err(&client->dev,
-				"Unable to read control2 register\n");
-			return -EIO;
-		}
-
-		err = regmap_write(regmap, ABX8XX_REG_CTRL2,
-				   data & ~ABX8XX_CTRL2_RSVD);
-		if (err < 0) {
-			dev_err(&client->dev,
-				"Unable to write control2 register\n");
-			return -EIO;
-		}
-
-		/*
-		 * Avoid extra power leakage. The RV1805 uses smaller
-		 * 10pin package and the EXTI input is not present.
-		 * Disable it to avoid leakage.
-		 */
-		err = regmap_read(regmap, ABX8XX_REG_OUT_CTRL, &data);
+		err = regmap_update_bits(regmap, ABX8XX_REG_CTRL2,
+					 ABX8XX_CTRL2_RSVD, 0);
 		if (err < 0) {
-			dev_err(&client->dev,
-				"Unable to read output control register\n");
+			dev_err(&client->dev, "Unable to write control2 register\n");
 			return -EIO;
 		}
 
@@ -937,8 +882,14 @@ static int abx80x_probe(struct i2c_client *client)
 		if (abx80x_write_config_key(&client->dev, ABX8XX_CFG_KEY_MISC) < 0)
 			return -EIO;
 
-		err = regmap_write(regmap, ABX8XX_REG_OUT_CTRL,
-				   data | ABX8XX_OUT_CTRL_EXDS);
+		/*
+		 * Avoid extra power leakage. The RV1805 uses smaller
+		 * 10pin package and the EXTI input is not present.
+		 * Disable it to avoid leakage.
+		 */
+		err = regmap_write_bits(regmap, ABX8XX_REG_OUT_CTRL,
+					ABX8XX_OUT_CTRL_EXDS,
+					ABX8XX_OUT_CTRL_EXDS);
 		if (err < 0) {
 			dev_err(&client->dev,
 				"Unable to write output control register\n");
Signature · 1 line
2.55.0
#16 lore raw

Re: [PATCH v3 6/8] rtc: abx80x: replace read-modify-write pattern with regmap helpers

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Using `guard(mutex)` inside a `switch` statement case without an enclosing block causes a…

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Using guard(mutex) inside a switch statement case without an enclosing block causes a compilation failure (or uninitialized cleanup execution).
- [High] Configuration Key unlock sequence is broken by unprotected concurrent reads and newly introduced read-modify-write regmap helpers. --

--- Patch [6]: [PATCH v3 6/8] rtc: abx80x: replace read-modify-write pattern with regmap helpers --- commit 68e1add63890d6b130ad7ce142d172f06e590b90
Author: Antoni Pokusinski <[email protected]>

rtc: abx80x: replace read-modify-write pattern with regmap helpers

This commit replaces manual read-modify-write register operations in the rtc-abx80x driver with regmap_update_bits() and regmap_write_bits() functions to simplify the code.

Quoted 4 lines
drivers/rtc/rtc-abx80x.c +0 -0
index 23823fc2f5458..52f958e2aecd2 100644

[ ... ]

Quoted 3 lines
drivers/rtc/rtc-abx80x.c +0 -0
@@ -371,7 +364,7 @@ static int abx80x_rtc_set_autocalibration(struct device *dev,
 					  int autocalibration)
 {

[ ... ]

drivers/rtc/rtc-abx80x.c static int abx80x_rtc_set_autocalibration(struct device *dev, · line 395

Quoted 11 lines
drivers/rtc/rtc-abx80x.c +3 -3
 	/* Unlock write access to Oscillator Control Register */
 	if (abx80x_write_config_key(dev, ABX8XX_CFG_KEY_OSC) < 0)
 		return -EIO;
 
-	retval = regmap_write(priv->regmap, ABX8XX_REG_OSC, flags);
-
-	return retval;
+	return regmap_write_bits(priv->regmap, ABX8XX_REG_OSC,
+				 ABX8XX_OSC_ACAL_1024 | ABX8XX_OSC_ACAL_512,
+				 flags);
 }

[Severity: High]
Does switching to regmap_write_bits() break the configuration key unlock sequence here?

The hardware requires writing the configuration key immediately before writing a protected register. If any other register is accessed in between, the unlock state is cleared.

Because regmap_write_bits() performs a read operation internally before writing, does that read happen after the configuration key is written, causing the hardware to relock itself before the write can execute?

Additionally, can an unlocked concurrent read sequence interleave here and clear the unlock state? Looking at abx80x_rtc_read_time(), it performs I2C reads without taking priv->lock:

abx80x_rtc_read_time() {
    ...
    err = regmap_bulk_read(priv->regmap, ABX8XX_REG_HTH, buf, sizeof(buf));
    ...
}

[ ... ]

Quoted 20 lines
drivers/rtc/rtc-abx80x.c +2 -11
@@ -566,17 +545,8 @@ static int abx80x_ioctl(struct device *dev, unsigned int cmd, unsigned long arg)
 	case RTC_VL_CLR:
 		guard(mutex)(&priv->lock);
 
-		err = regmap_read(priv->regmap, ABX8XX_REG_STATUS, &status);
-		if (err < 0)
-			return err;
-
-		status &= ~ABX8XX_STATUS_BLF;
-
-		err = regmap_write(priv->regmap, ABX8XX_REG_STATUS, status);
-		if (err < 0)
-			return err;
-
-		return 0;
+		return regmap_update_bits(priv->regmap, ABX8XX_REG_STATUS,
+					  ABX8XX_STATUS_BLF, 0);
 
 	default:
 		return -ENOIOCTLCMD;
Quoted 1 line

}

[Severity: High]
Will this cause a compiler error or undefined behavior?

The guard(mutex) is declared inside the case block, but without an enclosing scope. If cmd is an unhandled ioctl, the switch statement jumps to the default label, bypassing the initialization of the guard(mutex) variable.

Does this require adding braces around the RTC_VL_CLR case to restrict the lifetime of the guard?

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6
#17 lore raw

Re: [PATCH v3 6/8] rtc: abx80x: replace read-modify-write pattern with regmap helpers

Quoted 52 lines · On Fri, Jul 31, 2026 at 07:01:37PM +0000, [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Using guard(mutex) inside a switch statement case without an enclosing block causes a compilation failure (or uninitialized cleanup execution).
- [High] Configuration Key unlock sequence is broken by unprotected concurrent reads and newly introduced read-modify-write regmap helpers. --

--- Patch [6]: [PATCH v3 6/8] rtc: abx80x: replace read-modify-write pattern with regmap helpers --- commit 68e1add63890d6b130ad7ce142d172f06e590b90
Author: Antoni Pokusinski <[email protected]>

rtc: abx80x: replace read-modify-write pattern with regmap helpers

This commit replaces manual read-modify-write register operations in the rtc-abx80x driver with regmap_update_bits() and regmap_write_bits() functions to simplify the code.

Quoted 4 lines
drivers/rtc/rtc-abx80x.c +0 -0
index 23823fc2f5458..52f958e2aecd2 100644

[ ... ]

Quoted 3 lines
drivers/rtc/rtc-abx80x.c +0 -0
@@ -371,7 +364,7 @@ static int abx80x_rtc_set_autocalibration(struct device *dev,
 					  int autocalibration)
 {

[ ... ]

drivers/rtc/rtc-abx80x.c static int abx80x_rtc_set_autocalibration(struct device *dev, · line 395

Quoted 11 lines
drivers/rtc/rtc-abx80x.c +3 -3
 	/* Unlock write access to Oscillator Control Register */
 	if (abx80x_write_config_key(dev, ABX8XX_CFG_KEY_OSC) < 0)
 		return -EIO;
 
-	retval = regmap_write(priv->regmap, ABX8XX_REG_OSC, flags);
-
-	return retval;
+	return regmap_write_bits(priv->regmap, ABX8XX_REG_OSC,
+				 ABX8XX_OSC_ACAL_1024 | ABX8XX_OSC_ACAL_512,
+				 flags);
 }

[Severity: High]
Does switching to regmap_write_bits() break the configuration key unlock sequence here?

The hardware requires writing the configuration key immediately before writing a protected register. If any other register is accessed in between, the unlock state is cleared.

Because regmap_write_bits() performs a read operation internally before writing, does that read happen after the configuration key is written, causing the hardware to relock itself before the write can execute?

Additionally, can an unlocked concurrent read sequence interleave here and clear the unlock state? Looking at abx80x_rtc_read_time(), it performs I2C reads without taking priv->lock:

Quoted 5 lines
abx80x_rtc_read_time() {
    ...
    err = regmap_bulk_read(priv->regmap, ABX8XX_REG_HTH, buf, sizeof(buf));
    ...
}

Same answer as in patch 2 ("rtc: abx80x: add mutex protection for register writes"):
register reads do not cause a reset of the configuration key, so no issue here.

Quoted 37 lines

[ ... ]

Quoted 20 lines
drivers/rtc/rtc-abx80x.c +2 -11
@@ -566,17 +545,8 @@ static int abx80x_ioctl(struct device *dev, unsigned int cmd, unsigned long arg)
 	case RTC_VL_CLR:
 		guard(mutex)(&priv->lock);
 
-		err = regmap_read(priv->regmap, ABX8XX_REG_STATUS, &status);
-		if (err < 0)
-			return err;
-
-		status &= ~ABX8XX_STATUS_BLF;
-
-		err = regmap_write(priv->regmap, ABX8XX_REG_STATUS, status);
-		if (err < 0)
-			return err;
-
-		return 0;
+		return regmap_update_bits(priv->regmap, ABX8XX_REG_STATUS,
+					  ABX8XX_STATUS_BLF, 0);
 
 	default:
 		return -ENOIOCTLCMD;
Quoted 1 line

}

[Severity: High]
Will this cause a compiler error or undefined behavior?

The guard(mutex) is declared inside the case block, but without an enclosing scope. If cmd is an unhandled ioctl, the switch statement jumps to the default label, bypassing the initialization of the guard(mutex) variable.

Does this require adding braces around the RTC_VL_CLR case to restrict the lifetime of the guard?

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6
#18 lore raw

PATCH v3 7/8 rtc: abx80x: create abx80x_i2c_probe()

In reply to Antoni Pokusinski

Move the I2C-specific code from abx80x_probe() to the newly created function. This is a preparation for introducing the support for ABX81X RTCs which use SPI interface.

---
 drivers/rtc/rtc-abx80x.c | 167 ++++++++++++++++++++++++---------------
 1 file changed, 103 insertions(+), 64 deletions(-)
drivers/rtc/rtc-abx80x.c +103 -64
index 52f958e2aecd..6583e097af68 100644
@@ -773,73 +773,38 @@ static const struct regmap_access_table abx80x_write_table = {
 	.n_no_ranges = ARRAY_SIZE(abx80x_no_write_ranges),
 };
 
-static const struct regmap_config abx80x_regmap_config_i2c = {
-	.reg_bits = 8,
-	.val_bits = 8,
-	.max_register = ABX8XX_SRAM_BASE + ABX8XX_SRAM_WIN_SIZE - 1,
-
-	.rd_table = &abx80x_read_table,
-	.wr_table = &abx80x_write_table,
-};
-
-static const struct i2c_device_id abx80x_id[] = {
-	{ .name = "abx80x", .driver_data = ABX80X },
-	{ .name = "ab0801", .driver_data = AB0801 },
-	{ .name = "ab0803", .driver_data = AB0803 },
-	{ .name = "ab0804", .driver_data = AB0804 },
-	{ .name = "ab0805", .driver_data = AB0805 },
-	{ .name = "ab1801", .driver_data = AB1801 },
-	{ .name = "ab1803", .driver_data = AB1803 },
-	{ .name = "ab1804", .driver_data = AB1804 },
-	{ .name = "ab1805", .driver_data = AB1805 },
-	{ .name = "rv1805", .driver_data = RV1805 },
-	{ }
-};
-MODULE_DEVICE_TABLE(i2c, abx80x_id);
-
-static int abx80x_probe(struct i2c_client *client)
+static int abx80x_probe(struct device *dev, struct regmap *regmap, int irq,
+			struct device_node *np, unsigned int part)
 {
-	struct regmap *regmap;
-	struct device_node *np = client->dev.of_node;
 	struct abx80x_priv *priv;
 	int i, err, trickle_cfg = -EINVAL;
 	char buf[7];
-	unsigned int part = (uintptr_t)i2c_get_match_data(client);
 	unsigned int partnumber;
 	unsigned int majrev, minrev;
 	unsigned int lot;
 	unsigned int wafer;
 	unsigned int uid;
 
-	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
-		return -ENODEV;
-
-	regmap = devm_regmap_init_i2c(client, &abx80x_regmap_config_i2c);
-	if (IS_ERR(regmap)) {
-		dev_err(&client->dev, "Unable to allocate regmap\n");
-		return PTR_ERR(regmap);
-	}
-
-	priv = devm_kzalloc(&client->dev, sizeof(*priv), GFP_KERNEL);
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (priv == NULL)
 		return -ENOMEM;
 
-	priv->rtc = devm_rtc_allocate_device(&client->dev);
+	priv->rtc = devm_rtc_allocate_device(dev);
 	if (IS_ERR(priv->rtc))
 		return PTR_ERR(priv->rtc);
 
 	priv->rtc->ops = &abx80x_rtc_ops;
-	priv->irq = client->irq;
+	priv->irq = irq;
 	priv->regmap = regmap;
-	err = devm_mutex_init(&client->dev, &priv->lock);
+	err = devm_mutex_init(dev, &priv->lock);
 	if (err)
 		return err;
 
-	dev_set_drvdata(&client->dev, priv);
+	dev_set_drvdata(dev, priv);
 
 	err = regmap_bulk_read(regmap, ABX8XX_REG_ID0, buf, sizeof(buf));
 	if (err < 0) {
-		dev_err(&client->dev, "Unable to read partnumber\n");
+		dev_err(dev, "Unable to read partnumber\n");
 		return -EIO;
 	}
 
@@ -849,14 +814,14 @@ static int abx80x_probe(struct i2c_client *client)
 	lot = ((buf[4] & 0x80) << 2) | ((buf[6] & 0x80) << 1) | buf[3];
 	uid = ((buf[4] & 0x7f) << 8) | buf[5];
 	wafer = (buf[6] & 0x7c) >> 2;
-	dev_info(&client->dev, "model %04x, revision %u.%u, lot %x, wafer %x, uid %x\n",
+	dev_info(dev, "model %04x, revision %u.%u, lot %x, wafer %x, uid %x\n",
 		 partnumber, majrev, minrev, lot, wafer, uid);
 
 	err = regmap_update_bits(regmap, ABX8XX_REG_CTRL1,
 				 ABX8XX_CTRL_12_24 | ABX8XX_CTRL_ARST | ABX8XX_CTRL_WRITE,
 				 ABX8XX_CTRL_WRITE);
 	if (err < 0) {
-		dev_err(&client->dev, "Unable to write control register\n");
+		dev_err(dev, "Unable to write control register\n");
 		return -EIO;
 	}
 
@@ -871,7 +836,7 @@ static int abx80x_probe(struct i2c_client *client)
 		err = regmap_update_bits(regmap, ABX8XX_REG_CTRL2,
 					 ABX8XX_CTRL2_RSVD, 0);
 		if (err < 0) {
-			dev_err(&client->dev, "Unable to write control2 register\n");
+			dev_err(dev, "Unable to write control2 register\n");
 			return -EIO;
 		}
 
@@ -879,7 +844,7 @@ static int abx80x_probe(struct i2c_client *client)
 		 * Write the configuration key register to enable access to
 		 * the config2 register
 		 */
-		if (abx80x_write_config_key(&client->dev, ABX8XX_CFG_KEY_MISC) < 0)
+		if (abx80x_write_config_key(dev, ABX8XX_CFG_KEY_MISC) < 0)
 			return -EIO;
 
 		/*
@@ -891,7 +856,7 @@ static int abx80x_probe(struct i2c_client *client)
 					ABX8XX_OUT_CTRL_EXDS,
 					ABX8XX_OUT_CTRL_EXDS);
 		if (err < 0) {
-			dev_err(&client->dev,
+			dev_err(dev,
 				"Unable to write output control register\n");
 			return -EIO;
 		}
@@ -903,26 +868,24 @@ static int abx80x_probe(struct i2c_client *client)
 			if (partnumber == abx80x_caps[i].pn)
 				break;
 		if (abx80x_caps[i].pn == 0) {
-			dev_err(&client->dev, "Unknown part: %04x\n",
-				partnumber);
+			dev_err(dev, "Unknown part: %04x\n", partnumber);
 			return -EINVAL;
 		}
 		part = i;
 	}
 
 	if (partnumber != abx80x_caps[part].pn) {
-		dev_err(&client->dev, "partnumber mismatch %04x != %04x\n",
+		dev_err(dev, "partnumber mismatch %04x != %04x\n",
 			partnumber, abx80x_caps[part].pn);
 		return -EINVAL;
 	}
 
 	if (np && abx80x_caps[part].has_tc)
-		trickle_cfg = abx80x_dt_trickle_cfg(&client->dev);
+		trickle_cfg = abx80x_dt_trickle_cfg(dev);
 
 	if (trickle_cfg > 0) {
-		dev_info(&client->dev, "Enabling trickle charger: %02x\n",
-			 trickle_cfg);
-		abx80x_enable_trickle_charger(&client->dev, trickle_cfg);
+		dev_info(dev, "Enabling trickle charger: %02x\n", trickle_cfg);
+		abx80x_enable_trickle_charger(dev, trickle_cfg);
 	}
 
 	err = regmap_write(regmap, ABX8XX_REG_CD_TIMER_CTL, BIT(2));
@@ -930,7 +893,7 @@ static int abx80x_probe(struct i2c_client *client)
 		return err;
 
 	if (abx80x_caps[part].has_wdog) {
-		err = abx80x_setup_watchdog(&client->dev);
+		err = abx80x_setup_watchdog(dev);
 		if (err)
 			return err;
 	}
@@ -940,14 +903,14 @@ static int abx80x_probe(struct i2c_client *client)
 		return err;
 
 	if (priv->irq > 0) {
-		dev_info(&client->dev, "IRQ %d supplied\n", priv->irq);
-		err = devm_request_threaded_irq(&client->dev, priv->irq, NULL,
+		dev_info(dev, "IRQ %d supplied\n", priv->irq);
+		err = devm_request_threaded_irq(dev, priv->irq, NULL,
 						abx80x_handle_irq,
 						IRQF_SHARED | IRQF_ONESHOT,
 						"abx8xx",
-						&client->dev);
+						dev);
 		if (err) {
-			dev_err(&client->dev, "unable to request IRQ, alarms disabled\n");
+			dev_err(dev, "unable to request IRQ, alarms disabled\n");
 			priv->irq = 0;
 		}
 	}
@@ -956,14 +919,39 @@ static int abx80x_probe(struct i2c_client *client)
 
 	err = rtc_add_group(priv->rtc, &rtc_calib_attr_group);
 	if (err) {
-		dev_err(&client->dev, "Failed to create sysfs group: %d\n",
-			err);
+		dev_err(dev, "Failed to create sysfs group: %d\n", err);
 		return err;
 	}
 
 	return devm_rtc_register_device(priv->rtc);
 }
 
+#if IS_ENABLED(CONFIG_I2C)
+
+static const struct regmap_config abx80x_regmap_config_i2c = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = ABX8XX_SRAM_BASE + ABX8XX_SRAM_WIN_SIZE - 1,
+
+	.rd_table = &abx80x_read_table,
+	.wr_table = &abx80x_write_table,
+};
+
+static const struct i2c_device_id abx80x_id[] = {
+	{ .name = "abx80x", .driver_data = ABX80X },
+	{ .name = "ab0801", .driver_data = AB0801 },
+	{ .name = "ab0803", .driver_data = AB0803 },
+	{ .name = "ab0804", .driver_data = AB0804 },
+	{ .name = "ab0805", .driver_data = AB0805 },
+	{ .name = "ab1801", .driver_data = AB1801 },
+	{ .name = "ab1803", .driver_data = AB1803 },
+	{ .name = "ab1804", .driver_data = AB1804 },
+	{ .name = "ab1805", .driver_data = AB1805 },
+	{ .name = "rv1805", .driver_data = RV1805 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, abx80x_id);
+
 #ifdef CONFIG_OF
 static const struct of_device_id abx80x_of_match[] = {
 	{
@@ -1011,16 +999,67 @@ static const struct of_device_id abx80x_of_match[] = {
 MODULE_DEVICE_TABLE(of, abx80x_of_match);
 #endif
 
+static int abx80x_i2c_probe(struct i2c_client *client)
+{
+	unsigned int part = (uintptr_t)i2c_get_match_data(client);
+	struct regmap *regmap;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
+		return -ENODEV;
+
+	regmap = devm_regmap_init_i2c(client, &abx80x_regmap_config_i2c);
+	if (IS_ERR(regmap)) {
+		dev_err(&client->dev, "Unable to allocate regmap\n");
+		return PTR_ERR(regmap);
+	}
+
+	return abx80x_probe(&client->dev, regmap, client->irq,
+			    client->dev.of_node, part);
+}
+
 static struct i2c_driver abx80x_driver = {
 	.driver		= {
 		.name	= "rtc-abx80x",
 		.of_match_table = of_match_ptr(abx80x_of_match),
 	},
-	.probe		= abx80x_probe,
+	.probe		= abx80x_i2c_probe,
 	.id_table	= abx80x_id,
 };
 
-module_i2c_driver(abx80x_driver);
+static int abx80x_register_driver(void)
+{
+	return i2c_add_driver(&abx80x_driver);
+}
+
+static void abx80x_unregister_driver(void)
+{
+	i2c_del_driver(&abx80x_driver);
+}
+
+#else
+
+static int abx80x_register_driver(void)
+{
+	return 0;
+}
+
+static void abx80x_unregister_driver(void)
+{
+}
+
+#endif /* IS_ENABLED(CONFIG_I2C) */
+
+static int __init abx80x_init(void)
+{
+	return abx80x_register_driver();
+}
+module_init(abx80x_init);
+
+static void __exit abx80x_exit(void)
+{
+	abx80x_unregister_driver();
+}
+module_exit(abx80x_exit);
 
 MODULE_AUTHOR("Philippe De Muyter <[email protected]>");
 MODULE_AUTHOR("Alexandre Belloni <[email protected]>");
Signature · 1 line
2.55.0
#19 lore raw

Re: [PATCH v3 7/8] rtc: abx80x: create abx80x_i2c_probe()

Sashiko has reviewed this patch and found no issues. It looks great!
Quoted 5 lines

Move the I2C-specific code from abx80x_probe() to the newly created function. This is a preparation for introducing the support for ABX81X RTCs which use SPI interface.

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7
#20 lore raw

PATCH v3 8/8 rtc: abx80x: add support for ABX81X

In reply to Antoni Pokusinski

Abracon ABX81X is a family of RTCs with SPI interface featuring the same functionalities as the ABX80X I2C clocks.

---
 drivers/rtc/Kconfig      |  26 +++++-----
 drivers/rtc/rtc-abx80x.c | 106 +++++++++++++++++++++++++++++++++++++--
 2 files changed, 115 insertions(+), 17 deletions(-)
drivers/rtc/Kconfig +14 -12
index 4d14bcba616e..d35221f81024 100644
@@ -212,18 +212,6 @@ config RTC_DRV_ABEOZ9
 	  This driver can also be built as a module. If so, the module
 	  will be called rtc-ab-e0z9.
 
-config RTC_DRV_ABX80X
-	tristate "Abracon ABx80x"
-	select WATCHDOG_CORE if WATCHDOG
-	select REGMAP_I2C
-	help
-	  If you say yes here you get support for Abracon AB080X and AB180X
-	  families of ultra-low-power  battery- and capacitor-backed real-time
-	  clock chips.
-
-	  This driver can also be built as a module. If so, the module
-	  will be called rtc-abx80x.
-
 config RTC_DRV_AC100
 	tristate "X-Powers AC100"
 	depends on MFD_AC100
@@ -955,6 +943,20 @@ config RTC_I2C_AND_SPI
 
 comment "SPI and I2C RTC drivers"
 
+config RTC_DRV_ABX80X
+	tristate "Abracon ABx80x"
+	depends on RTC_I2C_AND_SPI
+	select WATCHDOG_CORE if WATCHDOG
+	select REGMAP_I2C if I2C
+	select REGMAP_SPI if SPI_MASTER
+	help
+	  If you say yes here you get support for Abracon AB080X, AB180X,
+	  AB081X and AB181X families of ultra-low-power  battery- and
+	  capacitor-backed real-time clock chips.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called rtc-abx80x.
+
 config RTC_DRV_DS3232
 	tristate "Dallas/Maxim DS3232/DS3234"
 	depends on RTC_I2C_AND_SPI
drivers/rtc/rtc-abx80x.c +101 -5

146 lines not on this page · raw message

Signature · 1 line
2.55.0
#21 lore raw

Re: [PATCH v3 8/8] rtc: abx80x: add support for ABX81X

Sashiko has reviewed this patch and found no issues. It looks great!
Quoted 4 lines

Abracon ABX81X is a family of RTCs with SPI interface featuring the same functionalities as the ABX80X I2C clocks.

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8