[PATCH 2nd try] RTC/i.MX/DryICE: add recovery routines to the driver

[PATCH 2nd try] RTC/i.MX/DryICE: add recovery routines to the driver

[Juergen Borleis]

2nd try, this time with a cover letter... m(

The built-in RTC unit on some i.MX SoCs isn't an RTC only. It is also a tamper
monitor unit which can keep some keys. When it does its tamper detection job
and a tamper violation is detected, this RTC unit locks completely including
the real-time counter. In this state the unit is completely useless. The only
way to bring it out of this locked state is a power on reset. At the next boot
time some flags signals the tamper violation and a specific register access
sequence must be done to finaly bring this unit into life again. Until this is
done, there is no way to use it again as an RTC.
But also without any enabled tamper detection sometimes this unit tends to
lock. And in this case the same steps must be done to bring it into life
again.
The current implementation of the DryIce driver isn't able to unlock the
device successfully in the case it is locked somehow. Only a full power cycle
including *battery power* can help in this case.

The attached change set adds the required routines to be able to unlock the
DryIce unit in the case the driver detects a locked unit. This includes
unlocking it if it is locked by accident or malfunction and not by a real
tamper violation.

The last patch of this series is for reference only and should not be part
of the kernel. It just adds some code to force a locked DryIce unit to check
if the new routines are able to unlock it again. This code was required
because I had no hardware which really uses the tamper detection features of
this unit.

Comments are welcome.

jbe

[rtc-linux] [PATCH 2nd try] RTC/i.MX/DryICE: add recovery routines to the driver

[Juergen Borleis]

2nd try, this time with a cover letter... m(

The built-in RTC unit on some i.MX SoCs isn't an RTC only. It is also a tamper
monitor unit which can keep some keys. When it does its tamper detection job
and a tamper violation is detected, this RTC unit locks completely including
the real-time counter. In this state the unit is completely useless. The only
way to bring it out of this locked state is a power on reset. At the next boot
time some flags signals the tamper violation and a specific register access
sequence must be done to finaly bring this unit into life again. Until this is
done, there is no way to use it again as an RTC.
But also without any enabled tamper detection sometimes this unit tends to
lock. And in this case the same steps must be done to bring it into life
again.
The current implementation of the DryIce driver isn't able to unlock the
device successfully in the case it is locked somehow. Only a full power cycle
including *battery power* can help in this case.

The attached change set adds the required routines to be able to unlock the
DryIce unit in the case the driver detects a locked unit. This includes
unlocking it if it is locked by accident or malfunction and not by a real
tamper violation.

The last patch of this series is for reference only and should not be part
of the kernel. It just adds some code to force a locked DryIce unit to check
if the new routines are able to unlock it again. This code was required
because I had no hardware which really uses the tamper detection features of
this unit.

Comments are welcome.

jbe

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH 2nd try] RTC/i.MX/DryICE: add recovery routines to the driver

[Juergen Borleis]

2nd try, this time with a cover letter... m(

The built-in RTC unit on some i.MX SoCs isn't an RTC only. It is also a tamper
monitor unit which can keep some keys. When it does its tamper detection job
and a tamper violation is detected, this RTC unit locks completely including
the real-time counter. In this state the unit is completely useless. The only
way to bring it out of this locked state is a power on reset. At the next boot
time some flags signals the tamper violation and a specific register access
sequence must be done to finaly bring this unit into life again. Until this is
done, there is no way to use it again as an RTC.
But also without any enabled tamper detection sometimes this unit tends to
lock. And in this case the same steps must be done to bring it into life
again.
The current implementation of the DryIce driver isn't able to unlock the
device successfully in the case it is locked somehow. Only a full power cycle
including *battery power* can help in this case.

The attached change set adds the required routines to be able to unlock the
DryIce unit in the case the driver detects a locked unit. This includes
unlocking it if it is locked by accident or malfunction and not by a real
tamper violation.

The last patch of this series is for reference only and should not be part
of the kernel. It just adds some code to force a locked DryIce unit to check
if the new routines are able to unlock it again. This code was required
because I had no hardware which really uses the tamper detection features of
this unit.

Comments are welcome.

jbe

[PATCH 1/5] RTC/i.MX/DryIce: add some background info about the states the machine can be in

[Juergen Borleis]

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
Signed-off-by: Robert Schwebel <rsc@pengutronix.de>
[rsc: got NDA clearance from Freescale]
---
 drivers/rtc/rtc-imxdi.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
index c666eab..8750477 100644
--- a/drivers/rtc/rtc-imxdi.c
+++ b/drivers/rtc/rtc-imxdi.c
@@ -129,6 +129,49 @@ struct imxdi_dev {
 	struct work_struct work;
 };
 
+/* Some background:
+ *
+ * The DryIce unit is a complex security/tamper monitor device. To be able do
+ * its job in a useful manner it runs a bigger statemachine to bring it into
+ * security/tamper failure state and once again to bring it out of this state.
+ *
+ * This unit can be in one of three states:
+ *
+ * - "NON-VALID STATE"
+ *   always after the battery power was removed
+ * - "FAILURE STATE"
+ *   if one of the enabled security events have happend
+ * - "VALID STATE"
+ *   if the unit works as expected
+ *
+ * Everything stops when the unit enters the failure state including the RTC
+ * counter (to be able to detect the time the security event happend).
+ *
+ * The following events (when enabled) let the DryIce unit enter the failure
+ * state:
+ *
+ * - wire-mesh-tamper detect
+ * - external tamper B detect
+ * - external tamper A detect
+ * - temperature tamper detect
+ * - clock tamper detect
+ * - voltage tamper detect
+ * - RTC counter overflow
+ * - monotonic counter overflow
+ * - external boot
+ *
+ * If we find the DryIce unit in "FAILURE STATE" and the TDCHL cleared, we
+ * can only detect this state. In this case the unit is completely locked and
+ * must force a second "SYSTEM POR" to bring the DryIce into the
+ * "NON-VALID STATE" + "FAILURE STATE" where a recovery is possible.
+ * If the TDCHL is set in the "FAILURE STATE" we are out of luck. In this case
+ * a battery power cycle is required.
+ *
+ * In the "NON-VALID STATE" + "FAILURE STATE" we can clear the "FAILURE STATE"
+ * and recover the DryIce unit. By clearing the "NON-VALID STATE" as the last
+ * task, we bring back this unit into life.
+ */
+
 /*
  * enable a dryice interrupt
  */
-- 
2.1.4

[rtc-linux] [PATCH 1/5] RTC/i.MX/DryIce: add some background info about the states the machine can be in

[Juergen Borleis]

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
Signed-off-by: Robert Schwebel <rsc@pengutronix.de>
[rsc: got NDA clearance from Freescale]
---
 drivers/rtc/rtc-imxdi.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
index c666eab..8750477 100644
--- a/drivers/rtc/rtc-imxdi.c
+++ b/drivers/rtc/rtc-imxdi.c
@@ -129,6 +129,49 @@ struct imxdi_dev {
 	struct work_struct work;
 };
 
+/* Some background:
+ *
+ * The DryIce unit is a complex security/tamper monitor device. To be able do
+ * its job in a useful manner it runs a bigger statemachine to bring it into
+ * security/tamper failure state and once again to bring it out of this state.
+ *
+ * This unit can be in one of three states:
+ *
+ * - "NON-VALID STATE"
+ *   always after the battery power was removed
+ * - "FAILURE STATE"
+ *   if one of the enabled security events have happend
+ * - "VALID STATE"
+ *   if the unit works as expected
+ *
+ * Everything stops when the unit enters the failure state including the RTC
+ * counter (to be able to detect the time the security event happend).
+ *
+ * The following events (when enabled) let the DryIce unit enter the failure
+ * state:
+ *
+ * - wire-mesh-tamper detect
+ * - external tamper B detect
+ * - external tamper A detect
+ * - temperature tamper detect
+ * - clock tamper detect
+ * - voltage tamper detect
+ * - RTC counter overflow
+ * - monotonic counter overflow
+ * - external boot
+ *
+ * If we find the DryIce unit in "FAILURE STATE" and the TDCHL cleared, we
+ * can only detect this state. In this case the unit is completely locked and
+ * must force a second "SYSTEM POR" to bring the DryIce into the
+ * "NON-VALID STATE" + "FAILURE STATE" where a recovery is possible.
+ * If the TDCHL is set in the "FAILURE STATE" we are out of luck. In this case
+ * a battery power cycle is required.
+ *
+ * In the "NON-VALID STATE" + "FAILURE STATE" we can clear the "FAILURE STATE"
+ * and recover the DryIce unit. By clearing the "NON-VALID STATE" as the last
+ * task, we bring back this unit into life.
+ */
+
 /*
  * enable a dryice interrupt
  */
-- 
2.1.4

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH 1/5] RTC/i.MX/DryIce: add some background info about the states the machine can be in

[Juergen Borleis]

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
Signed-off-by: Robert Schwebel <rsc@pengutronix.de>
[rsc: got NDA clearance from Freescale]
---
 drivers/rtc/rtc-imxdi.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
index c666eab..8750477 100644
--- a/drivers/rtc/rtc-imxdi.c
+++ b/drivers/rtc/rtc-imxdi.c
@@ -129,6 +129,49 @@ struct imxdi_dev {
 	struct work_struct work;
 };
 
+/* Some background:
+ *
+ * The DryIce unit is a complex security/tamper monitor device. To be able do
+ * its job in a useful manner it runs a bigger statemachine to bring it into
+ * security/tamper failure state and once again to bring it out of this state.
+ *
+ * This unit can be in one of three states:
+ *
+ * - "NON-VALID STATE"
+ *   always after the battery power was removed
+ * - "FAILURE STATE"
+ *   if one of the enabled security events have happend
+ * - "VALID STATE"
+ *   if the unit works as expected
+ *
+ * Everything stops when the unit enters the failure state including the RTC
+ * counter (to be able to detect the time the security event happend).
+ *
+ * The following events (when enabled) let the DryIce unit enter the failure
+ * state:
+ *
+ * - wire-mesh-tamper detect
+ * - external tamper B detect
+ * - external tamper A detect
+ * - temperature tamper detect
+ * - clock tamper detect
+ * - voltage tamper detect
+ * - RTC counter overflow
+ * - monotonic counter overflow
+ * - external boot
+ *
+ * If we find the DryIce unit in "FAILURE STATE" and the TDCHL cleared, we
+ * can only detect this state. In this case the unit is completely locked and
+ * must force a second "SYSTEM POR" to bring the DryIce into the
+ * "NON-VALID STATE" + "FAILURE STATE" where a recovery is possible.
+ * If the TDCHL is set in the "FAILURE STATE" we are out of luck. In this case
+ * a battery power cycle is required.
+ *
+ * In the "NON-VALID STATE" + "FAILURE STATE" we can clear the "FAILURE STATE"
+ * and recover the DryIce unit. By clearing the "NON-VALID STATE" as the last
+ * task, we bring back this unit into life.
+ */
+
 /*
  * enable a dryice interrupt
  */
-- 
2.1.4

[PATCH 2/5] RTC/i.MX/DryIce: add the unit recovery code

[Juergen Borleis]

This code is requiered to recover the unit from a security violation.
Hopefully this code can recover the unit from a hardware related invalid
state as well.

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
Signed-off-by: Robert Schwebel <rsc@pengutronix.de>
[rsc: got NDA clearance from Freescale]
---
 drivers/rtc/rtc-imxdi.c | 333 ++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 295 insertions(+), 38 deletions(-)

diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
index 8750477..f8abf2b 100644
--- a/drivers/rtc/rtc-imxdi.c
+++ b/drivers/rtc/rtc-imxdi.c
@@ -172,6 +172,296 @@ struct imxdi_dev {
  * task, we bring back this unit into life.
  */
 
+/* do a write into the unit without interrupt support */
+static void di_write_busy_wait(const struct imxdi_dev *imxdi, u32 val,
+								unsigned reg)
+{
+	/* do the register write */
+	__raw_writel(val, imxdi->ioaddr + reg);
+
+	/*
+	 * now it takes four 32,768 kHz clock cycles to take
+	 * the change into effect = 122 us
+	 */
+	udelay(200);
+}
+
+static void di_report_tamper_info(struct imxdi_dev *imxdi,  u32 dsr)
+{
+	u32 dtcr;
+
+	dtcr = __raw_readl(imxdi->ioaddr + DTCR);
+
+	dev_emerg(&imxdi->pdev->dev, "!! DryIce tamper event detected !!\n");
+	/* the following flags force a transition into the "FAILURE STATE" */
+	if (dsr & DSR_VTD) {
+		dev_emerg(&imxdi->pdev->dev, "!! Voltage Tamper event\n");
+		if (!(dtcr & DTCR_VTE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But Voltage Tamper detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_CTD) {
+		dev_emerg(&imxdi->pdev->dev, "!! 32768 Hz Clock Tamper Event\n");
+		if (!(dtcr & DTCR_CTE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But Clock Tamper detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_TTD) {
+		dev_emerg(&imxdi->pdev->dev, "!! Temperature Tamper Event\n");
+		if (!(dtcr & DTCR_TTE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But Temperature Tamper detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_SAD) {
+		dev_emerg(&imxdi->pdev->dev, "!! Secure Controller Alarm Event\n");
+		if (!(dtcr & DTCR_SAIE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But Secure Controller Alarm detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_EBD) {
+		dev_emerg(&imxdi->pdev->dev, "!! External Boot Tamper Event\n");
+		if (!(dtcr & DTCR_EBE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But External Boot detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_ETAD) {
+		dev_emerg(&imxdi->pdev->dev, "!! External Tamper A Event\n");
+		if (!(dtcr & DTCR_ETAE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But External Tamper A wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_ETBD) {
+		dev_emerg(&imxdi->pdev->dev, "!! External Tamper B Event\n");
+		if (!(dtcr & DTCR_ETBE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But External Tamper B wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_WTD) {
+		dev_emerg(&imxdi->pdev->dev, "!! Wire-mesh Tamper Event\n");
+		if (!(dtcr & DTCR_WTE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But wire-mesh Tamper detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_MCO) {
+		dev_emerg(&imxdi->pdev->dev, "!! Monotonic-counter Overflow Event\n");
+		if (!(dtcr & DTCR_MOE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But Monotonic-counter Overflow detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_TCO) {
+		dev_emerg(&imxdi->pdev->dev, "!! Timer-counter Overflow Event\n");
+		if (!(dtcr & DTCR_TOE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But Timer-counter Overflow detection wasn't enabled. False positive?\n");
+	}
+}
+
+static void di_what_is_to_be_done(struct imxdi_dev *imxdi,
+						const char *power_supply)
+{
+	dev_emerg(&imxdi->pdev->dev, " Please cycle the %s power supply in order to get the DryIce unit working again\n",
+			power_supply);
+}
+
+static int di_handle_failure_state(struct imxdi_dev *imxdi, u32 dsr)
+{
+	u32 dcr;
+
+	dev_dbg(&imxdi->pdev->dev, "DSR register reports: %08X\n", dsr);
+
+	/* report the cause */
+	di_report_tamper_info(imxdi, dsr);
+
+	dcr = __raw_readl(imxdi->ioaddr + DCR);
+
+	if (dcr & DCR_FSHL) {
+		/* we are out of luck */
+		di_what_is_to_be_done(imxdi, "battery");
+		return -ENODEV;
+	}
+	/*
+	 * with the next SYSTEM POR we will transit from the "FAILURE STATE"
+	 * into the "NON-VALID STATE" + "FAILURE STATE"
+	 */
+	di_what_is_to_be_done(imxdi, "main");
+
+	return -ENODEV;
+}
+
+static int di_handle_valid_state(struct imxdi_dev *imxdi, u32 dsr)
+{
+	/* initialize alarm */
+	di_write_busy_wait(imxdi, DCAMR_UNSET, DCAMR);
+	di_write_busy_wait(imxdi, 0, DCALR);
+
+	/* clear alarm flag */
+	if (dsr & DSR_CAF)
+		di_write_busy_wait(imxdi, DSR_CAF, DSR);
+
+	return 0;
+}
+
+static int di_handle_invalid_state(struct imxdi_dev *imxdi, u32 dsr)
+{
+	u32 dcr, sec;
+
+	/*
+	 * lets disable all sources which can force the DryIce unit into
+	 * the "FAILURE STATE" for now
+	 */
+	di_write_busy_wait(imxdi, 0x00000000, DTCR);
+	/* and lets protect them at runtime from any change */
+	di_write_busy_wait(imxdi, DCR_TDCSL, DCR);
+
+	sec = __raw_readl(imxdi->ioaddr + DTCMR);
+	if (sec != 0)
+		dev_warn(&imxdi->pdev->dev,
+			"The security violation has happend at %u seconds\n",
+			sec);
+	/*
+	 * the timer cannot be set/modified if
+	 * - the TCHL or TCSL bit is set in DCR
+	 */
+	dcr = __raw_readl(imxdi->ioaddr + DCR);
+	if (!(dcr & DCR_TCE)) {
+		if (dcr & DCR_TCHL) {
+			/* we are out of luck */
+			di_what_is_to_be_done(imxdi, "battery");
+			return -ENODEV;
+		}
+		if (dcr & DCR_TCSL) {
+			di_what_is_to_be_done(imxdi, "main");
+			return -ENODEV;
+		}
+
+	}
+	/*
+	 * - the timer counter stops/is stopped if
+	 *   - its overflow flag is set (TCO in DSR)
+	 *      -> clear overflow bit to make it count again
+	 *   - NVF is set in DSR
+	 *      -> clear non-valid bit to make it count again
+	 *   - its TCE (DCR) is cleared
+	 *      -> set TCE to make it count
+	 *   - it was never set before
+	 *      -> write a time into it (required again if the NVF was set)
+	 */
+	/* state handled */
+	di_write_busy_wait(imxdi, DSR_NVF, DSR);
+	/* clear overflow flag */
+	di_write_busy_wait(imxdi, DSR_TCO, DSR);
+	/* enable the counter */
+	di_write_busy_wait(imxdi, dcr | DCR_TCE, DCR);
+	/* set and trigger it to make it count */
+	di_write_busy_wait(imxdi, sec, DTCMR);
+
+	/* now prepare for the valid state */
+	return di_handle_valid_state(imxdi, __raw_readl(imxdi->ioaddr + DSR));
+}
+
+static int di_handle_invalid_and_failure_state(struct imxdi_dev *imxdi, u32 dsr)
+{
+	u32 dcr;
+
+	/*
+	 * now we must first remove the tamper sources in order to get the
+	 * device out of the "FAILURE STATE"
+	 * To disable any of the following sources we need to modify the DTCR
+	 */
+	if (dsr & (DSR_WTD | DSR_ETBD | DSR_ETAD | DSR_EBD | DSR_SAD |
+			DSR_TTD | DSR_CTD | DSR_VTD | DSR_MCO | DSR_TCO)) {
+		dcr = __raw_readl(imxdi->ioaddr + DCR);
+		if (dcr & DCR_TDCHL) {
+			/*
+			 * the tamper register is locked. We cannot disable the
+			 * tamper detection. The TDCHL can only be reset by a
+			 * DRYICE POR, but we cannot force a DRYICE POR in
+			 * softwere because we are still in "FAILURE STATE".
+			 * We need a DRYICE POR via battery power cycling....
+			 */
+			/*
+			 * out of luck!
+			 * we cannot disable them without a DRYICE POR
+			 */
+			di_what_is_to_be_done(imxdi, "battery");
+			return -ENODEV;
+		}
+		if (dcr & DCR_TDCSL) {
+			/* a soft lock can be removed by a SYSTEM POR */
+			di_what_is_to_be_done(imxdi, "main");
+			return -ENODEV;
+		}
+	}
+
+	/* disable all sources */
+	di_write_busy_wait(imxdi, 0x00000000, DTCR);
+
+	/* clear the status bits now */
+	di_write_busy_wait(imxdi, dsr & (DSR_WTD | DSR_ETBD | DSR_ETAD |
+			DSR_EBD | DSR_SAD | DSR_TTD | DSR_CTD | DSR_VTD |
+			DSR_MCO | DSR_TCO), DSR);
+
+	dsr = __raw_readl(imxdi->ioaddr + DSR);
+	if ((dsr & ~(DSR_NVF | DSR_SVF | DSR_WBF | DSR_WNF |
+			DSR_WCF | DSR_WEF)) != 0)
+		dev_warn(&imxdi->pdev->dev,
+			"There are still some sources of pain in DSR: %08x!\n",
+			dsr & ~(DSR_NVF | DSR_SVF | DSR_WBF | DSR_WNF |
+					DSR_WCF | DSR_WEF));
+
+	/*
+	 * now we are trying to clear the "Security-violation flag" to
+	 * get the DryIce out of this state
+	 */
+	di_write_busy_wait(imxdi, DSR_SVF, DSR);
+
+	/* success? */
+	dsr = __raw_readl(imxdi->ioaddr + DSR);
+	if (dsr & DSR_SVF) {
+		dev_crit(&imxdi->pdev->dev,
+			"Cannot clear the security violation flag. We are ending up in an endless loop!\n");
+		/* last resourt */
+		di_what_is_to_be_done(imxdi, "battery");
+		return -ENODEV;
+	}
+
+	/*
+	 * now we have left the "FAILURE STATE" and ending up in the
+	 * "NON-VALID STATE" time to recover everything
+	 */
+	return di_handle_invalid_state(imxdi, dsr);
+}
+
+static int di_handle_state(struct imxdi_dev *imxdi)
+{
+	int rc;
+	u32 dsr;
+
+	dsr = __raw_readl(imxdi->ioaddr + DSR);
+
+	switch (dsr & (DSR_NVF | DSR_SVF)) {
+	case DSR_NVF:
+		dev_warn(&imxdi->pdev->dev, "Invalid stated unit detected\n");
+		rc = di_handle_invalid_state(imxdi, dsr);
+		break;
+	case DSR_SVF:
+		dev_warn(&imxdi->pdev->dev, "Failure stated unit detected\n");
+		rc = di_handle_failure_state(imxdi, dsr);
+		break;
+	case DSR_NVF | DSR_SVF:
+		dev_warn(&imxdi->pdev->dev,
+				"Failure+Invalid stated unit detected\n");
+		rc = di_handle_invalid_and_failure_state(imxdi, dsr);
+		break;
+	default:
+		dev_notice(&imxdi->pdev->dev, "Unlocked unit detected\n");
+		rc = di_handle_valid_state(imxdi, dsr);
+	}
+
+	return rc;
+}
+
+
 /*
  * enable a dryice interrupt
  */
@@ -491,6 +781,11 @@ static int __init dryice_rtc_probe(struct platform_device *pdev)
 	/* mask all interrupts */
 	__raw_writel(0, imxdi->ioaddr + DIER);
 
+	rc = di_handle_state(imxdi);
+	if (rc != 0)
+		goto err;
+
+
 	rc = devm_request_irq(&pdev->dev, imxdi->irq, dryice_norm_irq,
 			IRQF_SHARED, pdev->name, imxdi);
 	if (rc) {
@@ -498,44 +793,6 @@ static int __init dryice_rtc_probe(struct platform_device *pdev)
 		goto err;
 	}
 
-	/* put dryice into valid state */
-	if (__raw_readl(imxdi->ioaddr + DSR) & DSR_NVF) {
-		rc = di_write_wait(imxdi, DSR_NVF | DSR_SVF, DSR);
-		if (rc)
-			goto err;
-	}
-
-	/* initialize alarm */
-	rc = di_write_wait(imxdi, DCAMR_UNSET, DCAMR);
-	if (rc)
-		goto err;
-	rc = di_write_wait(imxdi, 0, DCALR);
-	if (rc)
-		goto err;
-
-	/* clear alarm flag */
-	if (__raw_readl(imxdi->ioaddr + DSR) & DSR_CAF) {
-		rc = di_write_wait(imxdi, DSR_CAF, DSR);
-		if (rc)
-			goto err;
-	}
-
-	/* the timer won't count if it has never been written to */
-	if (__raw_readl(imxdi->ioaddr + DTCMR) == 0) {
-		rc = di_write_wait(imxdi, 0, DTCMR);
-		if (rc)
-			goto err;
-	}
-
-	/* start keeping time */
-	if (!(__raw_readl(imxdi->ioaddr + DCR) & DCR_TCE)) {
-		rc = di_write_wait(imxdi,
-				__raw_readl(imxdi->ioaddr + DCR) | DCR_TCE,
-				DCR);
-		if (rc)
-			goto err;
-	}
-
 	platform_set_drvdata(pdev, imxdi);
 	imxdi->rtc = devm_rtc_device_register(&pdev->dev, pdev->name,
 				  &dryice_rtc_ops, THIS_MODULE);
-- 
2.1.4

[rtc-linux] [PATCH 2/5] RTC/i.MX/DryIce: add the unit recovery code

[Juergen Borleis]

This code is requiered to recover the unit from a security violation.
Hopefully this code can recover the unit from a hardware related invalid
state as well.

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
Signed-off-by: Robert Schwebel <rsc@pengutronix.de>
[rsc: got NDA clearance from Freescale]
---
 drivers/rtc/rtc-imxdi.c | 333 ++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 295 insertions(+), 38 deletions(-)

diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
index 8750477..f8abf2b 100644
--- a/drivers/rtc/rtc-imxdi.c
+++ b/drivers/rtc/rtc-imxdi.c
@@ -172,6 +172,296 @@ struct imxdi_dev {
  * task, we bring back this unit into life.
  */
 
+/* do a write into the unit without interrupt support */
+static void di_write_busy_wait(const struct imxdi_dev *imxdi, u32 val,
+								unsigned reg)
+{
+	/* do the register write */
+	__raw_writel(val, imxdi->ioaddr + reg);
+
+	/*
+	 * now it takes four 32,768 kHz clock cycles to take
+	 * the change into effect = 122 us
+	 */
+	udelay(200);
+}
+
+static void di_report_tamper_info(struct imxdi_dev *imxdi,  u32 dsr)
+{
+	u32 dtcr;
+
+	dtcr = __raw_readl(imxdi->ioaddr + DTCR);
+
+	dev_emerg(&imxdi->pdev->dev, "!! DryIce tamper event detected !!\n");
+	/* the following flags force a transition into the "FAILURE STATE" */
+	if (dsr & DSR_VTD) {
+		dev_emerg(&imxdi->pdev->dev, "!! Voltage Tamper event\n");
+		if (!(dtcr & DTCR_VTE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But Voltage Tamper detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_CTD) {
+		dev_emerg(&imxdi->pdev->dev, "!! 32768 Hz Clock Tamper Event\n");
+		if (!(dtcr & DTCR_CTE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But Clock Tamper detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_TTD) {
+		dev_emerg(&imxdi->pdev->dev, "!! Temperature Tamper Event\n");
+		if (!(dtcr & DTCR_TTE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But Temperature Tamper detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_SAD) {
+		dev_emerg(&imxdi->pdev->dev, "!! Secure Controller Alarm Event\n");
+		if (!(dtcr & DTCR_SAIE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But Secure Controller Alarm detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_EBD) {
+		dev_emerg(&imxdi->pdev->dev, "!! External Boot Tamper Event\n");
+		if (!(dtcr & DTCR_EBE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But External Boot detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_ETAD) {
+		dev_emerg(&imxdi->pdev->dev, "!! External Tamper A Event\n");
+		if (!(dtcr & DTCR_ETAE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But External Tamper A wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_ETBD) {
+		dev_emerg(&imxdi->pdev->dev, "!! External Tamper B Event\n");
+		if (!(dtcr & DTCR_ETBE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But External Tamper B wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_WTD) {
+		dev_emerg(&imxdi->pdev->dev, "!! Wire-mesh Tamper Event\n");
+		if (!(dtcr & DTCR_WTE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But wire-mesh Tamper detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_MCO) {
+		dev_emerg(&imxdi->pdev->dev, "!! Monotonic-counter Overflow Event\n");
+		if (!(dtcr & DTCR_MOE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But Monotonic-counter Overflow detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_TCO) {
+		dev_emerg(&imxdi->pdev->dev, "!! Timer-counter Overflow Event\n");
+		if (!(dtcr & DTCR_TOE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But Timer-counter Overflow detection wasn't enabled. False positive?\n");
+	}
+}
+
+static void di_what_is_to_be_done(struct imxdi_dev *imxdi,
+						const char *power_supply)
+{
+	dev_emerg(&imxdi->pdev->dev, " Please cycle the %s power supply in order to get the DryIce unit working again\n",
+			power_supply);
+}
+
+static int di_handle_failure_state(struct imxdi_dev *imxdi, u32 dsr)
+{
+	u32 dcr;
+
+	dev_dbg(&imxdi->pdev->dev, "DSR register reports: %08X\n", dsr);
+
+	/* report the cause */
+	di_report_tamper_info(imxdi, dsr);
+
+	dcr = __raw_readl(imxdi->ioaddr + DCR);
+
+	if (dcr & DCR_FSHL) {
+		/* we are out of luck */
+		di_what_is_to_be_done(imxdi, "battery");
+		return -ENODEV;
+	}
+	/*
+	 * with the next SYSTEM POR we will transit from the "FAILURE STATE"
+	 * into the "NON-VALID STATE" + "FAILURE STATE"
+	 */
+	di_what_is_to_be_done(imxdi, "main");
+
+	return -ENODEV;
+}
+
+static int di_handle_valid_state(struct imxdi_dev *imxdi, u32 dsr)
+{
+	/* initialize alarm */
+	di_write_busy_wait(imxdi, DCAMR_UNSET, DCAMR);
+	di_write_busy_wait(imxdi, 0, DCALR);
+
+	/* clear alarm flag */
+	if (dsr & DSR_CAF)
+		di_write_busy_wait(imxdi, DSR_CAF, DSR);
+
+	return 0;
+}
+
+static int di_handle_invalid_state(struct imxdi_dev *imxdi, u32 dsr)
+{
+	u32 dcr, sec;
+
+	/*
+	 * lets disable all sources which can force the DryIce unit into
+	 * the "FAILURE STATE" for now
+	 */
+	di_write_busy_wait(imxdi, 0x00000000, DTCR);
+	/* and lets protect them at runtime from any change */
+	di_write_busy_wait(imxdi, DCR_TDCSL, DCR);
+
+	sec = __raw_readl(imxdi->ioaddr + DTCMR);
+	if (sec != 0)
+		dev_warn(&imxdi->pdev->dev,
+			"The security violation has happend at %u seconds\n",
+			sec);
+	/*
+	 * the timer cannot be set/modified if
+	 * - the TCHL or TCSL bit is set in DCR
+	 */
+	dcr = __raw_readl(imxdi->ioaddr + DCR);
+	if (!(dcr & DCR_TCE)) {
+		if (dcr & DCR_TCHL) {
+			/* we are out of luck */
+			di_what_is_to_be_done(imxdi, "battery");
+			return -ENODEV;
+		}
+		if (dcr & DCR_TCSL) {
+			di_what_is_to_be_done(imxdi, "main");
+			return -ENODEV;
+		}
+
+	}
+	/*
+	 * - the timer counter stops/is stopped if
+	 *   - its overflow flag is set (TCO in DSR)
+	 *      -> clear overflow bit to make it count again
+	 *   - NVF is set in DSR
+	 *      -> clear non-valid bit to make it count again
+	 *   - its TCE (DCR) is cleared
+	 *      -> set TCE to make it count
+	 *   - it was never set before
+	 *      -> write a time into it (required again if the NVF was set)
+	 */
+	/* state handled */
+	di_write_busy_wait(imxdi, DSR_NVF, DSR);
+	/* clear overflow flag */
+	di_write_busy_wait(imxdi, DSR_TCO, DSR);
+	/* enable the counter */
+	di_write_busy_wait(imxdi, dcr | DCR_TCE, DCR);
+	/* set and trigger it to make it count */
+	di_write_busy_wait(imxdi, sec, DTCMR);
+
+	/* now prepare for the valid state */
+	return di_handle_valid_state(imxdi, __raw_readl(imxdi->ioaddr + DSR));
+}
+
+static int di_handle_invalid_and_failure_state(struct imxdi_dev *imxdi, u32 dsr)
+{
+	u32 dcr;
+
+	/*
+	 * now we must first remove the tamper sources in order to get the
+	 * device out of the "FAILURE STATE"
+	 * To disable any of the following sources we need to modify the DTCR
+	 */
+	if (dsr & (DSR_WTD | DSR_ETBD | DSR_ETAD | DSR_EBD | DSR_SAD |
+			DSR_TTD | DSR_CTD | DSR_VTD | DSR_MCO | DSR_TCO)) {
+		dcr = __raw_readl(imxdi->ioaddr + DCR);
+		if (dcr & DCR_TDCHL) {
+			/*
+			 * the tamper register is locked. We cannot disable the
+			 * tamper detection. The TDCHL can only be reset by a
+			 * DRYICE POR, but we cannot force a DRYICE POR in
+			 * softwere because we are still in "FAILURE STATE".
+			 * We need a DRYICE POR via battery power cycling....
+			 */
+			/*
+			 * out of luck!
+			 * we cannot disable them without a DRYICE POR
+			 */
+			di_what_is_to_be_done(imxdi, "battery");
+			return -ENODEV;
+		}
+		if (dcr & DCR_TDCSL) {
+			/* a soft lock can be removed by a SYSTEM POR */
+			di_what_is_to_be_done(imxdi, "main");
+			return -ENODEV;
+		}
+	}
+
+	/* disable all sources */
+	di_write_busy_wait(imxdi, 0x00000000, DTCR);
+
+	/* clear the status bits now */
+	di_write_busy_wait(imxdi, dsr & (DSR_WTD | DSR_ETBD | DSR_ETAD |
+			DSR_EBD | DSR_SAD | DSR_TTD | DSR_CTD | DSR_VTD |
+			DSR_MCO | DSR_TCO), DSR);
+
+	dsr = __raw_readl(imxdi->ioaddr + DSR);
+	if ((dsr & ~(DSR_NVF | DSR_SVF | DSR_WBF | DSR_WNF |
+			DSR_WCF | DSR_WEF)) != 0)
+		dev_warn(&imxdi->pdev->dev,
+			"There are still some sources of pain in DSR: %08x!\n",
+			dsr & ~(DSR_NVF | DSR_SVF | DSR_WBF | DSR_WNF |
+					DSR_WCF | DSR_WEF));
+
+	/*
+	 * now we are trying to clear the "Security-violation flag" to
+	 * get the DryIce out of this state
+	 */
+	di_write_busy_wait(imxdi, DSR_SVF, DSR);
+
+	/* success? */
+	dsr = __raw_readl(imxdi->ioaddr + DSR);
+	if (dsr & DSR_SVF) {
+		dev_crit(&imxdi->pdev->dev,
+			"Cannot clear the security violation flag. We are ending up in an endless loop!\n");
+		/* last resourt */
+		di_what_is_to_be_done(imxdi, "battery");
+		return -ENODEV;
+	}
+
+	/*
+	 * now we have left the "FAILURE STATE" and ending up in the
+	 * "NON-VALID STATE" time to recover everything
+	 */
+	return di_handle_invalid_state(imxdi, dsr);
+}
+
+static int di_handle_state(struct imxdi_dev *imxdi)
+{
+	int rc;
+	u32 dsr;
+
+	dsr = __raw_readl(imxdi->ioaddr + DSR);
+
+	switch (dsr & (DSR_NVF | DSR_SVF)) {
+	case DSR_NVF:
+		dev_warn(&imxdi->pdev->dev, "Invalid stated unit detected\n");
+		rc = di_handle_invalid_state(imxdi, dsr);
+		break;
+	case DSR_SVF:
+		dev_warn(&imxdi->pdev->dev, "Failure stated unit detected\n");
+		rc = di_handle_failure_state(imxdi, dsr);
+		break;
+	case DSR_NVF | DSR_SVF:
+		dev_warn(&imxdi->pdev->dev,
+				"Failure+Invalid stated unit detected\n");
+		rc = di_handle_invalid_and_failure_state(imxdi, dsr);
+		break;
+	default:
+		dev_notice(&imxdi->pdev->dev, "Unlocked unit detected\n");
+		rc = di_handle_valid_state(imxdi, dsr);
+	}
+
+	return rc;
+}
+
+
 /*
  * enable a dryice interrupt
  */
@@ -491,6 +781,11 @@ static int __init dryice_rtc_probe(struct platform_device *pdev)
 	/* mask all interrupts */
 	__raw_writel(0, imxdi->ioaddr + DIER);
 
+	rc = di_handle_state(imxdi);
+	if (rc != 0)
+		goto err;
+
+
 	rc = devm_request_irq(&pdev->dev, imxdi->irq, dryice_norm_irq,
 			IRQF_SHARED, pdev->name, imxdi);
 	if (rc) {
@@ -498,44 +793,6 @@ static int __init dryice_rtc_probe(struct platform_device *pdev)
 		goto err;
 	}
 
-	/* put dryice into valid state */
-	if (__raw_readl(imxdi->ioaddr + DSR) & DSR_NVF) {
-		rc = di_write_wait(imxdi, DSR_NVF | DSR_SVF, DSR);
-		if (rc)
-			goto err;
-	}
-
-	/* initialize alarm */
-	rc = di_write_wait(imxdi, DCAMR_UNSET, DCAMR);
-	if (rc)
-		goto err;
-	rc = di_write_wait(imxdi, 0, DCALR);
-	if (rc)
-		goto err;
-
-	/* clear alarm flag */
-	if (__raw_readl(imxdi->ioaddr + DSR) & DSR_CAF) {
-		rc = di_write_wait(imxdi, DSR_CAF, DSR);
-		if (rc)
-			goto err;
-	}
-
-	/* the timer won't count if it has never been written to */
-	if (__raw_readl(imxdi->ioaddr + DTCMR) == 0) {
-		rc = di_write_wait(imxdi, 0, DTCMR);
-		if (rc)
-			goto err;
-	}
-
-	/* start keeping time */
-	if (!(__raw_readl(imxdi->ioaddr + DCR) & DCR_TCE)) {
-		rc = di_write_wait(imxdi,
-				__raw_readl(imxdi->ioaddr + DCR) | DCR_TCE,
-				DCR);
-		if (rc)
-			goto err;
-	}
-
 	platform_set_drvdata(pdev, imxdi);
 	imxdi->rtc = devm_rtc_device_register(&pdev->dev, pdev->name,
 				  &dryice_rtc_ops, THIS_MODULE);
-- 
2.1.4

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH 2/5] RTC/i.MX/DryIce: add the unit recovery code

[Juergen Borleis]

This code is requiered to recover the unit from a security violation.
Hopefully this code can recover the unit from a hardware related invalid
state as well.

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
Signed-off-by: Robert Schwebel <rsc@pengutronix.de>
[rsc: got NDA clearance from Freescale]
---
 drivers/rtc/rtc-imxdi.c | 333 ++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 295 insertions(+), 38 deletions(-)

diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
index 8750477..f8abf2b 100644
--- a/drivers/rtc/rtc-imxdi.c
+++ b/drivers/rtc/rtc-imxdi.c
@@ -172,6 +172,296 @@ struct imxdi_dev {
  * task, we bring back this unit into life.
  */
 
+/* do a write into the unit without interrupt support */
+static void di_write_busy_wait(const struct imxdi_dev *imxdi, u32 val,
+								unsigned reg)
+{
+	/* do the register write */
+	__raw_writel(val, imxdi->ioaddr + reg);
+
+	/*
+	 * now it takes four 32,768 kHz clock cycles to take
+	 * the change into effect = 122 us
+	 */
+	udelay(200);
+}
+
+static void di_report_tamper_info(struct imxdi_dev *imxdi,  u32 dsr)
+{
+	u32 dtcr;
+
+	dtcr = __raw_readl(imxdi->ioaddr + DTCR);
+
+	dev_emerg(&imxdi->pdev->dev, "!! DryIce tamper event detected !!\n");
+	/* the following flags force a transition into the "FAILURE STATE" */
+	if (dsr & DSR_VTD) {
+		dev_emerg(&imxdi->pdev->dev, "!! Voltage Tamper event\n");
+		if (!(dtcr & DTCR_VTE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But Voltage Tamper detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_CTD) {
+		dev_emerg(&imxdi->pdev->dev, "!! 32768 Hz Clock Tamper Event\n");
+		if (!(dtcr & DTCR_CTE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But Clock Tamper detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_TTD) {
+		dev_emerg(&imxdi->pdev->dev, "!! Temperature Tamper Event\n");
+		if (!(dtcr & DTCR_TTE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But Temperature Tamper detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_SAD) {
+		dev_emerg(&imxdi->pdev->dev, "!! Secure Controller Alarm Event\n");
+		if (!(dtcr & DTCR_SAIE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But Secure Controller Alarm detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_EBD) {
+		dev_emerg(&imxdi->pdev->dev, "!! External Boot Tamper Event\n");
+		if (!(dtcr & DTCR_EBE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But External Boot detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_ETAD) {
+		dev_emerg(&imxdi->pdev->dev, "!! External Tamper A Event\n");
+		if (!(dtcr & DTCR_ETAE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But External Tamper A wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_ETBD) {
+		dev_emerg(&imxdi->pdev->dev, "!! External Tamper B Event\n");
+		if (!(dtcr & DTCR_ETBE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But External Tamper B wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_WTD) {
+		dev_emerg(&imxdi->pdev->dev, "!! Wire-mesh Tamper Event\n");
+		if (!(dtcr & DTCR_WTE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But wire-mesh Tamper detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_MCO) {
+		dev_emerg(&imxdi->pdev->dev, "!! Monotonic-counter Overflow Event\n");
+		if (!(dtcr & DTCR_MOE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But Monotonic-counter Overflow detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_TCO) {
+		dev_emerg(&imxdi->pdev->dev, "!! Timer-counter Overflow Event\n");
+		if (!(dtcr & DTCR_TOE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But Timer-counter Overflow detection wasn't enabled. False positive?\n");
+	}
+}
+
+static void di_what_is_to_be_done(struct imxdi_dev *imxdi,
+						const char *power_supply)
+{
+	dev_emerg(&imxdi->pdev->dev, " Please cycle the %s power supply in order to get the DryIce unit working again\n",
+			power_supply);
+}
+
+static int di_handle_failure_state(struct imxdi_dev *imxdi, u32 dsr)
+{
+	u32 dcr;
+
+	dev_dbg(&imxdi->pdev->dev, "DSR register reports: %08X\n", dsr);
+
+	/* report the cause */
+	di_report_tamper_info(imxdi, dsr);
+
+	dcr = __raw_readl(imxdi->ioaddr + DCR);
+
+	if (dcr & DCR_FSHL) {
+		/* we are out of luck */
+		di_what_is_to_be_done(imxdi, "battery");
+		return -ENODEV;
+	}
+	/*
+	 * with the next SYSTEM POR we will transit from the "FAILURE STATE"
+	 * into the "NON-VALID STATE" + "FAILURE STATE"
+	 */
+	di_what_is_to_be_done(imxdi, "main");
+
+	return -ENODEV;
+}
+
+static int di_handle_valid_state(struct imxdi_dev *imxdi, u32 dsr)
+{
+	/* initialize alarm */
+	di_write_busy_wait(imxdi, DCAMR_UNSET, DCAMR);
+	di_write_busy_wait(imxdi, 0, DCALR);
+
+	/* clear alarm flag */
+	if (dsr & DSR_CAF)
+		di_write_busy_wait(imxdi, DSR_CAF, DSR);
+
+	return 0;
+}
+
+static int di_handle_invalid_state(struct imxdi_dev *imxdi, u32 dsr)
+{
+	u32 dcr, sec;
+
+	/*
+	 * lets disable all sources which can force the DryIce unit into
+	 * the "FAILURE STATE" for now
+	 */
+	di_write_busy_wait(imxdi, 0x00000000, DTCR);
+	/* and lets protect them at runtime from any change */
+	di_write_busy_wait(imxdi, DCR_TDCSL, DCR);
+
+	sec = __raw_readl(imxdi->ioaddr + DTCMR);
+	if (sec != 0)
+		dev_warn(&imxdi->pdev->dev,
+			"The security violation has happend at %u seconds\n",
+			sec);
+	/*
+	 * the timer cannot be set/modified if
+	 * - the TCHL or TCSL bit is set in DCR
+	 */
+	dcr = __raw_readl(imxdi->ioaddr + DCR);
+	if (!(dcr & DCR_TCE)) {
+		if (dcr & DCR_TCHL) {
+			/* we are out of luck */
+			di_what_is_to_be_done(imxdi, "battery");
+			return -ENODEV;
+		}
+		if (dcr & DCR_TCSL) {
+			di_what_is_to_be_done(imxdi, "main");
+			return -ENODEV;
+		}
+
+	}
+	/*
+	 * - the timer counter stops/is stopped if
+	 *   - its overflow flag is set (TCO in DSR)
+	 *      -> clear overflow bit to make it count again
+	 *   - NVF is set in DSR
+	 *      -> clear non-valid bit to make it count again
+	 *   - its TCE (DCR) is cleared
+	 *      -> set TCE to make it count
+	 *   - it was never set before
+	 *      -> write a time into it (required again if the NVF was set)
+	 */
+	/* state handled */
+	di_write_busy_wait(imxdi, DSR_NVF, DSR);
+	/* clear overflow flag */
+	di_write_busy_wait(imxdi, DSR_TCO, DSR);
+	/* enable the counter */
+	di_write_busy_wait(imxdi, dcr | DCR_TCE, DCR);
+	/* set and trigger it to make it count */
+	di_write_busy_wait(imxdi, sec, DTCMR);
+
+	/* now prepare for the valid state */
+	return di_handle_valid_state(imxdi, __raw_readl(imxdi->ioaddr + DSR));
+}
+
+static int di_handle_invalid_and_failure_state(struct imxdi_dev *imxdi, u32 dsr)
+{
+	u32 dcr;
+
+	/*
+	 * now we must first remove the tamper sources in order to get the
+	 * device out of the "FAILURE STATE"
+	 * To disable any of the following sources we need to modify the DTCR
+	 */
+	if (dsr & (DSR_WTD | DSR_ETBD | DSR_ETAD | DSR_EBD | DSR_SAD |
+			DSR_TTD | DSR_CTD | DSR_VTD | DSR_MCO | DSR_TCO)) {
+		dcr = __raw_readl(imxdi->ioaddr + DCR);
+		if (dcr & DCR_TDCHL) {
+			/*
+			 * the tamper register is locked. We cannot disable the
+			 * tamper detection. The TDCHL can only be reset by a
+			 * DRYICE POR, but we cannot force a DRYICE POR in
+			 * softwere because we are still in "FAILURE STATE".
+			 * We need a DRYICE POR via battery power cycling....
+			 */
+			/*
+			 * out of luck!
+			 * we cannot disable them without a DRYICE POR
+			 */
+			di_what_is_to_be_done(imxdi, "battery");
+			return -ENODEV;
+		}
+		if (dcr & DCR_TDCSL) {
+			/* a soft lock can be removed by a SYSTEM POR */
+			di_what_is_to_be_done(imxdi, "main");
+			return -ENODEV;
+		}
+	}
+
+	/* disable all sources */
+	di_write_busy_wait(imxdi, 0x00000000, DTCR);
+
+	/* clear the status bits now */
+	di_write_busy_wait(imxdi, dsr & (DSR_WTD | DSR_ETBD | DSR_ETAD |
+			DSR_EBD | DSR_SAD | DSR_TTD | DSR_CTD | DSR_VTD |
+			DSR_MCO | DSR_TCO), DSR);
+
+	dsr = __raw_readl(imxdi->ioaddr + DSR);
+	if ((dsr & ~(DSR_NVF | DSR_SVF | DSR_WBF | DSR_WNF |
+			DSR_WCF | DSR_WEF)) != 0)
+		dev_warn(&imxdi->pdev->dev,
+			"There are still some sources of pain in DSR: %08x!\n",
+			dsr & ~(DSR_NVF | DSR_SVF | DSR_WBF | DSR_WNF |
+					DSR_WCF | DSR_WEF));
+
+	/*
+	 * now we are trying to clear the "Security-violation flag" to
+	 * get the DryIce out of this state
+	 */
+	di_write_busy_wait(imxdi, DSR_SVF, DSR);
+
+	/* success? */
+	dsr = __raw_readl(imxdi->ioaddr + DSR);
+	if (dsr & DSR_SVF) {
+		dev_crit(&imxdi->pdev->dev,
+			"Cannot clear the security violation flag. We are ending up in an endless loop!\n");
+		/* last resourt */
+		di_what_is_to_be_done(imxdi, "battery");
+		return -ENODEV;
+	}
+
+	/*
+	 * now we have left the "FAILURE STATE" and ending up in the
+	 * "NON-VALID STATE" time to recover everything
+	 */
+	return di_handle_invalid_state(imxdi, dsr);
+}
+
+static int di_handle_state(struct imxdi_dev *imxdi)
+{
+	int rc;
+	u32 dsr;
+
+	dsr = __raw_readl(imxdi->ioaddr + DSR);
+
+	switch (dsr & (DSR_NVF | DSR_SVF)) {
+	case DSR_NVF:
+		dev_warn(&imxdi->pdev->dev, "Invalid stated unit detected\n");
+		rc = di_handle_invalid_state(imxdi, dsr);
+		break;
+	case DSR_SVF:
+		dev_warn(&imxdi->pdev->dev, "Failure stated unit detected\n");
+		rc = di_handle_failure_state(imxdi, dsr);
+		break;
+	case DSR_NVF | DSR_SVF:
+		dev_warn(&imxdi->pdev->dev,
+				"Failure+Invalid stated unit detected\n");
+		rc = di_handle_invalid_and_failure_state(imxdi, dsr);
+		break;
+	default:
+		dev_notice(&imxdi->pdev->dev, "Unlocked unit detected\n");
+		rc = di_handle_valid_state(imxdi, dsr);
+	}
+
+	return rc;
+}
+
+
 /*
  * enable a dryice interrupt
  */
@@ -491,6 +781,11 @@ static int __init dryice_rtc_probe(struct platform_device *pdev)
 	/* mask all interrupts */
 	__raw_writel(0, imxdi->ioaddr + DIER);
 
+	rc = di_handle_state(imxdi);
+	if (rc != 0)
+		goto err;
+
+
 	rc = devm_request_irq(&pdev->dev, imxdi->irq, dryice_norm_irq,
 			IRQF_SHARED, pdev->name, imxdi);
 	if (rc) {
@@ -498,44 +793,6 @@ static int __init dryice_rtc_probe(struct platform_device *pdev)
 		goto err;
 	}
 
-	/* put dryice into valid state */
-	if (__raw_readl(imxdi->ioaddr + DSR) & DSR_NVF) {
-		rc = di_write_wait(imxdi, DSR_NVF | DSR_SVF, DSR);
-		if (rc)
-			goto err;
-	}
-
-	/* initialize alarm */
-	rc = di_write_wait(imxdi, DCAMR_UNSET, DCAMR);
-	if (rc)
-		goto err;
-	rc = di_write_wait(imxdi, 0, DCALR);
-	if (rc)
-		goto err;
-
-	/* clear alarm flag */
-	if (__raw_readl(imxdi->ioaddr + DSR) & DSR_CAF) {
-		rc = di_write_wait(imxdi, DSR_CAF, DSR);
-		if (rc)
-			goto err;
-	}
-
-	/* the timer won't count if it has never been written to */
-	if (__raw_readl(imxdi->ioaddr + DTCMR) == 0) {
-		rc = di_write_wait(imxdi, 0, DTCMR);
-		if (rc)
-			goto err;
-	}
-
-	/* start keeping time */
-	if (!(__raw_readl(imxdi->ioaddr + DCR) & DCR_TCE)) {
-		rc = di_write_wait(imxdi,
-				__raw_readl(imxdi->ioaddr + DCR) | DCR_TCE,
-				DCR);
-		if (rc)
-			goto err;
-	}
-
 	platform_set_drvdata(pdev, imxdi);
 	imxdi->rtc = devm_rtc_device_register(&pdev->dev, pdev->name,
 				  &dryice_rtc_ops, THIS_MODULE);
-- 
2.1.4

[PATCH 3/5] RTC/i.MX/DryIce: monitor a security violation at runtime

[Juergen Borleis]

Maybe the unit enters the hardware related state at runtime and not at
system boot time (after a power cycle).

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
Signed-off-by: Robert Schwebel <rsc@pengutronix.de>
[rsc: got NDA clearance from Freescale]
---
 drivers/rtc/rtc-imxdi.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
index f8abf2b..b04c64f 100644
--- a/drivers/rtc/rtc-imxdi.c
+++ b/drivers/rtc/rtc-imxdi.c
@@ -680,6 +680,25 @@ static irqreturn_t dryice_norm_irq(int irq, void *dev_id)
 	irqreturn_t rc = IRQ_NONE;
 
 	dier = __raw_readl(imxdi->ioaddr + DIER);
+	dsr = __raw_readl(imxdi->ioaddr + DSR);
+
+	/* handle the security violation event */
+	if (dier & DIER_SVIE) {
+		if (dsr & DSR_SVF) {
+			/*
+			 * Disable the interrupt when this kind of event has
+			 * happened.
+			 * There cannot be more than one event of this type,
+			 * because it needs a complex state change
+			 * including a main power cycle to get again out of
+			 * this state.
+			 */
+			di_int_disable(imxdi, DIER_SVIE);
+			/* report the violation */
+			di_report_tamper_info(imxdi, dsr);
+			rc = IRQ_HANDLED;
+		}
+	}
 
 	/* handle write complete and write error cases */
 	if (dier & DIER_WCIE) {
@@ -690,7 +709,6 @@ static irqreturn_t dryice_norm_irq(int irq, void *dev_id)
 			return rc;
 
 		/* DSR_WCF clears itself on DSR read */
-		dsr = __raw_readl(imxdi->ioaddr + DSR);
 		if (dsr & (DSR_WCF | DSR_WEF)) {
 			/* mask the interrupt */
 			di_int_disable(imxdi, DIER_WCIE);
@@ -706,7 +724,6 @@ static irqreturn_t dryice_norm_irq(int irq, void *dev_id)
 	/* handle the alarm case */
 	if (dier & DIER_CAIE) {
 		/* DSR_WCF clears itself on DSR read */
-		dsr = __raw_readl(imxdi->ioaddr + DSR);
 		if (dsr & DSR_CAF) {
 			/* mask the interrupt */
 			di_int_disable(imxdi, DIER_CAIE);
-- 
2.1.4

[rtc-linux] [PATCH 3/5] RTC/i.MX/DryIce: monitor a security violation at runtime

[Juergen Borleis]

Maybe the unit enters the hardware related state at runtime and not at
system boot time (after a power cycle).

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
Signed-off-by: Robert Schwebel <rsc@pengutronix.de>
[rsc: got NDA clearance from Freescale]
---
 drivers/rtc/rtc-imxdi.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
index f8abf2b..b04c64f 100644
--- a/drivers/rtc/rtc-imxdi.c
+++ b/drivers/rtc/rtc-imxdi.c
@@ -680,6 +680,25 @@ static irqreturn_t dryice_norm_irq(int irq, void *dev_id)
 	irqreturn_t rc = IRQ_NONE;
 
 	dier = __raw_readl(imxdi->ioaddr + DIER);
+	dsr = __raw_readl(imxdi->ioaddr + DSR);
+
+	/* handle the security violation event */
+	if (dier & DIER_SVIE) {
+		if (dsr & DSR_SVF) {
+			/*
+			 * Disable the interrupt when this kind of event has
+			 * happened.
+			 * There cannot be more than one event of this type,
+			 * because it needs a complex state change
+			 * including a main power cycle to get again out of
+			 * this state.
+			 */
+			di_int_disable(imxdi, DIER_SVIE);
+			/* report the violation */
+			di_report_tamper_info(imxdi, dsr);
+			rc = IRQ_HANDLED;
+		}
+	}
 
 	/* handle write complete and write error cases */
 	if (dier & DIER_WCIE) {
@@ -690,7 +709,6 @@ static irqreturn_t dryice_norm_irq(int irq, void *dev_id)
 			return rc;
 
 		/* DSR_WCF clears itself on DSR read */
-		dsr = __raw_readl(imxdi->ioaddr + DSR);
 		if (dsr & (DSR_WCF | DSR_WEF)) {
 			/* mask the interrupt */
 			di_int_disable(imxdi, DIER_WCIE);
@@ -706,7 +724,6 @@ static irqreturn_t dryice_norm_irq(int irq, void *dev_id)
 	/* handle the alarm case */
 	if (dier & DIER_CAIE) {
 		/* DSR_WCF clears itself on DSR read */
-		dsr = __raw_readl(imxdi->ioaddr + DSR);
 		if (dsr & DSR_CAF) {
 			/* mask the interrupt */
 			di_int_disable(imxdi, DIER_CAIE);
-- 
2.1.4

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH 3/5] RTC/i.MX/DryIce: monitor a security violation at runtime

[Juergen Borleis]

Maybe the unit enters the hardware related state at runtime and not at
system boot time (after a power cycle).

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
Signed-off-by: Robert Schwebel <rsc@pengutronix.de>
[rsc: got NDA clearance from Freescale]
---
 drivers/rtc/rtc-imxdi.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
index f8abf2b..b04c64f 100644
--- a/drivers/rtc/rtc-imxdi.c
+++ b/drivers/rtc/rtc-imxdi.c
@@ -680,6 +680,25 @@ static irqreturn_t dryice_norm_irq(int irq, void *dev_id)
 	irqreturn_t rc = IRQ_NONE;
 
 	dier = __raw_readl(imxdi->ioaddr + DIER);
+	dsr = __raw_readl(imxdi->ioaddr + DSR);
+
+	/* handle the security violation event */
+	if (dier & DIER_SVIE) {
+		if (dsr & DSR_SVF) {
+			/*
+			 * Disable the interrupt when this kind of event has
+			 * happened.
+			 * There cannot be more than one event of this type,
+			 * because it needs a complex state change
+			 * including a main power cycle to get again out of
+			 * this state.
+			 */
+			di_int_disable(imxdi, DIER_SVIE);
+			/* report the violation */
+			di_report_tamper_info(imxdi, dsr);
+			rc = IRQ_HANDLED;
+		}
+	}
 
 	/* handle write complete and write error cases */
 	if (dier & DIER_WCIE) {
@@ -690,7 +709,6 @@ static irqreturn_t dryice_norm_irq(int irq, void *dev_id)
 			return rc;
 
 		/* DSR_WCF clears itself on DSR read */
-		dsr = __raw_readl(imxdi->ioaddr + DSR);
 		if (dsr & (DSR_WCF | DSR_WEF)) {
 			/* mask the interrupt */
 			di_int_disable(imxdi, DIER_WCIE);
@@ -706,7 +724,6 @@ static irqreturn_t dryice_norm_irq(int irq, void *dev_id)
 	/* handle the alarm case */
 	if (dier & DIER_CAIE) {
 		/* DSR_WCF clears itself on DSR read */
-		dsr = __raw_readl(imxdi->ioaddr + DSR);
 		if (dsr & DSR_CAF) {
 			/* mask the interrupt */
 			di_int_disable(imxdi, DIER_CAIE);
-- 
2.1.4

[PATCH 4/5] RTC/i.MX/DryIce: when locked, do not fail silently

[Juergen Borleis]

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
Signed-off-by: Robert Schwebel <rsc@pengutronix.de>
[rsc: got NDA clearance from Freescale]
---
 drivers/rtc/rtc-imxdi.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
index b04c64f..de1a2e4 100644
--- a/drivers/rtc/rtc-imxdi.c
+++ b/drivers/rtc/rtc-imxdi.c
@@ -581,14 +581,36 @@ static int dryice_rtc_read_time(struct device *dev, struct rtc_time *tm)
 static int dryice_rtc_set_mmss(struct device *dev, unsigned long secs)
 {
 	struct imxdi_dev *imxdi = dev_get_drvdata(dev);
+	u32 dcr, dsr;
 	int rc;
 
+	dcr = __raw_readl(imxdi->ioaddr + DCR);
+	dsr = __raw_readl(imxdi->ioaddr + DSR);
+
+	if (!(dcr & DCR_TCE) || (dsr & DSR_SVF)) {
+		if (dcr & DCR_TCHL) {
+			/* we are even more out of luck */
+			di_what_is_to_be_done(imxdi, "battery");
+			return -EPERM;
+		}
+		if ((dcr & DCR_TCSL) || (dsr & DSR_SVF)) {
+			/* we are out of luck for now */
+			di_what_is_to_be_done(imxdi, "main");
+			return -EPERM;
+		}
+	}
+
 	/* zero the fractional part first */
 	rc = di_write_wait(imxdi, 0, DTCLR);
-	if (rc == 0)
-		rc = di_write_wait(imxdi, secs, DTCMR);
+	if (rc != 0)
+		return rc;
 
-	return rc;
+	rc = di_write_wait(imxdi, secs, DTCMR);
+	if (rc != 0)
+		return rc;
+
+	return di_write_wait(imxdi, __raw_readl(imxdi->ioaddr + DCR) |
+								DCR_TCE, DCR);
 }
 
 static int dryice_rtc_alarm_irq_enable(struct device *dev,
-- 
2.1.4

[rtc-linux] [PATCH 4/5] RTC/i.MX/DryIce: when locked, do not fail silently

[Juergen Borleis]

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
Signed-off-by: Robert Schwebel <rsc@pengutronix.de>
[rsc: got NDA clearance from Freescale]
---
 drivers/rtc/rtc-imxdi.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
index b04c64f..de1a2e4 100644
--- a/drivers/rtc/rtc-imxdi.c
+++ b/drivers/rtc/rtc-imxdi.c
@@ -581,14 +581,36 @@ static int dryice_rtc_read_time(struct device *dev, struct rtc_time *tm)
 static int dryice_rtc_set_mmss(struct device *dev, unsigned long secs)
 {
 	struct imxdi_dev *imxdi = dev_get_drvdata(dev);
+	u32 dcr, dsr;
 	int rc;
 
+	dcr = __raw_readl(imxdi->ioaddr + DCR);
+	dsr = __raw_readl(imxdi->ioaddr + DSR);
+
+	if (!(dcr & DCR_TCE) || (dsr & DSR_SVF)) {
+		if (dcr & DCR_TCHL) {
+			/* we are even more out of luck */
+			di_what_is_to_be_done(imxdi, "battery");
+			return -EPERM;
+		}
+		if ((dcr & DCR_TCSL) || (dsr & DSR_SVF)) {
+			/* we are out of luck for now */
+			di_what_is_to_be_done(imxdi, "main");
+			return -EPERM;
+		}
+	}
+
 	/* zero the fractional part first */
 	rc = di_write_wait(imxdi, 0, DTCLR);
-	if (rc == 0)
-		rc = di_write_wait(imxdi, secs, DTCMR);
+	if (rc != 0)
+		return rc;
 
-	return rc;
+	rc = di_write_wait(imxdi, secs, DTCMR);
+	if (rc != 0)
+		return rc;
+
+	return di_write_wait(imxdi, __raw_readl(imxdi->ioaddr + DCR) |
+								DCR_TCE, DCR);
 }
 
 static int dryice_rtc_alarm_irq_enable(struct device *dev,
-- 
2.1.4

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH 4/5] RTC/i.MX/DryIce: when locked, do not fail silently

[Juergen Borleis]

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
Signed-off-by: Robert Schwebel <rsc@pengutronix.de>
[rsc: got NDA clearance from Freescale]
---
 drivers/rtc/rtc-imxdi.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
index b04c64f..de1a2e4 100644
--- a/drivers/rtc/rtc-imxdi.c
+++ b/drivers/rtc/rtc-imxdi.c
@@ -581,14 +581,36 @@ static int dryice_rtc_read_time(struct device *dev, struct rtc_time *tm)
 static int dryice_rtc_set_mmss(struct device *dev, unsigned long secs)
 {
 	struct imxdi_dev *imxdi = dev_get_drvdata(dev);
+	u32 dcr, dsr;
 	int rc;
 
+	dcr = __raw_readl(imxdi->ioaddr + DCR);
+	dsr = __raw_readl(imxdi->ioaddr + DSR);
+
+	if (!(dcr & DCR_TCE) || (dsr & DSR_SVF)) {
+		if (dcr & DCR_TCHL) {
+			/* we are even more out of luck */
+			di_what_is_to_be_done(imxdi, "battery");
+			return -EPERM;
+		}
+		if ((dcr & DCR_TCSL) || (dsr & DSR_SVF)) {
+			/* we are out of luck for now */
+			di_what_is_to_be_done(imxdi, "main");
+			return -EPERM;
+		}
+	}
+
 	/* zero the fractional part first */
 	rc = di_write_wait(imxdi, 0, DTCLR);
-	if (rc == 0)
-		rc = di_write_wait(imxdi, secs, DTCMR);
+	if (rc != 0)
+		return rc;
 
-	return rc;
+	rc = di_write_wait(imxdi, secs, DTCMR);
+	if (rc != 0)
+		return rc;
+
+	return di_write_wait(imxdi, __raw_readl(imxdi->ioaddr + DCR) |
+								DCR_TCE, DCR);
 }
 
 static int dryice_rtc_alarm_irq_enable(struct device *dev,
-- 
2.1.4

[PATCH 5/5] RTC/i.MX/DryIce: prepare to force a security violation

[Juergen Borleis]

In order to test the new driver we need some mechanism to force a transition
into the security violation state. Two DryIce internal timers can be used
for this purpose. Both have an overflow feature which forces this transition
and can be triggered automatically (timer) or manually (monotonic via reading
the RTC time).

Note: this change is intended for development only to test the driver's
recovery capabilities. It is useless for regular use of the DryIce unit.

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
Signed-off-by: Robert Schwebel <rsc@pengutronix.de>
[rsc: got NDA clearance from Freescale]
---
 drivers/rtc/rtc-imxdi.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 75 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
index de1a2e4..cbc1ebe 100644
--- a/drivers/rtc/rtc-imxdi.c
+++ b/drivers/rtc/rtc-imxdi.c
@@ -29,6 +29,9 @@
  * not supported by the hardware.
  */
 
+#undef FORCE_VIOLATION
+# define USE_TIMER_VIOLATION
+
 #include <linux/io.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
@@ -288,6 +291,69 @@ static int di_handle_failure_state(struct imxdi_dev *imxdi, u32 dsr)
 	return -ENODEV;
 }
 
+/*
+ * Two types of security violations we can force:
+ *
+ * - regular timer counter overflow:
+ *   - set it up to 0xfffffff0
+ *   - enable its counting
+ *   - set TCSL bit to prevent any further change
+ *   - let the overflow happen which forces a security violation
+ *
+ * - monotonic counter overflow:
+ *   - set it up to 0xfffffffc
+ *   - enable its counting (MCE = 1)
+ *   - set MCSL bit to prevent any further change
+ *   - write 4 times to the monotonic counter register
+ */
+static void di_prepare_security_violation(struct imxdi_dev *imxdi)
+{
+	u32 dcr = __raw_readl(imxdi->ioaddr + DCR);
+	u32 dtcr = __raw_readl(imxdi->ioaddr + DTCR);
+
+#ifndef USE_TIMER_VIOLATION /* monotonic counter variant */
+
+	/* clear the MCO flag, otherwhise it cannot be programmed again */
+	di_write_busy_wait(imxdi, DSR_MCO, DSR);
+
+	/* stop monotonic-counter to be able to set its absolute value */
+	dcr &= ~DCR_MCE;
+	di_write_busy_wait(imxdi, dcr, DCR);
+
+	/* set a new value close to its overflow */
+	di_write_busy_wait(imxdi, 0xfffffff8, DMCR);
+
+	/* enable monotonic-counter to increment on each write */
+	dcr |= DCR_MCE;
+	di_write_busy_wait(imxdi, dcr, DCR);
+
+	/* lock this setting */
+	dcr |= DCR_MCSL;
+	di_write_busy_wait(imxdi, dcr, DCR);
+
+	/* let this overflow force the transition into the failure state */
+	di_write_busy_wait(imxdi, dtcr | DTCR_MOE, DTCR);
+#else /* timer counter variant */
+	/* clear the TCO flag, otherwhise it cannot be programmed again */
+	di_write_busy_wait(imxdi, DSR_TCO, DSR);
+
+	/* set a new value close to its overflow (16 seconds) */
+	di_write_busy_wait(imxdi, 0x00000000, DTCLR);
+	di_write_busy_wait(imxdi, 0xfffffff0, DTCMR);
+
+	/* enable timer-counter to increment on each write */
+	dcr |= DCR_TCE;
+	di_write_busy_wait(imxdi, dcr, DCR);
+
+	/* lock this setting */
+	dcr |= DCR_TCSL;
+	di_write_busy_wait(imxdi, dcr, DCR);
+
+	/* let this overflow force the transition into the failure state */
+	di_write_busy_wait(imxdi, dtcr | DTCR_TOE, DTCR);
+#endif
+}
+
 static int di_handle_valid_state(struct imxdi_dev *imxdi, u32 dsr)
 {
 	/* initialize alarm */
@@ -305,6 +371,7 @@ static int di_handle_invalid_state(struct imxdi_dev *imxdi, u32 dsr)
 {
 	u32 dcr, sec;
 
+#ifndef FORCE_VIOLATION
 	/*
 	 * lets disable all sources which can force the DryIce unit into
 	 * the "FAILURE STATE" for now
@@ -312,7 +379,7 @@ static int di_handle_invalid_state(struct imxdi_dev *imxdi, u32 dsr)
 	di_write_busy_wait(imxdi, 0x00000000, DTCR);
 	/* and lets protect them at runtime from any change */
 	di_write_busy_wait(imxdi, DCR_TDCSL, DCR);
-
+#endif
 	sec = __raw_readl(imxdi->ioaddr + DTCMR);
 	if (sec != 0)
 		dev_warn(&imxdi->pdev->dev,
@@ -571,6 +638,10 @@ static int dryice_rtc_read_time(struct device *dev, struct rtc_time *tm)
 	now = __raw_readl(imxdi->ioaddr + DTCMR);
 	rtc_time_to_tm(now, tm);
 
+#if defined(FORCE_VIOLATION) && !defined(USE_TIMER_VIOLATION)
+	/* don't use interrupts here */
+	di_write_busy_wait(imxdi, 0, DMCR);
+#endif
 	return 0;
 }
 
@@ -840,6 +911,9 @@ static int __init dryice_rtc_probe(struct platform_device *pdev)
 		goto err;
 	}
 
+#ifdef FORCE_VIOLATION
+	di_prepare_security_violation(imxdi);
+#endif
 	return 0;
 
 err:
-- 
2.1.4

[rtc-linux] [PATCH 5/5] RTC/i.MX/DryIce: prepare to force a security violation

[Juergen Borleis]

In order to test the new driver we need some mechanism to force a transition
into the security violation state. Two DryIce internal timers can be used
for this purpose. Both have an overflow feature which forces this transition
and can be triggered automatically (timer) or manually (monotonic via reading
the RTC time).

Note: this change is intended for development only to test the driver's
recovery capabilities. It is useless for regular use of the DryIce unit.

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
Signed-off-by: Robert Schwebel <rsc@pengutronix.de>
[rsc: got NDA clearance from Freescale]
---
 drivers/rtc/rtc-imxdi.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 75 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
index de1a2e4..cbc1ebe 100644
--- a/drivers/rtc/rtc-imxdi.c
+++ b/drivers/rtc/rtc-imxdi.c
@@ -29,6 +29,9 @@
  * not supported by the hardware.
  */
 
+#undef FORCE_VIOLATION
+# define USE_TIMER_VIOLATION
+
 #include <linux/io.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
@@ -288,6 +291,69 @@ static int di_handle_failure_state(struct imxdi_dev *imxdi, u32 dsr)
 	return -ENODEV;
 }
 
+/*
+ * Two types of security violations we can force:
+ *
+ * - regular timer counter overflow:
+ *   - set it up to 0xfffffff0
+ *   - enable its counting
+ *   - set TCSL bit to prevent any further change
+ *   - let the overflow happen which forces a security violation
+ *
+ * - monotonic counter overflow:
+ *   - set it up to 0xfffffffc
+ *   - enable its counting (MCE = 1)
+ *   - set MCSL bit to prevent any further change
+ *   - write 4 times to the monotonic counter register
+ */
+static void di_prepare_security_violation(struct imxdi_dev *imxdi)
+{
+	u32 dcr = __raw_readl(imxdi->ioaddr + DCR);
+	u32 dtcr = __raw_readl(imxdi->ioaddr + DTCR);
+
+#ifndef USE_TIMER_VIOLATION /* monotonic counter variant */
+
+	/* clear the MCO flag, otherwhise it cannot be programmed again */
+	di_write_busy_wait(imxdi, DSR_MCO, DSR);
+
+	/* stop monotonic-counter to be able to set its absolute value */
+	dcr &= ~DCR_MCE;
+	di_write_busy_wait(imxdi, dcr, DCR);
+
+	/* set a new value close to its overflow */
+	di_write_busy_wait(imxdi, 0xfffffff8, DMCR);
+
+	/* enable monotonic-counter to increment on each write */
+	dcr |= DCR_MCE;
+	di_write_busy_wait(imxdi, dcr, DCR);
+
+	/* lock this setting */
+	dcr |= DCR_MCSL;
+	di_write_busy_wait(imxdi, dcr, DCR);
+
+	/* let this overflow force the transition into the failure state */
+	di_write_busy_wait(imxdi, dtcr | DTCR_MOE, DTCR);
+#else /* timer counter variant */
+	/* clear the TCO flag, otherwhise it cannot be programmed again */
+	di_write_busy_wait(imxdi, DSR_TCO, DSR);
+
+	/* set a new value close to its overflow (16 seconds) */
+	di_write_busy_wait(imxdi, 0x00000000, DTCLR);
+	di_write_busy_wait(imxdi, 0xfffffff0, DTCMR);
+
+	/* enable timer-counter to increment on each write */
+	dcr |= DCR_TCE;
+	di_write_busy_wait(imxdi, dcr, DCR);
+
+	/* lock this setting */
+	dcr |= DCR_TCSL;
+	di_write_busy_wait(imxdi, dcr, DCR);
+
+	/* let this overflow force the transition into the failure state */
+	di_write_busy_wait(imxdi, dtcr | DTCR_TOE, DTCR);
+#endif
+}
+
 static int di_handle_valid_state(struct imxdi_dev *imxdi, u32 dsr)
 {
 	/* initialize alarm */
@@ -305,6 +371,7 @@ static int di_handle_invalid_state(struct imxdi_dev *imxdi, u32 dsr)
 {
 	u32 dcr, sec;
 
+#ifndef FORCE_VIOLATION
 	/*
 	 * lets disable all sources which can force the DryIce unit into
 	 * the "FAILURE STATE" for now
@@ -312,7 +379,7 @@ static int di_handle_invalid_state(struct imxdi_dev *imxdi, u32 dsr)
 	di_write_busy_wait(imxdi, 0x00000000, DTCR);
 	/* and lets protect them at runtime from any change */
 	di_write_busy_wait(imxdi, DCR_TDCSL, DCR);
-
+#endif
 	sec = __raw_readl(imxdi->ioaddr + DTCMR);
 	if (sec != 0)
 		dev_warn(&imxdi->pdev->dev,
@@ -571,6 +638,10 @@ static int dryice_rtc_read_time(struct device *dev, struct rtc_time *tm)
 	now = __raw_readl(imxdi->ioaddr + DTCMR);
 	rtc_time_to_tm(now, tm);
 
+#if defined(FORCE_VIOLATION) && !defined(USE_TIMER_VIOLATION)
+	/* don't use interrupts here */
+	di_write_busy_wait(imxdi, 0, DMCR);
+#endif
 	return 0;
 }
 
@@ -840,6 +911,9 @@ static int __init dryice_rtc_probe(struct platform_device *pdev)
 		goto err;
 	}
 
+#ifdef FORCE_VIOLATION
+	di_prepare_security_violation(imxdi);
+#endif
 	return 0;
 
 err:
-- 
2.1.4

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH 5/5] RTC/i.MX/DryIce: prepare to force a security violation

[Juergen Borleis]

In order to test the new driver we need some mechanism to force a transition
into the security violation state. Two DryIce internal timers can be used
for this purpose. Both have an overflow feature which forces this transition
and can be triggered automatically (timer) or manually (monotonic via reading
the RTC time).

Note: this change is intended for development only to test the driver's
recovery capabilities. It is useless for regular use of the DryIce unit.

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
Signed-off-by: Robert Schwebel <rsc@pengutronix.de>
[rsc: got NDA clearance from Freescale]
---
 drivers/rtc/rtc-imxdi.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 75 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
index de1a2e4..cbc1ebe 100644
--- a/drivers/rtc/rtc-imxdi.c
+++ b/drivers/rtc/rtc-imxdi.c
@@ -29,6 +29,9 @@
  * not supported by the hardware.
  */
 
+#undef FORCE_VIOLATION
+# define USE_TIMER_VIOLATION
+
 #include <linux/io.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
@@ -288,6 +291,69 @@ static int di_handle_failure_state(struct imxdi_dev *imxdi, u32 dsr)
 	return -ENODEV;
 }
 
+/*
+ * Two types of security violations we can force:
+ *
+ * - regular timer counter overflow:
+ *   - set it up to 0xfffffff0
+ *   - enable its counting
+ *   - set TCSL bit to prevent any further change
+ *   - let the overflow happen which forces a security violation
+ *
+ * - monotonic counter overflow:
+ *   - set it up to 0xfffffffc
+ *   - enable its counting (MCE = 1)
+ *   - set MCSL bit to prevent any further change
+ *   - write 4 times to the monotonic counter register
+ */
+static void di_prepare_security_violation(struct imxdi_dev *imxdi)
+{
+	u32 dcr = __raw_readl(imxdi->ioaddr + DCR);
+	u32 dtcr = __raw_readl(imxdi->ioaddr + DTCR);
+
+#ifndef USE_TIMER_VIOLATION /* monotonic counter variant */
+
+	/* clear the MCO flag, otherwhise it cannot be programmed again */
+	di_write_busy_wait(imxdi, DSR_MCO, DSR);
+
+	/* stop monotonic-counter to be able to set its absolute value */
+	dcr &= ~DCR_MCE;
+	di_write_busy_wait(imxdi, dcr, DCR);
+
+	/* set a new value close to its overflow */
+	di_write_busy_wait(imxdi, 0xfffffff8, DMCR);
+
+	/* enable monotonic-counter to increment on each write */
+	dcr |= DCR_MCE;
+	di_write_busy_wait(imxdi, dcr, DCR);
+
+	/* lock this setting */
+	dcr |= DCR_MCSL;
+	di_write_busy_wait(imxdi, dcr, DCR);
+
+	/* let this overflow force the transition into the failure state */
+	di_write_busy_wait(imxdi, dtcr | DTCR_MOE, DTCR);
+#else /* timer counter variant */
+	/* clear the TCO flag, otherwhise it cannot be programmed again */
+	di_write_busy_wait(imxdi, DSR_TCO, DSR);
+
+	/* set a new value close to its overflow (16 seconds) */
+	di_write_busy_wait(imxdi, 0x00000000, DTCLR);
+	di_write_busy_wait(imxdi, 0xfffffff0, DTCMR);
+
+	/* enable timer-counter to increment on each write */
+	dcr |= DCR_TCE;
+	di_write_busy_wait(imxdi, dcr, DCR);
+
+	/* lock this setting */
+	dcr |= DCR_TCSL;
+	di_write_busy_wait(imxdi, dcr, DCR);
+
+	/* let this overflow force the transition into the failure state */
+	di_write_busy_wait(imxdi, dtcr | DTCR_TOE, DTCR);
+#endif
+}
+
 static int di_handle_valid_state(struct imxdi_dev *imxdi, u32 dsr)
 {
 	/* initialize alarm */
@@ -305,6 +371,7 @@ static int di_handle_invalid_state(struct imxdi_dev *imxdi, u32 dsr)
 {
 	u32 dcr, sec;
 
+#ifndef FORCE_VIOLATION
 	/*
 	 * lets disable all sources which can force the DryIce unit into
 	 * the "FAILURE STATE" for now
@@ -312,7 +379,7 @@ static int di_handle_invalid_state(struct imxdi_dev *imxdi, u32 dsr)
 	di_write_busy_wait(imxdi, 0x00000000, DTCR);
 	/* and lets protect them at runtime from any change */
 	di_write_busy_wait(imxdi, DCR_TDCSL, DCR);
-
+#endif
 	sec = __raw_readl(imxdi->ioaddr + DTCMR);
 	if (sec != 0)
 		dev_warn(&imxdi->pdev->dev,
@@ -571,6 +638,10 @@ static int dryice_rtc_read_time(struct device *dev, struct rtc_time *tm)
 	now = __raw_readl(imxdi->ioaddr + DTCMR);
 	rtc_time_to_tm(now, tm);
 
+#if defined(FORCE_VIOLATION) && !defined(USE_TIMER_VIOLATION)
+	/* don't use interrupts here */
+	di_write_busy_wait(imxdi, 0, DMCR);
+#endif
 	return 0;
 }
 
@@ -840,6 +911,9 @@ static int __init dryice_rtc_probe(struct platform_device *pdev)
 		goto err;
 	}
 
+#ifdef FORCE_VIOLATION
+	di_prepare_security_violation(imxdi);
+#endif
 	return 0;
 
 err:
-- 
2.1.4

Re: [rtc-linux] [PATCH 1/5] RTC/i.MX/DryIce: add some background info about the states the machine can be in

[Alexandre Belloni]

On 14/04/2015 at 11:11:52 +0200, Juergen Borleis wrote :
> Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
> Signed-off-by: Robert Schwebel <rsc@pengutronix.de>
> [rsc: got NDA clearance from Freescale]
> ---
>  drivers/rtc/rtc-imxdi.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
> 
> diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
> index c666eab..8750477 100644
> --- a/drivers/rtc/rtc-imxdi.c
> +++ b/drivers/rtc/rtc-imxdi.c
> @@ -129,6 +129,49 @@ struct imxdi_dev {
>  	struct work_struct work;
>  };
>  
> +/* Some background:
> + *
> + * The DryIce unit is a complex security/tamper monitor device. To be able do
> + * its job in a useful manner it runs a bigger statemachine to bring it into
> + * security/tamper failure state and once again to bring it out of this state.
> + *
> + * This unit can be in one of three states:
> + *
> + * - "NON-VALID STATE"
> + *   always after the battery power was removed
> + * - "FAILURE STATE"
> + *   if one of the enabled security events have happend
                                 has happened ^
> + * - "VALID STATE"
> + *   if the unit works as expected
> + *
> + * Everything stops when the unit enters the failure state including the RTC
> + * counter (to be able to detect the time the security event happend).
                                                       happened ^
> + *
> + * The following events (when enabled) let the DryIce unit enter the failure
> + * state:
> + *
> + * - wire-mesh-tamper detect
> + * - external tamper B detect
> + * - external tamper A detect
> + * - temperature tamper detect
> + * - clock tamper detect
> + * - voltage tamper detect
> + * - RTC counter overflow
> + * - monotonic counter overflow
> + * - external boot
> + *
> + * If we find the DryIce unit in "FAILURE STATE" and the TDCHL cleared, we
> + * can only detect this state. In this case the unit is completely locked and
> + * must force a second "SYSTEM POR" to bring the DryIce into the
> + * "NON-VALID STATE" + "FAILURE STATE" where a recovery is possible.
> + * If the TDCHL is set in the "FAILURE STATE" we are out of luck. In this case
> + * a battery power cycle is required.
> + *
> + * In the "NON-VALID STATE" + "FAILURE STATE" we can clear the "FAILURE STATE"
> + * and recover the DryIce unit. By clearing the "NON-VALID STATE" as the last
> + * task, we bring back this unit into life.
> + */
> +
>  /*
>   * enable a dryice interrupt
>   */
> -- 
> 2.1.4
> 
> -- 
> -- 
> You received this message because you are subscribed to "rtc-linux".
> Membership options at http://groups.google.com/group/rtc-linux .
> Please read http://groups.google.com/group/rtc-linux/web/checklist
> before submitting a driver.
> --- 
> You received this message because you are subscribed to the Google Groups "rtc-linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[rtc-linux] [PATCH 1/5] RTC/i.MX/DryIce: add some background info about the states the machine can be in

[Alexandre Belloni]

On 14/04/2015 at 11:11:52 +0200, Juergen Borleis wrote :
> Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
> Signed-off-by: Robert Schwebel <rsc@pengutronix.de>
> [rsc: got NDA clearance from Freescale]
> ---
>  drivers/rtc/rtc-imxdi.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
> 
> diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
> index c666eab..8750477 100644
> --- a/drivers/rtc/rtc-imxdi.c
> +++ b/drivers/rtc/rtc-imxdi.c
> @@ -129,6 +129,49 @@ struct imxdi_dev {
>  	struct work_struct work;
>  };
>  
> +/* Some background:
> + *
> + * The DryIce unit is a complex security/tamper monitor device. To be able do
> + * its job in a useful manner it runs a bigger statemachine to bring it into
> + * security/tamper failure state and once again to bring it out of this state.
> + *
> + * This unit can be in one of three states:
> + *
> + * - "NON-VALID STATE"
> + *   always after the battery power was removed
> + * - "FAILURE STATE"
> + *   if one of the enabled security events have happend
                                 has happened ^
> + * - "VALID STATE"
> + *   if the unit works as expected
> + *
> + * Everything stops when the unit enters the failure state including the RTC
> + * counter (to be able to detect the time the security event happend).
                                                       happened ^
> + *
> + * The following events (when enabled) let the DryIce unit enter the failure
> + * state:
> + *
> + * - wire-mesh-tamper detect
> + * - external tamper B detect
> + * - external tamper A detect
> + * - temperature tamper detect
> + * - clock tamper detect
> + * - voltage tamper detect
> + * - RTC counter overflow
> + * - monotonic counter overflow
> + * - external boot
> + *
> + * If we find the DryIce unit in "FAILURE STATE" and the TDCHL cleared, we
> + * can only detect this state. In this case the unit is completely locked and
> + * must force a second "SYSTEM POR" to bring the DryIce into the
> + * "NON-VALID STATE" + "FAILURE STATE" where a recovery is possible.
> + * If the TDCHL is set in the "FAILURE STATE" we are out of luck. In this case
> + * a battery power cycle is required.
> + *
> + * In the "NON-VALID STATE" + "FAILURE STATE" we can clear the "FAILURE STATE"
> + * and recover the DryIce unit. By clearing the "NON-VALID STATE" as the last
> + * task, we bring back this unit into life.
> + */
> +
>  /*
>   * enable a dryice interrupt
>   */
> -- 
> 2.1.4
> 
> -- 
> -- 
> You received this message because you are subscribed to "rtc-linux".
> Membership options at http://groups.google.com/group/rtc-linux .
> Please read http://groups.google.com/group/rtc-linux/web/checklist
> before submitting a driver.
> --- 
> You received this message because you are subscribed to the Google Groups "rtc-linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe at googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [rtc-linux] [PATCH 2/5] RTC/i.MX/DryIce: add the unit recovery code

[Alexandre Belloni]

On 14/04/2015 at 11:11:53 +0200, Juergen Borleis wrote :
> This code is requiered to recover the unit from a security violation.
      required ^

> Hopefully this code can recover the unit from a hardware related invalid
> state as well.
> 
> Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
> Signed-off-by: Robert Schwebel <rsc@pengutronix.de>
> [rsc: got NDA clearance from Freescale]
> ---
>  drivers/rtc/rtc-imxdi.c | 333 ++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 295 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
> index 8750477..f8abf2b 100644
> --- a/drivers/rtc/rtc-imxdi.c
> +++ b/drivers/rtc/rtc-imxdi.c
> @@ -172,6 +172,296 @@ struct imxdi_dev {
>   * task, we bring back this unit into life.
>   */
>  
> +/* do a write into the unit without interrupt support */
> +static void di_write_busy_wait(const struct imxdi_dev *imxdi, u32 val,
> +								unsigned reg)

Alignment should match the open parenthesis, please fix all occurrences
in your patch.

> +{
> +	/* do the register write */
> +	__raw_writel(val, imxdi->ioaddr + reg);
> +
> +	/*
> +	 * now it takes four 32,768 kHz clock cycles to take
> +	 * the change into effect = 122 us
> +	 */
> +	udelay(200);

As the requirement is not that critical, you may want to use usleep_range()

> +}
> +
> +static void di_report_tamper_info(struct imxdi_dev *imxdi,  u32 dsr)
> +{
> +	u32 dtcr;
> +
> +	dtcr = __raw_readl(imxdi->ioaddr + DTCR);
> +
> +	dev_emerg(&imxdi->pdev->dev, "!! DryIce tamper event detected !!\n");
> +	/* the following flags force a transition into the "FAILURE STATE" */
> +	if (dsr & DSR_VTD) {
> +		dev_emerg(&imxdi->pdev->dev, "!! Voltage Tamper event\n");
> +		if (!(dtcr & DTCR_VTE))
> +			dev_emerg(&imxdi->pdev->dev,
> +				"!! But Voltage Tamper detection wasn't enabled. False positive?\n");

I'm not sure about prefixing all the messages with "!! ". dev_emerg()
seems important enough to be noticed.
I would remove " False positive?". What about
		dev_emerg(&imxdi->pdev->dev,
			  "%sVoltage Tamper event\n",
			  (dtcr & DTCR_VTE) ? "" : "Spurious ");

> +	}
> +	if (dsr & DSR_CTD) {
> +		dev_emerg(&imxdi->pdev->dev, "!! 32768 Hz Clock Tamper Event\n");
> +		if (!(dtcr & DTCR_CTE))
> +			dev_emerg(&imxdi->pdev->dev,
> +				"!! But Clock Tamper detection wasn't enabled. False positive?\n");
> +	}
> +	if (dsr & DSR_TTD) {
> +		dev_emerg(&imxdi->pdev->dev, "!! Temperature Tamper Event\n");
> +		if (!(dtcr & DTCR_TTE))
> +			dev_emerg(&imxdi->pdev->dev,
> +				"!! But Temperature Tamper detection wasn't enabled. False positive?\n");
> +	}
> +	if (dsr & DSR_SAD) {
> +		dev_emerg(&imxdi->pdev->dev, "!! Secure Controller Alarm Event\n");
> +		if (!(dtcr & DTCR_SAIE))
> +			dev_emerg(&imxdi->pdev->dev,
> +				"!! But Secure Controller Alarm detection wasn't enabled. False positive?\n");
> +	}
> +	if (dsr & DSR_EBD) {
> +		dev_emerg(&imxdi->pdev->dev, "!! External Boot Tamper Event\n");
> +		if (!(dtcr & DTCR_EBE))
> +			dev_emerg(&imxdi->pdev->dev,
> +				"!! But External Boot detection wasn't enabled. False positive?\n");
> +	}
> +	if (dsr & DSR_ETAD) {
> +		dev_emerg(&imxdi->pdev->dev, "!! External Tamper A Event\n");
> +		if (!(dtcr & DTCR_ETAE))
> +			dev_emerg(&imxdi->pdev->dev,
> +				"!! But External Tamper A wasn't enabled. False positive?\n");
> +	}
> +	if (dsr & DSR_ETBD) {
> +		dev_emerg(&imxdi->pdev->dev, "!! External Tamper B Event\n");
> +		if (!(dtcr & DTCR_ETBE))
> +			dev_emerg(&imxdi->pdev->dev,
> +				"!! But External Tamper B wasn't enabled. False positive?\n");
> +	}
> +	if (dsr & DSR_WTD) {
> +		dev_emerg(&imxdi->pdev->dev, "!! Wire-mesh Tamper Event\n");
> +		if (!(dtcr & DTCR_WTE))
> +			dev_emerg(&imxdi->pdev->dev,
> +				"!! But wire-mesh Tamper detection wasn't enabled. False positive?\n");
> +	}
> +	if (dsr & DSR_MCO) {
> +		dev_emerg(&imxdi->pdev->dev, "!! Monotonic-counter Overflow Event\n");
> +		if (!(dtcr & DTCR_MOE))
> +			dev_emerg(&imxdi->pdev->dev,
> +				"!! But Monotonic-counter Overflow detection wasn't enabled. False positive?\n");
> +	}
> +	if (dsr & DSR_TCO) {
> +		dev_emerg(&imxdi->pdev->dev, "!! Timer-counter Overflow Event\n");
> +		if (!(dtcr & DTCR_TOE))
> +			dev_emerg(&imxdi->pdev->dev,
> +				"!! But Timer-counter Overflow detection wasn't enabled. False positive?\n");
> +	}
> +}
> +
> +static void di_what_is_to_be_done(struct imxdi_dev *imxdi,
> +						const char *power_supply)
> +{
> +	dev_emerg(&imxdi->pdev->dev, " Please cycle the %s power supply in order to get the DryIce unit working again\n",
            remove that heading space ^

I would also explain that the RTC is part of the DryIce unit.

> +			power_supply);
> +}
> +
> +static int di_handle_failure_state(struct imxdi_dev *imxdi, u32 dsr)
> +{
> +	u32 dcr;
> +
> +	dev_dbg(&imxdi->pdev->dev, "DSR register reports: %08X\n", dsr);
> +
> +	/* report the cause */
> +	di_report_tamper_info(imxdi, dsr);
> +
> +	dcr = __raw_readl(imxdi->ioaddr + DCR);

Even if it is used across the whole driver, I would avoid using
__raw_readx as this makes the driver only working in little endian mode.

> +
> +	if (dcr & DCR_FSHL) {
> +		/* we are out of luck */
> +		di_what_is_to_be_done(imxdi, "battery");
> +		return -ENODEV;
> +	}
> +	/*
> +	 * with the next SYSTEM POR we will transit from the "FAILURE STATE"
> +	 * into the "NON-VALID STATE" + "FAILURE STATE"
> +	 */
> +	di_what_is_to_be_done(imxdi, "main");
> +
> +	return -ENODEV;
> +}
> +
> +static int di_handle_valid_state(struct imxdi_dev *imxdi, u32 dsr)
> +{
> +	/* initialize alarm */
> +	di_write_busy_wait(imxdi, DCAMR_UNSET, DCAMR);
> +	di_write_busy_wait(imxdi, 0, DCALR);
> +
> +	/* clear alarm flag */
> +	if (dsr & DSR_CAF)
> +		di_write_busy_wait(imxdi, DSR_CAF, DSR);
> +
> +	return 0;
> +}
> +
> +static int di_handle_invalid_state(struct imxdi_dev *imxdi, u32 dsr)
> +{
> +	u32 dcr, sec;
> +
> +	/*
> +	 * lets disable all sources which can force the DryIce unit into
> +	 * the "FAILURE STATE" for now
> +	 */
> +	di_write_busy_wait(imxdi, 0x00000000, DTCR);
> +	/* and lets protect them at runtime from any change */
> +	di_write_busy_wait(imxdi, DCR_TDCSL, DCR);
> +
> +	sec = __raw_readl(imxdi->ioaddr + DTCMR);
> +	if (sec != 0)
> +		dev_warn(&imxdi->pdev->dev,
> +			"The security violation has happend at %u seconds\n",

Further improvement for this driver: use both DTCMR and DTCLR to be year
2038 proof

> +			sec);
> +	/*
> +	 * the timer cannot be set/modified if
> +	 * - the TCHL or TCSL bit is set in DCR
> +	 */
> +	dcr = __raw_readl(imxdi->ioaddr + DCR);
> +	if (!(dcr & DCR_TCE)) {
> +		if (dcr & DCR_TCHL) {
> +			/* we are out of luck */
> +			di_what_is_to_be_done(imxdi, "battery");
> +			return -ENODEV;
> +		}
> +		if (dcr & DCR_TCSL) {
> +			di_what_is_to_be_done(imxdi, "main");
> +			return -ENODEV;
> +		}
> +
Unnecessary blank line

> +	}
> +	/*
> +	 * - the timer counter stops/is stopped if
> +	 *   - its overflow flag is set (TCO in DSR)
> +	 *      -> clear overflow bit to make it count again
> +	 *   - NVF is set in DSR
> +	 *      -> clear non-valid bit to make it count again
> +	 *   - its TCE (DCR) is cleared
> +	 *      -> set TCE to make it count
> +	 *   - it was never set before
> +	 *      -> write a time into it (required again if the NVF was set)
> +	 */
> +	/* state handled */
> +	di_write_busy_wait(imxdi, DSR_NVF, DSR);
> +	/* clear overflow flag */
> +	di_write_busy_wait(imxdi, DSR_TCO, DSR);
> +	/* enable the counter */
> +	di_write_busy_wait(imxdi, dcr | DCR_TCE, DCR);
> +	/* set and trigger it to make it count */
> +	di_write_busy_wait(imxdi, sec, DTCMR);
> +
> +	/* now prepare for the valid state */
> +	return di_handle_valid_state(imxdi, __raw_readl(imxdi->ioaddr + DSR));
> +}
> +
> +static int di_handle_invalid_and_failure_state(struct imxdi_dev *imxdi, u32 dsr)
> +{
> +	u32 dcr;
> +
> +	/*
> +	 * now we must first remove the tamper sources in order to get the
> +	 * device out of the "FAILURE STATE"
> +	 * To disable any of the following sources we need to modify the DTCR
> +	 */
> +	if (dsr & (DSR_WTD | DSR_ETBD | DSR_ETAD | DSR_EBD | DSR_SAD |
> +			DSR_TTD | DSR_CTD | DSR_VTD | DSR_MCO | DSR_TCO)) {
> +		dcr = __raw_readl(imxdi->ioaddr + DCR);
> +		if (dcr & DCR_TDCHL) {
> +			/*
> +			 * the tamper register is locked. We cannot disable the
> +			 * tamper detection. The TDCHL can only be reset by a
> +			 * DRYICE POR, but we cannot force a DRYICE POR in
> +			 * softwere because we are still in "FAILURE STATE".
> +			 * We need a DRYICE POR via battery power cycling....
> +			 */
> +			/*
> +			 * out of luck!
> +			 * we cannot disable them without a DRYICE POR
> +			 */
> +			di_what_is_to_be_done(imxdi, "battery");
> +			return -ENODEV;
> +		}
> +		if (dcr & DCR_TDCSL) {
> +			/* a soft lock can be removed by a SYSTEM POR */
> +			di_what_is_to_be_done(imxdi, "main");
> +			return -ENODEV;
> +		}
> +	}
> +
> +	/* disable all sources */
> +	di_write_busy_wait(imxdi, 0x00000000, DTCR);
> +
> +	/* clear the status bits now */
> +	di_write_busy_wait(imxdi, dsr & (DSR_WTD | DSR_ETBD | DSR_ETAD |
> +			DSR_EBD | DSR_SAD | DSR_TTD | DSR_CTD | DSR_VTD |
> +			DSR_MCO | DSR_TCO), DSR);
> +
> +	dsr = __raw_readl(imxdi->ioaddr + DSR);
> +	if ((dsr & ~(DSR_NVF | DSR_SVF | DSR_WBF | DSR_WNF |
> +			DSR_WCF | DSR_WEF)) != 0)
> +		dev_warn(&imxdi->pdev->dev,
> +			"There are still some sources of pain in DSR: %08x!\n",
> +			dsr & ~(DSR_NVF | DSR_SVF | DSR_WBF | DSR_WNF |
> +					DSR_WCF | DSR_WEF));
> +
> +	/*
> +	 * now we are trying to clear the "Security-violation flag" to
> +	 * get the DryIce out of this state
> +	 */
> +	di_write_busy_wait(imxdi, DSR_SVF, DSR);
> +
> +	/* success? */
> +	dsr = __raw_readl(imxdi->ioaddr + DSR);
> +	if (dsr & DSR_SVF) {
> +		dev_crit(&imxdi->pdev->dev,
> +			"Cannot clear the security violation flag. We are ending up in an endless loop!\n");
> +		/* last resourt */
                 resort ^
> +		di_what_is_to_be_done(imxdi, "battery");
> +		return -ENODEV;
> +	}
> +
> +	/*
> +	 * now we have left the "FAILURE STATE" and ending up in the
> +	 * "NON-VALID STATE" time to recover everything
> +	 */
> +	return di_handle_invalid_state(imxdi, dsr);
> +}
> +
> +static int di_handle_state(struct imxdi_dev *imxdi)
> +{
> +	int rc;
> +	u32 dsr;
> +
> +	dsr = __raw_readl(imxdi->ioaddr + DSR);
> +
> +	switch (dsr & (DSR_NVF | DSR_SVF)) {
> +	case DSR_NVF:
> +		dev_warn(&imxdi->pdev->dev, "Invalid stated unit detected\n");
> +		rc = di_handle_invalid_state(imxdi, dsr);
> +		break;
> +	case DSR_SVF:
> +		dev_warn(&imxdi->pdev->dev, "Failure stated unit detected\n");
> +		rc = di_handle_failure_state(imxdi, dsr);
> +		break;
> +	case DSR_NVF | DSR_SVF:
> +		dev_warn(&imxdi->pdev->dev,
> +				"Failure+Invalid stated unit detected\n");
> +		rc = di_handle_invalid_and_failure_state(imxdi, dsr);
> +		break;
> +	default:
> +		dev_notice(&imxdi->pdev->dev, "Unlocked unit detected\n");
> +		rc = di_handle_valid_state(imxdi, dsr);
> +	}
> +
> +	return rc;
> +}
> +
> +
Unnecessary blank line

>  /*
>   * enable a dryice interrupt
>   */
> @@ -491,6 +781,11 @@ static int __init dryice_rtc_probe(struct platform_device *pdev)
>  	/* mask all interrupts */
>  	__raw_writel(0, imxdi->ioaddr + DIER);
>  
> +	rc = di_handle_state(imxdi);
> +	if (rc != 0)
> +		goto err;
> +
> +
Unnecessary blank line

>  	rc = devm_request_irq(&pdev->dev, imxdi->irq, dryice_norm_irq,
>  			IRQF_SHARED, pdev->name, imxdi);
>  	if (rc) {
> @@ -498,44 +793,6 @@ static int __init dryice_rtc_probe(struct platform_device *pdev)
>  		goto err;
>  	}
>  
> -	/* put dryice into valid state */
> -	if (__raw_readl(imxdi->ioaddr + DSR) & DSR_NVF) {
> -		rc = di_write_wait(imxdi, DSR_NVF | DSR_SVF, DSR);

Multiple writes have switched from di_write_wait() which is checking
for a write error to di_write_busy_wait() which is not doing that check
is waiting 200us enough to ensure that the write has been done?

> -		if (rc)
> -			goto err;
> -	}
> -
> -	/* initialize alarm */
> -	rc = di_write_wait(imxdi, DCAMR_UNSET, DCAMR);
> -	if (rc)
> -		goto err;
> -	rc = di_write_wait(imxdi, 0, DCALR);
> -	if (rc)
> -		goto err;
> -
> -	/* clear alarm flag */
> -	if (__raw_readl(imxdi->ioaddr + DSR) & DSR_CAF) {
> -		rc = di_write_wait(imxdi, DSR_CAF, DSR);
> -		if (rc)
> -			goto err;
> -	}
> -
> -	/* the timer won't count if it has never been written to */
> -	if (__raw_readl(imxdi->ioaddr + DTCMR) == 0) {
> -		rc = di_write_wait(imxdi, 0, DTCMR);
> -		if (rc)
> -			goto err;
> -	}
> -
> -	/* start keeping time */
> -	if (!(__raw_readl(imxdi->ioaddr + DCR) & DCR_TCE)) {
> -		rc = di_write_wait(imxdi,
> -				__raw_readl(imxdi->ioaddr + DCR) | DCR_TCE,
> -				DCR);
> -		if (rc)
> -			goto err;
> -	}
> -
>  	platform_set_drvdata(pdev, imxdi);
>  	imxdi->rtc = devm_rtc_device_register(&pdev->dev, pdev->name,
>  				  &dryice_rtc_ops, THIS_MODULE);
> -- 
> 2.1.4
> 

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [rtc-linux] [PATCH 2/5] RTC/i.MX/DryIce: add the unit recovery code

[Alexandre Belloni]

On 14/04/2015 at 11:11:53 +0200, Juergen Borleis wrote :
> This code is requiered to recover the unit from a security violation.
      required ^

> Hopefully this code can recover the unit from a hardware related invalid
> state as well.
> 
> Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
> Signed-off-by: Robert Schwebel <rsc@pengutronix.de>
> [rsc: got NDA clearance from Freescale]
> ---
>  drivers/rtc/rtc-imxdi.c | 333 ++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 295 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
> index 8750477..f8abf2b 100644
> --- a/drivers/rtc/rtc-imxdi.c
> +++ b/drivers/rtc/rtc-imxdi.c
> @@ -172,6 +172,296 @@ struct imxdi_dev {
>   * task, we bring back this unit into life.
>   */
>  
> +/* do a write into the unit without interrupt support */
> +static void di_write_busy_wait(const struct imxdi_dev *imxdi, u32 val,
> +								unsigned reg)

Alignment should match the open parenthesis, please fix all occurrences
in your patch.

> +{
> +	/* do the register write */
> +	__raw_writel(val, imxdi->ioaddr + reg);
> +
> +	/*
> +	 * now it takes four 32,768 kHz clock cycles to take
> +	 * the change into effect = 122 us
> +	 */
> +	udelay(200);

As the requirement is not that critical, you may want to use usleep_range()

> +}
> +
> +static void di_report_tamper_info(struct imxdi_dev *imxdi,  u32 dsr)
> +{
> +	u32 dtcr;
> +
> +	dtcr = __raw_readl(imxdi->ioaddr + DTCR);
> +
> +	dev_emerg(&imxdi->pdev->dev, "!! DryIce tamper event detected !!\n");
> +	/* the following flags force a transition into the "FAILURE STATE" */
> +	if (dsr & DSR_VTD) {
> +		dev_emerg(&imxdi->pdev->dev, "!! Voltage Tamper event\n");
> +		if (!(dtcr & DTCR_VTE))
> +			dev_emerg(&imxdi->pdev->dev,
> +				"!! But Voltage Tamper detection wasn't enabled. False positive?\n");

I'm not sure about prefixing all the messages with "!! ". dev_emerg()
seems important enough to be noticed.
I would remove " False positive?". What about
		dev_emerg(&imxdi->pdev->dev,
			  "%sVoltage Tamper event\n",
			  (dtcr & DTCR_VTE) ? "" : "Spurious ");

> +	}
> +	if (dsr & DSR_CTD) {
> +		dev_emerg(&imxdi->pdev->dev, "!! 32768 Hz Clock Tamper Event\n");
> +		if (!(dtcr & DTCR_CTE))
> +			dev_emerg(&imxdi->pdev->dev,
> +				"!! But Clock Tamper detection wasn't enabled. False positive?\n");
> +	}
> +	if (dsr & DSR_TTD) {
> +		dev_emerg(&imxdi->pdev->dev, "!! Temperature Tamper Event\n");
> +		if (!(dtcr & DTCR_TTE))
> +			dev_emerg(&imxdi->pdev->dev,
> +				"!! But Temperature Tamper detection wasn't enabled. False positive?\n");
> +	}
> +	if (dsr & DSR_SAD) {
> +		dev_emerg(&imxdi->pdev->dev, "!! Secure Controller Alarm Event\n");
> +		if (!(dtcr & DTCR_SAIE))
> +			dev_emerg(&imxdi->pdev->dev,
> +				"!! But Secure Controller Alarm detection wasn't enabled. False positive?\n");
> +	}
> +	if (dsr & DSR_EBD) {
> +		dev_emerg(&imxdi->pdev->dev, "!! External Boot Tamper Event\n");
> +		if (!(dtcr & DTCR_EBE))
> +			dev_emerg(&imxdi->pdev->dev,
> +				"!! But External Boot detection wasn't enabled. False positive?\n");
> +	}
> +	if (dsr & DSR_ETAD) {
> +		dev_emerg(&imxdi->pdev->dev, "!! External Tamper A Event\n");
> +		if (!(dtcr & DTCR_ETAE))
> +			dev_emerg(&imxdi->pdev->dev,
> +				"!! But External Tamper A wasn't enabled. False positive?\n");
> +	}
> +	if (dsr & DSR_ETBD) {
> +		dev_emerg(&imxdi->pdev->dev, "!! External Tamper B Event\n");
> +		if (!(dtcr & DTCR_ETBE))
> +			dev_emerg(&imxdi->pdev->dev,
> +				"!! But External Tamper B wasn't enabled. False positive?\n");
> +	}
> +	if (dsr & DSR_WTD) {
> +		dev_emerg(&imxdi->pdev->dev, "!! Wire-mesh Tamper Event\n");
> +		if (!(dtcr & DTCR_WTE))
> +			dev_emerg(&imxdi->pdev->dev,
> +				"!! But wire-mesh Tamper detection wasn't enabled. False positive?\n");
> +	}
> +	if (dsr & DSR_MCO) {
> +		dev_emerg(&imxdi->pdev->dev, "!! Monotonic-counter Overflow Event\n");
> +		if (!(dtcr & DTCR_MOE))
> +			dev_emerg(&imxdi->pdev->dev,
> +				"!! But Monotonic-counter Overflow detection wasn't enabled. False positive?\n");
> +	}
> +	if (dsr & DSR_TCO) {
> +		dev_emerg(&imxdi->pdev->dev, "!! Timer-counter Overflow Event\n");
> +		if (!(dtcr & DTCR_TOE))
> +			dev_emerg(&imxdi->pdev->dev,
> +				"!! But Timer-counter Overflow detection wasn't enabled. False positive?\n");
> +	}
> +}
> +
> +static void di_what_is_to_be_done(struct imxdi_dev *imxdi,
> +						const char *power_supply)
> +{
> +	dev_emerg(&imxdi->pdev->dev, " Please cycle the %s power supply in order to get the DryIce unit working again\n",
            remove that heading space ^

I would also explain that the RTC is part of the DryIce unit.

> +			power_supply);
> +}
> +
> +static int di_handle_failure_state(struct imxdi_dev *imxdi, u32 dsr)
> +{
> +	u32 dcr;
> +
> +	dev_dbg(&imxdi->pdev->dev, "DSR register reports: %08X\n", dsr);
> +
> +	/* report the cause */
> +	di_report_tamper_info(imxdi, dsr);
> +
> +	dcr = __raw_readl(imxdi->ioaddr + DCR);

Even if it is used across the whole driver, I would avoid using
__raw_readx as this makes the driver only working in little endian mode.

> +
> +	if (dcr & DCR_FSHL) {
> +		/* we are out of luck */
> +		di_what_is_to_be_done(imxdi, "battery");
> +		return -ENODEV;
> +	}
> +	/*
> +	 * with the next SYSTEM POR we will transit from the "FAILURE STATE"
> +	 * into the "NON-VALID STATE" + "FAILURE STATE"
> +	 */
> +	di_what_is_to_be_done(imxdi, "main");
> +
> +	return -ENODEV;
> +}
> +
> +static int di_handle_valid_state(struct imxdi_dev *imxdi, u32 dsr)
> +{
> +	/* initialize alarm */
> +	di_write_busy_wait(imxdi, DCAMR_UNSET, DCAMR);
> +	di_write_busy_wait(imxdi, 0, DCALR);
> +
> +	/* clear alarm flag */
> +	if (dsr & DSR_CAF)
> +		di_write_busy_wait(imxdi, DSR_CAF, DSR);
> +
> +	return 0;
> +}
> +
> +static int di_handle_invalid_state(struct imxdi_dev *imxdi, u32 dsr)
> +{
> +	u32 dcr, sec;
> +
> +	/*
> +	 * lets disable all sources which can force the DryIce unit into
> +	 * the "FAILURE STATE" for now
> +	 */
> +	di_write_busy_wait(imxdi, 0x00000000, DTCR);
> +	/* and lets protect them at runtime from any change */
> +	di_write_busy_wait(imxdi, DCR_TDCSL, DCR);
> +
> +	sec = __raw_readl(imxdi->ioaddr + DTCMR);
> +	if (sec != 0)
> +		dev_warn(&imxdi->pdev->dev,
> +			"The security violation has happend at %u seconds\n",

Further improvement for this driver: use both DTCMR and DTCLR to be year
2038 proof

> +			sec);
> +	/*
> +	 * the timer cannot be set/modified if
> +	 * - the TCHL or TCSL bit is set in DCR
> +	 */
> +	dcr = __raw_readl(imxdi->ioaddr + DCR);
> +	if (!(dcr & DCR_TCE)) {
> +		if (dcr & DCR_TCHL) {
> +			/* we are out of luck */
> +			di_what_is_to_be_done(imxdi, "battery");
> +			return -ENODEV;
> +		}
> +		if (dcr & DCR_TCSL) {
> +			di_what_is_to_be_done(imxdi, "main");
> +			return -ENODEV;
> +		}
> +
Unnecessary blank line

> +	}
> +	/*
> +	 * - the timer counter stops/is stopped if
> +	 *   - its overflow flag is set (TCO in DSR)
> +	 *      -> clear overflow bit to make it count again
> +	 *   - NVF is set in DSR
> +	 *      -> clear non-valid bit to make it count again
> +	 *   - its TCE (DCR) is cleared
> +	 *      -> set TCE to make it count
> +	 *   - it was never set before
> +	 *      -> write a time into it (required again if the NVF was set)
> +	 */
> +	/* state handled */
> +	di_write_busy_wait(imxdi, DSR_NVF, DSR);
> +	/* clear overflow flag */
> +	di_write_busy_wait(imxdi, DSR_TCO, DSR);
> +	/* enable the counter */
> +	di_write_busy_wait(imxdi, dcr | DCR_TCE, DCR);
> +	/* set and trigger it to make it count */
> +	di_write_busy_wait(imxdi, sec, DTCMR);
> +
> +	/* now prepare for the valid state */
> +	return di_handle_valid_state(imxdi, __raw_readl(imxdi->ioaddr + DSR));
> +}
> +
> +static int di_handle_invalid_and_failure_state(struct imxdi_dev *imxdi, u32 dsr)
> +{
> +	u32 dcr;
> +
> +	/*
> +	 * now we must first remove the tamper sources in order to get the
> +	 * device out of the "FAILURE STATE"
> +	 * To disable any of the following sources we need to modify the DTCR
> +	 */
> +	if (dsr & (DSR_WTD | DSR_ETBD | DSR_ETAD | DSR_EBD | DSR_SAD |
> +			DSR_TTD | DSR_CTD | DSR_VTD | DSR_MCO | DSR_TCO)) {
> +		dcr = __raw_readl(imxdi->ioaddr + DCR);
> +		if (dcr & DCR_TDCHL) {
> +			/*
> +			 * the tamper register is locked. We cannot disable the
> +			 * tamper detection. The TDCHL can only be reset by a
> +			 * DRYICE POR, but we cannot force a DRYICE POR in
> +			 * softwere because we are still in "FAILURE STATE".
> +			 * We need a DRYICE POR via battery power cycling....
> +			 */
> +			/*
> +			 * out of luck!
> +			 * we cannot disable them without a DRYICE POR
> +			 */
> +			di_what_is_to_be_done(imxdi, "battery");
> +			return -ENODEV;
> +		}
> +		if (dcr & DCR_TDCSL) {
> +			/* a soft lock can be removed by a SYSTEM POR */
> +			di_what_is_to_be_done(imxdi, "main");
> +			return -ENODEV;
> +		}
> +	}
> +
> +	/* disable all sources */
> +	di_write_busy_wait(imxdi, 0x00000000, DTCR);
> +
> +	/* clear the status bits now */
> +	di_write_busy_wait(imxdi, dsr & (DSR_WTD | DSR_ETBD | DSR_ETAD |
> +			DSR_EBD | DSR_SAD | DSR_TTD | DSR_CTD | DSR_VTD |
> +			DSR_MCO | DSR_TCO), DSR);
> +
> +	dsr = __raw_readl(imxdi->ioaddr + DSR);
> +	if ((dsr & ~(DSR_NVF | DSR_SVF | DSR_WBF | DSR_WNF |
> +			DSR_WCF | DSR_WEF)) != 0)
> +		dev_warn(&imxdi->pdev->dev,
> +			"There are still some sources of pain in DSR: %08x!\n",
> +			dsr & ~(DSR_NVF | DSR_SVF | DSR_WBF | DSR_WNF |
> +					DSR_WCF | DSR_WEF));
> +
> +	/*
> +	 * now we are trying to clear the "Security-violation flag" to
> +	 * get the DryIce out of this state
> +	 */
> +	di_write_busy_wait(imxdi, DSR_SVF, DSR);
> +
> +	/* success? */
> +	dsr = __raw_readl(imxdi->ioaddr + DSR);
> +	if (dsr & DSR_SVF) {
> +		dev_crit(&imxdi->pdev->dev,
> +			"Cannot clear the security violation flag. We are ending up in an endless loop!\n");
> +		/* last resourt */
                 resort ^
> +		di_what_is_to_be_done(imxdi, "battery");
> +		return -ENODEV;
> +	}
> +
> +	/*
> +	 * now we have left the "FAILURE STATE" and ending up in the
> +	 * "NON-VALID STATE" time to recover everything
> +	 */
> +	return di_handle_invalid_state(imxdi, dsr);
> +}
> +
> +static int di_handle_state(struct imxdi_dev *imxdi)
> +{
> +	int rc;
> +	u32 dsr;
> +
> +	dsr = __raw_readl(imxdi->ioaddr + DSR);
> +
> +	switch (dsr & (DSR_NVF | DSR_SVF)) {
> +	case DSR_NVF:
> +		dev_warn(&imxdi->pdev->dev, "Invalid stated unit detected\n");
> +		rc = di_handle_invalid_state(imxdi, dsr);
> +		break;
> +	case DSR_SVF:
> +		dev_warn(&imxdi->pdev->dev, "Failure stated unit detected\n");
> +		rc = di_handle_failure_state(imxdi, dsr);
> +		break;
> +	case DSR_NVF | DSR_SVF:
> +		dev_warn(&imxdi->pdev->dev,
> +				"Failure+Invalid stated unit detected\n");
> +		rc = di_handle_invalid_and_failure_state(imxdi, dsr);
> +		break;
> +	default:
> +		dev_notice(&imxdi->pdev->dev, "Unlocked unit detected\n");
> +		rc = di_handle_valid_state(imxdi, dsr);
> +	}
> +
> +	return rc;
> +}
> +
> +
Unnecessary blank line

>  /*
>   * enable a dryice interrupt
>   */
> @@ -491,6 +781,11 @@ static int __init dryice_rtc_probe(struct platform_device *pdev)
>  	/* mask all interrupts */
>  	__raw_writel(0, imxdi->ioaddr + DIER);
>  
> +	rc = di_handle_state(imxdi);
> +	if (rc != 0)
> +		goto err;
> +
> +
Unnecessary blank line

>  	rc = devm_request_irq(&pdev->dev, imxdi->irq, dryice_norm_irq,
>  			IRQF_SHARED, pdev->name, imxdi);
>  	if (rc) {
> @@ -498,44 +793,6 @@ static int __init dryice_rtc_probe(struct platform_device *pdev)
>  		goto err;
>  	}
>  
> -	/* put dryice into valid state */
> -	if (__raw_readl(imxdi->ioaddr + DSR) & DSR_NVF) {
> -		rc = di_write_wait(imxdi, DSR_NVF | DSR_SVF, DSR);

Multiple writes have switched from di_write_wait() which is checking
for a write error to di_write_busy_wait() which is not doing that check
is waiting 200us enough to ensure that the write has been done?

> -		if (rc)
> -			goto err;
> -	}
> -
> -	/* initialize alarm */
> -	rc = di_write_wait(imxdi, DCAMR_UNSET, DCAMR);
> -	if (rc)
> -		goto err;
> -	rc = di_write_wait(imxdi, 0, DCALR);
> -	if (rc)
> -		goto err;
> -
> -	/* clear alarm flag */
> -	if (__raw_readl(imxdi->ioaddr + DSR) & DSR_CAF) {
> -		rc = di_write_wait(imxdi, DSR_CAF, DSR);
> -		if (rc)
> -			goto err;
> -	}
> -
> -	/* the timer won't count if it has never been written to */
> -	if (__raw_readl(imxdi->ioaddr + DTCMR) == 0) {
> -		rc = di_write_wait(imxdi, 0, DTCMR);
> -		if (rc)
> -			goto err;
> -	}
> -
> -	/* start keeping time */
> -	if (!(__raw_readl(imxdi->ioaddr + DCR) & DCR_TCE)) {
> -		rc = di_write_wait(imxdi,
> -				__raw_readl(imxdi->ioaddr + DCR) | DCR_TCE,
> -				DCR);
> -		if (rc)
> -			goto err;
> -	}
> -
>  	platform_set_drvdata(pdev, imxdi);
>  	imxdi->rtc = devm_rtc_device_register(&pdev->dev, pdev->name,
>  				  &dryice_rtc_ops, THIS_MODULE);
> -- 
> 2.1.4
> 

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] [PATCH 2/5] RTC/i.MX/DryIce: add the unit recovery code

[Alexandre Belloni]

On 14/04/2015 at 11:11:53 +0200, Juergen Borleis wrote :
> This code is requiered to recover the unit from a security violation.
      required ^

> Hopefully this code can recover the unit from a hardware related invalid
> state as well.
> 
> Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
> Signed-off-by: Robert Schwebel <rsc@pengutronix.de>
> [rsc: got NDA clearance from Freescale]
> ---
>  drivers/rtc/rtc-imxdi.c | 333 ++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 295 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
> index 8750477..f8abf2b 100644
> --- a/drivers/rtc/rtc-imxdi.c
> +++ b/drivers/rtc/rtc-imxdi.c
> @@ -172,6 +172,296 @@ struct imxdi_dev {
>   * task, we bring back this unit into life.
>   */
>  
> +/* do a write into the unit without interrupt support */
> +static void di_write_busy_wait(const struct imxdi_dev *imxdi, u32 val,
> +								unsigned reg)

Alignment should match the open parenthesis, please fix all occurrences
in your patch.

> +{
> +	/* do the register write */
> +	__raw_writel(val, imxdi->ioaddr + reg);
> +
> +	/*
> +	 * now it takes four 32,768 kHz clock cycles to take
> +	 * the change into effect = 122 us
> +	 */
> +	udelay(200);

As the requirement is not that critical, you may want to use usleep_range()

> +}
> +
> +static void di_report_tamper_info(struct imxdi_dev *imxdi,  u32 dsr)
> +{
> +	u32 dtcr;
> +
> +	dtcr = __raw_readl(imxdi->ioaddr + DTCR);
> +
> +	dev_emerg(&imxdi->pdev->dev, "!! DryIce tamper event detected !!\n");
> +	/* the following flags force a transition into the "FAILURE STATE" */
> +	if (dsr & DSR_VTD) {
> +		dev_emerg(&imxdi->pdev->dev, "!! Voltage Tamper event\n");
> +		if (!(dtcr & DTCR_VTE))
> +			dev_emerg(&imxdi->pdev->dev,
> +				"!! But Voltage Tamper detection wasn't enabled. False positive?\n");

I'm not sure about prefixing all the messages with "!! ". dev_emerg()
seems important enough to be noticed.
I would remove " False positive?". What about
		dev_emerg(&imxdi->pdev->dev,
			  "%sVoltage Tamper event\n",
			  (dtcr & DTCR_VTE) ? "" : "Spurious ");

> +	}
> +	if (dsr & DSR_CTD) {
> +		dev_emerg(&imxdi->pdev->dev, "!! 32768 Hz Clock Tamper Event\n");
> +		if (!(dtcr & DTCR_CTE))
> +			dev_emerg(&imxdi->pdev->dev,
> +				"!! But Clock Tamper detection wasn't enabled. False positive?\n");
> +	}
> +	if (dsr & DSR_TTD) {
> +		dev_emerg(&imxdi->pdev->dev, "!! Temperature Tamper Event\n");
> +		if (!(dtcr & DTCR_TTE))
> +			dev_emerg(&imxdi->pdev->dev,
> +				"!! But Temperature Tamper detection wasn't enabled. False positive?\n");
> +	}
> +	if (dsr & DSR_SAD) {
> +		dev_emerg(&imxdi->pdev->dev, "!! Secure Controller Alarm Event\n");
> +		if (!(dtcr & DTCR_SAIE))
> +			dev_emerg(&imxdi->pdev->dev,
> +				"!! But Secure Controller Alarm detection wasn't enabled. False positive?\n");
> +	}
> +	if (dsr & DSR_EBD) {
> +		dev_emerg(&imxdi->pdev->dev, "!! External Boot Tamper Event\n");
> +		if (!(dtcr & DTCR_EBE))
> +			dev_emerg(&imxdi->pdev->dev,
> +				"!! But External Boot detection wasn't enabled. False positive?\n");
> +	}
> +	if (dsr & DSR_ETAD) {
> +		dev_emerg(&imxdi->pdev->dev, "!! External Tamper A Event\n");
> +		if (!(dtcr & DTCR_ETAE))
> +			dev_emerg(&imxdi->pdev->dev,
> +				"!! But External Tamper A wasn't enabled. False positive?\n");
> +	}
> +	if (dsr & DSR_ETBD) {
> +		dev_emerg(&imxdi->pdev->dev, "!! External Tamper B Event\n");
> +		if (!(dtcr & DTCR_ETBE))
> +			dev_emerg(&imxdi->pdev->dev,
> +				"!! But External Tamper B wasn't enabled. False positive?\n");
> +	}
> +	if (dsr & DSR_WTD) {
> +		dev_emerg(&imxdi->pdev->dev, "!! Wire-mesh Tamper Event\n");
> +		if (!(dtcr & DTCR_WTE))
> +			dev_emerg(&imxdi->pdev->dev,
> +				"!! But wire-mesh Tamper detection wasn't enabled. False positive?\n");
> +	}
> +	if (dsr & DSR_MCO) {
> +		dev_emerg(&imxdi->pdev->dev, "!! Monotonic-counter Overflow Event\n");
> +		if (!(dtcr & DTCR_MOE))
> +			dev_emerg(&imxdi->pdev->dev,
> +				"!! But Monotonic-counter Overflow detection wasn't enabled. False positive?\n");
> +	}
> +	if (dsr & DSR_TCO) {
> +		dev_emerg(&imxdi->pdev->dev, "!! Timer-counter Overflow Event\n");
> +		if (!(dtcr & DTCR_TOE))
> +			dev_emerg(&imxdi->pdev->dev,
> +				"!! But Timer-counter Overflow detection wasn't enabled. False positive?\n");
> +	}
> +}
> +
> +static void di_what_is_to_be_done(struct imxdi_dev *imxdi,
> +						const char *power_supply)
> +{
> +	dev_emerg(&imxdi->pdev->dev, " Please cycle the %s power supply in order to get the DryIce unit working again\n",
            remove that heading space ^

I would also explain that the RTC is part of the DryIce unit.

> +			power_supply);
> +}
> +
> +static int di_handle_failure_state(struct imxdi_dev *imxdi, u32 dsr)
> +{
> +	u32 dcr;
> +
> +	dev_dbg(&imxdi->pdev->dev, "DSR register reports: %08X\n", dsr);
> +
> +	/* report the cause */
> +	di_report_tamper_info(imxdi, dsr);
> +
> +	dcr = __raw_readl(imxdi->ioaddr + DCR);

Even if it is used across the whole driver, I would avoid using
__raw_readx as this makes the driver only working in little endian mode.

> +
> +	if (dcr & DCR_FSHL) {
> +		/* we are out of luck */
> +		di_what_is_to_be_done(imxdi, "battery");
> +		return -ENODEV;
> +	}
> +	/*
> +	 * with the next SYSTEM POR we will transit from the "FAILURE STATE"
> +	 * into the "NON-VALID STATE" + "FAILURE STATE"
> +	 */
> +	di_what_is_to_be_done(imxdi, "main");
> +
> +	return -ENODEV;
> +}
> +
> +static int di_handle_valid_state(struct imxdi_dev *imxdi, u32 dsr)
> +{
> +	/* initialize alarm */
> +	di_write_busy_wait(imxdi, DCAMR_UNSET, DCAMR);
> +	di_write_busy_wait(imxdi, 0, DCALR);
> +
> +	/* clear alarm flag */
> +	if (dsr & DSR_CAF)
> +		di_write_busy_wait(imxdi, DSR_CAF, DSR);
> +
> +	return 0;
> +}
> +
> +static int di_handle_invalid_state(struct imxdi_dev *imxdi, u32 dsr)
> +{
> +	u32 dcr, sec;
> +
> +	/*
> +	 * lets disable all sources which can force the DryIce unit into
> +	 * the "FAILURE STATE" for now
> +	 */
> +	di_write_busy_wait(imxdi, 0x00000000, DTCR);
> +	/* and lets protect them at runtime from any change */
> +	di_write_busy_wait(imxdi, DCR_TDCSL, DCR);
> +
> +	sec = __raw_readl(imxdi->ioaddr + DTCMR);
> +	if (sec != 0)
> +		dev_warn(&imxdi->pdev->dev,
> +			"The security violation has happend at %u seconds\n",

Further improvement for this driver: use both DTCMR and DTCLR to be year
2038 proof

> +			sec);
> +	/*
> +	 * the timer cannot be set/modified if
> +	 * - the TCHL or TCSL bit is set in DCR
> +	 */
> +	dcr = __raw_readl(imxdi->ioaddr + DCR);
> +	if (!(dcr & DCR_TCE)) {
> +		if (dcr & DCR_TCHL) {
> +			/* we are out of luck */
> +			di_what_is_to_be_done(imxdi, "battery");
> +			return -ENODEV;
> +		}
> +		if (dcr & DCR_TCSL) {
> +			di_what_is_to_be_done(imxdi, "main");
> +			return -ENODEV;
> +		}
> +
Unnecessary blank line

> +	}
> +	/*
> +	 * - the timer counter stops/is stopped if
> +	 *   - its overflow flag is set (TCO in DSR)
> +	 *      -> clear overflow bit to make it count again
> +	 *   - NVF is set in DSR
> +	 *      -> clear non-valid bit to make it count again
> +	 *   - its TCE (DCR) is cleared
> +	 *      -> set TCE to make it count
> +	 *   - it was never set before
> +	 *      -> write a time into it (required again if the NVF was set)
> +	 */
> +	/* state handled */
> +	di_write_busy_wait(imxdi, DSR_NVF, DSR);
> +	/* clear overflow flag */
> +	di_write_busy_wait(imxdi, DSR_TCO, DSR);
> +	/* enable the counter */
> +	di_write_busy_wait(imxdi, dcr | DCR_TCE, DCR);
> +	/* set and trigger it to make it count */
> +	di_write_busy_wait(imxdi, sec, DTCMR);
> +
> +	/* now prepare for the valid state */
> +	return di_handle_valid_state(imxdi, __raw_readl(imxdi->ioaddr + DSR));
> +}
> +
> +static int di_handle_invalid_and_failure_state(struct imxdi_dev *imxdi, u32 dsr)
> +{
> +	u32 dcr;
> +
> +	/*
> +	 * now we must first remove the tamper sources in order to get the
> +	 * device out of the "FAILURE STATE"
> +	 * To disable any of the following sources we need to modify the DTCR
> +	 */
> +	if (dsr & (DSR_WTD | DSR_ETBD | DSR_ETAD | DSR_EBD | DSR_SAD |
> +			DSR_TTD | DSR_CTD | DSR_VTD | DSR_MCO | DSR_TCO)) {
> +		dcr = __raw_readl(imxdi->ioaddr + DCR);
> +		if (dcr & DCR_TDCHL) {
> +			/*
> +			 * the tamper register is locked. We cannot disable the
> +			 * tamper detection. The TDCHL can only be reset by a
> +			 * DRYICE POR, but we cannot force a DRYICE POR in
> +			 * softwere because we are still in "FAILURE STATE".
> +			 * We need a DRYICE POR via battery power cycling....
> +			 */
> +			/*
> +			 * out of luck!
> +			 * we cannot disable them without a DRYICE POR
> +			 */
> +			di_what_is_to_be_done(imxdi, "battery");
> +			return -ENODEV;
> +		}
> +		if (dcr & DCR_TDCSL) {
> +			/* a soft lock can be removed by a SYSTEM POR */
> +			di_what_is_to_be_done(imxdi, "main");
> +			return -ENODEV;
> +		}
> +	}
> +
> +	/* disable all sources */
> +	di_write_busy_wait(imxdi, 0x00000000, DTCR);
> +
> +	/* clear the status bits now */
> +	di_write_busy_wait(imxdi, dsr & (DSR_WTD | DSR_ETBD | DSR_ETAD |
> +			DSR_EBD | DSR_SAD | DSR_TTD | DSR_CTD | DSR_VTD |
> +			DSR_MCO | DSR_TCO), DSR);
> +
> +	dsr = __raw_readl(imxdi->ioaddr + DSR);
> +	if ((dsr & ~(DSR_NVF | DSR_SVF | DSR_WBF | DSR_WNF |
> +			DSR_WCF | DSR_WEF)) != 0)
> +		dev_warn(&imxdi->pdev->dev,
> +			"There are still some sources of pain in DSR: %08x!\n",
> +			dsr & ~(DSR_NVF | DSR_SVF | DSR_WBF | DSR_WNF |
> +					DSR_WCF | DSR_WEF));
> +
> +	/*
> +	 * now we are trying to clear the "Security-violation flag" to
> +	 * get the DryIce out of this state
> +	 */
> +	di_write_busy_wait(imxdi, DSR_SVF, DSR);
> +
> +	/* success? */
> +	dsr = __raw_readl(imxdi->ioaddr + DSR);
> +	if (dsr & DSR_SVF) {
> +		dev_crit(&imxdi->pdev->dev,
> +			"Cannot clear the security violation flag. We are ending up in an endless loop!\n");
> +		/* last resourt */
                 resort ^
> +		di_what_is_to_be_done(imxdi, "battery");
> +		return -ENODEV;
> +	}
> +
> +	/*
> +	 * now we have left the "FAILURE STATE" and ending up in the
> +	 * "NON-VALID STATE" time to recover everything
> +	 */
> +	return di_handle_invalid_state(imxdi, dsr);
> +}
> +
> +static int di_handle_state(struct imxdi_dev *imxdi)
> +{
> +	int rc;
> +	u32 dsr;
> +
> +	dsr = __raw_readl(imxdi->ioaddr + DSR);
> +
> +	switch (dsr & (DSR_NVF | DSR_SVF)) {
> +	case DSR_NVF:
> +		dev_warn(&imxdi->pdev->dev, "Invalid stated unit detected\n");
> +		rc = di_handle_invalid_state(imxdi, dsr);
> +		break;
> +	case DSR_SVF:
> +		dev_warn(&imxdi->pdev->dev, "Failure stated unit detected\n");
> +		rc = di_handle_failure_state(imxdi, dsr);
> +		break;
> +	case DSR_NVF | DSR_SVF:
> +		dev_warn(&imxdi->pdev->dev,
> +				"Failure+Invalid stated unit detected\n");
> +		rc = di_handle_invalid_and_failure_state(imxdi, dsr);
> +		break;
> +	default:
> +		dev_notice(&imxdi->pdev->dev, "Unlocked unit detected\n");
> +		rc = di_handle_valid_state(imxdi, dsr);
> +	}
> +
> +	return rc;
> +}
> +
> +
Unnecessary blank line

>  /*
>   * enable a dryice interrupt
>   */
> @@ -491,6 +781,11 @@ static int __init dryice_rtc_probe(struct platform_device *pdev)
>  	/* mask all interrupts */
>  	__raw_writel(0, imxdi->ioaddr + DIER);
>  
> +	rc = di_handle_state(imxdi);
> +	if (rc != 0)
> +		goto err;
> +
> +
Unnecessary blank line

>  	rc = devm_request_irq(&pdev->dev, imxdi->irq, dryice_norm_irq,
>  			IRQF_SHARED, pdev->name, imxdi);
>  	if (rc) {
> @@ -498,44 +793,6 @@ static int __init dryice_rtc_probe(struct platform_device *pdev)
>  		goto err;
>  	}
>  
> -	/* put dryice into valid state */
> -	if (__raw_readl(imxdi->ioaddr + DSR) & DSR_NVF) {
> -		rc = di_write_wait(imxdi, DSR_NVF | DSR_SVF, DSR);

Multiple writes have switched from di_write_wait() which is checking
for a write error to di_write_busy_wait() which is not doing that check
is waiting 200us enough to ensure that the write has been done?

> -		if (rc)
> -			goto err;
> -	}
> -
> -	/* initialize alarm */
> -	rc = di_write_wait(imxdi, DCAMR_UNSET, DCAMR);
> -	if (rc)
> -		goto err;
> -	rc = di_write_wait(imxdi, 0, DCALR);
> -	if (rc)
> -		goto err;
> -
> -	/* clear alarm flag */
> -	if (__raw_readl(imxdi->ioaddr + DSR) & DSR_CAF) {
> -		rc = di_write_wait(imxdi, DSR_CAF, DSR);
> -		if (rc)
> -			goto err;
> -	}
> -
> -	/* the timer won't count if it has never been written to */
> -	if (__raw_readl(imxdi->ioaddr + DTCMR) == 0) {
> -		rc = di_write_wait(imxdi, 0, DTCMR);
> -		if (rc)
> -			goto err;
> -	}
> -
> -	/* start keeping time */
> -	if (!(__raw_readl(imxdi->ioaddr + DCR) & DCR_TCE)) {
> -		rc = di_write_wait(imxdi,
> -				__raw_readl(imxdi->ioaddr + DCR) | DCR_TCE,
> -				DCR);
> -		if (rc)
> -			goto err;
> -	}
> -
>  	platform_set_drvdata(pdev, imxdi);
>  	imxdi->rtc = devm_rtc_device_register(&pdev->dev, pdev->name,
>  				  &dryice_rtc_ops, THIS_MODULE);
> -- 
> 2.1.4
> 

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [rtc-linux] [PATCH 2nd try] RTC/i.MX/DryICE: add recovery routines to the driver

[Alexandre Belloni]

Hi,

On 14/04/2015 at 11:11:51 +0200, Juergen Borleis wrote :
> 2nd try, this time with a cover letter... m(
> 
> The built-in RTC unit on some i.MX SoCs isn't an RTC only. It is also a tamper
> monitor unit which can keep some keys. When it does its tamper detection job

Does it have more functions? I would say that it also holds some keys
but I don't have a handy Freescale representative to contact ;)

I'm fine getting that unlocking done in the RTC driver but maybe in the
future, it will be necessary to handle that in an MFD driver when adding
support for the other functions.

> and a tamper violation is detected, this RTC unit locks completely including
> the real-time counter. In this state the unit is completely useless. The only
> way to bring it out of this locked state is a power on reset. At the next boot
> time some flags signals the tamper violation and a specific register access
> sequence must be done to finaly bring this unit into life again. Until this is
> done, there is no way to use it again as an RTC.
> But also without any enabled tamper detection sometimes this unit tends to
> lock. And in this case the same steps must be done to bring it into life
> again.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [rtc-linux] [PATCH 2nd try] RTC/i.MX/DryICE: add recovery routines to the driver

[Alexandre Belloni]

Hi,

On 14/04/2015 at 11:11:51 +0200, Juergen Borleis wrote :
> 2nd try, this time with a cover letter... m(
> 
> The built-in RTC unit on some i.MX SoCs isn't an RTC only. It is also a tamper
> monitor unit which can keep some keys. When it does its tamper detection job

Does it have more functions? I would say that it also holds some keys
but I don't have a handy Freescale representative to contact ;)

I'm fine getting that unlocking done in the RTC driver but maybe in the
future, it will be necessary to handle that in an MFD driver when adding
support for the other functions.

> and a tamper violation is detected, this RTC unit locks completely including
> the real-time counter. In this state the unit is completely useless. The only
> way to bring it out of this locked state is a power on reset. At the next boot
> time some flags signals the tamper violation and a specific register access
> sequence must be done to finaly bring this unit into life again. Until this is
> done, there is no way to use it again as an RTC.
> But also without any enabled tamper detection sometimes this unit tends to
> lock. And in this case the same steps must be done to bring it into life
> again.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] [PATCH 2nd try] RTC/i.MX/DryICE: add recovery routines to the driver

[Alexandre Belloni]

Hi,

On 14/04/2015 at 11:11:51 +0200, Juergen Borleis wrote :
> 2nd try, this time with a cover letter... m(
> 
> The built-in RTC unit on some i.MX SoCs isn't an RTC only. It is also a tamper
> monitor unit which can keep some keys. When it does its tamper detection job

Does it have more functions? I would say that it also holds some keys
but I don't have a handy Freescale representative to contact ;)

I'm fine getting that unlocking done in the RTC driver but maybe in the
future, it will be necessary to handle that in an MFD driver when adding
support for the other functions.

> and a tamper violation is detected, this RTC unit locks completely including
> the real-time counter. In this state the unit is completely useless. The only
> way to bring it out of this locked state is a power on reset. At the next boot
> time some flags signals the tamper violation and a specific register access
> sequence must be done to finaly bring this unit into life again. Until this is
> done, there is no way to use it again as an RTC.
> But also without any enabled tamper detection sometimes this unit tends to
> lock. And in this case the same steps must be done to bring it into life
> again.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [rtc-linux] [PATCH 4/5] RTC/i.MX/DryIce: when locked, do not fail silently

[Alexandre Belloni]

Please always include a commit message.

On 14/04/2015 at 11:11:55 +0200, Juergen Borleis wrote :
> Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
> Signed-off-by: Robert Schwebel <rsc@pengutronix.de>
> [rsc: got NDA clearance from Freescale]
> ---
>  drivers/rtc/rtc-imxdi.c | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
> index b04c64f..de1a2e4 100644
> --- a/drivers/rtc/rtc-imxdi.c
> +++ b/drivers/rtc/rtc-imxdi.c
> @@ -581,14 +581,36 @@ static int dryice_rtc_read_time(struct device *dev, struct rtc_time *tm)
>  static int dryice_rtc_set_mmss(struct device *dev, unsigned long secs)
>  {
>  	struct imxdi_dev *imxdi = dev_get_drvdata(dev);
> +	u32 dcr, dsr;
>  	int rc;
>  
> +	dcr = __raw_readl(imxdi->ioaddr + DCR);
> +	dsr = __raw_readl(imxdi->ioaddr + DSR);
> +
> +	if (!(dcr & DCR_TCE) || (dsr & DSR_SVF)) {
> +		if (dcr & DCR_TCHL) {
> +			/* we are even more out of luck */
> +			di_what_is_to_be_done(imxdi, "battery");
> +			return -EPERM;
> +		}
> +		if ((dcr & DCR_TCSL) || (dsr & DSR_SVF)) {
> +			/* we are out of luck for now */
> +			di_what_is_to_be_done(imxdi, "main");
> +			return -EPERM;
> +		}
> +	}
> +
>  	/* zero the fractional part first */
>  	rc = di_write_wait(imxdi, 0, DTCLR);
> -	if (rc == 0)
> -		rc = di_write_wait(imxdi, secs, DTCMR);
> +	if (rc != 0)
> +		return rc;
>  
> -	return rc;
> +	rc = di_write_wait(imxdi, secs, DTCMR);
> +	if (rc != 0)
> +		return rc;
> +
> +	return di_write_wait(imxdi, __raw_readl(imxdi->ioaddr + DCR) |
> +								DCR_TCE, DCR);
>  }
>  
>  static int dryice_rtc_alarm_irq_enable(struct device *dev,
> -- 
> 2.1.4
> 
> -- 
> -- 
> You received this message because you are subscribed to "rtc-linux".
> Membership options at http://groups.google.com/group/rtc-linux .
> Please read http://groups.google.com/group/rtc-linux/web/checklist
> before submitting a driver.
> --- 
> You received this message because you are subscribed to the Google Groups "rtc-linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [rtc-linux] [PATCH 4/5] RTC/i.MX/DryIce: when locked, do not fail silently

[Alexandre Belloni]

Please always include a commit message.

On 14/04/2015 at 11:11:55 +0200, Juergen Borleis wrote :
> Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
> Signed-off-by: Robert Schwebel <rsc@pengutronix.de>
> [rsc: got NDA clearance from Freescale]
> ---
>  drivers/rtc/rtc-imxdi.c | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
> index b04c64f..de1a2e4 100644
> --- a/drivers/rtc/rtc-imxdi.c
> +++ b/drivers/rtc/rtc-imxdi.c
> @@ -581,14 +581,36 @@ static int dryice_rtc_read_time(struct device *dev, struct rtc_time *tm)
>  static int dryice_rtc_set_mmss(struct device *dev, unsigned long secs)
>  {
>  	struct imxdi_dev *imxdi = dev_get_drvdata(dev);
> +	u32 dcr, dsr;
>  	int rc;
>  
> +	dcr = __raw_readl(imxdi->ioaddr + DCR);
> +	dsr = __raw_readl(imxdi->ioaddr + DSR);
> +
> +	if (!(dcr & DCR_TCE) || (dsr & DSR_SVF)) {
> +		if (dcr & DCR_TCHL) {
> +			/* we are even more out of luck */
> +			di_what_is_to_be_done(imxdi, "battery");
> +			return -EPERM;
> +		}
> +		if ((dcr & DCR_TCSL) || (dsr & DSR_SVF)) {
> +			/* we are out of luck for now */
> +			di_what_is_to_be_done(imxdi, "main");
> +			return -EPERM;
> +		}
> +	}
> +
>  	/* zero the fractional part first */
>  	rc = di_write_wait(imxdi, 0, DTCLR);
> -	if (rc == 0)
> -		rc = di_write_wait(imxdi, secs, DTCMR);
> +	if (rc != 0)
> +		return rc;
>  
> -	return rc;
> +	rc = di_write_wait(imxdi, secs, DTCMR);
> +	if (rc != 0)
> +		return rc;
> +
> +	return di_write_wait(imxdi, __raw_readl(imxdi->ioaddr + DCR) |
> +								DCR_TCE, DCR);
>  }
>  
>  static int dryice_rtc_alarm_irq_enable(struct device *dev,
> -- 
> 2.1.4
> 
> -- 
> -- 
> You received this message because you are subscribed to "rtc-linux".
> Membership options at http://groups.google.com/group/rtc-linux .
> Please read http://groups.google.com/group/rtc-linux/web/checklist
> before submitting a driver.
> --- 
> You received this message because you are subscribed to the Google Groups "rtc-linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] [PATCH 4/5] RTC/i.MX/DryIce: when locked, do not fail silently

[Alexandre Belloni]

Please always include a commit message.

On 14/04/2015 at 11:11:55 +0200, Juergen Borleis wrote :
> Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
> Signed-off-by: Robert Schwebel <rsc@pengutronix.de>
> [rsc: got NDA clearance from Freescale]
> ---
>  drivers/rtc/rtc-imxdi.c | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
> index b04c64f..de1a2e4 100644
> --- a/drivers/rtc/rtc-imxdi.c
> +++ b/drivers/rtc/rtc-imxdi.c
> @@ -581,14 +581,36 @@ static int dryice_rtc_read_time(struct device *dev, struct rtc_time *tm)
>  static int dryice_rtc_set_mmss(struct device *dev, unsigned long secs)
>  {
>  	struct imxdi_dev *imxdi = dev_get_drvdata(dev);
> +	u32 dcr, dsr;
>  	int rc;
>  
> +	dcr = __raw_readl(imxdi->ioaddr + DCR);
> +	dsr = __raw_readl(imxdi->ioaddr + DSR);
> +
> +	if (!(dcr & DCR_TCE) || (dsr & DSR_SVF)) {
> +		if (dcr & DCR_TCHL) {
> +			/* we are even more out of luck */
> +			di_what_is_to_be_done(imxdi, "battery");
> +			return -EPERM;
> +		}
> +		if ((dcr & DCR_TCSL) || (dsr & DSR_SVF)) {
> +			/* we are out of luck for now */
> +			di_what_is_to_be_done(imxdi, "main");
> +			return -EPERM;
> +		}
> +	}
> +
>  	/* zero the fractional part first */
>  	rc = di_write_wait(imxdi, 0, DTCLR);
> -	if (rc == 0)
> -		rc = di_write_wait(imxdi, secs, DTCMR);
> +	if (rc != 0)
> +		return rc;
>  
> -	return rc;
> +	rc = di_write_wait(imxdi, secs, DTCMR);
> +	if (rc != 0)
> +		return rc;
> +
> +	return di_write_wait(imxdi, __raw_readl(imxdi->ioaddr + DCR) |
> +								DCR_TCE, DCR);
>  }
>  
>  static int dryice_rtc_alarm_irq_enable(struct device *dev,
> -- 
> 2.1.4
> 
> -- 
> -- 
> You received this message because you are subscribed to "rtc-linux".
> Membership options at http://groups.google.com/group/rtc-linux .
> Please read http://groups.google.com/group/rtc-linux/web/checklist
> before submitting a driver.
> --- 
> You received this message because you are subscribed to the Google Groups "rtc-linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe at googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [rtc-linux] [PATCH 2/5] RTC/i.MX/DryIce: add the unit recovery code

[Alexandre Belloni]

On 22/04/2015 at 01:14:11 +0200, Alexandre Belloni wrote :
> > +	sec = __raw_readl(imxdi->ioaddr + DTCMR);
> > +	if (sec != 0)
> > +		dev_warn(&imxdi->pdev->dev,
> > +			"The security violation has happend at %u seconds\n",
> 
> Further improvement for this driver: use both DTCMR and DTCLR to be year
> 2038 proof

Please forget that comment I just realized that seconds were counted in
the most significant part...


-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [rtc-linux] [PATCH 2/5] RTC/i.MX/DryIce: add the unit recovery code

[Alexandre Belloni]

On 22/04/2015 at 01:14:11 +0200, Alexandre Belloni wrote :
> > +	sec = __raw_readl(imxdi->ioaddr + DTCMR);
> > +	if (sec != 0)
> > +		dev_warn(&imxdi->pdev->dev,
> > +			"The security violation has happend at %u seconds\n",
> 
> Further improvement for this driver: use both DTCMR and DTCLR to be year
> 2038 proof

Please forget that comment I just realized that seconds were counted in
the most significant part...


-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] [PATCH 2/5] RTC/i.MX/DryIce: add the unit recovery code

[Alexandre Belloni]

On 22/04/2015 at 01:14:11 +0200, Alexandre Belloni wrote :
> > +	sec = __raw_readl(imxdi->ioaddr + DTCMR);
> > +	if (sec != 0)
> > +		dev_warn(&imxdi->pdev->dev,
> > +			"The security violation has happend at %u seconds\n",
> 
> Further improvement for this driver: use both DTCMR and DTCLR to be year
> 2038 proof

Please forget that comment I just realized that seconds were counted in
the most significant part...


-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [rtc-linux] [PATCH 1/5] RTC/i.MX/DryIce: add some background info about the states the machine can be in

[Juergen Borleis]

Hi Alexandre,

On Wednesday 22 April 2015 00:09:42 Alexandre Belloni wrote:
> [...]
> > ---
> >  drivers/rtc/rtc-imxdi.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 43 insertions(+)
> >
> > diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
> > index c666eab..8750477 100644
> > --- a/drivers/rtc/rtc-imxdi.c
> > +++ b/drivers/rtc/rtc-imxdi.c
> > @@ -129,6 +129,49 @@ struct imxdi_dev {
> >  	struct work_struct work;
> >  };
> >
> > +/* Some background:
> > + *
> > + * The DryIce unit is a complex security/tamper monitor device. To be able do
> > + * its job in a useful manner it runs a bigger statemachine to bring it into
> > + * security/tamper failure state and once again to bring it out of this state.
> > + * 
> > + * This unit can be in one of three states:
> > + *
> > + * - "NON-VALID STATE"
> > + *   always after the battery power was removed
> > + * - "FAILURE STATE"
> > + *   if one of the enabled security events have happend
>
>                                  has happened ^
>
> > + * - "VALID STATE"
> > + *   if the unit works as expected
> > + *
> > + * Everything stops when the unit enters the failure state including the
> > RTC + * counter (to be able to detect the time the security event
> > happend).
>
>                                                        happened ^
> [...]

Thanks for the feedback. Fixed in the next version.

Regards,
Juergen
-- 
Pengutronix e.K.                              | Juergen Borleis             |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |

Re: [rtc-linux] [PATCH 1/5] RTC/i.MX/DryIce: add some background info about the states the machine can be in

[Juergen Borleis]

Hi Alexandre,

On Wednesday 22 April 2015 00:09:42 Alexandre Belloni wrote:
> [...]
> > ---
> >  drivers/rtc/rtc-imxdi.c | 43 +++++++++++++++++++++++++++++++++++++++++=
++
> >  1 file changed, 43 insertions(+)
> >
> > diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
> > index c666eab..8750477 100644
> > --- a/drivers/rtc/rtc-imxdi.c
> > +++ b/drivers/rtc/rtc-imxdi.c
> > @@ -129,6 +129,49 @@ struct imxdi_dev {
> >  	struct work_struct work;
> >  };
> >
> > +/* Some background:
> > + *
> > + * The DryIce unit is a complex security/tamper monitor device. To be =
able do
> > + * its job in a useful manner it runs a bigger statemachine to bring i=
t into
> > + * security/tamper failure state and once again to bring it out of thi=
s state.
> > + *=20
> > + * This unit can be in one of three states:
> > + *
> > + * - "NON-VALID STATE"
> > + *   always after the battery power was removed
> > + * - "FAILURE STATE"
> > + *   if one of the enabled security events have happend
>
>                                  has happened ^
>
> > + * - "VALID STATE"
> > + *   if the unit works as expected
> > + *
> > + * Everything stops when the unit enters the failure state including t=
he
> > RTC + * counter (to be able to detect the time the security event
> > happend).
>
>                                                        happened ^
> [...]

Thanks for the feedback. Fixed in the next version.

Regards,
Juergen
=2D-=20
Pengutronix e.K. =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0| Juergen Borleis =A0 =A0 =A0 =A0 =A0 =A0 |
Industrial Linux Solutions               =A0 =A0 =A0| http://www.pengutroni=
x.de/  |

[rtc-linux] [PATCH 1/5] RTC/i.MX/DryIce: add some background info about the states the machine can be in

[Juergen Borleis]

Hi Alexandre,

On Wednesday 22 April 2015 00:09:42 Alexandre Belloni wrote:
> [...]
> > ---
> >  drivers/rtc/rtc-imxdi.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 43 insertions(+)
> >
> > diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
> > index c666eab..8750477 100644
> > --- a/drivers/rtc/rtc-imxdi.c
> > +++ b/drivers/rtc/rtc-imxdi.c
> > @@ -129,6 +129,49 @@ struct imxdi_dev {
> >  	struct work_struct work;
> >  };
> >
> > +/* Some background:
> > + *
> > + * The DryIce unit is a complex security/tamper monitor device. To be able do
> > + * its job in a useful manner it runs a bigger statemachine to bring it into
> > + * security/tamper failure state and once again to bring it out of this state.
> > + * 
> > + * This unit can be in one of three states:
> > + *
> > + * - "NON-VALID STATE"
> > + *   always after the battery power was removed
> > + * - "FAILURE STATE"
> > + *   if one of the enabled security events have happend
>
>                                  has happened ^
>
> > + * - "VALID STATE"
> > + *   if the unit works as expected
> > + *
> > + * Everything stops when the unit enters the failure state including the
> > RTC + * counter (to be able to detect the time the security event
> > happend).
>
>                                                        happened ^
> [...]

Thanks for the feedback. Fixed in the next version.

Regards,
Juergen
-- 
Pengutronix e.K. ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?| Juergen Borleis ? ? ? ? ? ? |
Industrial Linux Solutions               ? ? ?| http://www.pengutronix.de/  |

Re: [rtc-linux] [PATCH 2/5] RTC/i.MX/DryIce: add the unit recovery code

[Juergen Borleis]

Hi Alexandre,

On Wednesday 22 April 2015 01:14:11 Alexandre Belloni wrote:
> On 14/04/2015 at 11:11:53 +0200, Juergen Borleis wrote :
> > This code is requiered to recover the unit from a security violation.
>
>       required ^

Sure :)

> > [...]
> > +/* do a write into the unit without interrupt support */
> > +static void di_write_busy_wait(const struct imxdi_dev *imxdi, u32 val,
> > +								unsigned reg)
>
> Alignment should match the open parenthesis, please fix all occurrences
> in your patch.

Okay.

> > +{
> > +	/* do the register write */
> > +	__raw_writel(val, imxdi->ioaddr + reg);
> > +
> > +	/*
> > +	 * now it takes four 32,768 kHz clock cycles to take
> > +	 * the change into effect = 122 us
> > +	 */
> > +	udelay(200);
>
> As the requirement is not that critical, you may want to use usleep_range()

Good point. Will change it.

> > +}
> > +
> > +static void di_report_tamper_info(struct imxdi_dev *imxdi,  u32 dsr)
> > +{
> > +	u32 dtcr;
> > +
> > +	dtcr = __raw_readl(imxdi->ioaddr + DTCR);
> > +
> > +	dev_emerg(&imxdi->pdev->dev, "!! DryIce tamper event detected !!\n");
> > +	/* the following flags force a transition into the "FAILURE STATE" */
> > +	if (dsr & DSR_VTD) {
> > +		dev_emerg(&imxdi->pdev->dev, "!! Voltage Tamper event\n");
> > +		if (!(dtcr & DTCR_VTE))
> > +			dev_emerg(&imxdi->pdev->dev,
> > +				"!! But Voltage Tamper detection wasn't enabled. False positive?\n");
>
> I'm not sure about prefixing all the messages with "!! ". dev_emerg()
> seems important enough to be noticed.
> I would remove " False positive?". What about
> 		dev_emerg(&imxdi->pdev->dev,
> 			  "%sVoltage Tamper event\n",
> 			  (dtcr & DTCR_VTE) ? "" : "Spurious ");

"spurious" sounds better. And I will change the messages.

> [...]
> > +static void di_what_is_to_be_done(struct imxdi_dev *imxdi,
> > +						const char *power_supply)
> > +{
> > +	dev_emerg(&imxdi->pdev->dev, " Please cycle the %s power supply in order to get the DryIce unit working again\n",
>
>             remove that heading space ^

Yes.

> I would also explain that the RTC is part of the DryIce unit.

Within this message? Makes sense. Will do so.

> [...]
> > +			sec);
> > +	/*
> > +	 * the timer cannot be set/modified if
> > +	 * - the TCHL or TCSL bit is set in DCR
> > +	 */
> > +	dcr = __raw_readl(imxdi->ioaddr + DCR);
> > +	if (!(dcr & DCR_TCE)) {
> > +		if (dcr & DCR_TCHL) {
> > +			/* we are out of luck */
> > +			di_what_is_to_be_done(imxdi, "battery");
> > +			return -ENODEV;
> > +		}
> > +		if (dcr & DCR_TCSL) {
> > +			di_what_is_to_be_done(imxdi, "main");
> > +			return -ENODEV;
> > +		}
> > +
>
> Unnecessary blank line

Ups. Made accidentally.

> [...]
> > +	/* success? */
> > +	dsr = __raw_readl(imxdi->ioaddr + DSR);
> > +	if (dsr & DSR_SVF) {
> > +		dev_crit(&imxdi->pdev->dev,
> > +			"Cannot clear the security violation flag. We are ending up in an  endless loop!\n");
> > +		/* last resourt */ 
>
>                  resort ^

.oO(note to myself: update dictionary...)

> [...]
> > @@ -498,44 +793,6 @@ static int __init dryice_rtc_probe(struct platform_device *pdev) goto err;
> >  	}
> >
> > -	/* put dryice into valid state */
> > -	if (__raw_readl(imxdi->ioaddr + DSR) & DSR_NVF) {
> > -		rc = di_write_wait(imxdi, DSR_NVF | DSR_SVF, DSR);
>
> Multiple writes have switched from di_write_wait() which is checking
> for a write error to di_write_busy_wait() which is not doing that check
> is waiting 200us enough to ensure that the write has been done?

Each write requires four 32 kHz clock cycles. So the 200 us should be enough.
But I will take a closer look for what have to be really done in the case of
an access error.

Regards,
Juergen

-- 
Pengutronix e.K.                              | Juergen Borleis             |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |

Re: [rtc-linux] [PATCH 2/5] RTC/i.MX/DryIce: add the unit recovery code

[Juergen Borleis]

Hi Alexandre,

On Wednesday 22 April 2015 01:14:11 Alexandre Belloni wrote:
> On 14/04/2015 at 11:11:53 +0200, Juergen Borleis wrote :
> > This code is requiered to recover the unit from a security violation.
>
>       required ^

Sure :)

> > [...]
> > +/* do a write into the unit without interrupt support */
> > +static void di_write_busy_wait(const struct imxdi_dev *imxdi, u32 val,
> > +								unsigned reg)
>
> Alignment should match the open parenthesis, please fix all occurrences
> in your patch.

Okay.

> > +{
> > +	/* do the register write */
> > +	__raw_writel(val, imxdi->ioaddr + reg);
> > +
> > +	/*
> > +	 * now it takes four 32,768 kHz clock cycles to take
> > +	 * the change into effect =3D 122 us
> > +	 */
> > +	udelay(200);
>
> As the requirement is not that critical, you may want to use usleep_range=
()

Good point. Will change it.

> > +}
> > +
> > +static void di_report_tamper_info(struct imxdi_dev *imxdi,  u32 dsr)
> > +{
> > +	u32 dtcr;
> > +
> > +	dtcr =3D __raw_readl(imxdi->ioaddr + DTCR);
> > +
> > +	dev_emerg(&imxdi->pdev->dev, "!! DryIce tamper event detected !!\n");
> > +	/* the following flags force a transition into the "FAILURE STATE" */
> > +	if (dsr & DSR_VTD) {
> > +		dev_emerg(&imxdi->pdev->dev, "!! Voltage Tamper event\n");
> > +		if (!(dtcr & DTCR_VTE))
> > +			dev_emerg(&imxdi->pdev->dev,
> > +				"!! But Voltage Tamper detection wasn't enabled. False positive?\n=
");
>
> I'm not sure about prefixing all the messages with "!! ". dev_emerg()
> seems important enough to be noticed.
> I would remove " False positive?". What about
> 		dev_emerg(&imxdi->pdev->dev,
> 			  "%sVoltage Tamper event\n",
> 			  (dtcr & DTCR_VTE) ? "" : "Spurious ");

"spurious" sounds better. And I will change the messages.

> [...]
> > +static void di_what_is_to_be_done(struct imxdi_dev *imxdi,
> > +						const char *power_supply)
> > +{
> > +	dev_emerg(&imxdi->pdev->dev, " Please cycle the %s power supply in or=
der to get the DryIce unit working again\n",
>
>             remove that heading space ^

Yes.

> I would also explain that the RTC is part of the DryIce unit.

Within this message? Makes sense. Will do so.

> [...]
> > +			sec);
> > +	/*
> > +	 * the timer cannot be set/modified if
> > +	 * - the TCHL or TCSL bit is set in DCR
> > +	 */
> > +	dcr =3D __raw_readl(imxdi->ioaddr + DCR);
> > +	if (!(dcr & DCR_TCE)) {
> > +		if (dcr & DCR_TCHL) {
> > +			/* we are out of luck */
> > +			di_what_is_to_be_done(imxdi, "battery");
> > +			return -ENODEV;
> > +		}
> > +		if (dcr & DCR_TCSL) {
> > +			di_what_is_to_be_done(imxdi, "main");
> > +			return -ENODEV;
> > +		}
> > +
>
> Unnecessary blank line

Ups. Made accidentally.

> [...]
> > +	/* success? */
> > +	dsr =3D __raw_readl(imxdi->ioaddr + DSR);
> > +	if (dsr & DSR_SVF) {
> > +		dev_crit(&imxdi->pdev->dev,
> > +			"Cannot clear the security violation flag. We are ending up in an  =
endless loop!\n");
> > +		/* last resourt */=20
>
>                  resort ^

=2EoO(note to myself: update dictionary...)

> [...]
> > @@ -498,44 +793,6 @@ static int __init dryice_rtc_probe(struct platform=
_device *pdev) goto err;
> >  	}
> >
> > -	/* put dryice into valid state */
> > -	if (__raw_readl(imxdi->ioaddr + DSR) & DSR_NVF) {
> > -		rc =3D di_write_wait(imxdi, DSR_NVF | DSR_SVF, DSR);
>
> Multiple writes have switched from di_write_wait() which is checking
> for a write error to di_write_busy_wait() which is not doing that check
> is waiting 200us enough to ensure that the write has been done?

Each write requires four 32 kHz clock cycles. So the 200 us should be enoug=
h.
But I will take a closer look for what have to be really done in the case of
an access error.

Regards,
Juergen

=2D-=20
Pengutronix e.K. =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0| Juergen Borleis =A0 =A0 =A0 =A0 =A0 =A0 |
Industrial Linux Solutions               =A0 =A0 =A0| http://www.pengutroni=
x.de/  |

[rtc-linux] [PATCH 2/5] RTC/i.MX/DryIce: add the unit recovery code

[Juergen Borleis]

Hi Alexandre,

On Wednesday 22 April 2015 01:14:11 Alexandre Belloni wrote:
> On 14/04/2015 at 11:11:53 +0200, Juergen Borleis wrote :
> > This code is requiered to recover the unit from a security violation.
>
>       required ^

Sure :)

> > [...]
> > +/* do a write into the unit without interrupt support */
> > +static void di_write_busy_wait(const struct imxdi_dev *imxdi, u32 val,
> > +								unsigned reg)
>
> Alignment should match the open parenthesis, please fix all occurrences
> in your patch.

Okay.

> > +{
> > +	/* do the register write */
> > +	__raw_writel(val, imxdi->ioaddr + reg);
> > +
> > +	/*
> > +	 * now it takes four 32,768 kHz clock cycles to take
> > +	 * the change into effect = 122 us
> > +	 */
> > +	udelay(200);
>
> As the requirement is not that critical, you may want to use usleep_range()

Good point. Will change it.

> > +}
> > +
> > +static void di_report_tamper_info(struct imxdi_dev *imxdi,  u32 dsr)
> > +{
> > +	u32 dtcr;
> > +
> > +	dtcr = __raw_readl(imxdi->ioaddr + DTCR);
> > +
> > +	dev_emerg(&imxdi->pdev->dev, "!! DryIce tamper event detected !!\n");
> > +	/* the following flags force a transition into the "FAILURE STATE" */
> > +	if (dsr & DSR_VTD) {
> > +		dev_emerg(&imxdi->pdev->dev, "!! Voltage Tamper event\n");
> > +		if (!(dtcr & DTCR_VTE))
> > +			dev_emerg(&imxdi->pdev->dev,
> > +				"!! But Voltage Tamper detection wasn't enabled. False positive?\n");
>
> I'm not sure about prefixing all the messages with "!! ". dev_emerg()
> seems important enough to be noticed.
> I would remove " False positive?". What about
> 		dev_emerg(&imxdi->pdev->dev,
> 			  "%sVoltage Tamper event\n",
> 			  (dtcr & DTCR_VTE) ? "" : "Spurious ");

"spurious" sounds better. And I will change the messages.

> [...]
> > +static void di_what_is_to_be_done(struct imxdi_dev *imxdi,
> > +						const char *power_supply)
> > +{
> > +	dev_emerg(&imxdi->pdev->dev, " Please cycle the %s power supply in order to get the DryIce unit working again\n",
>
>             remove that heading space ^

Yes.

> I would also explain that the RTC is part of the DryIce unit.

Within this message? Makes sense. Will do so.

> [...]
> > +			sec);
> > +	/*
> > +	 * the timer cannot be set/modified if
> > +	 * - the TCHL or TCSL bit is set in DCR
> > +	 */
> > +	dcr = __raw_readl(imxdi->ioaddr + DCR);
> > +	if (!(dcr & DCR_TCE)) {
> > +		if (dcr & DCR_TCHL) {
> > +			/* we are out of luck */
> > +			di_what_is_to_be_done(imxdi, "battery");
> > +			return -ENODEV;
> > +		}
> > +		if (dcr & DCR_TCSL) {
> > +			di_what_is_to_be_done(imxdi, "main");
> > +			return -ENODEV;
> > +		}
> > +
>
> Unnecessary blank line

Ups. Made accidentally.

> [...]
> > +	/* success? */
> > +	dsr = __raw_readl(imxdi->ioaddr + DSR);
> > +	if (dsr & DSR_SVF) {
> > +		dev_crit(&imxdi->pdev->dev,
> > +			"Cannot clear the security violation flag. We are ending up in an  endless loop!\n");
> > +		/* last resourt */ 
>
>                  resort ^

.oO(note to myself: update dictionary...)

> [...]
> > @@ -498,44 +793,6 @@ static int __init dryice_rtc_probe(struct platform_device *pdev) goto err;
> >  	}
> >
> > -	/* put dryice into valid state */
> > -	if (__raw_readl(imxdi->ioaddr + DSR) & DSR_NVF) {
> > -		rc = di_write_wait(imxdi, DSR_NVF | DSR_SVF, DSR);
>
> Multiple writes have switched from di_write_wait() which is checking
> for a write error to di_write_busy_wait() which is not doing that check
> is waiting 200us enough to ensure that the write has been done?

Each write requires four 32 kHz clock cycles. So the 200 us should be enough.
But I will take a closer look for what have to be really done in the case of
an access error.

Regards,
Juergen

-- 
Pengutronix e.K. ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?| Juergen Borleis ? ? ? ? ? ? |
Industrial Linux Solutions               ? ? ?| http://www.pengutronix.de/  |

Re: [rtc-linux] [PATCH 2nd try] RTC/i.MX/DryICE: add recovery routines to the driver

[Juergen Borleis]

Hi Alexandre,

On Wednesday 22 April 2015 01:26:15 Alexandre Belloni wrote:
> On 14/04/2015 at 11:11:51 +0200, Juergen Borleis wrote :
> > 2nd try, this time with a cover letter... m(
> >
> > The built-in RTC unit on some i.MX SoCs isn't an RTC only. It is also a
> > tamper monitor unit which can keep some keys. When it does its tamper
> > detection job
>
> Does it have more functions? I would say that it also holds some keys
> but I don't have a handy Freescale representative to contact ;)

Yes it can hold some keys and will delete them if it detects a security 
violation via its tamper detection feature.

> I'm fine getting that unlocking done in the RTC driver but maybe in the
> future, it will be necessary to handle that in an MFD driver when adding
> support for the other functions.

This change set just makes the RTC work again. Observation was, *sometimes* and 
*somehow* this unit tends to lock even if no of these tamper detection 
features were enabled nor the externals signals (to detect these security 
violations) were used.

Regards,
Juergen

-- 
Pengutronix e.K.                              | Juergen Borleis             |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |

Re: [rtc-linux] [PATCH 2nd try] RTC/i.MX/DryICE: add recovery routines to the driver

[Juergen Borleis]

Hi Alexandre,

On Wednesday 22 April 2015 01:26:15 Alexandre Belloni wrote:
> On 14/04/2015 at 11:11:51 +0200, Juergen Borleis wrote :
> > 2nd try, this time with a cover letter... m(
> >
> > The built-in RTC unit on some i.MX SoCs isn't an RTC only. It is also a
> > tamper monitor unit which can keep some keys. When it does its tamper
> > detection job
>
> Does it have more functions? I would say that it also holds some keys
> but I don't have a handy Freescale representative to contact ;)

Yes it can hold some keys and will delete them if it detects a security=20
violation via its tamper detection feature.

> I'm fine getting that unlocking done in the RTC driver but maybe in the
> future, it will be necessary to handle that in an MFD driver when adding
> support for the other functions.

This change set just makes the RTC work again. Observation was, *sometimes*=
 and=20
*somehow* this unit tends to lock even if no of these tamper detection=20
features were enabled nor the externals signals (to detect these security=20
violations) were used.

Regards,
Juergen

=2D-=20
Pengutronix e.K. =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0| Juergen Borleis =A0 =A0 =A0 =A0 =A0 =A0 |
Industrial Linux Solutions               =A0 =A0 =A0| http://www.pengutroni=
x.de/  |

[rtc-linux] [PATCH 2nd try] RTC/i.MX/DryICE: add recovery routines to the driver

[Juergen Borleis]

Hi Alexandre,

On Wednesday 22 April 2015 01:26:15 Alexandre Belloni wrote:
> On 14/04/2015 at 11:11:51 +0200, Juergen Borleis wrote :
> > 2nd try, this time with a cover letter... m(
> >
> > The built-in RTC unit on some i.MX SoCs isn't an RTC only. It is also a
> > tamper monitor unit which can keep some keys. When it does its tamper
> > detection job
>
> Does it have more functions? I would say that it also holds some keys
> but I don't have a handy Freescale representative to contact ;)

Yes it can hold some keys and will delete them if it detects a security 
violation via its tamper detection feature.

> I'm fine getting that unlocking done in the RTC driver but maybe in the
> future, it will be necessary to handle that in an MFD driver when adding
> support for the other functions.

This change set just makes the RTC work again. Observation was, *sometimes* and 
*somehow* this unit tends to lock even if no of these tamper detection 
features were enabled nor the externals signals (to detect these security 
violations) were used.

Regards,
Juergen

-- 
Pengutronix e.K. ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?| Juergen Borleis ? ? ? ? ? ? |
Industrial Linux Solutions               ? ? ?| http://www.pengutronix.de/  |

[PATCH 2/5] RTC/i.MX/DryIce: add the unit recovery code

[Juergen Borleis]

This code is requiered to recover the unit from a security violation.
Hopefully this code can recover the unit from a hardware related invalid
state as well.

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
Signed-off-by: Robert Schwebel <rsc@pengutronix.de>
[rsc: got NDA clearance from Freescale]
---
 drivers/rtc/rtc-imxdi.c | 333 ++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 295 insertions(+), 38 deletions(-)

diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
index 8750477..f8abf2b 100644
--- a/drivers/rtc/rtc-imxdi.c
+++ b/drivers/rtc/rtc-imxdi.c
@@ -172,6 +172,296 @@ struct imxdi_dev {
  * task, we bring back this unit into life.
  */
 
+/* do a write into the unit without interrupt support */
+static void di_write_busy_wait(const struct imxdi_dev *imxdi, u32 val,
+								unsigned reg)
+{
+	/* do the register write */
+	__raw_writel(val, imxdi->ioaddr + reg);
+
+	/*
+	 * now it takes four 32,768 kHz clock cycles to take
+	 * the change into effect = 122 us
+	 */
+	udelay(200);
+}
+
+static void di_report_tamper_info(struct imxdi_dev *imxdi,  u32 dsr)
+{
+	u32 dtcr;
+
+	dtcr = __raw_readl(imxdi->ioaddr + DTCR);
+
+	dev_emerg(&imxdi->pdev->dev, "!! DryIce tamper event detected !!\n");
+	/* the following flags force a transition into the "FAILURE STATE" */
+	if (dsr & DSR_VTD) {
+		dev_emerg(&imxdi->pdev->dev, "!! Voltage Tamper event\n");
+		if (!(dtcr & DTCR_VTE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But Voltage Tamper detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_CTD) {
+		dev_emerg(&imxdi->pdev->dev, "!! 32768 Hz Clock Tamper Event\n");
+		if (!(dtcr & DTCR_CTE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But Clock Tamper detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_TTD) {
+		dev_emerg(&imxdi->pdev->dev, "!! Temperature Tamper Event\n");
+		if (!(dtcr & DTCR_TTE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But Temperature Tamper detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_SAD) {
+		dev_emerg(&imxdi->pdev->dev, "!! Secure Controller Alarm Event\n");
+		if (!(dtcr & DTCR_SAIE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But Secure Controller Alarm detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_EBD) {
+		dev_emerg(&imxdi->pdev->dev, "!! External Boot Tamper Event\n");
+		if (!(dtcr & DTCR_EBE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But External Boot detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_ETAD) {
+		dev_emerg(&imxdi->pdev->dev, "!! External Tamper A Event\n");
+		if (!(dtcr & DTCR_ETAE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But External Tamper A wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_ETBD) {
+		dev_emerg(&imxdi->pdev->dev, "!! External Tamper B Event\n");
+		if (!(dtcr & DTCR_ETBE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But External Tamper B wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_WTD) {
+		dev_emerg(&imxdi->pdev->dev, "!! Wire-mesh Tamper Event\n");
+		if (!(dtcr & DTCR_WTE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But wire-mesh Tamper detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_MCO) {
+		dev_emerg(&imxdi->pdev->dev, "!! Monotonic-counter Overflow Event\n");
+		if (!(dtcr & DTCR_MOE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But Monotonic-counter Overflow detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_TCO) {
+		dev_emerg(&imxdi->pdev->dev, "!! Timer-counter Overflow Event\n");
+		if (!(dtcr & DTCR_TOE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But Timer-counter Overflow detection wasn't enabled. False positive?\n");
+	}
+}
+
+static void di_what_is_to_be_done(struct imxdi_dev *imxdi,
+						const char *power_supply)
+{
+	dev_emerg(&imxdi->pdev->dev, " Please cycle the %s power supply in order to get the DryIce unit working again\n",
+			power_supply);
+}
+
+static int di_handle_failure_state(struct imxdi_dev *imxdi, u32 dsr)
+{
+	u32 dcr;
+
+	dev_dbg(&imxdi->pdev->dev, "DSR register reports: %08X\n", dsr);
+
+	/* report the cause */
+	di_report_tamper_info(imxdi, dsr);
+
+	dcr = __raw_readl(imxdi->ioaddr + DCR);
+
+	if (dcr & DCR_FSHL) {
+		/* we are out of luck */
+		di_what_is_to_be_done(imxdi, "battery");
+		return -ENODEV;
+	}
+	/*
+	 * with the next SYSTEM POR we will transit from the "FAILURE STATE"
+	 * into the "NON-VALID STATE" + "FAILURE STATE"
+	 */
+	di_what_is_to_be_done(imxdi, "main");
+
+	return -ENODEV;
+}
+
+static int di_handle_valid_state(struct imxdi_dev *imxdi, u32 dsr)
+{
+	/* initialize alarm */
+	di_write_busy_wait(imxdi, DCAMR_UNSET, DCAMR);
+	di_write_busy_wait(imxdi, 0, DCALR);
+
+	/* clear alarm flag */
+	if (dsr & DSR_CAF)
+		di_write_busy_wait(imxdi, DSR_CAF, DSR);
+
+	return 0;
+}
+
+static int di_handle_invalid_state(struct imxdi_dev *imxdi, u32 dsr)
+{
+	u32 dcr, sec;
+
+	/*
+	 * lets disable all sources which can force the DryIce unit into
+	 * the "FAILURE STATE" for now
+	 */
+	di_write_busy_wait(imxdi, 0x00000000, DTCR);
+	/* and lets protect them at runtime from any change */
+	di_write_busy_wait(imxdi, DCR_TDCSL, DCR);
+
+	sec = __raw_readl(imxdi->ioaddr + DTCMR);
+	if (sec != 0)
+		dev_warn(&imxdi->pdev->dev,
+			"The security violation has happend at %u seconds\n",
+			sec);
+	/*
+	 * the timer cannot be set/modified if
+	 * - the TCHL or TCSL bit is set in DCR
+	 */
+	dcr = __raw_readl(imxdi->ioaddr + DCR);
+	if (!(dcr & DCR_TCE)) {
+		if (dcr & DCR_TCHL) {
+			/* we are out of luck */
+			di_what_is_to_be_done(imxdi, "battery");
+			return -ENODEV;
+		}
+		if (dcr & DCR_TCSL) {
+			di_what_is_to_be_done(imxdi, "main");
+			return -ENODEV;
+		}
+
+	}
+	/*
+	 * - the timer counter stops/is stopped if
+	 *   - its overflow flag is set (TCO in DSR)
+	 *      -> clear overflow bit to make it count again
+	 *   - NVF is set in DSR
+	 *      -> clear non-valid bit to make it count again
+	 *   - its TCE (DCR) is cleared
+	 *      -> set TCE to make it count
+	 *   - it was never set before
+	 *      -> write a time into it (required again if the NVF was set)
+	 */
+	/* state handled */
+	di_write_busy_wait(imxdi, DSR_NVF, DSR);
+	/* clear overflow flag */
+	di_write_busy_wait(imxdi, DSR_TCO, DSR);
+	/* enable the counter */
+	di_write_busy_wait(imxdi, dcr | DCR_TCE, DCR);
+	/* set and trigger it to make it count */
+	di_write_busy_wait(imxdi, sec, DTCMR);
+
+	/* now prepare for the valid state */
+	return di_handle_valid_state(imxdi, __raw_readl(imxdi->ioaddr + DSR));
+}
+
+static int di_handle_invalid_and_failure_state(struct imxdi_dev *imxdi, u32 dsr)
+{
+	u32 dcr;
+
+	/*
+	 * now we must first remove the tamper sources in order to get the
+	 * device out of the "FAILURE STATE"
+	 * To disable any of the following sources we need to modify the DTCR
+	 */
+	if (dsr & (DSR_WTD | DSR_ETBD | DSR_ETAD | DSR_EBD | DSR_SAD |
+			DSR_TTD | DSR_CTD | DSR_VTD | DSR_MCO | DSR_TCO)) {
+		dcr = __raw_readl(imxdi->ioaddr + DCR);
+		if (dcr & DCR_TDCHL) {
+			/*
+			 * the tamper register is locked. We cannot disable the
+			 * tamper detection. The TDCHL can only be reset by a
+			 * DRYICE POR, but we cannot force a DRYICE POR in
+			 * softwere because we are still in "FAILURE STATE".
+			 * We need a DRYICE POR via battery power cycling....
+			 */
+			/*
+			 * out of luck!
+			 * we cannot disable them without a DRYICE POR
+			 */
+			di_what_is_to_be_done(imxdi, "battery");
+			return -ENODEV;
+		}
+		if (dcr & DCR_TDCSL) {
+			/* a soft lock can be removed by a SYSTEM POR */
+			di_what_is_to_be_done(imxdi, "main");
+			return -ENODEV;
+		}
+	}
+
+	/* disable all sources */
+	di_write_busy_wait(imxdi, 0x00000000, DTCR);
+
+	/* clear the status bits now */
+	di_write_busy_wait(imxdi, dsr & (DSR_WTD | DSR_ETBD | DSR_ETAD |
+			DSR_EBD | DSR_SAD | DSR_TTD | DSR_CTD | DSR_VTD |
+			DSR_MCO | DSR_TCO), DSR);
+
+	dsr = __raw_readl(imxdi->ioaddr + DSR);
+	if ((dsr & ~(DSR_NVF | DSR_SVF | DSR_WBF | DSR_WNF |
+			DSR_WCF | DSR_WEF)) != 0)
+		dev_warn(&imxdi->pdev->dev,
+			"There are still some sources of pain in DSR: %08x!\n",
+			dsr & ~(DSR_NVF | DSR_SVF | DSR_WBF | DSR_WNF |
+					DSR_WCF | DSR_WEF));
+
+	/*
+	 * now we are trying to clear the "Security-violation flag" to
+	 * get the DryIce out of this state
+	 */
+	di_write_busy_wait(imxdi, DSR_SVF, DSR);
+
+	/* success? */
+	dsr = __raw_readl(imxdi->ioaddr + DSR);
+	if (dsr & DSR_SVF) {
+		dev_crit(&imxdi->pdev->dev,
+			"Cannot clear the security violation flag. We are ending up in an endless loop!\n");
+		/* last resourt */
+		di_what_is_to_be_done(imxdi, "battery");
+		return -ENODEV;
+	}
+
+	/*
+	 * now we have left the "FAILURE STATE" and ending up in the
+	 * "NON-VALID STATE" time to recover everything
+	 */
+	return di_handle_invalid_state(imxdi, dsr);
+}
+
+static int di_handle_state(struct imxdi_dev *imxdi)
+{
+	int rc;
+	u32 dsr;
+
+	dsr = __raw_readl(imxdi->ioaddr + DSR);
+
+	switch (dsr & (DSR_NVF | DSR_SVF)) {
+	case DSR_NVF:
+		dev_warn(&imxdi->pdev->dev, "Invalid stated unit detected\n");
+		rc = di_handle_invalid_state(imxdi, dsr);
+		break;
+	case DSR_SVF:
+		dev_warn(&imxdi->pdev->dev, "Failure stated unit detected\n");
+		rc = di_handle_failure_state(imxdi, dsr);
+		break;
+	case DSR_NVF | DSR_SVF:
+		dev_warn(&imxdi->pdev->dev,
+				"Failure+Invalid stated unit detected\n");
+		rc = di_handle_invalid_and_failure_state(imxdi, dsr);
+		break;
+	default:
+		dev_notice(&imxdi->pdev->dev, "Unlocked unit detected\n");
+		rc = di_handle_valid_state(imxdi, dsr);
+	}
+
+	return rc;
+}
+
+
 /*
  * enable a dryice interrupt
  */
@@ -491,6 +781,11 @@ static int __init dryice_rtc_probe(struct platform_device *pdev)
 	/* mask all interrupts */
 	__raw_writel(0, imxdi->ioaddr + DIER);
 
+	rc = di_handle_state(imxdi);
+	if (rc != 0)
+		goto err;
+
+
 	rc = devm_request_irq(&pdev->dev, imxdi->irq, dryice_norm_irq,
 			IRQF_SHARED, pdev->name, imxdi);
 	if (rc) {
@@ -498,44 +793,6 @@ static int __init dryice_rtc_probe(struct platform_device *pdev)
 		goto err;
 	}
 
-	/* put dryice into valid state */
-	if (__raw_readl(imxdi->ioaddr + DSR) & DSR_NVF) {
-		rc = di_write_wait(imxdi, DSR_NVF | DSR_SVF, DSR);
-		if (rc)
-			goto err;
-	}
-
-	/* initialize alarm */
-	rc = di_write_wait(imxdi, DCAMR_UNSET, DCAMR);
-	if (rc)
-		goto err;
-	rc = di_write_wait(imxdi, 0, DCALR);
-	if (rc)
-		goto err;
-
-	/* clear alarm flag */
-	if (__raw_readl(imxdi->ioaddr + DSR) & DSR_CAF) {
-		rc = di_write_wait(imxdi, DSR_CAF, DSR);
-		if (rc)
-			goto err;
-	}
-
-	/* the timer won't count if it has never been written to */
-	if (__raw_readl(imxdi->ioaddr + DTCMR) == 0) {
-		rc = di_write_wait(imxdi, 0, DTCMR);
-		if (rc)
-			goto err;
-	}
-
-	/* start keeping time */
-	if (!(__raw_readl(imxdi->ioaddr + DCR) & DCR_TCE)) {
-		rc = di_write_wait(imxdi,
-				__raw_readl(imxdi->ioaddr + DCR) | DCR_TCE,
-				DCR);
-		if (rc)
-			goto err;
-	}
-
 	platform_set_drvdata(pdev, imxdi);
 	imxdi->rtc = devm_rtc_device_register(&pdev->dev, pdev->name,
 				  &dryice_rtc_ops, THIS_MODULE);
-- 
2.1.4

[PATCH 2/5] RTC/i.MX/DryIce: add the unit recovery code

[Juergen Borleis]

This code is requiered to recover the unit from a security violation.
Hopefully this code can recover the unit from a hardware related invalid
state as well.

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
Signed-off-by: Robert Schwebel <rsc@pengutronix.de>
[rsc: got NDA clearance from Freescale]
---
 drivers/rtc/rtc-imxdi.c | 333 ++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 295 insertions(+), 38 deletions(-)

diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
index 8750477..f8abf2b 100644
--- a/drivers/rtc/rtc-imxdi.c
+++ b/drivers/rtc/rtc-imxdi.c
@@ -172,6 +172,296 @@ struct imxdi_dev {
  * task, we bring back this unit into life.
  */
 
+/* do a write into the unit without interrupt support */
+static void di_write_busy_wait(const struct imxdi_dev *imxdi, u32 val,
+								unsigned reg)
+{
+	/* do the register write */
+	__raw_writel(val, imxdi->ioaddr + reg);
+
+	/*
+	 * now it takes four 32,768 kHz clock cycles to take
+	 * the change into effect = 122 us
+	 */
+	udelay(200);
+}
+
+static void di_report_tamper_info(struct imxdi_dev *imxdi,  u32 dsr)
+{
+	u32 dtcr;
+
+	dtcr = __raw_readl(imxdi->ioaddr + DTCR);
+
+	dev_emerg(&imxdi->pdev->dev, "!! DryIce tamper event detected !!\n");
+	/* the following flags force a transition into the "FAILURE STATE" */
+	if (dsr & DSR_VTD) {
+		dev_emerg(&imxdi->pdev->dev, "!! Voltage Tamper event\n");
+		if (!(dtcr & DTCR_VTE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But Voltage Tamper detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_CTD) {
+		dev_emerg(&imxdi->pdev->dev, "!! 32768 Hz Clock Tamper Event\n");
+		if (!(dtcr & DTCR_CTE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But Clock Tamper detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_TTD) {
+		dev_emerg(&imxdi->pdev->dev, "!! Temperature Tamper Event\n");
+		if (!(dtcr & DTCR_TTE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But Temperature Tamper detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_SAD) {
+		dev_emerg(&imxdi->pdev->dev, "!! Secure Controller Alarm Event\n");
+		if (!(dtcr & DTCR_SAIE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But Secure Controller Alarm detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_EBD) {
+		dev_emerg(&imxdi->pdev->dev, "!! External Boot Tamper Event\n");
+		if (!(dtcr & DTCR_EBE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But External Boot detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_ETAD) {
+		dev_emerg(&imxdi->pdev->dev, "!! External Tamper A Event\n");
+		if (!(dtcr & DTCR_ETAE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But External Tamper A wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_ETBD) {
+		dev_emerg(&imxdi->pdev->dev, "!! External Tamper B Event\n");
+		if (!(dtcr & DTCR_ETBE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But External Tamper B wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_WTD) {
+		dev_emerg(&imxdi->pdev->dev, "!! Wire-mesh Tamper Event\n");
+		if (!(dtcr & DTCR_WTE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But wire-mesh Tamper detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_MCO) {
+		dev_emerg(&imxdi->pdev->dev, "!! Monotonic-counter Overflow Event\n");
+		if (!(dtcr & DTCR_MOE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But Monotonic-counter Overflow detection wasn't enabled. False positive?\n");
+	}
+	if (dsr & DSR_TCO) {
+		dev_emerg(&imxdi->pdev->dev, "!! Timer-counter Overflow Event\n");
+		if (!(dtcr & DTCR_TOE))
+			dev_emerg(&imxdi->pdev->dev,
+				"!! But Timer-counter Overflow detection wasn't enabled. False positive?\n");
+	}
+}
+
+static void di_what_is_to_be_done(struct imxdi_dev *imxdi,
+						const char *power_supply)
+{
+	dev_emerg(&imxdi->pdev->dev, " Please cycle the %s power supply in order to get the DryIce unit working again\n",
+			power_supply);
+}
+
+static int di_handle_failure_state(struct imxdi_dev *imxdi, u32 dsr)
+{
+	u32 dcr;
+
+	dev_dbg(&imxdi->pdev->dev, "DSR register reports: %08X\n", dsr);
+
+	/* report the cause */
+	di_report_tamper_info(imxdi, dsr);
+
+	dcr = __raw_readl(imxdi->ioaddr + DCR);
+
+	if (dcr & DCR_FSHL) {
+		/* we are out of luck */
+		di_what_is_to_be_done(imxdi, "battery");
+		return -ENODEV;
+	}
+	/*
+	 * with the next SYSTEM POR we will transit from the "FAILURE STATE"
+	 * into the "NON-VALID STATE" + "FAILURE STATE"
+	 */
+	di_what_is_to_be_done(imxdi, "main");
+
+	return -ENODEV;
+}
+
+static int di_handle_valid_state(struct imxdi_dev *imxdi, u32 dsr)
+{
+	/* initialize alarm */
+	di_write_busy_wait(imxdi, DCAMR_UNSET, DCAMR);
+	di_write_busy_wait(imxdi, 0, DCALR);
+
+	/* clear alarm flag */
+	if (dsr & DSR_CAF)
+		di_write_busy_wait(imxdi, DSR_CAF, DSR);
+
+	return 0;
+}
+
+static int di_handle_invalid_state(struct imxdi_dev *imxdi, u32 dsr)
+{
+	u32 dcr, sec;
+
+	/*
+	 * lets disable all sources which can force the DryIce unit into
+	 * the "FAILURE STATE" for now
+	 */
+	di_write_busy_wait(imxdi, 0x00000000, DTCR);
+	/* and lets protect them at runtime from any change */
+	di_write_busy_wait(imxdi, DCR_TDCSL, DCR);
+
+	sec = __raw_readl(imxdi->ioaddr + DTCMR);
+	if (sec != 0)
+		dev_warn(&imxdi->pdev->dev,
+			"The security violation has happend at %u seconds\n",
+			sec);
+	/*
+	 * the timer cannot be set/modified if
+	 * - the TCHL or TCSL bit is set in DCR
+	 */
+	dcr = __raw_readl(imxdi->ioaddr + DCR);
+	if (!(dcr & DCR_TCE)) {
+		if (dcr & DCR_TCHL) {
+			/* we are out of luck */
+			di_what_is_to_be_done(imxdi, "battery");
+			return -ENODEV;
+		}
+		if (dcr & DCR_TCSL) {
+			di_what_is_to_be_done(imxdi, "main");
+			return -ENODEV;
+		}
+
+	}
+	/*
+	 * - the timer counter stops/is stopped if
+	 *   - its overflow flag is set (TCO in DSR)
+	 *      -> clear overflow bit to make it count again
+	 *   - NVF is set in DSR
+	 *      -> clear non-valid bit to make it count again
+	 *   - its TCE (DCR) is cleared
+	 *      -> set TCE to make it count
+	 *   - it was never set before
+	 *      -> write a time into it (required again if the NVF was set)
+	 */
+	/* state handled */
+	di_write_busy_wait(imxdi, DSR_NVF, DSR);
+	/* clear overflow flag */
+	di_write_busy_wait(imxdi, DSR_TCO, DSR);
+	/* enable the counter */
+	di_write_busy_wait(imxdi, dcr | DCR_TCE, DCR);
+	/* set and trigger it to make it count */
+	di_write_busy_wait(imxdi, sec, DTCMR);
+
+	/* now prepare for the valid state */
+	return di_handle_valid_state(imxdi, __raw_readl(imxdi->ioaddr + DSR));
+}
+
+static int di_handle_invalid_and_failure_state(struct imxdi_dev *imxdi, u32 dsr)
+{
+	u32 dcr;
+
+	/*
+	 * now we must first remove the tamper sources in order to get the
+	 * device out of the "FAILURE STATE"
+	 * To disable any of the following sources we need to modify the DTCR
+	 */
+	if (dsr & (DSR_WTD | DSR_ETBD | DSR_ETAD | DSR_EBD | DSR_SAD |
+			DSR_TTD | DSR_CTD | DSR_VTD | DSR_MCO | DSR_TCO)) {
+		dcr = __raw_readl(imxdi->ioaddr + DCR);
+		if (dcr & DCR_TDCHL) {
+			/*
+			 * the tamper register is locked. We cannot disable the
+			 * tamper detection. The TDCHL can only be reset by a
+			 * DRYICE POR, but we cannot force a DRYICE POR in
+			 * softwere because we are still in "FAILURE STATE".
+			 * We need a DRYICE POR via battery power cycling....
+			 */
+			/*
+			 * out of luck!
+			 * we cannot disable them without a DRYICE POR
+			 */
+			di_what_is_to_be_done(imxdi, "battery");
+			return -ENODEV;
+		}
+		if (dcr & DCR_TDCSL) {
+			/* a soft lock can be removed by a SYSTEM POR */
+			di_what_is_to_be_done(imxdi, "main");
+			return -ENODEV;
+		}
+	}
+
+	/* disable all sources */
+	di_write_busy_wait(imxdi, 0x00000000, DTCR);
+
+	/* clear the status bits now */
+	di_write_busy_wait(imxdi, dsr & (DSR_WTD | DSR_ETBD | DSR_ETAD |
+			DSR_EBD | DSR_SAD | DSR_TTD | DSR_CTD | DSR_VTD |
+			DSR_MCO | DSR_TCO), DSR);
+
+	dsr = __raw_readl(imxdi->ioaddr + DSR);
+	if ((dsr & ~(DSR_NVF | DSR_SVF | DSR_WBF | DSR_WNF |
+			DSR_WCF | DSR_WEF)) != 0)
+		dev_warn(&imxdi->pdev->dev,
+			"There are still some sources of pain in DSR: %08x!\n",
+			dsr & ~(DSR_NVF | DSR_SVF | DSR_WBF | DSR_WNF |
+					DSR_WCF | DSR_WEF));
+
+	/*
+	 * now we are trying to clear the "Security-violation flag" to
+	 * get the DryIce out of this state
+	 */
+	di_write_busy_wait(imxdi, DSR_SVF, DSR);
+
+	/* success? */
+	dsr = __raw_readl(imxdi->ioaddr + DSR);
+	if (dsr & DSR_SVF) {
+		dev_crit(&imxdi->pdev->dev,
+			"Cannot clear the security violation flag. We are ending up in an endless loop!\n");
+		/* last resourt */
+		di_what_is_to_be_done(imxdi, "battery");
+		return -ENODEV;
+	}
+
+	/*
+	 * now we have left the "FAILURE STATE" and ending up in the
+	 * "NON-VALID STATE" time to recover everything
+	 */
+	return di_handle_invalid_state(imxdi, dsr);
+}
+
+static int di_handle_state(struct imxdi_dev *imxdi)
+{
+	int rc;
+	u32 dsr;
+
+	dsr = __raw_readl(imxdi->ioaddr + DSR);
+
+	switch (dsr & (DSR_NVF | DSR_SVF)) {
+	case DSR_NVF:
+		dev_warn(&imxdi->pdev->dev, "Invalid stated unit detected\n");
+		rc = di_handle_invalid_state(imxdi, dsr);
+		break;
+	case DSR_SVF:
+		dev_warn(&imxdi->pdev->dev, "Failure stated unit detected\n");
+		rc = di_handle_failure_state(imxdi, dsr);
+		break;
+	case DSR_NVF | DSR_SVF:
+		dev_warn(&imxdi->pdev->dev,
+				"Failure+Invalid stated unit detected\n");
+		rc = di_handle_invalid_and_failure_state(imxdi, dsr);
+		break;
+	default:
+		dev_notice(&imxdi->pdev->dev, "Unlocked unit detected\n");
+		rc = di_handle_valid_state(imxdi, dsr);
+	}
+
+	return rc;
+}
+
+
 /*
  * enable a dryice interrupt
  */
@@ -491,6 +781,11 @@ static int __init dryice_rtc_probe(struct platform_device *pdev)
 	/* mask all interrupts */
 	__raw_writel(0, imxdi->ioaddr + DIER);
 
+	rc = di_handle_state(imxdi);
+	if (rc != 0)
+		goto err;
+
+
 	rc = devm_request_irq(&pdev->dev, imxdi->irq, dryice_norm_irq,
 			IRQF_SHARED, pdev->name, imxdi);
 	if (rc) {
@@ -498,44 +793,6 @@ static int __init dryice_rtc_probe(struct platform_device *pdev)
 		goto err;
 	}
 
-	/* put dryice into valid state */
-	if (__raw_readl(imxdi->ioaddr + DSR) & DSR_NVF) {
-		rc = di_write_wait(imxdi, DSR_NVF | DSR_SVF, DSR);
-		if (rc)
-			goto err;
-	}
-
-	/* initialize alarm */
-	rc = di_write_wait(imxdi, DCAMR_UNSET, DCAMR);
-	if (rc)
-		goto err;
-	rc = di_write_wait(imxdi, 0, DCALR);
-	if (rc)
-		goto err;
-
-	/* clear alarm flag */
-	if (__raw_readl(imxdi->ioaddr + DSR) & DSR_CAF) {
-		rc = di_write_wait(imxdi, DSR_CAF, DSR);
-		if (rc)
-			goto err;
-	}
-
-	/* the timer won't count if it has never been written to */
-	if (__raw_readl(imxdi->ioaddr + DTCMR) == 0) {
-		rc = di_write_wait(imxdi, 0, DTCMR);
-		if (rc)
-			goto err;
-	}
-
-	/* start keeping time */
-	if (!(__raw_readl(imxdi->ioaddr + DCR) & DCR_TCE)) {
-		rc = di_write_wait(imxdi,
-				__raw_readl(imxdi->ioaddr + DCR) | DCR_TCE,
-				DCR);
-		if (rc)
-			goto err;
-	}
-
 	platform_set_drvdata(pdev, imxdi);
 	imxdi->rtc = devm_rtc_device_register(&pdev->dev, pdev->name,
 				  &dryice_rtc_ops, THIS_MODULE);
-- 
2.1.4