[OpenWrt-Devel] [PATCH] ath79: use downstream ag71xx for Kernel 5.4

[Bjørn Mork]

David Bauer <mail at david-bauer.net> writes:

> --- a/target/linux/ath79/files-4.19/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c
> +++ b/target/linux/ath79/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c
> @@ -1581,7 +1581,7 @@ static int ag71xx_probe(struct platform_device *pdev)
>  	ag->stop_desc->next = (u32) ag->stop_desc_dma;
>  
>  	mac_addr = of_get_mac_address(np);
> -	if (mac_addr)
> +	if (mac_addr && !IS_ERR(mac_addr))

bikeshedding...  But how about

   if (!IS_ERR_OR_NULL(mac_addr))

?


>  		memcpy(dev->dev_addr, mac_addr, ETH_ALEN);
>  	if (!mac_addr || !is_valid_ether_addr(dev->dev_addr)) {
>  		dev_err(&pdev->dev, "invalid MAC address, using random address\n");


This looks odd to me.  If we needed that !mac_addr test before, then
it will probably have to change to !IS_ERR_OR_NULL as well.

But do we really need to test mac_addr again?  dev was allocated by
devm_alloc_etherdev() a few lines up, so dev->dev_addr is guaranteed to
be all zeros if of_get_mac_address() failed.  Checking for
!is_valid_ether_addr(dev->dev_addr) should be more than sufficient here.

Micro optimizing for the !mac_addr case does not make any sense here
IMHO.


Bjørn

_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel