linux-kernel - Re: [V6,3/9] irqdomain: Don't set type when mapping an IRQ

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <59959014.9040603@rock-chips.com>
Date:   Thu, 17 Aug 2017 20:46:12 +0800
From:   jeffy <jeffy.chen@...k-chips.com>
To:     Jon Hunter <jonathanh@...dia.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Jason Cooper <jason@...edaemon.net>,
        Marc Zyngier <marc.zyngier@....com>
CC:     Rob Herring <robh+dt@...nel.org>, Pawel Moll <pawel.moll@....com>,
        Mark Rutland <mark.rutland@....com>,
        Ian Campbell <ijc+devicetree@...lion.org.uk>,
        Kumar Gala <galak@...eaurora.org>,
        Stephen Warren <swarren@...dotorg.org>,
        Thierry Reding <thierry.reding@...il.com>,
        Kevin Hilman <khilman@...nel.org>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Grygorii Strashko <grygorii.strashko@...com>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Linus Walleij <linus.walleij@...aro.org>,
        linux-tegra@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, briannorris@...omium.com
Subject: Re: [V6,3/9] irqdomain: Don't set type when mapping an IRQ

Hi guys,

I hit an problem when testing a level triggered irq with:

irq = platform_get_irq_byname(pdev, "wake");
...<-- irq trigger type is correct
irq_set_status_flags(irq, IRQ_NOAUTOEN);
...<-- irq trigger type become zero
request_threaded_irq(irq, ...)

the trigger type lost(irqd_get_trigger_type returns zero) after 
irq_set_status_flags.


it looks like irq_set_status_flags would try to use 
irq_settings_is_level to get level trigger type, which would get zero 
since we removed set type here...could you help to check this, thanks :)



more details in https://patchwork.kernel.org/patch/9903205/

On 06/07/2016 11:12 PM, Jon Hunter wrote:
> Some IRQ chips, such as GPIO controllers or secondary level
> interrupt controllers, may require require additional runtime power
> management control to ensure they are accessible. For such IRQ chips,
> it makes sense to enable the IRQ chip when interrupts are requested
> and disabled them again once all interrupts have been freed.
>
> When mapping an IRQ, the IRQ type settings are read and then
> programmed. The mapping of the IRQ happens before the IRQ is
> requested and so the programming of the type settings occurs before
> the IRQ is requested. This is a problem for IRQ chips that require
> additional power management control because they may not be
> accessible yet. Therefore, when mapping the IRQ, don't program the
> type settings, just save them and then program these saved settings
> when the IRQ is requested (so long as if they are not overridden via
>  the call to request the IRQ).
>
> Add a stub function for irq_domain_free_irqs() to avoid any
> compilation errors when CONFIG_IRQ_DOMAIN_HIERARCHY is not selected.
>
> Signed-off-by: Jon Hunter <jonathanh@...dia.com> Reviewed-by: Marc
> Zyngier <marc.zyngier@....com> --- include/linux/irqdomain.h |  3
> +++ kernel/irq/irqdomain.c    | 23 ++++++++++++++++++----- 2 files
> changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index f1f36e04d885..317503763314 100644 ---
> a/include/linux/irqdomain.h +++ b/include/linux/irqdomain.h @@ -452,6
> +452,9 @@ static inline int irq_domain_alloc_irqs(struct irq_domain
> *domain, return -1; }
>
> +static inline void irq_domain_free_irqs(unsigned int virq, +
> unsigned int nr_irqs) { } + static inline bool
> irq_domain_is_hierarchy(struct irq_domain *domain) { return false;
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c index
> f3ff1eb8dd09..caa6a63d26f0 100644 --- a/kernel/irq/irqdomain.c +++
> b/kernel/irq/irqdomain.c @@ -567,6 +567,7 @@ static void
> of_phandle_args_to_fwspec(struct of_phandle_args *irq_data, unsigned
>  int irq_create_fwspec_mapping(struct irq_fwspec *fwspec) { struct
> irq_domain *domain; +	struct irq_data *irq_data; irq_hw_number_t
> hwirq; unsigned int type = IRQ_TYPE_NONE; int virq; @@ -614,7 +615,11
> @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)
> * it now and return the interrupt number. */ if
> (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) { -
> irq_set_irq_type(virq, type); +			irq_data = irq_get_irq_data(virq);
> +			if (!irq_data) +				return 0; + + irqd_set_trigger_type(irq_data,
> type); return virq; }
>
> @@ -634,10 +639,18 @@ unsigned int irq_create_fwspec_mapping(struct
> irq_fwspec *fwspec) return virq; }
>
> -	/* Set type if specified and different than the current one */ - if
> (type != IRQ_TYPE_NONE && -	    type != irq_get_trigger_type(virq)) -
> irq_set_irq_type(virq, type); + irq_data = irq_get_irq_data(virq); +
> if (!irq_data) { +		if (irq_domain_is_hierarchy(domain)) +
> irq_domain_free_irqs(virq, 1); + else + irq_dispose_mapping(virq); +
> return 0; +	} + +	/* Store trigger type */ +
> irqd_set_trigger_type(irq_data, type); + return virq; }
> EXPORT_SYMBOL_GPL(irq_create_fwspec_mapping);
>