[Sound-open-firmware] [PATCH v2 2/3] interrupt: enable layer 2 interrupt support
Liam Girdwood
liam.r.girdwood at linux.intel.com
Wed Sep 20 18:05:29 CEST 2017
On Wed, 2017-09-20 at 21:09 +0800, Keyon Jie wrote:
> Make it possible to support layer 2 (e.g. external level 2/3/4,
> which may include up to 16/32 sources) interrupt, which may be
> used for timer/IPC/DMAC/SSP...
>
Best to explain here the new API and layers. i.e. the client API, l2
API, arch API and how it all fits together.
> Signed-off-by: Keyon Jie <yang.jie at linux.intel.com>
> ---
> src/arch/xtensa/include/arch/interrupt.h | 4 +-
> src/include/reef/interrupt.h | 64 +++++++++---
> src/lib/Makefile.am | 3 +-
> src/lib/interrupt.c | 169 +++++++++++++++++++++++++++++++
> src/platform/baytrail/platform.c | 30 +++++-
> 5 files changed, 253 insertions(+), 17 deletions(-)
> create mode 100644 src/lib/interrupt.c
>
> diff --git a/src/arch/xtensa/include/arch/interrupt.h b/src/arch/xtensa/include/arch/interrupt.h
> index 14f3eee..dd44c01 100644
> --- a/src/arch/xtensa/include/arch/interrupt.h
> +++ b/src/arch/xtensa/include/arch/interrupt.h
> @@ -38,7 +38,7 @@
> #include <stdint.h>
> #include <stdlib.h>
>
> -static inline int arch_interrupt_register(int irq,
> +static inline int arch_interrupt_register(uint32_t irq,
> void(*handler)(void *arg), void *arg)
> {
> irq = REEF_IRQ_NUMBER(irq);
> @@ -47,7 +47,7 @@ static inline int arch_interrupt_register(int irq,
> return 0;
> }
>
> -static inline void arch_interrupt_unregister(int irq)
> +static inline void arch_interrupt_unregister(uint32_t irq)
> {
> irq = REEF_IRQ_NUMBER(irq);
> _xtos_set_interrupt_handler_arg(irq, NULL, NULL);
> diff --git a/src/include/reef/interrupt.h b/src/include/reef/interrupt.h
> index 3c78e77..7111a3c 100644
> --- a/src/include/reef/interrupt.h
> +++ b/src/include/reef/interrupt.h
> @@ -35,38 +35,76 @@
> #include <arch/interrupt.h>
> #include <reef/trace.h>
> #include <reef/debug.h>
> +#include <reef/list.h>
> +#include <reef/lock.h>
>
> #define trace_irq(__e) trace_event(TRACE_CLASS_IRQ | __e)
>
> -static inline int interrupt_register(int irq,
> +typedef void (irq2_handler)(void *arg);
irq_level2_handler
> +typedef void (irq_num_handler)(void *arg);
> +
irq_level1_handler ?
> +/* source item under L2 interrupt aggregator */
> +struct l2src_irq {
irq_level2 ?
> + uint32_t bit_id;
is this bit mask ? please comment some of the structure members that
are not obvious.
> + uint32_t enabled;
> + irq2_handler *handler;
Is this the client handler ?
> + void *handler_arg;
> + /* list in the registered irq of the same number */
> + struct list_item list;
> +};
> +
> +/* root interrupt item which has Level 2 sources */
> +struct l2root_irq {
irq_level1
> + int num;
IRQ num ? is this not now a uint32_t ?
> + irq_num_handler *handler;
level2_handler ?
> + uint32_t src_count;
is this a count of level2 users for this level1 ?
> + uint32_t enabled_count;
> + spinlock_t lock;
> + /* list in the registered l2 irq of the same number */
> + struct list_item src_list;
> +};
> +
> +int platform_interrupt_register(uint32_t irq,
> + void(*handler)(void *arg), void *arg);
> +void platform_interrupt_unregister(uint32_t irq);
> +uint32_t platform_interrupt_enable(uint32_t irq);
> +uint32_t platform_interrupt_disable(uint32_t irq);
> +void platform_interrupt_set(int irq);
> +void platform_interrupt_clear(uint32_t irq, uint32_t mask);
> +uint32_t platform_interrupt_get_enabled();
> +void platform_interrupt_mask(uint32_t irq);
> +void platform_interrupt_unmask(uint32_t irq);
> +
> +static inline int interrupt_register(uint32_t irq,
> void(*handler)(void *arg), void *arg)
> {
> - return arch_interrupt_register(irq, handler, arg);
> + return platform_interrupt_register(irq, handler, arg);
Sorry, I mean here that interrupt_register() will call
arch_interrupt_register on platforms with only 1 level of IRQs, but on
systems with more than 1 level will call the interrupt_level2_register
etc.
> }
>
> -static inline void interrupt_unregister(int irq)
> +static inline void interrupt_unregister(uint32_t irq)
> {
> - arch_interrupt_unregister(irq);
> + platform_interrupt_unregister(irq);
> }
>
> static inline uint32_t interrupt_enable(uint32_t irq)
> {
> - return arch_interrupt_enable_mask(1 << irq);
> + return platform_interrupt_enable(irq);
> }
>
> static inline uint32_t interrupt_disable(uint32_t irq)
> {
> - return arch_interrupt_disable_mask(1 <<irq);
> + return platform_interrupt_disable(irq);
> }
>
> +/* only support set/clear for root level irq */
> static inline void interrupt_set(int irq)
> {
> - arch_interrupt_set(irq);
> + return arch_interrupt_set(REEF_IRQ_NUMBER(irq));
> }
>
> static inline void interrupt_clear(int irq)
> {
> - arch_interrupt_clear(irq);
> + arch_interrupt_clear(REEF_IRQ_NUMBER(irq));
> }
>
> static inline uint32_t interrupt_global_disable(void)
> @@ -79,9 +117,11 @@ static inline void interrupt_global_enable(uint32_t flags)
> arch_interrupt_global_enable(flags);
> }
>
> -uint32_t platform_interrupt_get_enabled();
> -void platform_interrupt_clear(uint32_t irq, uint32_t mask);
> -void platform_interrupt_mask(uint32_t irq, uint32_t mask);
> -void platform_interrupt_unmask(uint32_t irq, uint32_t mask);
> +/* called by platform interrupt ops*/
> +int irq_l2_register(int irq, struct l2root_irq *l2_int,
> + void (*handler)(void *arg), void *arg);
> +void irq_l2_unregister(int irq, struct l2root_irq *l2_int);
> +uint32_t irq_l2_enable(int irq, struct l2root_irq *l2_int);
> +uint32_t irq_l2_disable(int irq, struct l2root_irq *l2_int);
>
> #endif
> diff --git a/src/lib/Makefile.am b/src/lib/Makefile.am
> index acdbd85..9923467 100644
> --- a/src/lib/Makefile.am
> +++ b/src/lib/Makefile.am
> @@ -6,7 +6,8 @@ libcore_a_SOURCES = \
> work.c \
> notifier.c \
> trace.c \
> - schedule.c
> + schedule.c \
> + interrupt.c
>
> libcore_a_CFLAGS = \
> $(ARCH_CFLAGS) \
> diff --git a/src/lib/interrupt.c b/src/lib/interrupt.c
> new file mode 100644
> index 0000000..5b08ee5
> --- /dev/null
> +++ b/src/lib/interrupt.c
> @@ -0,0 +1,169 @@
> +/*
> + * Copyright (c) 2017, Intel Corporation
> + * All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are met:
> + * * Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * * Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + * * Neither the name of the Intel Corporation nor the
> + * names of its contributors may be used to endorse or promote products
> + * derived from this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> + * POSSIBILITY OF SUCH DAMAGE.
> + *
> + * Author: Keyon Jie <yang.jie at linux.intel.com>
> + *
> + */
> +
> +#include <reef/interrupt.h>
> +#include <reef/interrupt-map.h>
> +#include <reef/alloc.h>
> +#include <arch/interrupt.h>
> +#include <platform/interrupt.h>
> +#include <stdint.h>
> +#include <stdlib.h>
> +
> +/*
> + * for those layer 2 interrupts, please call these irq_l2_xxx to
> + * register/unregister/enable/disable t
mask/unmask instead of enable/disable
> hem, the logic of their
> + * root IRQs are handled here.
level 1 IRQs instead of root IRQs
> + */
> +int irq_l2_register(int irq, struct l2root_irq *l1_int,
> + void (*handler)(void *arg), void *arg)
> +{
> + int ret = 0;
pleas put structures first in variable declarations.
> + struct l2src_irq *l2_src = NULL;
> +
> + if (l1_int == NULL)
trace error
> + return -EINVAL;
> +
> + /* Todo: may need to check if this bit id has already been registered */
> + l2_src = rzalloc(RZONE_SYS, RFLAGS_NONE, sizeof(struct l2src_irq));
> + list_init(&l2_src->list);
> + l2_src->bit_id = REEF_IRQ_BIT(irq);
> + l2_src->enabled = 0;
> + l2_src->handler = handler;
> + l2_src->handler_arg = arg;
> +
> + spin_lock(&l1_int->lock);
will this list below be iterated during an IRQ ? if so, use
spin_lock_irq()
> + if (l1_int->src_count == 0) {
> + list_init(&l1_int->src_list);
> + ret = arch_interrupt_register(l1_int->num,
> + l1_int->handler, l1_int);
do we really want to add it to the list when reg fails ?
> + }
> +
> + /* add to registered list */
> + list_item_append(&l2_src->list, &l1_int->src_list);
> + l1_int->src_count += 1;
count++;
> + spin_unlock(&l1_int->lock);
> +
> + return ret;
> +
> +}
> +
> +void irq_l2_unregister(int irq, struct l2root_irq *l1_int)
> +{
> + struct l2src_irq *l2_src = NULL;
> + struct list_item *slist, *tmp;
> +
> + spin_lock(&l1_int->lock);
ditto for IRQ context
> + /* looking for the bit id l2 src */
> + list_for_item_safe(slist, tmp, &l1_int->src_list) {
> + l2_src = container_of(slist, struct l2src_irq, list);
> + if (l2_src->bit_id != REEF_IRQ_BIT(irq))
> + continue;
> +
> + /* delete from lists */
> + list_item_del(&l2_src->list);
> + l1_int->src_count -= 1;
> +
count--
> + /* free the l2 src which unregistered */
> + if (l2_src->handler)
> + l2_src->handler = NULL;
> + rfree(l2_src);
system memory zone are never freed, as they are intended for FW lifetime
resources.
> + }
> +
> + /*
> + * unregister the root interrupt if the this l2 is
> + * the last registered one.
> + */
> + if (l1_int->src_count == 0)
> + arch_interrupt_unregister(l1_int->num);
ok, just use list_empty() here and you can remove the counts.
> +
> + spin_unlock(&l1_int->lock);
> +
> +}
> +
> +uint32_t irq_l2_enable(int irq, struct l2root_irq *l1_int)
rename to irq_level2_unmask
can we directly infer the l2 from the irq and no need to pass l1_int ?
i.e. can have an array of level1 since they are fixed on the arch, but
level2 are client facing so will be lists.
> +{
> + struct l2src_irq *l2_src = NULL;
> + struct list_item *slist;
> +
> + spin_lock(&l1_int->lock);
newline
> + /* looking for the bit id l2 src */
> + list_for_item(slist, &l1_int->src_list) {
newline
> + l2_src = container_of(slist, struct l2src_irq, list);
> + if (l2_src->bit_id != REEF_IRQ_BIT(irq))
> + continue;
> +
> + if (!l2_src->enabled) {
> + /* enable the root interrupt */
> + if (l1_int->enabled_count == 0)
> + arch_interrupt_enable_mask(1 <<
> + REEF_IRQ_NUMBER(irq));
> + /* enable the src interrupt */
> + platform_interrupt_unmask(irq);
> + l2_src->enabled = 1;
> + l1_int->enabled_count++;
> + }
> +
> + }
> +
> + spin_unlock(&l1_int->lock);
> +
> + return 0;
> +
> +}
> +
> +uint32_t irq_l2_disable(int irq, struct l2root_irq *l1_int)
irq_level2_mask()
> +{
> + struct l2src_irq *l2_src = NULL;
> + struct list_item *slist;
> +
> + /* looking for the bit id l2 src */
> + list_for_item(slist, &l1_int->src_list) {
> + l2_src = container_of(slist, struct l2src_irq, list);
> + if (l2_src->bit_id != REEF_IRQ_BIT(irq))
> + continue;
> +
> + if (l2_src->enabled) {
> + /* disable the src interrupt */
> + platform_interrupt_mask(irq);
> + l2_src->enabled = 0;
> + l1_int->enabled_count--;
> + /* disable the root interrupt */
> + if (l1_int->enabled_count == 0)
> + arch_interrupt_disable_mask(1 <<
> + REEF_IRQ_NUMBER(irq));
> + }
> +
> + }
> +
> + return 0;
> +
> +}
> +
> diff --git a/src/platform/baytrail/platform.c b/src/platform/baytrail/platform.c
> index b8c3e49..eed0c78 100644
> --- a/src/platform/baytrail/platform.c
> +++ b/src/platform/baytrail/platform.c
> @@ -99,6 +99,32 @@ int platform_boot_complete(uint32_t boot_message)
> return 0;
> }
>
> +int platform_interrupt_register(uint32_t irq,
> + void(*handler)(void *arg), void *arg)
> +{
> + return arch_interrupt_register(irq, handler, arg);
> +}
> +
> +void platform_interrupt_unregister(uint32_t irq)
> +{
> + arch_interrupt_unregister(irq);
> +}
> +
> +uint32_t platform_interrupt_enable(uint32_t irq)
> +{
> + return arch_interrupt_enable_mask(1 << irq);
> +}
> +
> +uint32_t platform_interrupt_disable(uint32_t irq)
> +{
> + return arch_interrupt_disable_mask(1 <<irq);
> +}
> +
> +void platform_interrupt_set(int irq)
> +{
> + arch_interrupt_set(irq);
> +}
> +
> /* clear mask in PISR, bits are W1C in docs but some bits need preserved ?? */
> void platform_interrupt_clear(uint32_t irq, uint32_t mask)
> {
> @@ -128,12 +154,12 @@ uint32_t platform_interrupt_get_enabled(void)
> return shim_read(SHIM_PIMR);
> }
>
> -void platform_interrupt_mask(uint32_t irq, uint32_t mask)
> +void platform_interrupt_mask(uint32_t irq)
> {
>
> }
>
> -void platform_interrupt_unmask(uint32_t irq, uint32_t mask)
> +void platform_interrupt_unmask(uint32_t irq)
> {
>
> }
More information about the Sound-open-firmware
mailing list