All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 4/4] rtc: isl1208: Add "evdet" interrupt source for isl1219.
  2018-03-05 10:43 [PATCH v3 0/4] rtc: isl1208: fixes, documentation and isl1219 support Denis OSTERLAND
@ 2018-03-05 10:43 ` Denis OSTERLAND
  2018-03-05 10:43 ` [PATCH v3 3/4] rtc: isl1208: add support for isl1219 with tamper detection Denis OSTERLAND
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 15+ messages in thread
From: Denis OSTERLAND @ 2018-03-05 10:43 UTC (permalink / raw)
  To: a.zummo, alexandre.belloni
  Cc: linux-kernel, mgr, devicetree, linux, jdelvare, linux-rtc,
	kernel, Denis OSTERLAND

From: Denis Osterland <Denis.Osterland@diehl.com>

Add support for "evdet" named interrupt source.

The check if i2c client irq matches evdet irq is needed
for the case that there is only one interrupt named "evdet".
In this case i2c client code handles this like an unnamed
interrupt souce and assigns the value.

Signed-off-by: Denis Osterland <Denis.Osterland@diehl.com>
Reviewed-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/rtc/rtc-isl1208.c | 47 +++++++++++++++++++++++++++++++----------------
 1 file changed, 31 insertions(+), 16 deletions(-)

diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c
index 164371b..f2f148b 100644
--- a/drivers/rtc/rtc-isl1208.c
+++ b/drivers/rtc/rtc-isl1208.c
@@ -14,6 +14,7 @@
 #include <linux/i2c.h>
 #include <linux/bcd.h>
 #include <linux/rtc.h>
+#include <linux/of_irq.h>
 
 /* Register map */
 /* rtc section */
@@ -79,6 +80,7 @@ enum {
 struct isl1208 {
 	struct rtc_device *rtc;
 	const struct attribute_group *sysfs_files;
+	int evdet_irq;
 };
 
 /* block read */
@@ -730,6 +732,24 @@ static const struct attribute_group isl1219_rtc_sysfs_files = {
 	.attrs	= isl1219_rtc_attrs,
 };
 
+static int isl1208_setup_irq(struct i2c_client *client, int irq)
+{
+	int rc = devm_request_threaded_irq(&client->dev, irq, NULL,
+					isl1208_rtc_interrupt,
+					IRQF_SHARED | IRQF_ONESHOT,
+					isl1208_driver.driver.name,
+					client);
+	if (!rc) {
+		device_init_wakeup(&client->dev, 1);
+		enable_irq_wake(irq);
+	} else {
+		dev_err(&client->dev,
+			"Unable to request irq %d, no alarm support\n",
+			irq);
+	}
+	return rc;
+}
+
 static int
 isl1208_probe(struct i2c_client *client, const struct i2c_device_id *id)
 {
@@ -772,6 +792,8 @@ isl1208_probe(struct i2c_client *client, const struct i2c_device_id *id)
 			return rc;
 		}
 		isl1208->sysfs_files = &isl1219_rtc_sysfs_files;
+		isl1208->evdet_irq = of_irq_get_byname(client->dev.of_node,
+								"evdet");
 	} else {
 		isl1208->sysfs_files = &isl1208_rtc_sysfs_files;
 	}
@@ -780,22 +802,15 @@ isl1208_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	if (rc)
 		return rc;
 
-	if (client->irq > 0) {
-		rc = devm_request_threaded_irq(&client->dev, client->irq, NULL,
-					       isl1208_rtc_interrupt,
-					       IRQF_SHARED | IRQF_ONESHOT,
-					       isl1208_driver.driver.name,
-					       client);
-		if (!rc) {
-			device_init_wakeup(&client->dev, 1);
-			enable_irq_wake(client->irq);
-		} else {
-			dev_err(&client->dev,
-				"Unable to request irq %d, no alarm support\n",
-				client->irq);
-			client->irq = 0;
-		}
-	}
+	if (client->irq > 0)
+		rc = isl1208_setup_irq(client, client->irq);
+	if (rc)
+		return rc;
+
+	if (isl1208->evdet_irq > 0 && isl1208->evdet_irq != client->irq)
+		rc = isl1208_setup_irq(client, isl1208->evdet_irq);
+	if (rc)
+		return rc;
 
 	return rtc_register_device(isl1208->rtc);
 }
-- 
2.7.4


Diehl AKO Stiftung & Co. KG, Pfannerstraße 75-83, 88239 Wangen im Allgäu
Bereichsvorstand: Dr.-Ing. Michael Siedentop (Sprecher), Josef Fellner (Mitglied)
Sitz der Gesellschaft: Wangen i.A. – Registergericht: Amtsgericht Ulm HRA 620609 – Persönlich haftende Gesellschafterin: Diehl Verwaltungs-Stiftung – Sitz: Nürnberg – Registergericht: Amtsgericht Nürnberg HRA 11756 –
Vorstand: Dr.-Ing. E.h. Thomas Diehl (†) (Vorsitzender), Herr Dipl.-Wirtsch.-Ing. Wolfgang Weggen (stellvertretender Vorsitzender), Dipl.-Kfm. Claus Günther, Dipl.-Kfm. Frank Gutzeit, Dr.-Ing. Heinrich Schunk, Dr.-Ing. Michael Siedentop , Dipl.-Kfm. Dr.-Ing. Martin Sommer, Dipl.-Ing. (FH) Rainer von Borstel, Vorsitzender des Aufsichtsrates: Dr. Klaus Maier
___________________________________________________________________________________________________
Der Inhalt der vorstehenden E-Mail ist nicht rechtlich bindend. Diese E-Mail enthaelt vertrauliche und/oder rechtlich geschuetzte Informationen.
Informieren Sie uns bitte, wenn Sie diese E-Mail faelschlicherweise erhalten haben. Bitte loeschen Sie in diesem Fall die Nachricht. Jede unerlaubte Form der Reproduktion, Bekanntgabe, Aenderung, Verteilung und/oder Publikation dieser E-Mail ist strengstens untersagt.
The contents of the above mentioned e-mail is not legally binding. This e-mail contains confidential and/or legally protected information. Please inform us if you have received this e-mail by mistake and delete it in such a case. Each unauthorized reproduction, disclosure, alteration, distribution and/or publication of this e-mail is strictly prohibited.

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 3/4] rtc: isl1208: add support for isl1219 with tamper detection
  2018-03-05 10:43 [PATCH v3 0/4] rtc: isl1208: fixes, documentation and isl1219 support Denis OSTERLAND
  2018-03-05 10:43 ` [PATCH v3 4/4] rtc: isl1208: Add "evdet" interrupt source for isl1219 Denis OSTERLAND
@ 2018-03-05 10:43 ` Denis OSTERLAND
  2018-03-06 20:42   ` Alexandre Belloni
  2018-03-07 22:02   ` Rob Herring
  2018-03-05 10:43 ` [PATCH v3 2/4] rtc: isl1208: switch to rtc_register_device Denis OSTERLAND
  2018-03-05 10:43 ` [PATCH v3 1/4] rtc: isl1208: enable interrupt after context preparation Denis OSTERLAND
  3 siblings, 2 replies; 15+ messages in thread
From: Denis OSTERLAND @ 2018-03-05 10:43 UTC (permalink / raw)
  To: a.zummo, alexandre.belloni
  Cc: linux-kernel, mgr, m.grzeschik, devicetree, linux, jdelvare,
	linux-rtc, kernel, Denis OSTERLAND

From: Michael Grzeschik <m.grzeschik@pengutronix.de>

We add support for the ISL1219 chip that got an integrated tamper
detection function. This patch implements the feature by adding
an additional timestamp0 file to sysfs device path.
This file contains seconds since epoch, if an event occurred,
or is empty, if none occurred.

The devicetree documentation for the ISL1219 device tree
binding is added with an short example. It is not a trivial
device, because it supports two interrupt souces.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
Signed-off-by: Denis Osterland <Denis.Osterland@diehl.com>
---
 .../devicetree/bindings/rtc/isil,isl1219.txt       |  28 ++++
 drivers/rtc/rtc-isl1208.c                          | 160 ++++++++++++++++++---
 2 files changed, 171 insertions(+), 17 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/rtc/isil,isl1219.txt

diff --git a/Documentation/devicetree/bindings/rtc/isil,isl1219.txt b/Documentation/devicetree/bindings/rtc/isil,isl1219.txt
new file mode 100644
index 0000000..7937c13
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/isil,isl1219.txt
@@ -0,0 +1,28 @@
+Intersil ISL1219 I2C RTC/Alarm chip with event in
+
+ISL1219 has additional pins EVIN and #EVDET for tamper detection.
+
+Required properties supported by the device:
+
+ - "compatible": must be "isil,isl1219"
+ - "reg": I2C bus address of the device
+
+Optional properties:
+
+ - "interrupt-names": list which may contains "irq" and "evdet"
+ - "interrupt-parent", "interrupts", "interrupts-extended":
+   for passing the interrupt line of the SoC connected to #IRQ pin
+   and #EVDET pin of the RTC chip.
+
+
+Example isl1219 node with #IRQ pin connected to SoC gpio1 pin12
+ and #EVDET pin connected to SoC gpio2 pin 24:
+
+	isl1219: rtc@68 {
+		compatible = "isil,isl1219";
+		reg = <0x68>;
+		interrupt-names = "irq", "evdet";
+		interrupts-extended = <&gpio1 12 IRQ_TYPE_EDGE_FALLING>,
+			<&gpio2 24 IRQ_TYPE_EDGE_FALLING>;
+	};
+
diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c
index 1a2c38c..164371b 100644
--- a/drivers/rtc/rtc-isl1208.c
+++ b/drivers/rtc/rtc-isl1208.c
@@ -33,6 +33,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 +58,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;
+	const struct attribute_group *sysfs_files;
+};
+
 /* block read */
 static int
 isl1208_i2c_read_regs(struct i2c_client *client, u8 reg, u8 buf[],
@@ -80,8 +102,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 +126,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 +515,78 @@ 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_rtc_event_clear(struct device *dev,
+		struct device_attribute *attr,
+		const char *buf, size_t count)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	int sr;
+
+	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_rtc_event_show_timestamp(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct i2c_client *client = to_i2c_client(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 sprintf(buf, "\n");
+
+	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, "%lld\n", (long long) tv64.tv_sec);
+};
+
+static DEVICE_ATTR(timestamp0, 0640,
+		isl1208_rtc_event_show_timestamp, isl1208_rtc_event_clear);
+
 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 +609,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 +626,13 @@ isl1208_rtc_interrupt(int irq, void *data)
 			return err;
 	}
 
+	if (sr & ISL1208_REG_SR_EVT) {
+		sysfs_notify(&client->dev.kobj, NULL,
+			dev_attr_timestamp0.attr.name);
+		dev_warn(&client->dev, "event detected");
+		handled = 1;
+	}
+
 	return handled ? IRQ_HANDLED : IRQ_NONE;
 }
 
@@ -623,11 +718,23 @@ static const struct attribute_group isl1208_rtc_sysfs_files = {
 	.attrs	= isl1208_rtc_attrs,
 };
 
+static struct attribute *isl1219_rtc_attrs[] = {
+	&dev_attr_atrim.attr,
+	&dev_attr_dtrim.attr,
+	&dev_attr_usr.attr,
+	&dev_attr_timestamp0.attr,
+	NULL
+};
+
+static const struct attribute_group isl1219_rtc_sysfs_files = {
+	.attrs	= isl1219_rtc_attrs,
+};
+
 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 +742,18 @@ isl1208_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	if (isl1208_i2c_validate_client(client) < 0)
 		return -ENODEV;
 
-	rtc = devm_rtc_allocate_device(&client->dev);
-	if (IS_ERR(rtc))
-		return PTR_ERR(rtc);
+	isl1208 = devm_kzalloc(&client->dev, sizeof(struct isl1208),
+				GFP_KERNEL);
+	if (!isl1208)
+		return -ENOMEM;
 
-	rtc->ops = &isl1208_rtc_ops;
+	isl1208->rtc = devm_rtc_allocate_device(&client->dev);
+	if (IS_ERR(isl1208->rtc))
+		return PTR_ERR(isl1208->rtc);
 
-	i2c_set_clientdata(client, rtc);
+	isl1208->rtc->ops = &isl1208_rtc_ops;
+
+	i2c_set_clientdata(client, isl1208);
 
 	rc = isl1208_i2c_get_sr(client);
 	if (rc < 0) {
@@ -653,7 +765,18 @@ isl1208_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		dev_warn(&client->dev, "rtc power failure detected, "
 			 "please set clock.\n");
 
-	rc = sysfs_create_group(&client->dev.kobj, &isl1208_rtc_sysfs_files);
+	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->sysfs_files = &isl1219_rtc_sysfs_files;
+	} else {
+		isl1208->sysfs_files = &isl1208_rtc_sysfs_files;
+	}
+
+	rc = sysfs_create_group(&client->dev.kobj, isl1208->sysfs_files);
 	if (rc)
 		return rc;
 
@@ -674,20 +797,23 @@ isl1208_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		}
 	}
 
-	return rtc_register_device(rtc);
+	return rtc_register_device(isl1208->rtc);
 }
 
 static int
 isl1208_remove(struct i2c_client *client)
 {
-	sysfs_remove_group(&client->dev.kobj, &isl1208_rtc_sysfs_files);
+	struct isl1208 *isl1208 = i2c_get_clientdata(client);
+
+	sysfs_remove_group(&client->dev.kobj, isl1208->sysfs_files);
 
 	return 0;
 }
 
 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);
-- 
2.7.4


