netdev - Re: [PATCH] net: davinci_emac: Replace devm_request_irq with request

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>] [day] [month] [year] [list]

Message-ID: <CAGVrzcb6hM6N6qLz_r3Y4j57=n3U_Du1Ls5KdDJE4cCoVbx4RQ@mail.gmail.com>
Date:	Thu, 6 Mar 2014 09:16:05 -0800
From:	Florian Fainelli <f.fainelli@...il.com>
To:	Christian Riesch <christian.riesch@...cron.at>
Cc:	Mugunthan V N <mugunthanvnm@...com>,
	netdev <netdev@...r.kernel.org>, stable <stable@...r.kernel.org>,
	dlos <davinci-linux-open-source@...ux.davincidsp.com>,
	Jon Ringle <jon@...gle.org>
Subject: Re: [PATCH] net: davinci_emac: Replace devm_request_irq with request_irq

2014-03-06 0:51 GMT-08:00 Christian Riesch <christian.riesch@...cron.at>:
> --On March 06, 2014 13:57 +0530 Mugunthan V N <mugunthanvnm@...com> wrote:
>>
>> On Wednesday 05 March 2014 03:55 PM, Christian Riesch wrote:
>>>
>>> In commit 6892b41d9701283085b655c6086fb57a5d63fa47
>>>
>>> Author: Lad, Prabhakar <prabhakar.csengg@...il.com>
>>> Date:   Tue Jun 25 21:24:51 2013 +0530
>>> net: davinci: emac: Convert to devm_* api
>>>
>>> the call of request_irq is replaced by devm_request_irq and the call
>>> of free_irq is removed. But since interrupts are requested in
>>> emac_dev_open, doing ifconfig up/down on the board requests the
>>> interrupts again each time, causing devm_request_irq to fail.
>>>
>>> This patch reverts said commit partially: It changes the driver back
>>> to use request_irq instead of devm_request_irq, puts free_irq back in
>>> place, but keeps the remaining changes of the original patch.
>>>
>>> Reported-by: Jon Ringle <jon@...gle.org>
>>> Signed-off-by: Christian Riesch <christian.riesch@...cron.at>
>>> Cc: Lad, Prabhakar <prabhakar.csengg@...il.com>
>>> Cc: <stable@...r.kernel.org>
>>
>>
>> Instead of moving back to request irq. can you move the devm interrupt
>> request code from open to probe so that the issue is fixed, IRQ will be
>> requested on module insert and IRQ will be free in module remove and
>> there will not be any failures in devm request irq while repeating
>> ifup/ifdown and also during module insert/remove.
>
>
> This is exactly what I tried first, please have a look at my first patch and
> the discussion [1]. Florian Fainelli asked me to revert Prabhakar's patch
> instead to bring back the "usual situation", "just in case some uncontrolled
> interrupt bit triggers an interrupt while the interface is down for
> instance..."
>
> Florian Fainelli, Mugunthan VN, which solution is better?

I think the solution that does:

- request_irq() in ndo_open()
- free_irq() in ndo_stop()

is the safest option because it ensure that the various interrupt
lines are masked at the interrupt controller level, and not just at
Ethernet MAC level, and as such gives you more control over what the
hardware does. It also mimics what 99% of the Ethernet drivers do,
which will help identifying those common patterns programmatically.
-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html