Re: [PATCH net-next v12 01/15] phylink: Add phylink_mac_interrupt

[Birger Koblitz <mail@birger-koblitz.de>]

Hi Andrew,

On 27/09/2026 4:18 pm, Andrew Lunn wrote:
> On Sun, Sep 27, 2026 at 11:03:17AM +0200, Birger Koblitz wrote:
>>
>>
>> On 17/09/2026 11:24 pm, netdev-bot+sashiko@kernel.org wrote:
>>> Thank you for your contribution! Sashiko AI review found 2 potential
>>> issue(s) to consider:
>>>
>>> Critical: 0 · High: 0 · Medium: 1 · Low: 1
>>>
>>> - [Medium] `phylink_mac_interrupt()`
>>>     (drivers/net/phy/phylink.c:1621-1628) reads `pl->phydev` with no…
>>
>> This does not look like an actual problem, as the interrupt is not really an
>> IRQ, it is a call from the MAC layer, calling phy_mac_interrupt(phy) did the
>> same in the past.
> 
> We need to be a little bit careful here. phy_mac_interrupt(phy) can be
> called in interrupt context. It does not perform any blocking
> operations, it just queues up the work to handle the actual event.
> 
> Here it is used in interrupt context.
> https://elixir.bootlin.com/linux/v7.2.8/source/drivers/net/ethernet/broadcom/asp2/bcmasp.c#L81
> 
> I would expect the phylink equivalent to be the same.
> 
> Since we are in interrupt context we cannot take a mutex. But do we
> need to worry about phydev disappearing? I don't think so. In
> practice, ignoring unbind via sysfs, the only way for a PHY to
> disappear is for an SFP module to be hot unplugged. But SPF don't
> support interrupts, so there should be no need to use
> phylink_mac_interrupt().
> 
> It might be worth adding some comments here?
Thanks for the reference, I understand this better, now. My suggestion 
would be to add a comment as part of the function description:
This function may be called in interrupt context, and therefore cannot 
take a mutex. However, it is safe to assume that the reference to phy 
does not disappear during the call, as only an SFP module may be 
unplugged to make a PHY disappear. SFP modules however do not support 
interrupts.

Birger