diff mbox series

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

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

Comments

Andrew Lunn Oct. 6, 2018, 1:13 a.m. UTC | #1 
> +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
Igor Russkikh Oct. 8, 2018, 1:43 p.m. UTC | #2 
>> +	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
Oliver Neukum Oct. 8, 2018, 2:07 p.m. UTC | #3 
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
Bjørn Mork Oct. 9, 2018, 1:50 p.m. UTC | #4 
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
diff mbox series

Patch

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;