| Message ID | 1393532822-28217-1-git-send-email-swarren@wwwdotorg.org |
|---|---|
| State | Accepted |
| Delegated to: | Tom Rini |
| Headers | show |
Hi Stephen, On 27 February 2014 13:27, Stephen Warren <swarren@wwwdotorg.org> wrote: > From: Stephen Warren <swarren@nvidia.com> > > For Ethernet/USB RX packets, the ASIX HW pads odd-sized packets so that > they have an even size. Currently, asix_recv() does remove this padding, > and asic_send() adds equivalent padding in the TX path. However, the HW > does not appear to need this packing for TX packets in practical testing > with "ASIX Elec. Corp. AX88x72A 000001" Vendor: 0x0b95 Product 0x7720 > Version 0.1. The Linux kernel does no such padding for the TX path. > > Remove the padding from the TX path: > > * For consistency with the Linux kernel. > * NVIDIA has a Tegra simulator which validates that the length of USB > packets sent to an ASIX device matches the packet length value inside > the packet data. Having U-Boot and the kernel do the same thing when > creating the TX packets simplifies the simulator's validation. > > Cc: Lucas Stach <dev@lynxeye.de> > Cc: Marek Vasut <marex@denx.de> > Cc: Simon Glass <sjg@chromium.org> > Signed-off-by: Stephen Warren <swarren@nvidia.com> > My notes indicate I added this due to a problem I had at the time with bootp. However, since you have tested it, perhaps the root cause was somewhere else. Acked-by: Simon Glass <sjg@chromium.org> > --- > drivers/usb/eth/asix.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/usb/eth/asix.c b/drivers/usb/eth/asix.c > index 659533a8d4e4..ce133f00698d 100644 > --- a/drivers/usb/eth/asix.c > +++ b/drivers/usb/eth/asix.c > @@ -468,8 +468,6 @@ static int asix_send(struct eth_device *eth, void > *packet, int length) > > memcpy(msg, &packet_len, sizeof(packet_len)); > memcpy(msg + sizeof(packet_len), (void *)packet, length); > - if (length & 1) > - length++; > > err = usb_bulk_msg(dev->pusb_dev, > usb_sndbulkpipe(dev->pusb_dev, > dev->ep_out), > -- > 1.8.1.5 > >
On Thursday, February 27, 2014 at 09:38:48 PM, Simon Glass wrote: > Hi Stephen, > > On 27 February 2014 13:27, Stephen Warren <swarren@wwwdotorg.org> wrote: > > From: Stephen Warren <swarren@nvidia.com> > > > > For Ethernet/USB RX packets, the ASIX HW pads odd-sized packets so that > > they have an even size. Currently, asix_recv() does remove this padding, > > and asic_send() adds equivalent padding in the TX path. However, the HW > > does not appear to need this packing for TX packets in practical testing > > with "ASIX Elec. Corp. AX88x72A 000001" Vendor: 0x0b95 Product 0x7720 > > Version 0.1. The Linux kernel does no such padding for the TX path. > > > > Remove the padding from the TX path: > > > > * For consistency with the Linux kernel. > > * NVIDIA has a Tegra simulator which validates that the length of USB > > > > packets sent to an ASIX device matches the packet length value inside > > the packet data. Having U-Boot and the kernel do the same thing when > > creating the TX packets simplifies the simulator's validation. > > > > Cc: Lucas Stach <dev@lynxeye.de> > > Cc: Marek Vasut <marex@denx.de> > > Cc: Simon Glass <sjg@chromium.org> > > Signed-off-by: Stephen Warren <swarren@nvidia.com> > > My notes indicate I added this due to a problem I had at the time with > bootp. However, since you have tested it, perhaps the root cause was > somewhere else. > > Acked-by: Simon Glass <sjg@chromium.org> Tested on two different ASIX dongles: 0x2001:0x3c05 ; DUB-E100 ; D-Link Corporation 0x0b95:0x7720 ; ZoWii ; Zoltan Tech Acked-by: Marek Vasut <marex@denx.de> Tested-by: Marek Vasut <marex@denx.de> Best regards, Marek Vasut
On Fri, Feb 28, 2014 at 11:54 +0100, Marek Vasut wrote: > > On Thursday, February 27, 2014 at 09:38:48 PM, Simon Glass wrote: > > Hi Stephen, > > > > On 27 February 2014 13:27, Stephen Warren <swarren@wwwdotorg.org> wrote: > > > From: Stephen Warren <swarren@nvidia.com> > > > > > > For Ethernet/USB RX packets, the ASIX HW pads odd-sized packets so that > > > they have an even size. Currently, asix_recv() does remove this padding, > > > and asic_send() adds equivalent padding in the TX path. However, the HW > > > does not appear to need this packing for TX packets in practical testing > > > with "ASIX Elec. Corp. AX88x72A 000001" Vendor: 0x0b95 Product 0x7720 > > > Version 0.1. The Linux kernel does no such padding for the TX path. > > > > > > Remove the padding from the TX path: > > > > > > * For consistency with the Linux kernel. > > > * NVIDIA has a Tegra simulator which validates that the length of USB > > > > > > packets sent to an ASIX device matches the packet length value inside > > > the packet data. Having U-Boot and the kernel do the same thing when > > > creating the TX packets simplifies the simulator's validation. > > > > > > Cc: Lucas Stach <dev@lynxeye.de> > > > Cc: Marek Vasut <marex@denx.de> > > > Cc: Simon Glass <sjg@chromium.org> > > > Signed-off-by: Stephen Warren <swarren@nvidia.com> > > > > My notes indicate I added this due to a problem I had at the time with > > bootp. However, since you have tested it, perhaps the root cause was > > somewhere else. > > > > Acked-by: Simon Glass <sjg@chromium.org> > > Tested on two different ASIX dongles: > > 0x2001:0x3c05 ; DUB-E100 ; D-Link Corporation > 0x0b95:0x7720 ; ZoWii ; Zoltan Tech > > Acked-by: Marek Vasut <marex@denx.de> > Tested-by: Marek Vasut <marex@denx.de> Tested with TFTP over a "Logilink UA0144" dongle ('usb info' says "ASIX Elec. Corp. AX88772B 000001", vendor 0x0b95, prod 0x772b) and "Edimax EU-4208" ("ASIX Elec. Corp. AX88772B 0E62D5", 0x0b95, 0x772b). Tested-by: Gerhard Sittig <gsi@denx.de> virtually yours Gerhard Sittig
On Monday, March 03, 2014 at 03:14:34 PM, Gerhard Sittig wrote: > On Fri, Feb 28, 2014 at 11:54 +0100, Marek Vasut wrote: > > On Thursday, February 27, 2014 at 09:38:48 PM, Simon Glass wrote: > > > Hi Stephen, > > > > > > On 27 February 2014 13:27, Stephen Warren <swarren@wwwdotorg.org> wrote: > > > > From: Stephen Warren <swarren@nvidia.com> > > > > > > > > For Ethernet/USB RX packets, the ASIX HW pads odd-sized packets so > > > > that they have an even size. Currently, asix_recv() does remove this > > > > padding, and asic_send() adds equivalent padding in the TX path. > > > > However, the HW does not appear to need this packing for TX packets > > > > in practical testing with "ASIX Elec. Corp. AX88x72A 000001" Vendor: > > > > 0x0b95 Product 0x7720 Version 0.1. The Linux kernel does no such > > > > padding for the TX path. > > > > > > > > Remove the padding from the TX path: > > > > > > > > * For consistency with the Linux kernel. > > > > * NVIDIA has a Tegra simulator which validates that the length of USB > > > > > > > > packets sent to an ASIX device matches the packet length value > > > > inside the packet data. Having U-Boot and the kernel do the same > > > > thing when creating the TX packets simplifies the simulator's > > > > validation. > > > > > > > > Cc: Lucas Stach <dev@lynxeye.de> > > > > Cc: Marek Vasut <marex@denx.de> > > > > Cc: Simon Glass <sjg@chromium.org> > > > > Signed-off-by: Stephen Warren <swarren@nvidia.com> > > > > > > My notes indicate I added this due to a problem I had at the time with > > > bootp. However, since you have tested it, perhaps the root cause was > > > somewhere else. > > > > > > Acked-by: Simon Glass <sjg@chromium.org> > > > > Tested on two different ASIX dongles: > > > > 0x2001:0x3c05 ; DUB-E100 ; D-Link Corporation > > 0x0b95:0x7720 ; ZoWii ; Zoltan Tech > > > > Acked-by: Marek Vasut <marex@denx.de> > > Tested-by: Marek Vasut <marex@denx.de> > > Tested with TFTP over a "Logilink UA0144" dongle ('usb info' says > "ASIX Elec. Corp. AX88772B 000001", vendor 0x0b95, prod 0x772b) > and "Edimax EU-4208" ("ASIX Elec. Corp. AX88772B 0E62D5", 0x0b95, > 0x772b). > > Tested-by: Gerhard Sittig <gsi@denx.de> Putting Joe back on CC. Joe, can you please pick this one ? Thanks! Best regards, Marek Vasut
On Thu, Feb 27, 2014 at 01:27:02PM -0700, Stephen Warren wrote: > From: Stephen Warren <swarren@nvidia.com> > > For Ethernet/USB RX packets, the ASIX HW pads odd-sized packets so that > they have an even size. Currently, asix_recv() does remove this padding, > and asic_send() adds equivalent padding in the TX path. However, the HW > does not appear to need this packing for TX packets in practical testing > with "ASIX Elec. Corp. AX88x72A 000001" Vendor: 0x0b95 Product 0x7720 > Version 0.1. The Linux kernel does no such padding for the TX path. > > Remove the padding from the TX path: > > * For consistency with the Linux kernel. > * NVIDIA has a Tegra simulator which validates that the length of USB > packets sent to an ASIX device matches the packet length value inside > the packet data. Having U-Boot and the kernel do the same thing when > creating the TX packets simplifies the simulator's validation. > > Cc: Lucas Stach <dev@lynxeye.de> > Cc: Marek Vasut <marex@denx.de> > Cc: Simon Glass <sjg@chromium.org> > Signed-off-by: Stephen Warren <swarren@nvidia.com> > Acked-by: Simon Glass <sjg@chromium.org> > Acked-by: Marek Vasut <marex@denx.de> > Tested-by: Marek Vasut <marex@denx.de> > Tested-by: Gerhard Sittig <gsi@denx.de> Applied to u-boot/master, thanks!
On Friday, March 07, 2014 at 11:27:37 PM, Tom Rini wrote: > On Thu, Feb 27, 2014 at 01:27:02PM -0700, Stephen Warren wrote: > > From: Stephen Warren <swarren@nvidia.com> > > > > For Ethernet/USB RX packets, the ASIX HW pads odd-sized packets so that > > they have an even size. Currently, asix_recv() does remove this padding, > > and asic_send() adds equivalent padding in the TX path. However, the HW > > does not appear to need this packing for TX packets in practical testing > > with "ASIX Elec. Corp. AX88x72A 000001" Vendor: 0x0b95 Product 0x7720 > > Version 0.1. The Linux kernel does no such padding for the TX path. > > > > Remove the padding from the TX path: > > > > * For consistency with the Linux kernel. > > * NVIDIA has a Tegra simulator which validates that the length of USB > > > > packets sent to an ASIX device matches the packet length value inside > > the packet data. Having U-Boot and the kernel do the same thing when > > creating the TX packets simplifies the simulator's validation. > > > > Cc: Lucas Stach <dev@lynxeye.de> > > Cc: Marek Vasut <marex@denx.de> > > Cc: Simon Glass <sjg@chromium.org> > > Signed-off-by: Stephen Warren <swarren@nvidia.com> > > Acked-by: Simon Glass <sjg@chromium.org> > > Acked-by: Marek Vasut <marex@denx.de> > > Tested-by: Marek Vasut <marex@denx.de> > > Tested-by: Gerhard Sittig <gsi@denx.de> > > Applied to u-boot/master, thanks! Uh, where did Joe disappear to anyway ? Best regards, Marek Vasut
Hi Marek, On Sat, Mar 8, 2014 at 5:32 AM, Marek Vasut <marex@denx.de> wrote: > On Friday, March 07, 2014 at 11:27:37 PM, Tom Rini wrote: > > > > Applied to u-boot/master, thanks! > > Uh, where did Joe disappear to anyway ? > > Best regards, > Marek Vasut > I've been traveling a lot recently and have not had much time for reviewing and testing patches. I expect to get back into it u-boot more heavily in a few months. Regards, -Joe
diff --git a/drivers/usb/eth/asix.c b/drivers/usb/eth/asix.c index 659533a8d4e4..ce133f00698d 100644 --- a/drivers/usb/eth/asix.c +++ b/drivers/usb/eth/asix.c @@ -468,8 +468,6 @@ static int asix_send(struct eth_device *eth, void *packet, int length) memcpy(msg, &packet_len, sizeof(packet_len)); memcpy(msg + sizeof(packet_len), (void *)packet, length); - if (length & 1) - length++; err = usb_bulk_msg(dev->pusb_dev, usb_sndbulkpipe(dev->pusb_dev, dev->ep_out),