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

PATCH New Lokesh Vutla 3 participants 10 messages Feb 5, 2016 – Feb 24, 2016

Subsystems (5)

Arm (32-bit) Arm sub-architectures OMAP2+ support OMAP hwmod data +1 more subsystems

lore

Share & tools

lore

Lokesh Vutla

author

2016-02-05 05:42 UTC #0 lore raw

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

This series implements lock and unlock functions for RTC and hooks the same to DRA7 and AMx3xx hwmod.

Tested RTC functionality on:
DRA74-EVM: http://pastebin.ubuntu.com/14885542/
BeagleBoard-X15: http://pastebin.ubuntu.com/14885568/
AM437x-GP EVM: http://pastebin.ubuntu.com/14885589/
AM335x BeagleBoneBlack: http://pastebin.ubuntu.com/14885600/

Changes since v3:
- Rebased on top of latest Mainline.
Link to v3 series: https://www.mail-archive.com/linux-omap%40vger.kernel.org/msg118342.html

Lokesh Vutla (3):
ARM: hwmod: RTC: Add lock and unlock functions
ARM: DRA7: RTC: Add lock and unlock functions
ARM: AMx3xx: RTC: Add lock and unlock functions

 arch/arm/mach-omap2/omap_hwmod.h                   |  2 +
 .../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             | 65 ++++++++++++++++++++++
 4 files changed, 71 insertions(+)

Signature · 1 line

2.1.4

Lokesh Vutla

author

2016-02-05 05:42 UTC #1 lore raw

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

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.

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

---
 arch/arm/mach-omap2/omap_hwmod.h       |  2 ++
 arch/arm/mach-omap2/omap_hwmod_reset.c | 65 ++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+)

arch/arm/mach-omap2/omap_hwmod.h +2 -0

index 76bce11..649576e 100644
@@ -751,6 +751,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_reset.c +65 -0

74 lines not on this page · raw message

Signature · 1 line

2.1.4

Paul Walmsley

maintainer

2016-02-07 22:24 UTC #2 lore raw

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

Hi Lokesh

I apologize for the delay.

A few comments:

Quoted 8 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.
Signed-off-by Lokesh Vutla <[email protected]>
---

...

Quoted 7 lines

+/**
+ * omap_rtc_wait_not_busy - Wait for the RTC BUSY flag
+ * @oh: struct omap_hwmod *
+ *
+ * For updating certain RTC registers, the MPU must wait
+ * for the BUSY status in OMAP_RTC_STATUS_REG to become zero.
+ * Once the BUSY status is zero, there is a 15-?s access

Probably best just to write out "microseconds" or "us" here to avoid the high-bit character problem.

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

Quoted 9 lines

+ * period in which the MPU can program.
+ */

arch/arm/mach-omap2/omap_hwmod_reset.c +7 -0

