Re: [PATCH net-next] net: airoha: configure hw mac address according to the port id

[Jakub Kicinski <kuba@kernel.org>]

On Mon, 19 Aug 2024 13:10:09 +0200 Lorenzo Bianconi wrote:
> GDM1 port on EN7581 SoC is connected to the lan dsa switch.
> GDM{2,3,4} can be used as wan port connected to an external
> phy module. Configure hw mac address registers according to the port id.
> 
> ---
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

nit: sign-off under ---

>  drivers/net/ethernet/mediatek/airoha_eth.c | 33 +++++++++++++++++++++++-------
>  1 file changed, 26 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mediatek/airoha_eth.c b/drivers/net/ethernet/mediatek/airoha_eth.c
> index 1fb46db0c1e9..4914565c2fac 100644
> --- a/drivers/net/ethernet/mediatek/airoha_eth.c
> +++ b/drivers/net/ethernet/mediatek/airoha_eth.c
> @@ -67,6 +67,10 @@
>  #define FE_RST_GDM3_MBI_ARB_MASK	BIT(2)
>  #define FE_RST_CORE_MASK		BIT(0)
>  
> +#define REG_FE_WAN_MAC_H		0x0030
> +#define REG_FE_WAN_MAC_LMIN		0x0034
> +#define REG_FE_WAN_MAC_LMAX		0x0038
> +
>  #define REG_FE_LAN_MAC_H		0x0040
>  #define REG_FE_LAN_MAC_LMIN		0x0044
>  #define REG_FE_LAN_MAC_LMAX		0x0048

Isn't it better to define the base address and offsets?

#define REG_FE_MAC_BASE_WAN	0x0030
#define REG_FE_MAC_BASE_LAN	0x0040
#define REG_FE_MAC_OFF_H		0x00
#define REG_FE_MAC_OFF_LMIN		0x04
#define REG_FE_MAC_OFF_LMAX		0x08

Then you only need to select the base and the rest of the code won't
have conditionals (marginally improving readability)
-- 
pw-bot: cr