Re: [PATCHv6 01/11] omap: prcm: switch to a chained IRQ handler mechanism

2011-09-14 Thread Paul Walmsley
Hello Tero,

On Fri, 2 Sep 2011, Tero Kristo wrote:

 On Fri, 2011-09-02 at 11:20 +0200, Paul Walmsley wrote:

  If it would help, I'd be happy to do a first draft of the OMAP3430 PRM 
  hwmod data.
 
 If you can do this it would help, as you have much better understanding
 of the hwmod data than I do. It will probably drop a couple of review
 rounds away as the hwmod data would be close to what it should be from
 beginning.
 
 If you are busy with other things, I can see what I can craft myself.

Not sure if you're still waiting on this, but here's a draft of a PRM 
hwmod for OMAP34xx for testing purposes.  It needs a little more review 
and thought before being ready for merging, but I think it will work for 
your purposes.  The OMAP2430 version should be quite similar.  Comments 
welcome,


- Paul

From: Paul Walmsley p...@pwsan.com
Date: Sun, 4 Sep 2011 18:26:58 -0600
Subject: [PATCH] OMAP3xxx: hwmod data: add PRM hwmod

*** DRAFT ***
---
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |   63 
 1 files changed, 63 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c 
b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index 25bf43b..4b3640e 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -161,6 +161,7 @@ static struct omap_hwmod omap3xxx_l3_main_hwmod = {
 };
 
 static struct omap_hwmod omap3xxx_l4_wkup_hwmod;
+static struct omap_hwmod omap3xxx_prm_hwmod;
 static struct omap_hwmod omap3xxx_uart1_hwmod;
 static struct omap_hwmod omap3xxx_uart2_hwmod;
 static struct omap_hwmod omap3xxx_uart3_hwmod;
@@ -478,6 +479,29 @@ static struct omap_hwmod omap3xxx_l4_per_hwmod = {
.flags  = HWMOD_NO_IDLEST,
 };
 
