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

[Andrew Lunn]

> > 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()

	Andrew