* [PATCH 1/6] dt-bindings: rtc: abx80x: document ABX81X RTCs
2026-07-22 19:58 [PATCH 0/6] rtc: abx80x: add support for ABX81X Antoni Pokusinski
@ 2026-07-22 19:58 ` Antoni Pokusinski
2026-07-22 20:09 ` sashiko-bot
2026-07-22 19:58 ` [PATCH 2/6] rtc: abx80x: add irq to struct abx80x_priv Antoni Pokusinski
` (4 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Antoni Pokusinski @ 2026-07-22 19:58 UTC (permalink / raw)
To: alexandre.belloni, robh, krzk+dt, conor+dt
Cc: linux-rtc, devicetree, Antoni Pokusinski
Abracon AB0815 and AB1815 are RTC modules with SPI interface.
Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com>
---
.../devicetree/bindings/rtc/abracon,abx80x.yaml | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/rtc/abracon,abx80x.yaml b/Documentation/devicetree/bindings/rtc/abracon,abx80x.yaml
index 355b0598411a..234a103724b5 100644
--- a/Documentation/devicetree/bindings/rtc/abracon,abx80x.yaml
+++ b/Documentation/devicetree/bindings/rtc/abracon,abx80x.yaml
@@ -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:
- linux-rtc@vger.kernel.org
@@ -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:
@@ -96,3 +98,14 @@ examples:
interrupts = <44 IRQ_TYPE_EDGE_FALLING>;
};
};
+
+ - |
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ rtc@0 {
+ compatible = "abracon,ab1815";
+ reg = <0>;
+ };
+ };
--
2.55.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 1/6] dt-bindings: rtc: abx80x: document ABX81X RTCs
2026-07-22 19:58 ` [PATCH 1/6] dt-bindings: rtc: abx80x: document ABX81X RTCs Antoni Pokusinski
@ 2026-07-22 20:09 ` sashiko-bot
0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-07-22 20:09 UTC (permalink / raw)
To: Antoni Pokusinski
Cc: Alexandre Belloni, linux-rtc, conor+dt, devicetree, robh
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The newly added SPI compatible strings `abracon,ab0815` and `abracon,ab1815` were omitted from the `if-then` conditional block that restricts trickle charger properties.
- [Low] The schema explicitly introduces SPI support but fails to reference the standard SPI peripheral properties schema, causing common SPI properties to be rejected.
--
--- Patch [1]: [PATCH 1/6] dt-bindings: rtc: abx80x: document ABX81X RTCs ---
commit 74ad54b7ffcb981494fb52031a1edb5415b809ae
Author: Antoni Pokusinski <apokusinski01@gmail.com>
dt-bindings: rtc: abx80x: document ABX81X RTCs
Abracon AB0815 and AB1815 are RTC modules with SPI interface.
> diff --git a/Documentation/devicetree/bindings/rtc/abracon,abx80x.yaml b/Documentation/devicetree/bindings/rtc/abracon,abx80x.yaml
> index 355b0598411a6..234a103724b56 100644
> --- a/Documentation/devicetree/bindings/rtc/abracon,abx80x.yaml
> +++ b/Documentation/devicetree/bindings/rtc/abracon,abx80x.yaml
[ ... ]
> @@ -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
[Severity: Low]
Will the schema validator reject standard trickle charger properties for the
newly added AB0815 and AB1815 compatible strings?
Looking at the conditional block later in the file:
allOf:
- $ref: rtc.yaml#
- if:
properties:
compatible:
not:
contains:
enum:
- abracon,abx80x
- abracon,ab0804
- abracon,ab1804
- abracon,ab0805
- abracon,ab1805
Because abracon,ab0815 and abracon,ab1815 were omitted from this enum list,
abracon,tc-diode and abracon,tc-resistor will evaluate to false for these
variants. Should they be added to the list?
[ ... ]
> @@ -96,3 +98,14 @@ examples:
> interrupts = <44 IRQ_TYPE_EDGE_FALLING>;
> };
> };
> +
> + - |
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + rtc@0 {
> + compatible = "abracon,ab1815";
> + reg = <0>;
> + };
> + };
[Severity: Low]
Does this binding need to reference the standard SPI peripheral properties?
Because the schema adds SPI support and specifies unevaluatedProperties: false,
standard SPI properties like spi-max-frequency will be rejected by the
validator.
Could a reference to /schemas/spi/spi-peripheral-props.yaml# be added to the
allOf block to permit these common SPI properties?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260722195814.298552-1-apokusinski01@gmail.com?part=1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/6] rtc: abx80x: add irq to struct abx80x_priv
2026-07-22 19:58 [PATCH 0/6] rtc: abx80x: add support for ABX81X Antoni Pokusinski
2026-07-22 19:58 ` [PATCH 1/6] dt-bindings: rtc: abx80x: document ABX81X RTCs Antoni Pokusinski
@ 2026-07-22 19:58 ` Antoni Pokusinski
2026-07-22 20:13 ` sashiko-bot
2026-07-22 19:58 ` [PATCH 3/6] rtc: abx80x: use regmap instead of I2C specific API Antoni Pokusinski
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Antoni Pokusinski @ 2026-07-22 19:58 UTC (permalink / raw)
To: alexandre.belloni, robh, krzk+dt, conor+dt
Cc: linux-rtc, devicetree, 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.
Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com>
---
drivers/rtc/rtc-abx80x.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/rtc/rtc-abx80x.c b/drivers/rtc/rtc-abx80x.c
index 5486d9d0b1e5..9b0e1d48d9ba 100644
--- a/drivers/rtc/rtc-abx80x.c
+++ b/drivers/rtc/rtc-abx80x.c
@@ -127,6 +127,7 @@ struct abx80x_priv {
struct rtc_device *rtc;
struct i2c_client *client;
struct watchdog_device wdog;
+ int irq;
};
static int abx80x_write_config_key(struct i2c_client *client, u8 key)
@@ -285,11 +286,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,
@@ -317,10 +319,11 @@ 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;
- if (client->irq <= 0)
+ if (priv->irq <= 0)
return -EINVAL;
alarm[0] = 0x0;
@@ -908,6 +911,7 @@ static int abx80x_probe(struct i2c_client *client)
priv->rtc->ops = &abx80x_rtc_ops;
priv->client = client;
+ priv->irq = client->irq;
i2c_set_clientdata(client, priv);
@@ -921,19 +925,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);
--
2.55.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 2/6] rtc: abx80x: add irq to struct abx80x_priv
2026-07-22 19:58 ` [PATCH 2/6] rtc: abx80x: add irq to struct abx80x_priv Antoni Pokusinski
@ 2026-07-22 20:13 ` sashiko-bot
0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-07-22 20:13 UTC (permalink / raw)
To: Antoni Pokusinski
Cc: robh, conor+dt, linux-rtc, devicetree, Alexandre Belloni
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] The shared IRQ handler unconditionally returns IRQ_HANDLED and inadvertently clears all status bits.
- [High] The `abx80x_read_alarm` function fails unconditionally on success due to incorrect error checking.
--
--- Patch [2]: [PATCH 2/6] rtc: abx80x: add irq to struct abx80x_priv ---
commit ccef71986338cc749e8440b88d073968d465ea5c
Author: Antoni Pokusinski <apokusinski01@gmail.com>
rtc: abx80x: add irq to struct abx80x_priv
This patch moves the irq number into struct abx80x_priv so that it
is no longer tied to the i2c_client. This allows the irq number to
be accessed in the case of ABX81X SPI clocks.
> diff --git a/drivers/rtc/rtc-abx80x.c b/drivers/rtc/rtc-abx80x.c
> index 5486d9d0b1e5d..9b0e1d48d9ba3 100644
> --- a/drivers/rtc/rtc-abx80x.c
> +++ b/drivers/rtc/rtc-abx80x.c
[ ... ]
> @@ -285,11 +286,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,
[Severity: High]
This is a pre-existing issue, but doesn't i2c_smbus_read_i2c_block_data()
return the number of bytes read on success?
Looking at drivers/rtc/rtc-abx80x.c:abx80x_read_alarm():
err = i2c_smbus_read_i2c_block_data(client, ABX8XX_REG_ASC,
sizeof(buf), buf);
if (err)
return err;
Since it returns 7 on success, will this unconditionally fail and return
early with an error code, leaving the alarm structure uninitialized?
[ ... ]
> @@ -921,19 +925,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,
[Severity: High]
This isn't a bug introduced by this patch, but since the IRQ is registered
here with IRQF_SHARED, does abx80x_handle_irq() correctly handle shared
interrupts?
Looking at drivers/rtc/rtc-abx80x.c:abx80x_handle_irq():
static irqreturn_t abx80x_handle_irq(int irq, void *dev_id)
{
[ ... ]
status = i2c_smbus_read_byte_data(client, ABX8XX_REG_STATUS);
if (status < 0)
return IRQ_NONE;
[ ... ]
i2c_smbus_write_byte_data(client, ABX8XX_REG_STATUS, 0);
return IRQ_HANDLED;
}
If no relevant interrupt flags are set for this device, it still returns
IRQ_HANDLED and writes 0 to ABX8XX_REG_STATUS, clearing all status bits
including non-IRQ bits like BLF. Can this break other devices sharing the
interrupt line and silently drop hardware status flags?
> "abx8xx",
> client);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260722195814.298552-1-apokusinski01@gmail.com?part=2
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/6] rtc: abx80x: use regmap instead of I2C specific API
2026-07-22 19:58 [PATCH 0/6] rtc: abx80x: add support for ABX81X Antoni Pokusinski
2026-07-22 19:58 ` [PATCH 1/6] dt-bindings: rtc: abx80x: document ABX81X RTCs Antoni Pokusinski
2026-07-22 19:58 ` [PATCH 2/6] rtc: abx80x: add irq to struct abx80x_priv Antoni Pokusinski
@ 2026-07-22 19:58 ` Antoni Pokusinski
2026-07-22 20:16 ` sashiko-bot
2026-07-22 19:58 ` [PATCH 4/6] rtc: abx80x: replace read-modify-write pattern with regmap helpers Antoni Pokusinski
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Antoni Pokusinski @ 2026-07-22 19:58 UTC (permalink / raw)
To: alexandre.belloni, robh, krzk+dt, conor+dt
Cc: linux-rtc, devicetree, 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.
Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com>
---
drivers/rtc/Kconfig | 1 +
drivers/rtc/rtc-abx80x.c | 331 +++++++++++++++++++++------------------
2 files changed, 180 insertions(+), 152 deletions(-)
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 01def8231873..4d14bcba616e 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -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
diff --git a/drivers/rtc/rtc-abx80x.c b/drivers/rtc/rtc-abx80x.c
index 9b0e1d48d9ba..e5ba821a6768 100644
--- a/drivers/rtc/rtc-abx80x.c
+++ b/drivers/rtc/rtc-abx80x.c
@@ -16,6 +16,7 @@
#include <linux/kstrtox.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/regmap.h>
#include <linux/rtc.h>
#include <linux/watchdog.h>
@@ -125,52 +126,53 @@ static struct abx80x_cap abx80x_caps[] = {
struct abx80x_priv {
struct rtc_device *rtc;
- struct i2c_client *client;
+ struct regmap *regmap;
struct watchdog_device wdog;
int irq;
};
-static int abx80x_write_config_key(struct i2c_client *client, u8 key)
+static int abx80x_write_config_key(struct device *dev, u8 key)
{
- if (i2c_smbus_write_byte_data(client, ABX8XX_REG_CFG_KEY, key) < 0) {
- dev_err(&client->dev, "Unable to write configuration key\n");
+ struct abx80x_priv *priv = dev_get_drvdata(dev);
+
+ if (regmap_write(priv->regmap, ABX8XX_REG_CFG_KEY, key) < 0) {
+ dev_err(dev, "Unable to write configuration key\n");
return -EIO;
}
return 0;
}
-static int abx80x_is_rc_mode(struct i2c_client *client)
+static int abx80x_is_rc_mode(struct device *dev)
{
- int flags = 0;
+ struct abx80x_priv *priv = dev_get_drvdata(dev);
+ int err, flags = 0;
- flags = i2c_smbus_read_byte_data(client, ABX8XX_REG_OSS);
- if (flags < 0) {
- dev_err(&client->dev,
- "Failed to read autocalibration attribute\n");
- return flags;
+ err = regmap_read(priv->regmap, ABX8XX_REG_OSS, &flags);
+ if (err < 0) {
+ dev_err(dev, "Failed to read autocalibration attribute\n");
+ return err;
}
return (flags & ABX8XX_OSS_OMODE) ? 1 : 0;
}
-static int abx80x_enable_trickle_charger(struct i2c_client *client,
- u8 trickle_cfg)
+static int abx80x_enable_trickle_charger(struct device *dev, u8 trickle_cfg)
{
+ struct abx80x_priv *priv = dev_get_drvdata(dev);
int err;
/*
* Write the configuration key register to enable access to the Trickle
* register
*/
- if (abx80x_write_config_key(client, ABX8XX_CFG_KEY_MISC) < 0)
+ if (abx80x_write_config_key(dev, ABX8XX_CFG_KEY_MISC) < 0)
return -EIO;
- err = i2c_smbus_write_byte_data(client, ABX8XX_REG_TRICKLE,
- ABX8XX_TRICKLE_CHARGE_ENABLE |
- trickle_cfg);
+ err = regmap_write(priv->regmap, ABX8XX_REG_TRICKLE,
+ ABX8XX_TRICKLE_CHARGE_ENABLE | trickle_cfg);
if (err < 0) {
- dev_err(&client->dev, "Unable to write trickle register\n");
+ dev_err(dev, "Unable to write trickle register\n");
return -EIO;
}
@@ -179,19 +181,19 @@ static int abx80x_enable_trickle_charger(struct i2c_client *client,
static int abx80x_rtc_read_time(struct device *dev, struct rtc_time *tm)
{
- struct i2c_client *client = to_i2c_client(dev);
+ struct abx80x_priv *priv = dev_get_drvdata(dev);
unsigned char buf[8];
int err, flags, rc_mode = 0;
/* Read the Oscillator Failure only in XT mode */
- rc_mode = abx80x_is_rc_mode(client);
+ rc_mode = abx80x_is_rc_mode(dev);
if (rc_mode < 0)
return rc_mode;
if (!rc_mode) {
- flags = i2c_smbus_read_byte_data(client, ABX8XX_REG_OSS);
- if (flags < 0)
- return flags;
+ err = regmap_read(priv->regmap, ABX8XX_REG_OSS, &flags);
+ if (err < 0)
+ return err;
if (flags & ABX8XX_OSS_OF) {
dev_err(dev, "Oscillator failure, data is invalid.\n");
@@ -199,10 +201,9 @@ static int abx80x_rtc_read_time(struct device *dev, struct rtc_time *tm)
}
}
- err = i2c_smbus_read_i2c_block_data(client, ABX8XX_REG_HTH,
- sizeof(buf), buf);
+ err = regmap_bulk_read(priv->regmap, ABX8XX_REG_HTH, buf, sizeof(buf));
if (err < 0) {
- dev_err(&client->dev, "Unable to read date\n");
+ dev_err(dev, "Unable to read date\n");
return -EIO;
}
@@ -219,7 +220,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 = dev_get_drvdata(dev);
unsigned char buf[8];
int err, flags;
@@ -235,22 +236,22 @@ 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;
- err = i2c_smbus_write_i2c_block_data(client, ABX8XX_REG_HTH,
- sizeof(buf), buf);
+ err = regmap_bulk_write(priv->regmap, ABX8XX_REG_HTH, buf,
+ sizeof(buf));
if (err < 0) {
- dev_err(&client->dev, "Unable to write to date registers\n");
+ dev_err(dev, "Unable to write to date registers\n");
return -EIO;
}
/* 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);
+ if (err < 0)
+ return err;
- err = i2c_smbus_write_byte_data(client, ABX8XX_REG_OSS,
- flags & ~ABX8XX_OSS_OF);
+ err = regmap_write(priv->regmap, ABX8XX_REG_OSS,
+ flags & ~ABX8XX_OSS_OF);
if (err < 0) {
- dev_err(&client->dev, "Unable to write oscillator status register\n");
+ dev_err(dev, "Unable to write oscillator status register\n");
return err;
}
@@ -259,13 +260,13 @@ static int abx80x_rtc_set_time(struct device *dev, struct rtc_time *tm)
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 device *dev = dev_id;
+ struct abx80x_priv *priv = dev_get_drvdata(dev);
struct rtc_device *rtc = priv->rtc;
- int status;
+ int err, status;
- status = i2c_smbus_read_byte_data(client, ABX8XX_REG_STATUS);
- if (status < 0)
+ err = regmap_read(priv->regmap, ABX8XX_REG_STATUS, &status);
+ if (err < 0)
return IRQ_NONE;
if (status & ABX8XX_STATUS_AF)
@@ -276,17 +277,16 @@ static irqreturn_t abx80x_handle_irq(int irq, void *dev_id)
* reset kicks in.
*/
if (status & ABX8XX_STATUS_WDT)
- dev_alert(&client->dev, "watchdog timeout interrupt.\n");
+ dev_alert(dev, "watchdog timeout interrupt.\n");
- i2c_smbus_write_byte_data(client, ABX8XX_REG_STATUS, 0);
+ regmap_write(priv->regmap, ABX8XX_REG_STATUS, 0);
return IRQ_HANDLED;
}
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);
+ struct abx80x_priv *priv = dev_get_drvdata(dev);
unsigned char buf[7];
int irq_mask, err;
@@ -294,14 +294,13 @@ static int abx80x_read_alarm(struct device *dev, struct rtc_wkalrm *t)
if (priv->irq <= 0)
return -EINVAL;
- err = i2c_smbus_read_i2c_block_data(client, ABX8XX_REG_ASC,
- sizeof(buf), buf);
+ err = regmap_bulk_read(priv->regmap, ABX8XX_REG_ASC, buf, sizeof(buf));
if (err)
return err;
- irq_mask = i2c_smbus_read_byte_data(client, ABX8XX_REG_IRQ);
- if (irq_mask < 0)
- return irq_mask;
+ err = regmap_read(priv->regmap, ABX8XX_REG_IRQ, &irq_mask);
+ if (err < 0)
+ return err;
t->time.tm_sec = bcd2bin(buf[0] & 0x7F);
t->time.tm_min = bcd2bin(buf[1] & 0x7F);
@@ -318,8 +317,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);
+ struct abx80x_priv *priv = dev_get_drvdata(dev);
u8 alarm[6];
int err;
@@ -333,17 +331,16 @@ 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);
- err = i2c_smbus_write_i2c_block_data(client, ABX8XX_REG_AHTH,
- sizeof(alarm), alarm);
+ err = regmap_bulk_write(priv->regmap, ABX8XX_REG_AHTH,
+ alarm, sizeof(alarm));
if (err < 0) {
- dev_err(&client->dev, "Unable to write alarm registers\n");
+ dev_err(dev, "Unable to write alarm registers\n");
return -EIO;
}
if (t->enabled) {
- err = i2c_smbus_write_byte_data(client, ABX8XX_REG_IRQ,
- (ABX8XX_IRQ_IM_1_4 |
- ABX8XX_IRQ_AIE));
+ err = regmap_write(priv->regmap, ABX8XX_REG_IRQ,
+ ABX8XX_IRQ_IM_1_4 | ABX8XX_IRQ_AIE);
if (err)
return err;
}
@@ -354,7 +351,7 @@ static int abx80x_set_alarm(struct device *dev, struct rtc_wkalrm *t)
static int abx80x_rtc_set_autocalibration(struct device *dev,
int autocalibration)
{
- struct i2c_client *client = to_i2c_client(dev);
+ struct abx80x_priv *priv = dev_get_drvdata(dev);
int retval, flags = 0;
if ((autocalibration != 0) && (autocalibration != 1024) &&
@@ -363,9 +360,9 @@ static int abx80x_rtc_set_autocalibration(struct device *dev,
return -EINVAL;
}
- flags = i2c_smbus_read_byte_data(client, ABX8XX_REG_OSC);
- if (flags < 0)
- return flags;
+ 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);
@@ -379,22 +376,22 @@ static int abx80x_rtc_set_autocalibration(struct device *dev,
}
/* Unlock write access to Oscillator Control Register */
- if (abx80x_write_config_key(client, ABX8XX_CFG_KEY_OSC) < 0)
+ if (abx80x_write_config_key(dev, ABX8XX_CFG_KEY_OSC) < 0)
return -EIO;
- retval = i2c_smbus_write_byte_data(client, ABX8XX_REG_OSC, flags);
+ retval = regmap_write(priv->regmap, ABX8XX_REG_OSC, flags);
return retval;
}
static int abx80x_rtc_get_autocalibration(struct device *dev)
{
- struct i2c_client *client = to_i2c_client(dev);
- int flags = 0, autocalibration;
+ struct abx80x_priv *priv = dev_get_drvdata(dev);
+ int flags = 0, autocalibration, err;
- flags = i2c_smbus_read_byte_data(client, ABX8XX_REG_OSC);
- if (flags < 0)
- return flags;
+ err = regmap_read(priv->regmap, ABX8XX_REG_OSC, &flags);
+ if (err < 0)
+ return err;
if (flags & ABX8XX_OSC_ACAL_512)
autocalibration = 512;
@@ -445,7 +442,7 @@ static ssize_t oscillator_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
- struct i2c_client *client = to_i2c_client(dev->parent);
+ struct abx80x_priv *priv = dev_get_drvdata(dev->parent);
int retval, flags, rc_mode = 0;
if (strncmp(buf, "rc", 2) == 0) {
@@ -457,9 +454,9 @@ static ssize_t oscillator_store(struct device *dev,
return -EINVAL;
}
- flags = i2c_smbus_read_byte_data(client, ABX8XX_REG_OSC);
- if (flags < 0)
- return flags;
+ retval = regmap_read(priv->regmap, ABX8XX_REG_OSC, &flags);
+ if (retval < 0)
+ return retval;
if (rc_mode == 0)
flags &= ~(ABX8XX_OSC_OSEL);
@@ -467,10 +464,10 @@ static ssize_t oscillator_store(struct device *dev,
flags |= (ABX8XX_OSC_OSEL);
/* Unlock write access on Oscillator Control register */
- if (abx80x_write_config_key(client, ABX8XX_CFG_KEY_OSC) < 0)
+ if (abx80x_write_config_key(dev->parent, ABX8XX_CFG_KEY_OSC) < 0)
return -EIO;
- retval = i2c_smbus_write_byte_data(client, ABX8XX_REG_OSC, flags);
+ retval = regmap_write(priv->regmap, ABX8XX_REG_OSC, flags);
if (retval < 0) {
dev_err(dev, "Failed to write Oscillator Control register\n");
return retval;
@@ -483,9 +480,8 @@ static ssize_t oscillator_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
int rc_mode = 0;
- struct i2c_client *client = to_i2c_client(dev->parent);
- rc_mode = abx80x_is_rc_mode(client);
+ rc_mode = abx80x_is_rc_mode(dev->parent);
if (rc_mode < 0) {
dev_err(dev, "Failed to read RTC oscillator selection\n");
@@ -513,45 +509,43 @@ 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 = dev_get_drvdata(dev);
int err;
if (enabled)
- err = i2c_smbus_write_byte_data(client, ABX8XX_REG_IRQ,
- (ABX8XX_IRQ_IM_1_4 |
- ABX8XX_IRQ_AIE));
+ err = regmap_write(priv->regmap, ABX8XX_REG_IRQ,
+ ABX8XX_IRQ_IM_1_4 | ABX8XX_IRQ_AIE);
else
- err = i2c_smbus_write_byte_data(client, ABX8XX_REG_IRQ,
- ABX8XX_IRQ_IM_1_4);
+ err = regmap_write(priv->regmap, ABX8XX_REG_IRQ,
+ ABX8XX_IRQ_IM_1_4);
return err;
}
static int abx80x_ioctl(struct device *dev, unsigned int cmd, unsigned long arg)
{
- struct i2c_client *client = to_i2c_client(dev);
- int status, tmp;
+ struct abx80x_priv *priv = dev_get_drvdata(dev);
+ int err, status, tmp;
switch (cmd) {
case RTC_VL_READ:
- status = i2c_smbus_read_byte_data(client, ABX8XX_REG_STATUS);
- if (status < 0)
- return status;
+ err = regmap_read(priv->regmap, ABX8XX_REG_STATUS, &status);
+ if (err < 0)
+ return err;
tmp = status & ABX8XX_STATUS_BLF ? RTC_VL_BACKUP_LOW : 0;
return put_user(tmp, (unsigned int __user *)arg);
case RTC_VL_CLR:
- status = i2c_smbus_read_byte_data(client, ABX8XX_REG_STATUS);
- if (status < 0)
- return status;
+ err = regmap_read(priv->regmap, ABX8XX_REG_STATUS, &status);
+ if (err < 0)
+ return err;
status &= ~ABX8XX_STATUS_BLF;
- tmp = i2c_smbus_write_byte_data(client, ABX8XX_REG_STATUS,
- status);
- if (tmp < 0)
- return tmp;
+ err = regmap_write(priv->regmap, ABX8XX_REG_STATUS, status);
+ if (err < 0)
+ return err;
return 0;
@@ -569,9 +563,9 @@ static const struct rtc_class_ops abx80x_rtc_ops = {
.ioctl = abx80x_ioctl,
};
-static int abx80x_dt_trickle_cfg(struct i2c_client *client)
+static int abx80x_dt_trickle_cfg(struct device *dev)
{
- struct device_node *np = client->dev.of_node;
+ struct device_node *np = dev->of_node;
const char *diode;
int trickle_cfg = 0;
int i, ret;
@@ -586,7 +580,7 @@ static int abx80x_dt_trickle_cfg(struct i2c_client *client)
} else if (!strcmp(diode, "schottky")) {
trickle_cfg |= ABX8XX_TRICKLE_SCHOTTKY_DIODE;
} else {
- dev_dbg(&client->dev, "Invalid tc-diode value: %s\n", diode);
+ dev_dbg(dev, "Invalid tc-diode value: %s\n", diode);
return -EINVAL;
}
@@ -599,7 +593,7 @@ static int abx80x_dt_trickle_cfg(struct i2c_client *client)
break;
if (i == sizeof(trickle_resistors)) {
- dev_dbg(&client->dev, "Invalid tc-resistor value: %u\n", tmp);
+ dev_dbg(dev, "Invalid tc-resistor value: %u\n", tmp);
return -EINVAL;
}
@@ -624,7 +618,7 @@ static int __abx80x_wdog_set_timeout(struct watchdog_device *wdog,
* Writing any timeout to the WDT register resets the watchdog timer.
* Writing 0 disables it.
*/
- return i2c_smbus_write_byte_data(priv->client, ABX8XX_REG_WDT, val);
+ return regmap_write(priv->regmap, ABX8XX_REG_WDT, val);
}
static int abx80x_wdog_set_timeout(struct watchdog_device *wdog,
@@ -669,9 +663,11 @@ static const struct watchdog_ops abx80x_wdog_ops = {
.set_timeout = abx80x_wdog_set_timeout,
};
-static int abx80x_setup_watchdog(struct abx80x_priv *priv)
+static int abx80x_setup_watchdog(struct device *dev)
{
- priv->wdog.parent = &priv->client->dev;
+ struct abx80x_priv *priv = dev_get_drvdata(dev);
+
+ priv->wdog.parent = dev;
priv->wdog.ops = &abx80x_wdog_ops;
priv->wdog.info = &abx80x_wdog_info;
priv->wdog.min_timeout = 1;
@@ -680,10 +676,10 @@ static int abx80x_setup_watchdog(struct abx80x_priv *priv)
watchdog_set_drvdata(&priv->wdog, priv);
- return devm_watchdog_register_device(&priv->client->dev, &priv->wdog);
+ return devm_watchdog_register_device(dev, &priv->wdog);
}
#else
-static int abx80x_setup_watchdog(struct abx80x_priv *priv)
+static int abx80x_setup_watchdog(struct device *dev)
{
return 0;
}
@@ -695,31 +691,27 @@ static int abx80x_nvmem_xfer(struct abx80x_priv *priv, unsigned int offset,
int ret;
while (bytes) {
- u8 extram, reg, len, lower, upper;
+ u8 reg, len, lower, upper;
lower = FIELD_GET(NVMEM_ADDR_LOWER, offset);
upper = FIELD_GET(NVMEM_ADDR_UPPER, offset);
- extram = FIELD_PREP(ABX8XX_EXTRAM_XADS, upper);
reg = ABX8XX_SRAM_BASE + lower;
len = min(lower + bytes, (size_t)ABX8XX_SRAM_WIN_SIZE) - lower;
len = min_t(u8, len, I2C_SMBUS_BLOCK_MAX);
- ret = i2c_smbus_write_byte_data(priv->client, ABX8XX_REG_EXTRAM,
- extram);
+ ret = regmap_update_bits(priv->regmap, ABX8XX_REG_EXTRAM,
+ ABX8XX_EXTRAM_XADS, upper);
if (ret)
return ret;
if (write) {
- ret = i2c_smbus_write_i2c_block_data(priv->client, reg,
- len, val);
+ ret = regmap_bulk_write(priv->regmap, reg, val, len);
if (ret)
return ret;
} else {
- ret = i2c_smbus_read_i2c_block_data(priv->client, reg,
- len, val);
- if (ret <= 0)
- return ret ? ret : -EIO;
- len = ret;
+ ret = regmap_bulk_read(priv->regmap, reg, val, len);
+ if (ret)
+ return ret;
}
offset += len;
@@ -755,6 +747,38 @@ static int abx80x_setup_nvmem(struct abx80x_priv *priv)
return devm_rtc_nvmem_register(priv->rtc, &config);
}
+static const struct regmap_range abx80x_no_read_ranges[] = {
+ regmap_reg_range(0x1e, 0x1e),
+ regmap_reg_range(0x22, 0x25),
+ regmap_reg_range(0x31, 0x3e),
+};
+
+static const struct regmap_range abx80x_no_write_ranges[] = {
+ regmap_reg_range(0x1e, 0x1e),
+ regmap_reg_range(0x22, 0x25),
+ regmap_reg_range(ABX8XX_REG_ID0, ABX8XX_REG_ID0 + 6),
+ regmap_reg_range(0x31, 0x3e),
+};
+
+static const struct regmap_access_table abx80x_read_table = {
+ .no_ranges = abx80x_no_read_ranges,
+ .n_no_ranges = ARRAY_SIZE(abx80x_no_read_ranges),
+};
+
+static const struct regmap_access_table abx80x_write_table = {
+ .no_ranges = abx80x_no_write_ranges,
+ .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 },
@@ -772,6 +796,7 @@ MODULE_DEVICE_TABLE(i2c, abx80x_id);
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;
@@ -786,8 +811,27 @@ static int abx80x_probe(struct i2c_client *client)
if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
return -ENODEV;
- err = i2c_smbus_read_i2c_block_data(client, ABX8XX_REG_ID0,
- sizeof(buf), buf);
+ regmap = devm_regmap_init_i2c(client, &abx80x_regmap_config_i2c);
+ if (IS_ERR(regmap)) {
+ dev_err(&client->dev, "Unable to allocate regmap\n");
+ return -EIO;
+ }
+
+ priv = devm_kzalloc(&client->dev, sizeof(*priv), GFP_KERNEL);
+ if (priv == NULL)
+ return -ENOMEM;
+
+ priv->rtc = devm_rtc_allocate_device(&client->dev);
+ if (IS_ERR(priv->rtc))
+ return PTR_ERR(priv->rtc);
+
+ priv->rtc->ops = &abx80x_rtc_ops;
+ priv->irq = client->irq;
+ priv->regmap = regmap;
+
+ dev_set_drvdata(&client->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");
return -EIO;
@@ -802,16 +846,14 @@ 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);
- data = i2c_smbus_read_byte_data(client, ABX8XX_REG_CTRL1);
- if (data < 0) {
+ err = regmap_read(regmap, ABX8XX_REG_CTRL1, &data);
+ if (err < 0) {
dev_err(&client->dev, "Unable to read control register\n");
return -EIO;
}
- err = i2c_smbus_write_byte_data(client, ABX8XX_REG_CTRL1,
- ((data & ~(ABX8XX_CTRL_12_24 |
- ABX8XX_CTRL_ARST)) |
- ABX8XX_CTRL_WRITE));
+ err = regmap_write(regmap, ABX8XX_REG_CTRL1,
+ (data & ~(ABX8XX_CTRL_12_24 | ABX8XX_CTRL_ARST)) | ABX8XX_CTRL_WRITE);
if (err < 0) {
dev_err(&client->dev, "Unable to write control register\n");
return -EIO;
@@ -825,15 +867,15 @@ 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.
*/
- data = i2c_smbus_read_byte_data(client, ABX8XX_REG_CTRL2);
- if (data < 0) {
+ err = regmap_read(regmap, ABX8XX_REG_CTRL2, &data);
+ if (err < 0) {
dev_err(&client->dev,
"Unable to read control2 register\n");
return -EIO;
}
- err = i2c_smbus_write_byte_data(client, ABX8XX_REG_CTRL2,
- data & ~ABX8XX_CTRL2_RSVD);
+ err = regmap_write(regmap, ABX8XX_REG_CTRL2,
+ data & ~ABX8XX_CTRL2_RSVD);
if (err < 0) {
dev_err(&client->dev,
"Unable to write control2 register\n");
@@ -845,8 +887,8 @@ static int abx80x_probe(struct i2c_client *client)
* 10pin package and the EXTI input is not present.
* Disable it to avoid leakage.
*/
- data = i2c_smbus_read_byte_data(client, ABX8XX_REG_OUT_CTRL);
- if (data < 0) {
+ err = regmap_read(regmap, ABX8XX_REG_OUT_CTRL, &data);
+ if (err < 0) {
dev_err(&client->dev,
"Unable to read output control register\n");
return -EIO;
@@ -856,11 +898,11 @@ 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, ABX8XX_CFG_KEY_MISC) < 0)
+ if (abx80x_write_config_key(&client->dev, ABX8XX_CFG_KEY_MISC) < 0)
return -EIO;
- err = i2c_smbus_write_byte_data(client, ABX8XX_REG_OUT_CTRL,
- data | ABX8XX_OUT_CTRL_EXDS);
+ err = regmap_write(regmap, ABX8XX_REG_OUT_CTRL,
+ data | ABX8XX_OUT_CTRL_EXDS);
if (err < 0) {
dev_err(&client->dev,
"Unable to write output control register\n");
@@ -888,35 +930,20 @@ static int abx80x_probe(struct i2c_client *client)
}
if (np && abx80x_caps[part].has_tc)
- trickle_cfg = abx80x_dt_trickle_cfg(client);
+ trickle_cfg = abx80x_dt_trickle_cfg(&client->dev);
if (trickle_cfg > 0) {
dev_info(&client->dev, "Enabling trickle charger: %02x\n",
trickle_cfg);
- abx80x_enable_trickle_charger(client, trickle_cfg);
+ abx80x_enable_trickle_charger(&client->dev, trickle_cfg);
}
- err = i2c_smbus_write_byte_data(client, ABX8XX_REG_CD_TIMER_CTL,
- BIT(2));
+ err = regmap_write(regmap, ABX8XX_REG_CD_TIMER_CTL, BIT(2));
if (err)
return err;
- priv = devm_kzalloc(&client->dev, sizeof(*priv), GFP_KERNEL);
- if (priv == NULL)
- return -ENOMEM;
-
- priv->rtc = devm_rtc_allocate_device(&client->dev);
- if (IS_ERR(priv->rtc))
- return PTR_ERR(priv->rtc);
-
- priv->rtc->ops = &abx80x_rtc_ops;
- priv->client = client;
- priv->irq = client->irq;
-
- i2c_set_clientdata(client, priv);
-
if (abx80x_caps[part].has_wdog) {
- err = abx80x_setup_watchdog(priv);
+ err = abx80x_setup_watchdog(&client->dev);
if (err)
return err;
}
@@ -931,7 +958,7 @@ static int abx80x_probe(struct i2c_client *client)
abx80x_handle_irq,
IRQF_SHARED | IRQF_ONESHOT,
"abx8xx",
- client);
+ &client->dev);
if (err) {
dev_err(&client->dev, "unable to request IRQ, alarms disabled\n");
priv->irq = 0;
--
2.55.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 3/6] rtc: abx80x: use regmap instead of I2C specific API
2026-07-22 19:58 ` [PATCH 3/6] rtc: abx80x: use regmap instead of I2C specific API Antoni Pokusinski
@ 2026-07-22 20:16 ` sashiko-bot
0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-07-22 20:16 UTC (permalink / raw)
To: Antoni Pokusinski
Cc: linux-rtc, robh, conor+dt, Alexandre Belloni, devicetree
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Hardware unlock sequence is broken by read-modify-write inversion
- [Low] Incompatible pointer types passed to regmap_read()
--
--- Patch [3]: [PATCH 3/6] rtc: abx80x: use regmap instead of I2C specific API ---
commit 97cd1f33d168c96f848a8f59ba284bfe9c5f89fe
Author: Antoni Pokusinski <apokusinski01@gmail.com>
Date: Wed Jul 22 19:58:11 2026 +0000
rtc: abx80x: use regmap instead of I2C specific API
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.
> diff --git a/drivers/rtc/rtc-abx80x.c b/drivers/rtc/rtc-abx80x.c
> index 9b0e1d48d9ba3..e5ba821a67689 100644
> --- a/drivers/rtc/rtc-abx80x.c
> +++ b/drivers/rtc/rtc-abx80x.c
> @@ -125,52 +126,53 @@ static struct abx80x_cap abx80x_caps[] = {
[ ... ]
> -static int abx80x_is_rc_mode(struct i2c_client *client)
> +static int abx80x_is_rc_mode(struct device *dev)
> {
> - int flags = 0;
> + struct abx80x_priv *priv = dev_get_drvdata(dev);
> + int err, flags = 0;
>
> - flags = i2c_smbus_read_byte_data(client, ABX8XX_REG_OSS);
> - if (flags < 0) {
> - dev_err(&client->dev,
> - "Failed to read autocalibration attribute\n");
> - return flags;
> + err = regmap_read(priv->regmap, ABX8XX_REG_OSS, &flags);
[Severity: Low]
Does this pass an incompatible pointer type to regmap_read()?
The API signature of regmap_read() expects an unsigned int * for its third
argument, but the address of a local int variable (&flags) is passed here.
While this may not cause memory corruption since int and unsigned int are
the same size, it violates the function signature and generates
-Wincompatible-pointer-types warnings, which can break builds compiling with
-Werror.
> + if (err < 0) {
> + dev_err(dev, "Failed to read autocalibration attribute\n");
> + return err;
> }
[ ... ]
> @@ -259,13 +260,13 @@ static int abx80x_rtc_set_time(struct device *dev, struct rtc_time *tm)
>
> 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 device *dev = dev_id;
> + struct abx80x_priv *priv = dev_get_drvdata(dev);
> struct rtc_device *rtc = priv->rtc;
> - int status;
> + int err, status;
>
> - status = i2c_smbus_read_byte_data(client, ABX8XX_REG_STATUS);
> - if (status < 0)
> + err = regmap_read(priv->regmap, ABX8XX_REG_STATUS, &status);
[Severity: Low]
Does this pass an incompatible pointer type to regmap_read()?
Similar to abx80x_is_rc_mode() above, this passes an int pointer (&status)
instead of an unsigned int pointer.
> + if (err < 0)
> return IRQ_NONE;
[ ... ]
> @@ -294,14 +294,13 @@ static int abx80x_read_alarm(struct device *dev, struct rtc_wkalrm *t)
> if (priv->irq <= 0)
> return -EINVAL;
>
> - err = i2c_smbus_read_i2c_block_data(client, ABX8XX_REG_ASC,
> - sizeof(buf), buf);
> + err = regmap_bulk_read(priv->regmap, ABX8XX_REG_ASC, buf, sizeof(buf));
> if (err)
> return err;
>
> - irq_mask = i2c_smbus_read_byte_data(client, ABX8XX_REG_IRQ);
> - if (irq_mask < 0)
> - return irq_mask;
> + err = regmap_read(priv->regmap, ABX8XX_REG_IRQ, &irq_mask);
[Severity: Low]
Does this pass an incompatible pointer type to regmap_read()?
This passes an int pointer (&irq_mask) instead of an unsigned int pointer.
> + if (err < 0)
> + return err;
[ ... ]
> @@ -379,22 +376,22 @@ static int abx80x_rtc_set_autocalibration(struct device *dev,
> }
>
> /* Unlock write access to Oscillator Control Register */
> - if (abx80x_write_config_key(client, ABX8XX_CFG_KEY_OSC) < 0)
> + if (abx80x_write_config_key(dev, ABX8XX_CFG_KEY_OSC) < 0)
> return -EIO;
>
> - retval = i2c_smbus_write_byte_data(client, ABX8XX_REG_OSC, flags);
> + retval = regmap_write(priv->regmap, ABX8XX_REG_OSC, flags);
[Severity: High]
Does this hardware unlock sequence get broken by read-modify-write inversion?
According to the datasheet, any subsequent I2C access (read or write) resets
the configuration key to zero. If this sequence uses a read-modify-write
operation (such as regmap_write_bits()), a hardware read is performed after
the unlock key is written. This read resets the configuration key, causing the
hardware to ignore the subsequent write and breaking features like
autocalibration.
>
> return retval;
> }
>
> static int abx80x_rtc_get_autocalibration(struct device *dev)
> {
> - struct i2c_client *client = to_i2c_client(dev);
> - int flags = 0, autocalibration;
> + struct abx80x_priv *priv = dev_get_drvdata(dev);
> + int flags = 0, autocalibration, err;
>
> - flags = i2c_smbus_read_byte_data(client, ABX8XX_REG_OSC);
> - if (flags < 0)
> - return flags;
> + err = regmap_read(priv->regmap, ABX8XX_REG_OSC, &flags);
[Severity: Low]
Does this pass an incompatible pointer type to regmap_read()?
This passes an int pointer (&flags) instead of an unsigned int pointer.
> + if (err < 0)
> + return err;
[ ... ]
> @@ -467,10 +464,10 @@ static ssize_t oscillator_store(struct device *dev,
> flags |= (ABX8XX_OSC_OSEL);
>
> /* Unlock write access on Oscillator Control register */
> - if (abx80x_write_config_key(client, ABX8XX_CFG_KEY_OSC) < 0)
> + if (abx80x_write_config_key(dev->parent, ABX8XX_CFG_KEY_OSC) < 0)
> return -EIO;
>
> - retval = i2c_smbus_write_byte_data(client, ABX8XX_REG_OSC, flags);
> + retval = regmap_write(priv->regmap, ABX8XX_REG_OSC, flags);
[Severity: High]
Does this hardware unlock sequence get broken by read-modify-write inversion?
As noted in abx80x_rtc_set_autocalibration() above, a hardware read performed
after the unlock key is written clears the key. This resets the key before the
protected write if a read-modify-write operation like regmap_write_bits() is
used.
> if (retval < 0) {
> dev_err(dev, "Failed to write Oscillator Control register\n");
[ ... ]
> @@ -513,45 +509,43 @@ static const struct attribute_group rtc_calib_attr_group = {
>
> 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);
> - int status, tmp;
> + struct abx80x_priv *priv = dev_get_drvdata(dev);
> + int err, status, tmp;
>
> switch (cmd) {
> case RTC_VL_READ:
> - status = i2c_smbus_read_byte_data(client, ABX8XX_REG_STATUS);
> - if (status < 0)
> - return status;
> + err = regmap_read(priv->regmap, ABX8XX_REG_STATUS, &status);
[Severity: Low]
Does this pass an incompatible pointer type to regmap_read()?
This passes an int pointer (&status) instead of an unsigned int pointer.
> + if (err < 0)
> + return err;
[ ... ]
> @@ -856,11 +898,11 @@ 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, ABX8XX_CFG_KEY_MISC) < 0)
> + if (abx80x_write_config_key(&client->dev, ABX8XX_CFG_KEY_MISC) < 0)
> return -EIO;
>
> - err = i2c_smbus_write_byte_data(client, ABX8XX_REG_OUT_CTRL,
> - data | ABX8XX_OUT_CTRL_EXDS);
> + err = regmap_write(regmap, ABX8XX_REG_OUT_CTRL,
> + data | ABX8XX_OUT_CTRL_EXDS);
[Severity: High]
Does this hardware unlock sequence get broken by read-modify-write inversion?
For RV1805, the key is written, but if a read-modify-write operation like
regmap_write_bits() is used here, it performs a read before writing the EXTI
disable bit. This read clears the key, leaving the write ineffective and
defeating the extra power leakage workaround.
> if (err < 0) {
> dev_err(&client->dev,
> "Unable to write output control register\n");
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260722195814.298552-1-apokusinski01@gmail.com?part=3
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/6] rtc: abx80x: replace read-modify-write pattern with regmap helpers
2026-07-22 19:58 [PATCH 0/6] rtc: abx80x: add support for ABX81X Antoni Pokusinski
` (2 preceding siblings ...)
2026-07-22 19:58 ` [PATCH 3/6] rtc: abx80x: use regmap instead of I2C specific API Antoni Pokusinski
@ 2026-07-22 19:58 ` Antoni Pokusinski
2026-07-22 20:11 ` sashiko-bot
2026-07-22 19:58 ` [PATCH 5/6] rtc: abx80x: create abx80x_i2c_probe() Antoni Pokusinski
2026-07-22 19:58 ` [PATCH 6/6] rtc: abx80x: add support for ABX81X Antoni Pokusinski
5 siblings, 1 reply; 13+ messages in thread
From: Antoni Pokusinski @ 2026-07-22 19:58 UTC (permalink / raw)
To: alexandre.belloni, robh, krzk+dt, conor+dt
Cc: linux-rtc, devicetree, 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.
Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com>
---
drivers/rtc/rtc-abx80x.c | 105 +++++++++++----------------------------
1 file changed, 28 insertions(+), 77 deletions(-)
diff --git a/drivers/rtc/rtc-abx80x.c b/drivers/rtc/rtc-abx80x.c
index e5ba821a6768..7761cb353994 100644
--- a/drivers/rtc/rtc-abx80x.c
+++ b/drivers/rtc/rtc-abx80x.c
@@ -222,7 +222,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;
@@ -244,18 +244,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)
@@ -352,7 +345,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)) {
@@ -360,10 +353,6 @@ static int abx80x_rtc_set_autocalibration(struct device *dev,
return -EINVAL;
}
- 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) {
@@ -379,9 +368,9 @@ static int abx80x_rtc_set_autocalibration(struct device *dev,
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)
@@ -443,7 +432,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;
@@ -454,24 +443,14 @@ static ssize_t oscillator_store(struct device *dev,
return -EINVAL;
}
- 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;
}
@@ -537,17 +516,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:
- 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;
@@ -799,7 +769,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;
@@ -846,14 +816,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;
@@ -867,30 +832,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;
}
@@ -901,8 +846,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");
--
2.55.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 4/6] rtc: abx80x: replace read-modify-write pattern with regmap helpers
2026-07-22 19:58 ` [PATCH 4/6] rtc: abx80x: replace read-modify-write pattern with regmap helpers Antoni Pokusinski
@ 2026-07-22 20:11 ` sashiko-bot
0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-07-22 20:11 UTC (permalink / raw)
To: Antoni Pokusinski
Cc: conor+dt, devicetree, robh, Alexandre Belloni, linux-rtc
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Replacing manual read-modify-write patterns with `regmap_write_bits()` or `regmap_update_bits()` breaks the hardware's unlock sequence because they inject an implicit I2C read between unlocking the configuration key and the actual write, causing the hardware to re-lock the register.
--
--- Patch [4]: [PATCH 4/6] rtc: abx80x: replace read-modify-write pattern with regmap helpers ---
commit ac40d4eea0cf607d67d28eb1332803662646f9c2
Author: Antoni Pokusinski <apokusinski01@gmail.com>
rtc: abx80x: replace read-modify-write pattern with regmap helpers
This patch refactors the driver to use regmap_update_bits() and
regmap_write_bits() instead of manually reading, modifying, and writing
registers. The goal is to simplify the codebase.
> diff --git a/drivers/rtc/rtc-abx80x.c b/drivers/rtc/rtc-abx80x.c
> index e5ba821a67689..7761cb353994e 100644
> --- a/drivers/rtc/rtc-abx80x.c
> +++ b/drivers/rtc/rtc-abx80x.c
[ ... ]
> @@ -379,9 +368,9 @@ static int abx80x_rtc_set_autocalibration(struct device *dev,
> 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 replacing regmap_write() with regmap_write_bits() here break the hardware's
unlock sequence?
The regmap_write_bits() function performs an implicit read before writing.
Because it is called immediately after abx80x_write_config_key(), this injects
an I2C read between unlocking the configuration key and the actual write.
Since the AB08XX/RV1805 RTC requires the protected register write to be the
very next transaction after the configuration key write, won't this read
clear the unlock state and cause the hardware to silently discard the write?
> }
>
> static int abx80x_rtc_get_autocalibration(struct device *dev)
[ ... ]
> @@ -454,24 +443,14 @@ static ssize_t oscillator_store(struct device *dev,
> return -EINVAL;
> }
>
> - 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));
[Severity: High]
Could the implicit read introduced by regmap_write_bits() cause the
hardware to re-lock the register?
As with abx80x_rtc_set_autocalibration(), the injected read operation occurs
after unlocking the configuration key with abx80x_write_config_key(), which
disrupts the sequence required to successfully write to the Oscillator Control
register.
> + if (retval < 0)
> dev_err(dev, "Failed to write Oscillator Control register\n");
> - return retval;
> - }
>
> return retval ? retval : count;
> }
[ ... ]
> @@ -901,8 +846,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);
[Severity: High]
Is it possible that the OUT_CTRL EXDS bit configuration silently fails here?
Since regmap_write_bits() performs a read under the hood, placing it directly
after the abx80x_write_config_key() unlock command interrupts the required
write sequence, causing the hardware to discard the write operation.
> if (err < 0) {
> dev_err(&client->dev,
> "Unable to write output control register\n");
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260722195814.298552-1-apokusinski01@gmail.com?part=4
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 5/6] rtc: abx80x: create abx80x_i2c_probe()
2026-07-22 19:58 [PATCH 0/6] rtc: abx80x: add support for ABX81X Antoni Pokusinski
` (3 preceding siblings ...)
2026-07-22 19:58 ` [PATCH 4/6] rtc: abx80x: replace read-modify-write pattern with regmap helpers Antoni Pokusinski
@ 2026-07-22 19:58 ` Antoni Pokusinski
2026-07-22 20:10 ` sashiko-bot
2026-07-22 19:58 ` [PATCH 6/6] rtc: abx80x: add support for ABX81X Antoni Pokusinski
5 siblings, 1 reply; 13+ messages in thread
From: Antoni Pokusinski @ 2026-07-22 19:58 UTC (permalink / raw)
To: alexandre.belloni, robh, krzk+dt, conor+dt
Cc: linux-rtc, devicetree, 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.
Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com>
---
drivers/rtc/rtc-abx80x.c | 165 ++++++++++++++++++++++++---------------
1 file changed, 102 insertions(+), 63 deletions(-)
diff --git a/drivers/rtc/rtc-abx80x.c b/drivers/rtc/rtc-abx80x.c
index 7761cb353994..6e0c14695bdf 100644
--- a/drivers/rtc/rtc-abx80x.c
+++ b/drivers/rtc/rtc-abx80x.c
@@ -740,70 +740,35 @@ 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 -EIO;
- }
-
- 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;
- 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;
}
@@ -813,14 +778,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;
}
@@ -835,7 +800,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;
}
@@ -843,7 +808,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;
/*
@@ -855,7 +820,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;
}
@@ -867,26 +832,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));
@@ -894,7 +857,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;
}
@@ -904,14 +867,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;
}
}
@@ -920,14 +883,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[] = {
{
@@ -975,16 +963,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 -EIO;
+ }
+
+ 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 <phdm@macqel.be>");
MODULE_AUTHOR("Alexandre Belloni <alexandre.belloni@bootlin.com>");
--
2.55.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 5/6] rtc: abx80x: create abx80x_i2c_probe()
2026-07-22 19:58 ` [PATCH 5/6] rtc: abx80x: create abx80x_i2c_probe() Antoni Pokusinski
@ 2026-07-22 20:10 ` sashiko-bot
0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-07-22 20:10 UTC (permalink / raw)
To: Antoni Pokusinski
Cc: Alexandre Belloni, conor+dt, linux-rtc, devicetree, robh
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] RV1805 hardware workarounds are skipped during auto-detection.
--
--- Patch [5]: [PATCH 5/6] rtc: abx80x: create abx80x_i2c_probe() ---
commit 0661079a8b8384fb14bc67f5100b45ebe0a01c3e
Author: Antoni Pokusinski <apokusinski01@gmail.com>
rtc: abx80x: create abx80x_i2c_probe()
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.
> diff --git a/drivers/rtc/rtc-abx80x.c b/drivers/rtc/rtc-abx80x.c
> index 7761cb353994e..6e0c14695bdf1 100644
> --- a/drivers/rtc/rtc-abx80x.c
> +++ b/drivers/rtc/rtc-abx80x.c
[ ... ]
> @@ -855,7 +820,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;
> }
> @@ -867,26 +832,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;
> }
[Severity: High]
This is a pre-existing issue, but are the RV1805 hardware workarounds being
skipped during auto-detection?
Looking at abx80x_probe() in drivers/rtc/rtc-abx80x.c, the RV1805 specifics
are configured before the part auto-detection logic resolves the hardware type:
/* Configure RV1805 specifics */
if (part == RV1805) {
...
}
/* part autodetection */
if (part == ABX80X) {
for (i = 0; abx80x_caps[i].pn; i++)
if (partnumber == abx80x_caps[i].pn)
break;
...
part = i;
}
When the driver is bound using the generic "abracon,abx80x" compatible string
or I2C ID, the part argument is initialized to ABX80X. Because the workarounds
check if (part == RV1805) before part is updated to RV1805, they appear to be
silently bypassed.
Could this cause the RV1805 RTC to accidentally enter test mode and suffer
from extra power leakage?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260722195814.298552-1-apokusinski01@gmail.com?part=5
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 6/6] rtc: abx80x: add support for ABX81X
2026-07-22 19:58 [PATCH 0/6] rtc: abx80x: add support for ABX81X Antoni Pokusinski
` (4 preceding siblings ...)
2026-07-22 19:58 ` [PATCH 5/6] rtc: abx80x: create abx80x_i2c_probe() Antoni Pokusinski
@ 2026-07-22 19:58 ` Antoni Pokusinski
2026-07-22 20:09 ` sashiko-bot
5 siblings, 1 reply; 13+ messages in thread
From: Antoni Pokusinski @ 2026-07-22 19:58 UTC (permalink / raw)
To: alexandre.belloni, robh, krzk+dt, conor+dt
Cc: linux-rtc, devicetree, Antoni Pokusinski
Abracon ABX81X is a family of RTCs with SPI interface featuring
the same functionalities as the ABX80X I2C clocks.
Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com>
---
drivers/rtc/Kconfig | 26 +++++-----
drivers/rtc/rtc-abx80x.c | 106 +++++++++++++++++++++++++++++++++++++--
2 files changed, 115 insertions(+), 17 deletions(-)
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 4d14bcba616e..d35221f81024 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -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
diff --git a/drivers/rtc/rtc-abx80x.c b/drivers/rtc/rtc-abx80x.c
index 6e0c14695bdf..8481148ce3a2 100644
--- a/drivers/rtc/rtc-abx80x.c
+++ b/drivers/rtc/rtc-abx80x.c
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
/*
- * A driver for the I2C members of the Abracon AB x8xx RTC family,
- * and compatible: AB 1805 and AB 0805
+ * A driver for the I2C and SPI members of the Abracon AB x8xx RTC family,
+ * and compatible: AB 1805, AB 0805, AB 1815 and AB 0815
*
* Copyright 2014-2015 Macq S.A.
*
@@ -18,6 +18,7 @@
#include <linux/of.h>
#include <linux/regmap.h>
#include <linux/rtc.h>
+#include <linux/spi/spi.h>
#include <linux/watchdog.h>
#define ABX8XX_REG_HTH 0x00
@@ -102,8 +103,8 @@
static u8 trickle_resistors[] = {0, 3, 6, 11};
-enum abx80x_chip {AB0801, AB0803, AB0804, AB0805,
- AB1801, AB1803, AB1804, AB1805, RV1805, ABX80X};
+enum abx80x_chip {AB0801, AB0803, AB0804, AB0805, AB0815,
+ AB1801, AB1803, AB1804, AB1805, AB1815, RV1805, ABX80X};
struct abx80x_cap {
u16 pn;
@@ -116,10 +117,12 @@ static struct abx80x_cap abx80x_caps[] = {
[AB0803] = {.pn = 0x0803},
[AB0804] = {.pn = 0x0804, .has_tc = true, .has_wdog = true},
[AB0805] = {.pn = 0x0805, .has_tc = true, .has_wdog = true},
+ [AB0815] = {.pn = 0x0815, .has_tc = true, .has_wdog = true},
[AB1801] = {.pn = 0x1801},
[AB1803] = {.pn = 0x1803},
[AB1804] = {.pn = 0x1804, .has_tc = true, .has_wdog = true},
[AB1805] = {.pn = 0x1805, .has_tc = true, .has_wdog = true},
+ [AB1815] = {.pn = 0x1815, .has_tc = true, .has_wdog = true},
[RV1805] = {.pn = 0x1805, .has_tc = true, .has_wdog = true},
[ABX80X] = {.pn = 0}
};
@@ -1013,14 +1016,107 @@ static void abx80x_unregister_driver(void)
#endif /* IS_ENABLED(CONFIG_I2C) */
+#if IS_ENABLED(CONFIG_SPI_MASTER)
+
+static const struct regmap_config abx80x_regmap_config_spi = {
+ .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,
+
+ .write_flag_mask = BIT(7),
+};
+
+static const struct spi_device_id abx81x_id[] = {
+ { "ab0815", AB0815 },
+ { "ab1815", AB1815 },
+ { }
+};
+MODULE_DEVICE_TABLE(spi, abx81x_id);
+
+#ifdef CONFIG_OF
+static const struct of_device_id abx81x_of_match[] = {
+ {
+ .compatible = "abracon,ab0815",
+ .data = (void *)AB0815
+ },
+ {
+ .compatible = "abracon,ab1815",
+ .data = (void *)AB1815
+ },
+ { }
+};
+MODULE_DEVICE_TABLE(of, abx81x_of_match);
+#endif
+
+static int abx81x_spi_probe(struct spi_device *spi)
+{
+ unsigned int part = (uintptr_t)spi_get_device_match_data(spi);
+ struct regmap *regmap;
+
+ regmap = devm_regmap_init_spi(spi, &abx80x_regmap_config_spi);
+ if (IS_ERR(regmap)) {
+ dev_err(&spi->dev, "Unable to allocate regmap\n");
+ return -EIO;
+ }
+
+ return abx80x_probe(&spi->dev, regmap, spi->irq,
+ spi->dev.of_node, part);
+}
+
+static struct spi_driver abx81x_driver = {
+ .driver = {
+ .name = "rtc-abx81x",
+ .of_match_table = of_match_ptr(abx81x_of_match),
+ },
+ .probe = abx81x_spi_probe,
+ .id_table = abx81x_id,
+};
+
+static int abx81x_register_driver(void)
+{
+ return spi_register_driver(&abx81x_driver);
+}
+
+static void abx81x_unregister_driver(void)
+{
+ spi_unregister_driver(&abx81x_driver);
+}
+
+#else
+
+static int abx81x_register_driver(void)
+{
+ return 0;
+}
+
+static void abx81x_unregister_driver(void)
+{
+}
+
+#endif /* IS_ENABLED(CONFIG_SPI_MASTER) */
+
static int __init abx80x_init(void)
{
- return abx80x_register_driver();
+ int ret;
+
+ ret = abx80x_register_driver();
+ if (ret)
+ return ret;
+
+ ret = abx81x_register_driver();
+ if (ret)
+ abx80x_unregister_driver();
+
+ return ret;
}
module_init(abx80x_init);
static void __exit abx80x_exit(void)
{
+ abx81x_unregister_driver();
abx80x_unregister_driver();
}
module_exit(abx80x_exit);
--
2.55.0
^ permalink raw reply related [flat|nested] 13+ messages in thread