[PATCH 1/1] AX88179_178A: Enable the hardware pseudo header in case of the NET_IP

public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed

* [PATCH 1/1] AX88179_178A: Enable the hardware pseudo header in case of the NET_IP_ALIGN equals 0
@ 2013-12-06  9:58 freddy
       [not found] ` <1386323898-2246-1-git-send-email-freddy-knRN6Y/kmf1NUHwG+Fw1Kw@public.gmane.org>
  0 siblings, 1 reply; 5+ messages in thread
From: freddy @ 2013-12-06  9:58 UTC (permalink / raw)
  To: davem, linux-usb, linux-kernel, netdev, louis, allan; +Cc: Freddy Xin

From: Freddy Xin <freddy@asix•com.tw>

The AX88179_178A has a hardware feature that it can insert a 2-bytes pseudo
header in front of each received frame by setting the AX_RX_CTL_IPE bit.
This feature is used to let the IP header be aligned on a doubleword-aligned address,
but the NET_IP_ALIGN may equals to 2 and the __netdev_alloc_skb_ip_align in USBNET will
reserve 2 bytes also, so in this case the driver shouldn't enable this bit.

This patch modifies the driver to set AX_RX_CTL_IPE just in case of the NET_IP_ALIGN equals 0.

Signed-off-by: Freddy Xin <freddy@asix•com.tw>
---
 drivers/net/usb/ax88179_178a.c | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index 8e8d0fc..d84f0e0 100644
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -473,8 +473,10 @@ static int ax88179_resume(struct usb_interface *intf)
 	msleep(100);
 
 	/* Configure RX control register => start operation */
-	tmp16 = AX_RX_CTL_DROPCRCERR | AX_RX_CTL_IPE | AX_RX_CTL_START |
-		AX_RX_CTL_AP | AX_RX_CTL_AMALL | AX_RX_CTL_AB;
+	tmp16 = AX_RX_CTL_DROPCRCERR | AX_RX_CTL_START | AX_RX_CTL_AP |
+		AX_RX_CTL_AMALL | AX_RX_CTL_AB;
+	if (NET_IP_ALIGN == 0)
+		tmp16 |= AX_RX_CTL_IPE;
 	ax88179_write_cmd_nopm(dev, AX_ACCESS_MAC, AX_RX_CTL, 2, 2, &tmp16);
 
 	return usbnet_resume(intf);
@@ -599,7 +601,9 @@ static void ax88179_set_multicast(struct net_device *net)
 	struct ax88179_data *data = (struct ax88179_data *)dev->data;
 	u8 *m_filter = ((u8 *)dev->data) + 12;
 
-	data->rxctl = (AX_RX_CTL_START | AX_RX_CTL_AB | AX_RX_CTL_IPE);
+	data->rxctl = (AX_RX_CTL_START | AX_RX_CTL_AB);
+	if (NET_IP_ALIGN == 0)
+		data->rxctl |= AX_RX_CTL_IPE;
 
 	if (net->flags & IFF_PROMISC) {
 		data->rxctl |= AX_RX_CTL_PRO;
@@ -1054,8 +1058,10 @@ static int ax88179_bind(struct usbnet *dev, struct usb_interface *intf)
 	ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_TXCOE_CTL, 1, 1, tmp);
 
 	/* Configure RX control register => start operation */
-	*tmp16 = AX_RX_CTL_DROPCRCERR | AX_RX_CTL_IPE | AX_RX_CTL_START |
-		 AX_RX_CTL_AP | AX_RX_CTL_AMALL | AX_RX_CTL_AB;
+	*tmp16 = AX_RX_CTL_DROPCRCERR | AX_RX_CTL_START | AX_RX_CTL_AP |
+		 AX_RX_CTL_AMALL | AX_RX_CTL_AB;
+	if (NET_IP_ALIGN == 0)
+		*tmp16 |= AX_RX_CTL_IPE;
 	ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_RX_CTL, 2, 2, tmp16);
 
 	*tmp = AX_MONITOR_MODE_PMETYPE | AX_MONITOR_MODE_PMEPOL |
@@ -1143,7 +1149,8 @@ static int ax88179_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 
 		if (pkt_cnt == 0) {
 			/* Skip IP alignment psudo header */
-			skb_pull(skb, 2);
+			if (NET_IP_ALIGN == 0)
+				skb_pull(skb, 2);
 			skb->len = pkt_len;
 			skb_set_tail_pointer(skb, pkt_len);
 			skb->truesize = pkt_len + sizeof(struct sk_buff);
@@ -1154,7 +1161,8 @@ static int ax88179_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 		ax_skb = skb_clone(skb, GFP_ATOMIC);
 		if (ax_skb) {
 			ax_skb->len = pkt_len;
-			ax_skb->data = skb->data + 2;
+			if (NET_IP_ALIGN == 0)
+				ax_skb->data = skb->data + 2;
 			skb_set_tail_pointer(ax_skb, pkt_len);
 			ax_skb->truesize = pkt_len + sizeof(struct sk_buff);
 			ax88179_rx_checksum(ax_skb, pkt_hdr);
@@ -1328,8 +1336,10 @@ static int ax88179_reset(struct usbnet *dev)
 	ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_TXCOE_CTL, 1, 1, tmp);
 
 	/* Configure RX control register => start operation */
-	*tmp16 = AX_RX_CTL_DROPCRCERR | AX_RX_CTL_IPE | AX_RX_CTL_START |
-		 AX_RX_CTL_AP | AX_RX_CTL_AMALL | AX_RX_CTL_AB;
+	*tmp16 = AX_RX_CTL_DROPCRCERR | AX_RX_CTL_START | AX_RX_CTL_AP |
+		 AX_RX_CTL_AMALL | AX_RX_CTL_AB;
+	if (NET_IP_ALIGN == 0)
+		*tmp16 |= AX_RX_CTL_IPE;
 	ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_RX_CTL, 2, 2, tmp16);
 
 	*tmp = AX_MONITOR_MODE_PMETYPE | AX_MONITOR_MODE_PMEPOL |
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 5+ messages in thread

[parent not found: <1386323898-2246-1-git-send-email-freddy-knRN6Y/kmf1NUHwG+Fw1Kw@public.gmane.org>]

* Re: [PATCH 1/1] AX88179_178A: Enable the hardware pseudo header in case of the NET_IP_ALIGN equals 0
       [not found] ` <1386323898-2246-1-git-send-email-freddy-knRN6Y/kmf1NUHwG+Fw1Kw@public.gmane.org>
@ 2013-12-10  1:01   ` David Miller
       [not found]     ` <20131209.200131.1376169274733452706.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
  0 siblings, 1 reply; 5+ messages in thread
From: David Miller @ 2013-12-10  1:01 UTC (permalink / raw)
  To: freddy-knRN6Y/kmf1NUHwG+Fw1Kw
  Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, louis-knRN6Y/kmf1NUHwG+Fw1Kw,
	allan-knRN6Y/kmf1NUHwG+Fw1Kw

From: freddy-knRN6Y/kmf1NUHwG+Fw1Kw@public•gmane.org
Date: Fri,  6 Dec 2013 17:58:18 +0800

> From: Freddy Xin <freddy-knRN6Y/kmf1NUHwG+Fw1Kw@public•gmane.org>
> 
> The AX88179_178A has a hardware feature that it can insert a 2-bytes pseudo
> header in front of each received frame by setting the AX_RX_CTL_IPE bit.
> This feature is used to let the IP header be aligned on a doubleword-aligned address,
> but the NET_IP_ALIGN may equals to 2 and the __netdev_alloc_skb_ip_align in USBNET will
> reserve 2 bytes also, so in this case the driver shouldn't enable this bit.
> 
> This patch modifies the driver to set AX_RX_CTL_IPE just in case of the NET_IP_ALIGN equals 0.
> 
> Signed-off-by: Freddy Xin <freddy-knRN6Y/kmf1NUHwG+Fw1Kw@public•gmane.org>

