[net-next,0/6] net: pcs: add helpers to xpcs and lynx to manage mdiodev

Message

Russell King (Oracle) May 26, 2023, 10:13 a.m. UTC

Hi,

This morning, we have had two instances where the destruction of the
MDIO device associated with XPCS and Lynx has been wrong. Rather than
allowing this pattern of errors to continue, let's make it easier for
driver authors to get this right by adding a helper.

The changes are essentially:

1. Add two new mdio device helpers to manage the underlying struct
   device reference count. Note that the existing mdio_device_free()
   doesn't actually free anything, it merely puts the reference count.

2. Make the existing _create() and _destroy() PCS driver methods
   increment and decrement this refcount using these helpers. This
   results in no overall change, although drivers may hang on to
   the mdio device for a few cycles longer.

3. Add _create_mdiodev() which creates the mdio device before calling
   the existing _create() method. Once the _create() method has
   returned, we put the reference count on the mdio device.

   If _create() was successful, then the reference count taken there
   will "hold" the mdio device for the lifetime of the PCS (in other
   words, until _destroy() is called.) However, if _create() failed,
   then dropping the refcount at this point will free the mdio device.

   This is the exact behaviour we desire.

4. Convert users that create a mdio device and then call the PCS's
   _create() method over to the new _create_mdiodev() method, and
   simplify the cleanup.

We also have DPAA2 and fmem_memac that look up their PCS rather than
creating it. These could also drop their reference count on the MDIO
device immediately after calling lynx_pcs_create(), which would then
mean we wouldn't need lynx_get_mdio_device() and the associated
complexity to put the device in dpaa2_pcs_destroy() and pcs_put().
Note that DPAA2 bypasses the mdio device's abstractions by calling
put_device() directly.

 drivers/net/dsa/ocelot/felix_vsc9959.c            | 20 +++------------
 drivers/net/dsa/ocelot/seville_vsc9953.c          | 20 +++------------
 drivers/net/ethernet/freescale/enetc/enetc_pf.c   | 22 +++-------------
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 15 +++--------
 drivers/net/pcs/pcs-lynx.c                        | 31 +++++++++++++++++++++++
 drivers/net/pcs/pcs-xpcs.c                        | 28 ++++++++++++++++++++
 include/linux/mdio.h                              | 10 ++++++++
 include/linux/pcs-lynx.h                          |  1 +
 include/linux/pcs/pcs-xpcs.h                      |  2 ++
 9 files changed, 87 insertions(+), 62 deletions(-)

Comments

[e-mail] May 30, 2023, 5 a.m. UTC | #1

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <[e-mail]>:

On Fri, 26 May 2023 11:13:59 +0100 you wrote:
> Hi,
> 
> This morning, we have had two instances where the destruction of the
> MDIO device associated with XPCS and Lynx has been wrong. Rather than
> allowing this pattern of errors to continue, let's make it easier for
> driver authors to get this right by adding a helper.
> 
> [...]

Here is the summary with links:
  - [net-next,1/6] net: mdio: add mdio_device_get() and mdio_device_put()
    https://git.kernel.org/netdev/net-next/c/c4933fa88a68
  - [net-next,2/6] net: pcs: xpcs: add xpcs_create_mdiodev()
    https://git.kernel.org/netdev/net-next/c/9a5d500cffdb
  - [net-next,3/6] net: stmmac: use xpcs_create_mdiodev()
    https://git.kernel.org/netdev/net-next/c/727e373f897d
  - [net-next,4/6] net: pcs: lynx: add lynx_pcs_create_mdiodev()
    https://git.kernel.org/netdev/net-next/c/86b5f2d8cd78
  - [net-next,5/6] net: dsa: ocelot: use lynx_pcs_create_mdiodev()
    https://git.kernel.org/netdev/net-next/c/5767c6a8d9b7
  - [net-next,6/6] net: enetc: use lynx_pcs_create_mdiodev()
    https://git.kernel.org/netdev/net-next/c/b7d5d0438e01

You are awesome, thank you!