Skip to contentSkip to conversation

ARM: OMAP2+: hwmod: RTC: Add lock and unlock hooks

PATCH New Lokesh Vutla 4 participants 16 messages Jun 10, 2015 – Jul 16, 2015

Subsystems (5)
Arm (32-bit) Arm sub-architectures OMAP2+ support OMAP hwmod data +1 more subsystems
lore
Share & tools
lore

Lokesh Vutla

author
#0 lore raw

cover ARM: OMAP2+: hwmod: RTC: Add lock and unlock hooks

Some IP blocks like RTC, needs an additional unlocking mechanism for writing to its registers. This patch adds optional lock and unlock function pointers to the IP block's hwmod data which gets executed before and after writing into IP sysconfig register.
And also hook lock and unlock functions to AMx3xx, DRA7 RTC hwmod data, so that sysconfig registers are updated properly.

Tested on:
DRA7-evm: http://pastebin.ubuntu.com/11688889/
DRA72-evm: http://pastebin.ubuntu.com/11688901/
BeagleBoard-x15: http://pastebin.ubuntu.com/11688907/
BeagleBoneBlack: http://pastebin.ubuntu.com/11688923/
AM437x-gp-evm: http://pastebin.ubuntu.com/11689157/ (Used an out of tree patch to enable RTC)

Lokesh Vutla (3):
ARM: OMAP2+: hwmod: add support for lock and unlock hooks
ARM: DRA: hwmod: RTC: Add lock and unlock functions
ARM: AMx3xx: RTC: Add lock and unlock functions

 arch/arm/mach-omap2/omap_hwmod.c                   | 13 ++++++
 arch/arm/mach-omap2/omap_hwmod.h                   |  6 +++
 .../mach-omap2/omap_hwmod_33xx_43xx_ipblock_data.c |  2 +
 arch/arm/mach-omap2/omap_hwmod_7xx_data.c          |  2 +
 arch/arm/mach-omap2/omap_hwmod_reset.c             | 47 ++++++++++++++++++++++
 5 files changed, 70 insertions(+)
Signature · 1 line
1.9.1
author
#1 lore raw

PATCH 1/3 ARM: OMAP2+: hwmod: add support for lock and unlock hooks

Some IP blocks like RTC, needs an additional setting for writing to its registers. This is to prevent any spurious writes from changing the register values.

This patch adds optional lock and unlock function pointers to the IP block's hwmod data. These unlock and lock function pointers are called by hwmod code before and after writing sysconfig registers.

Signed-off-by Lokesh Vutla <[email protected]>
---
 arch/arm/mach-omap2/omap_hwmod.c | 13 +++++++++++++
 arch/arm/mach-omap2/omap_hwmod.h |  4 ++++
 2 files changed, 17 insertions(+)
arch/arm/mach-omap2/omap_hwmod.c +13 -0
index 752969f..44c916d3 100644
@@ -299,7 +299,20 @@ static void _write_sysconfig(u32 v, struct omap_hwmod *oh)
 
 	/* Module might have lost context, always update cache and register */
 	oh->_sysc_cache = v;
+
+	/*
+	 * Some IP blocks (such as RTC) require unlocking of IP before
+	 * accessing its registers. If a function pointer is present
+	 * to unlock, then call it before accessing sysconfig and
+	 * call lock after writing sysconfig.
+	 */
+	if (oh->class->unlock)
+		oh->class->unlock(oh);
+
 	omap_hwmod_write(v, oh, oh->class->sysc->sysc_offs);
+
+	if (oh->class->lock)
+		oh->class->lock(oh);
 }
 
 /**
arch/arm/mach-omap2/omap_hwmod.h +4 -0
index 9611c91..44c7db9 100644
@@ -570,6 +570,8 @@ struct omap_hwmod_omap4_prcm {
  * @pre_shutdown: ptr to fn to be executed immediately prior to device shutdown
  * @reset: ptr to fn to be executed in place of the standard hwmod reset fn
  * @enable_preprogram:  ptr to fn to be executed during device enable
+ * @lock: ptr to fn to be executed to lock IP registers
+ * @unlock: ptr to fn to be executed to unlock IP registers
  *
  * Represent the class of a OMAP hardware "modules" (e.g. timer,
  * smartreflex, gpio, uart...)

@@ -594,6 +596,8 @@ struct omap_hwmod_class {
 	int					(*pre_shutdown)(struct omap_hwmod *oh);
 	int					(*reset)(struct omap_hwmod *oh);
 	int					(*enable_preprogram)(struct omap_hwmod *oh);
+	void				(*lock)(struct omap_hwmod *oh);
+	void				(*unlock)(struct omap_hwmod *oh);
 };
 
 /**
Signature · 1 line
1.9.1
maintainer
#2 lore raw

PATCH 1/3 ARM: OMAP2+: hwmod: add support for lock and unlock hooks

Quoted 9 lines · Lokesh Vutla

Some IP blocks like RTC, needs an additional setting for writing to its registers. This is to prevent any spurious writes from changing the register values.

This patch adds optional lock and unlock function pointers to the IP block's hwmod data. These unlock and lock function pointers are called by hwmod code before and after writing sysconfig registers.

Signed-off-by Lokesh Vutla <[email protected]>

Thanks, queued for v4.3.

- Paul

author
#3 lore raw

PATCH 2/3 ARM: DRA: hwmod: RTC: Add lock and unlock functions

In reply to Lokesh Vutla

RTC IP have kicker feature which prevents spurious writes to its registers. In order to write into any of the RTC registers, KICK values has to be written to KICK registers.

Introduce omap_hwmod_rtc_unlock/lock functions, which writes into these KICK registers inorder to lock and unlock RTC registers. Also hook these functions to RTC hwmod.

Signed-off-by Lokesh Vutla <[email protected]>
---
 arch/arm/mach-omap2/omap_hwmod.h          |  2 ++
 arch/arm/mach-omap2/omap_hwmod_7xx_data.c |  2 ++
 arch/arm/mach-omap2/omap_hwmod_reset.c    | 47 +++++++++++++++++++++++++++++++
 3 files changed, 51 insertions(+)
arch/arm/mach-omap2/omap_hwmod.h +2 -0
index 44c7db9..04855ab 100644
@@ -742,6 +742,8 @@ const char *omap_hwmod_get_main_clk(struct omap_hwmod *oh);
  */
 
 extern int omap_hwmod_aess_preprogram(struct omap_hwmod *oh);
