diff mbox series

[net-next,19/19] net: usb: aqc111: Add support for wake on LAN by MAGIC packet

Message ID 1a7d4af1b3a6397b76338d543f86bb4c327f5060.1538734658.git.igor.russkikh@aquantia.com (mailing list archive)
State Superseded
Headers show
Series Add support for Aquantia AQtion USB to 5/2.5GbE devices | expand

Commit Message

Igor Russkikh Oct. 5, 2018, 10:25 a.m. UTC 
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 | 203 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/net/usb/aqc111.h |  11 +++
 2 files changed, 214 insertions(+)

Comments

Andrew Lunn Oct. 6, 2018, 5:49 p.m. UTC | #1 
> +		if (aqc111_data->dpa) {
> +			aqc111_set_phy_speed(dev, AUTONEG_DISABLE, SPEED_100);

I don't think that works. You should leave AUTONEG on, but only
advertise SPEED_100 and trigger auto-neg. If you force it to 100,
there is no guarantee the peer will figure out what the new link speed
is. I've often seen failed auto-net result in 10/Half. So you will
loose the link, making WoL pointless.

> +static int aqc111_resume(struct usb_interface *intf)
> +{
> +	struct usbnet *dev = usb_get_intfdata(intf);
> +	struct aqc111_data *aqc111_data = (struct aqc111_data *)dev->data[0];
> +	u8 reg8;
> +	u16 reg16;
> +
> +	netif_carrier_off(dev->net);
> +
> +	/* Power up ethernet PHY */
> +	aqc111_data->phy_ops.phy_power = 1;
> +	aqc111_data->phy_ops.low_power = 0;
> +	aqc111_data->phy_ops.wol = 0;
> +	if (aqc111_data->dpa) {
> +		aqc111_read_cmd_nopm(dev, AQ_PHY_POWER, 0, 0, 1, &reg8);
> +		if (reg8 == 0x00) {
> +			reg8 = 0x02;
> +			aqc111_write_cmd_nopm(dev, AQ_PHY_POWER, 0, 0,
> +					      1, &reg8);
> +			msleep(200);
> +		}
> +
> +		aq_mdio_read_cmd(dev, AQ_GLB_STD_CTRL_REG, AQ_PHY_GLOBAL_ADDR,
> +				 2, &reg16);
> +		if (reg16 & AQ_PHY_LOW_POWER_MODE) {
> +			reg16 &= ~AQ_PHY_LOW_POWER_MODE;
> +			aq_mdio_write_cmd(dev, AQ_GLB_STD_CTRL_REG,
> +					  AQ_PHY_GLOBAL_ADDR, 2, &reg16);
> +		}
> +	}
> +
> +	reg8 = 0xFF;
> +	aqc111_write_cmd_nopm(dev, AQ_ACCESS_MAC, SFR_BM_INT_MASK,
> +			      1, 1, &reg8);
> +	/* Configure RX control register => start operation */
> +	reg16 = aqc111_data->rxctl;
> +	reg16 &= ~SFR_RX_CTL_START;
> +	aqc111_write_cmd_nopm(dev, AQ_ACCESS_MAC, SFR_RX_CTL, 2, 2, &reg16);
> +
> +	reg16 |= SFR_RX_CTL_START;
> +	aqc111_write_cmd_nopm(dev, AQ_ACCESS_MAC, SFR_RX_CTL, 2, 2, &reg16);
> +
> +	aqc111_set_phy_speed(dev, aqc111_data->autoneg,
> +			     aqc111_data->advertised_speed);
> +

Should that be conditional on aqc111_data->dpa?

> +struct aqc111_wol_cfg {
> +	u8 hw_addr[6];
> +	u8 flags;
> +	u8 rsvd[283];
> +};

Do you really need these 283 bytes??

> +
> +#define WOL_CFG_SIZE sizeof(struct aqc111_wol_cfg)
> +
>  struct aqc111_data {
>  	u16 rxctl;
>  	u8 rx_checksum;
> @@ -228,6 +238,7 @@ struct aqc111_data {
>  	} fw_ver;
>  	u8 dpa; /*direct PHY access*/
>  	struct aqc111_phy_options phy_ops;
> +	struct aqc111_wol_cfg wol_cfg;

Those 283 bytes make this whole structure bigger...

      Andrew
Igor Russkikh Oct. 8, 2018, 2:12 p.m. UTC | #2 
Hi Andrew,

>> +		if (aqc111_data->dpa) {
>> +			aqc111_set_phy_speed(dev, AUTONEG_DISABLE, SPEED_100);
> 
> I don't think that works. You should leave AUTONEG on, but only
> advertise SPEED_100 and trigger auto-neg. If you force it to 100,
> there is no guarantee the peer will figure out what the new link speed
> is. I've often seen failed auto-net result in 10/Half. So you will
> loose the link, making WoL pointless.

Phy does not support 10M, low power mode explicitly uses 100M
for power safety reasons.

It is meaningless here to add Autoneg to 100M because thats the only
speedmask bit anyway.


>> +	aqc111_set_phy_speed(dev, aqc111_data->autoneg,
>> +			     aqc111_data->advertised_speed);
>> +
> 
> Should that be conditional on aqc111_data->dpa?

Actually no, because set_phy_speed internally checks this flag.

>> +	u8 rsvd[283];
>> +};
> 
> Do you really need these 283 bytes??

>>  	struct aqc111_phy_options phy_ops;
>> +	struct aqc111_wol_cfg wol_cfg;
> 
> Those 283 bytes make this whole structure bigger...

FW interface expects the WOL config request WOL_CFG_SIZE bytes.
These reserved fields are just not used now by linux driver.
They configure extra wol features like a sleep proxy.
Thus, we anyway have to allocate this somewhere.

Regards,
  Igor
Andrew Lunn Oct. 8, 2018, 2:47 p.m. UTC | #3 
On Mon, Oct 08, 2018 at 02:12:59PM +0000, Igor Russkikh wrote:
> Hi Andrew,
> 
> >> +		if (aqc111_data->dpa) {
> >> +			aqc111_set_phy_speed(dev, AUTONEG_DISABLE, SPEED_100);
> > 
> > I don't think that works. You should leave AUTONEG on, but only
> > advertise SPEED_100 and trigger auto-neg. If you force it to 100,
> > there is no guarantee the peer will figure out what the new link speed
> > is. I've often seen failed auto-net result in 10/Half. So you will
> > loose the link, making WoL pointless.
> 
> Phy does not support 10M, low power mode explicitly uses 100M
> for power safety reasons.
> 
> It is meaningless here to add Autoneg to 100M because thats the only
> speedmask bit anyway.

If you have AUTONEG_DISABLE, i would assume you PHY is not even trying
to auto_neg. So the speedmask is irrelevent, it is not sent to the
peer. And since the peer is not receiving any auto-neg information, it
will fail to auto-neg, and most likely default to 10/Half.

To do this right, please take a look at this commit

commit 2b9672ddb6f347467d7b33b86c5dfc4d5c0501a8
Author: Heiner Kallweit <hkallweit1@gmail.com>
Date:   Thu Jul 12 21:32:53 2018 +0200

    net: phy: add phy_speed_down and phy_speed_up
    
    Some network drivers include functionality to speed down the PHY when
    suspending and just waiting for a WoL packet because this saves energy.
    This functionality is quite generic, therefore let's factor it out to
    phylib.
    
> 
> >> +	aqc111_set_phy_speed(dev, aqc111_data->autoneg,
> >> +			     aqc111_data->advertised_speed);
> >> +
> > 
> > Should that be conditional on aqc111_data->dpa?
> 
> Actually no, because set_phy_speed internally checks this flag.

So you should probably remove the check above when forcing the speed
to 100. Make the code symmetrical. 

> 
> >> +	u8 rsvd[283];
> >> +};
> > 
> > Do you really need these 283 bytes??
> 
> >>  	struct aqc111_phy_options phy_ops;
> >> +	struct aqc111_wol_cfg wol_cfg;
> > 
> > Those 283 bytes make this whole structure bigger...
> 
> FW interface expects the WOL config request WOL_CFG_SIZE bytes.
> These reserved fields are just not used now by linux driver.
> They configure extra wol features like a sleep proxy.
> Thus, we anyway have to allocate this somewhere.

Well, i think your low level function for actually sending a command
does a dup before sending. You don't actually send this, you send a
copy. Maybe you can pad it out then?

      Andrew
diff mbox series

Patch

diff --git a/drivers/net/usb/aqc111.c b/drivers/net/usb/aqc111.c
index 20d4347ea3ad..e2ea8dc54d3a 100644
--- a/drivers/net/usb/aqc111.c
+++ b/drivers/net/usb/aqc111.c
@@ -192,6 +192,35 @@  static void aqc111_get_drvinfo(struct net_device *net,
 	info->regdump_len = 0x00;
 }
 
+static void aqc111_get_wol(struct net_device *net,
+			   struct ethtool_wolinfo *wolinfo)
+{
+	struct usbnet *dev = netdev_priv(net);
+	struct aqc111_data *aqc111_data = (struct aqc111_data *)dev->data[0];
+
+	wolinfo->supported = WAKE_MAGIC;
+	wolinfo->wolopts = 0;
+
+	if (aqc111_data->wol_cfg.flags & AQ_WOL_FLAG_MP)
+		wolinfo->wolopts |= WAKE_MAGIC;
+}
+
+static int aqc111_set_wol(struct net_device *net,
+			  struct ethtool_wolinfo *wolinfo)
+{
+	struct usbnet *dev = netdev_priv(net);
+	struct aqc111_data *aqc111_data = (struct aqc111_data *)dev->data[0];
+
+	if (wolinfo->wolopts & ~WAKE_MAGIC)
+		return -EINVAL;
+
+	aqc111_data->wol_cfg.flags = 0;
+	if (wolinfo->wolopts & WAKE_MAGIC)
+		aqc111_data->wol_cfg.flags |= AQ_WOL_FLAG_MP;
+
+	return 0;
+}
+
 static void aqc111_speed_to_link_mode(u32 speed,
 				      struct ethtool_link_ksettings *elk)
 {
@@ -441,6 +470,8 @@  static int aqc111_set_link_ksettings(struct net_device *net,
 
 static const struct ethtool_ops aqc111_ethtool_ops = {
 	.get_drvinfo = aqc111_get_drvinfo,
+	.get_wol = aqc111_get_wol,
+	.set_wol = aqc111_set_wol,
 	.get_msglevel = usbnet_get_msglevel,
 	.set_msglevel = usbnet_set_msglevel,
 	.get_link = ethtool_op_get_link,
@@ -1346,6 +1377,176 @@  static const struct driver_info aqc111_info = {
 	.tx_fixup	= aqc111_tx_fixup,
 };
 
+static int aqc111_suspend(struct usb_interface *intf, pm_message_t message)
+{
+	struct usbnet *dev = usb_get_intfdata(intf);
+	struct aqc111_data *aqc111_data = (struct aqc111_data *)dev->data[0];
+	u8 reg8;
+	u16 reg16;
+	u16 temp_rx_ctrl = 0x00;
+
+	usbnet_suspend(intf, message);
+
+	aqc111_read_cmd_nopm(dev, AQ_ACCESS_MAC, SFR_RX_CTL, 2, 2, &reg16);
+	temp_rx_ctrl = reg16;
+	/* Stop RX operations*/
+	reg16 &= ~SFR_RX_CTL_START;
+	aqc111_write_cmd_nopm(dev, AQ_ACCESS_MAC, SFR_RX_CTL,
+			      2, 2, &reg16);
+	/* Force bz */
+	aqc111_read_cmd_nopm(dev, AQ_ACCESS_MAC, SFR_PHYPWR_RSTCTL,
+			     2, 2, &reg16);
+	reg16 |= SFR_PHYPWR_RSTCTL_BZ;
+	aqc111_write_cmd_nopm(dev, AQ_ACCESS_MAC, SFR_PHYPWR_RSTCTL,
+			      2, 2, &reg16);
+
+	reg8 = SFR_BULK_OUT_EFF_EN;
+	aqc111_write_cmd_nopm(dev, AQ_ACCESS_MAC, SFR_BULK_OUT_CTRL,
+			      1, 1, &reg8);
+
+	temp_rx_ctrl &= ~(SFR_RX_CTL_START | SFR_RX_CTL_RF_WAK |
+			  SFR_RX_CTL_AP | SFR_RX_CTL_AM);
+	aqc111_write_cmd_nopm(dev, AQ_ACCESS_MAC, SFR_RX_CTL,
+			      2, 2, &temp_rx_ctrl);
+
+	reg8 = 0x00;
+	aqc111_write_cmd_nopm(dev, AQ_ACCESS_MAC, SFR_ETH_MAC_PATH,
+			      1, 1, &reg8);
+
+	if (aqc111_data->wol_cfg.flags) {
+		aqc111_data->phy_ops.wol = 1;
+		if (aqc111_data->dpa) {
+			reg8 = 0;
+			if (aqc111_data->wol_cfg.flags & AQ_WOL_FLAG_MP)
+				reg8 |= SFR_MONITOR_MODE_RWMP;
+			aqc111_write_cmd_nopm(dev, AQ_ACCESS_MAC,
+					      SFR_MONITOR_MODE, 1, 1, &reg8);
+		} else {
+			memcpy(aqc111_data->wol_cfg.hw_addr,
+			       dev->net->dev_addr, ETH_ALEN);
+		}
+
+		temp_rx_ctrl |= (SFR_RX_CTL_AB | SFR_RX_CTL_START);
+		aqc111_write_cmd_nopm(dev, AQ_ACCESS_MAC, SFR_RX_CTL,
+				      2, 2, &temp_rx_ctrl);
+		reg8 = 0x00;
+		aqc111_write_cmd_nopm(dev, AQ_ACCESS_MAC, SFR_BM_INT_MASK,
+				      1, 1, &reg8);
+		reg8 = SFR_BMRX_DMA_EN;
+		aqc111_write_cmd_nopm(dev, AQ_ACCESS_MAC, SFR_BMRX_DMA_CONTROL,
+				      1, 1, &reg8);
+		reg8 = SFR_RX_PATH_READY;
+		aqc111_write_cmd_nopm(dev, AQ_ACCESS_MAC, SFR_ETH_MAC_PATH,
+				      1, 1, &reg8);
+		reg8 = 0x07;
+		aqc111_write_cmd_nopm(dev, AQ_ACCESS_MAC, SFR_RX_BULKIN_QCTRL,
+				      1, 1, &reg8);
+		reg8 = 0x00;
+		aqc111_write_cmd_nopm(dev, AQ_ACCESS_MAC,
+				      SFR_RX_BULKIN_QTIMR_LOW, 1, 1, &reg8);
+		aqc111_write_cmd_nopm(dev, AQ_ACCESS_MAC,
+				      SFR_RX_BULKIN_QTIMR_HIGH, 1, 1, &reg8);
+		reg8 = 0xFF;
+		aqc111_write_cmd_nopm(dev, AQ_ACCESS_MAC, SFR_RX_BULKIN_QSIZE,
+				      1, 1, &reg8);
+		aqc111_write_cmd_nopm(dev, AQ_ACCESS_MAC, SFR_RX_BULKIN_QIFG,
+				      1, 1, &reg8);
+
+		aqc111_read_cmd_nopm(dev, AQ_ACCESS_MAC, SFR_MEDIUM_STATUS_MODE,
+				     2, 2, &reg16);
+		reg16 |= SFR_MEDIUM_RECEIVE_EN;
+		aqc111_write_cmd_nopm(dev, AQ_ACCESS_MAC,
+				      SFR_MEDIUM_STATUS_MODE, 2, 2, &reg16);
+
+		if (aqc111_data->dpa) {
+			aqc111_set_phy_speed(dev, AUTONEG_DISABLE, SPEED_100);
+		} else {
+			aqc111_write_cmd(dev, AQ_WOL_CFG, 0, 0,
+					 WOL_CFG_SIZE, &aqc111_data->wol_cfg);
+			aqc111_write_cmd(dev, AQ_PHY_OPS, 0, 0,
+					 4, &aqc111_data->phy_ops);
+		}
+	} else {
+		aqc111_data->phy_ops.low_power = 1;
+		if (!aqc111_data->dpa) {
+			aqc111_write_cmd(dev, AQ_PHY_OPS, 0, 0,
+					 4, &aqc111_data->phy_ops);
+		} else {
+			reg16 = AQ_PHY_LOW_POWER_MODE;
+			aq_mdio_write_cmd(dev, AQ_GLB_STD_CTRL_REG,
+					  AQ_PHY_GLOBAL_ADDR, 2, &reg16);
+		}
+
+		/* Disable RX path */
+		aqc111_read_cmd_nopm(dev, AQ_ACCESS_MAC,
+				     SFR_MEDIUM_STATUS_MODE, 2, 2, &reg16);
+		reg16 &= ~SFR_MEDIUM_RECEIVE_EN;
+		aqc111_write_cmd_nopm(dev, AQ_ACCESS_MAC,
+				      SFR_MEDIUM_STATUS_MODE, 2, 2, &reg16);
+	}
+
+	return 0;
+}
+
+static int aqc111_resume(struct usb_interface *intf)
+{
+	struct usbnet *dev = usb_get_intfdata(intf);
+	struct aqc111_data *aqc111_data = (struct aqc111_data *)dev->data[0];
+	u8 reg8;
+	u16 reg16;
+
+	netif_carrier_off(dev->net);
+
+	/* Power up ethernet PHY */
+	aqc111_data->phy_ops.phy_power = 1;
+	aqc111_data->phy_ops.low_power = 0;
+	aqc111_data->phy_ops.wol = 0;
+	if (aqc111_data->dpa) {
+		aqc111_read_cmd_nopm(dev, AQ_PHY_POWER, 0, 0, 1, &reg8);
+		if (reg8 == 0x00) {
+			reg8 = 0x02;
+			aqc111_write_cmd_nopm(dev, AQ_PHY_POWER, 0, 0,
+					      1, &reg8);
+			msleep(200);
+		}
+
+		aq_mdio_read_cmd(dev, AQ_GLB_STD_CTRL_REG, AQ_PHY_GLOBAL_ADDR,
+				 2, &reg16);
+		if (reg16 & AQ_PHY_LOW_POWER_MODE) {
+			reg16 &= ~AQ_PHY_LOW_POWER_MODE;
+			aq_mdio_write_cmd(dev, AQ_GLB_STD_CTRL_REG,
+					  AQ_PHY_GLOBAL_ADDR, 2, &reg16);
+		}
+	}
+
+	reg8 = 0xFF;
+	aqc111_write_cmd_nopm(dev, AQ_ACCESS_MAC, SFR_BM_INT_MASK,
+			      1, 1, &reg8);
+	/* Configure RX control register => start operation */
+	reg16 = aqc111_data->rxctl;
+	reg16 &= ~SFR_RX_CTL_START;
+	aqc111_write_cmd_nopm(dev, AQ_ACCESS_MAC, SFR_RX_CTL, 2, 2, &reg16);
+
+	reg16 |= SFR_RX_CTL_START;
+	aqc111_write_cmd_nopm(dev, AQ_ACCESS_MAC, SFR_RX_CTL, 2, 2, &reg16);
+
+	aqc111_set_phy_speed(dev, aqc111_data->autoneg,
+			     aqc111_data->advertised_speed);
+
+	aqc111_read_cmd_nopm(dev, AQ_ACCESS_MAC,
+			     SFR_MEDIUM_STATUS_MODE, 2, 2, &reg16);
+	reg16 |= SFR_MEDIUM_RECEIVE_EN;
+	aqc111_write_cmd_nopm(dev, AQ_ACCESS_MAC,
+			      SFR_MEDIUM_STATUS_MODE, 2, 2, &reg16);
+	reg8 = SFR_RX_PATH_READY;
+	aqc111_write_cmd_nopm(dev, AQ_ACCESS_MAC, SFR_ETH_MAC_PATH,
+			      1, 1, &reg8);
+	reg8 = 0x0;
+	aqc111_write_cmd(dev, AQ_ACCESS_MAC, SFR_BMRX_DMA_CONTROL, 1, 1, &reg8);
+
+	return usbnet_resume(intf);
+}
+
 #define AQC111_USB_ETH_DEV(vid, pid, table) \
 	.match_flags = USB_DEVICE_ID_MATCH_DEVICE | \
 			USB_DEVICE_ID_MATCH_INT_CLASS, \
@@ -1371,6 +1572,8 @@  static struct usb_driver aq_driver = {
 	.name		= "aqc111",
 	.id_table	= products,
 	.probe		= usbnet_probe,
+	.suspend	= aqc111_suspend,
+	.resume		= aqc111_resume,
 	.disconnect	= usbnet_disconnect,
 };
 
diff --git a/drivers/net/usb/aqc111.h b/drivers/net/usb/aqc111.h
index a7ccee225e93..9a900c325273 100644
--- a/drivers/net/usb/aqc111.h
+++ b/drivers/net/usb/aqc111.h
@@ -19,6 +19,7 @@ 
 #define AQ_FLASH_PARAMETERS		0x20
 #define AQ_PHY_POWER			0x31
 #define AQ_PHY_CMD			0x32
+#define AQ_WOL_CFG			0x60
 #define AQ_PHY_OPS			0x61
 
 #define AQC111_PHY_ID			0x00
@@ -169,6 +170,7 @@ 
 #define AQ_FW_VER_MINOR			0xDB
 #define AQ_FW_VER_REV			0xDC
 
+#define AQ_WOL_FLAG_MP			0x2
 /******************************************************************************/
 
 struct aqc111_phy_options {
@@ -214,6 +216,14 @@  struct aqc111_phy_options {
 	};
 };
 
+struct aqc111_wol_cfg {
+	u8 hw_addr[6];
+	u8 flags;
+	u8 rsvd[283];
+};
+
+#define WOL_CFG_SIZE sizeof(struct aqc111_wol_cfg)
+
 struct aqc111_data {
 	u16 rxctl;
 	u8 rx_checksum;
@@ -228,6 +238,7 @@  struct aqc111_data {
 	} fw_ver;
 	u8 dpa; /*direct PHY access*/
 	struct aqc111_phy_options phy_ops;
+	struct aqc111_wol_cfg wol_cfg;
 } __packed;
 
 struct aqc111_int_data {