public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn•ch>
To: Igor Russkikh <Igor.Russkikh@aquantia•com>
Cc: "David S . Miller" <davem@davemloft•net>,
	"linux-usb@vger•kernel.org" <linux-usb@vger•kernel.org>,
	"netdev@vger•kernel.org" <netdev@vger•kernel.org>,
	Dmitry Bezrukov <Dmitry.Bezrukov@aquantia•com>
Subject: Re: [PATCH net-next 07/19] net: usb: aqc111: Add support for getting and setting of MAC address
Date: Sat, 6 Oct 2018 03:03:46 +0200	[thread overview]
Message-ID: <20181006010346.GA32455@lunn.ch> (raw)
In-Reply-To: <9ab691fa129ceb7a8947fde6f307fbd855d085cf.1538734658.git.igor.russkikh@aquantia.com>

On Fri, Oct 05, 2018 at 10:24:58AM +0000, Igor Russkikh wrote:
> From: Dmitry Bezrukov <dmitry.bezrukov@aquantia•com>
> 
> Signed-off-by: Dmitry Bezrukov <dmitry.bezrukov@aquantia•com>
> Signed-off-by: Igor Russkikh <igor.russkikh@aquantia•com>
> ---
>  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

  reply	other threads:[~2018-10-06  8:05 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-05 10:24 [PATCH net-next 00/19] Add support for Aquantia AQtion USB to 5/2.5GbE devices Igor Russkikh
2018-10-05 10:24 ` [PATCH net-next 01/19] net: usb: aqc111: Driver skeleton for Aquantia AQtion USB to 5GbE Igor Russkikh
2018-10-09 13:37   ` Bjørn Mork
2018-10-05 10:24 ` [PATCH net-next 02/19] net: usb: aqc111: Add bind and empty unbind callbacks Igor Russkikh
2018-10-05 17:39   ` David Miller
2018-10-05 10:24 ` [PATCH net-next 03/19] net: usb: aqc111: Add implementation of read and write commands Igor Russkikh
2018-10-05 17:40   ` David Miller
2018-10-08 13:44   ` Oliver Neukum
2018-10-09 13:33   ` Bjørn Mork
2018-10-05 10:24 ` [PATCH net-next 04/19] net: usb: aqc111: Various callbacks implementation Igor Russkikh
2018-10-08 13:47   ` Oliver Neukum
2018-10-09 13:27     ` Bjørn Mork
2018-10-10 11:33       ` Oliver Neukum
2018-10-05 10:24 ` [PATCH net-next 05/19] net: usb: aqc111: Introduce PHY access Igor Russkikh
2018-10-05 22:04   ` Andrew Lunn
2018-10-08  9:09     ` Igor Russkikh
2018-10-08 12:17       ` Andrew Lunn
2018-10-08 14:07         ` Igor Russkikh
2018-10-10  0:58       ` Andrew Lunn
2018-10-10  7:54         ` Igor Russkikh
2018-10-08 13:52   ` Oliver Neukum
2018-10-08 14:10     ` Igor Russkikh
2018-10-08 14:24       ` Oliver Neukum
2018-10-05 10:24 ` [PATCH net-next 06/19] net: usb: aqc111: Introduce link management Igor Russkikh
2018-10-05 17:11   ` Andrew Lunn
2018-10-08 13:22     ` Igor Russkikh
2018-10-05 17:46   ` David Miller
2018-10-06 17:35   ` Andrew Lunn
2018-10-08  9:29     ` Igor Russkikh
2018-10-08 12:12       ` Andrew Lunn
2018-10-08 13:59   ` Oliver Neukum
2018-10-05 10:24 ` [PATCH net-next 07/19] net: usb: aqc111: Add support for getting and setting of MAC address Igor Russkikh
2018-10-06  1:03   ` Andrew Lunn [this message]
2018-10-09 14:34     ` Igor Russkikh
2018-10-09 14:46       ` Andrew Lunn
2018-10-05 10:25 ` [PATCH net-next 08/19] net: usb: aqc111: Implement TX data path Igor Russkikh
2018-10-06  1:13   ` Andrew Lunn
2018-10-08 13:43     ` Igor Russkikh
2018-10-08 14:07       ` Oliver Neukum
2018-10-09 13:50   ` Bjørn Mork
2018-10-05 10:25 ` [PATCH net-next 09/19] net: usb: aqc111: Implement RX " Igor Russkikh
2018-10-06  1:18   ` Andrew Lunn
2018-10-09 13:39   ` Bjørn Mork
2018-10-05 10:25 ` [PATCH net-next 10/19] net: usb: aqc111: Add checksum offload support Igor Russkikh
2018-10-05 10:25 ` [PATCH net-next 11/19] net: usb: aqc111: Add support for changing MTU Igor Russkikh
2018-10-06 16:56   ` Andrew Lunn
2018-10-05 10:25 ` [PATCH net-next 12/19] net: usb: aqc111: Add support for enable/disable checksum offload Igor Russkikh
2018-10-05 10:25 ` [PATCH net-next 13/19] net: usb: aqc111: Add support for TSO Igor Russkikh
2018-10-08 14:12   ` Oliver Neukum
2018-10-05 10:25 ` [PATCH net-next 14/19] net: usb: aqc111: Implement set_rx_mode callback Igor Russkikh
2018-10-06 17:03   ` Andrew Lunn
2018-10-08 13:49     ` Igor Russkikh
2018-10-05 10:25 ` [PATCH net-next 15/19] net: usb: aqc111: Add support for VLAN_CTAG_TX/RX offload Igor Russkikh
2018-10-08 14:14   ` Oliver Neukum
2018-10-05 10:25 ` [PATCH net-next 16/19] net: usb: aqc111: Add RX VLAN filtering support Igor Russkikh
2018-10-06 17:05   ` Andrew Lunn
2018-10-05 10:25 ` [PATCH net-next 17/19] net: usb: aqc111: Initialize ethtool_ops structure Igor Russkikh
2018-10-06 17:08   ` Andrew Lunn
2018-10-05 10:25 ` [PATCH net-next 18/19] net: usb: aqc111: Implement get/set_link_ksettings callbacks Igor Russkikh
2018-10-06 17:38   ` Andrew Lunn
2018-10-08 14:18   ` Oliver Neukum
2018-10-05 10:25 ` [PATCH net-next 19/19] net: usb: aqc111: Add support for wake on LAN by MAGIC packet Igor Russkikh
2018-10-06 17:49   ` Andrew Lunn
2018-10-08 14:12     ` Igor Russkikh
2018-10-08 14:47       ` Andrew Lunn
2018-10-06 17:51 ` [PATCH net-next 00/19] Add support for Aquantia AQtion USB to 5/2.5GbE devices Andrew Lunn
2018-10-08  7:58   ` Igor Russkikh
2018-10-08 14:21 ` Oliver Neukum
2018-10-08 14:52   ` Igor Russkikh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181006010346.GA32455@lunn.ch \
    --to=andrew@lunn$(echo .)ch \
    --cc=Dmitry.Bezrukov@aquantia$(echo .)com \
    --cc=Igor.Russkikh@aquantia$(echo .)com \
    --cc=davem@davemloft$(echo .)net \
    --cc=linux-usb@vger$(echo .)kernel.org \
    --cc=netdev@vger$(echo .)kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox