Re: [PATCHv8 06/16] power: omap-prm: added chain interrupt handler

2011-09-21 Thread Paul Walmsley
Hi

some comments (also see the comments for the 1/16 patch, some of which 
apply to this patch)

On Fri, 16 Sep 2011, Tero Kristo wrote:

 diff --git a/drivers/power/omap_prm.c b/drivers/power/omap_prm.c
 index dfc0920..880748a 100644
 --- a/drivers/power/omap_prm.c
 +++ b/drivers/power/omap_prm.c

...

 +static int __init omap_prm_probe(struct platform_device *pdev)
 +{
 + struct omap_hwmod *oh;

This is a sign that something is wrong here.  Device driver code shouldn't 
have any hwmod dependencies.  The driver should just use whatever data it 
gets from the struct platform_device / platform_data.  

Code in arch/arm/mach-omap2 can use hwmod data to populate platform_data, 
if needed.  But it doesn't even look like that would be needed in this 
case.  The PRM revision register resides inside the PRM IP block, so it's 
valid for the PRM driver to read it directly.

Anyway, this PRM revision code is presumably mooted anyway if different 
drivers are probed for OMAP2430 vs. OMAP4-style PRMs.

 + int rev;
 +
 + oh = omap_hwmod_lookup(prm);
 +
 + if (!oh) {
 + pr_err(prm hwmod not found\n);
 + return -ENODEV;
 + }
 +
 + prm_dev.base = omap_hwmod_get_mpu_rt_va(oh);
 +
 + rev = prm_read_reg(oh-class-sysc-rev_offs);
 +
 + switch (rev) {
 + case OMAP3_PRM_REVISION:
 + prm_dev.irq_setup = omap3_prcm_irq_setup;
 + prm_dev.revision = PRM_OMAP3;
 + break;
 + case OMAP4_PRM_REVISION:
 + prm_dev.irq_setup = omap4_prcm_irq_setup;
 + prm_dev.revision = PRM_OMAP4;
 + break;
 + default:
 + pr_err(unknown PRM revision: %08x\n, rev);
 + return -ENODEV;
 + }
 +
 + prm_dev.irq = oh-mpu_irqs[0].irq;
 +
 + omap_prcm_irq_init();
 +
 + 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


Re: [PATCHv8 06/16] power: omap-prm: added chain interrupt handler

2011-09-21 Thread Paul Walmsley
Hi

a few more comments here:

On Fri, 16 Sep 2011, Tero Kristo wrote:

 diff --git a/drivers/power/omap_prm.c b/drivers/power/omap_prm.c
 index dfc0920..880748a 100644
 --- a/drivers/power/omap_prm.c
 +++ b/drivers/power/omap_prm.c
  #define DRIVER_NAME omap-prm
 +#define OMAP_PRCM_MAX_NR_PENDING_REG 2
 +
 +struct omap_prcm_irq_setup {
 + u32 ack;
 + u32 mask;
 + int nr_regs;
 +};
  
  struct omap_prm_device {
 - struct platform_device  pdev;
 + struct platform_device  pdev;
 + const struct omap_prcm_irq_setup*irq_setup;
 + struct irq_chip_generic **irq_chips;
 + int suspended;
 + u32 *saved_ena;
 + u32 *priority_mask;
 + int base_irq;
 + int irq;
 + void __iomem*base;
 + int revision;
 +};
 +
 +#define OMAP3_PRM_REVISION   0x10
 +#define OMAP4_PRM_REVISION   0x4100
 +
 +#define PRM_OMAP30x1
 +#define PRM_OMAP40x2

These should no longer be needed, as far as I can see.

 +#define OMAP3_PRM_IRQSTATUS_OFFSET   0x818
 +#define OMAP3_PRM_IRQENABLE_OFFSET   0x81c
 +#define OMAP4_PRM_IRQSTATUS_OFFSET   0x10
 +#define OMAP4_PRM_IRQENABLE_OFFSET   0x18

It probably would be best to put these in header files, and also just to 
cut and paste the needed macros from the arch/arm/mach-omap2/prm*.h files 
- the 44xx files are autogenerated and the 24xx/34xx files were partially 
autogenerated.

 +
 +static const struct omap_prcm_irq_setup omap3_prcm_irq_setup = {
 + .ack= OMAP3_PRM_IRQSTATUS_OFFSET,
 + .mask   = OMAP3_PRM_IRQENABLE_OFFSET,
 + .nr_regs= 1,
 +};
 +
 +static const struct omap_prcm_irq_setup omap4_prcm_irq_setup = {
 + .ack= OMAP4_PRM_IRQSTATUS_OFFSET,
 + .mask   = OMAP4_PRM_IRQENABLE_OFFSET,
 + .nr_regs= 2,
  };
  
  static struct omap_prm_device prm_dev = {
 @@ -33,20 +77,321 @@ static struct omap_prm_device prm_dev = {
   },
  };
  
 -static int __init omap_prm_probe(struct platform_device *pdev)
 +struct omap_prcm_irq {
 + const char *name;
 + unsigned int offset;
 + bool priority;
 + u32 supported_rev;
 +};
 +
 +#define OMAP_PRCM_IRQ(_name, _offset, _high_priority, _rev) {\
 + .name = _name,  \
 + .offset = _offset,  \
 + .priority = _high_priority, \
 + .supported_rev = _rev   \
 + }
 +
 +static struct omap_prcm_irq omap_prcm_irqs[] = {
 + OMAP_PRCM_IRQ(wkup,   0,  0,  PRM_OMAP3),
 + OMAP_PRCM_IRQ(io, 9,  1,  PRM_OMAP3 | PRM_OMAP4),
 +};
 +
 +static inline u32 prm_read_reg(int offset)
 +{
 + return __raw_readl(prm_dev.base + offset);
 +}
 +
 +static inline void prm_write_reg(u32 value, int offset)
 +{
 + __raw_writel(value, prm_dev.base + offset);
 +}

