lkml.org 
[lkml]   [2015]   [Apr]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH/RFC 3/6] staging: board: Add support for translating hwirq to virq numbers
Date
From
On 2015-04-03 13:42, Geert Uytterhoeven wrote:
> As of commit 9a1091ef0017c40a ("irqchip: gic: Support hierarchy irq
> domain."), GIC IRQ numbers are virtual, breaking hardcoded hardware
> IRQ
> numbers in platform device resources.
>
> Add support for translating hardware IRQ numbers to virtual IRQ
> numbers,
> and fixing up platform device resources with hardcoded IRQ numbers.
>
> Add a copyright header, including the original author.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> drivers/staging/board/board.c | 66
> +++++++++++++++++++++++++++++++++++++++++++
> drivers/staging/board/board.h | 5 ++++
> 2 files changed, 71 insertions(+)
>
> diff --git a/drivers/staging/board/board.c
> b/drivers/staging/board/board.c
> index d5a6abc845191c93..b84ac2837a20bf06 100644
> --- a/drivers/staging/board/board.c
> +++ b/drivers/staging/board/board.c
> @@ -1,10 +1,27 @@
> +/*
> + * Copyright (C) 2014 Magnus Damm
> + * Copyright (C) 2015 Glider bvba
> + *
> + * This file is subject to the terms and conditions of the GNU
> General Public
> + * License. See the file "COPYING" in the main directory of this
> archive
> + * for more details.
> + */
> +
> +#define pr_fmt(fmt) "board_staging: " fmt
> +
> #include <linux/init.h>
> +#include <linux/irq.h>
> #include <linux/device.h>
> #include <linux/kernel.h>
> #include <linux/of.h>
> #include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +
> #include "board.h"
>
> +static struct device_node *irqc_node __initdata;
> +static unsigned int irqc_base __initdata;
> +
> static bool find_by_address(u64 base_address)
> {
> struct device_node *dn = of_find_all_nodes(NULL);
> @@ -38,3 +55,52 @@ bool __init board_staging_dt_node_available(const
> struct resource *resource,
>
> return false; /* Nothing found */
> }
> +
> +int __init board_staging_setup_hwirq_xlate(const char *irqc_match,
> + unsigned int base)
> +{
> + irqc_node = of_find_compatible_node(NULL, NULL, irqc_match);
> +
> + WARN_ON(!irqc_node);
> + if (!irqc_node)
> + return -ENOENT;
> +
> + irqc_base = base;
> + return 0;
> +}

And what happens when you have multiple (cascaded) interrupt
controllers, each wanting to register with this translation service? You
should at least check that nobody has been here before.

> +static unsigned int __init xlate_hwirq(unsigned int hwirq)
> +{
> + struct of_phandle_args irq_data;
> + unsigned int irq;
> +
> + if (!irqc_node)
> + return hwirq;
> +
> + irq_data.np = irqc_node;
> + irq_data.args_count = 3;
> + irq_data.args[0] = 0;
> + irq_data.args[1] = hwirq - irqc_base;
> + irq_data.args[2] = IRQ_TYPE_LEVEL_HIGH;

And what happens for edge interrupts? Or LEVEL_LOW? This is very much
GIC specific (3 args...). How does it work with non-GIC interrupt
controllers?

> +
> + irq = irq_create_of_mapping(&irq_data);
> + if (WARN_ON(!irq))
> + irq = hwirq;
> +
> + return irq;
> +}
> +
> +void __init board_staging_fixup_irq_resources(struct resource *res,
> + unsigned int nres)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < nres; i++)
> + if (res[i].flags == IORESOURCE_IRQ) {
> + unsigned int hwirq = res[i].start;
> + unsigned int virq = xlate_hwirq(hwirq);
> +
> + pr_debug("hwirq %u -> virq %u\n", hwirq, virq);
> + res[i].start = virq;
> + }
> +}
> diff --git a/drivers/staging/board/board.h
> b/drivers/staging/board/board.h
> index e9c914985d4acb36..4cedc3c46e287eb7 100644
> --- a/drivers/staging/board/board.h
> +++ b/drivers/staging/board/board.h
> @@ -1,10 +1,15 @@
> #ifndef __BOARD_H__
> #define __BOARD_H__
> +
> #include <linux/init.h>
> #include <linux/of.h>
>
> +struct resource;
> +
> bool board_staging_dt_node_available(const struct resource
> *resource,
> unsigned int num_resources);
> +int board_staging_setup_hwirq_xlate(const char *irqc_match, unsigned
> int base);
> +void board_staging_fixup_irq_resources(struct resource *res,
> unsigned int nres);
>
> #define board_staging(str, fn) \
> static int __init runtime_board_check(void) \

It won't surprise you that I don't really like this approach. It is
controller-specific, restrictive, and allows platform code to continue
doing something that is essentially wrong. Should this code ever move
out of staging, it should depend on CONFIG_BROKEN, because this is
essentially what it is - support code for broken systems. I'd also
welcome moving parts of the OMAP4/5 code to such CONFIG_BROKEN
dependency.

Thanks,

M.
--
Fast, cheap, reliable. Pick two.


\
 
 \ /
  Last update: 2015-04-06 13:21    [W:0.134 / U:0.076 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site