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]
Date:   Wed, 10 Apr 2019 09:53:40 +0000
From:   Jisheng Zhang <>
To:     "" 
        Andrew Lunn <>,
        Florian Fainelli <>
CC:     "" <>,
        "" <>,
        "" <>,
        "" <>,
        "" <>,
        "" <>,
        "" <>
Subject: Re: [PATCH] net: lan78xx: fix "enabled interrupts" warninig

On Wed, 10 Apr 2019 09:20:38 +0000 wrote:

> > > >
> > > > The warning comes from calling generic_handle_irq() in usb tasklet  
> > context.  
> > > > This is not correct.
> > > >
> > > > Per my understanding, if there's chained irq, we could introduce
> > > > extra irqdomain. E.g
> > > >
> > > > GIC <--> another irqchip controller <--> HW device
> > > >  
> > > Correct, IRQ domain is generally used in chained irq controllers.
> > > Yes, We need to check why irq domain is used in the current driver.
> > >  
> >
> > It's introduced in the commit cc89c323a30e
> >  
> Hi Jisheng,


> I had spent some time to look into the history of commit. The reason for having irq domain in driver is because of the unavailability of real hardware interrupt, here we have USB interrupt pipe which is not actual interrupt.
> So changes were proposed to have pseudo phy interrupt and handlers in phy lib. But later it was suggested to implement linux interrupt controller in driver itself.

+ Andrew, Florian

USB net is different with Andrew's case.

there's no irq at all in usb net

but in Andrew's case, interrupt is connected to a GPIO line. As is known
GPIO can behave as irq chip.

> You can see the archive here

Per my understanding, the purpose is to avoid phy_state_machine() poll
phy_read_status() to monitor phy link. But as pointed out by Florian,
phy_mac_interrupt() is intended for that purpose. And the poll should be
fixed in phy. Since the email was sent in 2016, maybe the bug has been fixed.

> I want to understand if there is any functionality impact with this warning? Because I'm afraid if the current changes are removed we might hit some other issues (or older ones). We have to go through rigorous testing before going ahead.

Warning indicates there's something wrong in the code.

IMHO phy_mac_interrupt() and PHY_IGNORE_INTERRUPT is the correct
solution. If the phy_mac_interrupt() poll is fixed, I think maybe
old issue which commit cc89c323a30e want to fix won't exist.


Powered by blists - more mailing lists