David Bauer <m...@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@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
