Re: [RFC PATCH] ARM: DRA: hwmod: RTC: Add reset function for RTC

2015-02-12 Thread Lokesh Vutla
Hi Paul,
On Thursday 12 February 2015 10:11 PM, Paul Walmsley wrote:
 + Felipe, Nishanth
 
 Hi Lokesh,
 
 what's the status here?
Sorry for the delayed response.
I am currently on a high priority issue. Once I am done with it Ill address your
comments and repost the patch.

Thanks and regards,
Lokesh
 
 - Paul
 
 
 On Fri, 2 Jan 2015, Paul Walmsley wrote:
 
 Ping.  Are you going to redo this one?

 - Paul

 On Wed, 26 Nov 2014, Paul Walmsley wrote:

 Hi Lokesh 

 On Tue, 25 Nov 2014, Lokesh Vutla wrote:

 Hi Paul,
 On Thursday 20 November 2014 10:26 PM, Paul Walmsley wrote:
 On Thu, 20 Nov 2014, Lokesh Vutla wrote:

 On Monday 17 November 2014 10:13 AM, 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 te be
 written to KICK registers. Currently hwmod is updating the IDLEMODE in 
 rtc
 sysconfig register without writing into any kick register which is a 
 noop.
 When autoidle is allowed for rtc, interruts are not received until 
 IDLEMODE
 is set to SIDLE_SMART_WKUP.
 Adding a reset function to unlock RTC registers so that IDLEMODE is
 updated.
 Ping..!!
 Is this patch acceptable?

 Lokesh

 1. This looks like a fix.  Is this intended to go in as a v3.18-rc patch, 
 or against v3.19?  If so it would be very helpful for the maintainers if 
 you were to state that somewhere.
 Yes. This is a fix, intended to go in 3.18-rc. Sorry should have
 mentioned it.

 A few questions.  Do you know when this problem started (in terms of 
 kernel versions)?

 Also: the patch description states that this is only a problem when 
 autoidle is allowed for RTC.  What enables autoidle for RTC - the RTCSS 
 driver?  Or does hwmod wind up doing this after the RTCSS driver unlocks 
 it?

 2. Your patch unlocks the RTC registers, but never relocks it.  This 
 seems 
 to defeat the point of the spurious write protection.  Shouldn't your 
 patch only unlock the RTC immediately before the hwmod code touches the 
 RTC registers, and then relock it immediately afterwards, per SPRUHZ6 
 section 23.4.3.3?  If so then the .reset function pointer is the wrong 
 place for this; I would suggest adding some .lock and .unlock function 
 pointers that are to be called before and after any register write to the 
 IP block.
 Yeah I agree with you.
 Currently rtc driver unlocks these kick registers in probe and leaves it 
 unlocks for
 further use.
 But if hwmod does unlock and lock for every sysconfig write driver should 
 also
 implement unlock and lock for every rtc register write, which adds an 
 extra overhead.
 I am not sure if some one writes into rtc registers other than hwmod and 
 driver.
 IMO we can leave it unlocked as the driver does.

 I would think that the best approach would be to set up .lock and .unlock 
 function pointers, then add a temporary hwmod flag that, if set, would 
 prevent the .lock function from ever being called.  Then once the driver 
 is fixed, that flag can be dropped.

 3. Your macros don't mention DRA7xx specifically.  Does this sequence 
 apply to some other TI chips, or just DRA7xx?  Please document this in a 
 comment above the macros, and possibly change the name of the macros to 
 refer to DRA7XX.
 This sequence applies to AM43xx and AM33xx also. So made it generic.
 Ill document it.

 OK but it would need more than just documentation, right?  Wouldn't the 
 hwmod data also need to be modified for AM33xx/AM43xx to add the .reset 
 function pointer?  Any reason why folks wouldn't have seen this problem on 
 AM33xx/AM43xx?


 - Paul
 --
 To unsubscribe from this list: send the line unsubscribe linux-omap in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html



 - Paul

 
 
 - Paul
 

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH] ARM: DRA: hwmod: RTC: Add reset function for RTC

2015-02-12 Thread Paul Walmsley
+ Felipe, Nishanth

Hi Lokesh,

what's the status here?

- Paul


On Fri, 2 Jan 2015, Paul Walmsley wrote:

 Ping.  Are you going to redo this one?
 
 - Paul
 
 On Wed, 26 Nov 2014, Paul Walmsley wrote:
 
  Hi Lokesh 
  
  On Tue, 25 Nov 2014, Lokesh Vutla wrote:
  
   Hi Paul,
   On Thursday 20 November 2014 10:26 PM, Paul Walmsley wrote:
On Thu, 20 Nov 2014, Lokesh Vutla wrote:

On Monday 17 November 2014 10:13 AM, 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 te be
written to KICK registers. Currently hwmod is updating the IDLEMODE 
in rtc
sysconfig register without writing into any kick register which is a 
noop.
When autoidle is allowed for rtc, interruts are not received until 
IDLEMODE
is set to SIDLE_SMART_WKUP.
Adding a reset function to unlock RTC registers so that IDLEMODE is
updated.
Ping..!!
Is this patch acceptable?

Lokesh

1. This looks like a fix.  Is this intended to go in as a v3.18-rc 
patch, 
or against v3.19?  If so it would be very helpful for the maintainers 
if 
you were to state that somewhere.
   Yes. This is a fix, intended to go in 3.18-rc. Sorry should have
   mentioned it.
  
  A few questions.  Do you know when this problem started (in terms of 
  kernel versions)?
  
  Also: the patch description states that this is only a problem when 
  autoidle is allowed for RTC.  What enables autoidle for RTC - the RTCSS 
  driver?  Or does hwmod wind up doing this after the RTCSS driver unlocks 
  it?
  
2. Your patch unlocks the RTC registers, but never relocks it.  This 
seems 
to defeat the point of the spurious write protection.  Shouldn't your 
patch only unlock the RTC immediately before the hwmod code touches the 
RTC registers, and then relock it immediately afterwards, per SPRUHZ6 
section 23.4.3.3?  If so then the .reset function pointer is the wrong 
place for this; I would suggest adding some .lock and .unlock function 
pointers that are to be called before and after any register write to 
the 
IP block.
   Yeah I agree with you.
   Currently rtc driver unlocks these kick registers in probe and leaves it 
   unlocks for
   further use.
   But if hwmod does unlock and lock for every sysconfig write driver should 
   also
   implement unlock and lock for every rtc register write, which adds an 
   extra overhead.
   I am not sure if some one writes into rtc registers other than hwmod and 
   driver.
   IMO we can leave it unlocked as the driver does.
  
  I would think that the best approach would be to set up .lock and .unlock 
  function pointers, then add a temporary hwmod flag that, if set, would 
  prevent the .lock function from ever being called.  Then once the driver 
  is fixed, that flag can be dropped.
  
3. Your macros don't mention DRA7xx specifically.  Does this sequence 
apply to some other TI chips, or just DRA7xx?  Please document this in 
a 
comment above the macros, and possibly change the name of the macros to 
refer to DRA7XX.
   This sequence applies to AM43xx and AM33xx also. So made it generic.
   Ill document it.
  
  OK but it would need more than just documentation, right?  Wouldn't the 
  hwmod data also need to be modified for AM33xx/AM43xx to add the .reset 
  function pointer?  Any reason why folks wouldn't have seen this problem on 
  AM33xx/AM43xx?
  
  
  - Paul
  --
  To unsubscribe from this list: send the line unsubscribe linux-omap in
  the body of a message to majord...@vger.kernel.org
  More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
 
 
 - Paul
 


- Paul
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH] ARM: DRA: hwmod: RTC: Add reset function for RTC

2015-01-02 Thread Paul Walmsley
Ping.  Are you going to redo this one?

- Paul

