Re: [PATCH net-next 4/8] net: pcs: lynx: add lynx_pcs_create_fwnode()

[Sean Anderson <sean.anderson@seco.com>]

On 6/2/23 11:45, Russell King (Oracle) wrote:
> Add a helper to create a lynx PCS from a fwnode handle.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/net/pcs/pcs-lynx.c | 29 +++++++++++++++++++++++++++++
>  include/linux/pcs-lynx.h   |  1 +
>  2 files changed, 30 insertions(+)
> 
> diff --git a/drivers/net/pcs/pcs-lynx.c b/drivers/net/pcs/pcs-lynx.c
> index a90f74172f49..b0907c67d469 100644
> --- a/drivers/net/pcs/pcs-lynx.c
> +++ b/drivers/net/pcs/pcs-lynx.c
> @@ -353,6 +353,35 @@ struct phylink_pcs *lynx_pcs_create_mdiodev(struct mii_bus *bus, int addr)
>  }
>  EXPORT_SYMBOL(lynx_pcs_create_mdiodev);
>  
> +struct phylink_pcs *lynx_pcs_create_fwnode(struct fwnode_handle *node)
> +{
> +	struct mdio_device *mdio;
> +	struct phylink_pcs *pcs;

I think you should put the available check here as well.

> +	mdio = fwnode_mdio_find_device(node);
> +	if (!mdio)
> +		return ERR_PTR(-EPROBE_DEFER);
> +
> +	pcs = lynx_pcs_create(mdio);
> +
> +	/* Convert failure to create the PCS to an error pointer, so this
> +	 * function has a consistent return value strategy.
> +	 */
> +	if (!pcs)
> +		pcs = ERR_PTR(-ENOMEM);
> +
> +	/* lynx_create() has taken a refcount on the mdiodev if it was
> +	 * successful. If lynx_create() fails, this will free the mdio
> +	 * device here. In any case, we don't need to hold our reference
> +	 * anymore, and putting it here will allow mdio_device_put() in
> +	 * lynx_destroy() to automatically free the mdio device.
> +	 */
> +	mdio_device_put(mdio);
> +
> +	return pcs;
> +}
> +EXPORT_SYMBOL_GPL(lynx_pcs_create_fwnode);
> +
>  void lynx_pcs_destroy(struct phylink_pcs *pcs)
>  {
>  	struct lynx_pcs *lynx = phylink_pcs_to_lynx(pcs);
> diff --git a/include/linux/pcs-lynx.h b/include/linux/pcs-lynx.h
> index 25f68a096bfe..123e813df771 100644
> --- a/include/linux/pcs-lynx.h
> +++ b/include/linux/pcs-lynx.h
> @@ -11,6 +11,7 @@
>  
>  struct phylink_pcs *lynx_pcs_create(struct mdio_device *mdio);
>  struct phylink_pcs *lynx_pcs_create_mdiodev(struct mii_bus *bus, int addr);
> +struct phylink_pcs *lynx_pcs_create_fwnode(struct fwnode_handle *node);
>  
>  void lynx_pcs_destroy(struct phylink_pcs *pcs);
>  

Anyway, the rest of this series looks good to me.

--Sean