+static void omap_rtc_wait_not_busy(struct omap_hwmod *oh)
+{
+	int i;
+
+	/* BUSY may stay active for 1/32768 second (~30 usec) */
+	omap_test_timeout(omap_hwmod_read(oh, OMAP_RTC_STATUS_REG)
+			  & OMAP_RTC_STATUS_REG, OMAP_RTC_MAX_READY_TIME, i);

This test looks bogus. Shouldn't it AND the register value with OMAP_RTC_STATUS_BUSY? Right now the code is AND-ing with 0x44, which doesn't include the BUSY bit. So I guess the tests that you mentioned in the first message of the series don't cover the BUSY case?

Quoted 9 lines

arch/arm/mach-omap2/omap_hwmod_reset.c +9 -0

+	/* now we have ~15 usec to read/write various registers */
+}
+
+/**
+ * omap_hwmod_rtc_unlock - 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

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

Quoted 10 lines

arch/arm/mach-omap2/omap_hwmod_reset.c +10 -0

+ * written in respective KICK registers. This is needed for hwmod to write into
+ * sysconfig register.
+ */
+void omap_hwmod_rtc_unlock(struct omap_hwmod *oh)
+{
+	local_irq_disable();
+	omap_rtc_wait_not_busy(oh);
+	omap_hwmod_write(OMAP_RTC_KICK0_VALUE, oh, OMAP_RTC_KICK0_REG);
+	omap_hwmod_write(OMAP_RTC_KICK1_VALUE, oh, OMAP_RTC_KICK1_REG);
+	local_irq_enable();

Finally, could you ask the IP block maintainer to confirm the interpretation that, for any STATUS_REG read where the BUSY bit is 0, that we are guaranteed to have at least 15 microseconds from that point in time to access the IP block? It appears to be this way from my reading of the TRM; but to me, the phrasing is not explicit. Another interpretation could be that the BUSY bit reflects the IP block's current status. If this latter case is true, then to ensure that the IP block accesses complete inside the access window, we'll either need to test the BUSY bit after the writes to ensure that it is still 0, and otherwise repeat the busy test and writes; or we'll need to wait for a 0->1 BUSY transition before we wait for a 1->0 BUSY transition.

Quoted 22 lines

arch/arm/mach-omap2/omap_hwmod_reset.c +19 -0

+}
+
+/**
+ * omap_hwmod_rtc_lock - Lock the Kicker mechanism.
+ * @oh: struct omap_hwmod *
+ *
+ * RTC IP have kicker feature. This prevents spurious writes to its registers.
+ * Once the RTC registers are written, KICK mechanism needs to be locked,
+ * in order to prevent any spurious writes. This function locks back the RTC
+ * registers once hwmod completes its write into sysconfig register.
+ */
+void omap_hwmod_rtc_lock(struct omap_hwmod *oh)
+{
+	local_irq_disable();
+	omap_rtc_wait_not_busy(oh);
+	omap_hwmod_write(0x0, oh, OMAP_RTC_KICK0_REG);
+	omap_hwmod_write(0x0, oh, OMAP_RTC_KICK1_REG);
+	local_irq_enable();
+}

Signature · 1 line

2.1.4

- Paul

Paul Walmsley

maintainer

2016-02-18 06:27 UTC #3 lore raw

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

Hi Lokesh

Quoted 84 lines · Paul Walmsley

