Re: [PATCH net-next resubmit 1/2] net: phy: core: remove now uneeded disabling of interrupts

[David Miller <davem@davemloft.net>]

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Date: Mon, 4 Dec 2017 15:46:55 +0000

> On 4 December 2017 at 15:24, David Miller <davem@davemloft.net> wrote:
>> From: Heiner Kallweit <hkallweit1@gmail.com>
>> Date: Thu, 30 Nov 2017 23:55:15 +0100
>>
>>> After commits c974bdbc3e "net: phy: Use threaded IRQ, to allow IRQ from
>>> sleeping devices" and 664fcf123a30 "net: phy: Threaded interrupts allow
>>> some simplification" all relevant code pieces run in process context
>>> anyway and I don't think we need the disabling of interrupts any longer.
>>>
>>> Interestingly enough, latter commit already removed the comment
>>> explaining why interrupts need to be temporarily disabled.
>>>
>>> On my system phy interrupt mode works fine with this patch.
>>> However I may miss something, especially in the context of shared phy
>>> interrupts, therefore I'd appreciate if more people could test this.
>>>
>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>> Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>
>> Ok, applied.
>>
>> But if this causes regressions I'm reverting.
> 
> Thanks. But please note that the code in question does seem to use the
> interrupt API incorrectly, and tbh, I was expecting some more
> discussion first. For reference, here's the commit log for the mostly
> equivalent patch [0] I sent out almost at the same time:

Yes, it seemed to me that when the code was converted to threaded IRQS
this {enable,disable}_irq() stuff was not considered.

Again, I read these patches over and I'm willing to own up to the
changes for now.  And if they cause regressions or someone screams
loudly enough we can revert and talk about it some more.