Re: [PATCH net-next 07/19] net: usb: aqc111: Add support for getting and setting of MAC address
From: Andrew Lunn <andrew@lunn.ch>
Date: 2018-10-06 01:03:46
Also in:
linux-usb
On Fri, Oct 05, 2018 at 10:24:58AM +0000, Igor Russkikh wrote:
quoted hunk
From: Dmitry Bezrukov <redacted> Signed-off-by: Dmitry Bezrukov <redacted> Signed-off-by: Igor Russkikh <redacted> --- drivers/net/usb/aqc111.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++ drivers/net/usb/aqc111.h | 1 + 2 files changed, 52 insertions(+)diff --git a/drivers/net/usb/aqc111.c b/drivers/net/usb/aqc111.c index 1d366f4a1c51..075f51cd04ab 100644 --- a/drivers/net/usb/aqc111.c +++ b/drivers/net/usb/aqc111.c@@ -11,6 +11,7 @@ #include <linux/netdevice.h> #include <linux/mii.h> #include <linux/usb.h> +#include <linux/if_vlan.h> #include <linux/usb/cdc.h> #include <linux/usb/usbnet.h>@@ -266,11 +267,46 @@ static void aqc111_set_phy_speed(struct usbnet *dev, u8 autoneg, u16 speed) aqc111_set_phy_speed_fw_iface(dev, aqc111_data); } +static int aqc111_set_mac_addr(struct net_device *net, void *p) +{ + struct usbnet *dev = netdev_priv(net); + struct sockaddr *addr = p; + + if (netif_running(net)) + return -EBUSY; + if (!is_valid_ether_addr(addr->sa_data)) + return -EADDRNOTAVAIL;
It is probably better to use eth_mac_addr().
+
+ memcpy(net->dev_addr, addr->sa_data, ETH_ALEN);
+
+ /* Set the MAC address */
+ return aqc111_write_cmd(dev, AQ_ACCESS_MAC, SFR_NODE_ID, ETH_ALEN,
+ ETH_ALEN, net->dev_addr);
+}
+
static const struct net_device_ops aqc111_netdev_ops = {
.ndo_open = usbnet_open,
.ndo_stop = usbnet_stop,
+ .ndo_set_mac_address = aqc111_set_mac_addr,
+ .ndo_validate_addr = eth_validate_addr,
};
+static int aqc111_get_mac(struct usbnet *dev, u8 *buf)
+{
+ int ret;
+
+ ret = aqc111_read_cmd(dev, AQ_FLASH_PARAMETERS, 0, 0, 6, buf);ETH_ALEN instead of 6?
+ if (ret < 0) + goto out; + + memcpy(dev->net->dev_addr, buf, ETH_ALEN); + memcpy(dev->net->perm_addr, dev->net->dev_addr, ETH_ALEN);
Is this really the permanent address? If i call aqc111_set_mac_addr() followed by aqc111_get_mac() i still get what is in the OTP EEPROM? +
quoted hunk
+ return 0; +out: + return ret; +} + static void aqc111_read_fw_version(struct usbnet *dev, struct aqc111_data *aqc111_data) {@@ -289,6 +325,7 @@ static void aqc111_read_fw_version(struct usbnet *dev, static int aqc111_bind(struct usbnet *dev, struct usb_interface *intf) { + u8 buf[6] = { 0 };
ETH_ALEN
quoted hunk
int ret; struct usb_device *udev = interface_to_usbdev(intf); struct aqc111_data *aqc111_data;@@ -316,6 +353,12 @@ static int aqc111_bind(struct usbnet *dev, struct usb_interface *intf) dev->data[0] = (unsigned long)aqc111_data; memset(aqc111_data, 0, sizeof(*aqc111_data)); + /* Get the MAC address */ + memset(buf, 0, ETH_ALEN);
You initialized it above as {0}. You don't need to memset it here.
+ ret = aqc111_get_mac(dev, buf);
Do you even need to zero it? If aqc111_get_mac() fails, it will be left undefined, but you fail the bind anyway.
quoted hunk
+ if (ret) + goto out; + dev->net->netdev_ops = &aqc111_netdev_ops; aqc111_read_fw_version(dev, aqc111_data);@@ -324,6 +367,10 @@ static int aqc111_bind(struct usbnet *dev, struct usb_interface *intf) SPEED_5000 : SPEED_1000; return 0; + +out: + kfree(aqc111_data); + return ret; }
Andrew