netdev - Re: [PATCH net-next 4/8] net: pcs: lynx: add lynx_pcs_create

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <64b55156-81e2-44cf-224d-d362e10955e3@seco.com>
Date: Fri, 2 Jun 2023 11:51:23 -0400
From: Sean Anderson <sean.anderson@...o.com>
To: "Russell King (Oracle)" <rmk+kernel@...linux.org.uk>,
 Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>
Cc: "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Ioana Ciornei <ioana.ciornei@....com>,
 Jakub Kicinski <kuba@...nel.org>, Madalin Bucur <madalin.bucur@....com>,
 netdev@...r.kernel.org, Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH net-next 4/8] net: pcs: lynx: add lynx_pcs_create_fwnode()

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@...linux.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