Re: [PATCH net-next 4/4] net: phy: change phy_start_interrupts to phy_request_interrupt

[Heiner Kallweit <hkallweit1@gmail.com>]

On 20.01.2019 00:43, Florian Fainelli wrote:
> 
> 
> On January 19, 2019 3:30:05 AM PST, Heiner Kallweit <hkallweit1@gmail.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@gmail.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.