+void omap_hwmod_rtc_unlock(struct omap_hwmod *oh);
+void omap_hwmod_rtc_lock(struct omap_hwmod *oh);
 
 /*
  * Chip variant-specific hwmod init routines - XXX should be converted
arch/arm/mach-omap2/omap_hwmod_7xx_data.c +2 -0
index 0e64c2f..983042f 100644
@@ -1548,6 +1548,8 @@ static struct omap_hwmod_class_sysconfig dra7xx_rtcss_sysc = {
 static struct omap_hwmod_class dra7xx_rtcss_hwmod_class = {
 	.name	= "rtcss",
 	.sysc	= &dra7xx_rtcss_sysc,
+	.unlock	= &omap_hwmod_rtc_unlock,
+	.lock	= &omap_hwmod_rtc_lock,
 };
 
 /* rtcss */
arch/arm/mach-omap2/omap_hwmod_reset.c +47 -0

61 lines not on this page · raw message

Signature · 1 line
1.9.1
maintainer
#4 lore raw

PATCH 2/3 ARM: DRA: hwmod: RTC: Add lock and unlock functions

Hi,

some comments.

Quoted 33 lines · Lokesh Vutla

RTC IP have kicker feature which prevents spurious writes to its registers. In order to write into any of the RTC registers, KICK values has to be written to KICK registers.

Introduce omap_hwmod_rtc_unlock/lock functions, which writes into these KICK registers inorder to lock and unlock RTC registers. Also hook these functions to RTC hwmod.

Signed-off-by Lokesh Vutla <[email protected]>
---
 arch/arm/mach-omap2/omap_hwmod.h          |  2 ++
 arch/arm/mach-omap2/omap_hwmod_7xx_data.c |  2 ++
 arch/arm/mach-omap2/omap_hwmod_reset.c    | 47 +++++++++++++++++++++++++++++++
 3 files changed, 51 insertions(+)
arch/arm/mach-omap2/omap_hwmod.h +2 -0
index 44c7db9..04855ab 100644
@@ -742,6 +742,8 @@ const char *omap_hwmod_get_main_clk(struct omap_hwmod *oh);
  */
 
 extern int omap_hwmod_aess_preprogram(struct omap_hwmod *oh);
+void omap_hwmod_rtc_unlock(struct omap_hwmod *oh);
+void omap_hwmod_rtc_lock(struct omap_hwmod *oh);
 
 /*
  * Chip variant-specific hwmod init routines - XXX should be converted
arch/arm/mach-omap2/omap_hwmod_7xx_data.c +0 -0
index 0e64c2f..983042f 100644

arch/arm/mach-omap2/omap_hwmod_7xx_data.c static struct omap_hwmod_class_sysconfig dra7xx_rtcss_sysc = { · line 1555

Quoted 8 lines
arch/arm/mach-omap2/omap_hwmod_7xx_data.c +2 -0
@@ -1548,6 +1548,8 @@ static struct omap_hwmod_class_sysconfig dra7xx_rtcss_sysc = {
 static struct omap_hwmod_class dra7xx_rtcss_hwmod_class = {
 	.name	= "rtcss",
 	.sysc	= &dra7xx_rtcss_sysc,
+	.unlock	= &omap_hwmod_rtc_unlock,
+	.lock	= &omap_hwmod_rtc_lock,
 };
 
 /* rtcss */

Is the DRA7xx the only chip that has this lock/unlock feature, or do other OMAP chips use the same RTC IP block?

Quoted 26 lines
arch/arm/mach-omap2/omap_hwmod_reset.c +9 -0
index 65e186c..1fb106d 100644
@@ -25,11 +25,20 @@
  */
 #include <linux/kernel.h>
 #include <linux/errno.h>
+#include <linux/delay.h>
 
 #include <sound/aess.h>
 
 #include "omap_hwmod.h"
 
