* Re: alignment faults in 3.6
From: Eric Dumazet @ 2012-10-11 11:54 UTC (permalink / raw)
To: mbizon
Cc: Måns Rullgård, Russell King - ARM Linux, netdev,
David Laight, Jon Masters, linux-arm-kernel
In-Reply-To: <1349956055.1232.9.camel@sakura.staff.proxad.net>
On Thu, 2012-10-11 at 13:47 +0200, Maxime Bizon wrote:
> On Thu, 2012-10-11 at 13:28 +0200, Eric Dumazet wrote:
>
> > You probably are aware that a driver can use :
> >
> > - a fragment to hold the frame given by the hardware, with whatever
> > alignment is needed by the hardware.
> >
> > Then allocate an skb with enough room (128 bytes) to pull the headers as
> > needed later.
> >
> > skb = netdev_alloc_skb_ip_align(dev, 128);
>
> What happen at tx time, supposing that same hardware cannot do SG ?
>
> Aren't we going to memcpy the data into the head ?
>
Of course, if you use a forwarding setup, and the tx driver is not SG
capable, performance will be bad (You have to copy the data into a
single skb (linearize the skb))
But in 2012, having to use hardware without SG for a router sounds a bad
joke (if cpu speed is _also_ too low)
Adding get_unaligned() everywhere in linux network stacks is not an
option.
We actually want to be able to read the code and fix the bugs, not only
run it on a cheap low end router.
^ permalink raw reply
* Re: alignment faults in 3.6
From: Eric Dumazet @ 2012-10-11 12:00 UTC (permalink / raw)
To: mbizon
Cc: Måns Rullgård, Russell King - ARM Linux, netdev,
David Laight, Jon Masters, linux-arm-kernel
In-Reply-To: <1349956456.21172.8820.camel@edumazet-glaptop>
On Thu, 2012-10-11 at 13:54 +0200, Eric Dumazet wrote:
> On Thu, 2012-10-11 at 13:47 +0200, Maxime Bizon wrote:
> > On Thu, 2012-10-11 at 13:28 +0200, Eric Dumazet wrote:
> >
> > > You probably are aware that a driver can use :
> > >
> > > - a fragment to hold the frame given by the hardware, with whatever
> > > alignment is needed by the hardware.
> > >
> > > Then allocate an skb with enough room (128 bytes) to pull the headers as
> > > needed later.
> > >
> > > skb = netdev_alloc_skb_ip_align(dev, 128);
> >
> > What happen at tx time, supposing that same hardware cannot do SG ?
> >
> > Aren't we going to memcpy the data into the head ?
> >
>
> Of course, if you use a forwarding setup, and the tx driver is not SG
> capable, performance will be bad (You have to copy the data into a
> single skb (linearize the skb))
By the way, if said driver also has alignments issues, it will probably
copy the packet given by the stack. (Think you added an IPIP
encapsulation)
It would be better for such driver to still pretend it can do SG,
and it does the copy itself, avoiding the prior copies in core stack.
^ permalink raw reply
* Re: removing the timer from cdc-ncm
From: Bjørn Mork @ 2012-10-11 12:15 UTC (permalink / raw)
To: Oliver Neukum
Cc: Alexey ORISHKO, netdev@vger.kernel.org, linux-usb@vger.kernel.org
In-Reply-To: <87zk3te2j9.fsf-lbf33ChDnrE/G1V5fR+Y7Q@public.gmane.org>
Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org> writes:
> [48880.048494] BUG: unable to handle kernel NULL pointer dereference at 0000000000000068
> [48880.048573] IP: [<ffffffffa06ba879>] cdc_ncm_tx_bundle+0x168/0x43b [cdc_ncm]
This one is because you removed the "if (skb == NULL)" from the for
loop, but left the "skb = NULL;" at the end:
@@ -719,28 +707,15 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
/* compute maximum buffer size */
rem = ctx->tx_max - offset;
- if (skb == NULL) {
- skb = ctx->tx_rem_skb;
- ctx->tx_rem_skb = NULL;
-
- /* check for end of skb */
- if (skb == NULL)
- break;
- }
-
if (skb->len > rem) {
if (n == 0) {
/* won't fit, MTU problem? */
dev_kfree_skb_any(skb);
skb = NULL;
ctx->netdev->stats.tx_dropped++;
+ error = 1;
} else {
- /* no room for skb - store for later */
- if (ctx->tx_rem_skb != NULL) {
- dev_kfree_skb_any(ctx->tx_rem_skb);
- ctx->netdev->stats.tx_dropped++;
- }
- ctx->tx_rem_skb = skb;
+
skb = NULL;
ready2send = 1;
}
@@ -768,13 +743,6 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
skb = NULL;
}
If I understand your intentions with this code, then the for loop should
probably just go away completely?
Anyway, after avoiding that one, I ended up with
[ 857.112692] cdc_ncm_fill_tx_frame: skb_out=ffff880218624cc0, length=2048
[ 857.112696] cdc_ncm_tx_fixup: returning (null)
[ 857.112731] BUG: unable to handle kernel NULL pointer dereference at 0000000000000068
[ 857.112807] IP: [<ffffffffa030e68b>] usbnet_start_xmit+0x128/0x4e9 [usbnet]
No need for the reset of the trace here. Removing the "goto not_drop"
makes usbnet_start_xmit continue after tx_fixup returns NULL:
- if (info->tx_fixup) {
+ if (transmit_now && info->tx_fixup) {
skb = info->tx_fixup (dev, skb, GFP_ATOMIC);
if (!skb) {
if (netif_msg_tx_err(dev)) {
netif_dbg(dev, tx_err, dev->net, "can't tx_fixup skb\n");
goto drop;
- } else {
- /* cdc_ncm collected packet; waits for more */
- goto not_drop;
}
}
}
// some devices want funky USB-level framing, for
// win32 driver (usually) and/or hardware quirks
if (transmit_now && info->tx_fixup) {
skb = info->tx_fixup (dev, skb, GFP_ATOMIC);
if (!skb) {
if (netif_msg_tx_err(dev)) {
netif_dbg(dev, tx_err, dev->net, "can't tx_fixup skb\n");
goto drop;
}
}
}
length = skb->len;
I stopped there. I am not exactly sure where you were going with this
anymore. tx_fixup will return the tx_curr_skb being currently built
every time it is called, without anything resetting it at that point.
Then when finally cdc_ncm_fill_tx_frame has been called enough times to
fill it completely, you end up just dropping it on the floor. The
result is that nothing is ever transmitted even after fixing the above
errors because the tx_curr_skb has an incomplete header every time
tx_fixup returns it.
I do like your idea, but if you don't mind I think we'll go ahead and
base the upcoming cdc_mbim driver on the current cdc_ncm *with* the
timer. The plan is to reuse as much of cdc_ncm as possible, so in
theory it should be easy to update both drivers when the timer goes away
(there is no need for cdc_mbim be aware of the timer at all). But
unfortunately it seems that cdc_ncm_fill_tx_frame must be changed a lot
to support the concept of multiple NDPs in a single NTB. This means
that the timer removal patches will conflict heavily with the
preparations for MBIM support. With some luck and acceptance from all
contributors, I hope to be able to post the first version of that as the
merge window closes.
Bjørn
--
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
* Re: alignment faults in 3.6
From: Arnd Bergmann @ 2012-10-11 12:28 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Eric Dumazet, Russell King - ARM Linux, Jon Masters, netdev,
Måns Rullgård, David Laight, Rob Herring
In-Reply-To: <1349952574.21172.8604.camel@edumazet-glaptop>
On Thursday 11 October 2012, Eric Dumazet wrote:
> On Thu, 2012-10-11 at 11:32 +0100, Russell King - ARM Linux wrote:
> > On Thu, Oct 11, 2012 at 12:22:06PM +0200, Eric Dumazet wrote:
> > > I took a look, and I dont see why/how gcc could use a ldm instruction
> > >
> > > Doing so assumed the alignment of the structure was 8 bytes, but its
> > > not.
> > >
> > > Networking stack mandates that IP headers are aligned on 4 bytes
> > > boundaries, not 8 bytes.
> >
> > Err, no. ldm is "load multiple" not "load double". It loads multiple
> > 32-bit registers, and its requirement for non-faulting behaviour is for
> > the pointer to be 4 byte aligned. However, "load double" requires 8
> > byte alignment.
>
> So if you have an alignment fault, thats because IP header is not
> aligned on 4 bytes ?
>
> If so a driver is buggy and must be fixed.
>
> Please send us full stack trace
Rob Herring as the original reporter has dropped off the Cc list, adding
him back.
I assume that the calxeda xgmac driver is the culprit then. It uses
netdev_alloc_skb() rather than netdev_alloc_skb_ip_align() in
xgmac_rx_refill but it is not clear whether it does so intentionally
or by accident.
Arnd
^ permalink raw reply
* [PATCH v1 00/11] usbnet: usb_control_msg cleanup
From: Ming Lei @ 2012-10-11 12:39 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman
Cc: Oliver Neukum, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
Hi,
This patch set introduces 3 helpers for handling usb read, write
and write_async command, and replaces the low level's implemention
with the generic ones.
This patchset is a cleanup and about 300 lines code can be saved.
Also, the patchset fixes problem of DMA on buffer embedded inside
one dynamic allocated buffer, and such usages can be found
in cdc-ncm, sierra_net, mcs7830 and asix drivers.
v1:
- drop previous patch 12, and let net/core handle
runtime PM in ioctl path
drivers/net/usb/asix_common.c | 117 ++++--------------------------------
drivers/net/usb/cdc_ncm.c | 73 +++++++---------------
drivers/net/usb/dm9601.c | 107 +++++----------------------------
drivers/net/usb/int51x1.c | 52 +---------------
drivers/net/usb/mcs7830.c | 85 ++------------------------
drivers/net/usb/net1080.c | 110 ++++++++++------------------------
drivers/net/usb/plusb.c | 11 ++--
drivers/net/usb/sierra_net.c | 45 ++++++--------
drivers/net/usb/smsc75xx.c | 39 +++++-------
drivers/net/usb/smsc95xx.c | 115 +++++++++--------------------------
drivers/net/usb/usbnet.c | 133 +++++++++++++++++++++++++++++++++++++++++
include/linux/usb/usbnet.h | 6 ++
12 files changed, 291 insertions(+), 602 deletions(-)
Thanks,
--
Ming Lei
--
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
* [PATCH v1 01/11] usbnet: introduce usbnet 3 command helpers
From: Ming Lei @ 2012-10-11 12:39 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman
Cc: Oliver Neukum, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Ming Lei
In-Reply-To: <1349959169-9432-1-git-send-email-ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
This patch introduces the below 3 usb command helpers:
usbnet_read_cmd / usbnet_write_cmd / usbnet_write_cmd_async
so that each low level driver doesn't need to implement them
by itself, and the dma buffer allocation for usb transfer and
runtime PM things can be handled just in one place.
Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
drivers/net/usb/usbnet.c | 133 ++++++++++++++++++++++++++++++++++++++++++++
include/linux/usb/usbnet.h | 6 ++
2 files changed, 139 insertions(+)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index fc9f578..3b51554 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1592,6 +1592,139 @@ int usbnet_resume (struct usb_interface *intf)
}
EXPORT_SYMBOL_GPL(usbnet_resume);
+/*-------------------------------------------------------------------------*/
+int usbnet_read_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
+ u16 value, u16 index, void *data, u16 size)
+{
+ void *buf = NULL;
+ int err = -ENOMEM;
+
+ netdev_dbg(dev->net, "usbnet_read_cmd cmd=0x%02x reqtype=%02x"
+ " value=0x%04x index=0x%04x size=%d\n",
+ cmd, reqtype, value, index, size);
+
+ if (data) {
+ buf = kmalloc(size, GFP_KERNEL);
+ if (!buf)
+ goto out;
+ }
+
+ err = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
+ cmd, reqtype, value, index, buf, size,
+ USB_CTRL_GET_TIMEOUT);
+ if (err > 0 && err <= size)
+ memcpy(data, buf, err);
+ kfree(buf);
+out:
+ return err;
+}
+EXPORT_SYMBOL_GPL(usbnet_read_cmd);
+
+int usbnet_write_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
+ u16 value, u16 index, const void *data, u16 size)
+{
+ void *buf = NULL;
+ int err = -ENOMEM;
+
+ netdev_dbg(dev->net, "usbnet_write_cmd cmd=0x%02x reqtype=%02x"
+ " value=0x%04x index=0x%04x size=%d\n",
+ cmd, reqtype, value, index, size);
+
+ if (data) {
+ buf = kmemdup(data, size, GFP_KERNEL);
+ if (!buf)
+ goto out;
+ }
+
+ err = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
+ cmd, reqtype, value, index, buf, size,
+ USB_CTRL_SET_TIMEOUT);
+ kfree(buf);
+
+out:
+ return err;
+}
+EXPORT_SYMBOL_GPL(usbnet_write_cmd);
+
+static void usbnet_async_cmd_cb(struct urb *urb)
+{
+ struct usb_ctrlrequest *req = (struct usb_ctrlrequest *)urb->context;
+ int status = urb->status;
+
+ if (status < 0)
+ dev_dbg(&urb->dev->dev, "%s failed with %d",
+ __func__, status);
+
+ kfree(req);
+ usb_free_urb(urb);
+}
+
+int usbnet_write_cmd_async(struct usbnet *dev, u8 cmd, u8 reqtype,
+ u16 value, u16 index, const void *data, u16 size)
+{
+ struct usb_ctrlrequest *req = NULL;
+ struct urb *urb;
+ int err = -ENOMEM;
+ void *buf = NULL;
+
+ netdev_dbg(dev->net, "usbnet_write_cmd cmd=0x%02x reqtype=%02x"
+ " value=0x%04x index=0x%04x size=%d\n",
+ cmd, reqtype, value, index, size);
+
+ urb = usb_alloc_urb(0, GFP_ATOMIC);
+ if (!urb) {
+ netdev_err(dev->net, "Error allocating URB in"
+ " %s!\n", __func__);
+ goto fail;
+ }
+
+ if (data) {
+ buf = kmemdup(data, size, GFP_ATOMIC);
+ if (!buf) {
+ netdev_err(dev->net, "Error allocating buffer"
+ " in %s!\n", __func__);
+ goto fail_free;
+ }
+ }
+
+ req = kmalloc(sizeof(struct usb_ctrlrequest), GFP_ATOMIC);
+ if (!req) {
+ netdev_err(dev->net, "Failed to allocate memory for %s\n",
+ __func__);
+ goto fail_free_buf;
+ }
+
+ req->bRequestType = reqtype;
+ req->bRequest = cmd;
+ req->wValue = cpu_to_le16(value);
+ req->wIndex = cpu_to_le16(index);
+ req->wLength = cpu_to_le16(size);
+
+ usb_fill_control_urb(urb, dev->udev,
+ usb_sndctrlpipe(dev->udev, 0),
+ (void *)req, buf, size,
+ usbnet_async_cmd_cb, req);
+ urb->transfer_flags |= URB_FREE_BUFFER;
+
+ err = usb_submit_urb(urb, GFP_ATOMIC);
+ if (err < 0) {
+ netdev_err(dev->net, "Error submitting the control"
+ " message: status=%d\n", err);
+ goto fail_free;
+ }
+ return 0;
+
+fail_free_buf:
+ kfree(buf);
+fail_free:
+ kfree(req);
+ usb_free_urb(urb);
+fail:
+ return err;
+
+}
+EXPORT_SYMBOL_GPL(usbnet_write_cmd_async);
+
/*-------------------------------------------------------------------------*/
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index f87cf62..32a57dd 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -161,6 +161,12 @@ extern int usbnet_suspend(struct usb_interface *, pm_message_t);
extern int usbnet_resume(struct usb_interface *);
extern void usbnet_disconnect(struct usb_interface *);
+extern int usbnet_read_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
+ u16 value, u16 index, void *data, u16 size);
+extern int usbnet_write_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
+ u16 value, u16 index, const void *data, u16 size);
+extern int usbnet_write_cmd_async(struct usbnet *dev, u8 cmd, u8 reqtype,
+ u16 value, u16 index, const void *data, u16 size);
/* Drivers that reuse some of the standard USB CDC infrastructure
* (notably, using multiple interfaces according to the CDC
--
1.7.9.5
--
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 related
* [PATCH v1 03/11] usbnet: cdc-ncm: apply introduced usb command APIs
From: Ming Lei @ 2012-10-11 12:39 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman
Cc: Oliver Neukum, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Ming Lei
In-Reply-To: <1349959169-9432-1-git-send-email-ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
drivers/net/usb/cdc_ncm.c | 73 ++++++++++++++-------------------------------
1 file changed, 23 insertions(+), 50 deletions(-)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 4cd582a..429a2ad 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -159,16 +159,16 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
u8 iface_no;
int err;
u16 ntb_fmt_supported;
+ struct usbnet *dev = netdev_priv(ctx->netdev);
iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
- err = usb_control_msg(ctx->udev,
- usb_rcvctrlpipe(ctx->udev, 0),
+ err = usbnet_read_cmd(dev,
USB_CDC_GET_NTB_PARAMETERS,
USB_TYPE_CLASS | USB_DIR_IN
| USB_RECIP_INTERFACE,
0, iface_no, &ctx->ncm_parm,
- sizeof(ctx->ncm_parm), 10000);
+ sizeof(ctx->ncm_parm));
if (err < 0) {
pr_debug("failed GET_NTB_PARAMETERS\n");
return 1;
@@ -217,40 +217,23 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
if (ctx->rx_max != le32_to_cpu(ctx->ncm_parm.dwNtbInMaxSize)) {
if (flags & USB_CDC_NCM_NCAP_NTB_INPUT_SIZE) {
- struct usb_cdc_ncm_ndp_input_size *ndp_in_sz;
+ struct usb_cdc_ncm_ndp_input_size ndp_in_sz;
- ndp_in_sz = kzalloc(sizeof(*ndp_in_sz), GFP_KERNEL);
- if (!ndp_in_sz) {
- err = -ENOMEM;
- goto size_err;
- }
-
- err = usb_control_msg(ctx->udev,
- usb_sndctrlpipe(ctx->udev, 0),
+ memset(&ndp_in_sz, 0, sizeof(ndp_in_sz));
+ err = usbnet_write_cmd(dev,
USB_CDC_SET_NTB_INPUT_SIZE,
USB_TYPE_CLASS | USB_DIR_OUT
| USB_RECIP_INTERFACE,
- 0, iface_no, ndp_in_sz, 8, 1000);
- kfree(ndp_in_sz);
+ 0, iface_no, &ndp_in_sz, 8);
} else {
- __le32 *dwNtbInMaxSize;
- dwNtbInMaxSize = kzalloc(sizeof(*dwNtbInMaxSize),
- GFP_KERNEL);
- if (!dwNtbInMaxSize) {
- err = -ENOMEM;
- goto size_err;
- }
- *dwNtbInMaxSize = cpu_to_le32(ctx->rx_max);
+ __le32 dwNtbInMaxSize = cpu_to_le32(ctx->rx_max);
- err = usb_control_msg(ctx->udev,
- usb_sndctrlpipe(ctx->udev, 0),
+ err = usbnet_write_cmd(dev,
USB_CDC_SET_NTB_INPUT_SIZE,
USB_TYPE_CLASS | USB_DIR_OUT
| USB_RECIP_INTERFACE,
- 0, iface_no, dwNtbInMaxSize, 4, 1000);
- kfree(dwNtbInMaxSize);
+ 0, iface_no, &dwNtbInMaxSize, 4);
}
-size_err:
if (err < 0)
pr_debug("Setting NTB Input Size failed\n");
}
@@ -306,23 +289,23 @@ size_err:
/* set CRC Mode */
if (flags & USB_CDC_NCM_NCAP_CRC_MODE) {
- err = usb_control_msg(ctx->udev, usb_sndctrlpipe(ctx->udev, 0),
+ err = usbnet_write_cmd(dev,
USB_CDC_SET_CRC_MODE,
USB_TYPE_CLASS | USB_DIR_OUT
| USB_RECIP_INTERFACE,
USB_CDC_NCM_CRC_NOT_APPENDED,
- iface_no, NULL, 0, 1000);
+ iface_no, NULL, 0);
if (err < 0)
pr_debug("Setting CRC mode off failed\n");
}
/* set NTB format, if both formats are supported */
if (ntb_fmt_supported & USB_CDC_NCM_NTH32_SIGN) {
- err = usb_control_msg(ctx->udev, usb_sndctrlpipe(ctx->udev, 0),
+ err = usbnet_write_cmd(dev,
USB_CDC_SET_NTB_FORMAT, USB_TYPE_CLASS
| USB_DIR_OUT | USB_RECIP_INTERFACE,
USB_CDC_NCM_NTB16_FORMAT,
- iface_no, NULL, 0, 1000);
+ iface_no, NULL, 0);
if (err < 0)
pr_debug("Setting NTB format to 16-bit failed\n");
}
@@ -331,28 +314,21 @@ size_err:
/* set Max Datagram Size (MTU) */
if (flags & USB_CDC_NCM_NCAP_MAX_DATAGRAM_SIZE) {
- __le16 *max_datagram_size;
+ __le16 max_datagram_size;
u16 eth_max_sz = le16_to_cpu(ctx->ether_desc->wMaxSegmentSize);
- max_datagram_size = kzalloc(sizeof(*max_datagram_size),
- GFP_KERNEL);
- if (!max_datagram_size) {
- err = -ENOMEM;
- goto max_dgram_err;
- }
-
- err = usb_control_msg(ctx->udev, usb_rcvctrlpipe(ctx->udev, 0),
+ err = usbnet_write_cmd(dev,
USB_CDC_GET_MAX_DATAGRAM_SIZE,
USB_TYPE_CLASS | USB_DIR_IN
| USB_RECIP_INTERFACE,
- 0, iface_no, max_datagram_size,
- 2, 1000);
+ 0, iface_no, &max_datagram_size,
+ 2);
if (err < 0) {
pr_debug("GET_MAX_DATAGRAM_SIZE failed, use size=%u\n",
CDC_NCM_MIN_DATAGRAM_SIZE);
} else {
ctx->max_datagram_size =
- le16_to_cpu(*max_datagram_size);
+ le16_to_cpu(max_datagram_size);
/* Check Eth descriptor value */
if (ctx->max_datagram_size > eth_max_sz)
ctx->max_datagram_size = eth_max_sz;
@@ -367,23 +343,20 @@ size_err:
/* if value changed, update device */
if (ctx->max_datagram_size !=
- le16_to_cpu(*max_datagram_size)) {
- err = usb_control_msg(ctx->udev,
- usb_sndctrlpipe(ctx->udev, 0),
+ le16_to_cpu(max_datagram_size)) {
+ err = usbnet_write_cmd(dev,
USB_CDC_SET_MAX_DATAGRAM_SIZE,
USB_TYPE_CLASS | USB_DIR_OUT
| USB_RECIP_INTERFACE,
0,
- iface_no, max_datagram_size,
- 2, 1000);
+ iface_no, &max_datagram_size,
+ 2);
if (err < 0)
pr_debug("SET_MAX_DGRAM_SIZE failed\n");
}
}
- kfree(max_datagram_size);
}
-max_dgram_err:
if (ctx->netdev->mtu != (ctx->max_datagram_size - ETH_HLEN))
ctx->netdev->mtu = ctx->max_datagram_size - ETH_HLEN;
--
1.7.9.5
--
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 related
* [PATCH v1 04/11] usbnet: dm9601: apply introduced usb command APIs
From: Ming Lei @ 2012-10-11 12:39 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman
Cc: Oliver Neukum, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Ming Lei
In-Reply-To: <1349959169-9432-1-git-send-email-ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
drivers/net/usb/dm9601.c | 107 +++++++---------------------------------------
1 file changed, 15 insertions(+), 92 deletions(-)
diff --git a/drivers/net/usb/dm9601.c b/drivers/net/usb/dm9601.c
index e0433ce..3f554c1 100644
--- a/drivers/net/usb/dm9601.c
+++ b/drivers/net/usb/dm9601.c
@@ -56,27 +56,12 @@
static int dm_read(struct usbnet *dev, u8 reg, u16 length, void *data)
{
- void *buf;
- int err = -ENOMEM;
-
- netdev_dbg(dev->net, "dm_read() reg=0x%02x length=%d\n", reg, length);
-
- buf = kmalloc(length, GFP_KERNEL);
- if (!buf)
- goto out;
-
- err = usb_control_msg(dev->udev,
- usb_rcvctrlpipe(dev->udev, 0),
- DM_READ_REGS,
- USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
- 0, reg, buf, length, USB_CTRL_SET_TIMEOUT);
- if (err == length)
- memcpy(data, buf, length);
- else if (err >= 0)
+ int err;
+ err = usbnet_read_cmd(dev, DM_READ_REGS,
+ USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+ 0, reg, data, length);
+ if(err != length && err >= 0)
err = -EINVAL;
- kfree(buf);
-
- out:
return err;
}
@@ -87,91 +72,29 @@ static int dm_read_reg(struct usbnet *dev, u8 reg, u8 *value)
static int dm_write(struct usbnet *dev, u8 reg, u16 length, void *data)
{
- void *buf = NULL;
- int err = -ENOMEM;
-
- netdev_dbg(dev->net, "dm_write() reg=0x%02x, length=%d\n", reg, length);
+ int err;
+ err = usbnet_write_cmd(dev, DM_WRITE_REGS,
+ USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+ 0, reg, data, length);
- if (data) {
- buf = kmemdup(data, length, GFP_KERNEL);
- if (!buf)
- goto out;
- }
-
- err = usb_control_msg(dev->udev,
- usb_sndctrlpipe(dev->udev, 0),
- DM_WRITE_REGS,
- USB_DIR_OUT | USB_TYPE_VENDOR |USB_RECIP_DEVICE,
- 0, reg, buf, length, USB_CTRL_SET_TIMEOUT);
- kfree(buf);
if (err >= 0 && err < length)
err = -EINVAL;
- out:
return err;
}
static int dm_write_reg(struct usbnet *dev, u8 reg, u8 value)
{
- netdev_dbg(dev->net, "dm_write_reg() reg=0x%02x, value=0x%02x\n",
- reg, value);
- return usb_control_msg(dev->udev,
- usb_sndctrlpipe(dev->udev, 0),
- DM_WRITE_REG,
- USB_DIR_OUT | USB_TYPE_VENDOR |USB_RECIP_DEVICE,
- value, reg, NULL, 0, USB_CTRL_SET_TIMEOUT);
-}
-
-static void dm_write_async_callback(struct urb *urb)
-{
- struct usb_ctrlrequest *req = (struct usb_ctrlrequest *)urb->context;
- int status = urb->status;
-
- if (status < 0)
- printk(KERN_DEBUG "dm_write_async_callback() failed with %d\n",
- status);
-
- kfree(req);
- usb_free_urb(urb);
+ return usbnet_write_cmd(dev, DM_WRITE_REGS,
+ USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+ value, reg, NULL, 0);
}
static void dm_write_async_helper(struct usbnet *dev, u8 reg, u8 value,
u16 length, void *data)
{
- struct usb_ctrlrequest *req;
- struct urb *urb;
- int status;
-
- urb = usb_alloc_urb(0, GFP_ATOMIC);
- if (!urb) {
- netdev_err(dev->net, "Error allocating URB in dm_write_async_helper!\n");
- return;
- }
-
- req = kmalloc(sizeof(struct usb_ctrlrequest), GFP_ATOMIC);
- if (!req) {
- netdev_err(dev->net, "Failed to allocate memory for control request\n");
- usb_free_urb(urb);
- return;
- }
-
- req->bRequestType = USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE;
- req->bRequest = length ? DM_WRITE_REGS : DM_WRITE_REG;
- req->wValue = cpu_to_le16(value);
- req->wIndex = cpu_to_le16(reg);
- req->wLength = cpu_to_le16(length);
-
- usb_fill_control_urb(urb, dev->udev,
- usb_sndctrlpipe(dev->udev, 0),
- (void *)req, data, length,
- dm_write_async_callback, req);
^ permalink raw reply related
* [PATCH v1 05/11] usbnet: int51x1: apply introduced usb command APIs
From: Ming Lei @ 2012-10-11 12:39 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman
Cc: Oliver Neukum, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Ming Lei
In-Reply-To: <1349959169-9432-1-git-send-email-ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
drivers/net/usb/int51x1.c | 52 +++------------------------------------------
1 file changed, 3 insertions(+), 49 deletions(-)
diff --git a/drivers/net/usb/int51x1.c b/drivers/net/usb/int51x1.c
index 8de6417..ace9e74 100644
--- a/drivers/net/usb/int51x1.c
+++ b/drivers/net/usb/int51x1.c
@@ -116,23 +116,8 @@ static struct sk_buff *int51x1_tx_fixup(struct usbnet *dev,
return skb;
}
-static void int51x1_async_cmd_callback(struct urb *urb)
-{
- struct usb_ctrlrequest *req = (struct usb_ctrlrequest *)urb->context;
- int status = urb->status;
-
- if (status < 0)
- dev_warn(&urb->dev->dev, "async callback failed with %d\n", status);
-
- kfree(req);
- usb_free_urb(urb);
-}
-
static void int51x1_set_multicast(struct net_device *netdev)
{
- struct usb_ctrlrequest *req;
- int status;
- struct urb *urb;
struct usbnet *dev = netdev_priv(netdev);
u16 filter = PACKET_TYPE_DIRECTED | PACKET_TYPE_BROADCAST;
@@ -149,40 +134,9 @@ static void int51x1_set_multicast(struct net_device *netdev)
netdev_dbg(dev->net, "receive own packets only\n");
}
- urb = usb_alloc_urb(0, GFP_ATOMIC);
- if (!urb) {
- netdev_warn(dev->net, "Error allocating URB\n");
- return;
- }
-
- req = kmalloc(sizeof(*req), GFP_ATOMIC);
- if (!req) {
- netdev_warn(dev->net, "Error allocating control msg\n");
- goto out;
- }
-
- req->bRequestType = USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE;
- req->bRequest = SET_ETHERNET_PACKET_FILTER;
- req->wValue = cpu_to_le16(filter);
- req->wIndex = 0;
- req->wLength = 0;
-
- usb_fill_control_urb(urb, dev->udev, usb_sndctrlpipe(dev->udev, 0),
- (void *)req, NULL, 0,
- int51x1_async_cmd_callback,
- (void *)req);
^ permalink raw reply related
* [PATCH v1 06/11] usbnet: mcs7830: apply introduced usb command APIs
From: Ming Lei @ 2012-10-11 12:39 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman
Cc: Oliver Neukum, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Ming Lei
In-Reply-To: <1349959169-9432-1-git-send-email-ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
drivers/net/usb/mcs7830.c | 85 ++++-----------------------------------------
1 file changed, 6 insertions(+), 79 deletions(-)
diff --git a/drivers/net/usb/mcs7830.c b/drivers/net/usb/mcs7830.c
index 03c2d8d..db46a68 100644
--- a/drivers/net/usb/mcs7830.c
+++ b/drivers/net/usb/mcs7830.c
@@ -123,93 +123,20 @@ static const char driver_name[] = "MOSCHIP usb-ethernet driver";
static int mcs7830_get_reg(struct usbnet *dev, u16 index, u16 size, void *data)
{
- struct usb_device *xdev = dev->udev;
- int ret;
- void *buffer;
-
- buffer = kmalloc(size, GFP_NOIO);
- if (buffer == NULL)
- return -ENOMEM;
-
- ret = usb_control_msg(xdev, usb_rcvctrlpipe(xdev, 0), MCS7830_RD_BREQ,
- MCS7830_RD_BMREQ, 0x0000, index, buffer,
- size, MCS7830_CTRL_TIMEOUT);
- memcpy(data, buffer, size);
- kfree(buffer);
-
- return ret;
+ return usbnet_read_cmd(dev, MCS7830_RD_BREQ, MCS7830_RD_BMREQ,
+ 0x0000, index, data, size);
}
static int mcs7830_set_reg(struct usbnet *dev, u16 index, u16 size, const void *data)
{
- struct usb_device *xdev = dev->udev;
- int ret;
- void *buffer;
-
- buffer = kmemdup(data, size, GFP_NOIO);
- if (buffer == NULL)
- return -ENOMEM;
-
- ret = usb_control_msg(xdev, usb_sndctrlpipe(xdev, 0), MCS7830_WR_BREQ,
- MCS7830_WR_BMREQ, 0x0000, index, buffer,
- size, MCS7830_CTRL_TIMEOUT);
- kfree(buffer);
- return ret;
-}
-
-static void mcs7830_async_cmd_callback(struct urb *urb)
-{
- struct usb_ctrlrequest *req = (struct usb_ctrlrequest *)urb->context;
- int status = urb->status;
-
- if (status < 0)
- printk(KERN_DEBUG "%s() failed with %d\n",
- __func__, status);
-
- kfree(req);
- usb_free_urb(urb);
+ return usbnet_write_cmd(dev, MCS7830_WR_BREQ, MCS7830_WR_BMREQ,
+ 0x0000, index, data, size);
}
static void mcs7830_set_reg_async(struct usbnet *dev, u16 index, u16 size, void *data)
{
- struct usb_ctrlrequest *req;
- int ret;
- struct urb *urb;
-
- urb = usb_alloc_urb(0, GFP_ATOMIC);
- if (!urb) {
- dev_dbg(&dev->udev->dev,
- "Error allocating URB in write_cmd_async!\n");
- return;
- }
-
- req = kmalloc(sizeof *req, GFP_ATOMIC);
- if (!req) {
- dev_err(&dev->udev->dev,
- "Failed to allocate memory for control request\n");
- goto out;
- }
- req->bRequestType = MCS7830_WR_BMREQ;
- req->bRequest = MCS7830_WR_BREQ;
- req->wValue = 0;
- req->wIndex = cpu_to_le16(index);
- req->wLength = cpu_to_le16(size);
-
- usb_fill_control_urb(urb, dev->udev,
- usb_sndctrlpipe(dev->udev, 0),
- (void *)req, data, size,
- mcs7830_async_cmd_callback, req);
^ permalink raw reply related
* [PATCH v1 07/11] usbnet: net1080: apply introduced usb command APIs
From: Ming Lei @ 2012-10-11 12:39 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman
Cc: Oliver Neukum, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Ming Lei
In-Reply-To: <1349959169-9432-1-git-send-email-ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
drivers/net/usb/net1080.c | 110 +++++++++++++--------------------------------
1 file changed, 30 insertions(+), 80 deletions(-)
diff --git a/drivers/net/usb/net1080.c b/drivers/net/usb/net1080.c
index c062a3e..93e0716 100644
--- a/drivers/net/usb/net1080.c
+++ b/drivers/net/usb/net1080.c
@@ -109,13 +109,11 @@ struct nc_trailer {
static int
nc_vendor_read(struct usbnet *dev, u8 req, u8 regnum, u16 *retval_ptr)
{
- int status = usb_control_msg(dev->udev,
- usb_rcvctrlpipe(dev->udev, 0),
- req,
- USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
- 0, regnum,
- retval_ptr, sizeof *retval_ptr,
- USB_CTRL_GET_TIMEOUT);
+ int status = usbnet_read_cmd(dev, req,
+ USB_DIR_IN | USB_TYPE_VENDOR |
+ USB_RECIP_DEVICE,
+ 0, regnum, retval_ptr,
+ sizeof *retval_ptr);
if (status > 0)
status = 0;
if (!status)
@@ -133,13 +131,9 @@ nc_register_read(struct usbnet *dev, u8 regnum, u16 *retval_ptr)
static void
nc_vendor_write(struct usbnet *dev, u8 req, u8 regnum, u16 value)
{
- usb_control_msg(dev->udev,
- usb_sndctrlpipe(dev->udev, 0),
- req,
- USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
- value, regnum,
- NULL, 0, // data is in setup packet
- USB_CTRL_SET_TIMEOUT);
+ usbnet_write_cmd(dev, req,
+ USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+ value, regnum, NULL, 0);
}
static inline void
@@ -288,37 +282,34 @@ static inline void nc_dump_ttl(struct usbnet *dev, u16 ttl)
static int net1080_reset(struct usbnet *dev)
{
u16 usbctl, status, ttl;
- u16 *vp = kmalloc(sizeof (u16), GFP_KERNEL);
+ u16 vp;
int retval;
- if (!vp)
- return -ENOMEM;
-
// nc_dump_registers(dev);
- if ((retval = nc_register_read(dev, REG_STATUS, vp)) < 0) {
+ if ((retval = nc_register_read(dev, REG_STATUS, &vp)) < 0) {
netdev_dbg(dev->net, "can't read %s-%s status: %d\n",
dev->udev->bus->bus_name, dev->udev->devpath, retval);
goto done;
}
- status = *vp;
+ status = vp;
nc_dump_status(dev, status);
- if ((retval = nc_register_read(dev, REG_USBCTL, vp)) < 0) {
+ if ((retval = nc_register_read(dev, REG_USBCTL, &vp)) < 0) {
netdev_dbg(dev->net, "can't read USBCTL, %d\n", retval);
goto done;
}
- usbctl = *vp;
+ usbctl = vp;
nc_dump_usbctl(dev, usbctl);
nc_register_write(dev, REG_USBCTL,
USBCTL_FLUSH_THIS | USBCTL_FLUSH_OTHER);
- if ((retval = nc_register_read(dev, REG_TTL, vp)) < 0) {
+ if ((retval = nc_register_read(dev, REG_TTL, &vp)) < 0) {
netdev_dbg(dev->net, "can't read TTL, %d\n", retval);
goto done;
}
- ttl = *vp;
+ ttl = vp;
// nc_dump_ttl(dev, ttl);
nc_register_write(dev, REG_TTL,
@@ -331,7 +322,6 @@ static int net1080_reset(struct usbnet *dev)
retval = 0;
done:
- kfree(vp);
return retval;
}
@@ -339,13 +329,10 @@ static int net1080_check_connect(struct usbnet *dev)
{
int retval;
u16 status;
- u16 *vp = kmalloc(sizeof (u16), GFP_KERNEL);
+ u16 vp;
- if (!vp)
- return -ENOMEM;
- retval = nc_register_read(dev, REG_STATUS, vp);
- status = *vp;
- kfree(vp);
+ retval = nc_register_read(dev, REG_STATUS, &vp);
+ status = vp;
if (retval != 0) {
netdev_dbg(dev->net, "net1080_check_conn read - %d\n", retval);
return retval;
@@ -355,59 +342,22 @@ static int net1080_check_connect(struct usbnet *dev)
return 0;
}
-static void nc_flush_complete(struct urb *urb)
-{
- kfree(urb->context);
- usb_free_urb(urb);
-}
-
static void nc_ensure_sync(struct usbnet *dev)
{
- dev->frame_errors++;
- if (dev->frame_errors > 5) {
- struct urb *urb;
- struct usb_ctrlrequest *req;
- int status;
-
- /* Send a flush */
- urb = usb_alloc_urb(0, GFP_ATOMIC);
- if (!urb)
- return;
-
- req = kmalloc(sizeof *req, GFP_ATOMIC);
- if (!req) {
- usb_free_urb(urb);
- return;
- }
+ if (++dev->frame_errors <= 5)
+ return;
- req->bRequestType = USB_DIR_OUT
- | USB_TYPE_VENDOR
- | USB_RECIP_DEVICE;
- req->bRequest = REQUEST_REGISTER;
- req->wValue = cpu_to_le16(USBCTL_FLUSH_THIS
- | USBCTL_FLUSH_OTHER);
- req->wIndex = cpu_to_le16(REG_USBCTL);
- req->wLength = cpu_to_le16(0);
-
- /* queue an async control request, we don't need
- * to do anything when it finishes except clean up.
- */
- usb_fill_control_urb(urb, dev->udev,
- usb_sndctrlpipe(dev->udev, 0),
- (unsigned char *) req,
- NULL, 0,
- nc_flush_complete, req);
- status = usb_submit_urb(urb, GFP_ATOMIC);
- if (status) {
- kfree(req);
- usb_free_urb(urb);
- return;
- }
+ if (usbnet_write_cmd_async(dev, REQUEST_REGISTER,
+ USB_DIR_OUT | USB_TYPE_VENDOR |
+ USB_RECIP_DEVICE,
+ USBCTL_FLUSH_THIS |
+ USBCTL_FLUSH_OTHER,
+ REG_USBCTL, NULL, 0))
+ return;
- netif_dbg(dev, rx_err, dev->net,
- "flush net1080; too many framing errors\n");
- dev->frame_errors = 0;
- }
+ netif_dbg(dev, rx_err, dev->net,
+ "flush net1080; too many framing errors\n");
+ dev->frame_errors = 0;
}
static int net1080_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
--
1.7.9.5
--
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 related
* [PATCH v1 11/11] usbnet: smsc95xx: apply introduced usb command APIs
From: Ming Lei @ 2012-10-11 12:39 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman
Cc: Oliver Neukum, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Ming Lei
In-Reply-To: <1349959169-9432-1-git-send-email-ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
drivers/net/usb/smsc95xx.c | 115 +++++++++++---------------------------------
1 file changed, 27 insertions(+), 88 deletions(-)
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 7479a57..1730f75 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -65,11 +65,6 @@ struct smsc95xx_priv {
spinlock_t mac_cr_lock;
};
-struct usb_context {
- struct usb_ctrlrequest req;
- struct usbnet *dev;
-};
-
static bool turbo_mode = true;
module_param(turbo_mode, bool, 0644);
MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx transaction");
@@ -77,25 +72,20 @@ MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx transaction");
static int __must_check smsc95xx_read_reg(struct usbnet *dev, u32 index,
u32 *data)
{
- u32 *buf = kmalloc(4, GFP_KERNEL);
+ u32 buf;
int ret;
BUG_ON(!dev);
- if (!buf)
- return -ENOMEM;
-
- ret = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
- USB_VENDOR_REQUEST_READ_REGISTER,
- USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
- 00, index, buf, 4, USB_CTRL_GET_TIMEOUT);
-
+ ret = usbnet_read_cmd(dev, USB_VENDOR_REQUEST_READ_REGISTER,
+ USB_DIR_IN | USB_TYPE_VENDOR |
+ USB_RECIP_DEVICE,
+ 0, index, &buf, 4);
if (unlikely(ret < 0))
netdev_warn(dev->net, "Failed to read register index 0x%08x\n", index);
- le32_to_cpus(buf);
- *data = *buf;
- kfree(buf);
+ le32_to_cpus(&buf);
+ *data = buf;
return ret;
}
@@ -103,27 +93,22 @@ static int __must_check smsc95xx_read_reg(struct usbnet *dev, u32 index,
static int __must_check smsc95xx_write_reg(struct usbnet *dev, u32 index,
u32 data)
{
- u32 *buf = kmalloc(4, GFP_KERNEL);
+ u32 buf;
int ret;
BUG_ON(!dev);
- if (!buf)
- return -ENOMEM;
-
- *buf = data;
- cpu_to_le32s(buf);
+ buf = data;
+ cpu_to_le32s(&buf);
- ret = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
- USB_VENDOR_REQUEST_WRITE_REGISTER,
- USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
- 00, index, buf, 4, USB_CTRL_SET_TIMEOUT);
+ ret = usbnet_write_cmd(dev, USB_VENDOR_REQUEST_WRITE_REGISTER,
+ USB_DIR_OUT | USB_TYPE_VENDOR |
+ USB_RECIP_DEVICE,
+ 0, index, &buf, 4);
if (unlikely(ret < 0))
netdev_warn(dev->net, "Failed to write register index 0x%08x\n", index);
- kfree(buf);
-
return ret;
}
@@ -132,11 +117,8 @@ static int smsc95xx_set_feature(struct usbnet *dev, u32 feature)
if (WARN_ON_ONCE(!dev))
return -EINVAL;
- cpu_to_le32s(&feature);
-
- return usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
- USB_REQ_SET_FEATURE, USB_RECIP_DEVICE, feature, 0, NULL, 0,
- USB_CTRL_SET_TIMEOUT);
+ return usbnet_write_cmd(dev, USB_REQ_SET_FEATURE,
+ USB_RECIP_DEVICE, feature, 0, NULL, 0);
}
static int smsc95xx_clear_feature(struct usbnet *dev, u32 feature)
@@ -144,11 +126,8 @@ static int smsc95xx_clear_feature(struct usbnet *dev, u32 feature)
if (WARN_ON_ONCE(!dev))
return -EINVAL;
- cpu_to_le32s(&feature);
-
- return usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
- USB_REQ_CLEAR_FEATURE, USB_RECIP_DEVICE, feature, 0, NULL, 0,
- USB_CTRL_SET_TIMEOUT);
+ return usbnet_write_cmd(dev, USB_REQ_CLEAR_FEATURE,
+ USB_RECIP_DEVICE, feature, 0, NULL, 0);
}
/* Loop until the read is completed with timeout
@@ -350,60 +329,20 @@ static int smsc95xx_write_eeprom(struct usbnet *dev, u32 offset, u32 length,
return 0;
}
-static void smsc95xx_async_cmd_callback(struct urb *urb)
-{
- struct usb_context *usb_context = urb->context;
- struct usbnet *dev = usb_context->dev;
- int status = urb->status;
-
- check_warn(status, "async callback failed with %d\n", status);
-
- kfree(usb_context);
- usb_free_urb(urb);
-}
-
static int __must_check smsc95xx_write_reg_async(struct usbnet *dev, u16 index,
u32 *data)
{
- struct usb_context *usb_context;
- int status;
- struct urb *urb;
const u16 size = 4;
+ int ret;
- urb = usb_alloc_urb(0, GFP_ATOMIC);
- if (!urb) {
- netdev_warn(dev->net, "Error allocating URB\n");
- return -ENOMEM;
- }
-
- usb_context = kmalloc(sizeof(struct usb_context), GFP_ATOMIC);
- if (usb_context == NULL) {
- netdev_warn(dev->net, "Error allocating control msg\n");
- usb_free_urb(urb);
- return -ENOMEM;
- }
-
- usb_context->req.bRequestType =
- USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE;
- usb_context->req.bRequest = USB_VENDOR_REQUEST_WRITE_REGISTER;
- usb_context->req.wValue = 00;
- usb_context->req.wIndex = cpu_to_le16(index);
- usb_context->req.wLength = cpu_to_le16(size);
-
- usb_fill_control_urb(urb, dev->udev, usb_sndctrlpipe(dev->udev, 0),
- (void *)&usb_context->req, data, size,
- smsc95xx_async_cmd_callback,
- (void *)usb_context);
-
- status = usb_submit_urb(urb, GFP_ATOMIC);
- if (status < 0) {
- netdev_warn(dev->net, "Error submitting control msg, sts=%d\n",
- status);
- kfree(usb_context);
- usb_free_urb(urb);
- }
^ permalink raw reply related
* [PATCH v1 02/11] usbnet: asix: apply introduced usb command APIs
From: Ming Lei @ 2012-10-11 12:39 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman
Cc: Oliver Neukum, netdev, linux-usb, Ming Lei
In-Reply-To: <1349959169-9432-1-git-send-email-ming.lei@canonical.com>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/net/usb/asix_common.c | 117 +++++------------------------------------
1 file changed, 13 insertions(+), 104 deletions(-)
diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
index 774d9ce..50d1673 100644
--- a/drivers/net/usb/asix_common.c
+++ b/drivers/net/usb/asix_common.c
@@ -25,121 +25,30 @@
int asix_read_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,
u16 size, void *data)
{
- void *buf;
- int err = -ENOMEM;
-
- netdev_dbg(dev->net, "asix_read_cmd() cmd=0x%02x value=0x%04x index=0x%04x size=%d\n",
- cmd, value, index, size);
-
- buf = kmalloc(size, GFP_KERNEL);
- if (!buf)
- goto out;
-
- err = usb_control_msg(
- dev->udev,
- usb_rcvctrlpipe(dev->udev, 0),
- cmd,
- USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
- value,
- index,
- buf,
- size,
- USB_CTRL_GET_TIMEOUT);
- if (err == size)
- memcpy(data, buf, size);
- else if (err >= 0)
- err = -EINVAL;
- kfree(buf);
+ int ret;
+ ret = usbnet_read_cmd(dev, cmd,
+ USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+ value, index, data, size);
-out:
- return err;
+ if (ret != size && ret >= 0)
+ return -EINVAL;
+ return ret;
}
int asix_write_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,
u16 size, void *data)
{
- void *buf = NULL;
- int err = -ENOMEM;
-
- netdev_dbg(dev->net, "asix_write_cmd() cmd=0x%02x value=0x%04x index=0x%04x size=%d\n",
- cmd, value, index, size);
-
- if (data) {
- buf = kmemdup(data, size, GFP_KERNEL);
- if (!buf)
- goto out;
- }
-
- err = usb_control_msg(
- dev->udev,
- usb_sndctrlpipe(dev->udev, 0),
- cmd,
- USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
- value,
- index,
- buf,
- size,
- USB_CTRL_SET_TIMEOUT);
- kfree(buf);
-
-out:
- return err;
-}
-
-static void asix_async_cmd_callback(struct urb *urb)
-{
- struct usb_ctrlrequest *req = (struct usb_ctrlrequest *)urb->context;
- int status = urb->status;
-
- if (status < 0)
- printk(KERN_DEBUG "asix_async_cmd_callback() failed with %d",
- status);
-
- kfree(req);
- usb_free_urb(urb);
+ return usbnet_write_cmd(dev, cmd,
+ USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+ value, index, data, size);
}
void asix_write_cmd_async(struct usbnet *dev, u8 cmd, u16 value, u16 index,
u16 size, void *data)
{
- struct usb_ctrlrequest *req;
- int status;
- struct urb *urb;
-
- netdev_dbg(dev->net, "asix_write_cmd_async() cmd=0x%02x value=0x%04x index=0x%04x size=%d\n",
- cmd, value, index, size);
-
- urb = usb_alloc_urb(0, GFP_ATOMIC);
- if (!urb) {
- netdev_err(dev->net, "Error allocating URB in write_cmd_async!\n");
- return;
- }
-
- req = kmalloc(sizeof(struct usb_ctrlrequest), GFP_ATOMIC);
- if (!req) {
- netdev_err(dev->net, "Failed to allocate memory for control request\n");
- usb_free_urb(urb);
- return;
- }
-
- req->bRequestType = USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE;
- req->bRequest = cmd;
- req->wValue = cpu_to_le16(value);
- req->wIndex = cpu_to_le16(index);
- req->wLength = cpu_to_le16(size);
-
- usb_fill_control_urb(urb, dev->udev,
- usb_sndctrlpipe(dev->udev, 0),
- (void *)req, data, size,
- asix_async_cmd_callback, req);
-
- status = usb_submit_urb(urb, GFP_ATOMIC);
- if (status < 0) {
- netdev_err(dev->net, "Error submitting the control message: status=%d\n",
- status);
- kfree(req);
- usb_free_urb(urb);
- }
+ usbnet_write_cmd_async(dev, cmd,
+ USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+ value, index, data, size);
}
int asix_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
--
1.7.9.5
^ permalink raw reply related
* Re: alignment faults in 3.6
From: Eric Dumazet @ 2012-10-11 12:40 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel, Russell King - ARM Linux, Jon Masters, netdev,
Måns Rullgård, David Laight, Rob Herring
In-Reply-To: <201210111228.25995.arnd@arndb.de>
On Thu, 2012-10-11 at 12:28 +0000, Arnd Bergmann wrote:
>
> Rob Herring as the original reporter has dropped off the Cc list, adding
> him back.
>
> I assume that the calxeda xgmac driver is the culprit then. It uses
> netdev_alloc_skb() rather than netdev_alloc_skb_ip_align() in
> xgmac_rx_refill but it is not clear whether it does so intentionally
> or by accident.
Thanks Arnd
It seems an accident, since driver doesnt check skb->data alignment at
all (this can change with SLAB debug on/off)
It also incorrectly adds 64 bytes to bfsize, there is no need for this.
(or if its needed, a comment would be nice, because on prior kernels,
this makes skb->head allocations uses kmalloc-4096 instead of
kmalloc-2048 slab cache... With 3.7 its less an issue now we use order-3
pages to deliver fragments for rx skbs
So the following patch should fix the alignment, and makes driver uses
half memory than before for stable kernels
diff --git a/drivers/net/ethernet/calxeda/xgmac.c b/drivers/net/ethernet/calxeda/xgmac.c
index 16814b3..a895e18 100644
--- a/drivers/net/ethernet/calxeda/xgmac.c
+++ b/drivers/net/ethernet/calxeda/xgmac.c
@@ -671,7 +671,8 @@ static void xgmac_rx_refill(struct xgmac_priv *priv)
p = priv->dma_rx + entry;
if (priv->rx_skbuff[entry] == NULL) {
- skb = netdev_alloc_skb(priv->dev, priv->dma_buf_sz);
+ skb = netdev_alloc_skb_ip_align(priv->dev,
+ priv->dma_buf_sz);
if (unlikely(skb == NULL))
break;
@@ -703,7 +704,7 @@ static int xgmac_dma_desc_rings_init(struct net_device *dev)
/* Set the Buffer size according to the MTU;
* indeed, in case of jumbo we need to bump-up the buffer sizes.
*/
- bfsize = ALIGN(dev->mtu + ETH_HLEN + ETH_FCS_LEN + NET_IP_ALIGN + 64,
+ bfsize = ALIGN(dev->mtu + ETH_HLEN + ETH_FCS_LEN,
64);
netdev_dbg(priv->dev, "mtu [%d] bfsize [%d]\n", dev->mtu, bfsize);
^ permalink raw reply related
* [PATCH v1 08/11] usbnet: plusb: apply introduced usb command APIs
From: Ming Lei @ 2012-10-11 12:39 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman
Cc: Oliver Neukum, netdev, linux-usb, Ming Lei
In-Reply-To: <1349959169-9432-1-git-send-email-ming.lei@canonical.com>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/net/usb/plusb.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/net/usb/plusb.c b/drivers/net/usb/plusb.c
index 4584b9a..0fcc8e6 100644
--- a/drivers/net/usb/plusb.c
+++ b/drivers/net/usb/plusb.c
@@ -71,13 +71,10 @@
static inline int
pl_vendor_req(struct usbnet *dev, u8 req, u8 val, u8 index)
{
- return usb_control_msg(dev->udev,
- usb_rcvctrlpipe(dev->udev, 0),
- req,
- USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
- val, index,
- NULL, 0,
- USB_CTRL_GET_TIMEOUT);
+ return usbnet_read_cmd(dev, req,
+ USB_DIR_IN | USB_TYPE_VENDOR |
+ USB_RECIP_DEVICE,
+ val, index, NULL, 0);
}
static inline int
--
1.7.9.5
^ permalink raw reply related
* [PATCH v1 09/11] usbnet: sierra_net: apply introduced usb command APIs
From: Ming Lei @ 2012-10-11 12:39 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman
Cc: Oliver Neukum, netdev, linux-usb, Ming Lei
In-Reply-To: <1349959169-9432-1-git-send-email-ming.lei@canonical.com>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/net/usb/sierra_net.c | 45 ++++++++++++++++--------------------------
1 file changed, 17 insertions(+), 28 deletions(-)
diff --git a/drivers/net/usb/sierra_net.c b/drivers/net/usb/sierra_net.c
index c27d277..eb5c7a8 100644
--- a/drivers/net/usb/sierra_net.c
+++ b/drivers/net/usb/sierra_net.c
@@ -311,10 +311,9 @@ static int sierra_net_send_cmd(struct usbnet *dev,
struct sierra_net_data *priv = sierra_net_get_private(dev);
int status;
- status = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
- USB_CDC_SEND_ENCAPSULATED_COMMAND,
- USB_DIR_OUT|USB_TYPE_CLASS|USB_RECIP_INTERFACE, 0,
- priv->ifnum, cmd, cmdlen, USB_CTRL_SET_TIMEOUT);
+ status = usbnet_write_cmd(dev, USB_CDC_SEND_ENCAPSULATED_COMMAND,
+ USB_DIR_OUT|USB_TYPE_CLASS|USB_RECIP_INTERFACE,
+ 0, priv->ifnum, cmd, cmdlen);
if (status != cmdlen && status != -ENODEV)
netdev_err(dev->net, "Submit %s failed %d\n", cmd_name, status);
@@ -632,32 +631,22 @@ static int sierra_net_change_mtu(struct net_device *net, int new_mtu)
static int sierra_net_get_fw_attr(struct usbnet *dev, u16 *datap)
{
int result = 0;
- u16 *attrdata;
-
- attrdata = kmalloc(sizeof(*attrdata), GFP_KERNEL);
- if (!attrdata)
- return -ENOMEM;
-
- result = usb_control_msg(
- dev->udev,
- usb_rcvctrlpipe(dev->udev, 0),
- /* _u8 vendor specific request */
- SWI_USB_REQUEST_GET_FW_ATTR,
- USB_DIR_IN | USB_TYPE_VENDOR, /* __u8 request type */
- 0x0000, /* __u16 value not used */
- 0x0000, /* __u16 index not used */
- attrdata, /* char *data */
- sizeof(*attrdata), /* __u16 size */
- USB_CTRL_SET_TIMEOUT); /* int timeout */
-
- if (result < 0) {
- kfree(attrdata);
+ u16 attrdata;
+
+ result = usbnet_read_cmd(dev,
+ /* _u8 vendor specific request */
+ SWI_USB_REQUEST_GET_FW_ATTR,
+ USB_DIR_IN | USB_TYPE_VENDOR, /* __u8 request type */
+ 0x0000, /* __u16 value not used */
+ 0x0000, /* __u16 index not used */
+ &attrdata, /* char *data */
+ sizeof(attrdata) /* __u16 size */
+ );
+
+ if (result < 0)
return -EIO;
- }
-
- *datap = le16_to_cpu(*attrdata);
- kfree(attrdata);
+ *datap = le16_to_cpu(attrdata);
return result;
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH v1 10/11] usbnet: smsc75xx: apply introduced usb command APIs
From: Ming Lei @ 2012-10-11 12:39 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman
Cc: Oliver Neukum, netdev, linux-usb, Ming Lei
In-Reply-To: <1349959169-9432-1-git-send-email-ming.lei@canonical.com>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/net/usb/smsc75xx.c | 39 ++++++++++++++-------------------------
1 file changed, 14 insertions(+), 25 deletions(-)
diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index b77ae76..1baa53a 100644
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -85,26 +85,21 @@ MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx transaction");
static int __must_check smsc75xx_read_reg(struct usbnet *dev, u32 index,
u32 *data)
{
- u32 *buf = kmalloc(4, GFP_KERNEL);
+ u32 buf;
int ret;
BUG_ON(!dev);
- if (!buf)
- return -ENOMEM;
-
- ret = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
- USB_VENDOR_REQUEST_READ_REGISTER,
- USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
- 00, index, buf, 4, USB_CTRL_GET_TIMEOUT);
-
+ ret = usbnet_read_cmd(dev, USB_VENDOR_REQUEST_READ_REGISTER,
+ USB_DIR_IN | USB_TYPE_VENDOR |
+ USB_RECIP_DEVICE,
+ 0, index, &buf, 4);
if (unlikely(ret < 0))
netdev_warn(dev->net,
"Failed to read reg index 0x%08x: %d", index, ret);
- le32_to_cpus(buf);
- *data = *buf;
- kfree(buf);
+ le32_to_cpus(&buf);
+ *data = buf;
return ret;
}
@@ -112,28 +107,22 @@ static int __must_check smsc75xx_read_reg(struct usbnet *dev, u32 index,
static int __must_check smsc75xx_write_reg(struct usbnet *dev, u32 index,
u32 data)
{
- u32 *buf = kmalloc(4, GFP_KERNEL);
+ u32 buf;
int ret;
BUG_ON(!dev);
- if (!buf)
- return -ENOMEM;
-
- *buf = data;
- cpu_to_le32s(buf);
-
- ret = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
- USB_VENDOR_REQUEST_WRITE_REGISTER,
- USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
- 00, index, buf, 4, USB_CTRL_SET_TIMEOUT);
+ buf = data;
+ cpu_to_le32s(&buf);
+ ret = usbnet_write_cmd(dev, USB_VENDOR_REQUEST_WRITE_REGISTER,
+ USB_DIR_OUT | USB_TYPE_VENDOR |
+ USB_RECIP_DEVICE,
+ 0, index, &buf, 4);
if (unlikely(ret < 0))
netdev_warn(dev->net,
"Failed to write reg index 0x%08x: %d", index, ret);
- kfree(buf);
-
return ret;
}
--
1.7.9.5
^ permalink raw reply related
* Re: alignment faults in 3.6
From: Maxime Bizon @ 2012-10-11 12:51 UTC (permalink / raw)
To: Eric Dumazet
Cc: Måns Rullgård, Russell King - ARM Linux, netdev,
David Laight, Jon Masters, linux-arm-kernel
In-Reply-To: <1349956456.21172.8820.camel@edumazet-glaptop>
On Thu, 2012-10-11 at 13:54 +0200, Eric Dumazet wrote:
> Of course, if you use a forwarding setup, and the tx driver is not SG
> capable, performance will be bad (You have to copy the data into a
> single skb (linearize the skb))
>
> But in 2012, having to use hardware without SG for a router sounds a bad
> joke (if cpu speed is _also_ too low)
Hey I cannot go back in time, when that hardware was built in 2004 (mips
@250Mhz), it was considered good, and we did manufacture a lot of it, so
it's still maintained.
People run recent kernels on older hardware because they are *encouraged
to do so*.
I fought inside my company to be good kernel citizen, not using
proprietary BSP, rewrite & mainline the drivers, because that was the
community promise: mainline it, we will support it for you, you will get
the latest kernel features for free.
That worked, but with some drawbacks:
- kernel footprint grew that much (we started from 2.4) that it does
not fit in device flash anymore
- performance took a hit each time we upgrade, mostly because of cache
footprint growth.
- as kernel footprint grew, available RAM for conntrack & route cache
entries was smaller each time
But I had to stop upgrading after 2.6.20. Everything below is not
anybody's fault. Bloat is unavoidable for software project that big.
I'm perfectly ok with that, but I don't want to be ridiculed for running
mainline kernel on old hardware.
> Adding get_unaligned() everywhere in linux network stacks is not an
> option.
>
> We actually want to be able to read the code and fix the bugs, not only
> run it on a cheap low end router.
That was not a request, I just needed a clarification.
Documentation/unaligned-memory-access.txt does not say it's a big no-no,
it says you can give unaligned pointers to the networking stack if you
arch can do unaligned access (with an "efficiency" notion).
MIPS and ARM have a software handler for this, and performance wise in
my case it's better to take the faults, a driver writer may think a
benchmark will dictate what to do.
--
Maxime
^ permalink raw reply
* [PATCH] usbnet: Support devices reporting idleness
From: Oliver Neukum @ 2012-10-11 12:50 UTC (permalink / raw)
To: davem, netdev; +Cc: Oliver Neukum, Oliver Neukum
Some device types support a form of power management in which
the device suggests to the host that the device may be suspended
now. Support for that is best located in usbnet.
Signed-off-by: Oliver Neukum <oneukum@suse.de>
---
drivers/net/usb/cdc_eem.c | 4 ++++
drivers/net/usb/usbnet.c | 17 +++++++++++++++++
include/linux/usb/usbnet.h | 2 ++
3 files changed, 23 insertions(+), 0 deletions(-)
diff --git a/drivers/net/usb/cdc_eem.c b/drivers/net/usb/cdc_eem.c
index 434d5af..c81e278 100644
--- a/drivers/net/usb/cdc_eem.c
+++ b/drivers/net/usb/cdc_eem.c
@@ -244,8 +244,12 @@ static int eem_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
* - suspend: peripheral ready to suspend
* - response: suggest N millisec polling
* - response complete: suggest N sec polling
+ *
+ * Suspend is reported and maybe heeded.
*/
case 2: /* Suspend hint */
+ usbnet_device_suggests_idle(dev);
+ continue;
case 3: /* Response hint */
case 4: /* Response complete hint */
continue;
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index fd4b26d..38c534a 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1580,10 +1580,27 @@ int usbnet_resume (struct usb_interface *intf)
tasklet_schedule (&dev->bh);
}
}
+
+ if (test_and_clear_bit(EVENT_DEVICE_REPORT_IDLE, &dev->flags))
+ usb_autopm_get_interface_no_resume(intf);
+
return 0;
}
EXPORT_SYMBOL_GPL(usbnet_resume);
+/*
+ * Either a subdriver implements manage_power, then it is assumed to always
+ * be ready to be suspended or it reports the readiness to be suspended
+ * explicitly
+ */
+void usbnet_device_suggests_idle(struct usbnet *dev)
+{
+ if (!test_and_set_bit(EVENT_DEVICE_REPORT_IDLE, &dev->flags)) {
+ dev->intf->needs_remote_wakeup = 1;
+ usb_autopm_put_interface_async(dev->intf);
+ }
+}
+EXPORT_SYMBOL(usbnet_device_suggests_idle);
/*-------------------------------------------------------------------------*/
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index f87cf62..ddbbb7d 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -68,6 +68,7 @@ struct usbnet {
# define EVENT_RX_PAUSED 5
# define EVENT_DEV_ASLEEP 6
# define EVENT_DEV_OPEN 7
+# define EVENT_DEVICE_REPORT_IDLE 8
};
static inline struct usb_driver *driver_of(struct usb_interface *intf)
@@ -160,6 +161,7 @@ extern int usbnet_probe(struct usb_interface *, const struct usb_device_id *);
extern int usbnet_suspend(struct usb_interface *, pm_message_t);
extern int usbnet_resume(struct usb_interface *);
extern void usbnet_disconnect(struct usb_interface *);
+extern void usbnet_device_suggests_idle(struct usbnet *dev);
/* Drivers that reuse some of the standard USB CDC infrastructure
--
1.7.7
^ permalink raw reply related
* Re: alignment faults in 3.6
From: Eric Dumazet @ 2012-10-11 12:59 UTC (permalink / raw)
To: mbizon
Cc: Måns Rullgård, Russell King - ARM Linux, netdev,
David Laight, Jon Masters, linux-arm-kernel
In-Reply-To: <1349959881.1232.44.camel@sakura.staff.proxad.net>
On Thu, 2012-10-11 at 14:51 +0200, Maxime Bizon wrote:
> Hey I cannot go back in time, when that hardware was built in 2004 (mips
> @250Mhz), it was considered good, and we did manufacture a lot of it, so
> it's still maintained.
>
> People run recent kernels on older hardware because they are *encouraged
> to do so*.
>
> I fought inside my company to be good kernel citizen, not using
> proprietary BSP, rewrite & mainline the drivers, because that was the
> community promise: mainline it, we will support it for you, you will get
> the latest kernel features for free.
>
>
> That worked, but with some drawbacks:
>
> - kernel footprint grew that much (we started from 2.4) that it does
> not fit in device flash anymore
>
> - performance took a hit each time we upgrade, mostly because of cache
> footprint growth.
>
> - as kernel footprint grew, available RAM for conntrack & route cache
> entries was smaller each time
>
>
> But I had to stop upgrading after 2.6.20. Everything below is not
> anybody's fault. Bloat is unavoidable for software project that big.
>
> I'm perfectly ok with that, but I don't want to be ridiculed for running
> mainline kernel on old hardware.
Hmm, I am sorry if you felt that, it was not my intent.
>
>
> > Adding get_unaligned() everywhere in linux network stacks is not an
> > option.
> >
> > We actually want to be able to read the code and fix the bugs, not only
> > run it on a cheap low end router.
>
> That was not a request, I just needed a clarification.
>
> Documentation/unaligned-memory-access.txt does not say it's a big no-no,
> it says you can give unaligned pointers to the networking stack if you
> arch can do unaligned access (with an "efficiency" notion).
>
> MIPS and ARM have a software handler for this, and performance wise in
> my case it's better to take the faults, a driver writer may think a
> benchmark will dictate what to do.
>
Sure, but all this discussion started because one arch apparently did
not like these mis alignments, and some people complained that network
guys would not add _needed_ get_unaligned_xxx() wrappers ...
So far, linux is 20 years old, I dont think we are going to add wrappers
right now. Machines that could have cared are dying anyway.
Please note we still support NET_IP_ALIGN, even if its 0 on x86.
^ permalink raw reply
* Re: alignment faults in 3.6
From: Rob Herring @ 2012-10-11 13:20 UTC (permalink / raw)
To: Eric Dumazet
Cc: Arnd Bergmann, linux-arm-kernel, Russell King - ARM Linux,
Jon Masters, netdev, Måns Rullgård, David Laight
In-Reply-To: <1349959248.21172.8970.camel@edumazet-glaptop>
On 10/11/2012 07:40 AM, Eric Dumazet wrote:
> On Thu, 2012-10-11 at 12:28 +0000, Arnd Bergmann wrote:
>
>>
>> Rob Herring as the original reporter has dropped off the Cc list, adding
>> him back.
>>
>> I assume that the calxeda xgmac driver is the culprit then. It uses
>> netdev_alloc_skb() rather than netdev_alloc_skb_ip_align() in
>> xgmac_rx_refill but it is not clear whether it does so intentionally
>> or by accident.
This in fact does work and eliminates the unaligned traps. However, not
all h/w can do IP aligned DMA (i.MX FEC for example), so I still think
this is a questionable optimization by the compiler. We're saving 1 load
instruction here for data that is likely already in the cache. It may be
legal per the ABI, but the downside of this optimization is much greater
than the upside.
>
> Thanks Arnd
>
> It seems an accident, since driver doesnt check skb->data alignment at
> all (this can change with SLAB debug on/off)
>
> It also incorrectly adds 64 bytes to bfsize, there is no need for this.
I'm pretty sure this was needed as the h/w writes out full bursts of
data, but I'll go back and check.
Rob
> (or if its needed, a comment would be nice, because on prior kernels,
> this makes skb->head allocations uses kmalloc-4096 instead of
> kmalloc-2048 slab cache... With 3.7 its less an issue now we use order-3
> pages to deliver fragments for rx skbs
>
> So the following patch should fix the alignment, and makes driver uses
> half memory than before for stable kernels
>
> diff --git a/drivers/net/ethernet/calxeda/xgmac.c b/drivers/net/ethernet/calxeda/xgmac.c
> index 16814b3..a895e18 100644
> --- a/drivers/net/ethernet/calxeda/xgmac.c
> +++ b/drivers/net/ethernet/calxeda/xgmac.c
> @@ -671,7 +671,8 @@ static void xgmac_rx_refill(struct xgmac_priv *priv)
> p = priv->dma_rx + entry;
>
> if (priv->rx_skbuff[entry] == NULL) {
> - skb = netdev_alloc_skb(priv->dev, priv->dma_buf_sz);
> + skb = netdev_alloc_skb_ip_align(priv->dev,
> + priv->dma_buf_sz);
> if (unlikely(skb == NULL))
> break;
>
> @@ -703,7 +704,7 @@ static int xgmac_dma_desc_rings_init(struct net_device *dev)
> /* Set the Buffer size according to the MTU;
> * indeed, in case of jumbo we need to bump-up the buffer sizes.
> */
> - bfsize = ALIGN(dev->mtu + ETH_HLEN + ETH_FCS_LEN + NET_IP_ALIGN + 64,
> + bfsize = ALIGN(dev->mtu + ETH_HLEN + ETH_FCS_LEN,
> 64);
>
> netdev_dbg(priv->dev, "mtu [%d] bfsize [%d]\n", dev->mtu, bfsize);
>
>
^ permalink raw reply
* Re: alignment faults in 3.6
From: Måns Rullgård @ 2012-10-11 13:32 UTC (permalink / raw)
To: Rob Herring
Cc: Eric Dumazet, Arnd Bergmann, linux-arm-kernel,
Russell King - ARM Linux, Jon Masters, netdev,
Måns Rullgård, David Laight
In-Reply-To: <5076C78E.1020408@gmail.com>
Rob Herring <robherring2@gmail.com> writes:
> On 10/11/2012 07:40 AM, Eric Dumazet wrote:
>> On Thu, 2012-10-11 at 12:28 +0000, Arnd Bergmann wrote:
>>
>>>
>>> Rob Herring as the original reporter has dropped off the Cc list, adding
>>> him back.
>>>
>>> I assume that the calxeda xgmac driver is the culprit then. It uses
>>> netdev_alloc_skb() rather than netdev_alloc_skb_ip_align() in
>>> xgmac_rx_refill but it is not clear whether it does so intentionally
>>> or by accident.
>
> This in fact does work and eliminates the unaligned traps. However, not
> all h/w can do IP aligned DMA (i.MX FEC for example), so I still think
> this is a questionable optimization by the compiler. We're saving 1 load
> instruction here for data that is likely already in the cache. It may be
> legal per the ABI, but the downside of this optimization is much greater
> than the upside.
The compiler is working *exactly* as it should. Merging the loads saves
cycles *and* code size. Many of these added up can make a real difference.
When writing code, you must follow all the rules, whether you like them
or not. Without rules, the compiler would be very limited in the
optimisations it could perform.
Unfortunately, new optimisations occasionally uncover broken code
violating some constraint or other. When this happens, the correct
course of action is to fix the code, not cripple the compiler.
--
Måns Rullgård
mans@mansr.com
^ permalink raw reply
* Re: alignment faults in 3.6
From: Arnd Bergmann @ 2012-10-11 13:35 UTC (permalink / raw)
To: Rob Herring
Cc: Eric Dumazet, linux-arm-kernel, Russell King - ARM Linux,
Jon Masters, netdev, Måns Rullgård, David Laight
In-Reply-To: <5076C78E.1020408@gmail.com>
On Thursday 11 October 2012, Rob Herring wrote:
>
> On 10/11/2012 07:40 AM, Eric Dumazet wrote:
> > On Thu, 2012-10-11 at 12:28 +0000, Arnd Bergmann wrote:
> >
> >>
> >> Rob Herring as the original reporter has dropped off the Cc list, adding
> >> him back.
> >>
> >> I assume that the calxeda xgmac driver is the culprit then. It uses
> >> netdev_alloc_skb() rather than netdev_alloc_skb_ip_align() in
> >> xgmac_rx_refill but it is not clear whether it does so intentionally
> >> or by accident.
>
> This in fact does work and eliminates the unaligned traps. However, not
> all h/w can do IP aligned DMA (i.MX FEC for example), so I still think
> this is a questionable optimization by the compiler. We're saving 1 load
> instruction here for data that is likely already in the cache. It may be
> legal per the ABI, but the downside of this optimization is much greater
> than the upside.
>
What about the other approach that Eric suggested for such hardware
in http://www.spinics.net/lists/arm-kernel/msg200206.html ?
Arnd
^ permalink raw reply
* Re: alignment faults in 3.6
From: Eric Dumazet @ 2012-10-11 13:47 UTC (permalink / raw)
To: Rob Herring
Cc: Arnd Bergmann, linux-arm-kernel, Russell King - ARM Linux,
Jon Masters, netdev, Måns Rullgård, David Laight
In-Reply-To: <5076C78E.1020408@gmail.com>
On Thu, 2012-10-11 at 08:20 -0500, Rob Herring wrote:
> On 10/11/2012 07:40 AM, Eric Dumazet wrote:
> > On Thu, 2012-10-11 at 12:28 +0000, Arnd Bergmann wrote:
> >
> >>
> >> Rob Herring as the original reporter has dropped off the Cc list, adding
> >> him back.
> >>
> >> I assume that the calxeda xgmac driver is the culprit then. It uses
> >> netdev_alloc_skb() rather than netdev_alloc_skb_ip_align() in
> >> xgmac_rx_refill but it is not clear whether it does so intentionally
> >> or by accident.
>
> This in fact does work and eliminates the unaligned traps. However, not
> all h/w can do IP aligned DMA (i.MX FEC for example), so I still think
> this is a questionable optimization by the compiler. We're saving 1 load
> instruction here for data that is likely already in the cache. It may be
> legal per the ABI, but the downside of this optimization is much greater
> than the upside.
Compiler is asked to perform a 32bit load, it does it.
There is no questionable optimization here. Really.
Please stop pretending this, this makes no sense.
As I said, if some h/w cannot do IP aligned DMA, driver can use a
workaround, or a plain memmove() (some drivers seems to do this to work
around this h/w limitation, just grep for memmove() in drivers/net)
>
> >
> > Thanks Arnd
> >
> > It seems an accident, since driver doesnt check skb->data alignment at
> > all (this can change with SLAB debug on/off)
> >
> > It also incorrectly adds 64 bytes to bfsize, there is no need for this.
>
> I'm pretty sure this was needed as the h/w writes out full bursts of
> data, but I'll go back and check.
Maybe the ALIGN() was needed then. But the 64 + NE_IP_ALIGN sounds like
the head room that we allocate/reserve in netdev_alloc_skb_ip_align()
So you allocate this extra room twice.
Thanks
^ permalink raw reply
* regression in bonding driver with VLAN challenged interfaces when 8021q module is loaded
From: Jon Stanley @ 2012-10-11 14:28 UTC (permalink / raw)
To: fubar, andy, netdev, linux-kernel, jpirko
Since commit cc0e40700656b09d93b062ef6c818aa45429d09a, there is a
problem if you have the 8021q module loaded and you then attempt to
enslave a VLAN challenged interface. This is because VLAN 0 is
automatically added to the bond, and the new bond_vlan_used() function
simply checks for an empty list in bond->vlan_list without taking into
account that VLAN0 might be on the interface.
This specifically affects IPoIB interfaces in a bond if you do a
down/up cycle on the bond post-boot (during boot, the bonding module
is loaded prior to the 8021q module, so everything is fine).
Let me know if further information or testing is needed.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox