[net-next,08/19] net: usb: aqc111: Implement TX data path

[net-next,08/19] net: usb: aqc111: Implement TX data path

[Igor Russkikh]

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 | 73 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/net/usb/aqc111.h | 22 +++++++++++++++
 2 files changed, 95 insertions(+)

diff --git a/drivers/net/usb/aqc111.c b/drivers/net/usb/aqc111.c
index 075f51cd04ab..46832fd56f0a 100644
--- a/drivers/net/usb/aqc111.c
+++ b/drivers/net/usb/aqc111.c
@@ -287,6 +287,9 @@ static int aqc111_set_mac_addr(struct net_device *net, void *p)
 static const struct net_device_ops aqc111_netdev_ops = {
 	.ndo_open		= usbnet_open,
 	.ndo_stop		= usbnet_stop,
+	.ndo_start_xmit		= usbnet_start_xmit,
+	.ndo_tx_timeout		= usbnet_tx_timeout,
+	.ndo_get_stats64	= usbnet_get_stats64,
 	.ndo_set_mac_address	= aqc111_set_mac_addr,
 	.ndo_validate_addr	= eth_validate_addr,
 };
@@ -359,8 +362,18 @@ static int aqc111_bind(struct usbnet *dev, struct usb_interface *intf)
 	if (ret)
 		goto out;
 
+	/* Set TX needed headroom & tailroom */
+	dev->net->needed_headroom += AQ_TX_HEADER_SIZE;
+	dev->net->needed_tailroom += AQ_TX_HEADER_SIZE;
+
 	dev->net->netdev_ops = &aqc111_netdev_ops;
 
+	if (usb_device_no_sg_constraint(dev->udev))
+		dev->can_dma_sg = 1;
+
+	dev->net->hw_features |= AQ_SUPPORT_HW_FEATURE;
+	dev->net->features |= AQ_SUPPORT_FEATURE;
+
 	aqc111_read_fw_version(dev, aqc111_data);
 	aqc111_data->autoneg = AUTONEG_ENABLE;
 	aqc111_data->advertised_speed = (usb_speed == USB_SPEED_SUPER) ?
@@ -608,6 +621,12 @@ static int aqc111_reset(struct usbnet *dev)
 
 	usb_speed = dev->udev->speed;
 
+	if (usb_device_no_sg_constraint(dev->udev))
+		dev->can_dma_sg = 1;
+
+	dev->net->hw_features |= AQ_SUPPORT_HW_FEATURE;
+	dev->net->features |= AQ_SUPPORT_FEATURE;
+
 	/* Power up ethernet PHY */
 	aqc111_data->phy_ops.advertising = 0;
 	aqc111_data->phy_ops.phy_ctrl1 = 0;
@@ -689,6 +708,57 @@ static int aqc111_stop(struct usbnet *dev)
 	return 0;
 }
 