+#define OMAP_RTC_STATUS_REG	0x44
+#define OMAP_RTC_KICK0_REG	0x6c
+#define OMAP_RTC_KICK1_REG	0x70
+
+#define OMAP_RTC_KICK0_VALUE	0x83E70B13
+#define OMAP_RTC_KICK1_VALUE	0x95A4F1E0
+#define OMAP_RTC_STATUS_BUSY	BIT(0)
+
 /**
  * omap_hwmod_aess_preprogram - enable AESS internal autogating
  * @oh: struct omap_hwmod *

arch/arm/mach-omap2/omap_hwmod_reset.c int omap_hwmod_aess_preprogram(struct omap_hwmod *oh) · line 64

Quoted 5 lines
arch/arm/mach-omap2/omap_hwmod_reset.c +2 -0
@@ -51,3 +60,41 @@ int omap_hwmod_aess_preprogram(struct omap_hwmod *oh)
 
 	return 0;
 }
+
+static void omap_rtc_wait_not_busy(struct omap_hwmod *oh)

This function is missing kerneldoc and desperately needs it.

arch/arm/mach-omap2/omap_hwmod_reset.c int omap_hwmod_aess_preprogram(struct omap_hwmod *oh) · line 70

Quoted 6 lines
arch/arm/mach-omap2/omap_hwmod_reset.c +6 -0
+{
+	int count;
+	u8 status;
+
+	/* BUSY may stay active for 1/32768 second (~30 usec) */
+	for (count = 0; count < 50; count++) {

OK, I give up. Where does 50 come from? This should be moved into a macro with a comment,@the very least.

Quoted 10 lines
arch/arm/mach-omap2/omap_hwmod_reset.c +10 -0
+		status = omap_hwmod_read(oh, OMAP_RTC_STATUS_REG);
+		if (!(status & OMAP_RTC_STATUS_BUSY))
+			break;
+		udelay(1);
+	}
+	/* now we have ~15 usec to read/write various registers */
+}
+
+/**
+ * omap_hwmod_rtc_unlock - Reset and unlock the Kicker mechanism.

arch/arm/mach-omap2/omap_hwmod_reset.c int omap_hwmod_aess_preprogram(struct omap_hwmod *oh) · line 90

Quoted 10 lines
arch/arm/mach-omap2/omap_hwmod_reset.c +10 -0
+ * @oh: struct omap_hwmod *
+ *
+ * RTC IP have kicker feature.  This prevents spurious writes to its registers.
+ * In order to write into any of the RTC registers, KICK values has te be
+ * written in respective KICK registers. This is needed for hwmod to write into
+ * sysconfig register.
+ */
+void omap_hwmod_rtc_unlock(struct omap_hwmod *oh)
+{
+	omap_rtc_wait_not_busy(oh);

What prevents the CPU from context-switching away after the BUSY bit test and not returning back here by the time the ~15 ?s interval has ended? Looks to me like this whole thing needs to run with interrupts disabled.

arch/arm/mach-omap2/omap_hwmod_reset.c int omap_hwmod_aess_preprogram(struct omap_hwmod *oh) · line 97

Quoted 7 lines
arch/arm/mach-omap2/omap_hwmod_reset.c +7 -0
+	omap_hwmod_write(OMAP_RTC_KICK0_VALUE, oh, OMAP_RTC_KICK0_REG);
+	omap_hwmod_write(OMAP_RTC_KICK1_VALUE, oh, OMAP_RTC_KICK1_REG);
+}
+
+void omap_hwmod_rtc_lock(struct omap_hwmod *oh)
+{
+	omap_rtc_wait_not_busy(oh);

Same comment as the above.

Quoted 6 lines
arch/arm/mach-omap2/omap_hwmod_reset.c +3 -0
+	omap_hwmod_write(0x0, oh, OMAP_RTC_KICK0_REG);
+	omap_hwmod_write(0x0, oh, OMAP_RTC_KICK1_REG);
+}
Signature · 1 line
1.9.1

- Paul

#5 lore raw

PATCH 2/3 ARM: DRA: hwmod: RTC: Add lock and unlock functions

Hi Paul,

Quoted 78 lines · Paul Walmsley

Hi,

some comments.

Quoted 33 lines · Lokesh Vutla

RTC IP have kicker feature which prevents spurious writes to its registers. In order to write into any of the RTC registers, KICK values has to be written to KICK registers.

Introduce omap_hwmod_rtc_unlock/lock functions, which writes into these KICK registers inorder to lock and unlock RTC registers. Also hook these functions to RTC hwmod.

Signed-off-by Lokesh Vutla <[email protected]>
---
  arch/arm/mach-omap2/omap_hwmod.h          |  2 ++
  arch/arm/mach-omap2/omap_hwmod_7xx_data.c |  2 ++
  arch/arm/mach-omap2/omap_hwmod_reset.c    | 47 +++++++++++++++++++++++++++++++
  3 files changed, 51 insertions(+)
arch/arm/mach-omap2/omap_hwmod.h +2 -0
index 44c7db9..04855ab 100644
@@ -742,6 +742,8 @@ const char *omap_hwmod_get_main_clk(struct omap_hwmod *oh);
   */
 
  extern int omap_hwmod_aess_preprogram(struct omap_hwmod *oh);
