Re: [PATCH net-next v5] net: phy: Add driver for Motorcomm Quad 2.5GbE phy

[Kyle Switch]

On 7/28/26 05:36, Andrew Lunn wrote:
>>> So, please look at adding phy_package_lock() and
>>> phy_package_unlock(). Maybe it is possible to make use of struct
>>> mii_bus shared_lock? Please look at how that is currently used and
>>> check to see if there might be deadlocks.
>>>
>>
>> Ans: From my perspective, although a phy8824 has four UTPs, it uses the same mdio bus. 
>>      Wouldn't it be possible to achieve the desired effect without using an extra lock.
>>
>>      There are only two scenarios for phy8824 to swap to USXGMII reg space.
>>      1. init configuration 
>>      2. soft reset and isolate done in USXGMII side during UTP soft reset.
> 
> Soft reset is performed in phy_init_hw() which is called in a few
> different places, such as phy_attach_direct(), phy_mii_ioctl() etc. So
> you have to assume it can be called anytime. As such you need locking.
> 
> A big point of phylib is the collection of helpers to deal with
> anything defined in 802.3. The PHY driver should be using these
> helpers to do things defined in the standard. But if you are holding
> the mdio lock, you cannot use the helpers. What i don't want to see is
> your driver reimplementing code which we have in helpers because of
> your bad hardware design.
> 
> Locking is hard, and given how infrequently phylib does operations, it
> is going to be difficult to find and fix race conditions which result
> in the PHYs becoming miss configured. So you need a simple, robust
> solution.
> 
> If you think you have a solution to allow the helpers to be used,
> which is simple and robust, please post it. Otherwise, i think you
> need phy_package_lock()/phy_package_unlock()

Ans: i will post patch v7, please help comments on it.

> 
> 	Andrew