Re: [RFC 1/2] ARM: OMAP2+: hwmod: Add refcounting for modulemode shared by multiple hwmods

2014-05-27 Thread Rajendra Nayak
On Monday 26 May 2014 04:14 PM, Archit Taneja wrote:
 Generally, IP blocks/modules within a clock domain each have their own
 CM_x_CLKCTRL register, each having it's own MODULEMODE field to manage the
 module.
 
 DSS clockdoain, however, has multiple modules in it, but only one register
 named CM_DSS_DSS_CLKCTRL, which contains one MODULEMODE register field.
 
 Until now, we defined '.prcm.omap4.modulemode' only for the top level DSS
 hwmod(dss_core) and didn't define it for other DSS hwmods(like dss_dispc,
 dss_dsi1 and so on). This made the omapdss driver work as the top level DSS
 platform device and the rest had a parent-child relationship. This ensured 
 that
 the parent hwmod(dss_core) is enabled if any of the children hwmods are
 enabled while using omapdss.
 
 This method, however, doesn't work when each hwmod is enabled individually.
 This happens early in boot in omap_hwmod_setup_all, where each hwmod is 
 enabled,
 and then reset and idled. All the 'children' DSS hwmods fail to enable and
 reset, since they don't enable modulemode.
 
 The way to make such modules work both during initialization and when used by
 pm runtime API in the driver would be to add refcounting for 
 enabling/disabling
 the modulemode field.
 
 We add a new flag bit for the flag in omap_hwmod_omap4_prcm, which tells
 omap_hwmod that this hwmod uses a modulemode field shared by other hwmods.
 
 We create a struct called 'modulemode_shared'. The hwmod data file should 
 define
 a static instance of this struct. Each hwmod that uses this modulemode field
 should hold a pointer to this instance.
 
 omap_hwmod's soc enable_module and disable_module ops set the MODULEMODE
 reigster bits only when the first module using it is enabled, or the last 
 module
 using it is disabled. We serialize accesses to the struct with a spinlock.
 
 Signed-off-by: Archit Taneja arc...@ti.com
 ---
  arch/arm/mach-omap2/omap_hwmod.c | 123 
 +--
  arch/arm/mach-omap2/omap_hwmod.h |  38 +---
  2 files changed, 133 insertions(+), 28 deletions(-)
 
 diff --git a/arch/arm/mach-omap2/omap_hwmod.c 
 b/arch/arm/mach-omap2/omap_hwmod.c
 index 66c60fe..b42718d 100644
 --- a/arch/arm/mach-omap2/omap_hwmod.c
 +++ b/arch/arm/mach-omap2/omap_hwmod.c
 @@ -973,17 +973,33 @@ static void _disable_optional_clocks(struct omap_hwmod 
 *oh)
   */
  static void _omap4_enable_module(struct omap_hwmod *oh)
  {
 + unsigned long flags;
 + struct modulemode_shared *mmode = NULL;
 + bool enable = true;
 +
   if (!oh-clkdm || !oh-prcm.omap4.modulemode)
   return;
  
   pr_debug(omap_hwmod: %s: %s: %d\n,
oh-name, __func__, oh-prcm.omap4.modulemode);
  
 - omap4_cminst_module_enable(oh-prcm.omap4.modulemode,
 -oh-clkdm-prcm_partition,
 -oh-clkdm-cm_inst,
 -oh-clkdm-clkdm_offs,
 -oh-prcm.omap4.clkctrl_offs);
 + if (oh-prcm.omap4.flags  HWMOD_OMAP4_MODULEMODE_SHARED) {
 + mmode = oh-prcm.omap4.modulemode_ref;
 +
 + spin_lock_irqsave(mmode-lock, flags);
 +
 + enable = mmode-refcnt++ == 0;
 + }
 +
 + if (enable)
 + omap4_cminst_module_enable(oh-prcm.omap4.modulemode,
 +oh-clkdm-prcm_partition,
 +oh-clkdm-cm_inst,
 +oh-clkdm-clkdm_offs,
 +oh-prcm.omap4.clkctrl_offs);
 +
 + if (mmode)
 + spin_unlock_irqrestore(mmode-lock, flags);
  }
  
  /**
 @@ -995,15 +1011,32 @@ static void _omap4_enable_module(struct omap_hwmod *oh)
   */
  static void _am33xx_enable_module(struct omap_hwmod *oh)
  {
 + unsigned long flags;
 + struct modulemode_shared *mmode = NULL;
 + bool enable = true;
 +
   if (!oh-clkdm || !oh-prcm.omap4.modulemode)
   return;
  
   pr_debug(omap_hwmod: %s: %s: %d\n,
oh-name, __func__, oh-prcm.omap4.modulemode);
  
 - am33xx_cm_module_enable(oh-prcm.omap4.modulemode, oh-clkdm-cm_inst,
 - oh-clkdm-clkdm_offs,
 - oh-prcm.omap4.clkctrl_offs);
 + if (oh-prcm.omap4.flags  HWMOD_OMAP4_MODULEMODE_SHARED) {
 + mmode = oh-prcm.omap4.modulemode_ref;
 +
 + spin_lock_irqsave(mmode-lock, flags);
 +
 + enable = mmode-refcnt++ == 0;
 + }
 +
 + if (enable)
 + am33xx_cm_module_enable(oh-prcm.omap4.modulemode,
 + oh-clkdm-cm_inst,
 + oh-clkdm-clkdm_offs,
 + oh-prcm.omap4.clkctrl_offs);
 +
 + if (mmode)
 + spin_unlock_irqrestore(mmode-lock, flags);
  }
  
  /**
 @@ -1846,6 +1879,9 @@ static bool _are_any_hardreset_lines_asserted(struct 
 omap_hwmod *oh)
  

Re: [RFC 1/2] ARM: OMAP2+: hwmod: Add refcounting for modulemode shared by multiple hwmods

2014-05-27 Thread Archit Taneja

Hi,

On Tuesday 27 May 2014 03:50 PM, Rajendra Nayak wrote:

On Monday 26 May 2014 04:14 PM, Archit Taneja wrote:

Generally, IP blocks/modules within a clock domain each have their own
CM_x_CLKCTRL register, each having it's own MODULEMODE field to manage the
module.

snip


@@ -2751,6 +2820,13 @@ static int __init _register(struct omap_hwmod *oh)
if (_lookup(oh-name))
return -EEXIST;

+   if (oh-prcm.omap4.flags  HWMOD_OMAP4_MODULEMODE_SHARED 
+   !oh-prcm.omap4.modulemode_ref) {


You might also want to check for someone populating a modulemode_ref but
failing to populate the flag?

Alternatively, Since you expect a modulemode_ref to be always available for all 
modules which
share modulemode, that in itself can be used to identify such modules without 
the
need of an additional flag?


It does seem redundant to have a flag at the moment. But the flag make 
things more visible. 'prcm.omap4.modulemode' seems to work without a 
flag too, so I suppose I'll remove the flag.





+   pr_err(omap_hwmod: %s shares modulemode, but doesn't hold a ref to 
it\n,
+   oh-name);
+   return -EINVAL;
+   }
+
list_add_tail(oh-node, omap_hwmod_list);

INIT_LIST_HEAD(oh-master_ports);
@@ -2759,6 +2835,15 @@ static int __init _register(struct omap_hwmod *oh)

oh-_state = _HWMOD_STATE_REGISTERED;

+   if (oh-prcm.omap4.flags  HWMOD_OMAP4_MODULEMODE_SHARED) {
+   struct modulemode_shared *mmode = oh-prcm.omap4.modulemode_ref;
+
+   if (!mmode-registered) {
+   spin_lock_init(mmode-lock);
+   mmode-registered = true;


If this is only used to keep track of the spin_lock being initialized, maybe 
it'll be
more readable if you just call it mmode-spin_lock_init = true.


Yes, I'll fix this.

Archit

--
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