Diehl AKO Stiftung & Co. KG, Pfannerstraße 75-83, 88239 Wangen im Allgäu
Bereichsvorstand: Dr.-Ing. Michael Siedentop (Sprecher), Josef Fellner (Mitglied)
Sitz der Gesellschaft: Wangen i.A. – Registergericht: Amtsgericht Ulm HRA 620609 – Persönlich haftende Gesellschafterin: Diehl Verwaltungs-Stiftung – Sitz: Nürnberg – Registergericht: Amtsgericht Nürnberg HRA 11756 –
Vorstand: Dr.-Ing. E.h. Thomas Diehl (†) (Vorsitzender), Herr Dipl.-Wirtsch.-Ing. Wolfgang Weggen (stellvertretender Vorsitzender), Dipl.-Kfm. Claus Günther, Dipl.-Kfm. Frank Gutzeit, Dr.-Ing. Heinrich Schunk, Dr.-Ing. Michael Siedentop , Dipl.-Kfm. Dr.-Ing. Martin Sommer, Dipl.-Ing. (FH) Rainer von Borstel, Vorsitzender des Aufsichtsrates: Dr. Klaus Maier
___________________________________________________________________________________________________
Der Inhalt der vorstehenden E-Mail ist nicht rechtlich bindend. Diese E-Mail enthaelt vertrauliche und/oder rechtlich geschuetzte Informationen.
Informieren Sie uns bitte, wenn Sie diese E-Mail faelschlicherweise erhalten haben. Bitte loeschen Sie in diesem Fall die Nachricht. Jede unerlaubte Form der Reproduktion, Bekanntgabe, Aenderung, Verteilung und/oder Publikation dieser E-Mail ist strengstens untersagt.
The contents of the above mentioned e-mail is not legally binding. This e-mail contains confidential and/or legally protected information. Please inform us if you have received this e-mail by mistake and delete it in such a case. Each unauthorized reproduction, disclosure, alteration, distribution and/or publication of this e-mail is strictly prohibited.

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 1/4] rtc: isl1208: enable interrupt after context preparation
  2018-03-05 10:43 [PATCH v3 0/4] rtc: isl1208: fixes, documentation and isl1219 support Denis OSTERLAND
                   ` (2 preceding siblings ...)
  2018-03-05 10:43 ` [PATCH v3 2/4] rtc: isl1208: switch to rtc_register_device Denis OSTERLAND
@ 2018-03-05 10:43 ` Denis OSTERLAND
  2018-03-06 20:20   ` Alexandre Belloni
  3 siblings, 1 reply; 15+ messages in thread
From: Denis OSTERLAND @ 2018-03-05 10:43 UTC (permalink / raw)
  To: a.zummo, alexandre.belloni
  Cc: linux-kernel, mgr, m.grzeschik, devicetree, linux, jdelvare,
	linux-rtc, kernel, Denis OSTERLAND

From: Michael Grzeschik <m.grzeschik@pengutronix.de>

The interrupt handler got enabled very early. If the interrupt cause is
triggering immediately before the context is fully prepared. This can
lead to undefined behaviour. Therefor we move the interrupt enable code
to the end of the probe function.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
Signed-off-by: Denis Osterland <Denis.Osterland@diehl.com>
---
 drivers/rtc/rtc-isl1208.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c
index c8b4953..a13a4ba 100644
--- a/drivers/rtc/rtc-isl1208.c
+++ b/drivers/rtc/rtc-isl1208.c
@@ -635,23 +635,6 @@ isl1208_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	if (isl1208_i2c_validate_client(client) < 0)
 		return -ENODEV;
 
-	if (client->irq > 0) {
-		rc = devm_request_threaded_irq(&client->dev, client->irq, NULL,
-					       isl1208_rtc_interrupt,
-					       IRQF_SHARED | IRQF_ONESHOT,
-					       isl1208_driver.driver.name,
-					       client);
-		if (!rc) {
-			device_init_wakeup(&client->dev, 1);
-			enable_irq_wake(client->irq);
-		} else {
-			dev_err(&client->dev,
-				"Unable to request irq %d, no alarm support\n",
-				client->irq);
-			client->irq = 0;
-		}
-	}
-
 	rtc = devm_rtc_device_register(&client->dev, isl1208_driver.driver.name,
 				  &isl1208_rtc_ops,
 				  THIS_MODULE);
@@ -674,6 +657,23 @@ isl1208_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	if (rc)
 		return rc;
 
+	if (client->irq > 0) {
+		rc = devm_request_threaded_irq(&client->dev, client->irq, NULL,
+					       isl1208_rtc_interrupt,
+					       IRQF_SHARED | IRQF_ONESHOT,
+					       isl1208_driver.driver.name,
+					       client);
+		if (!rc) {
+			device_init_wakeup(&client->dev, 1);
+			enable_irq_wake(client->irq);
+		} else {
+			dev_err(&client->dev,
+				"Unable to request irq %d, no alarm support\n",
+				client->irq);
+			client->irq = 0;
+		}
+	}
+
 	return 0;
 }
 
-- 
2.7.4


Diehl AKO Stiftung & Co. KG, Pfannerstraße 75-83, 88239 Wangen im Allgäu
Bereichsvorstand: Dr.-Ing. Michael Siedentop (Sprecher), Josef Fellner (Mitglied)
Sitz der Gesellschaft: Wangen i.A. – Registergericht: Amtsgericht Ulm HRA 620609 – Persönlich haftende Gesellschafterin: Diehl Verwaltungs-Stiftung – Sitz: Nürnberg – Registergericht: Amtsgericht Nürnberg HRA 11756 –
Vorstand: Dr.-Ing. E.h. Thomas Diehl (†) (Vorsitzender), Herr Dipl.-Wirtsch.-Ing. Wolfgang Weggen (stellvertretender Vorsitzender), Dipl.-Kfm. Claus Günther, Dipl.-Kfm. Frank Gutzeit, Dr.-Ing. Heinrich Schunk, Dr.-Ing. Michael Siedentop , Dipl.-Kfm. Dr.-Ing. Martin Sommer, Dipl.-Ing. (FH) Rainer von Borstel, Vorsitzender des Aufsichtsrates: Dr. Klaus Maier
___________________________________________________________________________________________________
Der Inhalt der vorstehenden E-Mail ist nicht rechtlich bindend. Diese E-Mail enthaelt vertrauliche und/oder rechtlich geschuetzte Informationen.
Informieren Sie uns bitte, wenn Sie diese E-Mail faelschlicherweise erhalten haben. Bitte loeschen Sie in diesem Fall die Nachricht. Jede unerlaubte Form der Reproduktion, Bekanntgabe, Aenderung, Verteilung und/oder Publikation dieser E-Mail ist strengstens untersagt.
The contents of the above mentioned e-mail is not legally binding. This e-mail contains confidential and/or legally protected information. Please inform us if you have received this e-mail by mistake and delete it in such a case. Each unauthorized reproduction, disclosure, alteration, distribution and/or publication of this e-mail is strictly prohibited.

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 0/4] rtc: isl1208: fixes, documentation and isl1219 support
@ 2018-03-05 10:43 Denis OSTERLAND
  2018-03-05 10:43 ` [PATCH v3 4/4] rtc: isl1208: Add "evdet" interrupt source for isl1219 Denis OSTERLAND
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Denis OSTERLAND @ 2018-03-05 10:43 UTC (permalink / raw)
  To: a.zummo, alexandre.belloni
  Cc: linux-kernel, mgr, devicetree, linux, jdelvare, linux-rtc,
	kernel, Denis OSTERLAND

changes since v2:
Fix rebase issue in 2/4 and 3/4, where 2/4 uses a data type declared 3/4.

changes since v1:
Represent isl1219 tamper detection as RTC timestamp event,
instead of hwmon intrusion sensor.
Switch to rtc_register_device, to fix possible race conditions in probe.
Add documentation of the two possible interrupt sources for isl1219.
Support "evdet" named interrupt souce.

Michael Grzeschik (2):
  rtc: isl1208: enable interrupt after context preparation
  rtc: isl1208: add support for isl1219 with tamper detection

Denis Osterland (2):
  rtc: isl1208: switch to rtc_register_device
  rtc: isl1208: Add "evdet" interrupt source for isl1219.

 .../devicetree/bindings/rtc/isil,isl1219.txt       |  28 +++
 drivers/rtc/rtc-isl1208.c                          | 209 +++++++++++++++++----
 2 files changed, 203 insertions(+), 34 deletions(-)

Message-Id: 1519821214-22379-1-git-send-email-Denis.Osterland@diehl.com
Message-Id: 20180123121801.4214-1-m.grzeschik@pengutronix.de


Diehl AKO Stiftung & Co. KG, Pfannerstraße 75-83, 88239 Wangen im Allgäu
Bereichsvorstand: Dr.-Ing. Michael Siedentop (Sprecher), Josef Fellner (Mitglied)
Sitz der Gesellschaft: Wangen i.A. – Registergericht: Amtsgericht Ulm HRA 620609 – Persönlich haftende Gesellschafterin: Diehl Verwaltungs-Stiftung – Sitz: Nürnberg – Registergericht: Amtsgericht Nürnberg HRA 11756 –
Vorstand: Dr.-Ing. E.h. Thomas Diehl (†) (Vorsitzender), Herr Dipl.-Wirtsch.-Ing. Wolfgang Weggen (stellvertretender Vorsitzender), Dipl.-Kfm. Claus Günther, Dipl.-Kfm. Frank Gutzeit, Dr.-Ing. Heinrich Schunk, Dr.-Ing. Michael Siedentop , Dipl.-Kfm. Dr.-Ing. Martin Sommer, Dipl.-Ing. (FH) Rainer von Borstel, Vorsitzender des Aufsichtsrates: Dr. Klaus Maier
___________________________________________________________________________________________________
Der Inhalt der vorstehenden E-Mail ist nicht rechtlich bindend. Diese E-Mail enthaelt vertrauliche und/oder rechtlich geschuetzte Informationen.
Informieren Sie uns bitte, wenn Sie diese E-Mail faelschlicherweise erhalten haben. Bitte loeschen Sie in diesem Fall die Nachricht. Jede unerlaubte Form der Reproduktion, Bekanntgabe, Aenderung, Verteilung und/oder Publikation dieser E-Mail ist strengstens untersagt.
The contents of the above mentioned e-mail is not legally binding. This e-mail contains confidential and/or legally protected information. Please inform us if you have received this e-mail by mistake and delete it in such a case. Each unauthorized reproduction, disclosure, alteration, distribution and/or publication of this e-mail is strictly prohibited.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v3 2/4] rtc: isl1208: switch to rtc_register_device
  2018-03-05 10:43 [PATCH v3 0/4] rtc: isl1208: fixes, documentation and isl1219 support Denis OSTERLAND
  2018-03-05 10:43 ` [PATCH v3 4/4] rtc: isl1208: Add "evdet" interrupt source for isl1219 Denis OSTERLAND
  2018-03-05 10:43 ` [PATCH v3 3/4] rtc: isl1208: add support for isl1219 with tamper detection Denis OSTERLAND
@ 2018-03-05 10:43 ` Denis OSTERLAND
  2018-03-06 20:20   ` Alexandre Belloni
  2018-03-05 10:43 ` [PATCH v3 1/4] rtc: isl1208: enable interrupt after context preparation Denis OSTERLAND
  3 siblings, 1 reply; 15+ messages in thread
From: Denis OSTERLAND @ 2018-03-05 10:43 UTC (permalink / raw)
  To: a.zummo, alexandre.belloni
  Cc: linux-kernel, mgr, devicetree, linux, jdelvare, linux-rtc,
	kernel, Denis OSTERLAND

From: Denis Osterland <Denis.Osterland@diehl.com>

Fix possible race condition.
It is not allowed to return with an error code after RTC is registered.

Suggested-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Signed-off-by: Denis Osterland <Denis.Osterland@diehl.com>
Reviewed-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/rtc/rtc-isl1208.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c
index a13a4ba..1a2c38c 100644
--- a/drivers/rtc/rtc-isl1208.c
+++ b/drivers/rtc/rtc-isl1208.c
@@ -635,12 +635,12 @@ 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_rtc_ops,
-				  THIS_MODULE);
+	rtc = devm_rtc_allocate_device(&client->dev);
 	if (IS_ERR(rtc))
 		return PTR_ERR(rtc);
 
+	rtc->ops = &isl1208_rtc_ops;
+
 	i2c_set_clientdata(client, rtc);
 
 	rc = isl1208_i2c_get_sr(client);
@@ -674,7 +674,7 @@ isl1208_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		}
 	}
 
-	return 0;
+	return rtc_register_device(rtc);
 }
 
 static int
-- 
2.7.4


Diehl AKO Stiftung & Co. KG, Pfannerstraße 75-83, 88239 Wangen im Allgäu
Bereichsvorstand: Dr.-Ing. Michael Siedentop (Sprecher), Josef Fellner (Mitglied)
Sitz der Gesellschaft: Wangen i.A. – Registergericht: Amtsgericht Ulm HRA 620609 – Persönlich haftende Gesellschafterin: Diehl Verwaltungs-Stiftung – Sitz: Nürnberg – Registergericht: Amtsgericht Nürnberg HRA 11756 –
Vorstand: Dr.-Ing. E.h. Thomas Diehl (†) (Vorsitzender), Herr Dipl.-Wirtsch.-Ing. Wolfgang Weggen (stellvertretender Vorsitzender), Dipl.-Kfm. Claus Günther, Dipl.-Kfm. Frank Gutzeit, Dr.-Ing. Heinrich Schunk, Dr.-Ing. Michael Siedentop , Dipl.-Kfm. Dr.-Ing. Martin Sommer, Dipl.-Ing. (FH) Rainer von Borstel, Vorsitzender des Aufsichtsrates: Dr. Klaus Maier
___________________________________________________________________________________________________
Der Inhalt der vorstehenden E-Mail ist nicht rechtlich bindend. Diese E-Mail enthaelt vertrauliche und/oder rechtlich geschuetzte Informationen.
Informieren Sie uns bitte, wenn Sie diese E-Mail faelschlicherweise erhalten haben. Bitte loeschen Sie in diesem Fall die Nachricht. Jede unerlaubte Form der Reproduktion, Bekanntgabe, Aenderung, Verteilung und/oder Publikation dieser E-Mail ist strengstens untersagt.
The contents of the above mentioned e-mail is not legally binding. This e-mail contains confidential and/or legally protected information. Please inform us if you have received this e-mail by mistake and delete it in such a case. Each unauthorized reproduction, disclosure, alteration, distribution and/or publication of this e-mail is strictly prohibited.

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 1/4] rtc: isl1208: enable interrupt after context preparation
  2018-03-05 10:43 ` [PATCH v3 1/4] rtc: isl1208: enable interrupt after context preparation Denis OSTERLAND
@ 2018-03-06 20:20   ` Alexandre Belloni
  0 siblings, 0 replies; 15+ messages in thread
