On Fri, Oct 05, 2018 at 10:24:58AM +0000, Igor Russkikh wrote:
> From: Dmitry Bezrukov <[email protected]>
>
> Signed-off-by: Dmitry Bezrukov <[email protected]>
> Signed-off-by: Igor Russkikh <[email protected]>
> ---
> 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?
+
> + 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
> 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.
> + 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