On Wed, 26 Nov 2014, Paul Walmsley wrote:

 Hi Lokesh 
 
 On Tue, 25 Nov 2014, Lokesh Vutla wrote:
 
  Hi Paul,
  On Thursday 20 November 2014 10:26 PM, Paul Walmsley wrote:
   On Thu, 20 Nov 2014, Lokesh Vutla wrote:
   
   On Monday 17 November 2014 10:13 AM, 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 te be
   written to KICK registers. Currently hwmod is updating the IDLEMODE in 
   rtc
   sysconfig register without writing into any kick register which is a 
   noop.
   When autoidle is allowed for rtc, interruts are not received until 
   IDLEMODE
   is set to SIDLE_SMART_WKUP.
   Adding a reset function to unlock RTC registers so that IDLEMODE is
   updated.
   Ping..!!
   Is this patch acceptable?
   
   Lokesh
   
   1. This looks like a fix.  Is this intended to go in as a v3.18-rc patch, 
   or against v3.19?  If so it would be very helpful for the maintainers if 
   you were to state that somewhere.
  Yes. This is a fix, intended to go in 3.18-rc. Sorry should have
  mentioned it.
 
 A few questions.  Do you know when this problem started (in terms of 
 kernel versions)?
 
 Also: the patch description states that this is only a problem when 
 autoidle is allowed for RTC.  What enables autoidle for RTC - the RTCSS 
 driver?  Or does hwmod wind up doing this after the RTCSS driver unlocks 
 it?
 
   2. Your patch unlocks the RTC registers, but never relocks it.  This 
   seems 
   to defeat the point of the spurious write protection.  Shouldn't your 
   patch only unlock the RTC immediately before the hwmod code touches the 
   RTC registers, and then relock it immediately afterwards, per SPRUHZ6 
   section 23.4.3.3?  If so then the .reset function pointer is the wrong 
   place for this; I would suggest adding some .lock and .unlock function 
   pointers that are to be called before and after any register write to the 
   IP block.
  Yeah I agree with you.
  Currently rtc driver unlocks these kick registers in probe and leaves it 
  unlocks for
  further use.
  But if hwmod does unlock and lock for every sysconfig write driver should 
  also
  implement unlock and lock for every rtc register write, which adds an extra 
  overhead.
  I am not sure if some one writes into rtc registers other than hwmod and 
  driver.
  IMO we can leave it unlocked as the driver does.
 
 I would think that the best approach would be to set up .lock and .unlock 
 function pointers, then add a temporary hwmod flag that, if set, would 
 prevent the .lock function from ever being called.  Then once the driver 
 is fixed, that flag can be dropped.
 
   3. Your macros don't mention DRA7xx specifically.  Does this sequence 
   apply to some other TI chips, or just DRA7xx?  Please document this in a 
   comment above the macros, and possibly change the name of the macros to 
   refer to DRA7XX.
  This sequence applies to AM43xx and AM33xx also. So made it generic.
  Ill document it.
 
 OK but it would need more than just documentation, right?  Wouldn't the 
 hwmod data also need to be modified for AM33xx/AM43xx to add the .reset 
 function pointer?  Any reason why folks wouldn't have seen this problem on 
 AM33xx/AM43xx?
 
 
 - Paul
 --
 To unsubscribe from this list: send the line unsubscribe linux-omap in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 


- Paul
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH] ARM: DRA: hwmod: RTC: Add reset function for RTC