Please avoid larger than 80 column lines in your commit messages,
people use text-only tools to viee these.

Next, it makes no sense to restrict your change to NET_IP_ALIGN==0

Simply handle any case, by undoing the reservation if it's getting
in the way.  If there isn't an appropriate helper for this, add one.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public•gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 5+ messages in thread

[parent not found: <20131209.200131.1376169274733452706.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>]

* Re: [PATCH 1/1] AX88179_178A: Enable the hardware pseudo header in case of the NET_IP_ALIGN equals 0
       [not found]     ` <20131209.200131.1376169274733452706.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
@ 2013-12-11  6:17       ` Freddy Xin
       [not found]         ` <52A80397.2080508-knRN6Y/kmf1NUHwG+Fw1Kw@public.gmane.org>
  0 siblings, 1 reply; 5+ messages in thread
From: Freddy Xin @ 2013-12-11  6:17 UTC (permalink / raw)
  To: David Miller
  Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, louis-knRN6Y/kmf1NUHwG+Fw1Kw,
	allan-knRN6Y/kmf1NUHwG+Fw1Kw


On 2013年12月10日 09:01, David Miller wrote:
> From: freddy-knRN6Y/kmf1NUHwG+Fw1Kw@public•gmane.org
> Date: Fri,  6 Dec 2013 17:58:18 +0800
>
>> From: Freddy Xin <freddy-knRN6Y/kmf1NUHwG+Fw1Kw@public•gmane.org>
>>
>> The AX88179_178A has a hardware feature that it can insert a 2-bytes pseudo
>> header in front of each received frame by setting the AX_RX_CTL_IPE bit.
>> This feature is used to let the IP header be aligned on a doubleword-aligned address,
>> but the NET_IP_ALIGN may equals to 2 and the __netdev_alloc_skb_ip_align in USBNET will
>> reserve 2 bytes also, so in this case the driver shouldn't enable this bit.
>>
>> This patch modifies the driver to set AX_RX_CTL_IPE just in case of the NET_IP_ALIGN equals 0.
>>
>> Signed-off-by: Freddy Xin <freddy-knRN6Y/kmf1NUHwG+Fw1Kw@public•gmane.org>
> Please avoid larger than 80 column lines in your commit messages,
> people use text-only tools to viee these.
>
> Next, it makes no sense to restrict your change to NET_IP_ALIGN==0
>
> Simply handle any case, by undoing the reservation if it's getting
> in the way.  If there isn't an appropriate helper for this, add one.
>
I think there is no way of undoing the reservation in the driver.
Can I add a flag of the driver_info, and use it to determine
whether undoing the reservation in rx_submit of usbnet?
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public•gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 5+ messages in thread

[parent not found: <52A80397.2080508-knRN6Y/kmf1NUHwG+Fw1Kw@public.gmane.org>]

* RE: [PATCH 1/1] AX88179_178A: Enable the hardware pseudo header in case of the NET_IP_ALIGN equals 0
       [not found]         ` <52A80397.2080508-knRN6Y/kmf1NUHwG+Fw1Kw@public.gmane.org>
@ 2013-12-11 10:33           ` David Laight
  2013-12-11 17:16           ` David Miller
  1 sibling, 0 replies; 5+ messages in thread
From: David Laight @ 2013-12-11 10:33 UTC (permalink / raw)
  To: Freddy Xin, David Miller
  Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, louis-knRN6Y/kmf1NUHwG+Fw1Kw,
	allan-knRN6Y/kmf1NUHwG+Fw1Kw

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="UTF-8", Size: 2157 bytes --]