What to do with these functions will depend on how you split the files up. 
Based on a naïve look, I'd suggest putting copies of prcm_irq_handler() 
into each of the omap*_prm.c files, calling local static functions for 
read_pending_events() and save_and_disable_irqenable_bits(), and then 
calling into common code in omap_prm_common.c for the rest of the 
function.  Looks like omap_prm_complete() would also need 
PRM-variant-specific copies.  I guess most of the rest could be common 
code?

 +
 +static void prm_pending_events(unsigned long *events)
 +{
 + u32 ena, st;
 + int i;
 +
 + memset(events, 0, prm_dev.irq_setup-nr_regs * 4);
 +
 + for (i = 0; i  prm_dev.irq_setup-nr_regs; i++) {
 + ena = prm_read_reg(prm_dev.irq_setup-mask + i * 4);
 + st = prm_read_reg(prm_dev.irq_setup-ack + i * 4);
 + events[i] = ena  st;
 + }
 +}
 +
 +static void prm_events_filter_priority(unsigned long *events,
 + unsigned long *priority_events)
 +{
 + int i;
 +
 + for (i = 0; i  prm_dev.irq_setup-nr_regs; i++) {
 + priority_events[i] = events[i]  prm_dev.priority_mask[i];
 + events[i] ^= priority_events[i];
 + }
 +}
 +
 +/*
 + * 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
 + * 

Re: [PATCHv8 06/16] power: omap-prm: added chain interrupt handler

2011-09-19 Thread Govindraj
Hi Tero,

On Fri, Sep 16, 2011 at 9:35 PM, Tero Kristo t-kri...@ti.com 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.

 Driver detects the version of the PRM module in use via PRM hwmod
 revision. This hwmod also defines the interrupt number for the chain
 handler. Driver itself contains SoC specific data for register offsets
 within the PRM module, and contains a list of supported PRCM interrupt
 events.

 PRCM interrupts have two priority levels, high or normal. High priority
 is needed for IO event handling, so that we can be sure that IO events
 are processed before other events. This reduces latency for IO event
 customers and also prevents incorrect ack sequence on OMAP3.

 Signed-off-by: Tero Kristo t-kri...@ti.com
 Cc: Paul Walmsley p...@pwsan.com
 Cc: Kevin Hilman khil...@ti.com
 Cc: Avinash.H.M avinas...@ti.com
 Cc: Cousson, Benoit b-cous...@ti.com
 Cc: Tony Lindgren t...@atomide.com
 Cc: Govindraj.R govindraj.r...@ti.com
 ---
  drivers/power/omap_prm.c       |  351 
 +++-
  include/linux/power/omap_prm.h |   19 +++
  2 files changed, 368 insertions(+), 2 deletions(-)
  create mode 100644 include/linux/power/omap_prm.h


[..]


 diff --git a/include/linux/power/omap_prm.h b/include/linux/power/omap_prm.h
 new file mode 100644
 index 000..9b161b5
 --- /dev/null
 +++ b/include/linux/power/omap_prm.h
 @@ -0,0 +1,19 @@
 +/*
 + * OMAP Power and Reset Management (PRM) driver
 + *
 + * Copyright (C) 2011 Texas Instruments, Inc.
 + *
 + * Author: Tero Kristo t-kri...@ti.com
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License as published by
 + * the Free Software Foundation; either version 2 of the License, or
 + * (at your option) any later version.
 + */
 +
 +#ifndef __LINUX_POWER_OMAP_PRM_H__
 +#define __LINUX_POWER_OMAP_PRM_H__
 +
 +int omap_prcm_event_to_irq(const char *name);

