Hi,
some comments.
On Wed, 10 Jun 2015, Lokesh Vutla wrote:
> 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 <lokeshvu...@ti.com>
> ---
> 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(+)
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod.h
> b/arch/arm/mach-omap2/omap_hwmod.h
> index 44c7db9..04855ab 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.h
> +++ b/arch/arm/mach-omap2/omap_hwmod.h
> @@ -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
> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> index 0e64c2f..983042f 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> @@ -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?
> diff --git a/arch/arm/mach-omap2/omap_hwmod_reset.c
> b/arch/arm/mach-omap2/omap_hwmod_reset.c
> index 65e186c..1fb106d 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_reset.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_reset.c
> @@ -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)
>
> return 0;
> }
> +
> +static void omap_rtc_wait_not_busy(struct omap_hwmod *oh)
This function is missing kerneldoc and desperately needs it.
> +{
> + 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.
> + 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.
> + */
> +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.
> + 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.
> + omap_hwmod_write(0x0, oh, OMAP_RTC_KICK0_REG);
> + omap_hwmod_write(0x0, oh, OMAP_RTC_KICK1_REG);
> +}
> --
> 1.9.1
>
- Paul