+static struct sk_buff *aqc111_tx_fixup(struct usbnet *dev, struct sk_buff *skb,
+				       gfp_t flags)
+{
+	struct aq_tx_packet_desc tx_hdr;
+	int frame_size = dev->maxpacket;
+	int headroom = 0;
+	int tailroom = 0;
+	int padding_size = 0;
+	struct sk_buff *new_skb = NULL;
+
+	memset(&tx_hdr, 0x00, sizeof(tx_hdr));
+
+	/*Length of actual data*/
+	tx_hdr.length = (skb->len & 0x1FFFFF);
+
+	headroom = (skb->len + AQ_TX_HEADER_SIZE) % 8;
+	if (headroom != 0)
+		padding_size = 8 - headroom;
+
+	if (((skb->len + AQ_TX_HEADER_SIZE + padding_size) % frame_size) == 0) {
+		padding_size += 8;
+		tx_hdr.drop_padding = 1;
+	}
+
+	if (!dev->can_dma_sg && (dev->net->features & NETIF_F_SG) &&
+	    skb_linearize(skb))
+		return NULL;
+
+	headroom = skb_headroom(skb);
+	tailroom = skb_tailroom(skb);
+
+	if (!(headroom >= AQ_TX_HEADER_SIZE && tailroom >= padding_size)) {
+		new_skb = skb_copy_expand(skb, AQ_TX_HEADER_SIZE,
+					  padding_size, flags);
+		dev_kfree_skb_any(skb);
+		skb = new_skb;
+		if (!skb)
+			return NULL;
+	}
+	if (padding_size != 0)
+		skb_put(skb, padding_size);
+	/* Copy TX header */
+	skb_push(skb, AQ_TX_HEADER_SIZE);
+	cpu_to_le64s(&tx_hdr);
+	skb_copy_to_linear_data(skb, &tx_hdr, 8);
+
+	usbnet_set_skb_tx_stats(skb, 1, 0);
+
+	return skb;
+}
+
 static const struct driver_info aqc111_info = {
 	.description	= "Aquantia AQtion USB to 5GbE Controller",
 	.bind		= aqc111_bind,
@@ -697,6 +767,9 @@ static const struct driver_info aqc111_info = {
 	.link_reset	= aqc111_link_reset,
 	.reset		= aqc111_reset,
 	.stop		= aqc111_stop,
+	.flags		= FLAG_ETHER | FLAG_FRAMING_AX |
+			  FLAG_AVOID_UNLINK_URBS | FLAG_MULTI_PACKET,
+	.tx_fixup	= aqc111_tx_fixup,
 };
 
 #define AQC111_USB_ETH_DEV(vid, pid, table) \
diff --git a/drivers/net/usb/aqc111.h b/drivers/net/usb/aqc111.h
index 7ca5c5a6ec82..049012e26b26 100644
--- a/drivers/net/usb/aqc111.h
+++ b/drivers/net/usb/aqc111.h
@@ -60,6 +60,10 @@
 #define AQ_USB_PHY_SET_TIMEOUT		10000
 #define AQ_USB_SET_TIMEOUT		4000
 
+/* Feature. ********************************************/
+#define AQ_SUPPORT_FEATURE	(NETIF_F_SG)
+#define AQ_SUPPORT_HW_FEATURE	(NETIF_F_SG)
+
 /* SFR Reg. ********************************************/
 
 #define SFR_GENERAL_STATUS		0x03
@@ -224,6 +228,24 @@ struct aqc111_int_data {
 #define AQ_INT_SPEED_1G		0x11
 #define AQ_INT_SPEED_100M	0x13
 
+struct aq_tx_packet_desc {
+	struct {
+		u32 length:21;
+		u32 checksum:7;
+		u32 drop_padding:1;
+		u32 vlan_tag:1;
+		u32 cphi:1;
+		u32 dicf:1;
+	};
+	struct {
+		u32 max_seg_size:15;
+		u32 reserved:1;
+		u32 vlan_info:16;
+	};
+};
+
+#define AQ_TX_HEADER_SIZE sizeof(struct aq_tx_packet_desc)
+
 static struct {
 	unsigned char ctrl;
 	unsigned char timer_l;

[net-next,08/19] net: usb: aqc111: Implement TX data path

[Andrew Lunn]

> +static struct sk_buff *aqc111_tx_fixup(struct usbnet *dev, struct sk_buff *skb,
> +				       gfp_t flags)
> +{
> +	struct aq_tx_packet_desc tx_hdr;
> +	int frame_size = dev->maxpacket;
> +	int headroom = 0;
> +	int tailroom = 0;
> +	int padding_size = 0;
> +	struct sk_buff *new_skb = NULL;
> +
> +	memset(&tx_hdr, 0x00, sizeof(tx_hdr));
> +
> +	/*Length of actual data*/
> +	tx_hdr.length = (skb->len & 0x1FFFFF);
> +
> +	headroom = (skb->len + AQ_TX_HEADER_SIZE) % 8;
> +	if (headroom != 0)
> +		padding_size = 8 - headroom;
> +
> +	if (((skb->len + AQ_TX_HEADER_SIZE + padding_size) % frame_size) == 0) {
> +		padding_size += 8;
> +		tx_hdr.drop_padding = 1;
> +	}
> +
> +	if (!dev->can_dma_sg && (dev->net->features & NETIF_F_SG) &&
> +	    skb_linearize(skb))
> +		return NULL;
> +
> +	headroom = skb_headroom(skb);
> +	tailroom = skb_tailroom(skb);
> +
> +	if (!(headroom >= AQ_TX_HEADER_SIZE && tailroom >= padding_size)) {
> +		new_skb = skb_copy_expand(skb, AQ_TX_HEADER_SIZE,
> +					  padding_size, flags);
> +		dev_kfree_skb_any(skb);
> +		skb = new_skb;
> +		if (!skb)
> +			return NULL;
> +	}
> +	if (padding_size != 0)
> +		skb_put(skb, padding_size);
> +	/* Copy TX header */
> +	skb_push(skb, AQ_TX_HEADER_SIZE);
> +	cpu_to_le64s(&tx_hdr);

Is that portable? tx_hdr is a structure of 2x u32 bitfields.  What
endian have you tested that one?

> +	skb_copy_to_linear_data(skb, &tx_hdr, 8);
> +
> +	usbnet_set_skb_tx_stats(skb, 1, 0);
> +
> +	return skb;
> +}

> +struct aq_tx_packet_desc {
> +	struct {
> +		u32 length:21;
> +		u32 checksum:7;
> +		u32 drop_padding:1;
> +		u32 vlan_tag:1;
> +		u32 cphi:1;
> +		u32 dicf:1;
> +	};
> +	struct {
> +		u32 max_seg_size:15;
> +		u32 reserved:1;
> +		u32 vlan_info:16;
> +	};
> +};

  Andrew

[net-next,08/19] net: usb: aqc111: Implement TX data path

[Igor Russkikh]

>> +	skb_push(skb, AQ_TX_HEADER_SIZE);
>> +	cpu_to_le64s(&tx_hdr);
> 
> Is that portable? tx_hdr is a structure of 2x u32 bitfields.  What
> endian have you tested that one?
> 

You are right, this is wrong for BE hardware.

We don't have such a hardware to check unfortunately.
Think its better to drop endianess conversions and declare
the driver as little endian only.

Do you think that'll be acceptable?

Regards,
   Igor

[net-next,08/19] net: usb: aqc111: Implement TX data path

[Oliver Neukum]

On Mo, 2018-10-08 at 13:43 +0000, Igor Russkikh wrote:
> > > +	skb_push(skb, AQ_TX_HEADER_SIZE);
> > > +	cpu_to_le64s(&tx_hdr);
> > 
> > Is that portable? tx_hdr is a structure of 2x u32 bitfields.  What
> > endian have you tested that one?
> > 
> 
> You are right, this is wrong for BE hardware.
> 
> We don't have such a hardware to check unfortunately.
> Think its better to drop endianess conversions and declare
> the driver as little endian only.
> 
> Do you think that'll be acceptable?

No. If worse comes to worse define it u64 and set the values
manually.

	Regards
		Oliver

[net-next,08/19] net: usb: aqc111: Implement TX data path

[Bjørn Mork]

Igor Russkikh <Igor.Russkikh@aquantia.com> writes:

> +struct aq_tx_packet_desc {
> +	struct {
> +		u32 length:21;
> +		u32 checksum:7;
> +		u32 drop_padding:1;
> +		u32 vlan_tag:1;
> +		u32 cphi:1;
> +		u32 dicf:1;
> +	};
> +	struct {
> +		u32 max_seg_size:15;
> +		u32 reserved:1;
> +		u32 vlan_info:16;
> +	};
> +};


You might want to shift and mask instead to avoid going insane when
trying to use this header on a BE system...


Bjørn