netdev - Re: [PATCH net-next 4/4] net: phy: change phy_start_interrupts to phy_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>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <fc02a7ff-a523-7151-7eba-e72f607154fd@gmail.com>
Date:   Sun, 20 Jan 2019 09:34:36 +0100
From:   Heiner Kallweit <hkallweit1@...il.com>
To:     Florian Fainelli <f.fainelli@...il.com>,
        Andrew Lunn <andrew@...n.ch>,
        David Miller <davem@...emloft.net>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 4/4] net: phy: change phy_start_interrupts to
 phy_request_interrupt

On 20.01.2019 00:43, Florian Fainelli wrote:
> 
> 
> On January 19, 2019 3:30:05 AM PST, Heiner Kallweit <hkallweit1@...il.com> wrote:
>> Now that we enable the interrupts in phy_start() we don't have to do it
>> before. Therefore remove enabling interrupts from
>> phy_start_interrupts()
>> and rename this function to reflect the changed functionality.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@...il.com>
>> ---
> 
>> + * phy_request_interrupt - request interrupt for a PHY device
>>  * @phydev: target phy_device struct
>>  *
>>  * Description: Request the interrupt for the given PHY.
>>  *   If this fails, then we set irq to PHY_POLL.
>> - *   Otherwise, we enable the interrupts in the PHY.
>>  *   This should only be called with a valid IRQ number.
>> - *   Returns 0 on success or < 0 on error.
>>  */
>> -int phy_start_interrupts(struct phy_device *phydev)
>> +void phy_request_interrupt(struct phy_device *phydev)
>> {
>> 	if (request_threaded_irq(phydev->irq, NULL, phy_interrupt,
>> 				 IRQF_ONESHOT | IRQF_SHARED,
>> 				 phydev_name(phydev), phydev) < 0) {
>> 		phydev_warn(phydev, "Can't get IRQ %d\n", phydev->irq);
>> 		phydev->irq = PHY_POLL;
>> -		return 0;
>> 	}
> 
> We should propagate the return code here and/or indicate we are falling back to polling since may not be desired. An use case that should be considered is probe deferral for instance.
> 
I kept the current behavior to basically ignore the error and just
warn and fall back to polling. Seems that phylib has behaved this way
forever. If we come to the conclusion that changing this behavior
makes sense, shouldn't we do it separately? Also we may break systems
relying on the fallback to polling.
But something we can easily agree on is to extend the warning
to state that we fall back to polling.