2014-11-26 Thread Lokesh Vutla
Hi Paul,
On Wednesday 26 November 2014 12:34 PM, Paul Walmsley wrote:
 Hi Lokesh 
 
 On Tue, 25 Nov 2014, Lokesh Vutla wrote:
 
 Hi Paul,
 On Thursday 20 November 2014 10:26 PM, Paul Walmsley wrote:
 On Thu, 20 Nov 2014, Lokesh Vutla wrote:

 On Monday 17 November 2014 10:13 AM, 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 te be
 written to KICK registers. Currently hwmod is updating the IDLEMODE in rtc
 sysconfig register without writing into any kick register which is a noop.
 When autoidle is allowed for rtc, interruts are not received until 
 IDLEMODE
 is set to SIDLE_SMART_WKUP.
 Adding a reset function to unlock RTC registers so that IDLEMODE is
 updated.
 Ping..!!
 Is this patch acceptable?

 Lokesh

 1. This looks like a fix.  Is this intended to go in as a v3.18-rc patch, 
 or against v3.19?  If so it would be very helpful for the maintainers if 
 you were to state that somewhere.
 Yes. This is a fix, intended to go in 3.18-rc. Sorry should have
 mentioned it.
 
 A few questions.  Do you know when this problem started (in terms of 
 kernel versions)?
This is introduced in v3.18 by commit
(6af16a1da ARM: DRA7: Add hook in SoC initcalls to enable pm initialization)
 
 Also: the patch description states that this is only a problem when 
 autoidle is allowed for RTC.  What enables autoidle for RTC - the RTCSS 
 driver?  Or does hwmod wind up doing this after the RTCSS driver unlocks 
 it?
Autoidle is enabled by hwmod by omap2_clk_enable_autoidle_all().
The above patch does it.
 
 2. Your patch unlocks the RTC registers, but never relocks it.  This seems 
 to defeat the point of the spurious write protection.  Shouldn't your 
 patch only unlock the RTC immediately before the hwmod code touches the 
 RTC registers, and then relock it immediately afterwards, per SPRUHZ6 
 section 23.4.3.3?  If so then the .reset function pointer is the wrong 
 place for this; I would suggest adding some .lock and .unlock function 
 pointers that are to be called before and after any register write to the 
 IP block.
 Yeah I agree with you.
 Currently rtc driver unlocks these kick registers in probe and leaves it 
 unlocks for
 further use.
 But if hwmod does unlock and lock for every sysconfig write driver should 
 also
 implement unlock and lock for every rtc register write, which adds an extra 
 overhead.
 I am not sure if some one writes into rtc registers other than hwmod and 
 driver.
 IMO we can leave it unlocked as the driver does.
 
 I would think that the best approach would be to set up .lock and .unlock 
 function pointers, then add a temporary hwmod flag that, if set, would 
 prevent the .lock function from ever being called.  Then once the driver 
 is fixed, that flag can be dropped.
Okay will update it and repost.
 
 3. Your macros don't mention DRA7xx specifically.  Does this sequence 
 apply to some other TI chips, or just DRA7xx?  Please document this in a 
 comment above the macros, and possibly change the name of the macros to 
 refer to DRA7XX.
 This sequence applies to AM43xx and AM33xx also. So made it generic.
 Ill document it.
 
 OK but it would need more than just documentation, right?  Wouldn't the 
 hwmod data also need to be modified for AM33xx/AM43xx to add the .reset 
 function pointer?  Any reason why folks wouldn't have seen this problem on 
 AM33xx/AM43xx?
PRCM in AM33xx and AM43xx does not support HW AUTO for clock domains.
It is either SW_SLEEP/SW_WKUP or NO_SLEEP. 
So RTC is still getting interrupts even IDLEMODE is kept in SMART_IDLE(which is 
reset value).

Thanks and regards,
Lokesh

 
 
 - Paul
 

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH] ARM: DRA: hwmod: RTC: Add reset function for RTC

2014-11-25 Thread Paul Walmsley
Hi Lokesh 

On Tue, 25 Nov 2014, Lokesh Vutla wrote:

 Hi Paul,
 On Thursday 20 November 2014 10:26 PM, Paul Walmsley wrote:
  On Thu, 20 Nov 2014, Lokesh Vutla wrote:
  
  On Monday 17 November 2014 10:13 AM, 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 te be
  written to KICK registers. Currently hwmod is updating the IDLEMODE in rtc
  sysconfig register without writing into any kick register which is a noop.
  When autoidle is allowed for rtc, interruts are not received until 
  IDLEMODE
  is set to SIDLE_SMART_WKUP.
  Adding a reset function to unlock RTC registers so that IDLEMODE is
  updated.
  Ping..!!
  Is this patch acceptable?
  
  Lokesh
  
  1. This looks like a fix.  Is this intended to go in as a v3.18-rc patch, 
  or against v3.19?  If so it would be very helpful for the maintainers if 
  you were to state that somewhere.
 Yes. This is a fix, intended to go in 3.18-rc. Sorry should have
 mentioned it.