Minor comment,

When omap_prm driver is not enabled by default leads to compilation
break as in [1]

So we should either have omap_prm driver default enabled from Kconfig
or Have dummy omap_prcm_event_to_irq func if omap_prm is not defined.

--
Thanks,
Govindraj.R

[1]:
arch/arm/mach-omap2/built-in.o: In function `omap_mux_late_init':
/home/ubnuser/clones/mirror/linux-omap-pm/arch/arm/mach-omap2/mux.c:799:
undefined reference to `omap_prcm_event_to_irq'
arch/arm/mach-omap2/built-in.o: In function `omap3_pm_init':
/home/ubnuser/clones/mirror/linux-omap-pm/arch/arm/mach-omap2/pm34xx.c:817:
undefined reference to `omap_prcm_event_to_irq'
/home/ubnuser/clones/mirror/linux-omap-pm/arch/arm/mach-omap2/pm34xx.c:826:
undefined reference to `omap_prcm_event_to_irq'
make: *** [.tmp_vmlinux1] Error 1
--
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


[PATCHv8 06/16] power: omap-prm: added chain interrupt handler

2011-09-16 Thread Tero Kristo
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.

Driver detects the version of the PRM module in use via PRM hwmod
revision. This hwmod also defines the interrupt number for the chain
handler. Driver itself contains SoC specific data for register offsets
within the PRM module, and contains a list of supported PRCM interrupt
events.

PRCM interrupts have two priority levels, high or normal. High priority
is needed for IO event handling, so that we can be sure that IO events
are processed before other events. This reduces latency for IO event
customers and also prevents incorrect ack sequence on OMAP3.

Signed-off-by: Tero Kristo t-kri...@ti.com
Cc: Paul Walmsley p...@pwsan.com
Cc: Kevin Hilman khil...@ti.com
Cc: Avinash.H.M avinas...@ti.com
Cc: Cousson, Benoit b-cous...@ti.com
Cc: Tony Lindgren t...@atomide.com
Cc: Govindraj.R govindraj.r...@ti.com
---
 drivers/power/omap_prm.c   |  351 +++-
 include/linux/power/omap_prm.h |   19 +++
 2 files changed, 368 insertions(+), 2 deletions(-)
 create mode 100644 include/linux/power/omap_prm.h

diff --git a/drivers/power/omap_prm.c b/drivers/power/omap_prm.c
index dfc0920..880748a 100644
--- a/drivers/power/omap_prm.c
+++ b/drivers/power/omap_prm.c
@@ -15,15 +15,59 @@
 #include linux/ctype.h
 #include linux/module.h
 #include linux/io.h
+#include linux/irq.h
+#include linux/interrupt.h
 #include linux/slab.h
 #include linux/init.h
 #include linux/err.h
 #include linux/platform_device.h
 
+#include plat/irqs.h
+#include plat/omap_hwmod.h
+
 #define DRIVER_NAME omap-prm
+#define OMAP_PRCM_MAX_NR_PENDING_REG 2
+
+struct omap_prcm_irq_setup {
+   u32 ack;
+   u32 mask;
+   int nr_regs;
+};
 
 struct omap_prm_device {
-   struct platform_device  pdev;
+   struct platform_device  pdev;
+   const struct omap_prcm_irq_setup*irq_setup;
+   struct irq_chip_generic **irq_chips;
+   int suspended;
+   u32 *saved_ena;
+   u32 *priority_mask;
+   int base_irq;
+   int irq;
+   void __iomem*base;
+   int revision;
+};
+
+#define OMAP3_PRM_REVISION 0x10
+#define OMAP4_PRM_REVISION 0x4100
+
+#define PRM_OMAP3  0x1
+#define PRM_OMAP4  0x2
+
+#define OMAP3_PRM_IRQSTATUS_OFFSET 0x818
+#define OMAP3_PRM_IRQENABLE_OFFSET 0x81c
+#define OMAP4_PRM_IRQSTATUS_OFFSET 0x10
+#define OMAP4_PRM_IRQENABLE_OFFSET 0x18
+
+static const struct omap_prcm_irq_setup omap3_prcm_irq_setup = {
+   .ack= OMAP3_PRM_IRQSTATUS_OFFSET,
+   .mask   = OMAP3_PRM_IRQENABLE_OFFSET,
+   .nr_regs= 1,
+};
+
+static const struct omap_prcm_irq_setup omap4_prcm_irq_setup = {
+   .ack= OMAP4_PRM_IRQSTATUS_OFFSET,
+   .mask   = OMAP4_PRM_IRQENABLE_OFFSET,
+   .nr_regs= 2,
 };
 
 static struct omap_prm_device prm_dev = {
@@ -33,20 +77,321 @@ static struct omap_prm_device prm_dev = {
},
 };
 
-static int __init omap_prm_probe(struct platform_device *pdev)
+struct omap_prcm_irq {
+   const char *name;
+   unsigned int offset;
+   bool priority;
+   u32 supported_rev;
+};
+
+#define OMAP_PRCM_IRQ(_name, _offset, _high_priority, _rev) {  \
+   .name = _name,  \
+   .offset = _offset,  \
+   .priority = _high_priority, \
+   .supported_rev = _rev   \
+   }
+
+static struct omap_prcm_irq omap_prcm_irqs[] = {
+   OMAP_PRCM_IRQ(wkup,   0,  0,  PRM_OMAP3),
+   OMAP_PRCM_IRQ(io, 9,  1,  PRM_OMAP3 | PRM_OMAP4),
+};
+
+static inline u32 prm_read_reg(int offset)
+{
+   return __raw_readl(prm_dev.base + offset);
+}
+
+static inline void prm_write_reg(u32 value, int offset)
+{
+   __raw_writel(value, prm_dev.base + offset);
+}
+
+static void prm_pending_events(unsigned long *events)
+{
+   u32 ena, st;
+   int i;
+
+   memset(events, 0, prm_dev.irq_setup-nr_regs * 4);
+
+   for (i = 0; i  prm_dev.irq_setup-nr_regs; i++) {
+   ena = prm_read_reg(prm_dev.irq_setup-mask + i * 4);
+   st = prm_read_reg(prm_dev.irq_setup-ack + i * 4);
+   events[i] = ena  st;
+   }
+}
+
+static void prm_events_filter_priority(unsigned long *events,
+   unsigned long *priority_events)
+{
+   int i;
+
+   for (i = 0; i