| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <7a82f5d435aaaf8d121233769a77943c@www.loen.fr>
Date: Mon, 06 Apr 2015 11:45:44 +0100
From: Marc Zyngier <marc.zyngier@....com>
To: Geert Uytterhoeven <geert+renesas@...der.be>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Magnus Damm <damm+renesas@...nsource.se>,
Arnd Bergmann <arnd@...db.de>,
Simon Horman <horms+renesas@...ge.net.au>,
Laurent Pinchart <laurent.pinchart@...asonboard.com>,
Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>,
<devel@...verdev.osuosl.org>, <devicetree@...r.kernel.org>,
<linux-pm@...r.kernel.org>, <linux-sh@...r.kernel.org>,
<linux-kernel@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH/RFC 3/6] staging: board: Add support for translating hwirq to virq numbers
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@...der.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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists