| Message ID | 20230307192927.512757-1-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | cc4342f60f1a6d0f4a30ae1887a75834d0109444 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: mvpp2: Defer probe if MAC address source is not yet ready | expand |
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
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
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
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!
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;
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(-)