[PATCH v1 0/7] rtc-rv8803: Implement timestamp trigger over event pins

[PATCH v1 0/7] rtc-rv8803: Implement timestamp trigger over event pins

[Markus Burri]

The RV8901 RTC chip provides a function to store timestamp events.
There are three input pins (EVIN1-3) available for triggering.
The timestamp can be read to detect tamper alerts, for example.

This patch series extends the sysfs interface to enable, configure, and
read the timestamp events.

Currently the configuration is implemented in sysfs.
Maybe this could be moved into device-tree, since pull-resistor, edge and
timing is more hardware dependent.

The data-sheet can be found here:
https://download.epsondevice.com/td/pdf/brief/RX8901CE_en.pdf

This patch-queue applies to 'linux-6.13~rc6'

Markus Burri (7):
  dt-bindings: rtc: add new type for epson,rx8901
  rtc-rv8803: add new type for rv8901
  rtc-rv8803: add register definitions for rv8901 tamper detection
  rtc-rv8803: add tamper function to sysfs for rv8901
  rtc-rv8803: extend sysfs to trigger internal ts-event
  rtc-rv8803: make tamper function configurable via sysfs
  rtc-rv8803: extend sysfs to read ts-event and buffer status

 .../devicetree/bindings/rtc/epson,rx8900.yaml |   2 +
 drivers/rtc/rtc-rv8803.c                      | 778 +++++++++++++++++-
 2 files changed, 778 insertions(+), 2 deletions(-)

-- 
2.39.5

[PATCH v1 1/7] dt-bindings: rtc: add new type for epson,rx8901

[Markus Burri]

Add new compatibility string for the epson rx8901 chip.
The chip has input pins for tamper detection.
This patch add a new compatibility string for this type. This is
needed to enable the functionality for this type of chip.

The compatibility string is defined in the driver but not documented
in dt-bindings.

The patch also add compatibility string for epson,rx8803.
The type is supported by the driver but not documented.

Signed-off-by: Markus Burri <markus.burri@mt.com>

---
 Documentation/devicetree/bindings/rtc/epson,rx8900.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/rtc/epson,rx8900.yaml b/Documentation/devicetree/bindings/rtc/epson,rx8900.yaml
index b770149..03af817 100644
--- a/Documentation/devicetree/bindings/rtc/epson,rx8900.yaml
+++ b/Documentation/devicetree/bindings/rtc/epson,rx8900.yaml
@@ -15,8 +15,10 @@ allOf:
 properties:
   compatible:
     enum:
+      - epson,rx8803
       - epson,rx8804
       - epson,rx8900
+      - epson,rx8901
       - microcrystal,rv8803
 
   reg:
-- 
2.39.5

[PATCH v1 2/7] rtc-rv8803: add new type for rv8901

[Markus Burri]

The rtc-rv8901 has additional functionality (tamper detection).
To support this additional functionality the type must be extended.

Signed-off-by: Markus Burri <markus.burri@mt.com>

---
 drivers/rtc/rtc-rv8803.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-rv8803.c b/drivers/rtc/rtc-rv8803.c
index 1327251..50fbae9 100644
--- a/drivers/rtc/rtc-rv8803.c
+++ b/drivers/rtc/rtc-rv8803.c
@@ -62,7 +62,8 @@ enum rv8803_type {
 	rv_8803,
 	rx_8803,
 	rx_8804,
-	rx_8900
+	rx_8900,
+	rx_8901,
 };
 
 struct rv8803_data {
@@ -173,7 +174,8 @@ static int rv8803_regs_reset(struct rv8803_data *rv8803, bool full)
 	 * The RV-8803 resets all registers to POR defaults after voltage-loss,
 	 * the Epson RTCs don't, so we manually reset the remainder here.
 	 */
-	if (full || rv8803->type == rx_8803 || rv8803->type == rx_8900) {
+	if (full || rv8803->type == rx_8803 || rv8803->type == rx_8900 ||
+	    rv8803->type == rx_8901) {
 		int ret = rv8803_regs_init(rv8803);
 		if (ret)
 			return ret;
@@ -635,6 +637,7 @@ static const struct i2c_device_id rv8803_id[] = {
 	{ "rv8804", rx_8804 },
 	{ "rx8803", rx_8803 },
 	{ "rx8900", rx_8900 },
+	{ "rx8901", rx_8901 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, rv8803_id);
@@ -760,6 +763,10 @@ static const __maybe_unused struct of_device_id rv8803_of_match[] = {
 		.compatible = "epson,rx8900",
 		.data = (void *)rx_8900
 	},
+	{
+		.compatible = "epson,rx8901",
+		.data = (void *)rx_8901
+	},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, rv8803_of_match);
-- 
2.39.5

[PATCH v1 3/7] rtc-rv8803: add register definitions for rv8901 tamper detection

[Markus Burri]

Add register definition and string mapping for rv8901 tamper detection.

Signed-off-by: Markus Burri <markus.burri@mt.com>

---
 drivers/rtc/rtc-rv8803.c | 122 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 122 insertions(+)

diff --git a/drivers/rtc/rtc-rv8803.c b/drivers/rtc/rtc-rv8803.c
index 50fbae9..a4f2f53 100644
--- a/drivers/rtc/rtc-rv8803.c
+++ b/drivers/rtc/rtc-rv8803.c
@@ -58,6 +58,53 @@
 #define RX8900_FLAG_SWOFF		BIT(2)
 #define RX8900_FLAG_VDETOFF		BIT(3)
 
+#define RX8901_EVIN_EN			0x20
+#define RX8901_EVIN1_CFG		0x21
+#define RX8901_EVIN2_CFG		0x23
+#define RX8901_EVIN3_CFG		0x25
+#define RX8901_EVENTx_CFG_POL		GENMASK(1, 0)
+#define RX8901_EVENTx_CFG_PUPD		GENMASK(4, 2)
+
+#define RX8901_EVIN1_FLT		0x22
+#define RX8901_EVIN2_FLT		0x24
+#define RX8901_EVIN3_FLT		0x26
+
+#define RX8901_BUF1_CFG1		0x27
+#define RX8901_BUF2_CFG1		0x2A
+#define RX8901_BUF3_CFG1		0x2D
+
+#define RX8901_BUF1_STAT		0x28
+#define RX8901_BUF2_STAT		0x2B
+#define RX8901_BUF3_STAT		0x2E
+#define RX8901_BUFx_STAT_PTR		GENMASK(5, 0)
+#define RX8901_BUFx_STAT_EMPTF		BIT(6)
+#define RX8901_BUFx_STAT_FULLF		BIT(7)
+
+#define RX8901_BUF1_CFG2		0x29
+#define RX8901_BUF2_CFG2		0x2C
+#define RX8901_BUF3_CFG2		0x2F
+
+#define RX8901_WRCMD_CFG		0x41
+#define RX8901_WRCMD_TRG		0x42
+
+#define RX8901_EVNT_INTE		0x43
+#define RX8901_CAP_EN			0x44
+
+#define RX8901_BUF_INTF			0x46
+#define RX8901_BUF_INTF_BUF1F		BIT(5)
+
+#define RX8901_EVNT_INTF		0x47
+#define RX8901_EVNT_INTF_VBATLEVF	BIT(3)
+#define RX8901_EVNT_INTF_EVIN1F		BIT(5)
+
+#define RX8901_BUF_OVWF			0x4F
+
+#define NO_OF_EVIN			3
+
+#define EVIN_FILTER_FACTOR		125
+#define EVIN_FILTER_MAX			40
+#define EV_READ_MAX_LINE_SIZE		96
+
 enum rv8803_type {
 	rv_8803,
 	rx_8803,
@@ -66,6 +113,81 @@ enum rv8803_type {
 	rx_8901,
 };
 
+enum evin_pull_resistor {
+	no = 0b000,
+	pull_up_500k = 0b001,
+	pull_up_1M = 0b010,
+	pull_up_10M = 0b011,
+	pull_down_500k = 0b100,
+};
+
+enum evin_trigger {
+	falling_edge = 0b00,
+	rising_edge = 0b01,
+	both_edges = 0b10,
+};
+
+enum evin_buffer_mode {
+	inhibit = 0,
+	overwrite = 1,
+};
+
+struct cfg_val_txt {
+	char *txt;
+	u8 val;
+	bool hide;
+};
+
+const struct cfg_val_txt pull_resistor_txt[] = {
+	{ "no", no },
+	{ "PU/500k", pull_up_500k },
+	{ "PU/1M", pull_up_1M },
+	{ "PU/10M", pull_up_10M },
+	{ "PD/500k", pull_down_500k },
+	{ "no", 0b101, 1 },
+	{ "no", 0b110, 1 },
+	{ "no", 0b111, 1 },
+	{ NULL }
+};
+
+const struct cfg_val_txt trigger_txt[] = {
+	{ "falling", falling_edge },
+	{ "rising", rising_edge },
+	{ "both", both_edges },
+	{ "both", 0b11, 1 },
+	{ NULL }
+};
+
+const struct cfg_val_txt buffer_mode_txt[] = {
+	{ "inhibit", inhibit },
+	{ "overwrite", overwrite },
+	{ NULL }
+};
+
+const struct cfg_val_txt trg_status_txt[] = {
+	{ "EVIN1", BIT(5) },
+	{ "EVIN2", BIT(6) },
+	{ "EVIN3", BIT(7) },
+	{ "CMD", BIT(4) },
+	{ "VBATL", BIT(3) },
+	{ "VTMPL", BIT(2) },
+	{ "VDDL", BIT(1) },
+	{ "OSCSTP", BIT(0) },
+	{ NULL }
+};
+
+static const u8 evin_cfg_reg[] = {
+	RX8901_EVIN1_CFG,
+	RX8901_EVIN2_CFG,
+	RX8901_EVIN3_CFG
+};
+
+static const u8 evin_flt_reg[] = {
+	RX8901_EVIN1_FLT,
+	RX8901_EVIN2_FLT,
+	RX8901_EVIN3_FLT
+};
+
 struct rv8803_data {
 	struct i2c_client *client;
 	struct rtc_device *rtc;
-- 
2.39.5

[PATCH v1 4/7] rtc-rv8803: add tamper function to sysfs for rv8901

[Markus Burri]

Add sysfs interface to enable the EVINx pins and read the time-stamp
events from EVINX.

Signed-off-by: Markus Burri <markus.burri@mt.com>

---
 drivers/rtc/rtc-rv8803.c | 277 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 277 insertions(+)

diff --git a/drivers/rtc/rtc-rv8803.c b/drivers/rtc/rtc-rv8803.c
index a4f2f53..754657d 100644
--- a/drivers/rtc/rtc-rv8803.c
+++ b/drivers/rtc/rtc-rv8803.c
@@ -10,6 +10,7 @@
 #include <linux/bcd.h>
 #include <linux/bitops.h>
 #include <linux/bitfield.h>
+#include <linux/delay.h>
 #include <linux/log2.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
@@ -680,6 +681,276 @@ static int rv8803_nvram_read(void *priv, unsigned int offset,
 	return 0;
 }
 
+static int rv8803_ts_event_write_evin(int evin, struct rv8803_data *rv8803, int pullup_down,
+				      int trigger, int filter)
+{
+	int ret;
+	u8 reg_mask;
+	struct i2c_client *client = rv8803->client;
+
+	/* according to data-sheet, "1" is not valid for filter */
+	if (evin >= NO_OF_EVIN || filter == 1 || filter > EVIN_FILTER_MAX)
+		return -EINVAL;
+
+	/* set EVENTx pull-up edge trigger */
+	ret = rv8803_read_reg(client, evin_cfg_reg[evin]);
+	if (ret < 0)
+		return ret;
+	reg_mask = ret;
+	if (pullup_down != -1) {
+		reg_mask &= ~RX8901_EVENTx_CFG_PUPD;
+		reg_mask |= FIELD_PREP(RX8901_EVENTx_CFG_PUPD, pullup_down);
+	}
+	if (trigger != -1) {
+		reg_mask &= ~RX8901_EVENTx_CFG_POL;
+		reg_mask |= FIELD_PREP(RX8901_EVENTx_CFG_POL, trigger);
+	}
+	ret = rv8803_write_reg(client, evin_cfg_reg[evin], reg_mask);
+	if (ret < 0)
+		return ret;
+
+	/* set EVENTx noise filter */
+	if (filter != -1) {
+		ret = rv8803_write_reg(client, evin_flt_reg[evin], filter);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
+static ssize_t enable_store(struct device *dev, struct device_attribute *attr, const char *buf,
+			    size_t count)
+{
+	int ret;
+	int i;
+	unsigned long tmo;
+	u8 reg;
+	u8 reg_mask;
+	struct rv8803_data *rv8803 = dev_get_drvdata(dev->parent);
+	struct i2c_client *client = rv8803->client;
+
+	/* EVINxCPEN | EVINxEN */;
+	const u8 reg_mask_evin_en = GENMASK(5, 3) | GENMASK(2, 0);
+
+	bool enable = (strstr(buf, "1") == buf) ? true : false;
+
+	guard(mutex)(&rv8803->flags_lock);
+
+	/* check if event detection status match requested mode */
+	ret = rv8803_read_reg(client, RX8901_EVIN_EN);
+	if (ret < 0)
+		return ret;
+
+	/* requested mode match current state -> nothing to do */
+	if (ret == (enable ? reg_mask_evin_en : 0))
+		return count;
+
+	dev_info(&client->dev, "%s time-stamp event detection\n",
+		 (enable) ? "configure" : "disable");
+
+	/* 1. disable event detection interrupt */
+	ret = rv8803_read_reg(rv8803->client, RV8803_CTRL);
+	if (ret < 0)
+		return ret;
+	ret = rv8803_write_reg(rv8803->client, RV8803_CTRL, ret & ~RV8803_CTRL_EIE);
+	if (ret)
+		return ret;
+
+	/* 2. disable events for configuration */
+	ret = rv8803_write_reg(client, RX8901_EVIN_EN, 0);
+	if (ret < 0)
+		return ret;
+
+	/* for disable no configuration is needed */
+	if (!enable)
+		return count;
+
+	/* 3. set EVENTx pull-up edge trigger and noise filter */
+	for (i = 0; i < NO_OF_EVIN; ++i) {
+		ret = rv8803_ts_event_write_evin(i, rv8803, pull_up_1M, falling_edge, 0);
+		if (ret < 0)
+			return ret;
+	}
+
+	/* 4. enable EVENTx interrupt and VBATL,VDDL */
+	ret = rv8803_read_reg(client, RX8901_EVNT_INTE);
+	if (ret < 0)
+		return ret;
+	reg_mask = BIT(5) | BIT(6) | BIT(7); /* EVINxIEN 1-3 */
+	reg_mask |= BIT(3) | BIT(1); /* VBATLIEN | VDDLIEN */
+	reg = (ret & ~reg_mask) | reg_mask;
+	ret = rv8803_write_reg(client, RX8901_EVNT_INTE, reg);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * 5. set BUF1 inhibit and interrupt every 1 event
+	 *    NOTE: BUF2-3 are not used in FIFO-mode
+	 */
+	ret = rv8803_write_reg(client, RX8901_BUF1_CFG1, 0x01);
+	if (ret < 0)
+		return ret;
+
+	/* 6. clean and init for BUFx and event counter 1-3 and trigger cmd */
+	reg = BIT(7) | GENMASK(6, 4);
+	reg |= BIT(0); /* CMDTRGEN */
+	ret = rv8803_write_reg(client, RX8901_WRCMD_CFG, reg);
+	if (ret < 0)
+		return ret;
+	ret = rv8803_write_reg(client, RX8901_WRCMD_TRG, 0xFF);
+	if (ret < 0)
+		return ret;
+	tmo = jiffies + msecs_to_jiffies(100); /* timeout 100ms */
+	do {
+		usleep_range(10, 2000);
+		ret = rv8803_read_reg(client, RX8901_WRCMD_TRG);
+		if (ret < 0)
+			return ret;
+		if (time_after(jiffies, tmo))
+			return -EBUSY;
+	} while (ret);
+
+	/* 7. enable event detection interrupt */
+	ret = rv8803_read_reg(rv8803->client, RV8803_CTRL);
+	if (ret < 0)
+		return ret;
+	ret = rv8803_write_reg(rv8803->client, RV8803_CTRL, ret | RV8803_CTRL_EIE);
+	if (ret)
+		return ret;
+
+	/* 8. / 10. enable events for configuration in FIFO mode */
+	ret = rv8803_write_reg(client, RX8901_EVIN_EN, reg_mask_evin_en);
+	if (ret < 0)
+		return ret;
+
+	return count;
+}
+
+static ssize_t read_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	int ret;
+	int ev_idx;
+	int num_events;
+	unsigned long long time_s;
+	int time_ms;
+	int offset = 0;
+	u8 reg_mask;
+	u8 data[10];
+	struct rtc_time tm;
+
+	struct i2c_client *client = to_i2c_client(dev->parent);
+	struct rv8803_data *rv8803 = dev_get_drvdata(dev->parent);
+
+	guard(mutex)(&rv8803->flags_lock);
+
+	/*
+	 * For detailed description see datasheet:
+	 *  - Reading Time Stamp Data (FIFO mode)
+	 */
+
+	/* check interrupt source is from event 1-3 */
+	ret = rv8803_read_reg(client, RX8901_EVNT_INTF);
+	if (ret < 0)
+		return ret;
+
+	/* CHECK for EVF bit */
+	if (ret & BIT(2)) {
+		/* clear EVINxF 1-3 */
+		reg_mask = BIT(5) | BIT(6) | BIT(7);
+		ret = rv8803_write_reg(client, RX8901_EVNT_INTF, ret & ~reg_mask);
+		if (ret < 0)
+			return ret;
+	}
+
+	/* check interrupt source is from event 1-3 */
+	ret = rv8803_read_reg(client, RX8901_BUF_INTF);
+	if (ret < 0)
+		return ret;
+	if (ret & RX8901_BUF_INTF_BUF1F) {
+		/* disable interrupts */
+		ret = rv8803_read_reg(client, RV8803_CTRL);
+		if (ret < 0)
+			return ret;
+		ret = rv8803_write_reg(client, RV8803_CTRL, ret & ~RV8803_CTRL_EIE);
+		if (ret < 0)
+			return ret;
+
+		/* clear interrupt flag */
+		ret = rv8803_read_reg(client, RX8901_BUF_INTF);
+		if (ret < 0)
+			return ret;
+		ret = rv8803_write_reg(client, RX8901_BUF_INTF, ret & ~RX8901_BUF_INTF_BUF1F);
+		if (ret < 0)
+			return ret;
+	}
+
+	/* test if there are events available */
+	ret = rv8803_read_reg(client, RX8901_BUF1_STAT);
+	if (ret < 0)
+		return ret;
+	num_events = ret & RX8901_BUFx_STAT_PTR;
+
+	if (num_events) {
+		ret = rv8803_read_regs(client, 0x60, ARRAY_SIZE(data), data);
+		if (ret < 0)
+			return ret;
+
+		tm.tm_sec = bcd2bin(data[2]);
+		tm.tm_min = bcd2bin(data[3]);
+		tm.tm_hour = bcd2bin(data[4]);
+		tm.tm_mday = bcd2bin(data[5]);
+		tm.tm_mon = bcd2bin(data[6]) - 1;
+		tm.tm_year = bcd2bin(data[7]) + 100;
+		tm.tm_wday = -1;
+		tm.tm_yday = -1;
+		tm.tm_isdst = -1;
+
+		ret = rtc_valid_tm(&tm);
+		if (ret)
+			return ret;
+
+		/* calculate 1/1024 -> ms */
+		time_ms = (1000 * ((data[1] << 2) | (data[0] >> 6))) / 1024;
+		time_s = rtc_tm_to_time64(&tm);
+
+		offset += snprintf(buf + offset, PAGE_SIZE - offset, "%llu.%03d", time_s, time_ms);
+		for (ev_idx = 0; trg_status_txt[ev_idx].txt; ++ev_idx)
+			if (data[9] & trg_status_txt[ev_idx].val)
+				offset += snprintf(buf + offset, PAGE_SIZE - offset, " %s=%d",
+						   trg_status_txt[ev_idx].txt,
+						   !!(trg_status_txt[ev_idx].val & data[8]));
+		offset += snprintf(buf + offset, PAGE_SIZE - offset, "\n");
+
+		/* according to the datasheet we have to wait for 1ms */
+		usleep_range(1000, 2000);
+	}
+
+	/* re-enable interrupts */
+	ret = rv8803_read_reg(client, RV8803_CTRL);
+	if (ret < 0)
+		return ret;
+	ret = rv8803_write_reg(client, RV8803_CTRL, ret | RV8803_CTRL_EIE);
+	if (ret < 0)
+		return ret;
+
+	return offset;
+}
+
+static DEVICE_ATTR_WO(enable);
+static DEVICE_ATTR_RO(read);
+
+static struct attribute *rv8803_rtc_event_attrs[] = {
+	&dev_attr_enable.attr,
+	&dev_attr_read.attr,
+	NULL
+};
+
+static const struct attribute_group rv8803_rtc_sysfs_event_files = {
+	.name = "tamper",
+	.attrs	= rv8803_rtc_event_attrs,
+};
+
 static const struct rtc_class_ops rv8803_rtc_ops = {
 	.read_time = rv8803_get_time,
 	.set_time = rv8803_set_time,
@@ -854,6 +1125,12 @@ static int rv8803_probe(struct i2c_client *client)
 	if (err)
 		return err;
 
+	if (rv8803->type == rx_8901) {
+		err = rtc_add_group(rv8803->rtc, &rv8803_rtc_sysfs_event_files);
+		if (err)
+			return err;
+	}
+
 	rv8803->rtc->ops = &rv8803_rtc_ops;
 	rv8803->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
 	rv8803->rtc->range_max = RTC_TIMESTAMP_END_2099;
-- 
2.39.5

[PATCH v1 5/7] rtc-rv8803: extend sysfs to trigger internal ts-event

[Markus Burri]

Extend sysfs to trigger an internal time-stamp event.

The trigger function can be used from an application to trigger an
internal time-stamp event.

Signed-off-by: Markus Burri <markus.burri@mt.com>

---
 drivers/rtc/rtc-rv8803.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/rtc/rtc-rv8803.c b/drivers/rtc/rtc-rv8803.c
index 754657d..c479cc7 100644
--- a/drivers/rtc/rtc-rv8803.c
+++ b/drivers/rtc/rtc-rv8803.c
@@ -937,12 +937,45 @@ static ssize_t read_show(struct device *dev, struct device_attribute *attr, char
 	return offset;
 }
 
+static ssize_t trigger_store(struct device *dev, struct device_attribute *attr, const char *buf,
+			     size_t count)
+{
+	struct rv8803_data *rv8803 = dev_get_drvdata(dev->parent);
+	struct i2c_client *client = rv8803->client;
+	int ret;
+	unsigned long tmo;
+
+	guard(mutex)(&rv8803->flags_lock);
+
+	/* CMDTRGEN */
+	ret = rv8803_write_reg(client, RX8901_WRCMD_CFG, BIT(0));
+	if (ret < 0)
+		return ret;
+	ret = rv8803_write_reg(client, RX8901_WRCMD_TRG, 0xFF);
+	if (ret < 0)
+		return ret;
+
+	tmo = jiffies + msecs_to_jiffies(100); /* timeout 100ms */
+	do {
+		usleep_range(10, 2000);
+		ret = rv8803_read_reg(client, RX8901_WRCMD_TRG);
+		if (ret < 0)
+			return ret;
+		if (time_after(jiffies, tmo))
+			return -EBUSY;
+	} while (ret);
+
+	return count;
+}
+
 static DEVICE_ATTR_WO(enable);
 static DEVICE_ATTR_RO(read);
+static DEVICE_ATTR_WO(trigger);
 
 static struct attribute *rv8803_rtc_event_attrs[] = {
 	&dev_attr_enable.attr,
 	&dev_attr_read.attr,
+	&dev_attr_trigger.attr,
 	NULL
 };
 
-- 
2.39.5

[PATCH v1 6/7] rtc-rv8803: make tamper function configurable via sysfs

[Markus Burri]

Make the following settings via sysfs configurable:
  - For the input pins: input resistor, trigger edge, de-jitter filter.
  - For the buffer: overwrite or inhibit mode for the FIFO.

Signed-off-by: Markus Burri <markus.burri@mt.com>

---
 drivers/rtc/rtc-rv8803.c | 262 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 262 insertions(+)

diff --git a/drivers/rtc/rtc-rv8803.c b/drivers/rtc/rtc-rv8803.c
index c479cc7..cc8aa53 100644
--- a/drivers/rtc/rtc-rv8803.c
+++ b/drivers/rtc/rtc-rv8803.c
@@ -681,6 +681,31 @@ static int rv8803_nvram_read(void *priv, unsigned int offset,
 	return 0;
 }
 
+static int cfg2val(const struct cfg_val_txt *cfg, const char *text, u8 *value)
+{
+	if (!value)
+		return -EINVAL;
+
+	do {
+		if (strcasecmp(cfg->txt, text) == 0) {
+			*value = cfg->val;
+			return 0;
+		}
+	} while (++cfg && cfg->txt);
+
+	return -EINVAL;
+}
+
+static char *cfg2txt(const struct cfg_val_txt *cfg, u8 value)
+{
+	do {
+		if (cfg->val == value)
+			return cfg->txt;
+	} while (++cfg && cfg->txt);
+
+	return NULL;
+}
+
 static int rv8803_ts_event_write_evin(int evin, struct rv8803_data *rv8803, int pullup_down,
 				      int trigger, int filter)
 {
@@ -719,6 +744,31 @@ static int rv8803_ts_event_write_evin(int evin, struct rv8803_data *rv8803, int
 	return 0;
 }
 
+static int rv8803_ts_event_read_evin(int evin, struct rv8803_data *rv8803,
+				     int *pullup_down, int *trigger, int *filter)
+
+{
+	int ret;
+	struct i2c_client *client = rv8803->client;
+
+	/* get EVENTx pull-up edge trigger */
+	ret = rv8803_read_reg(client, evin_cfg_reg[evin]);
+	if (ret < 0)
+		return ret;
+
+	*pullup_down = FIELD_GET(RX8901_EVENTx_CFG_PUPD, ret);
+	*trigger = FIELD_GET(RX8901_EVENTx_CFG_POL, ret);
+
+	/* get EVENTx noise filter */
+	ret = rv8803_read_reg(client, evin_flt_reg[evin]);
+	if (ret < 0)
+		return ret;
+
+	*filter = ret;
+
+	return 0;
+}
+
 static ssize_t enable_store(struct device *dev, struct device_attribute *attr, const char *buf,
 			    size_t count)
 {
@@ -968,14 +1018,226 @@ static ssize_t trigger_store(struct device *dev, struct device_attribute *attr,
 	return count;
 }
 
+static ssize_t cfg_evin_available_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	int i;
+	int offset = 0;
+
+	offset += sprintf(buf + offset, "pull-resistor:\n");
+
+	for (i = 0; pull_resistor_txt[i].txt; ++i)
+		if (!pull_resistor_txt[i].hide)
+			offset += sprintf(buf + offset, "  %s\n", cfg2txt(pull_resistor_txt, i));
+	offset += sprintf(buf + offset, "\n");
+
+	offset += sprintf(buf + offset, "trigger:\n");
+	for (i = 0; trigger_txt[i].txt; ++i)
+		if (!trigger_txt[i].hide)
+			offset += sprintf(buf + offset, "  %s\n", cfg2txt(trigger_txt, i));
+	offset += sprintf(buf + offset, "\n");
+
+	offset += sprintf(buf + offset, "filter [ms]:\n");
+	for (i = 0; i <= EVIN_FILTER_MAX; ++i)
+		if (i != 1)
+			offset += sprintf(buf + offset, "  %d\n", EVIN_FILTER_FACTOR * i);
+
+	return offset;
+}
+
+static ssize_t cfg_evin_show(struct device *dev, int event, char *buf)
+{
+	int err;
+	struct rv8803_data *rv8803 = dev_get_drvdata(dev->parent);
+
+	int pullup_down;
+	int trigger;
+	int filter;
+
+	--event;
+	if (event >= NO_OF_EVIN)
+		return -ENOENT;
+
+	guard(mutex)(&rv8803->flags_lock);
+	err = rv8803_ts_event_read_evin(event, rv8803,
+					&pullup_down, &trigger, &filter);
+	if (err)
+		return err;
+
+	return sprintf(buf, "pull-resistor=%s, trigger=%s, filter=%dms\n",
+		       cfg2txt(pull_resistor_txt, pullup_down),
+		       cfg2txt(trigger_txt, trigger),
+		       EVIN_FILTER_FACTOR * filter);
+}
+
+static ssize_t cfg_evin1_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	return cfg_evin_show(dev, 1, buf);
+}
+
+static ssize_t cfg_evin2_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	return cfg_evin_show(dev, 2, buf);
+}
+
+static ssize_t cfg_evin3_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	return cfg_evin_show(dev, 3, buf);
+}
+
+static ssize_t cfg_evin_store(struct device *dev, int event, const char *buf, size_t count)
+{
+	int err;
+	struct rv8803_data *rv8803 = dev_get_drvdata(dev->parent);
+
+	char *buf_cpy;
+	char *token;
+	const char *startptr;
+	int pullup_down = -1;
+	int trigger = -1;
+	int filter = -1;
+	u8 v;
+
+	--event;
+	if (event >= NO_OF_EVIN)
+		return -ENOENT;
+
+	buf_cpy = kmalloc(count + 1, GFP_KERNEL);
+	if (!buf_cpy)
+		return -ENOMEM;
+
+	strscpy(buf_cpy, buf, count);
+	token = buf_cpy;
+	while ((startptr = strsep(&token, " ,\n"))) {
+		if (strstr(startptr, "pull-resistor=") == startptr)
+			if (cfg2val(pull_resistor_txt, strchr(startptr, '=') + 1, &v) == 0)
+				pullup_down = v;
+		if (strstr(startptr, "trigger=") == startptr)
+			if (cfg2val(trigger_txt, strchr(startptr, '=') + 1, &v) == 0)
+				trigger = v;
+		if (strstr(startptr, "filter=") == startptr)
+			filter = strtoul(strchr(startptr, '=') + 1, NULL, 0) / EVIN_FILTER_FACTOR;
+	}
+
+	kfree(buf_cpy);
+
+	guard(mutex)(&rv8803->flags_lock);
+	err = rv8803_ts_event_write_evin(event, rv8803, pullup_down, trigger, filter);
+	if (err)
+		return err;
+
+	return count;
+}
+
+static ssize_t cfg_evin1_store(struct device *dev, struct device_attribute *attr, const char *buf,
+			       size_t count)
+{
+	return cfg_evin_store(dev, 1, buf, count);
+}
+
+static ssize_t cfg_evin2_store(struct device *dev, struct device_attribute *attr, const char *buf,
+			       size_t count)
+{
+	return cfg_evin_store(dev, 2, buf, count);
+}
+
+static ssize_t cfg_evin3_store(struct device *dev, struct device_attribute *attr, const char *buf,
+			       size_t count)
+{
+	return cfg_evin_store(dev, 3, buf, count);
+}
+
+static ssize_t cfg_buf_available_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	int i;
+	int offset = 0;
+
+	offset += sprintf(buf + offset, "mode:\n");
+	for (i = 0; buffer_mode_txt[i].txt; ++i)
+		if (!buffer_mode_txt[i].hide)
+			offset += sprintf(buf + offset, "  %s\n", cfg2txt(buffer_mode_txt, i));
+
+	return offset;
+}
+
+static ssize_t cfg_buf_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	int ret;
+	struct i2c_client *client = to_i2c_client(dev->parent);
+	struct rv8803_data *rv8803 = dev_get_drvdata(dev->parent);
+
+	guard(mutex)(&rv8803->flags_lock);
+
+	ret = rv8803_read_reg(client, RX8901_BUF1_CFG1);
+	if (ret < 0)
+		return ret;
+
+	return sprintf(buf, "mode:%s\n",
+		       cfg2txt(buffer_mode_txt, FIELD_GET(BIT(6), ret)));
+}
+
+static ssize_t cfg_buf_store(struct device *dev, struct device_attribute *attr, const char *buf,
+			     size_t count)
+{
+	int ret;
+	char *buf_cpy;
+	char *token;
+	char *startptr;
+	int mode = -1;
+	u8 v;
+
+	struct i2c_client *client = to_i2c_client(dev->parent);
+	struct rv8803_data *rv8803 = dev_get_drvdata(dev->parent);
+
+	buf_cpy = kmalloc(count + 1, GFP_KERNEL);
+	if (!buf_cpy)
+		return -ENOMEM;
+
+	strscpy(buf_cpy, buf, count);
+	token = buf_cpy;
+	while ((startptr = strsep(&token, " ,\n"))) {
+		if (strstr(startptr, "mode:") == startptr)
+			if (cfg2val(buffer_mode_txt, strchr(startptr, ':') + 1, &v) == 0)
+				mode = v;
+	}
+
+	kfree(buf_cpy);
+
+	if (mode != -1) {
+		guard(mutex)(&rv8803->flags_lock);
+
+		ret = rv8803_read_reg(client, RX8901_BUF1_CFG1);
+		if (ret < 0)
+			return ret;
+
+		ret &= ~BIT(6);
+		ret |= FIELD_PREP(BIT(6), mode);
+		ret = rv8803_write_reg(client, RX8901_BUF1_CFG1, ret);
+		if (ret < 0)
+			return ret;
+	}
+	return count;
+}
+
 static DEVICE_ATTR_WO(enable);
 static DEVICE_ATTR_RO(read);
 static DEVICE_ATTR_WO(trigger);
+static DEVICE_ATTR_RO(cfg_evin_available);
+static DEVICE_ATTR_RO(cfg_buf_available);
+static DEVICE_ATTR_RW(cfg_evin1);
+static DEVICE_ATTR_RW(cfg_evin2);
+static DEVICE_ATTR_RW(cfg_evin3);
+static DEVICE_ATTR_RW(cfg_buf);
 
 static struct attribute *rv8803_rtc_event_attrs[] = {
 	&dev_attr_enable.attr,
 	&dev_attr_read.attr,
 	&dev_attr_trigger.attr,
+	&dev_attr_cfg_evin_available.attr,
+	&dev_attr_cfg_buf_available.attr,
+	&dev_attr_cfg_evin1.attr,
+	&dev_attr_cfg_evin2.attr,
+	&dev_attr_cfg_evin3.attr,
+	&dev_attr_cfg_buf.attr,
 	NULL
 };
 
-- 
2.39.5

[PATCH v1 7/7] rtc-rv8803: extend sysfs to read ts-event and buffer status

[Markus Burri]

Add sysfs functionality to read the status and configuration.
The functions provide the number of stored timestamp events, the current
EVIN pin configuration and the enabled/disabled status.

Signed-off-by: Markus Burri <markus.burri@mt.com>

---
 drivers/rtc/rtc-rv8803.c | 73 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/drivers/rtc/rtc-rv8803.c b/drivers/rtc/rtc-rv8803.c
index cc8aa53..5d31604 100644
--- a/drivers/rtc/rtc-rv8803.c
+++ b/drivers/rtc/rtc-rv8803.c
@@ -1218,6 +1218,77 @@ static ssize_t cfg_buf_store(struct device *dev, struct device_attribute *attr,
 	return count;
 }
 
+static ssize_t status_show(struct device *dev, struct device_attribute *attr,
+			   char *buf)
+{
+	int evin_en, evin_cfg, evin_flt, buf1_cfg, buf1_stat, buf_ovwf;
+	int i;
+	int offset = 0;
+	u8 ev_pullupdown[NO_OF_EVIN];
+	u8 ev_trigger[NO_OF_EVIN];
+	int ev_filter[NO_OF_EVIN];
+
+	struct i2c_client *client = to_i2c_client(dev->parent);
+	struct rv8803_data *rv8803 = dev_get_drvdata(dev->parent);
+
+	guard(mutex)(&rv8803->flags_lock);
+
+	evin_en = rv8803_read_reg(client, RX8901_EVIN_EN);
+	if (evin_en < 0)
+		return evin_en;
+
+	for (i = 0; i < NO_OF_EVIN; ++i) {
+		evin_cfg = rv8803_read_reg(client, evin_cfg_reg[i]);
+		evin_flt = rv8803_read_reg(client, evin_flt_reg[i]);
+		if (evin_cfg < 0 || evin_flt < 0)
+			return -EIO;
+
+		ev_pullupdown[i] = FIELD_GET(RX8901_EVENTx_CFG_PUPD, evin_cfg);
+		ev_trigger[i] = FIELD_GET(RX8901_EVENTx_CFG_POL, evin_cfg);
+		ev_filter[i] = EVIN_FILTER_FACTOR * evin_flt;
+	}
+
+	offset += sprintf(buf + offset, "Mode: %s\n\n",
+			  FIELD_GET(BIT(6), evin_en) ? "direct" : "fifo");
+	offset += sprintf(buf + offset, "Event config:\n");
+	offset += sprintf(buf + offset, "  %-13s  %-7s %-7s %-7s\n", "", "EVIN1", "EVIN2", "EVIN3");
+	offset += sprintf(buf + offset, "  %-13s  %-7ld %-7ld %-7ld\n", "Enable",
+			  FIELD_GET(BIT(0), evin_en), FIELD_GET(BIT(1), evin_en),
+			  FIELD_GET(BIT(2), evin_en));
+	offset += sprintf(buf + offset, "  %-13s  %-7ld %-7ld %-7ld\n", "Capture",
+			  FIELD_GET(BIT(3), evin_en), FIELD_GET(BIT(4), evin_en),
+			  FIELD_GET(BIT(5), evin_en));
+
+	offset += sprintf(buf + offset, "  %-13s  %-7s %-7s %-7s\n", "Pull-resistor",
+			  cfg2txt(pull_resistor_txt, ev_pullupdown[0]),
+			  cfg2txt(pull_resistor_txt, ev_pullupdown[1]),
+			  cfg2txt(pull_resistor_txt, ev_pullupdown[2]));
+	offset += sprintf(buf + offset, "  %-13s  %-7s %-7s %-7s\n", "Edge",
+			  cfg2txt(trigger_txt, ev_trigger[0]),
+			  cfg2txt(trigger_txt, ev_trigger[1]),
+			  cfg2txt(trigger_txt, ev_trigger[2]));
+	offset += sprintf(buf + offset, "  %-13s  %-7d %-7d %-7d\n\n", "Filter [ms]",
+			  ev_filter[0], ev_filter[1], ev_filter[2]);
+
+	buf1_cfg = rv8803_read_reg(client, RX8901_BUF1_CFG1);
+	buf1_stat = rv8803_read_reg(client, RX8901_BUF1_STAT);
+	buf_ovwf = rv8803_read_reg(client, RX8901_BUF_OVWF);
+	if (buf1_stat < 0 || buf1_stat < 0 || buf_ovwf < 0)
+		return -EIO;
+
+	offset += sprintf(buf + offset, "Buffer config:\n");
+	offset += sprintf(buf + offset, "  %-13s  %-10s\n", "Mode",
+			  (FIELD_GET(BIT(6), buf1_cfg) ? "overwrite" : "inhibit"));
+	offset += sprintf(buf + offset, "  %-13s  %-10s\n", "Status",
+			  (FIELD_GET(BIT(7), buf1_stat) ? "full" : "free"));
+	offset += sprintf(buf + offset,  "  %-13s  %-10ld\n", "Overrun",
+			  (FIELD_GET(BIT(4), buf_ovwf)));
+	offset += sprintf(buf + offset,  "  %-13s  %-10ld\n", "No of data",
+			  (FIELD_GET(GENMASK(5, 0), buf1_stat)));
+
+	return offset;
+}
+
 static DEVICE_ATTR_WO(enable);
 static DEVICE_ATTR_RO(read);
 static DEVICE_ATTR_WO(trigger);
@@ -1227,6 +1298,7 @@ static DEVICE_ATTR_RW(cfg_evin1);
 static DEVICE_ATTR_RW(cfg_evin2);
 static DEVICE_ATTR_RW(cfg_evin3);
 static DEVICE_ATTR_RW(cfg_buf);
+static DEVICE_ATTR_RO(status);
 
 static struct attribute *rv8803_rtc_event_attrs[] = {
 	&dev_attr_enable.attr,
@@ -1238,6 +1310,7 @@ static struct attribute *rv8803_rtc_event_attrs[] = {
 	&dev_attr_cfg_evin2.attr,
 	&dev_attr_cfg_evin3.attr,
 	&dev_attr_cfg_buf.attr,
+	&dev_attr_status.attr,
 	NULL
 };
 
-- 
2.39.5

Re: [PATCH v1 1/7] dt-bindings: rtc: add new type for epson,rx8901

[Krzysztof Kozlowski]

On Fri, Jan 10, 2025 at 07:13:55AM +0100, Markus Burri wrote:
> Add new compatibility string for the epson rx8901 chip.
> The chip has input pins for tamper detection.
> This patch add a new compatibility string for this type. This is

Please do not use "This commit/patch/change", but imperative mood. See
longer explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

> needed to enable the functionality for this type of chip.
> 
> The compatibility string is defined in the driver but not documented
> in dt-bindings.
> 
> The patch also add compatibility string for epson,rx8803.
> The type is supported by the driver but not documented.

All four sentences keep repeating the same, so just:

"Document compatibles for RX8901 and RX8803, which are already used by
the driver."

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v1 3/7] rtc-rv8803: add register definitions for rv8901 tamper detection

[Krzysztof Kozlowski]

On Fri, Jan 10, 2025 at 07:13:57AM +0100, Markus Burri wrote:
> Add register definition and string mapping for rv8901 tamper detection.
> 
> Signed-off-by: Markus Burri <markus.burri@mt.com>
> 
> ---
>  drivers/rtc/rtc-rv8803.c | 122 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 122 insertions(+)
> 

There are no users of this. Don't add dead code. Probably you wanted to
add it for some usage, so add defines/structs WITH the users.

> diff --git a/drivers/rtc/rtc-rv8803.c b/drivers/rtc/rtc-rv8803.c
> index 50fbae9..a4f2f53 100644
> --- a/drivers/rtc/rtc-rv8803.c
> +++ b/drivers/rtc/rtc-rv8803.c
> @@ -58,6 +58,53 @@
>  #define RX8900_FLAG_SWOFF		BIT(2)
>  #define RX8900_FLAG_VDETOFF		BIT(3)
>  
> +#define RX8901_EVIN_EN			0x20
> +#define RX8901_EVIN1_CFG		0x21
> +#define RX8901_EVIN2_CFG		0x23
> +#define RX8901_EVIN3_CFG		0x25
> +#define RX8901_EVENTx_CFG_POL		GENMASK(1, 0)
> +#define RX8901_EVENTx_CFG_PUPD		GENMASK(4, 2)
> +
> +#define RX8901_EVIN1_FLT		0x22
> +#define RX8901_EVIN2_FLT		0x24
> +#define RX8901_EVIN3_FLT		0x26
> +
> +#define RX8901_BUF1_CFG1		0x27
> +#define RX8901_BUF2_CFG1		0x2A
> +#define RX8901_BUF3_CFG1		0x2D
> +
> +#define RX8901_BUF1_STAT		0x28
> +#define RX8901_BUF2_STAT		0x2B
> +#define RX8901_BUF3_STAT		0x2E
> +#define RX8901_BUFx_STAT_PTR		GENMASK(5, 0)
> +#define RX8901_BUFx_STAT_EMPTF		BIT(6)
> +#define RX8901_BUFx_STAT_FULLF		BIT(7)
> +
> +#define RX8901_BUF1_CFG2		0x29
> +#define RX8901_BUF2_CFG2		0x2C
> +#define RX8901_BUF3_CFG2		0x2F
> +
> +#define RX8901_WRCMD_CFG		0x41
> +#define RX8901_WRCMD_TRG		0x42
> +
> +#define RX8901_EVNT_INTE		0x43
> +#define RX8901_CAP_EN			0x44
> +
> +#define RX8901_BUF_INTF			0x46
> +#define RX8901_BUF_INTF_BUF1F		BIT(5)
> +
> +#define RX8901_EVNT_INTF		0x47
> +#define RX8901_EVNT_INTF_VBATLEVF	BIT(3)
> +#define RX8901_EVNT_INTF_EVIN1F		BIT(5)
> +
> +#define RX8901_BUF_OVWF			0x4F
> +
> +#define NO_OF_EVIN			3
> +
> +#define EVIN_FILTER_FACTOR		125
> +#define EVIN_FILTER_MAX			40
> +#define EV_READ_MAX_LINE_SIZE		96
> +
>  enum rv8803_type {
>  	rv_8803,
>  	rx_8803,
> @@ -66,6 +113,81 @@ enum rv8803_type {
>  	rx_8901,
>  };
>  
> +enum evin_pull_resistor {
> +	no = 0b000,
> +	pull_up_500k = 0b001,
> +	pull_up_1M = 0b010,
> +	pull_up_10M = 0b011,
> +	pull_down_500k = 0b100,
> +};
> +
> +enum evin_trigger {
> +	falling_edge = 0b00,
> +	rising_edge = 0b01,
> +	both_edges = 0b10,
> +};
> +
> +enum evin_buffer_mode {
> +	inhibit = 0,
> +	overwrite = 1,
> +};
> +
> +struct cfg_val_txt {
> +	char *txt;
> +	u8 val;
> +	bool hide;
> +};
> +
> +const struct cfg_val_txt pull_resistor_txt[] = {

Why all these are not static? Where is the header exporting these?


Best regards,
Krzysztof

Re: [PATCH v1 4/7] rtc-rv8803: add tamper function to sysfs for rv8901

[Krzysztof Kozlowski]

On Fri, Jan 10, 2025 at 07:13:58AM +0100, Markus Burri wrote:
> +
> +static ssize_t enable_store(struct device *dev, struct device_attribute *attr, const char *buf,
> +			    size_t count)
> +{
> +	int ret;
> +	int i;
> +	unsigned long tmo;
> +	u8 reg;
> +	u8 reg_mask;
> +	struct rv8803_data *rv8803 = dev_get_drvdata(dev->parent);
> +	struct i2c_client *client = rv8803->client;
> +
> +	/* EVINxCPEN | EVINxEN */;
> +	const u8 reg_mask_evin_en = GENMASK(5, 3) | GENMASK(2, 0);
> +
> +	bool enable = (strstr(buf, "1") == buf) ? true : false;
> +
> +	guard(mutex)(&rv8803->flags_lock);


That's absolutely huge guard. Isn't this supposed to protect only flags?
Not all register writes?

> +
> +	/* check if event detection status match requested mode */
> +	ret = rv8803_read_reg(client, RX8901_EVIN_EN);
> +	if (ret < 0)
> +		return ret;

...

> +	/* re-enable interrupts */
> +	ret = rv8803_read_reg(client, RV8803_CTRL);
> +	if (ret < 0)
> +		return ret;
> +	ret = rv8803_write_reg(client, RV8803_CTRL, ret | RV8803_CTRL_EIE);
> +	if (ret < 0)
> +		return ret;
> +
> +	return offset;
> +}
> +
> +static DEVICE_ATTR_WO(enable);

You need to document the ABI.

Best regards,
Krzysztof

Re: [PATCH v1 5/7] rtc-rv8803: extend sysfs to trigger internal ts-event

[Krzysztof Kozlowski]

On Fri, Jan 10, 2025 at 07:13:59AM +0100, Markus Burri wrote:
> +	tmo = jiffies + msecs_to_jiffies(100); /* timeout 100ms */
> +	do {
> +		usleep_range(10, 2000);
> +		ret = rv8803_read_reg(client, RX8901_WRCMD_TRG);
> +		if (ret < 0)
> +			return ret;
> +		if (time_after(jiffies, tmo))
> +			return -EBUSY;
> +	} while (ret);
> +
> +	return count;
> +}
> +
>  static DEVICE_ATTR_WO(enable);
>  static DEVICE_ATTR_RO(read);
> +static DEVICE_ATTR_WO(trigger);

Missing ABI documentation.

Best regards,
Krzysztof

Re: [PATCH v1 1/7] dt-bindings: rtc: add new type for epson,rx8901

[Krzysztof Kozlowski]

On 11/01/2025 11:20, Krzysztof Kozlowski wrote:
> On Fri, Jan 10, 2025 at 07:13:55AM +0100, Markus Burri wrote:
>> Add new compatibility string for the epson rx8901 chip.
>> The chip has input pins for tamper detection.
>> This patch add a new compatibility string for this type. This is
> 
> Please do not use "This commit/patch/change", but imperative mood. See
> longer explanation here:
> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
> 
>> needed to enable the functionality for this type of chip.
>>
>> The compatibility string is defined in the driver but not documented
>> in dt-bindings.
>>
>> The patch also add compatibility string for epson,rx8803.
>> The type is supported by the driver but not documented.
> 
> All four sentences keep repeating the same, so just:
> 
> "Document compatibles for RX8901 and RX8803, which are already used by
> the driver."
> 
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

I see only one was undocumented, so:

"Document compatibles for:
 - RX8803: already used by the driver
 - RX8901: new device supporting also tamper detection and pinctrl ."

And then you need proper pinctrl bindings :/

Best regards,
Krzysztof

Re: [PATCH v1 6/7] rtc-rv8803: make tamper function configurable via sysfs

[Krzysztof Kozlowski]

On 10/01/2025 07:14, Markus Burri wrote:
>  static int rv8803_ts_event_write_evin(int evin, struct rv8803_data *rv8803, int pullup_down,
>  				      int trigger, int filter)
>  {
> @@ -719,6 +744,31 @@ static int rv8803_ts_event_write_evin(int evin, struct rv8803_data *rv8803, int
>  	return 0;
>  }
>  
> +static int rv8803_ts_event_read_evin(int evin, struct rv8803_data *rv8803,
> +				     int *pullup_down, int *trigger, int *filter)
> +
> +{
> +	int ret;
> +	struct i2c_client *client = rv8803->client;
> +
> +	/* get EVENTx pull-up edge trigger */
> +	ret = rv8803_read_reg(client, evin_cfg_reg[evin]);
> +	if (ret < 0)
> +		return ret;
> +
> +	*pullup_down = FIELD_GET(RX8901_EVENTx_CFG_PUPD, ret);
> +	*trigger = FIELD_GET(RX8901_EVENTx_CFG_POL, ret);

No. pinctrl is not done via sysfs, please do not re-invent existing
APIs. You must use proper pinctrl subsystem for this.

Best regards,
Krzysztof

Re: [PATCH v1 3/7] rtc-rv8803: add register definitions for rv8901 tamper detection

[kernel test robot]

Hi Markus,

kernel test robot noticed the following build warnings:

[auto build test WARNING on abelloni/rtc-next]
[also build test WARNING on robh/for-next linus/master v6.13-rc6 next-20250110]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Markus-Burri/dt-bindings-rtc-add-new-type-for-epson-rx8901/20250110-141934
base:   https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git rtc-next
patch link:    https://lore.kernel.org/r/20250110061401.358371-4-markus.burri%40mt.com
patch subject: [PATCH v1 3/7] rtc-rv8803: add register definitions for rv8901 tamper detection
config: m68k-randconfig-r122-20250111 (https://download.01.org/0day-ci/archive/20250112/202501121203.Kw9SnPYP-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 14.2.0
reproduce: (https://download.01.org/0day-ci/archive/20250112/202501121203.Kw9SnPYP-lkp@intel.com/reproduce)

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

sparse warnings: (new ones prefixed by >>)
>> drivers/rtc/rtc-rv8803.c:141:26: sparse: sparse: symbol 'pull_resistor_txt' was not declared. Should it be static?
>> drivers/rtc/rtc-rv8803.c:153:26: sparse: sparse: symbol 'trigger_txt' was not declared. Should it be static?
>> drivers/rtc/rtc-rv8803.c:161:26: sparse: sparse: symbol 'buffer_mode_txt' was not declared. Should it be static?
>> drivers/rtc/rtc-rv8803.c:167:26: sparse: sparse: symbol 'trg_status_txt' was not declared. Should it be static?

vim +/pull_resistor_txt +141 drivers/rtc/rtc-rv8803.c

   140	
 > 141	const struct cfg_val_txt pull_resistor_txt[] = {
   142		{ "no", no },
   143		{ "PU/500k", pull_up_500k },
   144		{ "PU/1M", pull_up_1M },
   145		{ "PU/10M", pull_up_10M },
   146		{ "PD/500k", pull_down_500k },
   147		{ "no", 0b101, 1 },
   148		{ "no", 0b110, 1 },
   149		{ "no", 0b111, 1 },
   150		{ NULL }
   151	};
   152	
 > 153	const struct cfg_val_txt trigger_txt[] = {
   154		{ "falling", falling_edge },
   155		{ "rising", rising_edge },
   156		{ "both", both_edges },
   157		{ "both", 0b11, 1 },
   158		{ NULL }
   159	};
   160	
 > 161	const struct cfg_val_txt buffer_mode_txt[] = {
   162		{ "inhibit", inhibit },
   163		{ "overwrite", overwrite },
   164		{ NULL }
   165	};
   166	
 > 167	const struct cfg_val_txt trg_status_txt[] = {
   168		{ "EVIN1", BIT(5) },
   169		{ "EVIN2", BIT(6) },
   170		{ "EVIN3", BIT(7) },
   171		{ "CMD", BIT(4) },
   172		{ "VBATL", BIT(3) },
   173		{ "VTMPL", BIT(2) },
   174		{ "VDDL", BIT(1) },
   175		{ "OSCSTP", BIT(0) },
   176		{ NULL }
   177	};
   178	

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