A few comments:
Quoted 8 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.
Signed-off-by Lokesh Vutla <[email protected]>
---
...
Quoted 7 lines
+/**
+ * omap_rtc_wait_not_busy - Wait for the RTC BUSY flag
+ * @oh: struct omap_hwmod *
+ *
+ * For updating certain RTC registers, the MPU must wait
+ * for the BUSY status in OMAP_RTC_STATUS_REG to become zero.
+ * Once the BUSY status is zero, there is a 15-?s access
Probably best just to write out "microseconds" or "us" here to avoid the high-bit character problem.
arch/arm/mach-omap2/omap_hwmod_reset.c int omap_hwmod_aess_preprogram(struct omap_hwmod *oh) · line 80
Quoted 9 lines
+ * period in which the MPU can program.
+ */
arch/arm/mach-omap2/omap_hwmod_reset.c +7 -0
+static void omap_rtc_wait_not_busy(struct omap_hwmod *oh)
+{
+	int i;
+
+	/* BUSY may stay active for 1/32768 second (~30 usec) */
+	omap_test_timeout(omap_hwmod_read(oh, OMAP_RTC_STATUS_REG)
+			  & OMAP_RTC_STATUS_REG, OMAP_RTC_MAX_READY_TIME, i);
This test looks bogus. Shouldn't it AND the register value with OMAP_RTC_STATUS_BUSY? Right now the code is AND-ing with 0x44, which doesn't include the BUSY bit. So I guess the tests that you mentioned in the first message of the series don't cover the BUSY case?
Quoted 9 lines
arch/arm/mach-omap2/omap_hwmod_reset.c +9 -0
+	/* now we have ~15 usec to read/write various registers */
+}
+
+/**
+ * omap_hwmod_rtc_unlock - 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
arch/arm/mach-omap2/omap_hwmod_reset.c int omap_hwmod_aess_preprogram(struct omap_hwmod *oh) · line 99
Quoted 10 lines
arch/arm/mach-omap2/omap_hwmod_reset.c +10 -0
+ * written in respective KICK registers. This is needed for hwmod to write into
+ * sysconfig register.
+ */
+void omap_hwmod_rtc_unlock(struct omap_hwmod *oh)
+{
+	local_irq_disable();
+	omap_rtc_wait_not_busy(oh);
+	omap_hwmod_write(OMAP_RTC_KICK0_VALUE, oh, OMAP_RTC_KICK0_REG);
+	omap_hwmod_write(OMAP_RTC_KICK1_VALUE, oh, OMAP_RTC_KICK1_REG);
+	local_irq_enable();
Finally, could you ask the IP block maintainer to confirm the interpretation that, for any STATUS_REG read where the BUSY bit is 0, that we are guaranteed to have at least 15 microseconds from that point in time to access the IP block? It appears to be this way from my reading of the TRM; but to me, the phrasing is not explicit. Another interpretation could be that the BUSY bit reflects the IP block's current status. If this latter case is true, then to ensure that the IP block accesses complete inside the access window, we'll either need to test the BUSY bit after the writes to ensure that it is still 0, and otherwise repeat the busy test and writes; or we'll need to wait for a 0->1 BUSY transition before we wait for a 1->0 BUSY transition.
Quoted 11 lines
arch/arm/mach-omap2/omap_hwmod_reset.c +11 -0
+}
+
+/**
+ * omap_hwmod_rtc_lock - Lock the Kicker mechanism.
+ * @oh: struct omap_hwmod *
+ *
+ * RTC IP have kicker feature. This prevents spurious writes to its registers.
+ * Once the RTC registers are written, KICK mechanism needs to be locked,
+ * in order to prevent any spurious writes. This function locks back the RTC
+ * registers once hwmod completes its 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 118
Quoted 8 lines
arch/arm/mach-omap2/omap_hwmod_reset.c +8 -0
+void omap_hwmod_rtc_lock(struct omap_hwmod *oh)
+{
+	local_irq_disable();
+	omap_rtc_wait_not_busy(oh);
+	omap_hwmod_write(0x0, oh, OMAP_RTC_KICK0_REG);
+	omap_hwmod_write(0x0, oh, OMAP_RTC_KICK1_REG);
+	local_irq_enable();
+}

Any update?

- Paul

Lokesh Vutla

2016-02-18 09:19 UTC #4 lore raw

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

Hi Paul,

Quoted 90 lines · Paul Walmsley

Hi Lokesh
Quoted 86 lines · Paul Walmsley
A few comments:
Quoted 8 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.
Signed-off-by Lokesh Vutla <[email protected]>
---
...
Quoted 7 lines
+/**
+ * omap_rtc_wait_not_busy - Wait for the RTC BUSY flag
+ * @oh: struct omap_hwmod *
+ *
+ * For updating certain RTC registers, the MPU must wait
+ * for the BUSY status in OMAP_RTC_STATUS_REG to become zero.
+ * Once the BUSY status is zero, there is a 15-?s access
Probably best just to write out "microseconds" or "us" here to avoid the high-bit character problem.
arch/arm/mach-omap2/omap_hwmod_reset.c int omap_hwmod_aess_preprogram(struct omap_hwmod *oh) · line 80
Quoted 9 lines
+ * period in which the MPU can program.
+ */
arch/arm/mach-omap2/omap_hwmod_reset.c +7 -0
+static void omap_rtc_wait_not_busy(struct omap_hwmod *oh)
+{
+	int i;
+
+	/* BUSY may stay active for 1/32768 second (~30 usec) */
+	omap_test_timeout(omap_hwmod_read(oh, OMAP_RTC_STATUS_REG)
+			  & OMAP_RTC_STATUS_REG, OMAP_RTC_MAX_READY_TIME, i);
This test looks bogus. Shouldn't it AND the register value with OMAP_RTC_STATUS_BUSY? Right now the code is AND-ing with 0x44, which doesn't include the BUSY bit. So I guess the tests that you mentioned in the first message of the series don't cover the BUSY case?
Quoted 9 lines
arch/arm/mach-omap2/omap_hwmod_reset.c +9 -0
+	/* now we have ~15 usec to read/write various registers */
+}
+
+/**
+ * omap_hwmod_rtc_unlock - 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
arch/arm/mach-omap2/omap_hwmod_reset.c int omap_hwmod_aess_preprogram(struct omap_hwmod *oh) · line 99
Quoted 10 lines
arch/arm/mach-omap2/omap_hwmod_reset.c +10 -0
+ * written in respective KICK registers. This is needed for hwmod to write into
+ * sysconfig register.
+ */
+void omap_hwmod_rtc_unlock(struct omap_hwmod *oh)
+{
+	local_irq_disable();
+	omap_rtc_wait_not_busy(oh);
+	omap_hwmod_write(OMAP_RTC_KICK0_VALUE, oh, OMAP_RTC_KICK0_REG);
+	omap_hwmod_write(OMAP_RTC_KICK1_VALUE, oh, OMAP_RTC_KICK1_REG);
+	local_irq_enable();
Finally, could you ask the IP block maintainer to confirm the interpretation that, for any STATUS_REG read where the BUSY bit is 0, that we are guaranteed to have at least 15 microseconds from that point in time to access the IP block? It appears to be this way from my reading of the TRM; but to me, the phrasing is not explicit. Another interpretation could be that the BUSY bit reflects the IP block's current status. If this latter case is true, then to ensure that the IP block accesses complete inside the access window, we'll either need to test the BUSY bit after the writes to ensure that it is still 0, and otherwise repeat the busy test and writes; or we'll need to wait for a 0->1 BUSY transition before we wait for a 1->0 BUSY transition.
Quoted 13 lines
arch/arm/mach-omap2/omap_hwmod_reset.c +13 -0
+}
+
+/**
+ * omap_hwmod_rtc_lock - Lock the Kicker mechanism.
+ * @oh: struct omap_hwmod *
+ *
+ * RTC IP have kicker feature. This prevents spurious writes to its registers.
+ * Once the RTC registers are written, KICK mechanism needs to be locked,
+ * in order to prevent any spurious writes. This function locks back the RTC
+ * registers once hwmod completes its write into sysconfig register.
+ */
+void omap_hwmod_rtc_lock(struct omap_hwmod *oh)
+{

Quoted 8 lines

Quoted 6 lines
arch/arm/mach-omap2/omap_hwmod_reset.c int omap_hwmod_aess_preprogram(struct omap_hwmod *oh) · line 118
Quoted 6 lines
arch/arm/mach-omap2/omap_hwmod_reset.c +6 -0
+	local_irq_disable();
+	omap_rtc_wait_not_busy(oh);
+	omap_hwmod_write(0x0, oh, OMAP_RTC_KICK0_REG);
+	omap_hwmod_write(0x0, oh, OMAP_RTC_KICK1_REG);
+	local_irq_enable();
+}
Any update?

