On Mon, Mar 25, 2024 at 06:31:50PM +0100, Jose Ignacio Tornos Martinez wrote:
> After the commit d2689b6a86b9 ("net: usb: ax88179_178a: avoid two
> consecutive device resets"), reset is not executed from bind operation and
> mac address is not read from the device registers or the devicetree at that
> moment. Since the check to configure if the assigned mac address is random
> or not for the interface, happens after the bind operation from
> usbnet_probe, the interface keeps configured as random address, although the
> address is correctly read and set during open operation (the only reset
> now).
>
> In order to keep only one reset for the device and to avoid the interface
> always configured as random address, after reset, configure correctly the
> suitable field from the driver, if the mac address is read successfully from
> the device registers or the devicetree.
Thanks Jose,
The above makes sense to me and I agree with your fix and
corresponding Fixes tag.
> In addition, if mac address can not be read from the driver, a random
> address is configured again, so it is not necessary to call
> eth_hw_addr_random from here. Indeed, in this situtatuon, when reset was
> also executed from bind, this was invalidating the check to configure if the
> assigned mac address for the interface was random or not.
I also agree with your analysis here. However it does seem to be a separate
problem. And perhaps warrants a separate patch. I am also wondering
if this is more of a clean-up than a fix: does it cause a bug
that is observable by users?
> cc: stable@xxxxxxxxxxxxxxx # 6.6+
> Fixes: d2689b6a86b9 ("net: usb: ax88179_178a: avoid two consecutive device resets")
> Reported-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>
> Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@xxxxxxxxxx>
> ---
> drivers/net/usb/ax88179_178a.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
> index 88e084534853..d2324cc02461 100644
> --- a/drivers/net/usb/ax88179_178a.c
> +++ b/drivers/net/usb/ax88179_178a.c
> @@ -1273,10 +1273,9 @@ static void ax88179_get_mac_addr(struct usbnet *dev)
>
> if (is_valid_ether_addr(mac)) {
> eth_hw_addr_set(dev->net, mac);
> - } else {
> + dev->net->addr_assign_type = NET_ADDR_PERM;
> + } else
> netdev_info(dev->net, "invalid MAC address, using random\n");
> - eth_hw_addr_random(dev->net);
> - }
nit: AFAIK, if one arm of a conditional has curly-brackets, then all should.
So there is no need to drop them here.
>
> ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_NODE_ID, ETH_ALEN, ETH_ALEN,
> dev->net->dev_addr);
> --
> 2.44.0
>
>