+void omap_hwmod_rtc_unlock(struct omap_hwmod *oh);
+void omap_hwmod_rtc_lock(struct omap_hwmod *oh);
 
  /*
   * Chip variant-specific hwmod init routines - XXX should be converted
arch/arm/mach-omap2/omap_hwmod_7xx_data.c +0 -0
index 0e64c2f..983042f 100644

arch/arm/mach-omap2/omap_hwmod_7xx_data.c static struct omap_hwmod_class_sysconfig dra7xx_rtcss_sysc = { · line 1555

Quoted 8 lines
arch/arm/mach-omap2/omap_hwmod_7xx_data.c +2 -0
@@ -1548,6 +1548,8 @@ static struct omap_hwmod_class_sysconfig dra7xx_rtcss_sysc = {
  static struct omap_hwmod_class dra7xx_rtcss_hwmod_class = {
  	.name	= "rtcss",
  	.sysc	= &dra7xx_rtcss_sysc,
+	.unlock	= &omap_hwmod_rtc_unlock,
+	.lock	= &omap_hwmod_rtc_lock,
  };
 
  /* rtcss */

Is the DRA7xx the only chip that has this lock/unlock feature, or do other OMAP chips use the same RTC IP block?

Quoted 27 lines
arch/arm/mach-omap2/omap_hwmod_reset.c +9 -0
index 65e186c..1fb106d 100644
@@ -25,11 +25,20 @@
   */
  #include <linux/kernel.h>
  #include <linux/errno.h>
+#include <linux/delay.h>
 
  #include <sound/aess.h>
 
  #include "omap_hwmod.h"
 
+#define OMAP_RTC_STATUS_REG	0x44
+#define OMAP_RTC_KICK0_REG	0x6c
+#define OMAP_RTC_KICK1_REG	0x70
+
+#define OMAP_RTC_KICK0_VALUE	0x83E70B13
+#define OMAP_RTC_KICK1_VALUE	0x95A4F1E0
+#define OMAP_RTC_STATUS_BUSY	BIT(0)
+
  /**
   * omap_hwmod_aess_preprogram - enable AESS internal autogating
   * @oh: struct omap_hwmod *

@@ -51,3 +60,41 @@ int omap_hwmod_aess_preprogram(struct omap_hwmod *oh)
Quoted 6 lines

arch/arm/mach-omap2/omap_hwmod_reset.c int omap_hwmod_aess_preprogram(struct omap_hwmod *oh) · line 64

Quoted 4 lines
arch/arm/mach-omap2/omap_hwmod_reset.c +2 -0
  	return 0;
  }
+
+static void omap_rtc_wait_not_busy(struct omap_hwmod *oh)

This function is missing kerneldoc and desperately needs it.

I should have written it, sorry about that.
For updating certain RTC registers, the MPU must wait for the BUSY status to become zero. Once the BUSY flag is zero, there is a 15-?s access period in which the MPU can program.

Quoted 27 lines

arch/arm/mach-omap2/omap_hwmod_reset.c int omap_hwmod_aess_preprogram(struct omap_hwmod *oh) · line 70

Quoted 7 lines
arch/arm/mach-omap2/omap_hwmod_reset.c +6 -0
+{
+	int count;
+	u8 status;
+
+	/* BUSY may stay active for 1/32768 second (~30 usec) */
+	for (count = 0; count < 50; count++) {

OK, I give up. Where does 50 come from? This should be moved into a macro with a comment, at the very least.

Quoted 16 lines
arch/arm/mach-omap2/omap_hwmod_reset.c +16 -0
+		status = omap_hwmod_read(oh, OMAP_RTC_STATUS_REG);
+		if (!(status & OMAP_RTC_STATUS_BUSY))
+			break;
+		udelay(1);
+	}
+	/* now we have ~15 usec to read/write various registers */
+}
+
+/**
+ * omap_hwmod_rtc_unlock - Reset and unlock the Kicker mechanism.
+ * @oh: struct omap_hwmod *
+ *
+ * RTC IP have kicker feature.  This prevents spurious writes to its registers.
+ * In order to write into any of the RTC registers, KICK values has te be
+ * written in respective KICK registers. This is needed for hwmod to write into
+ * sysconfig register.
Quoted 8 lines

arch/arm/mach-omap2/omap_hwmod_reset.c int omap_hwmod_aess_preprogram(struct omap_hwmod *oh) · line 90

Quoted 4 lines
arch/arm/mach-omap2/omap_hwmod_reset.c +4 -0
+ */
+void omap_hwmod_rtc_unlock(struct omap_hwmod *oh)
+{
+	omap_rtc_wait_not_busy(oh);

What prevents the CPU from context-switching away after the BUSY bit test and not returning back here by the time the ~15 ?s interval has ended? Looks to me like this whole thing needs to run with interrupts disabled.

Yes, you are correct. Ill update it and repost.

Thanks and regards,
Lokesh

Quoted 21 lines

arch/arm/mach-omap2/omap_hwmod_reset.c int omap_hwmod_aess_preprogram(struct omap_hwmod *oh) · line 97

Quoted 8 lines
arch/arm/mach-omap2/omap_hwmod_reset.c +7 -0
+	omap_hwmod_write(OMAP_RTC_KICK0_VALUE, oh, OMAP_RTC_KICK0_REG);
+	omap_hwmod_write(OMAP_RTC_KICK1_VALUE, oh, OMAP_RTC_KICK1_REG);
+}
+
+void omap_hwmod_rtc_lock(struct omap_hwmod *oh)
+{
+	omap_rtc_wait_not_busy(oh);

Same comment as the above.

Quoted 6 lines
arch/arm/mach-omap2/omap_hwmod_reset.c +3 -1
+	omap_hwmod_write(0x0, oh, OMAP_RTC_KICK0_REG);
+	omap_hwmod_write(0x0, oh, OMAP_RTC_KICK1_REG);
+}
--

