[PATCH net-next 0/8] complete Lynx mdio device handling

[PATCH net-next 0/8] complete Lynx mdio device handling

[Russell King (Oracle)]

Hi,

This series completes the mdio device lifetime handling for Lynx PCS
users which do not create their own mdio device, but instead fetch
it using a firmware description - namely the DPAA2 and FMAN_MEMAC
drivers.

In a previous patch set, lynx_pcs_create() was modified to increase
the mdio device refcount, and lynx_pcs_destroy() to drop that
refcount.

The first two patches change these two drivers to put the reference
which they hold immediately after lynx_pcs_create(), effectively
handing the responsibility for maintaining the refcount to the Lynx
PCS driver.

A side effect of the first two patches is that lynx_get_mdio_device()
is no longer used, so patch 3 removes it.

Patch 4 adds a new helper - lynx_pcs_create_fwnode(), which creates
a Lynx PCS instance from the fwnode.

Patch 5 and 6 convert the two drivers to make use of this new helper,
which simply has to find the mdio device, and then create the Lynx
PCS from that.

With those conversions done, lynx_pcs_create() is no longer required
outside pcs-lynx.c, so remove it from public view.

Finally, in patch 8 we change lynx_pcs_create() to return an
error-pointer rather than NULL to bring consistency to the return
style, and means that we can remove the NULL-to-error-pointer
conversion from both lynx_pcs_create_fwnode() and
lynx_pcs_create_mdiodev().

 drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c | 21 ++++++-------
 drivers/net/ethernet/freescale/fman/fman_memac.c | 18 +++--------
 drivers/net/pcs/pcs-lynx.c                       | 40 ++++++++++++++----------
 include/linux/pcs-lynx.h                         |  4 +--
 4 files changed, 39 insertions(+), 44 deletions(-)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

[PATCH net-next 1/8] net: dpaa2-mac: allow lynx PCS to manage mdiodev lifetime

[Russell King (Oracle)]

Put the mdiodev after lynx_pcs_create() so that the Lynx PCS driver
can manage the lifetime of the mdiodev its using.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
index cb70855e2b9a..c0f7dd3b4ac1 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
@@ -271,9 +271,9 @@ static int dpaa2_pcs_create(struct dpaa2_mac *mac,
 	}
 
 	mac->pcs = lynx_pcs_create(mdiodev);
+	mdio_device_put(mdiodev);
 	if (!mac->pcs) {
 		netdev_err(mac->net_dev, "lynx_pcs_create() failed\n");
-		mdio_device_free(mdiodev);
 		return -ENOMEM;
 	}
 
@@ -285,10 +285,7 @@ static void dpaa2_pcs_destroy(struct dpaa2_mac *mac)
 	struct phylink_pcs *phylink_pcs = mac->pcs;
 
 	if (phylink_pcs) {
-		struct mdio_device *mdio = lynx_get_mdio_device(phylink_pcs);
-
 		lynx_pcs_destroy(phylink_pcs);
-		mdio_device_free(mdio);
 		mac->pcs = NULL;
 	}
 }
-- 
2.30.2

[PATCH net-next 2/8] net: fman_memac: allow lynx PCS to handle mdiodev lifetime

[Russell King (Oracle)]

Put the mdiodev after lynx_pcs_create() so that the Lynx PCS driver
can manage the lifetime of the mdiodev its using.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/freescale/fman/fman_memac.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fman/fman_memac.c b/drivers/net/ethernet/freescale/fman/fman_memac.c
index 625c79d5636f..8f45caf4af12 100644
--- a/drivers/net/ethernet/freescale/fman/fman_memac.c
+++ b/drivers/net/ethernet/freescale/fman/fman_memac.c
@@ -976,14 +976,10 @@ static int memac_init(struct fman_mac *memac)
 
 static void pcs_put(struct phylink_pcs *pcs)
 {
-	struct mdio_device *mdiodev;
-
 	if (IS_ERR_OR_NULL(pcs))
 		return;
 
-	mdiodev = lynx_get_mdio_device(pcs);
 	lynx_pcs_destroy(pcs);
-	mdio_device_free(mdiodev);
 }
 
 static int memac_free(struct fman_mac *memac)
@@ -1055,8 +1051,7 @@ static struct phylink_pcs *memac_pcs_create(struct device_node *mac_node,
 		return ERR_PTR(-EPROBE_DEFER);
 
 	pcs = lynx_pcs_create(mdiodev);
-	if (!pcs)
-		mdio_device_free(mdiodev);
+	mdio_device_put(mdiodev);
 
 	return pcs;
 }
-- 
2.30.2

[PATCH net-next 3/8] net: pcs: lynx: remove lynx_get_mdio_device()

[Russell King (Oracle)]

lynx_get_mdio_device() is no longer necessary, let's remove it so the
lynx PCS code is always managing the lifetime of the mdiodev.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/pcs/pcs-lynx.c | 8 --------
 include/linux/pcs-lynx.h   | 2 --
 2 files changed, 10 deletions(-)

diff --git a/drivers/net/pcs/pcs-lynx.c b/drivers/net/pcs/pcs-lynx.c
index f04dc580ffb8..a90f74172f49 100644
--- a/drivers/net/pcs/pcs-lynx.c
+++ b/drivers/net/pcs/pcs-lynx.c
@@ -34,14 +34,6 @@ enum sgmii_speed {
 #define phylink_pcs_to_lynx(pl_pcs) container_of((pl_pcs), struct lynx_pcs, pcs)
 #define lynx_to_phylink_pcs(lynx) (&(lynx)->pcs)
 
-struct mdio_device *lynx_get_mdio_device(struct phylink_pcs *pcs)
-{
-	struct lynx_pcs *lynx = phylink_pcs_to_lynx(pcs);
-
-	return lynx->mdio;
-}
-EXPORT_SYMBOL(lynx_get_mdio_device);
-
 static void lynx_pcs_get_state_usxgmii(struct mdio_device *pcs,
 				       struct phylink_link_state *state)
 {
diff --git a/include/linux/pcs-lynx.h b/include/linux/pcs-lynx.h
index 885b59d10581..25f68a096bfe 100644
--- a/include/linux/pcs-lynx.h
+++ b/include/linux/pcs-lynx.h
@@ -9,8 +9,6 @@
 #include <linux/mdio.h>
 #include <linux/phylink.h>
 
-struct mdio_device *lynx_get_mdio_device(struct phylink_pcs *pcs);
-
 struct phylink_pcs *lynx_pcs_create(struct mdio_device *mdio);
 struct phylink_pcs *lynx_pcs_create_mdiodev(struct mii_bus *bus, int addr);
 
-- 
2.30.2

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

[Russell King (Oracle)]

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;
+
+	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);
 
-- 
2.30.2

[PATCH net-next 5/8] net: dpaa2-mac: use lynx_pcs_create_fwnode()

[Russell King (Oracle)]

Use lynx_pcs_create_fwnode() to create a lynx PCS from a fwnode handle.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 .../net/ethernet/freescale/dpaa2/dpaa2-mac.c   | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
index c0f7dd3b4ac1..38e6208f9e1a 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
@@ -247,8 +247,8 @@ static int dpaa2_pcs_create(struct dpaa2_mac *mac,
 			    struct fwnode_handle *dpmac_node,
 			    int id)
 {
-	struct mdio_device *mdiodev;
 	struct fwnode_handle *node;
+	struct phylink_pcs *pcs;
 
 	node = fwnode_find_reference(dpmac_node, "pcs-handle", 0);
 	if (IS_ERR(node)) {
@@ -263,20 +263,22 @@ static int dpaa2_pcs_create(struct dpaa2_mac *mac,
 		return -ENODEV;
 	}
 
-	mdiodev = fwnode_mdio_find_device(node);
+	pcs = lynx_pcs_create_fwnode(node);
 	fwnode_handle_put(node);
-	if (!mdiodev) {
+
+	if (pcs == ERR_PTR(-EPROBE_DEFER)) {
 		netdev_dbg(mac->net_dev, "missing PCS device\n");
 		return -EPROBE_DEFER;
 	}
 
-	mac->pcs = lynx_pcs_create(mdiodev);
-	mdio_device_put(mdiodev);
-	if (!mac->pcs) {
-		netdev_err(mac->net_dev, "lynx_pcs_create() failed\n");
-		return -ENOMEM;
+	if (IS_ERR(pcs)) {
+		netdev_err(mac->net_dev,
+			   "lynx_pcs_create_fwnode() failed: %pe\n", pcs);
+		return PTR_ERR(pcs);
 	}
 
+	mac->pcs = pcs;
+
 	return 0;
 }
 
-- 
2.30.2

[PATCH net-next 6/8] net: fman_memac: use lynx_pcs_create_fwnode()

[Russell King (Oracle)]

Use lynx_pcs_create_fwnode() to create a lynx PCS from a fwnode handle.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/freescale/fman/fman_memac.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fman/fman_memac.c b/drivers/net/ethernet/freescale/fman/fman_memac.c
index 8f45caf4af12..4fbdae996d05 100644
--- a/drivers/net/ethernet/freescale/fman/fman_memac.c
+++ b/drivers/net/ethernet/freescale/fman/fman_memac.c
@@ -1039,19 +1039,14 @@ static struct phylink_pcs *memac_pcs_create(struct device_node *mac_node,
 					    int index)
 {
 	struct device_node *node;
-	struct mdio_device *mdiodev = NULL;
 	struct phylink_pcs *pcs;
 
 	node = of_parse_phandle(mac_node, "pcsphy-handle", index);
-	if (node && of_device_is_available(node))
-		mdiodev = of_mdio_find_device(node);
-	of_node_put(node);
-
-	if (!mdiodev)
-		return ERR_PTR(-EPROBE_DEFER);
+	if (!node || !of_device_is_available(node))
+		return ERR_PTR(-ENODEV);
 
-	pcs = lynx_pcs_create(mdiodev);
-	mdio_device_put(mdiodev);
+	pcs = lynx_pcs_create_fwnode(of_fwnode_handle(node));
+	of_node_put(node);
 
 	return pcs;
 }
-- 
2.30.2

[PATCH net-next 7/8] net: pcs: lynx: make lynx_pcs_create() static

[Russell King (Oracle)]

We no longer need to export lynx_pcs_create() for drivers to use as we
now have all the functionality we need in the two new creation helpers.
Remove the export and prototype, and make it static.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/pcs/pcs-lynx.c | 3 +--
 include/linux/pcs-lynx.h   | 1 -
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/pcs/pcs-lynx.c b/drivers/net/pcs/pcs-lynx.c
index b0907c67d469..b8c66137e28d 100644
--- a/drivers/net/pcs/pcs-lynx.c
+++ b/drivers/net/pcs/pcs-lynx.c
@@ -307,7 +307,7 @@ static const struct phylink_pcs_ops lynx_pcs_phylink_ops = {
 	.pcs_link_up = lynx_pcs_link_up,
 };
 
-struct phylink_pcs *lynx_pcs_create(struct mdio_device *mdio)
+static struct phylink_pcs *lynx_pcs_create(struct mdio_device *mdio)
 {
 	struct lynx_pcs *lynx;
 
@@ -322,7 +322,6 @@ struct phylink_pcs *lynx_pcs_create(struct mdio_device *mdio)
 
 	return lynx_to_phylink_pcs(lynx);
 }
-EXPORT_SYMBOL(lynx_pcs_create);
 
 struct phylink_pcs *lynx_pcs_create_mdiodev(struct mii_bus *bus, int addr)
 {
diff --git a/include/linux/pcs-lynx.h b/include/linux/pcs-lynx.h
index 123e813df771..7958cccd16f2 100644
--- a/include/linux/pcs-lynx.h
+++ b/include/linux/pcs-lynx.h
@@ -9,7 +9,6 @@
 #include <linux/mdio.h>
 #include <linux/phylink.h>
 
-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);
 
-- 
2.30.2

[PATCH net-next 8/8] net: pcs: lynx: change lynx_pcs_create() to return error-pointers

[Russell King (Oracle)]

Change lynx_pcs_create() to return an error-pointer on failure to
allocate memory, rather than returning NULL. This allows the removal
of the conversion in lynx_pcs_create_fwnode() and
lynx_pcs_create_mdiodev().

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/pcs/pcs-lynx.c | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/drivers/net/pcs/pcs-lynx.c b/drivers/net/pcs/pcs-lynx.c
index b8c66137e28d..c2d01da40430 100644
--- a/drivers/net/pcs/pcs-lynx.c
+++ b/drivers/net/pcs/pcs-lynx.c
@@ -313,7 +313,7 @@ static struct phylink_pcs *lynx_pcs_create(struct mdio_device *mdio)
 
 	lynx = kzalloc(sizeof(*lynx), GFP_KERNEL);
 	if (!lynx)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
 	mdio_device_get(mdio);
 	lynx->mdio = mdio;
@@ -334,12 +334,6 @@ struct phylink_pcs *lynx_pcs_create_mdiodev(struct mii_bus *bus, int addr)
 
 	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
@@ -363,12 +357,6 @@ struct phylink_pcs *lynx_pcs_create_fwnode(struct fwnode_handle *node)
 
 	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
-- 
2.30.2

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

[Sean Anderson]

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

Re: [PATCH net-next 0/8] complete Lynx mdio device handling

[Russell King (Oracle)]

Hi,

It looks like patchwork didn't like this submission, which is no
different from any of my previous submissions:

https://patchwork.kernel.org/project/netdevbpf/list/?series=753590

It seems to have no patches associated with the series, yet it does
know that there are patches associated with the series:

https://patchwork.kernel.org/project/netdevbpf/cover/ZHoOe9K/dZuW2pOe@shell.armlinux.org.uk/
https://patchwork.kernel.org/project/netdevbpf/patch/E1q56xm-00BsuT-8I@rmk-PC.armlinux.org.uk/
https://patchwork.kernel.org/project/netdevbpf/patch/E1q56xr-00Bsua-Bg@rmk-PC.armlinux.org.uk/
https://patchwork.kernel.org/project/netdevbpf/patch/E1q56xw-00Bsug-Eb@rmk-PC.armlinux.org.uk/
... etc ...

each of which link back to the series. So there are back-links to the
series but no forward-links. Sounds like a bug in patchwork :(

Any ideas what went wrong with patchwork, and whether this is just a
one-off due to timings of emails hitting patchwork, or something more
fundamental?

On Fri, Jun 02, 2023 at 04:44:59PM +0100, Russell King (Oracle) wrote:
> Hi,
> 
> This series completes the mdio device lifetime handling for Lynx PCS
> users which do not create their own mdio device, but instead fetch
> it using a firmware description - namely the DPAA2 and FMAN_MEMAC
> drivers.
> 
> In a previous patch set, lynx_pcs_create() was modified to increase
> the mdio device refcount, and lynx_pcs_destroy() to drop that
> refcount.
> 
> The first two patches change these two drivers to put the reference
> which they hold immediately after lynx_pcs_create(), effectively
> handing the responsibility for maintaining the refcount to the Lynx
> PCS driver.
> 
> A side effect of the first two patches is that lynx_get_mdio_device()
> is no longer used, so patch 3 removes it.
> 
> Patch 4 adds a new helper - lynx_pcs_create_fwnode(), which creates
> a Lynx PCS instance from the fwnode.
> 
> Patch 5 and 6 convert the two drivers to make use of this new helper,
> which simply has to find the mdio device, and then create the Lynx
> PCS from that.
> 
> With those conversions done, lynx_pcs_create() is no longer required
> outside pcs-lynx.c, so remove it from public view.
> 
> Finally, in patch 8 we change lynx_pcs_create() to return an
> error-pointer rather than NULL to bring consistency to the return
> style, and means that we can remove the NULL-to-error-pointer
> conversion from both lynx_pcs_create_fwnode() and
> lynx_pcs_create_mdiodev().
> 
>  drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c | 21 ++++++-------
>  drivers/net/ethernet/freescale/fman/fman_memac.c | 18 +++--------
>  drivers/net/pcs/pcs-lynx.c                       | 40 ++++++++++++++----------
>  include/linux/pcs-lynx.h                         |  4 +--
>  4 files changed, 39 insertions(+), 44 deletions(-)
> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

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

[Russell King (Oracle)]

On Fri, Jun 02, 2023 at 11:51:23AM -0400, Sean Anderson wrote:
> 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.

Sorry, I totally missed your comment.

Yes, that would also fix the refcount leak in memac_pcs_create(). I
thought about that, but I decided against it because in dpaa2:

        if (!fwnode_device_is_available(node)) {
                netdev_err(mac->net_dev, "pcs-handle node not available\n");
                fwnode_handle_put(node);
                return -ENODEV;
        }

would become:

        if (IS_ERR(pcs)) {
                netdev_err(mac->net_dev,
                           "lynx_pcs_create_fwnode() failed: %pe\n", pcs);

If the device is not available, the error message changes from
	pcs-handle node not available
to
	lynx_pcs_create_fwnode() failed: ENODEV

which doesn't really say what the problem was. Is this something that
the DPAA2 maintainers care about?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!