From: Alexandre Belloni @ 2018-03-06 20:20 UTC (permalink / raw)
  To: Denis OSTERLAND
  Cc: a.zummo, linux-kernel, mgr, m.grzeschik, devicetree, linux,
	jdelvare, linux-rtc, kernel

On 05/03/2018 at 10:43:53 +0000, Denis OSTERLAND wrote:
> From: Michael Grzeschik <m.grzeschik@pengutronix.de>
> 
> The interrupt handler got enabled very early. If the interrupt cause is
> triggering immediately before the context is fully prepared. This can
> lead to undefined behaviour. Therefor we move the interrupt enable code
> to the end of the probe function.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> Signed-off-by: Denis Osterland <Denis.Osterland@diehl.com>
> ---
>  drivers/rtc/rtc-isl1208.c | 34 +++++++++++++++++-----------------
>  1 file changed, 17 insertions(+), 17 deletions(-)
> 
Applied, thanks.

-- 
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 2/4] rtc: isl1208: switch to rtc_register_device
  2018-03-05 10:43 ` [PATCH v3 2/4] rtc: isl1208: switch to rtc_register_device Denis OSTERLAND
@ 2018-03-06 20:20   ` Alexandre Belloni
  0 siblings, 0 replies; 15+ messages in thread
From: Alexandre Belloni @ 2018-03-06 20:20 UTC (permalink / raw)
  To: Denis OSTERLAND
  Cc: a.zummo, linux-kernel, mgr, devicetree, linux, jdelvare,
	linux-rtc, kernel

On 05/03/2018 at 10:43:53 +0000, Denis OSTERLAND wrote:
> From: Denis Osterland <Denis.Osterland@diehl.com>
> 
> Fix possible race condition.
> It is not allowed to return with an error code after RTC is registered.
> 
> Suggested-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> Signed-off-by: Denis Osterland <Denis.Osterland@diehl.com>
> Reviewed-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> ---
>  drivers/rtc/rtc-isl1208.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
Applied, thanks.

-- 
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 3/4] rtc: isl1208: add support for isl1219 with tamper detection
  2018-03-05 10:43 ` [PATCH v3 3/4] rtc: isl1208: add support for isl1219 with tamper detection Denis OSTERLAND
@ 2018-03-06 20:42   ` Alexandre Belloni
  2018-03-07  8:19       ` Denis OSTERLAND
  2018-03-07 22:02   ` Rob Herring
  1 sibling, 1 reply; 15+ messages in thread
From: Alexandre Belloni @ 2018-03-06 20:42 UTC (permalink / raw)
  To: Denis OSTERLAND
  Cc: a.zummo, linux-kernel, mgr, m.grzeschik, devicetree, linux,
	jdelvare, linux-rtc, kernel

On 05/03/2018 at 10:43:52 +0000, Denis OSTERLAND wrote:
> diff --git a/Documentation/devicetree/bindings/rtc/isil,isl1219.txt b/Documentation/devicetree/bindings/rtc/isil,isl1219.txt
> new file mode 100644
> index 0000000..7937c13
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/isil,isl1219.txt

If you want that file to be reviewed by Rob (DT maintainer), you should
probably separate it from that patch and copy his email. The bindings
seem fine to me though.

> diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c
> index 1a2c38c..164371b 100644
> --- a/drivers/rtc/rtc-isl1208.c
> +++ b/drivers/rtc/rtc-isl1208.c
> @@ -33,6 +33,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 +58,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
> +

Because they are not available on ISL1208, maybe it would be better to
prefix them with ISL1219.

> +
> +	tv64.tv_sec = rtc_tm_to_time64(&tm);

Why not using an unsigned long long directly here? time64_t is not the
correct type.

> +
> +	return sprintf(buf, "%lld\n", (long long) tv64.tv_sec);

And this should become %llu

> +};
> +
> +static DEVICE_ATTR(timestamp0, 0640,

Shouldn't the permissions be 644?

> +		isl1208_rtc_event_show_timestamp, isl1208_rtc_event_clear);
> +
>  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 +609,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 +626,13 @@ isl1208_rtc_interrupt(int irq, void *data)
>  			return err;
>  	}
> 
> +	if (sr & ISL1208_REG_SR_EVT) {
> +		sysfs_notify(&client->dev.kobj, NULL,
> +			dev_attr_timestamp0.attr.name);
> +		dev_warn(&client->dev, "event detected");
> +		handled = 1;
> +	}
> +
>  	return handled ? IRQ_HANDLED : IRQ_NONE;
>  }
> 
> @@ -623,11 +718,23 @@ static const struct attribute_group isl1208_rtc_sysfs_files = {
>  	.attrs	= isl1208_rtc_attrs,
>  };
> 
> +static struct attribute *isl1219_rtc_attrs[] = {
> +	&dev_attr_atrim.attr,
> +	&dev_attr_dtrim.attr,
> +	&dev_attr_usr.attr,
> +	&dev_attr_timestamp0.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group isl1219_rtc_sysfs_files = {
> +	.attrs	= isl1219_rtc_attrs,
> +};
> +
>  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 +742,18 @@ isl1208_probe(struct i2c_client *client, const struct i2c_device_id *id)
>  	if (isl1208_i2c_validate_client(client) < 0)
>  		return -ENODEV;
> 
> -	rtc = devm_rtc_allocate_device(&client->dev);
> -	if (IS_ERR(rtc))
> -		return PTR_ERR(rtc);
> +	isl1208 = devm_kzalloc(&client->dev, sizeof(struct isl1208),
> +				GFP_KERNEL);
> +	if (!isl1208)
> +		return -ENOMEM;
> 
> -	rtc->ops = &isl1208_rtc_ops;
> +	isl1208->rtc = devm_rtc_allocate_device(&client->dev);
> +	if (IS_ERR(isl1208->rtc))
> +		return PTR_ERR(isl1208->rtc);
> 
> -	i2c_set_clientdata(client, rtc);
> +	isl1208->rtc->ops = &isl1208_rtc_ops;
> +
> +	i2c_set_clientdata(client, isl1208);
> 
>  	rc = isl1208_i2c_get_sr(client);
>  	if (rc < 0) {
> @@ -653,7 +765,18 @@ isl1208_probe(struct i2c_client *client, const struct i2c_device_id *id)
>  		dev_warn(&client->dev, "rtc power failure detected, "
>  			 "please set clock.\n");
> 
> -	rc = sysfs_create_group(&client->dev.kobj, &isl1208_rtc_sysfs_files);
> +	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->sysfs_files = &isl1219_rtc_sysfs_files;
> +	} else {
> +		isl1208->sysfs_files = &isl1208_rtc_sysfs_files;
> +	}
> +

I don't think the whole isl1208 is necessary. You should probably use
the .is_visible callback of isl1219_rtc_sysfs_files. This will make the
changelog quite smaller.

> +	rc = sysfs_create_group(&client->dev.kobj, isl1208->sysfs_files);
>  	if (rc)
>  		return rc;
> 
> @@ -674,20 +797,23 @@ isl1208_probe(struct i2c_client *client, const struct i2c_device_id *id)
>  		}
>  	}
> 
> -	return rtc_register_device(rtc);
> +	return rtc_register_device(isl1208->rtc);
>  }
> 
>  static int
>  isl1208_remove(struct i2c_client *client)
>  {
> -	sysfs_remove_group(&client->dev.kobj, &isl1208_rtc_sysfs_files);
> +	struct isl1208 *isl1208 = i2c_get_clientdata(client);
> +
> +	sysfs_remove_group(&client->dev.kobj, isl1208->sysfs_files);
> 
>  	return 0;
>  }
> 
>  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);
> --
> 2.7.4
> 
> 
> Diehl AKO Stiftung & Co. KG, Pfannerstraße 75-83, 88239 Wangen im Allgäu
> Bereichsvorstand: Dr.-Ing. Michael Siedentop (Sprecher), Josef Fellner (Mitglied)
> Sitz der Gesellschaft: Wangen i.A. – Registergericht: Amtsgericht Ulm HRA 620609 – Persönlich haftende Gesellschafterin: Diehl Verwaltungs-Stiftung – Sitz: Nürnberg – Registergericht: Amtsgericht Nürnberg HRA 11756 –
> Vorstand: Dr.-Ing. E.h. Thomas Diehl (†) (Vorsitzender), Herr Dipl.-Wirtsch.-Ing. Wolfgang Weggen (stellvertretender Vorsitzender), Dipl.-Kfm. Claus Günther, Dipl.-Kfm. Frank Gutzeit, Dr.-Ing. Heinrich Schunk, Dr.-Ing. Michael Siedentop , Dipl.-Kfm. Dr.-Ing. Martin Sommer, Dipl.-Ing. (FH) Rainer von Borstel, Vorsitzender des Aufsichtsrates: Dr. Klaus Maier
> ___________________________________________________________________________________________________
> Der Inhalt der vorstehenden E-Mail ist nicht rechtlich bindend. Diese E-Mail enthaelt vertrauliche und/oder rechtlich geschuetzte Informationen.
> Informieren Sie uns bitte, wenn Sie diese E-Mail faelschlicherweise erhalten haben. Bitte loeschen Sie in diesem Fall die Nachricht. Jede unerlaubte Form der Reproduktion, Bekanntgabe, Aenderung, Verteilung und/oder Publikation dieser E-Mail ist strengstens untersagt.
> The contents of the above mentioned e-mail is not legally binding. This e-mail contains confidential and/or legally protected information. Please inform us if you have received this e-mail by mistake and delete it in such a case. Each unauthorized reproduction, disclosure, alteration, distribution and/or publication of this e-mail is strictly prohibited.

-- 
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 3/4] rtc: isl1208: add support for isl1219 with tamper detection
  2018-03-06 20:42   ` Alexandre Belloni
@ 2018-03-07  8:19       ` Denis OSTERLAND
  0 siblings, 0 replies; 15+ messages in thread
From: Denis OSTERLAND @ 2018-03-07  8:19 UTC (permalink / raw)
  To: alexandre.belloni
  Cc: linux-kernel, mgr, m.grzeschik, devicetree, a.zummo, linux,
	jdelvare, linux-rtc, kernel

