[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z7YE5NE1UWw52VMW@mt.com>
Date: Wed, 19 Feb 2025 17:20:52 +0100
From: Manuel Traut <manuel.traut@...com>
To: Markus Burri <markus.burri@...com>
Cc: linux-kernel@...r.kernel.org,
Alexandre Belloni <alexandre.belloni@...tlin.com>,
Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Marek Vasut <marex@...x.de>,
linux-rtc@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v2 3/5] rtc-rv8803: add tamper function to sysfs for
rv8901
On Thu, Jan 16, 2025 at 02:15:30PM +0100, Markus Burri wrote:
> Add sysfs interface to enable the EVINx pins and read the time-stamp
> events from EVINX.
>
> Signed-off-by: Markus Burri <markus.burri@...com>
> ---
> .../ABI/testing/sysfs-class-rtc-tamper | 21 +
> drivers/rtc/rtc-rv8803.c | 366 ++++++++++++++++++
> 2 files changed, 387 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-class-rtc-tamper
>
> diff --git a/Documentation/ABI/testing/sysfs-class-rtc-tamper b/Documentation/ABI/testing/sysfs-class-rtc-tamper
> new file mode 100644
> index 000000000..f035d0fa5
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-rtc-tamper
> @@ -0,0 +1,21 @@
> +What: /sys/class/rtc/rtcX/tamper/enable
> +Date: January 2025
> +KernelVersion: 6.13
> +Contact: Markus Burri <markus.burri@...com>
> +Description: (WO) Attribute to enable and disable the rtc tamper function.
> + Write a '1' to enable tamper detection or a '0' to disable.
> +
> +What: /sys/class/rtc/rtcX/tamper/read
> +Date: January 2025
> +KernelVersion: 6.13
> +Contact: Markus Burri <markus.burri@...com>
> +Description: (RO) Attribute to read the stored timestamps form buffer FIFO.
Description: (RO) Attribute to read tamper events.
> + The timestamps are returned one by one
Each read tries to retrieve a single entry from the hardware
FIFO. Events that are not retrieved stay in the hardware FIFO.
> + Format is 'seconds.milliseconds' since the epoch followed by the trigger events.
> + The value of the event is the current pin value.
> +
> + Example values:
> + - "1234.567 EVIN1=1" for a trigger from EVIN1 changed from low to high
> + - "1234.567 EVIN1=0 EVIN2=1 for a simultaneous trigger of EVIN1 changed to low and
> + EVIN2 changed to high.
> +
> diff --git a/drivers/rtc/rtc-rv8803.c b/drivers/rtc/rtc-rv8803.c
> index 50fbae931..764e654c2 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>
> @@ -58,6 +59,35 @@
> #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_BUF1_STAT 0x28
> +#define RX8901_BUFx_STAT_PTR GENMASK(5, 0)
> +#define RX8901_WRCMD_CFG 0x41
> +#define RX8901_WRCMD_TRG 0x42
> +
> +#define RX8901_EVNT_INTE 0x43
> +
> +#define RX8901_BUF_INTF 0x46
> +#define RX8901_BUF_INTF_BUF1F BIT(5)
> +
> +#define RX8901_EVNT_INTF 0x47
> +
> +#define NO_OF_EVIN 3
> +
> +#define EVIN_FILTER_MAX 40
> +
> enum rv8803_type {
> rv_8803,
> rx_8803,
> @@ -66,6 +96,50 @@ 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,
> +};
> +
> +struct cfg_val_txt {
> + char *txt;
> + u8 val;
> + bool hide;
> +};
> +
> +static 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_regs[] = {
> + RX8901_EVIN1_CFG,
> + RX8901_EVIN2_CFG,
> + RX8901_EVIN3_CFG
> +};
> +
> +static const u8 evin_flt_regs[] = {
> + RX8901_EVIN1_FLT,
> + RX8901_EVIN2_FLT,
> + RX8901_EVIN3_FLT
> +};
> +
> struct rv8803_data {
> struct i2c_client *client;
> struct rtc_device *rtc;
> @@ -558,6 +632,292 @@ 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,
> + enum evin_pull_resistor pullup_down,
> + enum evin_trigger 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;
> +
> + guard(mutex)(&rv8803->flags_lock);
> +
> + /* set EVENTx pull-up edge trigger */
> + ret = rv8803_read_reg(client, evin_cfg_regs[evin]);
> + if (ret < 0)
> + return ret;
> + reg_mask = ret;
> + reg_mask &= ~(RX8901_EVENTx_CFG_PUPD | RX8901_EVENTx_CFG_POL);
> + reg_mask |= FIELD_PREP(RX8901_EVENTx_CFG_PUPD, pullup_down);
> + reg_mask |= FIELD_PREP(RX8901_EVENTx_CFG_POL, trigger);
> + ret = rv8803_write_reg(client, evin_cfg_regs[evin], reg_mask);
> + if (ret < 0)
> + return ret;
> +
> + /* set EVENTx noise filter */
> + if (filter != -1) {
> + ret = rv8803_write_reg(client, evin_flt_regs[evin], filter);
> + if (ret < 0)
> + return ret;
This check is not needed.
> + }
> +
> + return 0;
return ret;
> +}
> +
> +static int rv8803_ts_enable_events(struct rv8803_data *rv8803, u8 enable_mask)
> +{
> + int ret;
> + u8 reg_mask;
> + struct i2c_client *client = rv8803->client;
> +
> + guard(mutex)(&rv8803->flags_lock);
> +
> + /* event detection interrupt */
> + ret = rv8803_read_reg(client, RV8803_CTRL);
> + if (ret < 0)
> + return ret;
> +
> + reg_mask = ret & ~RV8803_CTRL_EIE;
> + if (enable_mask)
> + reg_mask |= RV8803_CTRL_EIE;
> +
> + ret = rv8803_write_reg(client, RV8803_CTRL, reg_mask);
> + if (ret)
> + return ret;
> +
> + /* events for configuration */
> + ret = rv8803_write_reg(client, RX8901_EVIN_EN, enable_mask);
> + if (ret < 0)
> + return ret;
Not needed
> + return 0;
return ret;
> +}
> +
> +static ssize_t enable_store(struct device *dev, struct device_attribute *attr, const char *buf,
> + size_t count)
> +{
> + int ret;
> + int i;
can be declared in the scope of the for loop
> + 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;
> +
> + /* 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");
dev_dbg
> + /*
> + * 1. disable event detection interrupt
> + * 2. disable events for configuration
> + */
> + ret = rv8803_ts_enable_events(rv8803, 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) {
for (int 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;
> + }
> +
> + scoped_guard(mutex, &rv8803->flags_lock) {
> + /* 4. enable EVENTx interrupt */
> + 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 = (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
> + * 8. / 10. enable events for configuration in FIFO mode
> + */
> + ret = rv8803_ts_enable_events(rv8803, 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) {
if (!num_events)
goto exit_reenable_irq;
would be better readable.
> + 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 */
Is it fine that interrupts are disabled that long?
> + usleep_range(1000, 2000);
> + }
> +
exit_reenable_irq:
> + /* re-enable interrupts */
> + ret = rv8803_read_reg(client, RV8803_CTRL);
Is it fine, that interrupts are only enabled again, if all of the above code
doesn't return with a failure?
> + 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,
> @@ -732,6 +1092,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
>
Powered by blists - more mailing lists