> From: Freddy Xin
> On 2013å¹´12æœˆ10æ—¥ 09:01, David Miller wrote:
> > From: freddy@asix•com.tw
> > Date: Fri,  6 Dec 2013 17:58:18 +0800
> >
> >> From: Freddy Xin <freddy@asix•com.tw>
> >>
> >> The AX88179_178A has a hardware feature that it can insert a 2-bytes pseudo
> >> header in front of each received frame by setting the AX_RX_CTL_IPE bit.
> >> This feature is used to let the IP header be aligned on a doubleword-aligned address,
> >> but the NET_IP_ALIGN may equals to 2 and the __netdev_alloc_skb_ip_align in USBNET will
> >> reserve 2 bytes also, so in this case the driver shouldn't enable this bit.
> >>
> >> This patch modifies the driver to set AX_RX_CTL_IPE just in case of the NET_IP_ALIGN equals 0.
> >>
> >> Signed-off-by: Freddy Xin <freddy@asix•com.tw>
> > Please avoid larger than 80 column lines in your commit messages,
> > people use text-only tools to viee these.
> >
> > Next, it makes no sense to restrict your change to NET_IP_ALIGN==0
> >
> > Simply handle any case, by undoing the reservation if it's getting
> > in the way.  If there isn't an appropriate helper for this, add one.
> >
> I think there is no way of undoing the reservation in the driver.
> Can I add a flag of the driver_info, and use it to determine
> whether undoing the reservation in rx_submit of usbnet?

You probably want to arrange to have save the appropriate offset in
one of the structures - so avoiding any conditionals during the allocate.

I've noticed that the RX URB are 20k bytes long.
Allocating that much physically and virtually contiguous memory for
every rx frame seems sub-optimal to say the least!
What does the skb's 'realsize' end up as?
I don't remember seeing a copy for short frames either.

ISTR there is code to debatch multiple ethernet frames from a single
URB. I think that means that an ethernet frame might end up in two URB.
I'm not sure there is code to handle that either - but I've not looked
that closely at that part of the code.

	David

N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±ºÆâžØ^n‡r¡ö¦zË\x1aëh™¨èÚ&¢îý»\x05ËÛÔØï¦v¬Îf\x1dp)¹¹br	šê+€Ê+zf£¢·hšˆ§~††Ûiÿûàz¹\x1e®w¥¢¸?™¨èÚ&¢)ß¢^[f

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/1] AX88179_178A: Enable the hardware pseudo header in case of the NET_IP_ALIGN equals 0
       [not found]         ` <52A80397.2080508-knRN6Y/kmf1NUHwG+Fw1Kw@public.gmane.org>
  2013-12-11 10:33           ` David Laight
@ 2013-12-11 17:16           ` David Miller
  1 sibling, 0 replies; 5+ messages in thread
From: David Miller @ 2013-12-11 17:16 UTC (permalink / raw)
  To: freddy-knRN6Y/kmf1NUHwG+Fw1Kw
  Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, louis-knRN6Y/kmf1NUHwG+Fw1Kw,
	allan-knRN6Y/kmf1NUHwG+Fw1Kw

From: Freddy Xin <freddy-knRN6Y/kmf1NUHwG+Fw1Kw@public•gmane.org>
Date: Wed, 11 Dec 2013 14:17:59 +0800

> I think there is no way of undoing the reservation in the driver.

Then you should add a flag that, indeed, tells usbnet not to do the
reservation in the first place.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public•gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2013-12-11 17:16 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-06  9:58 [PATCH 1/1] AX88179_178A: Enable the hardware pseudo header in case of the NET_IP_ALIGN equals 0 freddy
     [not found] ` <1386323898-2246-1-git-send-email-freddy-knRN6Y/kmf1NUHwG+Fw1Kw@public.gmane.org>
2013-12-10  1:01   ` David Miller
     [not found]     ` <20131209.200131.1376169274733452706.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
2013-12-11  6:17       ` Freddy Xin
     [not found]         ` <52A80397.2080508-knRN6Y/kmf1NUHwG+Fw1Kw@public.gmane.org>
2013-12-11 10:33           ` David Laight
2013-12-11 17:16           ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox