[PATCH net-next v19 00/15] net: phy: Introduce PHY ports representation

[Chris Mason]

On 11/27/25 10:53 AM, Maxime Chevallier wrote:
> Hi Chris,
> 
> On 27/11/2025 04:00, Jakub Kicinski wrote:
>> On Sat, 22 Nov 2025 13:42:59 +0100 Maxime Chevallier wrote:
>>> This is v19 of the phy_port work. Patches 2 and 3 lack PHY maintainers reviews.
>>>
>>> This v19 has no changes compared to v18, but patch 2 was rebased on top
>>> of the recent 1.6T linkmodes.
>>>
>>> Thanks for everyone's patience and reviews on that work ! Now, the
>>> usual blurb for the series description.
>>
>> Hopefully we can still make v6.19, but we hooked up Claude Code review
>> to patchwork this week, and it points out some legit issues here :(
>> Some look transient but others are definitely legit, please look thru
>> this:
>>

[ ... urls mangled by meta email ... ]

> I was told by Paolo to reach out with any feedback on the LLM reviews :
>

Thanks for sending these along, it really helps me fix up the prompts.

[ ... ]

> 
> Does priv->line_interface need to be reset when enable is false? The
> old mv2222_sfp_remove() explicitly set it to PHY_INTERFACE_MODE_NA,
> but the new code leaves it at whatever value was set during the last
> module insertion. Functions like mv2222_config_aneg() check whether
> priv->line_interface equals PHY_INTERFACE_MODE_NA to determine if a
> module is present.
> 
> --------x8----------------------------------------------------------------------
> 
> 
> Looking at the call-sites, we can see that when the .configure_mii port ops is
> called with enabled = false, the interface is always PHY_INTERFACE_MODE_NA.
> 
> Looks like the potential problem was identified correctly, but it failed to see
> that this can't ever happen.

This is also a problem for bpf reviews, where claude finds a pretty
detailed path to a bug that can never happen in practice.  I'll use this
and a few similar false positive reports to try and improve the
elimination of impossible bugs.

> It's a bit tricky I guess, as the call-site in question
> is introduced by a previous patch in the same series though.

This part shouldn't be a factor (I hope), but I'll check.

-chris