Re: [PATCHv3 2/6] PRCM: Add support for PAD wakeup interrupts

2011-06-24 Thread Kevin Hilman
Govindraj  writes:

> On Wed, Jun 22, 2011 at 10:12 PM, Tero Kristo  wrote:
>> PRCM interrupt handler will now parse registered pads to see whether there
>> is an active wakeup event. If this is the case, the corresponding interrupt
>> will be triggered. This can be used for example with UART driver to register
>> PAD wakeup event for the UART RX pin, and when this happens, UART interrupt
>> will be triggered.
>>
>> Signed-off-by: Tero Kristo 

[...]

>> @@ -153,6 +184,25 @@ int omap_prcm_event_to_irq(const char *name)
>>  }
>>
>>  /*
>> + * Register interrupt handler for a given pad. When the PRCM interrupt
>> + * handler detects wakeupevent on the corresponding pad, the IRQ will
>> + * be triggered.
>> + */
>> +int omap_prcm_register_pad_irq(u32 pad, unsigned int irq)
>
> I tested this v3 series seems to work fine.
>
> Minor comments.
>
> Can we make the prcm_register interface params with mux_name
> or
> even omap_hmwod and with hwmod api's check
> wake-up event through a mux api as here [1]
> rather than passing pad offset.

Hmm, good point.

In fact, each omap_hwmod already knows about the pads and the IRQ for
each IP block, so another API to create that mapping isn't really
needed.

Instead, what we need is simply a way register an omap_hwmod with the
PRCM layer to indicate that it should trigger the IRQ whenever a wakup
is detected.   Then, when a wakeup event is detected, it could call
a new omap_hwmod_mux_* function which would use the IRQ in the hwmod and
call generic_handle_irq().

I think adding to Govindraj's patch below[1] with a new API to handle
the irq: omap_hwmod_mux_handle_irq() or something similar would be the
right way to handle this.

> since I planning to test uart-runtime changes with irq_chaining,
> pad offsets are no more available with uart-runtime
> and uses hmwod_mux api's.
>
> --
> Thanks,
> Govindraj.R
>
> [1] https://patchwork.kernel.org/patch/773932/

Kevin
--
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: [PATCHv3 2/6] PRCM: Add support for PAD wakeup interrupts

2011-06-24 Thread Kevin Hilman
Tero Kristo  writes:

> PRCM interrupt handler will now parse registered pads to see whether there
> is an active wakeup event. If this is the case, the corresponding interrupt
> will be triggered. This can be used for example with UART driver to register
> PAD wakeup event for the UART RX pin, and when this happens, UART interrupt
> will be triggered.
>
> Signed-off-by: Tero Kristo 
> ---
>  arch/arm/mach-omap2/prcm.c |   50 
> 
>  arch/arm/plat-omap/include/plat/prcm.h |1 +
>  2 files changed, 51 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/prcm.c b/arch/arm/mach-omap2/prcm.c
> index 362c59c..cc92064 100644
> --- a/arch/arm/mach-omap2/prcm.c
> +++ b/arch/arm/mach-omap2/prcm.c
> @@ -53,6 +53,15 @@ static struct omap_prcm_irq *omap_prcm_irqs;
>  /* Number of entries in omap_prcm_irqs */
>  static int omap_prcm_irqs_nr;
>  
> +/* PAD handlers list */
> +struct pad_def {
> + u32 pad;
> + unsigned int irq;
> + struct list_head node;
> +};
> +
> +static LIST_HEAD(pad_handler_list);
> +
>  /* Pointers to either OMAP3 or OMAP4 specific functions */
>  static void (*omap_prcm_mask_event)(unsigned event);
>  static void (*omap_prcm_unmask_event)(unsigned event);
> @@ -84,6 +93,25 @@ static struct irq_chip prcm_irq_chip = {
>   .irq_unmask = prcm_irq_unmask,
>  };
>  
> +
> +/*
> + * Handler for PAD irqs, called from PRCM interrupt handler
> + */
> +static void omap_prcm_handle_pad_irqs(void)
> +{
> + struct pad_def *def;
> + u16 val = 0;

minor: insert blank line between variables and code

> + list_for_each_entry(def, &pad_handler_list, node) {
> + /* Read padconf value based on cpu type */
> + if (cpu_is_omap34xx())
> + val = omap_ctrl_readw(def->pad);

We need to avoid cpu_is_* checks in driver code.

What we'll need is another control-module API for reading a padconf, and
the SoC-specific implementation can be managed by the control module code.

> + /* if pad wakeupevent is active, fire registered IRQ */
> + if (val & OMAP3_PADCONF_WAKEUPEVENT0)

Does this need to be masked with the wakeup enable?  IOW, is it possible
to find an pad with a masked wakeup event?  Might be worth double
checking the TRM on that one, but masking with the enable would not hurt
here.

> + generic_handle_irq(def->irq);

Nice, I wish I thought of that!  I was trying to figure out how to SW
trigger a hard IRQ, but this is much better and doesn't rely on any
hardware feature.   Sometimes the simplest solution is the best. :)

> + }
> +}
> +
>  /*
>   * PRCM Interrupt Handler
>   *
> @@ -106,6 +134,9 @@ static void prcm_irq_handler(unsigned int irq, struct 
> irq_desc *desc)
>   unsigned long pending[OMAP_PRCM_MAX_NR_PENDING_REG];
>   struct irq_chip *chip = irq_desc_get_chip(desc);
>  
> + /* Handle PAD events first, we don't want to ack them before parse */
> + omap_prcm_handle_pad_irqs();
> +