Sorry for the delay, I am on a vacation right now. I have already sent a mail to the HW guys. Will update you as soon as I receive a reply from them.

Thanks and regards,
Lokesh

Quoted 4 lines

- Paul

Paul Walmsley

maintainer

2016-02-18 17:17 UTC #5 lore raw

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

Quoted 3 lines · Lokesh Vutla

Sorry for the delay, I am on a vacation right now. I have already sent a mail to the HW guys. Will update you as soon as I receive a reply from them.

Got it; thanks Lokesh.

- Paul

Lokesh Vutla

2016-02-24 09:29 UTC #6 lore raw

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

In reply to Paul Walmsley

Hi Paul,

Quoted 68 lines · Paul Walmsley

Hi Lokesh
I apologize for the delay.
A few comments:
Quoted 8 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.
Signed-off-by Lokesh Vutla <[email protected]>
---
...
Quoted 7 lines
+/**
+ * omap_rtc_wait_not_busy - Wait for the RTC BUSY flag
+ * @oh: struct omap_hwmod *
+ *
+ * For updating certain RTC registers, the MPU must wait
+ * for the BUSY status in OMAP_RTC_STATUS_REG to become zero.
+ * Once the BUSY status is zero, there is a 15-?s access
Probably best just to write out "microseconds" or "us" here to avoid the high-bit character problem.
arch/arm/mach-omap2/omap_hwmod_reset.c int omap_hwmod_aess_preprogram(struct omap_hwmod *oh) · line 80
Quoted 9 lines
+ * period in which the MPU can program.
+ */
arch/arm/mach-omap2/omap_hwmod_reset.c +7 -0
+static void omap_rtc_wait_not_busy(struct omap_hwmod *oh)
+{
+	int i;
+
+	/* BUSY may stay active for 1/32768 second (~30 usec) */
+	omap_test_timeout(omap_hwmod_read(oh, OMAP_RTC_STATUS_REG)
+			  & OMAP_RTC_STATUS_REG, OMAP_RTC_MAX_READY_TIME, i);
This test looks bogus. Shouldn't it AND the register value with OMAP_RTC_STATUS_BUSY? Right now the code is AND-ing with 0x44, which doesn't include the BUSY bit. So I guess the tests that you mentioned in the first message of the series don't cover the BUSY case?
Quoted 9 lines
arch/arm/mach-omap2/omap_hwmod_reset.c +9 -0
+	/* now we have ~15 usec to read/write various registers */
+}
+
+/**
+ * omap_hwmod_rtc_unlock - 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
arch/arm/mach-omap2/omap_hwmod_reset.c int omap_hwmod_aess_preprogram(struct omap_hwmod *oh) · line 99
Quoted 10 lines
arch/arm/mach-omap2/omap_hwmod_reset.c +10 -0
+ * written in respective KICK registers. This is needed for hwmod to write into
+ * sysconfig register.
+ */
+void omap_hwmod_rtc_unlock(struct omap_hwmod *oh)
+{
+	local_irq_disable();
+	omap_rtc_wait_not_busy(oh);
+	omap_hwmod_write(OMAP_RTC_KICK0_VALUE, oh, OMAP_RTC_KICK0_REG);
+	omap_hwmod_write(OMAP_RTC_KICK1_VALUE, oh, OMAP_RTC_KICK1_REG);
+	local_irq_enable();
Finally, could you ask the IP block maintainer to confirm the interpretation that, for any STATUS_REG read where the BUSY bit is 0, that we are guaranteed to have at least 15 microseconds from that point in time

Quoted 8 lines

to access the IP block? It appears to be this way from my reading of the TRM; but to me, the phrasing is not explicit. Another interpretation could be that the BUSY bit reflects the IP block's current status. If this latter case is true, then to ensure that the IP block accesses complete inside the access window, we'll either need to test the BUSY bit after the writes to ensure that it is still 0, and otherwise repeat the busy test and writes; or we'll need to wait for a 0->1 BUSY transition before we wait for a 1->0 BUSY transition.

As per the HW folks, once SW sees the transistion from 1->0, it is guaranteed that SW has 15.25 us to do all the write transactions for all the TC updates.

Also as per TRM, in RTC there are three types of registers:
-> TC registers and TC alarm registers:
-> General registers
-> Compensation registers.

MPU must wait for the BUSY bit for accessing the TC registers and MPU can access the general registers at any point of time. SYSCONFIG, KICK registers comes under general registers. Since hwmod modifies only the sysconfig register, this omap_rtc_wait_not_busy() is not required here. We can directly write into sysconfig and KICK registers. Does this sound fine to you?
Shall I resend this patch without omap_rtc_wait_not_busy()?

Thanks and regards,
Lokesh

Quoted 27 lines

Quoted 23 lines

arch/arm/mach-omap2/omap_hwmod_reset.c +19 -0

+}
+
+/**
+ * omap_hwmod_rtc_lock - Lock the Kicker mechanism.
+ * @oh: struct omap_hwmod *
+ *
+ * RTC IP have kicker feature. This prevents spurious writes to its registers.
+ * Once the RTC registers are written, KICK mechanism needs to be locked,
+ * in order to prevent any spurious writes. This function locks back the RTC
+ * registers once hwmod completes its write into sysconfig register.
+ */
+void omap_hwmod_rtc_lock(struct omap_hwmod *oh)
+{
+	local_irq_disable();
+	omap_rtc_wait_not_busy(oh);
+	omap_hwmod_write(0x0, oh, OMAP_RTC_KICK0_REG);
+	omap_hwmod_write(0x0, oh, OMAP_RTC_KICK1_REG);
+	local_irq_enable();
+}