Am Dienstag, den 06.03.2018, 21:42 +0100 schrieb Alexandre Belloni:
> On 05/03/2018 at 10:43:52 +0000, Denis OSTERLAND wrote:
> > 
> > diff --git a/Documentation/devicetree/bindings/rtc/isil,isl1219.txt b/Documentation/devicetree/bindings/rtc/isil,isl1219.txt
> > new file mode 100644
> > index 0000000..7937c13
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/rtc/isil,isl1219.txt
> If you want that file to be reviewed by Rob (DT maintainer), you should
> probably separate it from that patch and copy his email. The bindings
> seem fine to me though.
OK
> 
> > 
> > diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c
> > index 1a2c38c..164371b 100644
> > --- a/drivers/rtc/rtc-isl1208.c
> > +++ b/drivers/rtc/rtc-isl1208.c
> > @@ -33,6 +33,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 +58,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
> > +
> Because they are not available on ISL1208, maybe it would be better to
> prefix them with ISL1219.
I see. Yes, this would clarify that they are only available on isl1219.
Shall we rename isl1208_rtc_event_show_timestamp/isl1208_rtc_event_clear
to isl1219_rtc_event_show_timestamp/isl1219_rtc_event_clear, too?
> 
> > 
> > +
> > +	tv64.tv_sec = rtc_tm_to_time64(&tm);
> Why not using an unsigned long long directly here? time64_t is not the
> correct type.
Do you mean timespec64 is not the correct type here?
Then yes, sould be time64_t.
If you mean time64_t is not the correct type here,
then can you give me some detail why there is no rtc_tm_to_u64,
or something like that?
sprintf(buf, "%lld\n", rtc_tm_to_time64(&tm)) seems correct to me.
By the way, is it needed to check for seconds < 0 and return error?
> 
> > 
> > +
> > +	return sprintf(buf, "%lld\n", (long long) tv64.tv_sec);
> And this should become %llu
> 
> > 
> > +};
> > +
> > +static DEVICE_ATTR(timestamp0, 0640,
> Shouldn't the permissions be 644?
644 is OK
> 
> > 
> > +		isl1208_rtc_event_show_timestamp, isl1208_rtc_event_clear);
> > +
> >  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 +609,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 +626,13 @@ isl1208_rtc_interrupt(int irq, void *data)
> >  			return err;
> >  	}
> > 
> > +	if (sr & ISL1208_REG_SR_EVT) {
> > +		sysfs_notify(&client->dev.kobj, NULL,
> > +			dev_attr_timestamp0.attr.name);
> > +		dev_warn(&client->dev, "event detected");
> > +		handled = 1;
> > +	}
> > +
> >  	return handled ? IRQ_HANDLED : IRQ_NONE;
> >  }
> > 
> > @@ -623,11 +718,23 @@ static const struct attribute_group isl1208_rtc_sysfs_files = {
> >  	.attrs	= isl1208_rtc_attrs,
> >  };
> > 
> > +static struct attribute *isl1219_rtc_attrs[] = {
> > +	&dev_attr_atrim.attr,
> > +	&dev_attr_dtrim.attr,
> > +	&dev_attr_usr.attr,
> > +	&dev_attr_timestamp0.attr,
> > +	NULL
> > +};
> > +
> > +static const struct attribute_group isl1219_rtc_sysfs_files = {
> > +	.attrs	= isl1219_rtc_attrs,
> > +};
> > +
> >  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 +742,18 @@ isl1208_probe(struct i2c_client *client, const struct i2c_device_id *id)
> >  	if (isl1208_i2c_validate_client(client) < 0)
> >  		return -ENODEV;
> > 
> > -	rtc = devm_rtc_allocate_device(&client->dev);
> > -	if (IS_ERR(rtc))
> > -		return PTR_ERR(rtc);
> > +	isl1208 = devm_kzalloc(&client->dev, sizeof(struct isl1208),
> > +				GFP_KERNEL);
> > +	if (!isl1208)
> > +		return -ENOMEM;
> > 
> > -	rtc->ops = &isl1208_rtc_ops;
> > +	isl1208->rtc = devm_rtc_allocate_device(&client->dev);
> > +	if (IS_ERR(isl1208->rtc))
> > +		return PTR_ERR(isl1208->rtc);
> > 
> > -	i2c_set_clientdata(client, rtc);
> > +	isl1208->rtc->ops = &isl1208_rtc_ops;
> > +
> > +	i2c_set_clientdata(client, isl1208);
> > 
> >  	rc = isl1208_i2c_get_sr(client);
> >  	if (rc < 0) {
> > @@ -653,7 +765,18 @@ isl1208_probe(struct i2c_client *client, const struct i2c_device_id *id)
> >  		dev_warn(&client->dev, "rtc power failure detected, "
> >  			 "please set clock.\n");
> > 
> > -	rc = sysfs_create_group(&client->dev.kobj, &isl1208_rtc_sysfs_files);
> > +	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->sysfs_files = &isl1219_rtc_sysfs_files;
> > +	} else {
> > +		isl1208->sysfs_files = &isl1208_rtc_sysfs_files;
> > +	}
> > +
> I don't think the whole isl1208 is necessary. You should probably use
> the .is_visible callback of isl1219_rtc_sysfs_files. This will make the
> changelog quite smaller.
> 
Well, I don´t know how to access i2c_device_id from kobject.
rtc_attr_is_visible shows how to convert kobject to device and rtc_device,
but how to do (id->driver_data == TYPE_ISL1219) here?
> > 
> > +	rc = sysfs_create_group(&client->dev.kobj, isl1208->sysfs_files);
> >  	if (rc)
> >  		return rc;
> > 
> > @@ -674,20 +797,23 @@ isl1208_probe(struct i2c_client *client, const struct i2c_device_id *id)
> >  		}
> >  	}
> > 
> > -	return rtc_register_device(rtc);
> > +	return rtc_register_device(isl1208->rtc);
> >  }
> > 
> >  static int
> >  isl1208_remove(struct i2c_client *client)
> >  {
> > -	sysfs_remove_group(&client->dev.kobj, &isl1208_rtc_sysfs_files);
> > +	struct isl1208 *isl1208 = i2c_get_clientdata(client);
> > +
> > +	sysfs_remove_group(&client->dev.kobj, isl1208->sysfs_files);
> > 
> >  	return 0;
> >  }
> > 
> >  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);
> > --
> > 2.7.4
> > 
> > 
> > Diehl AKO Stiftung & Co. KG, Pfannerstraße 75-83, 88239 Wangen im Allgäu
> > Bereichsvorstand: Dr.-Ing. Michael Siedentop (Sprecher), Josef Fellner (Mitglied)
> > Sitz der Gesellschaft: Wangen i.A. – Registergericht: Amtsgericht Ulm HRA 620609 – Persönlich haftende Gesellschafterin: Diehl Verwaltungs-Stiftung – Sitz: Nürnberg – Registergericht: Amtsgericht
> > Nürnberg HRA 11756 –
> > Vorstand: Dr.-Ing. E.h. Thomas Diehl (†) (Vorsitzender), Herr Dipl.-Wirtsch.-Ing. Wolfgang Weggen (stellvertretender Vorsitzender), Dipl.-Kfm. Claus Günther, Dipl.-Kfm. Frank Gutzeit, Dr.-Ing.
> > Heinrich Schunk, Dr.-Ing. Michael Siedentop , Dipl.-Kfm. Dr.-Ing. Martin Sommer, Dipl.-Ing. (FH) Rainer von Borstel, Vorsitzender des Aufsichtsrates: Dr. Klaus Maier
> > ___________________________________________________________________________________________________
> > Der Inhalt der vorstehenden E-Mail ist nicht rechtlich bindend. Diese E-Mail enthaelt vertrauliche und/oder rechtlich geschuetzte Informationen.
> > Informieren Sie uns bitte, wenn Sie diese E-Mail faelschlicherweise erhalten haben. Bitte loeschen Sie in diesem Fall die Nachricht. Jede unerlaubte Form der Reproduktion, Bekanntgabe, Aenderung,
> > Verteilung und/oder Publikation dieser E-Mail ist strengstens untersagt.
> > The contents of the above mentioned e-mail is not legally binding. This e-mail contains confidential and/or legally protected information. Please inform us if you have received this e-mail by
> > mistake and delete it in such a case. Each unauthorized reproduction, disclosure, alteration, distribution and/or publication of this e-mail is strictly prohibited.
Diehl AKO Stiftung & Co. KG, Pfannerstraße 75-83, 88239 Wangen im Allgäu
Bereichsvorstand: Dr.-Ing. Michael Siedentop (Sprecher), Josef Fellner (Mitglied)
Sitz der Gesellschaft: Wangen i.A. – Registergericht: Amtsgericht Ulm HRA 620609 – Persönlich haftende Gesellschafterin: Diehl Verwaltungs-Stiftung – Sitz: Nürnberg – Registergericht: Amtsgericht Nürnberg HRA 11756 –
Vorstand: Dr.-Ing. E.h. Thomas Diehl (†) (Vorsitzender), Herr Dipl.-Wirtsch.-Ing. Wolfgang Weggen (stellvertretender Vorsitzender), Dipl.-Kfm. Claus Günther, Dipl.-Kfm. Frank Gutzeit, Dr.-Ing. Heinrich Schunk, Dr.-Ing. Michael Siedentop , Dipl.-Kfm. Dr.-Ing. Martin Sommer, Dipl.-Ing. (FH) Rainer von Borstel, Vorsitzender des Aufsichtsrates: Dr. Klaus Maier
___________________________________________________________________________________________________
Der Inhalt der vorstehenden E-Mail ist nicht rechtlich bindend. Diese E-Mail enthaelt vertrauliche und/oder rechtlich geschuetzte Informationen.
Informieren Sie uns bitte, wenn Sie diese E-Mail faelschlicherweise erhalten haben. Bitte loeschen Sie in diesem Fall die Nachricht. Jede unerlaubte Form der Reproduktion, Bekanntgabe, Aenderung, Verteilung und/oder Publikation dieser E-Mail ist strengstens untersagt.
The contents of the above mentioned e-mail is not legally binding. This e-mail contains confidential and/or legally protected information. Please inform us if you have received this e-mail by mistake and delete it in such a case. Each unauthorized reproduction, disclosure, alteration, distribution and/or publication of this e-mail is strictly prohibited.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 3/4] rtc: isl1208: add support for isl1219 with tamper detection
@ 2018-03-07  8:19       ` Denis OSTERLAND
  0 siblings, 0 replies; 15+ messages in thread
From: Denis OSTERLAND @ 2018-03-07  8:19 UTC (permalink / raw)
  To: alexandre.belloni
  Cc: linux-kernel, mgr, m.grzeschik, devicetree, a.zummo, linux,
	jdelvare, linux-rtc, kernel

QW0gRGllbnN0YWcsIGRlbiAwNi4wMy4yMDE4LCAyMTo0MiArMDEwMCBzY2hyaWViIEFsZXhh
bmRyZSBCZWxsb25pOg0KPiBPbiAwNS8wMy8yMDE4IGF0IDEwOjQzOjUyICswMDAwLCBEZW5p
cyBPU1RFUkxBTkQgd3JvdGU6DQo+ID4gDQo+ID4gZGlmZiAtLWdpdCBhL0RvY3VtZW50YXRp
b24vZGV2aWNldHJlZS9iaW5kaW5ncy9ydGMvaXNpbCxpc2wxMjE5LnR4dCBiL0RvY3VtZW50
YXRpb24vZGV2aWNldHJlZS9iaW5kaW5ncy9ydGMvaXNpbCxpc2wxMjE5LnR4dA0KPiA+IG5l
dyBmaWxlIG1vZGUgMTAwNjQ0DQo+ID4gaW5kZXggMDAwMDAwMC4uNzkzN2MxMw0KPiA+IC0t
LSAvZGV2L251bGwNCj4gPiArKysgYi9Eb2N1bWVudGF0aW9uL2RldmljZXRyZWUvYmluZGlu
Z3MvcnRjL2lzaWwsaXNsMTIxOS50eHQNCj4gSWYgeW91IHdhbnQgdGhhdCBmaWxlIHRvIGJl
IHJldmlld2VkIGJ5IFJvYiAoRFQgbWFpbnRhaW5lciksIHlvdSBzaG91bGQNCj4gcHJvYmFi
bHkgc2VwYXJhdGUgaXQgZnJvbSB0aGF0IHBhdGNoIGFuZCBjb3B5IGhpcyBlbWFpbC4gVGhl
IGJpbmRpbmdzDQo+IHNlZW0gZmluZSB0byBtZSB0aG91Z2guDQpPSw0KPiANCj4gPiANCj4g
PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ydGMvcnRjLWlzbDEyMDguYyBiL2RyaXZlcnMvcnRj
L3J0Yy1pc2wxMjA4LmMNCj4gPiBpbmRleCAxYTJjMzhjLi4xNjQzNzFiIDEwMDY0NA0KPiA+
IC0tLSBhL2RyaXZlcnMvcnRjL3J0Yy1pc2wxMjA4LmMNCj4gPiArKysgYi9kcml2ZXJzL3J0
Yy9ydGMtaXNsMTIwOC5jDQo+ID4gQEAgLTMzLDYgKzMzLDcgQEANCj4gPiDCoCNkZWZpbmUg
SVNMMTIwOF9SRUdfU1JfQVJTVMKgwqDCoMKgKDE8PDcpCS8qIGF1dG8gcmVzZXQgKi8NCj4g
PiDCoCNkZWZpbmUgSVNMMTIwOF9SRUdfU1JfWFRPU0NCwqDCoCgxPDw2KQkvKiBjcnlzdGFs
IG9zY2lsbGF0b3IgKi8NCj4gPiDCoCNkZWZpbmUgSVNMMTIwOF9SRUdfU1JfV1JUQ8KgwqDC
oMKgKDE8PDQpCS8qIHdyaXRlIHJ0YyAqLw0KPiA+ICsjZGVmaW5lIElTTDEyMDhfUkVHX1NS
X0VWVMKgwqDCoMKgwqAoMTw8MykJLyogZXZlbnQgKi8NCj4gPiDCoCNkZWZpbmUgSVNMMTIw
OF9SRUdfU1JfQUxNwqDCoMKgwqDCoCgxPDwyKQkvKiBhbGFybSAqLw0KPiA+IMKgI2RlZmlu
ZSBJU0wxMjA4X1JFR19TUl9CQVTCoMKgwqDCoMKgKDE8PDEpCS8qIGJhdHRlcnkgKi8NCj4g
PiDCoCNkZWZpbmUgSVNMMTIwOF9SRUdfU1JfUlRDRsKgwqDCoMKgKDE8PDApCS8qIHJ0YyBm
YWlsICovDQo+ID4gQEAgLTU3LDggKzU4LDI5IEBADQo+ID4gwqAjZGVmaW5lIElTTDEyMDhf
UkVHX1VTUjIgMHgxMw0KPiA+IMKgI2RlZmluZSBJU0wxMjA4X1VTUl9TRUNUSU9OX0xFTiAy
DQo+ID4gDQo+ID4gKy8qIGV2ZW50IHNlY3Rpb24gKi8NCj4gPiArI2RlZmluZSBJU0wxMjA4
X1JFR19TQ1QgMHgxNA0KPiA+ICsjZGVmaW5lIElTTDEyMDhfUkVHX01OVCAweDE1DQo+ID4g
KyNkZWZpbmUgSVNMMTIwOF9SRUdfSFJUIDB4MTYNCj4gPiArI2RlZmluZSBJU0wxMjA4X1JF
R19EVFQgMHgxNw0KPiA+ICsjZGVmaW5lIElTTDEyMDhfUkVHX01PVCAweDE4DQo+ID4gKyNk
ZWZpbmUgSVNMMTIwOF9SRUdfWVJUIDB4MTkNCj4gPiArI2RlZmluZSBJU0wxMjA4X0VWVF9T
RUNUSU9OX0xFTiA2DQo+ID4gKw0KPiBCZWNhdXNlIHRoZXkgYXJlIG5vdCBhdmFpbGFibGUg
b24gSVNMMTIwOCwgbWF5YmUgaXQgd291bGQgYmUgYmV0dGVyIHRvDQo+IHByZWZpeCB0aGVt
IHdpdGggSVNMMTIxOS4NCkkgc2VlLiBZZXMsIHRoaXMgd291bGQgY2xhcmlmeSB0aGF0IHRo
ZXkgYXJlIG9ubHkgYXZhaWxhYmxlIG9uIGlzbDEyMTkuDQpTaGFsbCB3ZSByZW5hbWUgaXNs
MTIwOF9ydGNfZXZlbnRfc2hvd190aW1lc3RhbXAvaXNsMTIwOF9ydGNfZXZlbnRfY2xlYXIN
CnRvIGlzbDEyMTlfcnRjX2V2ZW50X3Nob3dfdGltZXN0YW1wL2lzbDEyMTlfcnRjX2V2ZW50
X2NsZWFyLCB0b28/DQo+IA0KPiA+IA0KPiA+ICsNCj4gPiArCXR2NjQudHZfc2VjID0gcnRj
X3RtX3RvX3RpbWU2NCgmdG0pOw0KPiBXaHkgbm90IHVzaW5nIGFuIHVuc2lnbmVkIGxvbmcg
bG9uZyBkaXJlY3RseSBoZXJlPyB0aW1lNjRfdCBpcyBub3QgdGhlDQo+IGNvcnJlY3QgdHlw
ZS4NCkRvIHlvdSBtZWFuIHRpbWVzcGVjNjQgaXMgbm90IHRoZSBjb3JyZWN0IHR5cGUgaGVy
ZT8NClRoZW4geWVzLCBzb3VsZCBiZSB0aW1lNjRfdC4NCklmIHlvdSBtZWFuIHRpbWU2NF90
IGlzIG5vdCB0aGUgY29ycmVjdCB0eXBlIGhlcmUsDQp0aGVuIGNhbiB5b3UgZ2l2ZSBtZSBz
b21lIGRldGFpbCB3aHkgdGhlcmUgaXMgbm8gcnRjX3RtX3RvX3U2NCwNCm9yIHNvbWV0aGlu
ZyBsaWtlIHRoYXQ/DQpzcHJpbnRmKGJ1ZiwgIiVsbGRcbiIswqBydGNfdG1fdG9fdGltZTY0
KCZ0bSkpIHNlZW1zIGNvcnJlY3QgdG8gbWUuDQpCeSB0aGUgd2F5LCBpcyBpdCBuZWVkZWQg
dG8gY2hlY2sgZm9yIHNlY29uZHMgPCAwIGFuZCByZXR1cm4gZXJyb3I/DQo+IA0KPiA+IA0K
PiA+ICsNCj4gPiArCXJldHVybiBzcHJpbnRmKGJ1ZiwgIiVsbGRcbiIsIChsb25nIGxvbmcp
IHR2NjQudHZfc2VjKTsNCj4gQW5kIHRoaXMgc2hvdWxkIGJlY29tZSAlbGx1DQo+IA0KPiA+
IA0KPiA+ICt9Ow0KPiA+ICsNCj4gPiArc3RhdGljIERFVklDRV9BVFRSKHRpbWVzdGFtcDAs
IDA2NDAsDQo+IFNob3VsZG4ndCB0aGUgcGVybWlzc2lvbnMgYmUgNjQ0Pw0KNjQ0IGlzIE9L
DQo+IA0KPiA+IA0KPiA+ICsJCWlzbDEyMDhfcnRjX2V2ZW50X3Nob3dfdGltZXN0YW1wLCBp
c2wxMjA4X3J0Y19ldmVudF9jbGVhcik7DQo+ID4gKw0KPiA+IMKgc3RhdGljIGlycXJldHVy
bl90DQo+ID4gwqBpc2wxMjA4X3J0Y19pbnRlcnJ1cHQoaW50IGlycSwgdm9pZCAqZGF0YSkN
Cj4gPiDCoHsNCj4gPiDCoAl1bnNpZ25lZCBsb25nIHRpbWVvdXQgPSBqaWZmaWVzICsgbXNl
Y3NfdG9famlmZmllcygxMDAwKTsNCj4gPiDCoAlzdHJ1Y3QgaTJjX2NsaWVudCAqY2xpZW50
ID0gZGF0YTsNCj4gPiAtCXN0cnVjdCBydGNfZGV2aWNlICpydGMgPSBpMmNfZ2V0X2NsaWVu
dGRhdGEoY2xpZW50KTsNCj4gPiArCXN0cnVjdCBpc2wxMjA4ICppc2wxMjA4ID0gaTJjX2dl
dF9jbGllbnRkYXRhKGNsaWVudCk7DQo+ID4gwqAJaW50IGhhbmRsZWQgPSAwLCBzciwgZXJy
Ow0KPiA+IA0KPiA+IMKgCS8qDQo+ID4gQEAgLTUyMSw3ICs2MDksNyBAQCBpc2wxMjA4X3J0
Y19pbnRlcnJ1cHQoaW50IGlycSwgdm9pZCAqZGF0YSkNCj4gPiDCoAlpZiAoc3IgJiBJU0wx
MjA4X1JFR19TUl9BTE0pIHsNCj4gPiDCoAkJZGV2X2RiZygmY2xpZW50LT5kZXYsICJhbGFy
bSFcbiIpOw0KPiA+IA0KPiA+IC0JCXJ0Y191cGRhdGVfaXJxKHJ0YywgMSwgUlRDX0lSUUYg
fCBSVENfQUYpOw0KPiA+ICsJCXJ0Y191cGRhdGVfaXJxKGlzbDEyMDgtPnJ0YywgMSwgUlRD
X0lSUUYgfCBSVENfQUYpOw0KPiA+IA0KPiA+IMKgCQkvKiBDbGVhciB0aGUgYWxhcm0gKi8N
Cj4gPiDCoAkJc3IgJj0gfklTTDEyMDhfUkVHX1NSX0FMTTsNCj4gPiBAQCAtNTM4LDYgKzYy
NiwxMyBAQCBpc2wxMjA4X3J0Y19pbnRlcnJ1cHQoaW50IGlycSwgdm9pZCAqZGF0YSkNCj4g
PiDCoAkJCXJldHVybiBlcnI7DQo+ID4gwqAJfQ0KPiA+IA0KPiA+ICsJaWYgKHNyICYgSVNM
MTIwOF9SRUdfU1JfRVZUKSB7DQo+ID4gKwkJc3lzZnNfbm90aWZ5KCZjbGllbnQtPmRldi5r
b2JqLCBOVUxMLA0KPiA+ICsJCQlkZXZfYXR0cl90aW1lc3RhbXAwLmF0dHIubmFtZSk7DQo+
ID4gKwkJZGV2X3dhcm4oJmNsaWVudC0+ZGV2LCAiZXZlbnQgZGV0ZWN0ZWQiKTsNCj4gPiAr
CQloYW5kbGVkID0gMTsNCj4gPiArCX0NCj4gPiArDQo+ID4gwqAJcmV0dXJuIGhhbmRsZWQg
PyBJUlFfSEFORExFRCA6IElSUV9OT05FOw0KPiA+IMKgfQ0KPiA+IA0KPiA+IEBAIC02MjMs
MTEgKzcxOCwyMyBAQCBzdGF0aWMgY29uc3Qgc3RydWN0IGF0dHJpYnV0ZV9ncm91cCBpc2wx
MjA4X3J0Y19zeXNmc19maWxlcyA9IHsNCj4gPiDCoAkuYXR0cnMJPSBpc2wxMjA4X3J0Y19h
dHRycywNCj4gPiDCoH07DQo+ID4gDQo+ID4gK3N0YXRpYyBzdHJ1Y3QgYXR0cmlidXRlICpp
c2wxMjE5X3J0Y19hdHRyc1tdID0gew0KPiA+ICsJJmRldl9hdHRyX2F0cmltLmF0dHIsDQo+
ID4gKwkmZGV2X2F0dHJfZHRyaW0uYXR0ciwNCj4gPiArCSZkZXZfYXR0cl91c3IuYXR0ciwN
Cj4gPiArCSZkZXZfYXR0cl90aW1lc3RhbXAwLmF0dHIsDQo+ID4gKwlOVUxMDQo+ID4gK307
DQo+ID4gKw0KPiA+ICtzdGF0aWMgY29uc3Qgc3RydWN0IGF0dHJpYnV0ZV9ncm91cCBpc2wx
MjE5X3J0Y19zeXNmc19maWxlcyA9IHsNCj4gPiArCS5hdHRycwk9IGlzbDEyMTlfcnRjX2F0
dHJzLA0KPiA+ICt9Ow0KPiA+ICsNCj4gPiDCoHN0YXRpYyBpbnQNCj4gPiDCoGlzbDEyMDhf
cHJvYmUoc3RydWN0IGkyY19jbGllbnQgKmNsaWVudCwgY29uc3Qgc3RydWN0IGkyY19kZXZp
Y2VfaWQgKmlkKQ0KPiA+IMKgew0KPiA+IMKgCWludCByYyA9IDA7DQo+ID4gLQlzdHJ1Y3Qg
cnRjX2RldmljZSAqcnRjOw0KPiA+ICsJc3RydWN0IGlzbDEyMDggKmlzbDEyMDg7DQo+ID4g
DQo+ID4gwqAJaWYgKCFpMmNfY2hlY2tfZnVuY3Rpb25hbGl0eShjbGllbnQtPmFkYXB0ZXIs
IEkyQ19GVU5DX0kyQykpDQo+ID4gwqAJCXJldHVybiAtRU5PREVWOw0KPiA+IEBAIC02MzUs
MTMgKzc0MiwxOCBAQCBpc2wxMjA4X3Byb2JlKHN0cnVjdCBpMmNfY2xpZW50ICpjbGllbnQs
IGNvbnN0IHN0cnVjdCBpMmNfZGV2aWNlX2lkICppZCkNCj4gPiDCoAlpZiAoaXNsMTIwOF9p
MmNfdmFsaWRhdGVfY2xpZW50KGNsaWVudCkgPCAwKQ0KPiA+IMKgCQlyZXR1cm4gLUVOT0RF
VjsNCj4gPiANCj4gPiAtCXJ0YyA9IGRldm1fcnRjX2FsbG9jYXRlX2RldmljZSgmY2xpZW50
LT5kZXYpOw0KPiA+IC0JaWYgKElTX0VSUihydGMpKQ0KPiA+IC0JCXJldHVybiBQVFJfRVJS
KHJ0Yyk7DQo+ID4gKwlpc2wxMjA4ID0gZGV2bV9remFsbG9jKCZjbGllbnQtPmRldiwgc2l6
ZW9mKHN0cnVjdCBpc2wxMjA4KSwNCj4gPiArCQkJCUdGUF9LRVJORUwpOw0KPiA+ICsJaWYg
KCFpc2wxMjA4KQ0KPiA+ICsJCXJldHVybiAtRU5PTUVNOw0KPiA+IA0KPiA+IC0JcnRjLT5v
cHMgPSAmaXNsMTIwOF9ydGNfb3BzOw0KPiA+ICsJaXNsMTIwOC0+cnRjID0gZGV2bV9ydGNf
YWxsb2NhdGVfZGV2aWNlKCZjbGllbnQtPmRldik7DQo+ID4gKwlpZiAoSVNfRVJSKGlzbDEy
MDgtPnJ0YykpDQo+ID4gKwkJcmV0dXJuIFBUUl9FUlIoaXNsMTIwOC0+cnRjKTsNCj4gPiAN
Cj4gPiAtCWkyY19zZXRfY2xpZW50ZGF0YShjbGllbnQsIHJ0Yyk7DQo+ID4gKwlpc2wxMjA4
LT5ydGMtPm9wcyA9ICZpc2wxMjA4X3J0Y19vcHM7DQo+ID4gKw0KPiA+ICsJaTJjX3NldF9j
bGllbnRkYXRhKGNsaWVudCwgaXNsMTIwOCk7DQo+ID4gDQo+ID4gwqAJcmMgPSBpc2wxMjA4
X2kyY19nZXRfc3IoY2xpZW50KTsNCj4gPiDCoAlpZiAocmMgPCAwKSB7DQo+ID4gQEAgLTY1
Myw3ICs3NjUsMTggQEAgaXNsMTIwOF9wcm9iZShzdHJ1Y3QgaTJjX2NsaWVudCAqY2xpZW50
LCBjb25zdCBzdHJ1Y3QgaTJjX2RldmljZV9pZCAqaWQpDQo+ID4gwqAJCWRldl93YXJuKCZj
bGllbnQtPmRldiwgInJ0YyBwb3dlciBmYWlsdXJlIGRldGVjdGVkLCAiDQo+ID4gwqAJCQnC
oCJwbGVhc2Ugc2V0IGNsb2NrLlxuIik7DQo+ID4gDQo+ID4gLQlyYyA9IHN5c2ZzX2NyZWF0
ZV9ncm91cCgmY2xpZW50LT5kZXYua29iaiwgJmlzbDEyMDhfcnRjX3N5c2ZzX2ZpbGVzKTsN
Cj4gPiArCWlmIChpZC0+ZHJpdmVyX2RhdGEgPT0gVFlQRV9JU0wxMjE5KSB7DQo+ID4gKwkJ
cmMgPSBpMmNfc21idXNfd3JpdGVfYnl0ZV9kYXRhKGNsaWVudCwgSVNMMTIwOF9SRUdfMDks
IDB4MTApOw0KPiA+ICsJCWlmIChyYyA8IDApIHsNCj4gPiArCQkJZGV2X2VycigmY2xpZW50
LT5kZXYsICJjb3VsZCBub3QgZW5hYmxlIHRhbXBlciBkZXRlY3Rpb25cbiIpOw0KPiA+ICsJ
CQlyZXR1cm4gcmM7DQo+ID4gKwkJfQ0KPiA+ICsJCWlzbDEyMDgtPnN5c2ZzX2ZpbGVzID0g
JmlzbDEyMTlfcnRjX3N5c2ZzX2ZpbGVzOw0KPiA+ICsJfSBlbHNlIHsNCj4gPiArCQlpc2wx
MjA4LT5zeXNmc19maWxlcyA9ICZpc2wxMjA4X3J0Y19zeXNmc19maWxlczsNCj4gPiArCX0N
Cj4gPiArDQo+IEkgZG9uJ3QgdGhpbmsgdGhlIHdob2xlIGlzbDEyMDggaXMgbmVjZXNzYXJ5
LiBZb3Ugc2hvdWxkIHByb2JhYmx5IHVzZQ0KPiB0aGUgLmlzX3Zpc2libGUgY2FsbGJhY2sg
b2YgaXNsMTIxOV9ydGNfc3lzZnNfZmlsZXMuIFRoaXMgd2lsbCBtYWtlIHRoZQ0KPiBjaGFu
Z2Vsb2cgcXVpdGUgc21hbGxlci4NCj4gDQpXZWxsLCBJIGRvbsK0dCBrbm93IGhvdyB0byBh
Y2Nlc3MgaTJjX2RldmljZV9pZCBmcm9tIGtvYmplY3QuDQpydGNfYXR0cl9pc192aXNpYmxl
IHNob3dzIGhvdyB0byBjb252ZXJ0IGtvYmplY3QgdG8gZGV2aWNlIGFuZCBydGNfZGV2aWNl
LA0KYnV0IGhvdyB0byBkb8KgKGlkLT5kcml2ZXJfZGF0YSA9PSBUWVBFX0lTTDEyMTkpIGhl
cmU/DQo+ID4gDQo+ID4gKwlyYyA9IHN5c2ZzX2NyZWF0ZV9ncm91cCgmY2xpZW50LT5kZXYu
a29iaiwgaXNsMTIwOC0+c3lzZnNfZmlsZXMpOw0KPiA+IMKgCWlmIChyYykNCj4gPiDCoAkJ
cmV0dXJuIHJjOw0KPiA+IA0KPiA+IEBAIC02NzQsMjAgKzc5NywyMyBAQCBpc2wxMjA4X3By
b2JlKHN0cnVjdCBpMmNfY2xpZW50ICpjbGllbnQsIGNvbnN0IHN0cnVjdCBpMmNfZGV2aWNl
X2lkICppZCkNCj4gPiDCoAkJfQ0KPiA+IMKgCX0NCj4gPiANCj4gPiAtCXJldHVybiBydGNf
cmVnaXN0ZXJfZGV2aWNlKHJ0Yyk7DQo+ID4gKwlyZXR1cm4gcnRjX3JlZ2lzdGVyX2Rldmlj
ZShpc2wxMjA4LT5ydGMpOw0KPiA+IMKgfQ0KPiA+IA0KPiA+IMKgc3RhdGljIGludA0KPiA+
IMKgaXNsMTIwOF9yZW1vdmUoc3RydWN0IGkyY19jbGllbnQgKmNsaWVudCkNCj4gPiDCoHsN
Cj4gPiAtCXN5c2ZzX3JlbW92ZV9ncm91cCgmY2xpZW50LT5kZXYua29iaiwgJmlzbDEyMDhf
cnRjX3N5c2ZzX2ZpbGVzKTsNCj4gPiArCXN0cnVjdCBpc2wxMjA4ICppc2wxMjA4ID0gaTJj
X2dldF9jbGllbnRkYXRhKGNsaWVudCk7DQo+ID4gKw0KPiA+ICsJc3lzZnNfcmVtb3ZlX2dy
b3VwKCZjbGllbnQtPmRldi5rb2JqLCBpc2wxMjA4LT5zeXNmc19maWxlcyk7DQo+ID4gDQo+
ID4gwqAJcmV0dXJuIDA7DQo+ID4gwqB9DQo+ID4gDQo+ID4gwqBzdGF0aWMgY29uc3Qgc3Ry
dWN0IGkyY19kZXZpY2VfaWQgaXNsMTIwOF9pZFtdID0gew0KPiA+IC0JeyAiaXNsMTIwOCIs
IDAgfSwNCj4gPiAtCXsgImlzbDEyMTgiLCAwIH0sDQo+ID4gKwl7ICJpc2wxMjA4IiwgVFlQ
RV9JU0wxMjA4IH0sDQo+ID4gKwl7ICJpc2wxMjE4IiwgVFlQRV9JU0wxMjE4IH0sDQo+ID4g
Kwl7ICJpc2wxMjE5IiwgVFlQRV9JU0wxMjE5IH0sDQo+ID4gwqAJeyB9DQo+ID4gwqB9Ow0K
PiA+IMKgTU9EVUxFX0RFVklDRV9UQUJMRShpMmMsIGlzbDEyMDhfaWQpOw0KPiA+IC0tDQo+
ID4gMi43LjQNCj4gPiANCj4gPiANCj4gPiBEaWVobCBBS08gU3RpZnR1bmcgJiBDby4gS0cs
IFBmYW5uZXJzdHJhw59lIDc1LTgzLCA4ODIzOSBXYW5nZW4gaW0gQWxsZ8OkdQ0KPiA+IEJl
cmVpY2hzdm9yc3RhbmQ6IERyLi1JbmcuIE1pY2hhZWwgU2llZGVudG9wIChTcHJlY2hlciks
IEpvc2VmIEZlbGxuZXIgKE1pdGdsaWVkKQ0KPiA+IFNpdHogZGVyIEdlc2VsbHNjaGFmdDog
V2FuZ2VuIGkuQS4g4oCTIFJlZ2lzdGVyZ2VyaWNodDogQW10c2dlcmljaHQgVWxtIEhSQSA2
MjA2MDkg4oCTIFBlcnPDtm5saWNoIGhhZnRlbmRlIEdlc2VsbHNjaGFmdGVyaW46IERpZWhs
IFZlcndhbHR1bmdzLVN0aWZ0dW5nIOKAkyBTaXR6OiBOw7xybmJlcmcg4oCTIFJlZ2lzdGVy
Z2VyaWNodDogQW10c2dlcmljaHQNCj4gPiBOw7xybmJlcmcgSFJBIDExNzU2IOKAkw0KPiA+
IFZvcnN0YW5kOiBEci4tSW5nLiBFLmguIFRob21hcyBEaWVobCAo4oCgKSAoVm9yc2l0emVu
ZGVyKSwgSGVyciBEaXBsLi1XaXJ0c2NoLi1JbmcuIFdvbGZnYW5nIFdlZ2dlbiAoc3RlbGx2
ZXJ0cmV0ZW5kZXIgVm9yc2l0emVuZGVyKSwgRGlwbC4tS2ZtLiBDbGF1cyBHw7xudGhlciwg
RGlwbC4tS2ZtLiBGcmFuayBHdXR6ZWl0LCBEci4tSW5nLg0KPiA+IEhlaW5yaWNoIFNjaHVu
aywgRHIuLUluZy4gTWljaGFlbCBTaWVkZW50b3AgLCBEaXBsLi1LZm0uIERyLi1JbmcuIE1h
cnRpbiBTb21tZXIsIERpcGwuLUluZy4gKEZIKSBSYWluZXIgdm9uIEJvcnN0ZWwsIFZvcnNp
dHplbmRlciBkZXMgQXVmc2ljaHRzcmF0ZXM6IERyLiBLbGF1cyBNYWllcg0KPiA+IF9fX19f
X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f
X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXw0KPiA+IERlciBJbmhh
bHQgZGVyIHZvcnN0ZWhlbmRlbiBFLU1haWwgaXN0IG5pY2h0IHJlY2h0bGljaCBiaW5kZW5k
LiBEaWVzZSBFLU1haWwgZW50aGFlbHQgdmVydHJhdWxpY2hlIHVuZC9vZGVyIHJlY2h0bGlj
aCBnZXNjaHVldHp0ZSBJbmZvcm1hdGlvbmVuLg0KPiA+IEluZm9ybWllcmVuIFNpZSB1bnMg
Yml0dGUsIHdlbm4gU2llIGRpZXNlIEUtTWFpbCBmYWVsc2NobGljaGVyd2Vpc2UgZXJoYWx0
ZW4gaGFiZW4uIEJpdHRlIGxvZXNjaGVuIFNpZSBpbiBkaWVzZW0gRmFsbCBkaWUgTmFjaHJp
Y2h0LiBKZWRlIHVuZXJsYXVidGUgRm9ybSBkZXIgUmVwcm9kdWt0aW9uLCBCZWthbm50Z2Fi
ZSwgQWVuZGVydW5nLA0KPiA+IFZlcnRlaWx1bmcgdW5kL29kZXIgUHVibGlrYXRpb24gZGll
c2VyIEUtTWFpbCBpc3Qgc3RyZW5nc3RlbnMgdW50ZXJzYWd0Lg0KPiA+IFRoZSBjb250ZW50
cyBvZiB0aGUgYWJvdmUgbWVudGlvbmVkIGUtbWFpbCBpcyBub3QgbGVnYWxseSBiaW5kaW5n
LiBUaGlzIGUtbWFpbCBjb250YWlucyBjb25maWRlbnRpYWwgYW5kL29yIGxlZ2FsbHkgcHJv
dGVjdGVkIGluZm9ybWF0aW9uLiBQbGVhc2UgaW5mb3JtIHVzIGlmIHlvdSBoYXZlIHJlY2Vp
dmVkIHRoaXMgZS1tYWlsIGJ5DQo+ID4gbWlzdGFrZSBhbmQgZGVsZXRlIGl0IGluIHN1Y2gg
YSBjYXNlLiBFYWNoIHVuYXV0aG9yaXplZCByZXByb2R1Y3Rpb24sIGRpc2Nsb3N1cmUsIGFs
dGVyYXRpb24sIGRpc3RyaWJ1dGlvbiBhbmQvb3IgcHVibGljYXRpb24gb2YgdGhpcyBlLW1h
aWwgaXMgc3RyaWN0bHkgcHJvaGliaXRlZC4NCkRpZWhsIEFLTyBTdGlmdHVuZyAmIENvLiBL
RywgUGZhbm5lcnN0cmHDn2UgNzUtODMsIDg4MjM5IFdhbmdlbiBpbSBBbGxnw6R1DQpCZXJl
aWNoc3ZvcnN0YW5kOiBEci4tSW5nLiBNaWNoYWVsIFNpZWRlbnRvcCAoU3ByZWNoZXIpLCBK
b3NlZiBGZWxsbmVyIChNaXRnbGllZCkNClNpdHogZGVyIEdlc2VsbHNjaGFmdDogV2FuZ2Vu
IGkuQS4g4oCTIFJlZ2lzdGVyZ2VyaWNodDogQW10c2dlcmljaHQgVWxtIEhSQSA2MjA2MDkg
4oCTIFBlcnPDtm5saWNoIGhhZnRlbmRlIEdlc2VsbHNjaGFmdGVyaW46IERpZWhsIFZlcndh
bHR1bmdzLVN0aWZ0dW5nIOKAkyBTaXR6OiBOw7xybmJlcmcg4oCTIFJlZ2lzdGVyZ2VyaWNo
dDogQW10c2dlcmljaHQgTsO8cm5iZXJnIEhSQSAxMTc1NiDigJMNClZvcnN0YW5kOiBEci4t
SW5nLiBFLmguIFRob21hcyBEaWVobCAo4oCgKSAoVm9yc2l0emVuZGVyKSwgSGVyciBEaXBs
Li1XaXJ0c2NoLi1JbmcuIFdvbGZnYW5nIFdlZ2dlbiAoc3RlbGx2ZXJ0cmV0ZW5kZXIgVm9y
c2l0emVuZGVyKSwgRGlwbC4tS2ZtLiBDbGF1cyBHw7xudGhlciwgRGlwbC4tS2ZtLiBGcmFu
ayBHdXR6ZWl0LCBEci4tSW5nLiBIZWlucmljaCBTY2h1bmssIERyLi1JbmcuIE1pY2hhZWwg
U2llZGVudG9wICwgRGlwbC4tS2ZtLiBEci4tSW5nLiBNYXJ0aW4gU29tbWVyLCBEaXBsLi1J
bmcuIChGSCkgUmFpbmVyIHZvbiBCb3JzdGVsLCBWb3JzaXR6ZW5kZXIgZGVzIEF1ZnNpY2h0
c3JhdGVzOiBEci4gS2xhdXMgTWFpZXINCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19f
X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f
X19fX19fX19fX19fX19fXw0KRGVyIEluaGFsdCBkZXIgdm9yc3RlaGVuZGVuIEUtTWFpbCBp
c3QgbmljaHQgcmVjaHRsaWNoIGJpbmRlbmQuIERpZXNlIEUtTWFpbCBlbnRoYWVsdCB2ZXJ0
cmF1bGljaGUgdW5kL29kZXIgcmVjaHRsaWNoIGdlc2NodWV0enRlIEluZm9ybWF0aW9uZW4u
DQpJbmZvcm1pZXJlbiBTaWUgdW5zIGJpdHRlLCB3ZW5uIFNpZSBkaWVzZSBFLU1haWwgZmFl
bHNjaGxpY2hlcndlaXNlIGVyaGFsdGVuIGhhYmVuLiBCaXR0ZSBsb2VzY2hlbiBTaWUgaW4g
ZGllc2VtIEZhbGwgZGllIE5hY2hyaWNodC4gSmVkZSB1bmVybGF1YnRlIEZvcm0gZGVyIFJl
cHJvZHVrdGlvbiwgQmVrYW5udGdhYmUsIEFlbmRlcnVuZywgVmVydGVpbHVuZyB1bmQvb2Rl
ciBQdWJsaWthdGlvbiBkaWVzZXIgRS1NYWlsIGlzdCBzdHJlbmdzdGVucyB1bnRlcnNhZ3Qu
DQpUaGUgY29udGVudHMgb2YgdGhlIGFib3ZlIG1lbnRpb25lZCBlLW1haWwgaXMgbm90IGxl
Z2FsbHkgYmluZGluZy4gVGhpcyBlLW1haWwgY29udGFpbnMgY29uZmlkZW50aWFsIGFuZC9v
ciBsZWdhbGx5IHByb3RlY3RlZCBpbmZvcm1hdGlvbi4gUGxlYXNlIGluZm9ybSB1cyBpZiB5
b3UgaGF2ZSByZWNlaXZlZCB0aGlzIGUtbWFpbCBieSBtaXN0YWtlIGFuZCBkZWxldGUgaXQg
aW4gc3VjaCBhIGNhc2UuIEVhY2ggdW5hdXRob3JpemVkIHJlcHJvZHVjdGlvbiwgZGlzY2xv
c3VyZSwgYWx0ZXJhdGlvbiwgZGlzdHJpYnV0aW9uIGFuZC9vciBwdWJsaWNhdGlvbiBvZiB0
aGlzIGUtbWFpbCBpcyBzdHJpY3RseSBwcm9oaWJpdGVkLg==

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 3/4] rtc: isl1208: add support for isl1219 with tamper detection
  2018-03-07  8:19       ` Denis OSTERLAND
  (?)
