Nilkesh
>-----Original Message-----
>From: [email protected]
>[mailto:[email protected]] On Behalf Of Patra,
>Nilkesh
>Sent: Sunday, December 13, 2009 10:51 PM
>To: [email protected]
>Cc: [email protected]; David Brownell; Gadiyar, Anand
>Subject: [RFC] MUSB: Workaround for Ethernet data alignment issue
>
>On the latest version of MUSB, DMA is not working properly if the Data in the
>packet doesn't start at
>32bit aligned address. This issue was found while using MUSB as g_ether. The
>basic ping does not
>work, if the data in the Ethernet packet does not start at 32bit aligned
>address.
>
>To overcome this issue, we found one solution mentioned in the below patch.
>This patch moves data
>(skb->data) by 2 bytes backwards in ethernet packet, which is nothing but
>skb->head.
>
>I want to know, if there are any better approaches to fix this issue.
I would NAK the first part of patch for following reasons (and suggestions are
inlined):
a) this change is specific to OMAP hw only and so should not try to change
generic files like usb/gadget/u_ether.c
that get compiled for all the architectures having usb.
b) there are two issues to be handled separately ( separate patches are good
for review )
RX side buffer alignments handling
TX side buffer alignments handling
>
>Signed-off-by: Nilkesh Patra <[email protected]>
>---
> drivers/usb/gadget/u_ether.c | 14 ++++++++++----
> 1 files changed, 10 insertions(+), 4 deletions(-)
>
>diff --git a/drivers/usb/gadget/u_ether.c b/drivers/usb/gadget/u_ether.c
>index 2fc02bd..432b1bd 100644
>--- a/drivers/usb/gadget/u_ether.c
>+++ b/drivers/usb/gadget/u_ether.c
>@@ -249,7 +249,6 @@ rx_submit(struct eth_dev *dev, struct usb_request *req,
>gfp_t gfp_flags)
> * but on at least one, checksumming fails otherwise. Note:
> * RNDIS headers involve variable numbers of LE32 values.
> */
>- skb_reserve(skb, NET_IP_ALIGN);
NAK. Do not remove this line,
Instead over-ride the NET_IP_ALIGN macro for your architecture.
One suggestion to do so:
diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
index 058e7e9..b16c5f7 100644
--- a/arch/arm/include/asm/system.h
+++ b/arch/arm/include/asm/system.h
@@ -514,6 +514,10 @@ static inline unsigned long long __cmpxchg64_mb(volatile
void *ptr,
#define arch_align_stack(x) (x)
+#if defined(CONFIG_ARCH_OMAP3) || defined(CONFIG_ARCH_OMAP4)
+#define NET_IP_ALIGN 0
+#endif
+
#endif /* __KERNEL__ */
#endif
This has been verified to work for OMAP3630.
>
> req->buf = skb->data;
> req->length = size;
>@@ -500,6 +499,8 @@ static netdev_tx_t eth_start_xmit(struct sk_buff *skb,
> unsigned long flags;
> struct usb_ep *in;
> u16 cdc_filter;
>+ int i = 0;
>+
>
> spin_lock_irqsave(&dev->lock, flags);
> if (dev->port_usb) {
>@@ -573,9 +574,6 @@ static netdev_tx_t eth_start_xmit(struct sk_buff *skb,
>
> length = skb->len;
> }
>- req->buf = skb->data;
>- req->context = skb;
>- req->complete = tx_complete;
This change would be to handle TX un-alignments.
So I would prefer to handle it separately ( a separate patch)
Have you explored the option of over-riding #define NET_SKB_PAD ??
>
> /* use zlp framing on tx for strict CDC-Ether conformance,
> * though any robust network rx path ignores extra padding.
>@@ -585,6 +583,14 @@ static netdev_tx_t eth_start_xmit(struct sk_buff *skb,
> if (!dev->zlp && (length % in->maxpacket) == 0)
> length++;
>
>+ if ((int)skb->data & 0x2) {
>+ skb_push(skb, 2);
Where have you made sure that skb->data allocation has this extra space of two
bytes?
>+ for (i = 0; i < length; i++)
>+ skb->data[i] = skb->data[i+2];
>+ }
>+ req->buf = skb->data;
>+ req->context = skb;
>+ req->complete = tx_complete;
> req->length = length;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
