Re: [PATCH v1 3/4] ARM: boards: skov-imx6: assigned separate MAC address to LAN2

[Sascha Hauer <sha@pengutronix.de>]

On Wed, Nov 02, 2022 at 11:07:43AM +0100, Oleksij Rempel wrote:
> We have two external ports and different MAC addresses on each port. So,
> assign different MAC to the LAN2. The address on LAN1 is the
> system default address.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  arch/arm/boards/skov-imx6/board.c | 47 ++++++++++++++++++++++++++++++-
>  1 file changed, 46 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boards/skov-imx6/board.c b/arch/arm/boards/skov-imx6/board.c
> index 3c51b76735..ab8b229a69 100644
> --- a/arch/arm/boards/skov-imx6/board.c
> +++ b/arch/arm/boards/skov-imx6/board.c
> @@ -17,6 +17,12 @@
>  
>  #include "version.h"
>  
> +struct skov_imx6_priv {
> +	struct device_d *dev;
> +};
> +
> +static struct skov_imx6_priv *skov_priv;
> +
>  static int eth_of_fixup_node(struct device_node *root, const char *node_path,
>  			     const u8 *ethaddr)
>  {
> @@ -542,6 +548,34 @@ static void skov_init_board(const struct board_description *variant)
>  	}
>  }
>  
> +/* */

Did you want to write a comment here?

> +static int skov_set_switch_lan2_mac(struct skov_imx6_priv *priv)
> +{
> +	const char *state = "/state/ethaddr/eth2";
> +	struct device_node *lan2_np;
> +	u8 ethaddr[ETH_ALEN];
> +	int ret;
> +
> +	ret = get_mac_address_from_env_variable("state.ethaddr.eth2", ethaddr);
> +	if (ret || !is_valid_ether_addr(ethaddr)) {
> +		ret = get_default_mac_address_from_state_node(state, ethaddr);
> +		if (ret || !is_valid_ether_addr(ethaddr)) {
> +			dev_err(priv->dev, "can't get MAC for LAN2\n");
> +			return -ENODEV;
> +		}
> +	}
> +
> +	lan2_np = of_find_node_by_path("/mdio/switch@0/ports/ports@1");
> +	if (!lan2_np) {
> +		dev_err(priv->dev, "LAN2 node not found\n");
> +		return -ENODEV;
> +	}
> +
> +	of_eth_register_ethaddr(lan2_np, ethaddr);
> +
> +	return 0;
> +}
> +
>  static int skov_switch_test(void)
>  {
>  	struct device_d *sw_dev;
> @@ -561,8 +595,11 @@ static int skov_switch_test(void)
>  		goto no_switch;
>  	}
>  
> -	if (dev_is_probed(sw_dev))
> +	if (dev_is_probed(sw_dev)) {
> +		skov_set_switch_lan2_mac(skov_priv);
> +		/* even if we fail, continue to boot as good as possible */
>  		return 0;
> +	}

This introduction of skov_priv to have a struct device_d * around for
dev_err is good, but is not subject to this patch. Please either make an
extra patch from it and use dev_err consistently or just stick to the
currently used pr_err.

>  
>  no_switch:
>  	skov_have_switch = false;
> @@ -584,6 +621,7 @@ late_initcall(skov_switch_test);
>  
>  static int skov_imx6_probe(struct device_d *dev)
>  {
> +	struct skov_imx6_priv *priv;
>  	unsigned v = 0;
>  	const struct board_description *variant;
>  
> @@ -603,6 +641,13 @@ static int skov_imx6_probe(struct device_d *dev)
>  
>  	skov_board_no = v;
>  
> +	priv = xzalloc(sizeof(*priv));
> +	if (!priv)
> +		return -ENOMEM;

xzalloc does not return with an error.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |