On Tue, Jan 23, 2018 at 10:22:54AM -0800, Guenter Roeck wrote: > On Tue, Jan 23, 2018 at 01:18:01PM +0100, Michael Grzeschik wrote: > > We add support for the ISL1219 chip that got an integrated tamper > > detection function. This patch implements the feature by using an hwmon > > interface. > > > > The ISL1219 can also describe the timestamp of the intrusion > > event. For this we add the documentation of the new interface > > intrusion[0-*]_timestamp. > > > > The devicetree documentation for the ISL1219 device tree > > binding is added with an short example. > > > > Signed-off-by: Michael Grzeschik <[email protected]> > > Signed-off-by: Denis Osterland <[email protected]> > > --- > > .../rtc/{intersil,isl1208.txt => isil,isl1208.txt} | 18 +- > > Documentation/hwmon/sysfs-interface | 7 + > > drivers/rtc/rtc-isl1208.c | 190 > > +++++++++++++++++++-- > > 3 files changed, 201 insertions(+), 14 deletions(-) > > rename Documentation/devicetree/bindings/rtc/{intersil,isl1208.txt => > > isil,isl1208.txt} (57%) > > > > diff --git a/Documentation/devicetree/bindings/rtc/intersil,isl1208.txt > > b/Documentation/devicetree/bindings/rtc/isil,isl1208.txt > > similarity index 57% > > rename from Documentation/devicetree/bindings/rtc/intersil,isl1208.txt > > rename to Documentation/devicetree/bindings/rtc/isil,isl1208.txt > > index a54e99feae1ca..d549699e1cfc4 100644 > > --- a/Documentation/devicetree/bindings/rtc/intersil,isl1208.txt > > +++ b/Documentation/devicetree/bindings/rtc/isil,isl1208.txt > > @@ -1,14 +1,21 @@ > > -Intersil ISL1208, ISL1218 I2C RTC/Alarm chip > > +Intersil ISL1208, ISL1218, ISL1219 I2C RTC/Alarm chip > > > > ISL1208 is a trivial I2C device (it has simple device tree bindings, > > consisting of a compatible field, an address and possibly an interrupt > > line). > > > > +ISL1219 supports tamper detection user space representation through > > +case intrusion hwmon sensor. > > +ISL1219 has additional pins EVIN and #EVDET for tamper detection. > > +I2C devices support only one irq. #IRQ and #EVDET are open-drain active > > low, > > +so it is possible layout them to one SoC pin with pull-up. > > + > > Required properties supported by the device: > > > > - "compatible": must be one of > > "isil,isl1208" > > "isil,isl1218" > > + "isil,isl1219" > > - "reg": I2C bus address of the device > > > > Optional properties: > > @@ -33,3 +40,12 @@ Example isl1208 node with #IRQ pin connected to SoC > > gpio1 pin 12: > > interrupt-parent = <&gpio1>; > > interrupts = <12 IRQ_TYPE_EDGE_FALLING>; > > }; > > + > > +Example isl1219 node with #IRQ pin and #EVDET pin connected to SoC gpio1 > > pin 12: > > + > > + isl1219: isl1219@68 { > > + compatible = "intersil,isl1219"; > > + reg = <0x68>; > > + interrupts-extended = <&gpio1 12 IRQ_TYPE_EDGE_FALLING>; > > + }; > > + > > diff --git a/Documentation/hwmon/sysfs-interface > > b/Documentation/hwmon/sysfs-interface > > index fc337c317c673..a12b3c2b2a18c 100644 > > --- a/Documentation/hwmon/sysfs-interface > > +++ b/Documentation/hwmon/sysfs-interface > > @@ -702,6 +702,13 @@ intrusion[0-*]_alarm > > the user. This is done by writing 0 to the file. Writing > > other values is unsupported. > > > > +intrusion[0-*]_timestamp > > + Chassis intrusion detection > > + YYYY-MM-DD HH:MM:SS UTC (ts.sec): intrusion detected > > + RO > > + The corresponding timestamp on which the intrustion > > + was detected. > > + > > Sneaky. Nack. You don't just add attributes to the ABI because you want it, > without serious discussion, and much less so hidden in an RTC driver > (and even less as unparseable attribute).
Right; but it was not meant to be sneaky. I should have stick to my first
thought and label this patch RFC. Sorry for that.
> In addition to that, I consider the attribute unnecessary. The intrusion
> already generates an event which should be sufficient for all practical
> purposes.
Would it make sense in between the other sysfs attributes of this driver?
Thanks,
Michael
> > intrusion[0-*]_beep
> > Chassis intrusion beep
> > 0: disable
> > diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c
> > index a13a4ba79004d..6e4d24614d98b 100644
> > --- a/drivers/rtc/rtc-isl1208.c
> > +++ b/drivers/rtc/rtc-isl1208.c
> > @@ -14,6 +14,8 @@
> > #include <linux/i2c.h>
> > #include <linux/bcd.h>
> > #include <linux/rtc.h>
> > +#include <linux/hwmon.h>
> > +#include <linux/hwmon-sysfs.h>
> >
> > /* Register map */
> > /* rtc section */
> > @@ -33,6 +35,7 @@
> > #define ISL1208_REG_SR_ARST (1<<7) /* auto reset */
> > #define ISL1208_REG_SR_XTOSCB (1<<6) /* crystal oscillator */
> > #define ISL1208_REG_SR_WRTC (1<<4) /* write rtc */
> > +#define ISL1208_REG_SR_EVT (1<<3) /* event */
> > #define ISL1208_REG_SR_ALM (1<<2) /* alarm */
> > #define ISL1208_REG_SR_BAT (1<<1) /* battery */
> > #define ISL1208_REG_SR_RTCF (1<<0) /* rtc fail */
> > @@ -57,8 +60,29 @@
> > #define ISL1208_REG_USR2 0x13
> > #define ISL1208_USR_SECTION_LEN 2
> >
> > +/* event section */
> > +#define ISL1208_REG_SCT 0x14
> > +#define ISL1208_REG_MNT 0x15
> > +#define ISL1208_REG_HRT 0x16
> > +#define ISL1208_REG_DTT 0x17
> > +#define ISL1208_REG_MOT 0x18
> > +#define ISL1208_REG_YRT 0x19
> > +#define ISL1208_EVT_SECTION_LEN 6
> > +
> > static struct i2c_driver isl1208_driver;
> >
> > +/* ISL1208 various variants */
> > +enum {
> > + TYPE_ISL1208 = 0,
> > + TYPE_ISL1218,
> > + TYPE_ISL1219,
> > +};
> > +
> > +struct isl1208 {
> > + struct rtc_device *rtc;
> > + struct device *hwmon;
> > +};
> > +
> > /* block read */
> > static int
> > isl1208_i2c_read_regs(struct i2c_client *client, u8 reg, u8 buf[],
> > @@ -80,8 +104,8 @@ isl1208_i2c_read_regs(struct i2c_client *client, u8 reg,
> > u8 buf[],
> > };
> > int ret;
> >
> > - BUG_ON(reg > ISL1208_REG_USR2);
> > - BUG_ON(reg + len > ISL1208_REG_USR2 + 1);
> > + WARN_ON(reg > ISL1208_REG_YRT);
> > + WARN_ON(reg + len > ISL1208_REG_YRT + 1);
> >
> > ret = i2c_transfer(client->adapter, msgs, 2);
> > if (ret > 0)
> > @@ -104,8 +128,8 @@ isl1208_i2c_set_regs(struct i2c_client *client, u8 reg,
> > u8 const buf[],
> > };
> > int ret;
> >
> > - BUG_ON(reg > ISL1208_REG_USR2);
> > - BUG_ON(reg + len > ISL1208_REG_USR2 + 1);
> > + WARN_ON(reg > ISL1208_REG_YRT);
> > + WARN_ON(reg + len > ISL1208_REG_YRT + 1);
> >
> > i2c_buf[0] = reg;
> > memcpy(&i2c_buf[1], &buf[0], len);
> > @@ -493,12 +517,128 @@ isl1208_rtc_set_alarm(struct device *dev, struct
> > rtc_wkalrm *alarm)
> > return isl1208_i2c_set_alarm(to_i2c_client(dev), alarm);
> > }
> >
> > +static ssize_t isl1208_hwmon_show_tamper(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct i2c_client *client = dev_get_drvdata(dev);
> > + int sr;
> > +
> > + sr = isl1208_i2c_get_sr(client);
> > + if (sr < 0) {
> > + dev_err(dev, "%s: reading SR failed\n", __func__);
> > + return sr;
> > + }
> > +
> > + if (sr & ISL1208_REG_SR_EVT)
> > + return sprintf(buf, "1\n");
> > +
> > + return sprintf(buf, "0\n");
> > +};
> > +
> > +static ssize_t isl1208_hwmon_clear_caseopen(struct device *dev,
> > + struct device_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + struct i2c_client *client = dev_get_drvdata(dev);
> > + unsigned long val;
> > + int sr;
> > +
> > + if (kstrtoul(buf, 10, &val) || val != 0)
> > + return -EINVAL;
> > +
> > + sr = isl1208_i2c_get_sr(client);
> > + if (sr < 0) {
> > + dev_err(dev, "%s: reading SR failed\n", __func__);
> > + return sr;
> > + }
> > +
> > + sr &= ~ISL1208_REG_SR_EVT;
> > +
> > + sr = i2c_smbus_write_byte_data(client, ISL1208_REG_SR, sr);
> > + if (sr < 0)
> > + dev_err(dev, "%s: writing SR failed\n",
> > + __func__);
> > +
> > + return count;
> > +};
> > +
> > +static ssize_t isl1208_hwmon_show_tamper_timestamp(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct i2c_client *client = dev_get_drvdata(dev);
> > + u8 regs[ISL1208_EVT_SECTION_LEN] = { 0, };
> > + struct timespec64 tv64;
> > + struct rtc_time tm;
> > + int sr;
> > +
> > + sr = isl1208_i2c_get_sr(client);
> > + if (sr < 0) {
> > + dev_err(dev, "%s: reading SR failed\n", __func__);
> > + return sr;
> > + }
> > +
> > + if (!(sr & ISL1208_REG_SR_EVT))
> > + return 0;
> > +
> > + sr = isl1208_i2c_read_regs(client, ISL1208_REG_SCT, regs,
> > + ISL1208_EVT_SECTION_LEN);
> > + if (sr < 0) {
> > + dev_err(dev, "%s: reading event section failed\n",
> > + __func__);
> > + return 0;
> > + }
> > +
> > + /* MSB of each alarm register is an enable bit */
> > + tm.tm_sec = bcd2bin(regs[ISL1208_REG_SCT - ISL1208_REG_SCT] & 0x7f);
> > + tm.tm_min = bcd2bin(regs[ISL1208_REG_MNT - ISL1208_REG_SCT] & 0x7f);
> > + tm.tm_hour = bcd2bin(regs[ISL1208_REG_HRT - ISL1208_REG_SCT] & 0x3f);
> > + tm.tm_mday = bcd2bin(regs[ISL1208_REG_DTT - ISL1208_REG_SCT] & 0x3f);
> > + tm.tm_mon =
> > + bcd2bin(regs[ISL1208_REG_MOT - ISL1208_REG_SCT] & 0x1f) - 1;
> > + tm.tm_year = bcd2bin(regs[ISL1208_REG_YRT - ISL1208_REG_SCT]) + 100;
> > +
> > + tv64.tv_sec = rtc_tm_to_time64(&tm);
> > +
> > + return sprintf(buf,
> > + "%d-%02d-%02d %02d:%02d:%02d UTC (%lld)\n",
> > + tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday,
> > + tm.tm_hour, tm.tm_min, tm.tm_sec,
> > + (long long) tv64.tv_sec);
> > +};
> > +
> > +static SENSOR_DEVICE_ATTR(intrusion0_alarm, 0644,
> > isl1208_hwmon_show_tamper,
> > + isl1208_hwmon_clear_caseopen, 0);
> > +
> > +static SENSOR_DEVICE_ATTR(intrusion0_timestamp, 0444,
> > + isl1208_hwmon_show_tamper_timestamp, NULL, 0);
> > +
> > +static struct attribute *isl1208_hwmon_attrs[] = {
> > + &sensor_dev_attr_intrusion0_alarm.dev_attr.attr,
> > + &sensor_dev_attr_intrusion0_timestamp.dev_attr.attr,
> > + NULL,
> > +};
> > +ATTRIBUTE_GROUPS(isl1208_hwmon);
> > +
> > +static void isl1208_hwmon_register(struct i2c_client *client)
> > +{
> > + struct isl1208 *isl1208 = i2c_get_clientdata(client);
> > +
> > + isl1208->hwmon = devm_hwmon_device_register_with_groups(&client->dev,
> > + client->name,
> > + client, isl1208_hwmon_groups);
> > + if (IS_ERR(isl1208->hwmon)) {
> > + dev_warn(&client->dev,
> > + "unable to register hwmon device %ld\n",
> > + PTR_ERR(isl1208->hwmon));
> > + }
> > +}
> > +
> > static irqreturn_t
> > isl1208_rtc_interrupt(int irq, void *data)
> > {
> > unsigned long timeout = jiffies + msecs_to_jiffies(1000);
> > struct i2c_client *client = data;
> > - struct rtc_device *rtc = i2c_get_clientdata(client);
> > + struct isl1208 *isl1208 = i2c_get_clientdata(client);
> > int handled = 0, sr, err;
> >
> > /*
> > @@ -521,7 +661,7 @@ isl1208_rtc_interrupt(int irq, void *data)
> > if (sr & ISL1208_REG_SR_ALM) {
> > dev_dbg(&client->dev, "alarm!\n");
> >
> > - rtc_update_irq(rtc, 1, RTC_IRQF | RTC_AF);
> > + rtc_update_irq(isl1208->rtc, 1, RTC_IRQF | RTC_AF);
> >
> > /* Clear the alarm */
> > sr &= ~ISL1208_REG_SR_ALM;
> > @@ -538,6 +678,13 @@ isl1208_rtc_interrupt(int irq, void *data)
> > return err;
> > }
> >
> > + if (isl1208->hwmon && (sr & ISL1208_REG_SR_EVT)) {
> > + sysfs_notify(&isl1208->hwmon->kobj, NULL,
> > + sensor_dev_attr_intrusion0_alarm.dev_attr.attr.name);
> > + dev_warn(&client->dev, "event detected");
> > + handled = 1;
> > + }
> > +
> > return handled ? IRQ_HANDLED : IRQ_NONE;
> > }
> >
> > @@ -627,7 +774,7 @@ static int
> > isl1208_probe(struct i2c_client *client, const struct i2c_device_id *id)
> > {
> > int rc = 0;
> > - struct rtc_device *rtc;
> > + struct isl1208 *isl1208;
> >
> > if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
> > return -ENODEV;
> > @@ -635,13 +782,19 @@ isl1208_probe(struct i2c_client *client, const struct
> > i2c_device_id *id)
> > if (isl1208_i2c_validate_client(client) < 0)
> > return -ENODEV;
> >
> > - rtc = devm_rtc_device_register(&client->dev, isl1208_driver.driver.name,
> > + isl1208 = devm_kzalloc(&client->dev, sizeof(struct isl1208),
> > + GFP_KERNEL);
> > + if (!isl1208)
> > + return -ENOMEM;
> > +
> > + isl1208->rtc = devm_rtc_device_register(&client->dev,
> > + isl1208_driver.driver.name,
> > &isl1208_rtc_ops,
> > THIS_MODULE);
> > - if (IS_ERR(rtc))
> > - return PTR_ERR(rtc);
> > + if (IS_ERR(isl1208->rtc))
> > + return PTR_ERR(isl1208->rtc);
> >
> > - i2c_set_clientdata(client, rtc);
> > + i2c_set_clientdata(client, isl1208);
> >
> > rc = isl1208_i2c_get_sr(client);
> > if (rc < 0) {
> > @@ -657,6 +810,15 @@ isl1208_probe(struct i2c_client *client, const struct
> > i2c_device_id *id)
> > if (rc)
> > return rc;
> >
> > + if (id->driver_data == TYPE_ISL1219) {
> > + rc = i2c_smbus_write_byte_data(client, ISL1208_REG_09, 0x10);
> > + if (rc < 0) {
> > + dev_err(&client->dev, "could not enable tamper
> > detection\n");
> > + return rc;
> > + }
> > + isl1208_hwmon_register(client);
> > + }
> > +
> > if (client->irq > 0) {
> > rc = devm_request_threaded_irq(&client->dev, client->irq, NULL,
> > isl1208_rtc_interrupt,
> > @@ -686,8 +848,9 @@ isl1208_remove(struct i2c_client *client)
> > }
> >
> > static const struct i2c_device_id isl1208_id[] = {
> > - { "isl1208", 0 },
> > - { "isl1218", 0 },
> > + { "isl1208", TYPE_ISL1208 },
> > + { "isl1218", TYPE_ISL1218 },
> > + { "isl1219", TYPE_ISL1219 },
> > { }
> > };
> > MODULE_DEVICE_TABLE(i2c, isl1208_id);
> > @@ -695,6 +858,7 @@ MODULE_DEVICE_TABLE(i2c, isl1208_id);
> > static const struct of_device_id isl1208_of_match[] = {
> > { .compatible = "isil,isl1208" },
> > { .compatible = "isil,isl1218" },
> > + { .compatible = "isil,isl1219" },
> > { }
> > };
> > MODULE_DEVICE_TABLE(of, isl1208_of_match);
> > --
> > 2.11.0
> >
>
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
signature.asc
Description: PGP signature