Signature · 1 line

2.1.4

- Paul

Paul Walmsley

maintainer

2016-02-24 18:07 UTC #7 lore raw

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

Lokesh,

Quoted 72 lines · Lokesh Vutla

Quoted 62 lines · Paul Walmsley
Quoted 8 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.
Signed-off-by Lokesh Vutla <[email protected]>
---
...
Quoted 7 lines
+/**
+ * omap_rtc_wait_not_busy - Wait for the RTC BUSY flag
+ * @oh: struct omap_hwmod *
+ *
+ * For updating certain RTC registers, the MPU must wait
+ * for the BUSY status in OMAP_RTC_STATUS_REG to become zero.
+ * Once the BUSY status is zero, there is a 15-?s access
Probably best just to write out "microseconds" or "us" here to avoid the high-bit character problem.
arch/arm/mach-omap2/omap_hwmod_reset.c int omap_hwmod_aess_preprogram(struct omap_hwmod *oh) · line 80
Quoted 9 lines
+ * period in which the MPU can program.
+ */
arch/arm/mach-omap2/omap_hwmod_reset.c +7 -0
+static void omap_rtc_wait_not_busy(struct omap_hwmod *oh)
+{
+	int i;
+
+	/* BUSY may stay active for 1/32768 second (~30 usec) */
+	omap_test_timeout(omap_hwmod_read(oh, OMAP_RTC_STATUS_REG)
+			  & OMAP_RTC_STATUS_REG, OMAP_RTC_MAX_READY_TIME, i);
This test looks bogus. Shouldn't it AND the register value with OMAP_RTC_STATUS_BUSY? Right now the code is AND-ing with 0x44, which doesn't include the BUSY bit. So I guess the tests that you mentioned in the first message of the series don't cover the BUSY case?
Quoted 9 lines
arch/arm/mach-omap2/omap_hwmod_reset.c +9 -0
+	/* now we have ~15 usec to read/write various registers */
+}
+
+/**
+ * omap_hwmod_rtc_unlock - 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
arch/arm/mach-omap2/omap_hwmod_reset.c int omap_hwmod_aess_preprogram(struct omap_hwmod *oh) · line 99
Quoted 10 lines
arch/arm/mach-omap2/omap_hwmod_reset.c +10 -0
+ * written in respective KICK registers. This is needed for hwmod to write into
+ * sysconfig register.
+ */
+void omap_hwmod_rtc_unlock(struct omap_hwmod *oh)
+{
+	local_irq_disable();
+	omap_rtc_wait_not_busy(oh);
+	omap_hwmod_write(OMAP_RTC_KICK0_VALUE, oh, OMAP_RTC_KICK0_REG);
+	omap_hwmod_write(OMAP_RTC_KICK1_VALUE, oh, OMAP_RTC_KICK1_REG);
+	local_irq_enable();
Finally, could you ask the IP block maintainer to confirm the interpretation that, for any STATUS_REG read where the BUSY bit is 0, that we are guaranteed to have at least 15 microseconds from that point in time
Quoted 8 lines
to access the IP block? It appears to be this way from my reading of the TRM; but to me, the phrasing is not explicit. Another interpretation could be that the BUSY bit reflects the IP block's current status. If this latter case is true, then to ensure that the IP block accesses complete inside the access window, we'll either need to test the BUSY bit after the writes to ensure that it is still 0, and otherwise repeat the busy test and writes; or we'll need to wait for a 0->1 BUSY transition before we wait for a 1->0 BUSY transition.

Quoted 3 lines

As per the HW folks, once SW sees the transistion from 1->0, it is guaranteed that SW has 15.25 us to do all the write transactions for all the TC updates.

Great

Quoted 5 lines

Also as per TRM, in RTC there are three types of registers:
-> TC registers and TC alarm registers:
-> General registers
-> Compensation registers.

Quoted 7 lines

MPU must wait for the BUSY bit for accessing the TC registers and MPU can access the general registers at any point of time. SYSCONFIG, KICK registers comes under general registers. Since hwmod modifies only the sysconfig register, this omap_rtc_wait_not_busy() is not required here. We can directly write into sysconfig and KICK registers. Does this sound fine to you?
Shall I resend this patch without omap_rtc_wait_not_busy()?

If there's no need for the unlock/lock sequence to ensure that the IP block is completely reset, then there doesn't seem to be much point in including it?

- Paul

Lokesh Vutla

author

2016-02-05 05:42 UTC #8 lore raw

PATCH v4 2/3 ARM: DRA7: RTC: Add lock and unlock functions

In reply to Lokesh Vutla

Hook 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_7xx_data.c | 2 ++
 1 file changed, 2 insertions(+)

arch/arm/mach-omap2/omap_hwmod_7xx_data.c +2 -0

index 848356e..6625650 100644
@@ -1609,6 +1609,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 */

Signature · 1 line

2.1.4

Lokesh Vutla

author

2016-02-05 05:42 UTC #9 lore raw

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

In reply to Lokesh Vutla

Hook 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 907a452b..aed3362 100644
@@ -918,6 +918,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

2.1.4