[PATCH net-next] net: mvpp2: Defer probe if MAC address source is not yet ready

[PATCH net-next] net: mvpp2: Defer probe if MAC address source is not yet ready

[Miquel Raynal]

NVMEM layouts are no longer registered early, and thus may not yet be
available when Ethernet drivers (or any other consumer) probe, leading
to possible probe deferrals errors. Forward the error code if this
happens. All other errors being discarded, the driver will eventually
use a random MAC address if no other source was considered valid (no
functional change on this regard).

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 24 ++++++++++++-------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 9b4ecbe4f36d..e7c7652ffac5 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -6081,18 +6081,19 @@ static bool mvpp2_port_has_irqs(struct mvpp2 *priv,
 	return true;
 }
 
-static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
-				     struct fwnode_handle *fwnode,
-				     char **mac_from)
+static int mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
+				    struct fwnode_handle *fwnode,
+				    char **mac_from)
 {
 	struct mvpp2_port *port = netdev_priv(dev);
 	char hw_mac_addr[ETH_ALEN] = {0};
 	char fw_mac_addr[ETH_ALEN];
+	int ret;
 
 	if (!fwnode_get_mac_address(fwnode, fw_mac_addr)) {
 		*mac_from = "firmware node";
 		eth_hw_addr_set(dev, fw_mac_addr);
-		return;
+		return 0;
 	}
 
 	if (priv->hw_version == MVPP21) {
@@ -6100,19 +6101,24 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
 		if (is_valid_ether_addr(hw_mac_addr)) {
 			*mac_from = "hardware";
 			eth_hw_addr_set(dev, hw_mac_addr);
-			return;
+			return 0;
 		}
 	}
 
 	/* Only valid on OF enabled platforms */
-	if (!of_get_mac_address_nvmem(to_of_node(fwnode), fw_mac_addr)) {
+	ret = of_get_mac_address_nvmem(to_of_node(fwnode), fw_mac_addr);
+	if (ret == -EPROBE_DEFER)
+		return ret;
+	if (!ret) {
 		*mac_from = "nvmem cell";
 		eth_hw_addr_set(dev, fw_mac_addr);
-		return;
+		return 0;
 	}
 
 	*mac_from = "random";
 	eth_hw_addr_random(dev);
+
+	return 0;
 }
 
 static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
@@ -6815,7 +6821,9 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 	mutex_init(&port->gather_stats_lock);
 	INIT_DELAYED_WORK(&port->stats_work, mvpp2_gather_hw_statistics);
 
-	mvpp2_port_copy_mac_addr(dev, priv, port_fwnode, &mac_from);
+	err = mvpp2_port_copy_mac_addr(dev, priv, port_fwnode, &mac_from);
+	if (err < 0)
+		goto err_free_stats;
 
 	port->tx_ring_size = MVPP2_MAX_TXD_DFLT;
 	port->rx_ring_size = MVPP2_MAX_RXD_DFLT;
-- 
2.34.1

Re: [PATCH net-next] net: mvpp2: Defer probe if MAC address source is not yet ready

[Marcin Wojtas]

Hi,


wt., 7 mar 2023 o 11:29 Miquel Raynal <miquel.raynal@bootlin.com> napisał(a):
>
> NVMEM layouts are no longer registered early, and thus may not yet be
> available when Ethernet drivers (or any other consumer) probe, leading
> to possible probe deferrals errors. Forward the error code if this
> happens. All other errors being discarded, the driver will eventually
> use a random MAC address if no other source was considered valid (no
> functional change on this regard).
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 24 ++++++++++++-------
>  1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index 9b4ecbe4f36d..e7c7652ffac5 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> @@ -6081,18 +6081,19 @@ static bool mvpp2_port_has_irqs(struct mvpp2 *priv,
>         return true;
>  }
>
> -static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
> -                                    struct fwnode_handle *fwnode,
> -                                    char **mac_from)
> +static int mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
> +                                   struct fwnode_handle *fwnode,
> +                                   char **mac_from)
>  {
>         struct mvpp2_port *port = netdev_priv(dev);
>         char hw_mac_addr[ETH_ALEN] = {0};
>         char fw_mac_addr[ETH_ALEN];
> +       int ret;
>
>         if (!fwnode_get_mac_address(fwnode, fw_mac_addr)) {
>                 *mac_from = "firmware node";
>                 eth_hw_addr_set(dev, fw_mac_addr);
> -               return;
> +               return 0;
>         }
>
>         if (priv->hw_version == MVPP21) {
> @@ -6100,19 +6101,24 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
>                 if (is_valid_ether_addr(hw_mac_addr)) {
>                         *mac_from = "hardware";
>                         eth_hw_addr_set(dev, hw_mac_addr);
> -                       return;
> +                       return 0;
>                 }
>         }
>
>         /* Only valid on OF enabled platforms */
> -       if (!of_get_mac_address_nvmem(to_of_node(fwnode), fw_mac_addr)) {
> +       ret = of_get_mac_address_nvmem(to_of_node(fwnode), fw_mac_addr);
> +       if (ret == -EPROBE_DEFER)
> +               return ret;
> +       if (!ret) {
>                 *mac_from = "nvmem cell";
>                 eth_hw_addr_set(dev, fw_mac_addr);
> -               return;
> +               return 0;
>         }
>
>         *mac_from = "random";
>         eth_hw_addr_random(dev);
> +
> +       return 0;
>  }
>
>  static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
> @@ -6815,7 +6821,9 @@ static int mvpp2_port_probe(struct platform_device *pdev,
>         mutex_init(&port->gather_stats_lock);
>         INIT_DELAYED_WORK(&port->stats_work, mvpp2_gather_hw_statistics);
>
> -       mvpp2_port_copy_mac_addr(dev, priv, port_fwnode, &mac_from);
> +       err = mvpp2_port_copy_mac_addr(dev, priv, port_fwnode, &mac_from);
> +       if (err < 0)
> +               goto err_free_stats;
>
>         port->tx_ring_size = MVPP2_MAX_TXD_DFLT;
>         port->rx_ring_size = MVPP2_MAX_RXD_DFLT;

LGTM.

Reviewed-by: Marcin Wojtas <mw@semihalf.com>

Thanks,
Marcin

Re: [PATCH net-next] net: mvpp2: Defer probe if MAC address source is not yet ready

[Paolo Abeni]

On Tue, 2023-03-07 at 20:29 +0100, Miquel Raynal wrote:
> NVMEM layouts are no longer registered early, and thus may not yet be
> available when Ethernet drivers (or any other consumer) probe, leading
> to possible probe deferrals errors. Forward the error code if this
> happens. All other errors being discarded, the driver will eventually
> use a random MAC address if no other source was considered valid (no
> functional change on this regard).
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

The patch LGTM, but if feels like a fix more than a new feature re-
factor. Any special reason to target the net-next tree?

Thanks!

Paolo

Re: [PATCH net-next] net: mvpp2: Defer probe if MAC address source is not yet ready

[Miquel Raynal]

Hi Paolo,

pabeni@redhat.com wrote on Thu, 09 Mar 2023 11:12:18 +0100:

> On Tue, 2023-03-07 at 20:29 +0100, Miquel Raynal wrote:
> > NVMEM layouts are no longer registered early, and thus may not yet be
> > available when Ethernet drivers (or any other consumer) probe, leading
> > to possible probe deferrals errors. Forward the error code if this
> > happens. All other errors being discarded, the driver will eventually
> > use a random MAC address if no other source was considered valid (no
> > functional change on this regard).
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>  
> 
> The patch LGTM, but if feels like a fix more than a new feature re-
> factor. Any special reason to target the net-next tree?

That's a good question, right now nvmem support can only be built-in,
there is no EPROBE_DEFER situation that can arise when reading from an
nvmem device. The introduction of nvmem layouts has been reported due
to their lack of "modularization". Support is being upstreamed right
now in the nvmem tree and this brings two subtleties in one: nvmem cell
reads can now return -EPROBE_DEFER when coming from layouts because 
they are no longer populated very early in the boot process.

Hence IMHO this patch does not "fix" anything as there is currently
nothing broken in 6.3. But as layouts and modules get in the Linux
kernel (6.4-final), users of these layouts (like this driver) should
also handle this new case. Having this patch be merged into linux-next
after the introduction of the nvmem changes has almost no impact. It
just slightly delays the moment in time when MAC addresses can be
retrieved from specific nvmem layouts.

IOW, I believe targeting the net-next tree is fine, but feel free to
take it into net if you prefer.

Thanks,
Miquèl

Re: [PATCH net-next] net: mvpp2: Defer probe if MAC address source is not yet ready

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue,  7 Mar 2023 20:29:27 +0100 you wrote:
> NVMEM layouts are no longer registered early, and thus may not yet be
> available when Ethernet drivers (or any other consumer) probe, leading
> to possible probe deferrals errors. Forward the error code if this
> happens. All other errors being discarded, the driver will eventually
> use a random MAC address if no other source was considered valid (no
> functional change on this regard).
> 
> [...]

Here is the summary with links:
  - [net-next] net: mvpp2: Defer probe if MAC address source is not yet ready
    https://git.kernel.org/netdev/net-next/c/cc4342f60f1a

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html