A few questions.  Do you know when this problem started (in terms of 
kernel versions)?

Also: the patch description states that this is only a problem when 
autoidle is allowed for RTC.  What enables autoidle for RTC - the RTCSS 
driver?  Or does hwmod wind up doing this after the RTCSS driver unlocks 
it?

  2. Your patch unlocks the RTC registers, but never relocks it.  This seems 
  to defeat the point of the spurious write protection.  Shouldn't your 
  patch only unlock the RTC immediately before the hwmod code touches the 
  RTC registers, and then relock it immediately afterwards, per SPRUHZ6 
  section 23.4.3.3?  If so then the .reset function pointer is the wrong 
  place for this; I would suggest adding some .lock and .unlock function 
  pointers that are to be called before and after any register write to the 
  IP block.
 Yeah I agree with you.
 Currently rtc driver unlocks these kick registers in probe and leaves it 
 unlocks for
 further use.
 But if hwmod does unlock and lock for every sysconfig write driver should also
 implement unlock and lock for every rtc register write, which adds an extra 
 overhead.
 I am not sure if some one writes into rtc registers other than hwmod and 
 driver.
 IMO we can leave it unlocked as the driver does.

I would think that the best approach would be to set up .lock and .unlock 
function pointers, then add a temporary hwmod flag that, if set, would 
prevent the .lock function from ever being called.  Then once the driver 
is fixed, that flag can be dropped.

  3. Your macros don't mention DRA7xx specifically.  Does this sequence 
  apply to some other TI chips, or just DRA7xx?  Please document this in a 
  comment above the macros, and possibly change the name of the macros to 
  refer to DRA7XX.
 This sequence applies to AM43xx and AM33xx also. So made it generic.
 Ill document it.

OK but it would need more than just documentation, right?  Wouldn't the 
hwmod data also need to be modified for AM33xx/AM43xx to add the .reset 
function pointer?  Any reason why folks wouldn't have seen this problem on 
AM33xx/AM43xx?


- Paul
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH] ARM: DRA: hwmod: RTC: Add reset function for RTC

2014-11-24 Thread Lokesh Vutla
Hi Paul,
On Thursday 20 November 2014 10:26 PM, Paul Walmsley wrote:
 On Thu, 20 Nov 2014, Lokesh Vutla wrote:
 
 On Monday 17 November 2014 10:13 AM, 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 te be
 written to KICK registers. Currently hwmod is updating the IDLEMODE in rtc
 sysconfig register without writing into any kick register which is a noop.
 When autoidle is allowed for rtc, interruts are not received until IDLEMODE
 is set to SIDLE_SMART_WKUP.
 Adding a reset function to unlock RTC registers so that IDLEMODE is
 updated.
 Ping..!!
 Is this patch acceptable?
 
 Lokesh
 
 1. This looks like a fix.  Is this intended to go in as a v3.18-rc patch, 
 or against v3.19?  If so it would be very helpful for the maintainers if 
 you were to state that somewhere.
Yes. This is a fix, intended to go in 3.18-rc. Sorry should have
mentioned it.
 
 2. Your patch unlocks the RTC registers, but never relocks it.  This seems 
 to defeat the point of the spurious write protection.  Shouldn't your 
 patch only unlock the RTC immediately before the hwmod code touches the 
 RTC registers, and then relock it immediately afterwards, per SPRUHZ6 
 section 23.4.3.3?  If so then the .reset function pointer is the wrong 
 place for this; I would suggest adding some .lock and .unlock function 
 pointers that are to be called before and after any register write to the 
 IP block.