@ 2018-03-07 10:47       ` Alexandre Belloni
  2018-03-08 11:53           ` Denis OSTERLAND
  -1 siblings, 1 reply; 15+ messages in thread
From: Alexandre Belloni @ 2018-03-07 10:47 UTC (permalink / raw)
  To: Denis OSTERLAND
  Cc: linux-kernel, mgr, m.grzeschik, devicetree, a.zummo, linux,
	jdelvare, linux-rtc, kernel

On 07/03/2018 at 08:19:15 +0000, Denis OSTERLAND wrote:
> > > +/* 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
> > > +
> > Because they are not available on ISL1208, maybe it would be better to
> > prefix them with ISL1219.
> I see. Yes, this would clarify that they are only available on isl1219.
> Shall we rename isl1208_rtc_event_show_timestamp/isl1208_rtc_event_clear
> to isl1219_rtc_event_show_timestamp/isl1219_rtc_event_clear, too?

That could be done too, yes.

> > 
> > > 
> > > +
> > > +	tv64.tv_sec = rtc_tm_to_time64(&tm);
> > Why not using an unsigned long long directly here? time64_t is not the
> > correct type.
> Do you mean timespec64 is not the correct type here?
> Then yes, sould be time64_t.
> If you mean time64_t is not the correct type here,
> then can you give me some detail why there is no rtc_tm_to_u64,
> or something like that?

The rtc subsystem forbids negative times, the proper type should be
unsigned.

> sprintf(buf, "%lld\n", rtc_tm_to_time64(&tm)) seems correct to me.
> By the way, is it needed to check for seconds < 0 and return error?

Indeed, you shoud check the tm with rtc_valid_tm before calling
rtc_tm_to_time64.

> > > -	rc = sysfs_create_group(&client->dev.kobj, &isl1208_rtc_sysfs_files);
> > > +	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->sysfs_files = &isl1219_rtc_sysfs_files;
> > > +	} else {
> > > +		isl1208->sysfs_files = &isl1208_rtc_sysfs_files;
> > > +	}
> > > +
> > I don't think the whole isl1208 is necessary. You should probably use
> > the .is_visible callback of isl1219_rtc_sysfs_files. This will make the
> > changelog quite smaller.
> > 
> Well, I don´t know how to access i2c_device_id from kobject.
> rtc_attr_is_visible shows how to convert kobject to device and rtc_device,
> but how to do (id->driver_data == TYPE_ISL1219) here?

I'd use i2c_set_clientdata/i2c_get_clientdata but I agree that then it
is basically the same as having isl1208->sysfs_files.

but this makes me realize that the timestamp file doesn't end up at the
correct location. What you do now is placing it under the i2c device
while it should be placed under the rtc device (i.e. in
/sys/class/rtc/rtcX/). This was a mistake made back in 2006.

I guess you'll have to add a new group instead of adding to the current
one.

-- 
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 3/4] rtc: isl1208: add support for isl1219 with tamper detection
  2018-03-05 10:43 ` [PATCH v3 3/4] rtc: isl1208: add support for isl1219 with tamper detection Denis OSTERLAND
  2018-03-06 20:42   ` Alexandre Belloni
@ 2018-03-07 22:02   ` Rob Herring
  1 sibling, 0 replies; 15+ messages in thread
From: Rob Herring @ 2018-03-07 22:02 UTC (permalink / raw)
  To: Denis OSTERLAND
  Cc: a.zummo, alexandre.belloni, linux-kernel, mgr, m.grzeschik,
	devicetree, linux, jdelvare, linux-rtc, kernel

On Mon, Mar 05, 2018 at 10:43:52AM +0000, Denis OSTERLAND wrote:
> From: Michael Grzeschik <m.grzeschik@pengutronix.de>
> 
> We add support for the ISL1219 chip that got an integrated tamper
> detection function. This patch implements the feature by adding
> an additional timestamp0 file to sysfs device path.
> This file contains seconds since epoch, if an event occurred,
> or is empty, if none occurred.
> 
> The devicetree documentation for the ISL1219 device tree
> binding is added with an short example. It is not a trivial
> device, because it supports two interrupt souces.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> Signed-off-by: Denis Osterland <Denis.Osterland@diehl.com>
> ---
>  .../devicetree/bindings/rtc/isil,isl1219.txt       |  28 ++++

While preferred to separate, I'm not going to ask for that if there are 
no other changes on the binding.

Reviewed-by: Rob Herring <robh@kernel.org>

>  drivers/rtc/rtc-isl1208.c                          | 160 ++++++++++++++++++---
>  2 files changed, 171 insertions(+), 17 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/rtc/isil,isl1219.txt

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 3/4] rtc: isl1208: add support for isl1219 with tamper detection
  2018-03-07 10:47       ` Alexandre Belloni
@ 2018-03-08 11:53           ` Denis OSTERLAND
  0 siblings, 0 replies; 15+ messages in thread
From: Denis OSTERLAND @ 2018-03-08 11:53 UTC (permalink / raw)
  To: alexandre.belloni
  Cc: linux-kernel, mgr, m.grzeschik, devicetree, a.zummo, linux,
	jdelvare, linux-rtc, kernel

Am Mittwoch, den 07.03.2018, 11:47 +0100 schrieb Alexandre Belloni:
> > > > +
> > > > +	tv64.tv_sec = rtc_tm_to_time64(&tm);
> > > Why not using an unsigned long long directly here? time64_t is not the
> > > correct type.
> > Do you mean timespec64 is not the correct type here?
> > Then yes, sould be time64_t.
> > If you mean time64_t is not the correct type here,
> > then can you give me some detail why there is no rtc_tm_to_u64,
> > or something like that?
> The rtc subsystem forbids negative times, the proper type should be
> unsigned.
I will add rtc_vaild_tm check.

Which sequence for time conversion would you expect?

time64_t secs = rtc_tm_to_time64(&tm);
BUG_ON(secs < 0);
return sprintf(buf, "%llu\n", (unsigned long long)secs);

or

return sprintf(buf, "%llu\n", (unsigned long long)rtc_tm_to_time64(&tm));
> 
> > 
> > sprintf(buf, "%lld\n", rtc_tm_to_time64(&tm)) seems correct to me.
> > By the way, is it needed to check for seconds < 0 and return error?
> Indeed, you shoud check the tm with rtc_valid_tm before calling
> rtc_tm_to_time64.
> 
> > 
> > > 
> > > > 
> > > > -	rc = sysfs_create_group(&client->dev.kobj, &isl1208_rtc_sysfs_files);
> > > > +	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->sysfs_files = &isl1219_rtc_sysfs_files;
> > > > +	} else {
> > > > +		isl1208->sysfs_files = &isl1208_rtc_sysfs_files;
> > > > +	}
> > > > +
> > > I don't think the whole isl1208 is necessary. You should probably use
> > > the .is_visible callback of isl1219_rtc_sysfs_files. This will make the
> > > changelog quite smaller.
> > > 
> > Well, I don´t know how to access i2c_device_id from kobject.
> > rtc_attr_is_visible shows how to convert kobject to device and rtc_device,
> > but how to do (id->driver_data == TYPE_ISL1219) here?
> I'd use i2c_set_clientdata/i2c_get_clientdata but I agree that then it
> is basically the same as having isl1208->sysfs_files.
> 
> but this makes me realize that the timestamp file doesn't end up at the
> correct location. What you do now is placing it under the i2c device
> while it should be placed under the rtc device (i.e. in
> /sys/class/rtc/rtcX/). This was a mistake made back in 2006.
> 
> I guess you'll have to add a new group instead of adding to the current
> one.
I guess I found a way to do it.

static struct attribute *isl1219_rtc_attrs[] = {
	&dev_attr_timestamp0.attr,
	NULL
};

in probe
	if (id->driver_data == TYPE_ISL1219) {
		sysfs_merge_group(&rtc->kobj, &isl1219_rtc_sysfs_files);

in remove
	struct rtc_device *rtc = i2c_get_clientdata(client);
	sysfs_unmerge_group(&rtc->kobj, &isl1219_rtc_sysfs_files);

As far as I got it, I can call unmerge even if group was not merged before.
If it works I don´t need struct isl1208 at all.
> 
Diehl AKO Stiftung & Co. KG, Pfannerstraße 75-83, 88239 Wangen im Allgäu
Bereichsvorstand: Dr.-Ing. Michael Siedentop (Sprecher), Josef Fellner (Mitglied)
Sitz der Gesellschaft: Wangen i.A. – Registergericht: Amtsgericht Ulm HRA 620609 – Persönlich haftende Gesellschafterin: Diehl Verwaltungs-Stiftung – Sitz: Nürnberg – Registergericht: Amtsgericht Nürnberg HRA 11756 –
Vorstand: Dr.-Ing. E.h. Thomas Diehl (†) (Vorsitzender), Herr Dipl.-Wirtsch.-Ing. Wolfgang Weggen (stellvertretender Vorsitzender), Dipl.-Kfm. Claus Günther, Dipl.-Kfm. Frank Gutzeit, Dr.-Ing. Heinrich Schunk, Dr.-Ing. Michael Siedentop , Dipl.-Kfm. Dr.-Ing. Martin Sommer, Dipl.-Ing. (FH) Rainer von Borstel, Vorsitzender des Aufsichtsrates: Dr. Klaus Maier
___________________________________________________________________________________________________
Der Inhalt der vorstehenden E-Mail ist nicht rechtlich bindend. Diese E-Mail enthaelt vertrauliche und/oder rechtlich geschuetzte Informationen.
Informieren Sie uns bitte, wenn Sie diese E-Mail faelschlicherweise erhalten haben. Bitte loeschen Sie in diesem Fall die Nachricht. Jede unerlaubte Form der Reproduktion, Bekanntgabe, Aenderung, Verteilung und/oder Publikation dieser E-Mail ist strengstens untersagt.
The contents of the above mentioned e-mail is not legally binding. This e-mail contains confidential and/or legally protected information. Please inform us if you have received this e-mail by mistake and delete it in such a case. Each unauthorized reproduction, disclosure, alteration, distribution and/or publication of this e-mail is strictly prohibited.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 3/4] rtc: isl1208: add support for isl1219 with tamper detection
@ 2018-03-08 11:53           ` Denis OSTERLAND
  0 siblings, 0 replies; 15+ messages in thread
From: Denis OSTERLAND @ 2018-03-08 11:53 UTC (permalink / raw)
  To: alexandre.belloni
  Cc: linux-kernel, mgr, m.grzeschik, devicetree, a.zummo, linux,
	jdelvare, linux-rtc, kernel

QW0gTWl0dHdvY2gsIGRlbiAwNy4wMy4yMDE4LCAxMTo0NyArMDEwMCBzY2hyaWViIEFsZXhh
bmRyZSBCZWxsb25pOg0KPiA+ID4gPiArDQo+ID4gPiA+ICsJdHY2NC50dl9zZWMgPSBydGNf
dG1fdG9fdGltZTY0KCZ0bSk7DQo+ID4gPiBXaHkgbm90IHVzaW5nIGFuIHVuc2lnbmVkIGxv
bmcgbG9uZyBkaXJlY3RseSBoZXJlPyB0aW1lNjRfdCBpcyBub3QgdGhlDQo+ID4gPiBjb3Jy
ZWN0IHR5cGUuDQo+ID4gRG8geW91IG1lYW4gdGltZXNwZWM2NCBpcyBub3QgdGhlIGNvcnJl
Y3QgdHlwZSBoZXJlPw0KPiA+IFRoZW4geWVzLCBzb3VsZCBiZSB0aW1lNjRfdC4NCj4gPiBJ
ZiB5b3UgbWVhbiB0aW1lNjRfdCBpcyBub3QgdGhlIGNvcnJlY3QgdHlwZSBoZXJlLA0KPiA+
IHRoZW4gY2FuIHlvdSBnaXZlIG1lIHNvbWUgZGV0YWlsIHdoeSB0aGVyZSBpcyBubyBydGNf
dG1fdG9fdTY0LA0KPiA+IG9yIHNvbWV0aGluZyBsaWtlIHRoYXQ/DQo+IFRoZSBydGMgc3Vi
c3lzdGVtIGZvcmJpZHMgbmVnYXRpdmUgdGltZXMsIHRoZSBwcm9wZXIgdHlwZSBzaG91bGQg
YmUNCj4gdW5zaWduZWQuDQpJIHdpbGwgYWRkIHJ0Y192YWlsZF90bSBjaGVjay4NCg0KV2hp
Y2ggc2VxdWVuY2UgZm9yIHRpbWUgY29udmVyc2lvbiB3b3VsZCB5b3UgZXhwZWN0Pw0KDQp0
aW1lNjRfdCBzZWNzID0gcnRjX3RtX3RvX3RpbWU2NCgmdG0pOw0KQlVHX09OKHNlY3MgPCAw
KTsNCnJldHVybiBzcHJpbnRmKGJ1ZiwgIiVsbHVcbiIsICh1bnNpZ25lZCBsb25nIGxvbmcp
c2Vjcyk7DQoNCm9yDQoNCnJldHVybiBzcHJpbnRmKGJ1ZiwgIiVsbHVcbiIsICh1bnNpZ25l
ZCBsb25nIGxvbmcpcnRjX3RtX3RvX3RpbWU2NCgmdG0pKTsNCj4gDQo+ID4gDQo+ID4gc3By
aW50ZihidWYsICIlbGxkXG4iLMKgcnRjX3RtX3RvX3RpbWU2NCgmdG0pKSBzZWVtcyBjb3Jy
ZWN0IHRvIG1lLg0KPiA+IEJ5IHRoZSB3YXksIGlzIGl0IG5lZWRlZCB0byBjaGVjayBmb3Ig
c2Vjb25kcyA8IDAgYW5kIHJldHVybiBlcnJvcj8NCj4gSW5kZWVkLCB5b3Ugc2hvdWQgY2hl
Y2sgdGhlIHRtIHdpdGggcnRjX3ZhbGlkX3RtIGJlZm9yZSBjYWxsaW5nDQo+IHJ0Y190bV90
b190aW1lNjQuDQo+IA0KPiA+IA0KPiA+ID4gDQo+ID4gPiA+IA0KPiA+ID4gPiAtCXJjID0g
c3lzZnNfY3JlYXRlX2dyb3VwKCZjbGllbnQtPmRldi5rb2JqLCAmaXNsMTIwOF9ydGNfc3lz
ZnNfZmlsZXMpOw0KPiA+ID4gPiArCWlmIChpZC0+ZHJpdmVyX2RhdGEgPT0gVFlQRV9JU0wx
MjE5KSB7DQo+ID4gPiA+ICsJCXJjID0gaTJjX3NtYnVzX3dyaXRlX2J5dGVfZGF0YShjbGll
bnQsIElTTDEyMDhfUkVHXzA5LCAweDEwKTsNCj4gPiA+ID4gKwkJaWYgKHJjIDwgMCkgew0K
PiA+ID4gPiArCQkJZGV2X2VycigmY2xpZW50LT5kZXYsICJjb3VsZCBub3QgZW5hYmxlIHRh
bXBlciBkZXRlY3Rpb25cbiIpOw0KPiA+ID4gPiArCQkJcmV0dXJuIHJjOw0KPiA+ID4gPiAr
CQl9DQo+ID4gPiA+ICsJCWlzbDEyMDgtPnN5c2ZzX2ZpbGVzID0gJmlzbDEyMTlfcnRjX3N5
c2ZzX2ZpbGVzOw0KPiA+ID4gPiArCX0gZWxzZSB7DQo+ID4gPiA+ICsJCWlzbDEyMDgtPnN5
c2ZzX2ZpbGVzID0gJmlzbDEyMDhfcnRjX3N5c2ZzX2ZpbGVzOw0KPiA+ID4gPiArCX0NCj4g
PiA+ID4gKw0KPiA+ID4gSSBkb24ndCB0aGluayB0aGUgd2hvbGUgaXNsMTIwOCBpcyBuZWNl
c3NhcnkuIFlvdSBzaG91bGQgcHJvYmFibHkgdXNlDQo+ID4gPiB0aGUgLmlzX3Zpc2libGUg
Y2FsbGJhY2sgb2YgaXNsMTIxOV9ydGNfc3lzZnNfZmlsZXMuIFRoaXMgd2lsbCBtYWtlIHRo
ZQ0KPiA+ID4gY2hhbmdlbG9nIHF1aXRlIHNtYWxsZXIuDQo+ID4gPiANCj4gPiBXZWxsLCBJ
IGRvbsK0dCBrbm93IGhvdyB0byBhY2Nlc3MgaTJjX2RldmljZV9pZCBmcm9tIGtvYmplY3Qu
DQo+ID4gcnRjX2F0dHJfaXNfdmlzaWJsZSBzaG93cyBob3cgdG8gY29udmVydCBrb2JqZWN0
IHRvIGRldmljZSBhbmQgcnRjX2RldmljZSwNCj4gPiBidXQgaG93IHRvIGRvwqAoaWQtPmRy
aXZlcl9kYXRhID09IFRZUEVfSVNMMTIxOSkgaGVyZT8NCj4gSSdkIHVzZSBpMmNfc2V0X2Ns
aWVudGRhdGEvaTJjX2dldF9jbGllbnRkYXRhIGJ1dCBJIGFncmVlIHRoYXQgdGhlbiBpdA0K
PiBpcyBiYXNpY2FsbHkgdGhlIHNhbWUgYXMgaGF2aW5nIGlzbDEyMDgtPnN5c2ZzX2ZpbGVz
Lg0KPiANCj4gYnV0IHRoaXMgbWFrZXMgbWUgcmVhbGl6ZSB0aGF0IHRoZSB0aW1lc3RhbXAg
ZmlsZSBkb2Vzbid0IGVuZCB1cCBhdCB0aGUNCj4gY29ycmVjdCBsb2NhdGlvbi4gV2hhdCB5
b3UgZG8gbm93IGlzIHBsYWNpbmcgaXQgdW5kZXIgdGhlIGkyYyBkZXZpY2UNCj4gd2hpbGUg
aXQgc2hvdWxkIGJlIHBsYWNlZCB1bmRlciB0aGUgcnRjIGRldmljZSAoaS5lLiBpbg0KPiAv
c3lzL2NsYXNzL3J0Yy9ydGNYLykuIFRoaXMgd2FzIGEgbWlzdGFrZSBtYWRlIGJhY2sgaW4g
MjAwNi4NCj4gDQo+IEkgZ3Vlc3MgeW91J2xsIGhhdmUgdG8gYWRkIGEgbmV3IGdyb3VwIGlu
c3RlYWQgb2YgYWRkaW5nIHRvIHRoZSBjdXJyZW50DQo+IG9uZS4NCkkgZ3Vlc3MgSSBmb3Vu
ZCBhIHdheSB0byBkbyBpdC4NCg0Kc3RhdGljIHN0cnVjdCBhdHRyaWJ1dGUgKmlzbDEyMTlf
cnRjX2F0dHJzW10gPSB7DQoJJmRldl9hdHRyX3RpbWVzdGFtcDAuYXR0ciwNCglOVUxMDQp9
Ow0KDQppbiBwcm9iZQ0KCWlmIChpZC0+ZHJpdmVyX2RhdGEgPT0gVFlQRV9JU0wxMjE5KSB7
DQoJCXN5c2ZzX21lcmdlX2dyb3VwKCZydGMtPmtvYmosICZpc2wxMjE5X3J0Y19zeXNmc19m
aWxlcyk7DQoNCmluIHJlbW92ZQ0KCXN0cnVjdCBydGNfZGV2aWNlICpydGMgPcKgaTJjX2dl
dF9jbGllbnRkYXRhKGNsaWVudCk7DQoJc3lzZnNfdW5tZXJnZV9ncm91cCgmcnRjLT5rb2Jq
LCAmaXNsMTIxOV9ydGNfc3lzZnNfZmlsZXMpOw0KDQpBcyBmYXIgYXMgSSBnb3QgaXQsIEkg
Y2FuIGNhbGwgdW5tZXJnZSBldmVuIGlmIGdyb3VwIHdhcyBub3QgbWVyZ2VkIGJlZm9yZS4N
CklmIGl0IHdvcmtzIEkgZG9uwrR0IG5lZWQgc3RydWN0IGlzbDEyMDggYXQgYWxsLg0KPiAN
CkRpZWhsIEFLTyBTdGlmdHVuZyAmIENvLiBLRywgUGZhbm5lcnN0cmHDn2UgNzUtODMsIDg4
MjM5IFdhbmdlbiBpbSBBbGxnw6R1DQpCZXJlaWNoc3ZvcnN0YW5kOiBEci4tSW5nLiBNaWNo
YWVsIFNpZWRlbnRvcCAoU3ByZWNoZXIpLCBKb3NlZiBGZWxsbmVyIChNaXRnbGllZCkNClNp
dHogZGVyIEdlc2VsbHNjaGFmdDogV2FuZ2VuIGkuQS4g4oCTIFJlZ2lzdGVyZ2VyaWNodDog
QW10c2dlcmljaHQgVWxtIEhSQSA2MjA2MDkg4oCTIFBlcnPDtm5saWNoIGhhZnRlbmRlIEdl
c2VsbHNjaGFmdGVyaW46IERpZWhsIFZlcndhbHR1bmdzLVN0aWZ0dW5nIOKAkyBTaXR6OiBO
w7xybmJlcmcg4oCTIFJlZ2lzdGVyZ2VyaWNodDogQW10c2dlcmljaHQgTsO8cm5iZXJnIEhS
QSAxMTc1NiDigJMNClZvcnN0YW5kOiBEci4tSW5nLiBFLmguIFRob21hcyBEaWVobCAo4oCg
KSAoVm9yc2l0emVuZGVyKSwgSGVyciBEaXBsLi1XaXJ0c2NoLi1JbmcuIFdvbGZnYW5nIFdl
Z2dlbiAoc3RlbGx2ZXJ0cmV0ZW5kZXIgVm9yc2l0emVuZGVyKSwgRGlwbC4tS2ZtLiBDbGF1
cyBHw7xudGhlciwgRGlwbC4tS2ZtLiBGcmFuayBHdXR6ZWl0LCBEci4tSW5nLiBIZWlucmlj
aCBTY2h1bmssIERyLi1JbmcuIE1pY2hhZWwgU2llZGVudG9wICwgRGlwbC4tS2ZtLiBEci4t
SW5nLiBNYXJ0aW4gU29tbWVyLCBEaXBsLi1JbmcuIChGSCkgUmFpbmVyIHZvbiBCb3JzdGVs
LCBWb3JzaXR6ZW5kZXIgZGVzIEF1ZnNpY2h0c3JhdGVzOiBEci4gS2xhdXMgTWFpZXINCl9f
X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f
X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXw0KRGVyIEluaGFs
dCBkZXIgdm9yc3RlaGVuZGVuIEUtTWFpbCBpc3QgbmljaHQgcmVjaHRsaWNoIGJpbmRlbmQu
IERpZXNlIEUtTWFpbCBlbnRoYWVsdCB2ZXJ0cmF1bGljaGUgdW5kL29kZXIgcmVjaHRsaWNo
IGdlc2NodWV0enRlIEluZm9ybWF0aW9uZW4uDQpJbmZvcm1pZXJlbiBTaWUgdW5zIGJpdHRl
LCB3ZW5uIFNpZSBkaWVzZSBFLU1haWwgZmFlbHNjaGxpY2hlcndlaXNlIGVyaGFsdGVuIGhh
YmVuLiBCaXR0ZSBsb2VzY2hlbiBTaWUgaW4gZGllc2VtIEZhbGwgZGllIE5hY2hyaWNodC4g
SmVkZSB1bmVybGF1YnRlIEZvcm0gZGVyIFJlcHJvZHVrdGlvbiwgQmVrYW5udGdhYmUsIEFl
bmRlcnVuZywgVmVydGVpbHVuZyB1bmQvb2RlciBQdWJsaWthdGlvbiBkaWVzZXIgRS1NYWls
IGlzdCBzdHJlbmdzdGVucyB1bnRlcnNhZ3QuDQpUaGUgY29udGVudHMgb2YgdGhlIGFib3Zl
IG1lbnRpb25lZCBlLW1haWwgaXMgbm90IGxlZ2FsbHkgYmluZGluZy4gVGhpcyBlLW1haWwg
Y29udGFpbnMgY29uZmlkZW50aWFsIGFuZC9vciBsZWdhbGx5IHByb3RlY3RlZCBpbmZvcm1h
dGlvbi4gUGxlYXNlIGluZm9ybSB1cyBpZiB5b3UgaGF2ZSByZWNlaXZlZCB0aGlzIGUtbWFp
bCBieSBtaXN0YWtlIGFuZCBkZWxldGUgaXQgaW4gc3VjaCBhIGNhc2UuIEVhY2ggdW5hdXRo
b3JpemVkIHJlcHJvZHVjdGlvbiwgZGlzY2xvc3VyZSwgYWx0ZXJhdGlvbiwgZGlzdHJpYnV0
aW9uIGFuZC9vciBwdWJsaWNhdGlvbiBvZiB0aGlzIGUtbWFpbCBpcyBzdHJpY3RseSBwcm9o
aWJpdGVkLg==

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 3/4] rtc: isl1208: add support for isl1219 with tamper detection
  2018-03-08 11:53           ` Denis OSTERLAND
  (?)
@ 2018-03-08 12:05           ` Alexandre Belloni
  -1 siblings, 0 replies; 15+ messages in thread
From: Alexandre Belloni @ 2018-03-08 12:05 UTC (permalink / raw)
  To: Denis OSTERLAND
  Cc: linux-kernel, mgr, m.grzeschik, devicetree, a.zummo, linux,
	jdelvare, linux-rtc, kernel

On 08/03/2018 at 11:53:09 +0000, Denis OSTERLAND wrote:
> Am Mittwoch, den 07.03.2018, 11:47 +0100 schrieb Alexandre Belloni:
> > > > > +
> > > > > +	tv64.tv_sec = rtc_tm_to_time64(&tm);
> > > > Why not using an unsigned long long directly here? time64_t is not the
> > > > correct type.
> > > Do you mean timespec64 is not the correct type here?
> > > Then yes, sould be time64_t.
> > > If you mean time64_t is not the correct type here,
> > > then can you give me some detail why there is no rtc_tm_to_u64,
> > > or something like that?
> > The rtc subsystem forbids negative times, the proper type should be
> > unsigned.
> I will add rtc_vaild_tm check.
> 
> Which sequence for time conversion would you expect?
> 
> time64_t secs = rtc_tm_to_time64(&tm);
> BUG_ON(secs < 0);
> return sprintf(buf, "%llu\n", (unsigned long long)secs);
> 
> or
> 
> return sprintf(buf, "%llu\n", (unsigned long long)rtc_tm_to_time64(&tm));

rtc_vaild_tm will already return EINVAL in case of negative time so this
is the one you should use.

-- 
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2018-03-08 12:05 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-05 10:43 [PATCH v3 0/4] rtc: isl1208: fixes, documentation and isl1219 support Denis OSTERLAND
2018-03-05 10:43 ` [PATCH v3 4/4] rtc: isl1208: Add "evdet" interrupt source for isl1219 Denis OSTERLAND
2018-03-05 10:43 ` [PATCH v3 3/4] rtc: isl1208: add support for isl1219 with tamper detection Denis OSTERLAND
2018-03-06 20:42   ` Alexandre Belloni
2018-03-07  8:19     ` Denis OSTERLAND
2018-03-07  8:19       ` Denis OSTERLAND
2018-03-07 10:47       ` Alexandre Belloni
2018-03-08 11:53         ` Denis OSTERLAND
2018-03-08 11:53           ` Denis OSTERLAND
2018-03-08 12:05           ` Alexandre Belloni
2018-03-07 22:02   ` Rob Herring
2018-03-05 10:43 ` [PATCH v3 2/4] rtc: isl1208: switch to rtc_register_device Denis OSTERLAND
2018-03-06 20:20   ` Alexandre Belloni
2018-03-05 10:43 ` [PATCH v3 1/4] rtc: isl1208: enable interrupt after context preparation Denis OSTERLAND
2018-03-06 20:20   ` Alexandre Belloni

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.