Handling here is fine, but it's not terribly clear on first read where
the events are eventually cleared.  

I see now that they are cleared upon entry into the idle path, but I
think it would help readability if they were cleared after this handler
runs.

>   /*
>* Loop until all pending irqs are handled, since
>* generic_handle_irq(), called by prcm_irq_handle_virtirqs()
> @@ -153,6 +184,25 @@ int omap_prcm_event_to_irq(const char *name)
>  }
>  
>  /*
> + * Register interrupt handler for a given pad. When the PRCM interrupt
> + * handler detects wakeupevent on the corresponding pad, the IRQ will
> + * be triggered.

This comment needs a minor change: technically, it's not the IRQ that is
triggered but the ISR for that IRQ will be called.

The same change should be made in the changelog.

> + */
> +int omap_prcm_register_pad_irq(u32 pad, unsigned int irq)
> +{
> + struct pad_def *def;
> +
> + def = kmalloc(sizeof(struct pad_def), GFP_ATOMIC);
> + if (!def)
> + return -ENOMEM;
> +
> + def->pad = pad;
> + def->irq = irq;
> + list_add(&def->node, &pad_handler_list);
> + return 0;
> +}

This wants an unregister counterpart.

> +/*
>   * Prepare the array of PRCM events corresponding to the current SoC,
>   * and set-up the chained interrupt handler mechanism.
>   */
> diff --git a/arch/arm/plat-omap/include/plat/prcm.h 
> b/arch/arm/plat-omap/include/plat/prcm.h
> index 578..789eb17 100644
> --- a/arch/arm/plat-omap/include/plat/prcm.h
> +++ b/arch/arm/plat-omap/include/plat/prcm.h
> @@ -72,6 +72,7 @@ void omap4_prcm_pending_events(unsigned long *pending);
>  int omap_prcm_event_to_irq(const char *name);
>  int omap_prcm_irq_init(void);
>  void omap_prcm_irq_cleanup(void);
> +int omap_prcm_register_pad_irq(u32 pad, unsigned int irq);
>  u32 omap_prcm_get_reset_sources(void);
>  int omap2_cm_wait_idlest(void __iomem *reg, u32 mask, u8 idlest,
>const char *

Re: [PATCHv3 2/6] PRCM: Add support for PAD wakeup interrupts

2011-06-23 Thread Govindraj
On Wed, Jun 22, 2011 at 10:12 PM, Tero Kristo  wrote:
> PRCM interrupt handler will now parse registered pads to see whether there
> is an active wakeup event. If this is the case, the corresponding interrupt
> will be triggered. This can be used for example with UART driver to register
> PAD wakeup event for the UART RX pin, and when this happens, UART interrupt
> will be triggered.
>
> Signed-off-by: Tero Kristo 
> ---
>  arch/arm/mach-omap2/prcm.c             |   50 
> 
>  arch/arm/plat-omap/include/plat/prcm.h |    1 +
>  2 files changed, 51 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/prcm.c b/arch/arm/mach-omap2/prcm.c
> index 362c59c..cc92064 100644
> --- a/arch/arm/mach-omap2/prcm.c
> +++ b/arch/arm/mach-omap2/prcm.c
> @@ -53,6 +53,15 @@ static struct omap_prcm_irq *omap_prcm_irqs;
>  /* Number of entries in omap_prcm_irqs */
>  static int omap_prcm_irqs_nr;
>
> +/* PAD handlers list */
> +struct pad_def {
> +       u32 pad;
> +       unsigned int irq;
> +       struct list_head node;
> +};
> +
> +static LIST_HEAD(pad_handler_list);
> +
>  /* Pointers to either OMAP3 or OMAP4 specific functions */
>  static void (*omap_prcm_mask_event)(unsigned event);
>  static void (*omap_prcm_unmask_event)(unsigned event);
> @@ -84,6 +93,25 @@ static struct irq_chip prcm_irq_chip = {
>        .irq_unmask     = prcm_irq_unmask,
>  };
>
> +
> +/*
> + * Handler for PAD irqs, called from PRCM interrupt handler
> + */
> +static void omap_prcm_handle_pad_irqs(void)
> +{
> +       struct pad_def *def;
> +       u16 val = 0;
> +       list_for_each_entry(def, &pad_handler_list, node) {
> +               /* Read padconf value based on cpu type */
> +               if (cpu_is_omap34xx())
> +                       val = omap_ctrl_readw(def->pad);
> +
> +               /* if pad wakeupevent is active, fire registered IRQ */
> +               if (val & OMAP3_PADCONF_WAKEUPEVENT0)
> +                       generic_handle_irq(def->irq);
> +       }
> +}
> +
>  /*
>  * PRCM Interrupt Handler
>  *
> @@ -106,6 +134,9 @@ static void prcm_irq_handler(unsigned int irq, struct 
> irq_desc *desc)
>        unsigned long pending[OMAP_PRCM_MAX_NR_PENDING_REG];
>        struct irq_chip *chip = irq_desc_get_chip(desc);
>
> +       /* Handle PAD events first, we don't want to ack them before parse */
> +       omap_prcm_handle_pad_irqs();
> +
>        /*
>         * Loop until all pending irqs are handled, since
>         * generic_handle_irq(), called by prcm_irq_handle_virtirqs()
> @@ -153,6 +184,25 @@ int omap_prcm_event_to_irq(const char *name)
>  }
>
>  /*
> + * Register interrupt handler for a given pad. When the PRCM interrupt
> + * handler detects wakeupevent on the corresponding pad, the IRQ will
> + * be triggered.
> + */
> +int omap_prcm_register_pad_irq(u32 pad, unsigned int irq)

I tested this v3 series seems to work fine.

Minor comments.

Can we make the prcm_register interface params with mux_name
or
even omap_hmwod and with hwmod api's check
wake-up event through a mux api as here [1]
rather than passing pad offset.

since I planning to test uart-runtime changes with irq_chaining,
pad offsets are no more available with uart-runtime
and uses hmwod_mux api's.

--
Thanks,
Govindraj.R

[1] https://patchwork.kernel.org/patch/773932/

> +{
> +       struct pad_def *def;
> +
> +       def = kmalloc(sizeof(struct pad_def), GFP_ATOMIC);
> +       if (!def)
> +               return -ENOMEM;
> +
> +       def->pad = pad;
> +       def->irq = irq;
> +       list_add(&def->node, &pad_handler_list);
> +       return 0;
> +}
> +
> +/*
>  * Prepare the array of PRCM events corresponding to the current SoC,
>  * and set-up the chained interrupt handler mechanism.
>  */
> diff --git a/arch/arm/plat-omap/include/plat/prcm.h 
> b/arch/arm/plat-omap/include/plat/prcm.h
> index 578..789eb17 100644
> --- a/arch/arm/plat-omap/include/plat/prcm.h
> +++ b/arch/arm/plat-omap/include/plat/prcm.h
> @@ -72,6 +72,7 @@ void omap4_prcm_pending_events(unsigned long *pending);
>  int omap_prcm_event_to_irq(const char *name);
>  int omap_prcm_irq_init(void);
>  void omap_prcm_irq_cleanup(void);
> +int omap_prcm_register_pad_irq(u32 pad, unsigned int irq);
>  u32 omap_prcm_get_reset_sources(void);
>  int omap2_cm_wait_idlest(void __iomem *reg, u32 mask, u8 idlest,
>                         const char *name);
> --
> 1.7.4.1
>
>
> 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
>
--
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