Yeah I agree with you.
Currently rtc driver unlocks these kick registers in probe and leaves it 
unlocks for
further use.
But if hwmod does unlock and lock for every sysconfig write driver should also
implement unlock and lock for every rtc register write, which adds an extra 
overhead.
I am not sure if some one writes into rtc registers other than hwmod and driver.
IMO we can leave it unlocked as the driver does.
 
 3. Your macros don't mention DRA7xx specifically.  Does this sequence 
 apply to some other TI chips, or just DRA7xx?  Please document this in a 
 comment above the macros, and possibly change the name of the macros to 
 refer to DRA7XX.
This sequence applies to AM43xx and AM33xx also. So made it generic.
Ill document it.

Thanks and regards,
Lokesh
 
 
 - Paul
 

 Thanks and regards,
 Lokesh

 Signed-off-by: Lokesh Vutla lokeshvu...@ti.com
 ---
  arch/arm/mach-omap2/omap_hwmod.h  |  1 +
  arch/arm/mach-omap2/omap_hwmod_7xx_data.c |  1 +
  arch/arm/mach-omap2/omap_hwmod_reset.c| 23 +++
  3 files changed, 25 insertions(+)

 diff --git a/arch/arm/mach-omap2/omap_hwmod.h 
 b/arch/arm/mach-omap2/omap_hwmod.h
 index 512f809..3703f99 100644
 --- a/arch/arm/mach-omap2/omap_hwmod.h
 +++ b/arch/arm/mach-omap2/omap_hwmod.h
 @@ -744,6 +744,7 @@ const char *omap_hwmod_get_main_clk(struct omap_hwmod 
 *oh);
   */
  
  extern int omap_hwmod_aess_preprogram(struct omap_hwmod *oh);
 +int omap_hwmod_rtc_unlock(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 5684f11..90e1df4 100644
 --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
 +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
 @@ -1584,6 +1584,7 @@ static struct omap_hwmod_class_sysconfig 
 dra7xx_rtcss_sysc = {
  static struct omap_hwmod_class dra7xx_rtcss_hwmod_class = {
 .name   = rtcss,
 .sysc   = dra7xx_rtcss_sysc,
 +   .reset  = omap_hwmod_rtc_unlock,
  };
  
  /* rtcss */
 diff --git a/arch/arm/mach-omap2/omap_hwmod_reset.c 
 b/arch/arm/mach-omap2/omap_hwmod_reset.c
 index 65e186c..b825a28 100644
 --- a/arch/arm/mach-omap2/omap_hwmod_reset.c
 +++ b/arch/arm/mach-omap2/omap_hwmod_reset.c
 @@ -30,6 +30,12 @@
  
  #include omap_hwmod.h
  
 +#define RTC_KICK0_REG_OFFSET   0x6c
 +#define RTC_KICK1_REG_OFFSET   0x70
 +
 +#define RTC_KICK0_VALUE0x83E70B13
 +#define RTC_KICK1_VALUE0x95A4F1E0
 +
  /**
   * omap_hwmod_aess_preprogram - enable AESS internal autogating
   * @oh: struct omap_hwmod *
 @@ -51,3 +57,20 @@ int omap_hwmod_aess_preprogram(struct omap_hwmod *oh)
  
 return 0;
  }
 +
 +/**
 + * 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.
 + */
 +int omap_hwmod_rtc_unlock(struct omap_hwmod *oh)
 +{
 +   omap_hwmod_write(RTC_KICK0_VALUE, oh, RTC_KICK0_REG_OFFSET);
 +   omap_hwmod_write(RTC_KICK1_VALUE, oh, RTC_KICK1_REG_OFFSET);
 +
 +   return 0;
 +}


 
 
 - Paul
 

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  

Re: [RFC PATCH] ARM: DRA: hwmod: RTC: Add reset function for RTC

2014-11-20 Thread Lokesh Vutla
Hi Paul,
On Monday 17 November 2014 10:13 AM, 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 te be
 written to KICK registers. Currently hwmod is updating the IDLEMODE in rtc
 sysconfig register without writing into any kick register which is a noop.
 When autoidle is allowed for rtc, interruts are not received until IDLEMODE
 is set to SIDLE_SMART_WKUP.
 Adding a reset function to unlock RTC registers so that IDLEMODE is
 updated.
Ping..!!
Is this patch acceptable?

Thanks and regards,
Lokesh
 
 Signed-off-by: Lokesh Vutla lokeshvu...@ti.com
 ---
  arch/arm/mach-omap2/omap_hwmod.h  |  1 +
  arch/arm/mach-omap2/omap_hwmod_7xx_data.c |  1 +
  arch/arm/mach-omap2/omap_hwmod_reset.c| 23 +++
  3 files changed, 25 insertions(+)
 
 diff --git a/arch/arm/mach-omap2/omap_hwmod.h 
 b/arch/arm/mach-omap2/omap_hwmod.h
 index 512f809..3703f99 100644
 --- a/arch/arm/mach-omap2/omap_hwmod.h
 +++ b/arch/arm/mach-omap2/omap_hwmod.h
 @@ -744,6 +744,7 @@ const char *omap_hwmod_get_main_clk(struct omap_hwmod 
 *oh);
   */
  
  extern int omap_hwmod_aess_preprogram(struct omap_hwmod *oh);
 +int omap_hwmod_rtc_unlock(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 5684f11..90e1df4 100644
 --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
 +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
 @@ -1584,6 +1584,7 @@ static struct omap_hwmod_class_sysconfig 
 dra7xx_rtcss_sysc = {
  static struct omap_hwmod_class dra7xx_rtcss_hwmod_class = {
   .name   = rtcss,
   .sysc   = dra7xx_rtcss_sysc,
 + .reset  = omap_hwmod_rtc_unlock,
  };
  
  /* rtcss */
 diff --git a/arch/arm/mach-omap2/omap_hwmod_reset.c 
 b/arch/arm/mach-omap2/omap_hwmod_reset.c
 index 65e186c..b825a28 100644
 --- a/arch/arm/mach-omap2/omap_hwmod_reset.c
 +++ b/arch/arm/mach-omap2/omap_hwmod_reset.c
 @@ -30,6 +30,12 @@
  
  #include omap_hwmod.h
  
 +#define RTC_KICK0_REG_OFFSET 0x6c
 +#define RTC_KICK1_REG_OFFSET 0x70
 +
 +#define RTC_KICK0_VALUE  0x83E70B13
 +#define RTC_KICK1_VALUE  0x95A4F1E0
 +
  /**
   * omap_hwmod_aess_preprogram - enable AESS internal autogating
   * @oh: struct omap_hwmod *
 @@ -51,3 +57,20 @@ int omap_hwmod_aess_preprogram(struct omap_hwmod *oh)
  
   return 0;
  }
 +
 +/**
 + * 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.
 + */
 +int omap_hwmod_rtc_unlock(struct omap_hwmod *oh)
 +{
 + omap_hwmod_write(RTC_KICK0_VALUE, oh, RTC_KICK0_REG_OFFSET);
 + omap_hwmod_write(RTC_KICK1_VALUE, oh, RTC_KICK1_REG_OFFSET);
 +
 + return 0;
 +}
 

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH] ARM: DRA: hwmod: RTC: Add reset function for RTC

2014-11-20 Thread Paul Walmsley
On Thu, 20 Nov 2014, Lokesh Vutla wrote:

 On Monday 17 November 2014 10:13 AM, 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 te be
  written to KICK registers. Currently hwmod is updating the IDLEMODE in rtc
  sysconfig register without writing into any kick register which is a noop.
  When autoidle is allowed for rtc, interruts are not received until IDLEMODE
  is set to SIDLE_SMART_WKUP.
  Adding a reset function to unlock RTC registers so that IDLEMODE is
  updated.
 Ping..!!
 Is this patch acceptable?

Lokesh

1. This looks like a fix.  Is this intended to go in as a v3.18-rc patch, 
or against v3.19?  If so it would be very helpful for the maintainers if 
you were to state that somewhere.

2. Your patch unlocks the RTC registers, but never relocks it.  This seems 
to defeat the point of the spurious write protection.  Shouldn't your 
patch only unlock the RTC immediately before the hwmod code touches the 
RTC registers, and then relock it immediately afterwards, per SPRUHZ6 
section 23.4.3.3?  If so then the .reset function pointer is the wrong 
place for this; I would suggest adding some .lock and .unlock function 
pointers that are to be called before and after any register write to the 
IP block.

3. Your macros don't mention DRA7xx specifically.  Does this sequence 
apply to some other TI chips, or just DRA7xx?  Please document this in a 
comment above the macros, and possibly change the name of the macros to 
refer to DRA7XX.


- Paul

 
 Thanks and regards,
 Lokesh
  
  Signed-off-by: Lokesh Vutla lokeshvu...@ti.com
  ---
   arch/arm/mach-omap2/omap_hwmod.h  |  1 +
   arch/arm/mach-omap2/omap_hwmod_7xx_data.c |  1 +
   arch/arm/mach-omap2/omap_hwmod_reset.c| 23 +++
   3 files changed, 25 insertions(+)
  
  diff --git a/arch/arm/mach-omap2/omap_hwmod.h 
  b/arch/arm/mach-omap2/omap_hwmod.h
  index 512f809..3703f99 100644
  --- a/arch/arm/mach-omap2/omap_hwmod.h
  +++ b/arch/arm/mach-omap2/omap_hwmod.h
  @@ -744,6 +744,7 @@ const char *omap_hwmod_get_main_clk(struct omap_hwmod 
  *oh);
*/
   
   extern int omap_hwmod_aess_preprogram(struct omap_hwmod *oh);
  +int omap_hwmod_rtc_unlock(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 5684f11..90e1df4 100644
  --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
  +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
  @@ -1584,6 +1584,7 @@ static struct omap_hwmod_class_sysconfig 
  dra7xx_rtcss_sysc = {
   static struct omap_hwmod_class dra7xx_rtcss_hwmod_class = {
  .name   = rtcss,
  .sysc   = dra7xx_rtcss_sysc,
  +   .reset  = omap_hwmod_rtc_unlock,
   };
   
   /* rtcss */
  diff --git a/arch/arm/mach-omap2/omap_hwmod_reset.c 
  b/arch/arm/mach-omap2/omap_hwmod_reset.c
  index 65e186c..b825a28 100644
  --- a/arch/arm/mach-omap2/omap_hwmod_reset.c
  +++ b/arch/arm/mach-omap2/omap_hwmod_reset.c
  @@ -30,6 +30,12 @@
   
   #include omap_hwmod.h
   
  +#define RTC_KICK0_REG_OFFSET   0x6c
  +#define RTC_KICK1_REG_OFFSET   0x70
  +
  +#define RTC_KICK0_VALUE0x83E70B13
  +#define RTC_KICK1_VALUE0x95A4F1E0
  +
   /**
* omap_hwmod_aess_preprogram - enable AESS internal autogating
* @oh: struct omap_hwmod *
  @@ -51,3 +57,20 @@ int omap_hwmod_aess_preprogram(struct omap_hwmod *oh)
   
  return 0;
   }
  +
  +/**
  + * 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.
  + */
  +int omap_hwmod_rtc_unlock(struct omap_hwmod *oh)
  +{
  +   omap_hwmod_write(RTC_KICK0_VALUE, oh, RTC_KICK0_REG_OFFSET);
  +   omap_hwmod_write(RTC_KICK1_VALUE, oh, RTC_KICK1_REG_OFFSET);
  +
  +   return 0;
  +}
  
 


- Paul
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html