netdev - Re: [PATCH 0/4] net: dsa: mv88e6xxx: novice fixes and irq handling

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: <20180319122456.GB19795@lunn.ch>
Date:   Mon, 19 Mar 2018 13:24:56 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
Cc:     Vivien Didelot <vivien.didelot@...oirfairelinux.com>,
        Marc Zyngier <marc.zyngier@....com>,
        Thomas Gleixner <tglx@...utronix.de>, kernel@...gutronix.de,
        Florian Fainelli <f.fainelli@...il.com>,
        netdev@...r.kernel.org,
        Gregory CLEMENT <gregory.clement@...e-electrons.com>
Subject: Re: [PATCH 0/4] net: dsa: mv88e6xxx: novice fixes and irq handling

On Mon, Mar 19, 2018 at 11:05:19AM +0100, Uwe Kleine-König wrote:
> Hello,
> 
> this is a set of patches I created while I tried to understand how DSA
> works. I don't claim I already got there and so the first 3 fixes are
> trivial only.
> 
> I tried to make the switch on the espressobin SBC use the interrupt line
> and needed the fourth patch to make this work. Given I don't have access
> to the documentation of the Marvell switch, the patch is probably not
> correct though (and so doesn't have a S-o-b).
> 
> When I first added the irq as:
> 
> 	interrupt-parent = <&gpiosb>;
> 	interrupts = <23 IRQ_TYPE_LEVEL_LOW>;
> 
> to the switch of the espressobin's dtb, the irq couldn't be used. The
> reason is the interaction of several things:
> 
>  - On the first try to probe the switch, the driver did:
> 
> 	irq = of_irq_get(np, 0);
> 	request_threaded_irq(irq, NULL, func, IRQF_ONESHOT | IRQF_TRIGGER_FALLING, ...);
> 
>    and then later aborted with -EPROBE_DEFER because another resource
>    wasn't available yet. This correctly undid the request_threaded_irq
>    above using free_irq.
> 
>  - When the probe routine was entered again, the call to of_irq_get(np, 0)
>    failed becauce the irq code remembered that the irq was requested
>    using edge triggered logic with:
> 
> 	irq: type mismatch, failed to map hwirq-23 for gpio!
> 
>    (kernel/irq/irqdomain.c, line 801)
> 
> Any of the following changes would make the problem disappear:
> 
>  - use IRQ_TYPE_EDGE_FALLING in the device tree;
>  - drop IRQF_TRIGGER_FALLING from the driver's use of
>    request_threaded_irq() (I think, see below); and
>  - make the irq code forget the trigger type when the last action is
>    removed

Hi Uwe

I think the correct fix here is to use the flag in device tree when
requesting the interrupt. If it says IRQ_TYPE_LEVEL_LOW, then request
it active low.

Thinking about the hardware, active low is probably right, not edge.
It will stay low until all the sources of interrupts are cleared.

   Andrew