On 4 December 2017 at 15:50, David Miller <da...@davemloft.net> wrote:
> From: Ard Biesheuvel <ard.biesheu...@linaro.org>
> Date: Mon, 4 Dec 2017 15:46:55 +0000
>
>> On 4 December 2017 at 15:24, David Miller <da...@davemloft.net> wrote:
>>> From: Heiner Kallweit <hkallwe...@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 <hkallwe...@gmail.com>
>>>> Acked-by: Ard Biesheuvel <ard.biesheu...@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.
Excellent, thanks.