1.9.1

- Paul

author
#6 lore raw

PATCH 3/3 ARM: AMx3xx: hwmod: RTC: Add lock and unlock functions

In reply to Lokesh Vutla

RTC IP have kicker feature which prevents spurious writes to its registers. In order to write into any of the RTC registers, KICK values has to be written to KICK registers.

omap_hwmod_rtc_unlock/lock functions writes into these KICK registers inorder to lock and unlock RTC registers.
This patch hooks omap_hwmod_rtc_unlock/lock functions into RTC hwmod, so that SYSCONFIG register is updated properly.

Signed-off-by Lokesh Vutla <[email protected]>
---
 arch/arm/mach-omap2/omap_hwmod_33xx_43xx_ipblock_data.c | 2 ++
 1 file changed, 2 insertions(+)
arch/arm/mach-omap2/omap_hwmod_33xx_43xx_ipblock_data.c +2 -0
index cabc569..2d92958 100644
@@ -904,6 +904,8 @@ static struct omap_hwmod_class_sysconfig am33xx_rtc_sysc = {
 static struct omap_hwmod_class am33xx_rtc_hwmod_class = {
 	.name		= "rtc",
 	.sysc		= &am33xx_rtc_sysc,
+	.unlock		= &omap_hwmod_rtc_unlock,
+	.lock		= &omap_hwmod_rtc_lock,
 };
 
 struct omap_hwmod am33xx_rtc_hwmod = {
Signature · 1 line
1.9.1
maintainer
#7 lore raw

PATCH 3/3 ARM: AMx3xx: hwmod: RTC: Add lock and unlock functions

Hi

Quoted 19 lines · Lokesh Vutla

RTC IP have kicker feature which prevents spurious writes to its registers. In order to write into any of the RTC registers, KICK values has to be written to KICK registers.

omap_hwmod_rtc_unlock/lock functions writes into these KICK registers inorder to lock and unlock RTC registers.
This patch hooks omap_hwmod_rtc_unlock/lock functions into RTC hwmod, so that SYSCONFIG register is updated properly.

Signed-off-by Lokesh Vutla <[email protected]>
---
 arch/arm/mach-omap2/omap_hwmod_33xx_43xx_ipblock_data.c | 2 ++
 1 file changed, 2 insertions(+)
arch/arm/mach-omap2/omap_hwmod_33xx_43xx_ipblock_data.c +0 -0
index cabc569..2d92958 100644

arch/arm/mach-omap2/omap_hwmod_33xx_43xx_ipblock_data.c static struct omap_hwmod_class_sysconfig am33xx_rtc_sysc = { · line 911

Quoted 8 lines
arch/arm/mach-omap2/omap_hwmod_33xx_43xx_ipblock_data.c +2 -0
@@ -904,6 +904,8 @@ static struct omap_hwmod_class_sysconfig am33xx_rtc_sysc = {
 static struct omap_hwmod_class am33xx_rtc_hwmod_class = {
 	.name		= "rtc",
 	.sysc		= &am33xx_rtc_sysc,
+	.unlock		= &omap_hwmod_rtc_unlock,
+	.lock		= &omap_hwmod_rtc_lock,
 };
 
 struct omap_hwmod am33xx_rtc_hwmod = {

The DRA7xx integration from the previous patch should either be combined with this patch, or moved into a separate patch like this one (your preference).

- Paul

#8 lore raw

PATCH 3/3 ARM: AMx3xx: hwmod: RTC: Add lock and unlock functions

Hi Paul,

Quoted 30 lines · Paul Walmsley

Hi

Quoted 26 lines · Lokesh Vutla

RTC IP have kicker feature which prevents spurious writes to its registers. In order to write into any of the RTC registers, KICK values has to be written to KICK registers.

omap_hwmod_rtc_unlock/lock functions writes into these KICK registers inorder to lock and unlock RTC registers.
This patch hooks omap_hwmod_rtc_unlock/lock functions into RTC hwmod, so that SYSCONFIG register is updated properly.

Signed-off-by Lokesh Vutla <[email protected]>
---
  arch/arm/mach-omap2/omap_hwmod_33xx_43xx_ipblock_data.c | 2 ++
  1 file changed, 2 insertions(+)
arch/arm/mach-omap2/omap_hwmod_33xx_43xx_ipblock_data.c +2 -0
index cabc569..2d92958 100644
@@ -904,6 +904,8 @@ static struct omap_hwmod_class_sysconfig am33xx_rtc_sysc = {
  static struct omap_hwmod_class am33xx_rtc_hwmod_class = {
  	.name		= "rtc",
  	.sysc		= &am33xx_rtc_sysc,
+	.unlock		= &omap_hwmod_rtc_unlock,
+	.lock		= &omap_hwmod_rtc_lock,
  };
Quoted 5 lines

arch/arm/mach-omap2/omap_hwmod_33xx_43xx_ipblock_data.c static struct omap_hwmod_class_sysconfig am33xx_rtc_sysc = { · line 911

Quoted 1 line
arch/arm/mach-omap2/omap_hwmod_33xx_43xx_ipblock_data.c +0 -0
  struct omap_hwmod am33xx_rtc_hwmod = {

The DRA7xx integration from the previous patch should either be combined with this patch, or moved into a separate patch like this one (your preference).

Ill make a separate patch for DRA7xx integration and repost it.

Thanks and regards,
Lokesh

Quoted 4 lines

- Paul

author
#9 lore raw

cover ARM: OMAP2+: hwmod: RTC: Add lock and unlock hooks

In reply to Lokesh Vutla

Hi,

Quoted 6 lines · Lokesh Vutla

Some IP blocks like RTC, needs an additional unlocking mechanism for writing to its registers. This patch adds optional lock and unlock function pointers to the IP block's hwmod data which gets executed before and after writing into IP sysconfig register.
And also hook lock and unlock functions to AMx3xx, DRA7 RTC hwmod data, so that sysconfig registers are updated properly.

ping on this series.

Thanks and regards,
Lokesh

Quoted 20 lines

Tested on:
DRA7-evm: http://pastebin.ubuntu.com/11688889/
DRA72-evm: http://pastebin.ubuntu.com/11688901/
BeagleBoard-x15: http://pastebin.ubuntu.com/11688907/
BeagleBoneBlack: http://pastebin.ubuntu.com/11688923/
AM437x-gp-evm: http://pastebin.ubuntu.com/11689157/ (Used an out of tree patch to enable RTC)

Lokesh Vutla (3):
ARM: OMAP2+: hwmod: add support for lock and unlock hooks
ARM: DRA: hwmod: RTC: Add lock and unlock functions
ARM: AMx3xx: RTC: Add lock and unlock functions

 arch/arm/mach-omap2/omap_hwmod.c                   | 13 ++++++
 arch/arm/mach-omap2/omap_hwmod.h                   |  6 +++
 .../mach-omap2/omap_hwmod_33xx_43xx_ipblock_data.c |  2 +
 arch/arm/mach-omap2/omap_hwmod_7xx_data.c          |  2 +
 arch/arm/mach-omap2/omap_hwmod_reset.c             | 47 ++++++++++++++++++++++
 5 files changed, 70 insertions(+)
#10 lore raw

cover ARM: OMAP2+: hwmod: RTC: Add lock and unlock hooks

Quoted 8 lines · Lokesh Vutla

Hi,

Quoted 6 lines · Lokesh Vutla

Some IP blocks like RTC, needs an additional unlocking mechanism for writing to its registers. This patch adds optional lock and unlock function pointers to the IP block's hwmod data which gets executed before and after writing into IP sysconfig register.
And also hook lock and unlock functions to AMx3xx, DRA7 RTC hwmod data, so that sysconfig registers are updated properly.

Quoted 4 lines

ping on this series.

Thanks and regards,
Lokesh

This looks kind of hackish to have the unlock + lock functionality copy pasted to both driver and hwmod.

It is also racy, as there is no locking in place to avoid concurrent access to the lock/unlock registers across hwmod+driver.

Can we avoid these issues somehow?

-Tero

Quoted 21 lines
Quoted 20 lines

Tested on:
DRA7-evm: http://pastebin.ubuntu.com/11688889/
DRA72-evm: http://pastebin.ubuntu.com/11688901/
BeagleBoard-x15: http://pastebin.ubuntu.com/11688907/
BeagleBoneBlack: http://pastebin.ubuntu.com/11688923/
AM437x-gp-evm: http://pastebin.ubuntu.com/11689157/ (Used an out of tree patch to enable RTC)

Lokesh Vutla (3):
ARM: OMAP2+: hwmod: add support for lock and unlock hooks
ARM: DRA: hwmod: RTC: Add lock and unlock functions
ARM: AMx3xx: RTC: Add lock and unlock functions

  arch/arm/mach-omap2/omap_hwmod.c                   | 13 ++++++
  arch/arm/mach-omap2/omap_hwmod.h                   |  6 +++
  .../mach-omap2/omap_hwmod_33xx_43xx_ipblock_data.c |  2 +
  arch/arm/mach-omap2/omap_hwmod_7xx_data.c          |  2 +
  arch/arm/mach-omap2/omap_hwmod_reset.c             | 47 ++++++++++++++++++++++
  5 files changed, 70 insertions(+)
maintainer
#11 lore raw

cover ARM: OMAP2+: hwmod: RTC: Add lock and unlock hooks

Quoted 14 lines · Tero Kristo
Quoted 12 lines · Lokesh Vutla

Hi,

Quoted 6 lines · Lokesh Vutla

Some IP blocks like RTC, needs an additional unlocking mechanism for writing to its registers. This patch adds optional lock and unlock function pointers to the IP block's hwmod data which gets executed before and after writing into IP sysconfig register.
And also hook lock and unlock functions to AMx3xx, DRA7 RTC hwmod data, so that sysconfig registers are updated properly.

ping on this series.

Thanks and regards,
Lokesh

[...]

Quoted 2 lines

It is also racy, as there is no locking in place to avoid concurrent access to the lock/unlock registers across hwmod+driver.

I don't see the race. Where is it?

- Paul

#12 lore raw

cover ARM: OMAP2+: hwmod: RTC: Add lock and unlock hooks

Quoted 17 lines · Paul Walmsley
Quoted 9 lines · Tero Kristo
Quoted 8 lines · Lokesh Vutla

Hi,

Quoted 6 lines · Lokesh Vutla

Some IP blocks like RTC, needs an additional unlocking mechanism for writing to its registers. This patch adds optional lock and unlock function pointers to the IP block's hwmod data which gets executed before and after writing into IP sysconfig register.
And also hook lock and unlock functions to AMx3xx, DRA7 RTC hwmod data, so that sysconfig registers are updated properly.

Quoted 5 lines
Quoted 4 lines

ping on this series.

Thanks and regards,
Lokesh

Quoted 6 lines

[...]

Quoted 2 lines

It is also racy, as there is no locking in place to avoid concurrent access to the lock/unlock registers across hwmod+driver.

I don't see the race. Where is it?

See drivers/rtc/rtc-omap.c, am3352_rtc_unlock and am3352_rtc_lock.

That code is accessing the exact same registers.

-Tero

maintainer
#13 lore raw

cover ARM: OMAP2+: hwmod: RTC: Add lock and unlock hooks

Quoted 25 lines · Tero Kristo
Quoted 24 lines · Paul Walmsley
Quoted 15 lines · Tero Kristo
Quoted 13 lines · Lokesh Vutla

Hi,

Quoted 7 lines · Lokesh Vutla

Some IP blocks like RTC, needs an additional unlocking mechanism for writing to its registers. This patch adds optional lock and unlock function pointers to the IP block's hwmod data which gets executed before and after writing into IP sysconfig register.
And also hook lock and unlock functions to AMx3xx, DRA7 RTC hwmod data,
so that sysconfig registers are updated properly.

ping on this series.

Thanks and regards,
Lokesh

[...]

Quoted 3 lines

It is also racy, as there is no locking in place to avoid concurrent access to
the lock/unlock registers across hwmod+driver.

Quoted 5 lines
Quoted 1 line

I don't see the race. Where is it?

See drivers/rtc/rtc-omap.c, am3352_rtc_unlock and am3352_rtc_lock.

That code is accessing the exact same registers.

I guess my question is, when is it possible that code could race with the hwmod code for the same device?

- Paul

#14 lore raw

cover ARM: OMAP2+: hwmod: RTC: Add lock and unlock hooks

Quoted 27 lines · Paul Walmsley
Quoted 25 lines · Tero Kristo
Quoted 24 lines · Paul Walmsley
Quoted 15 lines · Tero Kristo
Quoted 13 lines · Lokesh Vutla

Hi,

Quoted 7 lines · Lokesh Vutla

Some IP blocks like RTC, needs an additional unlocking mechanism for writing to its registers. This patch adds optional lock and unlock function pointers to the IP block's hwmod data which gets executed before and after writing into IP sysconfig register.
And also hook lock and unlock functions to AMx3xx, DRA7 RTC hwmod data,
so that sysconfig registers are updated properly.

ping on this series.

Thanks and regards,
Lokesh

[...]

Quoted 3 lines

It is also racy, as there is no locking in place to avoid concurrent access to
the lock/unlock registers across hwmod+driver.

Quoted 8 lines
Quoted 5 lines
Quoted 1 line

I don't see the race. Where is it?

See drivers/rtc/rtc-omap.c, am3352_rtc_unlock and am3352_rtc_lock.

That code is accessing the exact same registers.

I guess my question is, when is it possible that code could race with the hwmod code for the same device?

Hmm yea I think you are right, this only gets potentially called within pm_runtime_get/put_sync for RTC.

The current sequence is highly inefficient though, as we are doing multiple lock/unlock operations to the RTC from multiple sources. See following rtcwake trace on am43xx-gp-evm as an example.

/ # rtcwake -s 4 -m mem

[    7.425322] am3352_rtc_unlock
[    7.428330] am3352_rtc_lock
[    7.431139] am3352_rtc_unlock
[    7.434116] am3352_rtc_lock
wakeup from "mem" at Sat Jan  1 00:00:11 2000
[    7.448549] PM: Syncing filesystems ... done.
[    7.455425] Freezing user space processes ... (elapsed 0.001 seconds) 
done.
[    7.463738] Freezing remaining freezable tasks ... (elapsed 0.001 
seconds) do
ne.
[    7.472532] Suspending console(s) (use no_console_suspend to debug)
[    7.481878] am3352_rtc_unlock
[    7.481889] am3352_rtc_lock
[    7.482307] PM: suspend of devices complete after 2.713 msecs
[    7.483479] PM: late suspend of devices complete after 1.153 msecs
[    7.484727] omap_hwmod_rtc_unlock
[    7.484733] omap_hwmod_rtc_lock
[    7.485182] PM: noirq suspend of devices complete after 1.685 msecs
[    7.485190] Disabling non-boot CPUs ...
[    7.485199] PM: Successfully put all powerdomains to target state
[    7.485199] PM: Wakeup source RTC Alarm
[    7.499853] PM: noirq resume of devices complete after 14.558 msecs
[    7.500047] am3352_rtc_unlock
[    7.500052] am3352_rtc_lock
[    7.500123] am3352_rtc_unlock
[    7.500128] am3352_rtc_lock
[    7.501019] PM: early resume of devices complete after 0.809 msecs
[    7.501464] am3352_rtc_unlock
[    7.501472] am3352_rtc_lock
[    7.558046] PM: resume of devices complete after 57.007 msecs
[    7.638807] Restarting tasks ... done.
[    7.643173] am3352_rtc_unlock
[    7.646162] am3352_rtc_lock

But, I guess this is for some interested party to optimize if needed, and it is mostly an issue with the RTC driver itself.

-Tero

#15 lore raw

cover ARM: OMAP2+: hwmod: RTC: Add lock and unlock hooks

Hi Tero,

Quoted 32 lines · Tero Kristo
Quoted 31 lines · Paul Walmsley
Quoted 29 lines · Tero Kristo
Quoted 18 lines · Paul Walmsley
Quoted 10 lines · Tero Kristo
Quoted 9 lines · Lokesh Vutla

Hi,

Quoted 7 lines · Lokesh Vutla

Some IP blocks like RTC, needs an additional unlocking mechanism for writing to its registers. This patch adds optional lock and unlock function pointers to the IP block's hwmod data which gets executed before and after writing into IP sysconfig register.
And also hook lock and unlock functions to AMx3xx, DRA7 RTC hwmod data,
so that sysconfig registers are updated properly.

Quoted 5 lines
Quoted 4 lines

ping on this series.

Thanks and regards,
Lokesh

Quoted 7 lines

[...]

Quoted 3 lines

It is also racy, as there is no locking in place to avoid concurrent access to
the lock/unlock registers across hwmod+driver.

I don't see the race. Where is it?

See drivers/rtc/rtc-omap.c, am3352_rtc_unlock and am3352_rtc_lock.

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

That code is accessing the exact same registers.

I guess my question is, when is it possible that code could race with the hwmod code for the same device?

Hmm yea I think you are right, this only gets potentially called within pm_runtime_get/put_sync for RTC.

Yes, sysc is written from hwmod_init code and pm_runtime_get/put_sysnc. And we write into rtc registers only after pm_runtime_get_sync and rtc_unlock.
There cannot be a race condition here.

Quoted 4 lines

The current sequence is highly inefficient though, as we are doing multiple lock/unlock operations to the RTC from multiple sources. See following rtcwake trace on am43xx-gp-evm as an example.

Initially I had a patch which leaves rtc unlocked at hwmod_init instead of doing
unlock and lock for each set of register writes in the driver. But it was rejected stating that deviates the purpose of locking mechanism. So I updated the driver for adapting this locking and unlocking mechanism.

Thanks and regards,
Lokesh

Quoted 43 lines

/ # rtcwake -s 4 -m mem

[    7.425322] am3352_rtc_unlock
[    7.428330] am3352_rtc_lock
[    7.431139] am3352_rtc_unlock
[    7.434116] am3352_rtc_lock
wakeup from "mem" at Sat Jan  1 00:00:11 2000
[    7.448549] PM: Syncing filesystems ... done.
[    7.455425] Freezing user space processes ... (elapsed 0.001 seconds)
done.
[    7.463738] Freezing remaining freezable tasks ... (elapsed 0.001
seconds) do
ne.
[    7.472532] Suspending console(s) (use no_console_suspend to debug)
[    7.481878] am3352_rtc_unlock
[    7.481889] am3352_rtc_lock
[    7.482307] PM: suspend of devices complete after 2.713 msecs
[    7.483479] PM: late suspend of devices complete after 1.153 msecs
[    7.484727] omap_hwmod_rtc_unlock
[    7.484733] omap_hwmod_rtc_lock
[    7.485182] PM: noirq suspend of devices complete after 1.685 msecs
[    7.485190] Disabling non-boot CPUs ...
[    7.485199] PM: Successfully put all powerdomains to target state
[    7.485199] PM: Wakeup source RTC Alarm
[    7.499853] PM: noirq resume of devices complete after 14.558 msecs
[    7.500047] am3352_rtc_unlock
[    7.500052] am3352_rtc_lock
[    7.500123] am3352_rtc_unlock
[    7.500128] am3352_rtc_lock
[    7.501019] PM: early resume of devices complete after 0.809 msecs
[    7.501464] am3352_rtc_unlock
[    7.501472] am3352_rtc_lock
[    7.558046] PM: resume of devices complete after 57.007 msecs
[    7.638807] Restarting tasks ... done.
[    7.643173] am3352_rtc_unlock
[    7.646162] am3352_rtc_lock

But, I guess this is for some interested party to optimize if needed, and it is mostly an issue with the RTC driver itself.

-Tero