* [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
2013-12-10 1:01 ` David Miller
0 siblings, 1 reply; 9+ 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] 9+ messages in thread
* Re: [PATCH 1/1] AX88179_178A: Enable the hardware pseudo header in case of the NET_IP_ALIGN equals 0
@ 2013-12-10 1:01 ` David Miller
0 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2013-12-10 1:01 UTC (permalink / raw)
To: freddy; +Cc: linux-usb, linux-kernel, netdev, louis, allan
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] AX88179_178A: Enable the hardware pseudo header in case of the NET_IP_ALIGN equals 0
@ 2013-12-10 1:01 ` David Miller
0 siblings, 0 replies; 9+ 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] 9+ messages in thread
* Re: [PATCH 1/1] AX88179_178A: Enable the hardware pseudo header in case of the NET_IP_ALIGN equals 0
@ 2013-12-11 6:17 ` Freddy Xin
0 siblings, 0 replies; 9+ messages in thread
From: Freddy Xin @ 2013-12-11 6:17 UTC (permalink / raw)
To: David Miller; +Cc: linux-usb, linux-kernel, netdev, louis, allan
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?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] AX88179_178A: Enable the hardware pseudo header in case of the NET_IP_ALIGN equals 0
@ 2013-12-11 6:17 ` Freddy Xin
0 siblings, 0 replies; 9+ 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] 9+ messages in thread
* RE: [PATCH 1/1] AX88179_178A: Enable the hardware pseudo header in case of the NET_IP_ALIGN equals 0
@ 2013-12-11 10:33 ` David Laight
0 siblings, 0 replies; 9+ messages in thread
From: David Laight @ 2013-12-11 10:33 UTC (permalink / raw)
To: Freddy Xin, David Miller; +Cc: linux-usb, linux-kernel, netdev, louis, allan
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="UTF-8", Size: 2141 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Ç+·®+%Ëÿ±éݶ\x17¥wÿº{.nÇ+·¥{±þG«éÿ{ayº\x1dÊÚë,j\a¢f£¢·hïêÿêçz_è®\x03(éÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?¨èÚ&£ø§~á¶iOæ¬z·vØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?I¥
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH 1/1] AX88179_178A: Enable the hardware pseudo header in case of the NET_IP_ALIGN equals 0
@ 2013-12-11 10:33 ` David Laight
0 siblings, 0 replies; 9+ 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Ç+·¥{±ºÆâØ^nr¡ö¦zË\x1aëh¨èÚ&¢îý»\x05ËÛÔØï¦v¬Îf\x1dp)¹¹br ê+Ê+zf£¢·h§~Ûiÿûàz¹\x1e®w¥¢¸?¨èÚ&¢)ߢ^[f
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] AX88179_178A: Enable the hardware pseudo header in case of the NET_IP_ALIGN equals 0
@ 2013-12-11 17:16 ` David Miller
0 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2013-12-11 17:16 UTC (permalink / raw)
To: freddy; +Cc: linux-usb, linux-kernel, netdev, louis, allan
From: Freddy Xin <freddy@asix.com.tw>
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] AX88179_178A: Enable the hardware pseudo header in case of the NET_IP_ALIGN equals 0
@ 2013-12-11 17:16 ` David Miller
0 siblings, 0 replies; 9+ 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] 9+ messages in thread
end of thread, other threads:[~2013-12-11 17:16 UTC | newest]
Thread overview: 9+ 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
2013-12-10 1:01 ` David Miller
2013-12-10 1:01 ` David Miller
2013-12-11 6:17 ` Freddy Xin
2013-12-11 6:17 ` Freddy Xin
2013-12-11 10:33 ` David Laight
2013-12-11 10:33 ` David Laight
2013-12-11 17:16 ` David Miller
2013-12-11 17:16 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.