[PATCHv2 net-next] net: xgbe: use device_get_mac_addr

[PATCHv2 net-next] net: xgbe: use device_get_mac_addr

[Rosen Penev]

device_get_mac_addr is basically device_property_read_u8_array with an
is_valid_ether_addr call. Allows just checking for ret.

Remove XGBE_MAC_ADDR_PROPERTY. device_get_mac_addr supports more
properties than just "mac-address".

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 v2: remove XGBE_MAC_ADDR_PROPERTY
 drivers/net/ethernet/amd/xgbe/xgbe-platform.c | 8 +++-----
 drivers/net/ethernet/amd/xgbe/xgbe.h          | 1 -
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-platform.c b/drivers/net/ethernet/amd/xgbe/xgbe-platform.c
index 47d53e59ccf6..f2db920fbaec 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-platform.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-platform.c
@@ -252,11 +252,9 @@ static int xgbe_platform_probe(struct platform_device *pdev)
 		dev_dbg(dev, "sir1_regs  = %p\n", pdata->sir1_regs);

 	/* Retrieve the MAC address */
-	ret = device_property_read_u8_array(dev, XGBE_MAC_ADDR_PROPERTY,
-					    pdata->mac_addr,
-					    sizeof(pdata->mac_addr));
-	if (ret || !is_valid_ether_addr(pdata->mac_addr)) {
-		dev_err(dev, "invalid %s property\n", XGBE_MAC_ADDR_PROPERTY);
+	ret = device_get_mac_address(dev, pdata->mac_addr);
+	if (ret) {
+		dev_err(dev, "invalid MAC address property\n");
 		if (!ret)
 			ret = -EINVAL;
 		goto err_io;
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe.h b/drivers/net/ethernet/amd/xgbe/xgbe.h
index 4333d269ee84..e57e416a1d5a 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe.h
+++ b/drivers/net/ethernet/amd/xgbe/xgbe.h
@@ -96,7 +96,6 @@
 	min_t(unsigned int, IEEE_8021QAZ_MAX_TCS, (_cnt))

 /* Common property names */
-#define XGBE_MAC_ADDR_PROPERTY	"mac-address"
 #define XGBE_PHY_MODE_PROPERTY	"phy-mode"
 #define XGBE_DMA_IRQS_PROPERTY	"amd,per-channel-interrupt"
 #define XGBE_SPEEDSET_PROPERTY	"amd,speed-set"
--
2.53.0

Re: [PATCHv2,net-next] net: xgbe: use device_get_mac_addr

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net: xgbe: use device_get_mac_addr

This commit refactors the MAC address retrieval code to use
device_get_mac_address() instead of device_property_read_u8_array() with
an explicit is_valid_ether_addr() check. It also removes the
XGBE_MAC_ADDR_PROPERTY definition since the new function supports multiple
MAC address properties.

> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-platform.c b/drivers/net/ethernet/amd/xgbe/xgbe-platform.c
> index 47d53e59ccf6..f2db920fbaec 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-platform.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-platform.c
> @@ -252,11 +252,9 @@ static int xgbe_platform_probe(struct platform_device *pdev)
>  		dev_dbg(dev, "sir1_regs  = %p\n", pdata->sir1_regs);
>
>  	/* Retrieve the MAC address */
> -	ret = device_property_read_u8_array(dev, XGBE_MAC_ADDR_PROPERTY,
> -					    pdata->mac_addr,
> -					    sizeof(pdata->mac_addr));
> -	if (ret || !is_valid_ether_addr(pdata->mac_addr)) {
> -		dev_err(dev, "invalid %s property\n", XGBE_MAC_ADDR_PROPERTY);
> +	ret = device_get_mac_address(dev, pdata->mac_addr);
> +	if (ret) {
> +		dev_err(dev, "invalid MAC address property\n");
>  		if (!ret)
                     ^^^^^^^^

Is this check reachable? When execution enters the 'if (ret)' block above,
ret is guaranteed to be non-zero, so the nested 'if (!ret)' condition can
never be true.

The old code needed 'if (!ret) ret = -EINVAL;' because
device_property_read_u8_array() could return 0 with an invalid MAC address
(checked by the !is_valid_ether_addr() part of the condition).

The new device_get_mac_address() already returns an error when the MAC
address is invalid, so this conversion is no longer needed and the
condition can never execute.

>  			ret = -EINVAL;
>  		goto err_io;

[ ... ]
-- 
pw-bot: cr