+static struct omap_hwmod_addr_space omap3xxx_prm_addrs[] = {
+   {
+   .pa_start   = 0x48306000,
+   .pa_end = 0x48306000 + SZ_8K + SZ_4K - 1,
+   .flags  = ADDR_TYPE_RT
+   },
+   { }
+};
+
+/* L4_WKUP - PRM interface */
+static struct omap_hwmod_ocp_if omap3xxx_l4_wkup__prm = {
+   .master = omap3xxx_l4_wkup_hwmod,
+   .slave  = omap3xxx_prm_hwmod,
+   .clk= wkup_l4_ick,
+   .addr   = omap3xxx_prm_addrs,
+   .user   = OCP_USER_MPU,
+};
+
+/* Master interfaces on the L4_WKUP interconnect */
+static struct omap_hwmod_ocp_if *omap3xxx_l4_wkup_masters[] = {
+   omap3xxx_l4_wkup__prm,
+};
+
 /* Slave interfaces on the L4_WKUP interconnect */
 static struct omap_hwmod_ocp_if *omap3xxx_l4_wkup_slaves[] = {
omap3xxx_l4_core__l4_wkup,
@@ -487,12 +511,48 @@ static struct omap_hwmod_ocp_if 
*omap3xxx_l4_wkup_slaves[] = {
 static struct omap_hwmod omap3xxx_l4_wkup_hwmod = {
.name   = l4_wkup,
.class  = l4_hwmod_class,
+   .masters= omap3xxx_l4_wkup_masters,
+   .masters_cnt= ARRAY_SIZE(omap3xxx_l4_wkup_masters),
.slaves = omap3xxx_l4_wkup_slaves,
.slaves_cnt = ARRAY_SIZE(omap3xxx_l4_wkup_slaves),
.omap_chip  = OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
.flags  = HWMOD_NO_IDLEST,
 };
 
+/* Slave interfaces on the L4_WKUP interconnect */
+static struct omap_hwmod_ocp_if *omap3xxx_prm_slaves[] = {
+   omap3xxx_l4_wkup__prm,
+};
+
+static struct omap_hwmod_class_sysconfig omap3xxx_prm_sysc = {
+   .rev_offs   = 0x0804,
+   .sysc_offs  = 0x0814,
+   .sysc_flags = SYSC_HAS_AUTOIDLE,
+   .sysc_fields= omap_hwmod_sysc_type1,
+};
+
+static struct omap_hwmod_class omap3xxx_prm_hwmod_class = {
+   .name   = prm,
+   .sysc   = omap3xxx_prm_sysc,
+};
+
+static struct omap_hwmod_irq_info omap3xxx_prm_irqs[] = {
+   { .irq = 11 },
+   { .irq = -1 }
+};
+
+/* PRM */
+static struct omap_hwmod omap3xxx_prm_hwmod = {
+   .name   = prm,
+   .mpu_irqs   = omap3xxx_prm_irqs,
+   .class  = omap3xxx_prm_hwmod_class,
+   .main_clk   = wkup_32k_fck,
+   .slaves = omap3xxx_prm_slaves,
+   .slaves_cnt = ARRAY_SIZE(omap3xxx_prm_slaves),
+   .omap_chip  = OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
+   .flags  = HWMOD_NO_IDLEST,
+};
+
 /* Master interfaces on the MPU device */
 static struct omap_hwmod_ocp_if *omap3xxx_mpu_masters[] = {
omap3xxx_mpu__l3_main,
@@ -3201,6 +3261,9 @@ static __initdata struct omap_hwmod *omap3xxx_hwmods[] = {
omap3xxx_l4_core_hwmod,
omap3xxx_l4_per_hwmod,
omap3xxx_l4_wkup_hwmod,
+
+   omap3xxx_prm_hwmod,
+
omap3xxx_mmc1_hwmod,
omap3xxx_mmc2_hwmod,
omap3xxx_mmc3_hwmod,
-- 
1.7.5.4

--
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: [PATCHv6 01/11] omap: prcm: switch to a chained IRQ handler mechanism

2011-09-14 Thread Tero Kristo
On Wed, 2011-09-14 at 14:10 +0200, Paul Walmsley wrote:
 Hello Tero,
 
 On Fri, 2 Sep 2011, Tero Kristo wrote:
 
  On Fri, 2011-09-02 at 11:20 +0200, Paul Walmsley wrote:
 
   If it would help, I'd be happy to do a first draft of the OMAP3430 PRM 
   hwmod data.
  
  If you can do this it would help, as you have much better understanding
  of the hwmod data than I do. It will probably drop a couple of review
  rounds away as the hwmod data would be close to what it should be from
  beginning.
  
  If you are busy with other things, I can see what I can craft myself.
 
 Not sure if you're still waiting on this, but here's a draft of a PRM 
 hwmod for OMAP34xx for testing purposes.  It needs a little more review 
 and thought before being ready for merging, but I think it will work for 
 your purposes.  The OMAP2430 version should be quite similar.  Comments 
 welcome,
 

Thanks Paul,

I'll start experimenting with this.

 
 - Paul
 
 From: Paul Walmsley p...@pwsan.com
 Date: Sun, 4 Sep 2011 18:26:58 -0600
 Subject: [PATCH] OMAP3xxx: hwmod data: add PRM hwmod
 
 *** DRAFT ***
 ---
  arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |   63 
 
  1 files changed, 63 insertions(+), 0 deletions(-)
 
 diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c 
 b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
 index 25bf43b..4b3640e 100644
 --- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
 +++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
 @@ -161,6 +161,7 @@ static struct omap_hwmod omap3xxx_l3_main_hwmod = {
  };
  
  static struct omap_hwmod omap3xxx_l4_wkup_hwmod;
 +static struct omap_hwmod omap3xxx_prm_hwmod;
  static struct omap_hwmod omap3xxx_uart1_hwmod;
  static struct omap_hwmod omap3xxx_uart2_hwmod;
  static struct omap_hwmod omap3xxx_uart3_hwmod;
 @@ -478,6 +479,29 @@ static struct omap_hwmod omap3xxx_l4_per_hwmod = {
   .flags  = HWMOD_NO_IDLEST,
  };
  
 +static struct omap_hwmod_addr_space omap3xxx_prm_addrs[] = {
 + {
 + .pa_start   = 0x48306000,
 + .pa_end = 0x48306000 + SZ_8K + SZ_4K - 1,
 + .flags  = ADDR_TYPE_RT
 + },
 + { }
 +};
 +
 +/* L4_WKUP - PRM interface */
 +static struct omap_hwmod_ocp_if omap3xxx_l4_wkup__prm = {
 + .master = omap3xxx_l4_wkup_hwmod,
 + .slave  = omap3xxx_prm_hwmod,
 + .clk= wkup_l4_ick,
 + .addr   = omap3xxx_prm_addrs,
 + .user   = OCP_USER_MPU,
 +};
 +
 +/* Master interfaces on the L4_WKUP interconnect */
 +static struct omap_hwmod_ocp_if *omap3xxx_l4_wkup_masters[] = {
 + omap3xxx_l4_wkup__prm,
 +};
 +
  /* Slave interfaces on the L4_WKUP interconnect */
  static struct omap_hwmod_ocp_if *omap3xxx_l4_wkup_slaves[] = {
   omap3xxx_l4_core__l4_wkup,
 @@ -487,12 +511,48 @@ static struct omap_hwmod_ocp_if 
 *omap3xxx_l4_wkup_slaves[] = {
  static struct omap_hwmod omap3xxx_l4_wkup_hwmod = {
   .name   = l4_wkup,
   .class  = l4_hwmod_class,
 + .masters= omap3xxx_l4_wkup_masters,
 + .masters_cnt= ARRAY_SIZE(omap3xxx_l4_wkup_masters),
   .slaves = omap3xxx_l4_wkup_slaves,
   .slaves_cnt = ARRAY_SIZE(omap3xxx_l4_wkup_slaves),
   .omap_chip  = OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
   .flags  = HWMOD_NO_IDLEST,
  };
  
 +/* Slave interfaces on the L4_WKUP interconnect */
 +static struct omap_hwmod_ocp_if *omap3xxx_prm_slaves[] = {
 + omap3xxx_l4_wkup__prm,
 +};
 +
 +static struct omap_hwmod_class_sysconfig omap3xxx_prm_sysc = {
 + .rev_offs   = 0x0804,
 + .sysc_offs  = 0x0814,
 + .sysc_flags = SYSC_HAS_AUTOIDLE,
 + .sysc_fields= omap_hwmod_sysc_type1,
 +};
 +
 +static struct omap_hwmod_class omap3xxx_prm_hwmod_class = {
 + .name   = prm,
 + .sysc   = omap3xxx_prm_sysc,
 +};
 +
 +static struct omap_hwmod_irq_info omap3xxx_prm_irqs[] = {
 + { .irq = 11 },
 + { .irq = -1 }
 +};
 +
 +/* PRM */
 +static struct omap_hwmod omap3xxx_prm_hwmod = {
 + .name   = prm,
 + .mpu_irqs   = omap3xxx_prm_irqs,
 + .class  = omap3xxx_prm_hwmod_class,
 + .main_clk   = wkup_32k_fck,
 + .slaves = omap3xxx_prm_slaves,
 + .slaves_cnt = ARRAY_SIZE(omap3xxx_prm_slaves),
 + .omap_chip  = OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
 + .flags  = HWMOD_NO_IDLEST,
 +};
 +
  /* Master interfaces on the MPU device */
  static struct omap_hwmod_ocp_if *omap3xxx_mpu_masters[] = {
   omap3xxx_mpu__l3_main,
 @@ -3201,6 +3261,9 @@ static __initdata struct omap_hwmod *omap3xxx_hwmods[] 
 = {
   omap3xxx_l4_core_hwmod,
   omap3xxx_l4_per_hwmod,
   omap3xxx_l4_wkup_hwmod,
 +
 + omap3xxx_prm_hwmod,
 +
   omap3xxx_mmc1_hwmod,
   omap3xxx_mmc2_hwmod,
   omap3xxx_mmc3_hwmod,



Texas Instruments Oy, Tekniikantie 12, 02150 Espoo. Y-tunnus: 0115040-6. 
Kotipaikka: Helsinki
 

--
To unsubscribe from this list: send 

Re: [PATCHv6 01/11] omap: prcm: switch to a chained IRQ handler mechanism

2011-09-02 Thread Paul Walmsley
Hi Tero,

On Thu, 1 Sep 2011, Tero Kristo wrote:

 I've been looking at this now and got one question below. Otherwise your
 comments look okay to me and I can work with those.

Great.  As you work on it, please let me know if there's something that 
doesn't make sense with this arrangement.  I think this will work, and 
will move some code out of arch/arm/*omap*, but it's hard to tell, until 
someone tries it.

 On Fri, 2011-08-26 at 11:12 +0200, Paul Walmsley wrote:
 
  What I'd suggest is to create a short series that:
  
  1. adds PRM hwmod data for OMAP2430+ platforms
 
 How should this be done? It believe all the data in the hwmods should be
 autogenerated somehow... should I just make a temporary hack patch for
 one platform that could be then autogenerated by someone for all omap
 platforms?

Only OMAP4 is autogenerated, currently.  OMAP2  3 are still done by hand. 
I'd suggest starting with either OMAP4 (because the hwmod data should be 
autogeneratable) or OMAP3 (because we know that one pretty well and the 
hwmod data should not be too difficult to build).

If it would help, I'd be happy to do a first draft of the OMAP3430 PRM 
hwmod data.


regards,

- 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: [PATCHv6 01/11] omap: prcm: switch to a chained IRQ handler mechanism

2011-09-02 Thread Tero Kristo
Hi Paul,

On Fri, 2011-09-02 at 11:20 +0200, Paul Walmsley wrote:
 Hi Tero,
 
 On Thu, 1 Sep 2011, Tero Kristo wrote:
 
  I've been looking at this now and got one question below. Otherwise your
  comments look okay to me and I can work with those.
 
 Great.  As you work on it, please let me know if there's something that 
 doesn't make sense with this arrangement.  I think this will work, and 
 will move some code out of arch/arm/*omap*, but it's hard to tell, until 
 someone tries it.
 
  On Fri, 2011-08-26 at 11:12 +0200, Paul Walmsley wrote:
  
   What I'd suggest is to create a short series that:
   
   1. adds PRM hwmod data for OMAP2430+ platforms
  
  How should this be done? It believe all the data in the hwmods should be
  autogenerated somehow... should I just make a temporary hack patch for
  one platform that could be then autogenerated by someone for all omap
  platforms?
 
 Only OMAP4 is autogenerated, currently.  OMAP2  3 are still done by hand. 
 I'd suggest starting with either OMAP4 (because the hwmod data should be 
 autogeneratable) or OMAP3 (because we know that one pretty well and the 
 hwmod data should not be too difficult to build).
 
 If it would help, I'd be happy to do a first draft of the OMAP3430 PRM 
 hwmod data.

If you can do this it would help, as you have much better understanding
of the hwmod data than I do. It will probably drop a couple of review
rounds away as the hwmod data would be close to what it should be from
beginning.

If you are busy with other things, I can see what I can craft myself.

 
 
 regards,
 
 - Paul



Texas Instruments Oy, Tekniikantie 12, 02150 Espoo. Y-tunnus: 0115040-6. 
Kotipaikka: Helsinki
 

--
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: [PATCHv6 01/11] omap: prcm: switch to a chained IRQ handler mechanism

2011-09-01 Thread Tero Kristo
Hey Paul,

I've been looking at this now and got one question below. Otherwise your
comments look okay to me and I can work with those.

On Fri, 2011-08-26 at 11:12 +0200, Paul Walmsley wrote:
 Hello Tero,
 
 a few comments on this patch:
 
 On Mon, 25 Jul 2011, Tero Kristo wrote:
 
  Introduce a chained interrupt handler mechanism for the PRCM
  interrupt, so that individual PRCM event can cleanly be handled by
  handlers in separate drivers. We do this by introducing PRCM event
  names, which are then matched to the particular PRCM interrupt bit
  depending on the specific OMAP SoC being used.
  
  arch/arm/mach-omap2/prcm.c implements the chained interrupt mechanism
  itself, with SoC specific support / init structure defined in
  arch/arm/mach-omap2/prm2xxx_3xxx.c and arch/arm/mach-omap2/prm4xxx.c
  respectively. At initialization time, the set of PRCM events is filtered
  against the SoC on which we are running, keeping only the ones that are
  actually useful. All the logic is written to be generic with regard to
  OMAP3/OMAP4, even though OMAP3 has single PRCM event registers and OMAP4
  has two PRCM event registers.
 
 Looking over this patch, it seems that this functionality should be
 part of a PRM device driver.  That would allow the separation of the
 SoC-specific data from the code, so there wouldn't be a need to embed
 the OMAP_PRCM_IRQ data in the driver code.  Rather, that data could go
 into the dev_attr data for the PRM hwmod.  That avoids putting
 SoC-specific data in driver code, allows the removal of
 omap[34]_prcm_irq_setup(), and should also remove the dependency on
 omap_chip.
 
 Similarly, OMAP_PRCM_MAX_NR_PENDING_REG and OMAP_PRCM_NR_IRQS should
 be defined somewhere SoC-specific.  I'd suggest defining those in the
 hwmod dev_attr data.  That way that file won't need to be patched if
 those constants need change in the future.  Unfortunately, doing this
 in a clean way will probably mean that the variables that are
 allocated via these constants will need to be allocated and freed
 dynamically.
 
 What I'd suggest is to create a short series that:
 
 1. adds PRM hwmod data for OMAP2430+ platforms

How should this be done? It believe all the data in the hwmods should be
autogenerated somehow... should I just make a temporary hack patch for
one platform that could be then autogenerated by someone for all omap
platforms?


 
 2. adds a basic PRM device driver skeleton in a directory such as
drivers/power -- (I'm not convinced that this is the right place,
in the end; but seems like a good place to start)
 
 3. creates the chained interrupt handler in the PRM device driver,
and removes the old PRCM interrupt handler from pm34xx.c 
 
 ...
 
 A few other relatively minor comments:
 
 - Probably omap_prcm_irq_cleanup() shouldn't be called from pm34xx.c,
   since other code outside of pm34xx.c might wish to use the PRCM
   interrupt, even in the (admittedly unlikely) circumstance that some
   of the code in pm34xx.c fails?
 
 - It would be good to document struct omap_prcm_irq via KernelDoc,
   rather than inline comments
   (Documentation/kernel-doc-nano-HOWTO.txt).  It would be ideal if the
   patch's function documentation followed the same standard.
 
 
 - Paul



Texas Instruments Oy, Tekniikantie 12, 02150 Espoo. Y-tunnus: 0115040-6. 
Kotipaikka: Helsinki
 

--
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: [PATCHv6 01/11] omap: prcm: switch to a chained IRQ handler mechanism

2011-08-26 Thread Paul Walmsley
Hello Tero,

a few comments on this patch:

On Mon, 25 Jul 2011, Tero Kristo wrote:

 Introduce a chained interrupt handler mechanism for the PRCM
 interrupt, so that individual PRCM event can cleanly be handled by
 handlers in separate drivers. We do this by introducing PRCM event
 names, which are then matched to the particular PRCM interrupt bit
 depending on the specific OMAP SoC being used.
 
 arch/arm/mach-omap2/prcm.c implements the chained interrupt mechanism
 itself, with SoC specific support / init structure defined in
 arch/arm/mach-omap2/prm2xxx_3xxx.c and arch/arm/mach-omap2/prm4xxx.c
 respectively. At initialization time, the set of PRCM events is filtered
 against the SoC on which we are running, keeping only the ones that are
 actually useful. All the logic is written to be generic with regard to
 OMAP3/OMAP4, even though OMAP3 has single PRCM event registers and OMAP4
 has two PRCM event registers.

Looking over this patch, it seems that this functionality should be
part of a PRM device driver.  That would allow the separation of the
SoC-specific data from the code, so there wouldn't be a need to embed
the OMAP_PRCM_IRQ data in the driver code.  Rather, that data could go
into the dev_attr data for the PRM hwmod.  That avoids putting
SoC-specific data in driver code, allows the removal of
omap[34]_prcm_irq_setup(), and should also remove the dependency on
omap_chip.

Similarly, OMAP_PRCM_MAX_NR_PENDING_REG and OMAP_PRCM_NR_IRQS should
be defined somewhere SoC-specific.  I'd suggest defining those in the
hwmod dev_attr data.  That way that file won't need to be patched if
those constants need change in the future.  Unfortunately, doing this
in a clean way will probably mean that the variables that are
allocated via these constants will need to be allocated and freed
dynamically.

What I'd suggest is to create a short series that:

1. adds PRM hwmod data for OMAP2430+ platforms

2. adds a basic PRM device driver skeleton in a directory such as
   drivers/power -- (I'm not convinced that this is the right place,
   in the end; but seems like a good place to start)

3. creates the chained interrupt handler in the PRM device driver,
   and removes the old PRCM interrupt handler from pm34xx.c 

...

A few other relatively minor comments:

- Probably omap_prcm_irq_cleanup() shouldn't be called from pm34xx.c,
  since other code outside of pm34xx.c might wish to use the PRCM
  interrupt, even in the (admittedly unlikely) circumstance that some
  of the code in pm34xx.c fails?

- It would be good to document struct omap_prcm_irq via KernelDoc,
  rather than inline comments
  (Documentation/kernel-doc-nano-HOWTO.txt).  It would be ideal if the
  patch's function documentation followed the same standard.


- 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: [PATCHv6 01/11] omap: prcm: switch to a chained IRQ handler mechanism

2011-07-26 Thread Tero Kristo
On Mon, 2011-07-25 at 19:03 +0200, Balbi, Felipe wrote:
 Hi,
 
 On Mon, Jul 25, 2011 at 07:36:01PM +0300, Tero Kristo wrote:
  Introduce a chained interrupt handler mechanism for the PRCM
  interrupt, so that individual PRCM event can cleanly be handled by
  handlers in separate drivers. We do this by introducing PRCM event
 
 which drivers ? Are those somehow children of the PRCM device ??
 If that's the case, you shouldn't need to match against names as you
 could allocate a platform_device for your children and pass in your
 resources with correct IRQ numbers.

I am not quite sure what you mean by children in this case. There are a
couple of devices that might be interested in using these, e.g. SR and
ABB come to my mind. They are closely related to PRCM yes.

 
  names, which are then matched to the particular PRCM interrupt bit
  depending on the specific OMAP SoC being used.
  
  arch/arm/mach-omap2/prcm.c implements the chained interrupt mechanism
  itself, with SoC specific support / init structure defined in
  arch/arm/mach-omap2/prm2xxx_3xxx.c and arch/arm/mach-omap2/prm4xxx.c
  respectively. At initialization time, the set of PRCM events is filtered
  against the SoC on which we are running, keeping only the ones that are
  actually useful. All the logic is written to be generic with regard to
  OMAP3/OMAP4, even though OMAP3 has single PRCM event registers and OMAP4
  has two PRCM event registers.
 
 Then if OMAP5 has 3, OMAP6 4 and OMAP7 5, OMAP3 will also have an array
 of 5 PRCM events even though it only needs one, another argument for
 dynamic allocation ?
 
  ---
 
 [snip]
 
  @@ -246,64 +249,7 @@ static int _prcm_int_handle_wakeup(void)
  c += prcm_clear_mod_irqs(OMAP3430ES2_USBHOST_MOD, 1);
  }
   
  -   return c;
  -}
  -
  -/*
  - * PRCM Interrupt Handler
  - *
  - * The PRM_IRQSTATUS_MPU register indicates if there are any pending
  - * interrupts from the PRCM for the MPU. These bits must be cleared in
  - * order to clear the PRCM interrupt. The PRCM interrupt handler is
  - * implemented to simply clear the PRM_IRQSTATUS_MPU in order to clear
  - * the PRCM interrupt. Please note that bit 0 of the PRM_IRQSTATUS_MPU
  - * register indicates that a wake-up event is pending for the MPU and
  - * this bit can only be cleared if the all the wake-up events latched
  - * in the various PM_WKST_x registers have been cleared. The interrupt
  - * handler is implemented using a do-while loop so that if a wake-up
  - * event occurred during the processing of the prcm interrupt handler
  - * (setting a bit in the corresponding PM_WKST_x register and thus
  - * preventing us from clearing bit 0 of the PRM_IRQSTATUS_MPU register)
  - * this would be handled.
  - */
  -static irqreturn_t prcm_interrupt_handler (int irq, void *dev_id)
  -{
  -   u32 irqenable_mpu, irqstatus_mpu;
  -   int c = 0;
  -
  -   irqenable_mpu = omap2_prm_read_mod_reg(OCP_MOD,
  -OMAP3_PRM_IRQENABLE_MPU_OFFSET);
  -   irqstatus_mpu = omap2_prm_read_mod_reg(OCP_MOD,
  -OMAP3_PRM_IRQSTATUS_MPU_OFFSET);
  -   irqstatus_mpu = irqenable_mpu;
  -
  -   do {
  -   if (irqstatus_mpu  (OMAP3430_WKUP_ST_MASK |
  -OMAP3430_IO_ST_MASK)) {
  -   c = _prcm_int_handle_wakeup();
  -
  -   /*
  -* Is the MPU PRCM interrupt handler racing with the
  -* IVA2 PRCM interrupt handler ?
  -*/
  -   WARN(c == 0, prcm: WARNING: PRCM indicated MPU wakeup 
  -but no wakeup sources are marked\n);
  -   } else {
  -   /* XXX we need to expand our PRCM interrupt handler */
  -   WARN(1, prcm: WARNING: PRCM interrupt received, but 
  -no code to handle it (%08x)\n, irqstatus_mpu);
  -   }
  -
  -   omap2_prm_write_mod_reg(irqstatus_mpu, OCP_MOD,
  -   OMAP3_PRM_IRQSTATUS_MPU_OFFSET);
  -
  -   irqstatus_mpu = omap2_prm_read_mod_reg(OCP_MOD,
  -   OMAP3_PRM_IRQSTATUS_MPU_OFFSET);
  -   irqstatus_mpu = irqenable_mpu;
  -
  -   } while (irqstatus_mpu);
  -
  -   return IRQ_HANDLED;
  +   return c ? IRQ_HANDLED : IRQ_NONE;
   }
   
   static void omap34xx_save_context(u32 *save)
  @@ -875,20 +821,35 @@ static int __init omap3_pm_init(void)
  /* XXX prcm_setup_regs needs to be before enabling hw
   * supervised mode for powerdomains */
  prcm_setup_regs();
  +   ret = omap3_prcm_irq_init();
  +   if (ret) {
  +   pr_err(omap_prcm_irq_init() failed with %d\n, ret);
  +   goto err_prcm_irq_init;
  +   }
  +
  +   prcm_wkup_irq = omap_prcm_event_to_irq(wkup);
  +   prcm_io_irq = omap_prcm_event_to_irq(io);
  +
  +   ret = request_irq(prcm_wkup_irq, _prcm_int_handle_wakeup,
  +   IRQF_NO_SUSPEND, 

Re: [PATCHv6 01/11] omap: prcm: switch to a chained IRQ handler mechanism

2011-07-26 Thread Felipe Balbi
Hi,

On Tue, Jul 26, 2011 at 01:33:35PM +0300, Tero Kristo wrote:
   + while (1) {
   + unsigned int virtirq;
   +
   + chip-irq_ack(desc-irq_data);
   +
   + memset(pending, 0, sizeof(pending));
   + irq_setup-pending_events(pending);
   +
   + /* No bit set, then all IRQs are handled */
   + if (find_first_bit(pending, OMAP_PRCM_NR_IRQS)
   + = OMAP_PRCM_NR_IRQS) {
   + chip-irq_unmask(desc-irq_data);
   + break;
   + }
   +
   + /*
   +  * Loop on all currently pending irqs so that new irqs
   +  * cannot starve previously pending irqs
   +  */
   + for_each_set_bit(virtirq, pending, OMAP_PRCM_NR_IRQS)
   + generic_handle_irq(irq_setup-base_irq + virtirq);
   +
   + chip-irq_unmask(desc-irq_data);
  
  can't the IRQ subsystem handle this for you ? I was expecting it would
  call irq_ack() and irq_unmask() automatically and you wouldn't have to
  do it yourself. Maybe Thomas can clear this out ? Thomas, should we call
  -irq_ack() -irq_mask ourselves here ?
 
 These are needed, as we are using a handle and a level interrupt. If you
 leave out the ack, we will return to the handler immediately as nobody
 else acks it. If you leave out the unmask, we will only ever get 1
 interrupt and no more.

but that's the thing, I guess IRQ subsystem can handle this
automatically. At least from what I remember. That's how I converted the
retu driver, for instance, and I checked that IRQ subsystem calls
-irq_ack() and -irq_mask/unmask() for me...

Maybe the irq_gc stuff is different, dunno.

  if you make this a platform_driver, there would be no need for this
  trickery. You could pass this as driver data. Something like:
  
  
  struct omap_prcm_irq_setup omap3_prcm_irq_setup = {
  .ack= (u32)OMAP3430_PRM_IRQSTATUS_MPU,
  .mask   = (u32)OMAP3430_PRM_IRQENABLE_MPU,
  .pending_events = omap3_prm_pending_events,
  .irq= INT_34XX_PRCM_MPU_IRQ,
  };
  
  struct const struct platform_device_id prcm_id_table[] __devinitconst =
  {
  {
  .name   = omap3-prcm,
  .driver_data= omap3_prcm_irq_setup,
  },
  {
  .name   = omap4-prcm,
  .driver_data= omap4_prcm_irq_setup,
  },
  };
  MODULE_DEVICE_TABLE(platform, prcm_id_table);
  
  static struct platform_driver prcm_driver = {
  .probe  = prcm_probe,
  .remove = __devexit_p(prcm_remove),
  .driver = {
  .name   = prcm,
  .pm = DEV_PM_OPS,
  },
  .id_table   = prcm_id_table,
  };
  
  or something similar. Then on probe you can make a copy of irq_setup to
  your driver's context structure, or only use temporarily to initialize
  some fields and so on...
  
 
 Hmm, this might be useful, however you would still need a way to
 register the driver from somewhere, and you would essentially end up
 with omap_chip version check somewhere. The reason for all this trickery
 is that the omap chip version detection is heavily frowned upon right
 now... it would be much cleaner if there was an accepted way of doing
 this already in place.

wouldn't hwmod be able to handle this for you ? I mean, it's just the
driver name that has to change, rather than exporting a few functions.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCHv6 01/11] omap: prcm: switch to a chained IRQ handler mechanism

2011-07-25 Thread Felipe Balbi
Hi,

On Mon, Jul 25, 2011 at 07:36:01PM +0300, Tero Kristo wrote:
 Introduce a chained interrupt handler mechanism for the PRCM
 interrupt, so that individual PRCM event can cleanly be handled by
 handlers in separate drivers. We do this by introducing PRCM event

which drivers ? Are those somehow children of the PRCM device ??
If that's the case, you shouldn't need to match against names as you
could allocate a platform_device for your children and pass in your
resources with correct IRQ numbers.

 names, which are then matched to the particular PRCM interrupt bit
 depending on the specific OMAP SoC being used.
 
 arch/arm/mach-omap2/prcm.c implements the chained interrupt mechanism
 itself, with SoC specific support / init structure defined in
 arch/arm/mach-omap2/prm2xxx_3xxx.c and arch/arm/mach-omap2/prm4xxx.c
 respectively. At initialization time, the set of PRCM events is filtered
 against the SoC on which we are running, keeping only the ones that are
 actually useful. All the logic is written to be generic with regard to
 OMAP3/OMAP4, even though OMAP3 has single PRCM event registers and OMAP4
 has two PRCM event registers.

Then if OMAP5 has 3, OMAP6 4 and OMAP7 5, OMAP3 will also have an array
of 5 PRCM events even though it only needs one, another argument for
dynamic allocation ?

 ---

[snip]

 @@ -246,64 +249,7 @@ static int _prcm_int_handle_wakeup(void)
   c += prcm_clear_mod_irqs(OMAP3430ES2_USBHOST_MOD, 1);
   }
  
 - return c;
 -}
 -
 -/*
 - * PRCM Interrupt Handler
 - *
 - * The PRM_IRQSTATUS_MPU register indicates if there are any pending
 - * interrupts from the PRCM for the MPU. These bits must be cleared in
 - * order to clear the PRCM interrupt. The PRCM interrupt handler is
 - * implemented to simply clear the PRM_IRQSTATUS_MPU in order to clear
 - * the PRCM interrupt. Please note that bit 0 of the PRM_IRQSTATUS_MPU
 - * register indicates that a wake-up event is pending for the MPU and
 - * this bit can only be cleared if the all the wake-up events latched
 - * in the various PM_WKST_x registers have been cleared. The interrupt
 - * handler is implemented using a do-while loop so that if a wake-up
 - * event occurred during the processing of the prcm interrupt handler
 - * (setting a bit in the corresponding PM_WKST_x register and thus
 - * preventing us from clearing bit 0 of the PRM_IRQSTATUS_MPU register)
 - * this would be handled.
 - */
 -static irqreturn_t prcm_interrupt_handler (int irq, void *dev_id)
 -{
 - u32 irqenable_mpu, irqstatus_mpu;
 - int c = 0;
 -
 - irqenable_mpu = omap2_prm_read_mod_reg(OCP_MOD,
 -  OMAP3_PRM_IRQENABLE_MPU_OFFSET);
 - irqstatus_mpu = omap2_prm_read_mod_reg(OCP_MOD,
 -  OMAP3_PRM_IRQSTATUS_MPU_OFFSET);
 - irqstatus_mpu = irqenable_mpu;
 -
 - do {
 - if (irqstatus_mpu  (OMAP3430_WKUP_ST_MASK |
 -  OMAP3430_IO_ST_MASK)) {
 - c = _prcm_int_handle_wakeup();
 -
 - /*
 -  * Is the MPU PRCM interrupt handler racing with the
 -  * IVA2 PRCM interrupt handler ?
 -  */
 - WARN(c == 0, prcm: WARNING: PRCM indicated MPU wakeup 
 -  but no wakeup sources are marked\n);
 - } else {
 - /* XXX we need to expand our PRCM interrupt handler */
 - WARN(1, prcm: WARNING: PRCM interrupt received, but 
 -  no code to handle it (%08x)\n, irqstatus_mpu);
 - }
 -
 - omap2_prm_write_mod_reg(irqstatus_mpu, OCP_MOD,
 - OMAP3_PRM_IRQSTATUS_MPU_OFFSET);
 -
 - irqstatus_mpu = omap2_prm_read_mod_reg(OCP_MOD,
 - OMAP3_PRM_IRQSTATUS_MPU_OFFSET);
 - irqstatus_mpu = irqenable_mpu;
 -
 - } while (irqstatus_mpu);
 -
 - return IRQ_HANDLED;
 + return c ? IRQ_HANDLED : IRQ_NONE;
  }
  
  static void omap34xx_save_context(u32 *save)
 @@ -875,20 +821,35 @@ static int __init omap3_pm_init(void)
   /* XXX prcm_setup_regs needs to be before enabling hw
* supervised mode for powerdomains */
   prcm_setup_regs();
 + ret = omap3_prcm_irq_init();
 + if (ret) {
 + pr_err(omap_prcm_irq_init() failed with %d\n, ret);
 + goto err_prcm_irq_init;
 + }
 +
 + prcm_wkup_irq = omap_prcm_event_to_irq(wkup);
 + prcm_io_irq = omap_prcm_event_to_irq(io);
 +
 + ret = request_irq(prcm_wkup_irq, _prcm_int_handle_wakeup,
 + IRQF_NO_SUSPEND, prcm_wkup, NULL);
  
 - ret = request_irq(INT_34XX_PRCM_MPU_IRQ,
 -   (irq_handler_t)prcm_interrupt_handler,
 -   IRQF_DISABLED, prcm, NULL);
   if (ret) {
 - printk(KERN_ERR request_irq failed to register for 0x%x\n,
 -