Re: [PATCH 02/29] usb/gadget: move bind callback into driver struct usb_composite_driver
> Would it make sense to merge patches 2, 4, 5 and 6 into a single one? > They all deal with the same thing (ie. they bring back ->bind callback) > and it seems to me like all those changes belong in a single patch as > otherwise they produce some intermediate states, which don't have clean > purpose. - 2 introduces bind in struct usb_composite_driver. No gadget sets this, only composite does. - 4 adds only _ref so we don't have section missmatch warnings in between. - 5 moves the argument from usb_composite_probe() into struct usb_composite_driver. - 6 moves the argument from usb_gadget_probe_driver() into struct usb_gadget_driver. Each patch changes one logical part and should not break build / functionality during bisect. > For instance this patch fiddles with composite_bind_probe() so that it > allows both the bind argument and driver->bind callback, but than patch > 5 fiddles with this even more removing bind argument. I agree that 2 + 5 do the same thing as 6 but in a different struct. However if you look at changes 2 and 5 than you see that they are bigger than in 6. I would like to keep 4, 5, 6 seperated as they do different things. I could merge 2 and 5 if you really think it makes reading patch easier. > Others may disagree, but I feel that those changes would be simpler to > understand if put as a single patch. Also, even though I wrote three > paragraphs on that issue, I don't have strong feelings, and: > > Acked-by: Michal Nazarewicz Okay, thanks. So I keep things as they are :) Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: changing usbnet's API to better deal with cdc-ncm
On Friday 07 September 2012 11:55:53 Ming Lei wrote:
> On Fri, Sep 7, 2012 at 1:56 AM, Oliver Neukum wrote:
> > On Friday 07 September 2012 00:09:13 Ming Lei wrote:
> >> On Thu, Sep 6, 2012 at 4:30 PM, Bjørn Mork wrote:
> >> > Ming Lei writes:
> >
> >> >> Looks the introduced .tx_bundle is not necessary since .tx_fixup is OK.
> >> >
> >> > The minidriver does not have any information about tx in progress. The
> >>
> >> Inside .tx_fixup, the low level driver will get the tx progress
> >> information.
> >
> > That information changes after tx_fixup
>
> You mean the low level drive can't see if the later transmission
> for the aggregated frame is successful? If so, there is no any complete
Not successful, necessary.
> notification to all low level drivers (with aggregation capability
> or not) now, isn't there?
There doesn't need to be.
> > Well, that is the mistake. Using a timer is a bad idea.
>
> Why is a bad idea? Without a timer, how can usbnet or
> low level driver aggregate the later coming transmit frames to
> form a biger aggregated frame?
By registering demand in an abstract sense. The timer makes sense
only when no other buffers are being transfered.
The timer means that we transmit if no other more packages are coming
down from the upper layers. Now, either the device is still busy or it is not.
While it is busy, we might just as well wait, because the upper layer may
change its mind.
If the device is not busy we worsen latency by waiting for the timer.
In addition the timer causes complications with recursing into usbnet
and hassle with suspend/resume and disconnection.
> >> If we can abstract some common things about aggregation, it should be
> >> meaningful. As far as I know, most of aggregation protocol is very
> >> different,
> >> so almost all aggregation work is only done by low level driver, such as
> >> cdc_ncm.
> >>
> >> If we want to implement some aggregation framework, maybe below is
> >> one solution, correct me if it is wrong.
> >
> > It isn't so much wrong as incomplete.
> >
> > It seems to me we can have
> >
> > - can queue, buffer not full -> do nothing more
>
> As I said above, without a timeout timer, the queued skb
> might not be sent out even some long time passed if no
> frame comes later.
Therefore we check whether the device is still busy.
> So could you add the wait for more mechanism to your patch
> for further discussion?
I am attaching a version that is algorithmically complete
except for error handling.
> > - can queue, buffer full -> transmit
> > - cannot queue, buffer full -> transmit and then try again to queue
>
> This might not happen, the buffer full should have been observed
> in last xmit path.
Possibly, but the driver cannot know in advance how large the next package
will be.
Reagrds
Oliver
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 4cd582a..56ef743 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -135,9 +135,6 @@ struct cdc_ncm_ctx {
u16 connected;
};
-static void cdc_ncm_txpath_bh(unsigned long param);
-static void cdc_ncm_tx_timeout_start(struct cdc_ncm_ctx *ctx);
-static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *hr_timer);
static struct usb_driver cdc_ncm_driver;
static void
@@ -464,10 +461,6 @@ static int cdc_ncm_bind(struct usbnet *dev, struct
usb_interface *intf)
if (ctx == NULL)
return -ENODEV;
- hrtimer_init(&ctx->tx_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
- ctx->tx_timer.function = &cdc_ncm_tx_timer_cb;
- ctx->bh.data = (unsigned long)ctx;
- ctx->bh.func = cdc_ncm_txpath_bh;
atomic_set(&ctx->stop, 0);
spin_lock_init(&ctx->mtx);
ctx->netdev = dev->net;
@@ -650,7 +643,7 @@ static void cdc_ncm_zero_fill(u8 *ptr, u32 first, u32 end,
u32 max)
memset(ptr + first, 0, end - first);
}
-static struct sk_buff *
+static int
cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
{
struct sk_buff *skb_out;
@@ -659,12 +652,7 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct
sk_buff *skb)
u32 last_offset;
u16 n = 0, index;
u8 ready2send = 0;
-
- /* if there is a remaining skb, it gets priority */
- if (skb != NULL)
- swap(skb, ctx->tx_rem_skb);
- else
- ready2send = 1;
+ u8 error = 0;
/*
* ++
@@ -690,7 +678,7 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct
sk_buff *skb)
dev_kfree_skb_any(skb);
ctx->netdev->stats.tx_dropped++;
}
- goto exit_no_skb;
+ return -EBUSY;
}
/* make room for NTH and NDP */
@@ -719,28 +707,15 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct
sk_buff *skb)
/* compute maximum buffer siz
MBIM (was: Re: changing usbnet's API to better deal with cdc-ncm)
On Thursday 06 September 2012 10:17:46 Bjørn Mork wrote: > Not really related, but I am still worrying how MBIM is going to fit > into the usbnet model where you have a strict relation between one > netdev and one USB interface. Do you see any way usbnet could be > extended to manage a list of netdevs? > > All the netdev related functions doing > > struct usbnet *dev = netdev_priv(net); > > would still work. But all the USB related functions using dev->net to > get the netdev would fail. Maybe this will be too difficult to > implement within the usbnet framework at all? It depends on how much support we get from upper layers. You can give two IPs to an ethernet card as well. It would be best if you could come up with some preliminary code at least. Reagrds Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 8/29 v2] usb/gadget: remove global variable composite in composite.c
From: Sebastian Andrzej Siewior
This patch removes the global variable composite in composite.c.
The private data which was saved there is now passed via an additional
argument to the bind() function in struct usb_gadget_driver.
Only the "old-style" UDC drivers have to be touched here, new style are
doing it right because this change is made in udc-core.
Signed-off-by: Sebastian Andrzej Siewior
---
On Thu, Sep 06, 2012 at 09:56:02PM +0200, Michal Nazarewicz wrote:
> > + struct usb_composite_driver *cdriver = cdev->driver;
>
> I still think you should use “composite” for the variable name as this
> would get rid of a lot of the following hunks:
I thought we were passed this. But okay I made this change. This change
required me to rebase the series and modify patches
- 08
- 14
- 15
- 21
- 23
- 24
- 25
- 26
- 27
I will resend them once Felipe pings me for pick-up.
v1..v2: s/cdriver/composite
drivers/staging/ccg/composite.c |3 ++-
drivers/usb/gadget/amd5536udc.c |6 ++---
drivers/usb/gadget/composite.c| 52 +
drivers/usb/gadget/dbgp.c |3 ++-
drivers/usb/gadget/file_storage.c |3 ++-
drivers/usb/gadget/fsl_udc_core.c |6 ++---
drivers/usb/gadget/fusb300_udc.c |4 +--
drivers/usb/gadget/goku_udc.c |6 ++---
drivers/usb/gadget/inode.c|7 ++---
drivers/usb/gadget/m66592-udc.c |4 +--
drivers/usb/gadget/mv_udc_core.c |6 ++---
drivers/usb/gadget/omap_udc.c |6 ++---
drivers/usb/gadget/pch_udc.c |6 ++---
drivers/usb/gadget/pxa25x_udc.c |6 ++---
drivers/usb/gadget/pxa27x_udc.c |6 ++---
drivers/usb/gadget/s3c2410_udc.c |6 ++---
drivers/usb/gadget/udc-core.c |4 +--
include/linux/usb/composite.h |1 +
include/linux/usb/gadget.h|6 +++--
19 files changed, 78 insertions(+), 63 deletions(-)
diff --git a/drivers/staging/ccg/composite.c b/drivers/staging/ccg/composite.c
index 2a345f2..228b457 100644
--- a/drivers/staging/ccg/composite.c
+++ b/drivers/staging/ccg/composite.c
@@ -1422,7 +1422,8 @@ static u8 override_id(struct usb_composite_dev *cdev, u8
*desc)
return *desc;
}
-static int composite_bind(struct usb_gadget *gadget)
+static int composite_bind(struct usb_gadget *gadget,
+ struct usb_gadget_driver *driver)
{
struct usb_composite_dev*cdev;
int status = -ENOMEM;
diff --git a/drivers/usb/gadget/amd5536udc.c b/drivers/usb/gadget/amd5536udc.c
index 187d211..fc0ec5e 100644
--- a/drivers/usb/gadget/amd5536udc.c
+++ b/drivers/usb/gadget/amd5536udc.c
@@ -1401,7 +1401,7 @@ static int udc_wakeup(struct usb_gadget *gadget)
}
static int amd5536_start(struct usb_gadget_driver *driver,
- int (*bind)(struct usb_gadget *));
+ int (*bind)(struct usb_gadget *, struct usb_gadget_driver *));
static int amd5536_stop(struct usb_gadget_driver *driver);
/* gadget operations */
static const struct usb_gadget_ops udc_ops = {
@@ -1914,7 +1914,7 @@ static int setup_ep0(struct udc *dev)
/* Called by gadget driver to register itself */
static int amd5536_start(struct usb_gadget_driver *driver,
- int (*bind)(struct usb_gadget *))
+ int (*bind)(struct usb_gadget *, struct usb_gadget_driver *))
{
struct udc *dev = udc;
int retval;
@@ -1932,7 +1932,7 @@ static int amd5536_start(struct usb_gadget_driver *driver,
dev->driver = driver;
dev->gadget.dev.driver = &driver->driver;
- retval = bind(&dev->gadget);
+ retval = bind(&dev->gadget, driver);
/* Some gadget drivers use both ep0 directions.
* NOTE: to gadget driver, ep0 is just one endpoint...
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 2a345f2..0b6ee20 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -31,8 +31,6 @@
/* big enough to hold our biggest descriptor */
#define USB_BUFSIZ 1024
-static struct usb_composite_driver *composite;
-
/* Some systems will need runtime overrides for the product identifiers
* published in the device descriptor, either numbers or strings or both.
* String parameters are in UTF-8 (superset of ASCII's 7 bit characters).
@@ -889,6 +887,7 @@ static int lookup_string(
static int get_string(struct usb_composite_dev *cdev,
void *buf, u16 language, int id)
{
+ struct usb_composite_driver *composite = cdev->driver;
struct usb_configuration*c;
struct usb_function *f;
int len;
@@ -1359,8 +1358,8 @@ static void composite_disconnect(struct usb_gadget
*gadget)
spin_lock_irqsave(&cdev->lock, flags);
if (cdev->config)
reset_config(cdev);
- if (composite->disconnect)
- composite->disconnect(cdev);
+ if (cdev->d
Re: [PATCH 2/2] usb: chipidea: usbmisc: prepare driver to handle more than one soc
On Thu, Sep 06, 2012 at 05:18:29PM +0200, Marc Kleine-Budde wrote: > On 09/06/2012 05:15 PM, Richard Zhao wrote: > > Hi Marc, > > > > usbmisc_imx6q.c is only for imx6x. And for a certain running kernel, > > there will be always one driver instance. > > It's currently only for imx6q, but I've patches[1] in queue for mx53. > And mx35 is about to be written. It's per-SoC file. mx53 and mx35 should have their specific driver, because the register layout is SoC specific. Thanks Richard > > Marc > > [1] > http://git.pengutronix.de/?p=mkl/linux.git;a=commit;h=bdeff138ba8bb40856c9946b67df2604fdb4416b > > -- > Pengutronix e.K. | Marc Kleine-Budde | > Industrial Linux Solutions| Phone: +49-231-2826-924 | > Vertretung West/Dortmund | Fax: +49-5121-206917- | > Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: MBIM
Oliver Neukum writes: > On Thursday 06 September 2012 10:17:46 Bjørn Mork wrote: >> Not really related, but I am still worrying how MBIM is going to fit >> into the usbnet model where you have a strict relation between one >> netdev and one USB interface. Do you see any way usbnet could be >> extended to manage a list of netdevs? >> >> All the netdev related functions doing >> >> struct usbnet *dev = netdev_priv(net); >> >> would still work. But all the USB related functions using dev->net to >> get the netdev would fail. Maybe this will be too difficult to >> implement within the usbnet framework at all? > > It depends on how much support we get from upper layers. > You can give two IPs to an ethernet card as well. Yes, of course. But I don't think that fits the MBIM model, where multiple independent connections (Internet, VoIP, MMS, VPN1, VPN2, etc) to different APNs will be multiplexed over the same USB endpoints. And to make this not fly at all: Some of these may be IPv4, some IPv6 and some dual stack. You cannot support that on a single netdev. Mixing them all together on the host will not do. When the driver sees an IPv6 multicast packet (RS, ND, whatever) on the netdev, which MBIM session should it forward that packet too? If I understand MBIM correctly, we will see device configurations with _one_ CDC data interface carrying all data traffic for multiple device functions. A MBIM driver must de-multiplex this, and each IP function should be routed to its own netdev IMHO. > It would be best if you could come up with some preliminary code > at least. Yup. Finally got a device. Will start playing. Bjørn -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: cdc-ncm: struct usb_cdc_ncm_ndp_input_size not initialized
Hello, the mail address in the MAINTAINERS file is not correct, so I send the patch again. I hope it finds the way in the official kernel sources. -- Kind regards, Frank Hoffmann Thesycon GmbH On 06.09.2012 14:35, Frank Hoffmann wrote: Hello, we developing CDC/NCM firmware and found that the Linux CDC/NCM is not compliant to the USB CDC/NCM specification. The cause is a Bug in the cdc_ncm.c file. The structure usb_cdc_ncm_ndp_input_size is only allocated but not initialized. The CDC/NCM specification section "6.2.7 SetNtbInputSize" says that the dwNtbInMaxSize field must be at least 2048. But the current implementation leave it 0, because it is not initialized. --- linux-3.6-rc4.orig/drivers/net/usb/cdc_ncm.c2012-09-06 14:05:26.981402153 +0200 +++ linux-3.6-rc4/drivers/net/usb/cdc_ncm.c2012-09-06 14:11:57.137398615 +0200 @@ -224,6 +224,7 @@ err = -ENOMEM; goto size_err; } +ndp_in_sz->dwNtbInMaxSize = cpu_to_le32(ctx->rx_max); err = usb_control_msg(ctx->udev, usb_sndctrlpipe(ctx->udev, 0), -- Kind regards, Frank Hoffmann Thesycon GmbH -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Wrong SSD sector count with current F17 (right with live)
[Originally posted to fedora general, where Alan Cox suspected a usb issue and suggested this list.] F17 with current updates gets the sector count wrong (too large by 5) for my SSD (Jetflash USB), resulting in read errors (during boot, fdisk reading the disk etc.) for those sectors. While everything "seems to work" this is not OK, of course. I'd be grateful for any clue. [The device provides USB to S-ATA, is USB3, connected to a USB2 port.] Test systems F16 with updates (kernel 3.4.9-2) F17 live (kernel 3.3.4-5) F17 with updates (kernels 3.3.4-5, 3.5.0-2, 3.5.2-3, 3.5.3-1) F16 and F17 live are OK, F17 with updates is not (no matter which kernel), where "OK" means no read errors. So the relevant difference is not in the kernel but something else which has been updated since release. [Should I downgrade something besides the kernel to F17-release to check USB involvement?] lsusb = Bus 002 Device 003: ID 2174:07d0 dmesg = (after a common part about the USB to S-ATA device) F16 (booted from a different device): [ 287.229149] scsi6 : usb-storage 2-1.5:1.0 [ 288.230862] scsi 6:0:0:0: Direct-Access JetFlash TS128GSSD18C3 0004 PQ: 0 ANSI: 0 [ 288.231591] sd 6:0:0:0: Attached scsi generic sg7 type 0 [ 288.232184] sd 6:0:0:0: [sdg] 250069676 512-byte logical blocks: (128 GB/119 GiB) [ 288.234815] sd 6:0:0:0: [sdg] Write Protect is off [ 288.234820] sd 6:0:0:0: [sdg] Mode Sense: 03 00 00 00 [ 288.235676] sd 6:0:0:0: [sdg] No Caching mode page present [ 288.235680] sd 6:0:0:0: [sdg] Assuming drive cache: write through F17 (booted from that SSD): [2.661796] scsi5 : usb-storage 2-1.5:1.0 [3.665597] scsi 5:0:0:0: Direct-Access JetFlash TS128GSSD18C3 0004 PQ: 0 ANSI: 0 [3.666375] sd 5:0:0:0: Attached scsi generic sg6 type 0 [3.666917] sd 5:0:0:0: [sdf] 250069681 512-byte logical blocks: (128 GB/119 GiB) [3.667784] sd 5:0:0:0: [sdf] Write Protect is off [3.667789] sd 5:0:0:0: [sdf] Mode Sense: 03 00 00 00 [3.668657] sd 5:0:0:0: [sdf] No Caching mode page present [3.668661] sd 5:0:0:0: [sdf] Assuming drive cache: write through Note the different sector count! After enumerating the partitions, dmesg throws: [3.674173] sdf: sdf1 sdf2 sdf3 sdf4 < sdf5 > [3.678160] sd 5:0:0:0: [sdf] No Caching mode page present [3.678165] sd 5:0:0:0: [sdf] Assuming drive cache: write through [3.678168] sd 5:0:0:0: [sdf] Attached SCSI disk [3.694795] sd 5:0:0:0: [sdf] Unhandled sense code [3.694798] sd 5:0:0:0: [sdf] Result: hostbyte=invalid driverbyte=DRIVER_SENSE [3.694802] sd 5:0:0:0: [sdf] Sense Key : Medium Error [current] [3.694806] sd 5:0:0:0: [sdf] Add. Sense: Recorded entity not found [3.694811] sd 5:0:0:0: [sdf] CDB: Read(10): 28 00 0e e7 c2 b0 00 00 01 00 [3.694821] end_request: critical target error, dev sdf, sector 250069680 [3.694824] Buffer I/O error on device sdf, logical block 250069680 The last few lines get repeated. The same difference in sector size is reported by fdisk and hdparm: geometry = 15566/255/63, sectors = 250069676, start = 0 vs. geometry = 15566/255/63, sectors = 250069681, start = 0 Interestingly, "hdparm -N" agrees in both cases: max sectors = 250069680/1(250069680?), HPA setting seems invalid (buggy kernel device driver?) But ignore_hpa=0 for the libata module in all cases (Fedora default). Stomped! I can provide the full dmesg, of course. Please help :| -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH V2 2/2] USB: Set usb port's DevicerRemovable according acpi information in EHCI
On 2012/9/5 22:39, Alan Stern wrote:
On Wed, 5 Sep 2012, Lan Tianyu wrote:
This is copied from xhci_usb3_hub_descriptor().
struct usb_hub_descriptor {
__u8 bDescLength;
__u8 bDescriptorType;
__u8 bNbrPorts;
__le16 wHubCharacteristics;
__u8 bPwrOn2PwrGood;
__u8 bHubContrCurrent;
/* 2.0 and 3.0 hubs differ here */
union {
struct {
/* add 1 bit for hub status change; round to bytes */
__u8 DeviceRemovable[(USB_MAXCHILDREN + 1 + 7) / 8];
__u8 PortPwrCtrlMask[(USB_MAXCHILDREN + 1 + 7) / 8];
} __attribute__ ((packed)) hs;
struct {
__u8 bHubHdrDecLat;
__u16 wHubDelay;
__u16 DeviceRemovable;
} __attribute__ ((packed)) ss;
} u;
} __attribute__ ((packed));
The struct has been defined with __attribute__ ((packed)). So there is
no alignment problem. Or we can
hub->descriptor->u.ss.DeviceRemovable = (__force __u16)
cpu_to_le16(port_removable);
I think we also should define wHubDelay and DeviceRemovable as __le16
for ss since they are little-endian order, right?
Yes. Ask Sarah about changing it.
hi Sarah:
Do you have some comments about this?
Alan Stern
--
Best Regards
Tianyu Lan
linux kernel enabling team
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: XHCI: xhci-ring: Remove unused dma address calculation in inc_enq and inc_deq function
Hello.
On 06-09-2012 19:47, Girish Verma wrote:
Resending patch with latest kernel code:
That comment should have been put after the --- tear line.
In xhci-ring.c, function inc_enq and inc_deq calculate the dma address
of trb but never used.
Signed-off-by: Girish Verma
---
drivers/usb/host/xhci-ring.c |4
1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 643c2f3..5c5c47b 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -145,7 +145,6 @@ static void next_trb(struct xhci_hcd *xhci,
*/
static void inc_deq(struct xhci_hcd *xhci, struct xhci_ring *ring)
{
- unsigned long long addr;
ring->deq_updates++;
@@ -176,7 +175,6 @@ static void inc_deq(struct xhci_hcd *xhci, struct
xhci_ring *ring)
}
} while (last_trb(xhci, ring, ring->deq_seg, ring->dequeue));
- addr = (unsigned long long) xhci_trb_virt_to_dma(ring->deq_seg,
ring->dequeue);
}
/*
@@ -201,7 +199,6 @@ static void inc_enq(struct xhci_hcd *xhci, struct
xhci_ring *ring,
{
u32 chain;
union xhci_trb *next;
- unsigned long long addr;
chain = le32_to_cpu(ring->enqueue->generic.field[3]) & TRB_CHAIN;
/* If this is not event ring, there is one less usable TRB */
@@ -253,7 +250,6 @@ static void inc_enq(struct xhci_hcd *xhci, struct
xhci_ring *ring,
ring->enqueue = ring->enq_seg->trbs;
next = ring->enqueue;
}
- addr = (unsigned long long) xhci_trb_virt_to_dma(ring->enq_seg,
ring->enqueue);
The patch is line wrapped. Should be easy to fix by hand when applying
though...
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 2/6] usb/dwc3: fix isoc END TRANSFER Condition
Hello.
On 07-09-2012 8:33, Pratyush Anand wrote:
There were still some corner cases where isoc transfer was not able to
restart, specially when missed does not happen , and gadget does
Missed what does not happen?
not queue any new request during giveback.
Cleanup function calls giveback first, which provides a way to queue
another request to gadget. But gadget did not had any data so , it did
^^^ have
not call ep_queue. To twist it further, gadget did not queue till
cleanup for last queued TRB is called. If we ever reach this scenario,
we must call END TRANSFER.
Signed-off-by: Pratyush Anand
---
drivers/usb/dwc3/gadget.c | 22 --
1 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 0f6874c..ef4d9ef 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1115,9 +1115,13 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep,
struct dwc3_request *req)
* notion of current microframe.
*/
if (usb_endpoint_xfer_isoc(dep->endpoint.desc)) {
- dwc3_stop_active_transfer(dwc, dep->number);
+ if (list_empty(&dep->req_queued)) {
+ dwc3_stop_active_transfer(dwc, dep->number);
+ dep->flags = DWC3_EP_ENABLED;
+ }
return 0;
}
+
ret = __dwc3_gadget_kick_transfer(dep, 0, true);
if (ret && ret != -EBUSY) {
struct dwc3 *dwc = dep->dwc;
@@ -1760,11 +1764,17 @@ static int dwc3_cleanup_done_reqs(struct dwc3 *dwc,
struct dwc3_ep *dep,
break;
} while (1);
- if (list_empty(&dep->req_queued) &&
- (dep->flags & DWC3_EP_MISSED_ISOC)) {
- dwc3_stop_active_transfer(dwc, dep->number);
- dep->flags &= ~DWC3_EP_MISSED_ISOC;
- return 1;
+ if (usb_endpoint_xfer_isoc(dep->endpoint.desc)) {
+ if (list_empty(&dep->req_queued)) {
+ if (list_empty(&dep->request_list) ||
Looks like this *if* is indented too much... BTW, all three *if*
statements could be collapsed into one...
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 2/6] usb/dwc3: fix isoc END TRANSFER Condition
On 9/7/2012 4:50 PM, Sergei Shtylyov wrote: >There were still some corner cases where isoc transfer was not able to >restart, specially when missed does not happen , and gadget does Missed what does not happen? Its missed isoc. I will try to amend it more clearly in final submission. Can somebody test these patches on his platform too? -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: changing usbnet's API to better deal with cdc-ncm
> -Original Message- > From: Oliver Neukum [mailto:oneu...@suse.de] > Sent: Friday, September 07, 2012 9:36 AM > > > Well, that is the mistake. Using a timer is a bad idea. > > > > Why is a bad idea? Without a timer, how can usbnet or low level > > driver aggregate the later coming transmit frames to form a biger > > aggregated frame? > > By registering demand in an abstract sense. The timer makes sense only > when no other buffers are being transfered. > > The timer means that we transmit if no other more packages are coming > down from the upper layers. Now, either the device is still busy or it > is not. > While it is busy, we might just as well wait, because the upper layer > may change its mind. > If the device is not busy we worsen latency by waiting for the timer. ... > > As I said above, without a timeout timer, the queued skb might not be > > sent out even some long time passed if no frame comes later. > > Therefore we check whether the device is still busy. > The purpose of NCM is to reduce the amount of interrupts and the number of DMA jobs that must be handled by the device. It is most efficient to send and especially to receive NTBs of agreed size, padded if needed. Misunderstanding of the reason why NCM protocol is doing aggregation of several ETH frames into a single NTB leads to crippled implementation. Look at NCM gadget, which is not really NCM device at all, it's kinda ECM device with NCM framing. The experience from early implementations and prototyping of NCM was that using NCM with 4-8kB NTB increased max throughput in loop-mode by a factor of 5-6 even 8-10 times compared to using ECM. One real-world example was modem for 21+6Mbit/s what used 100% CPU with ECM responsible for approx. 40% of the MIPS used. Using NCM instead CPU was only at approx. 65% utilization. Which allowed multiple other functions to be added and significantly increased the usability and value of the modem. NCM efficiency is gained either by accessing TX queue of upper layer or by using timer. These findings were later confirmed by multiple major industry players (names withheld), and demonstrated during multiple NCM interoperability workshops using multiple device HW platforms, multiple operating systems and multiple host HW ranging from Beagleboard to latest quad-core x86. Until we do something with network device framework in order to get access to upper layer Tx queue we need to utilize timer. Regards, Alexey -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/29 v2] usb/gadget: remove global variable composite in composite.c
On Fri, Sep 07 2012, Sebastian Andrzej Siewior wrote: > From: Sebastian Andrzej Siewior > > This patch removes the global variable composite in composite.c. > The private data which was saved there is now passed via an additional > argument to the bind() function in struct usb_gadget_driver. > > Only the "old-style" UDC drivers have to be touched here, new style are > doing it right because this change is made in udc-core. > > Signed-off-by: Sebastian Andrzej Siewior Like previously: Acked-by: Michal Nazarewicz -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +--ooO--(_)--Ooo-- pgpyJhymBKtVv.pgp Description: PGP signature
[PATCH] usb/core: use bin2bcd() for bcdDevice in RH
The kernel's version number is used as decimal in the bcdDevice field of
the RH descriptor. For kernel version v3.12 we would see 3.0c in lsusb.
I am not sure how important it is to stick with bcd values since this is
this way since we started git history and nobody complained (however back
then we reported only 2.6).
Signed-off-by: Sebastian Andrzej Siewior
---
drivers/usb/core/hcd.c |6 +++---
include/linux/bcd.h| 17 +++--
lib/bcd.c |8
3 files changed, 22 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index bc84106..6e4fd28 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -22,6 +22,7 @@
* Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*/
+#include
#include
#include
#include
@@ -123,9 +124,8 @@ static inline int is_root_hub(struct usb_device *udev)
*/
/*-*/
-
-#define KERNEL_REL ((LINUX_VERSION_CODE >> 16) & 0x0ff)
-#define KERNEL_VER ((LINUX_VERSION_CODE >> 8) & 0x0ff)
+#define KERNEL_REL bin2bcd(((LINUX_VERSION_CODE >> 16) & 0x0ff))
+#define KERNEL_VER bin2bcd(((LINUX_VERSION_CODE >> 8) & 0x0ff))
/* usb 3.0 root hub device descriptor */
static const u8 usb3_rh_dev_descriptor[18] = {
diff --git a/include/linux/bcd.h b/include/linux/bcd.h
index 22ea563..18fff11 100644
--- a/include/linux/bcd.h
+++ b/include/linux/bcd.h
@@ -3,7 +3,20 @@
#include
-unsigned bcd2bin(unsigned char val) __attribute_const__;
-unsigned char bin2bcd(unsigned val) __attribute_const__;
+#define bcd2bin(x) \
+ (__builtin_constant_p((u8 )(x)) ? \
+ const_bcd2bin(x) : \
+ _bcd2bin(x))
+
+#define bin2bcd(x) \
+ (__builtin_constant_p((u8 )(x)) ? \
+ const_bin2bcd(x) : \
+ _bin2bcd(x))
+
+#define const_bcd2bin(x) (((x) & 0x0f) + ((x) >> 4) * 10)
+#define const_bin2bcd(x) x) / 10) << 4) + (x) % 10)
+
+unsigned _bcd2bin(unsigned char val) __attribute_const__;
+unsigned char _bin2bcd(unsigned val) __attribute_const__;
#endif /* _BCD_H */
diff --git a/lib/bcd.c b/lib/bcd.c
index 55efaf7..40d304e 100644
--- a/lib/bcd.c
+++ b/lib/bcd.c
@@ -1,14 +1,14 @@
#include
#include
-unsigned bcd2bin(unsigned char val)
+unsigned _bcd2bin(unsigned char val)
{
return (val & 0x0f) + (val >> 4) * 10;
}
-EXPORT_SYMBOL(bcd2bin);
+EXPORT_SYMBOL(_bcd2bin);
-unsigned char bin2bcd(unsigned val)
+unsigned char _bin2bcd(unsigned val)
{
return ((val / 10) << 4) + val % 10;
}
-EXPORT_SYMBOL(bin2bcd);
+EXPORT_SYMBOL(_bin2bcd);
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: changing usbnet's API to better deal with cdc-ncm
On Thu, Sep 6, 2012 at 4:13 PM, Alexey ORISHKO wrote: >> -Original Message- >> From: Oliver Neukum [mailto:oneu...@suse.de] > > >> looking at cdc-ncm it seeems to me that cdc-ncm is forced to play very >> dirty games because usbnet doesn't have a notion about aggregating >> packets for a single transfer. > > Several issues need to be improved: > Tx path: > 1. IP packets must be accumulated in one NTB. Currently it's done via data > copy. > Preferred way would be a possibility to have a list of skb-s in resulting > frame sent down. Looks the copy is inevitable because the buffer length of each skb in the list is not possible to be max endpoint packet aligned, so one short packet may terminate the current tx urb transfer. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Wrong SSD sector count with current F17 (right with live)
On Fri, Sep 7, 2012 at 11:30 AM, Michael J Gruber wrote: > [Originally posted to fedora general, where Alan Cox suspected a usb > issue and suggested this list.] > > F17 with current updates gets the sector count wrong (too large by 5) > for my SSD (Jetflash USB), resulting in read errors (during boot, fdisk > reading the disk etc.) for those sectors. While everything "seems to > work" this is not OK, of course. I'd be grateful for any clue. > > [The device provides USB to S-ATA, is USB3, connected to a USB2 port.] > > Test systems > > F16 with updates (kernel 3.4.9-2) > F17 live (kernel 3.3.4-5) > F17 with updates (kernels 3.3.4-5, 3.5.0-2, 3.5.2-3, 3.5.3-1) > > F16 and F17 live are OK, F17 with updates is not (no matter which > kernel), where "OK" means no read errors. So the relevant difference is > not in the kernel but something else which has been updated since release. Some additional info on the confusing part about 3.3.4-5 being OK with live (everything release), not with an otherwise updated install: The real issue here seems to be the difference between a (soft) reboot and a hard shutdown, then boot. More specifically: Boot with 3.3.4-5: OK Reboot into 3.5.3-1: not OK Reboot into 3.3.4-5: not OK Shutdown, then boot into 3.3.4-5: OK So, something in the newer kernels seems to put my SSD into a state which is cleared by a power off only. Smells like HPA?? (again, OK = no read errors, no conflicting sector counts) I've also tried the newest Fedora kernel 3.6.0-0.rc2.git2.1.fc18, same problems. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: changing usbnet's API to better deal with cdc-ncm
On Friday 07 September 2012 14:01:23 Alexey ORISHKO wrote: > The experience from early implementations and prototyping of NCM was that > using NCM with 4-8kB NTB increased max throughput in loop-mode by a factor > of 5-6 even 8-10 times compared to using ECM. One real-world example was > modem for 21+6Mbit/s what used 100% CPU with ECM responsible for approx. 40% > of the MIPS used. Using NCM instead CPU was only at approx. 65% utilization. > Which allowed multiple other functions to be added and significantly increased > the usability and value of the modem. > > NCM efficiency is gained either by accessing TX queue of upper layer or > by using timer. > > These findings were later confirmed by multiple major industry players > (names withheld), and demonstrated during multiple NCM interoperability > workshops using multiple device HW platforms, multiple operating systems > and multiple host HW ranging from Beagleboard to latest quad-core x86. > > Until we do something with network device framework in order to get access > to upper layer Tx queue we need to utilize timer. Could you explain your reasoning? From what you say we must reduce the number of transfers, thus use them efficiently, but why by means of a timer? Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: at91_udc: fix dt support
Don't fail the initialization check for the platform_data
if there is avaiable an associated device tree node.
Signed-off-by: Fabio Porcedda
---
drivers/usb/gadget/at91_udc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/at91_udc.c b/drivers/usb/gadget/at91_udc.c
index c9e66df..4147219 100644
--- a/drivers/usb/gadget/at91_udc.c
+++ b/drivers/usb/gadget/at91_udc.c
@@ -1703,7 +1703,7 @@ static int __devinit at91udc_probe(struct platform_device
*pdev)
int retval;
struct resource *res;
- if (!dev->platform_data) {
+ if (!dev->platform_data && !pdev->dev.of_node) {
/* small (so we copy it) but critical! */
DBG("missing platform_data\n");
return -ENODEV;
--
1.7.11.3
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: at91_udc: fix dt support
On 09/07/2012 03:27 PM, Fabio Porcedda :
> Don't fail the initialization check for the platform_data
> if there is avaiable an associated device tree node.
>
> Signed-off-by: Fabio Porcedda
Acked-by: Nicolas Ferre
Thanks, bye,
> ---
> drivers/usb/gadget/at91_udc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/at91_udc.c b/drivers/usb/gadget/at91_udc.c
> index c9e66df..4147219 100644
> --- a/drivers/usb/gadget/at91_udc.c
> +++ b/drivers/usb/gadget/at91_udc.c
> @@ -1703,7 +1703,7 @@ static int __devinit at91udc_probe(struct
> platform_device *pdev)
> int retval;
> struct resource *res;
>
> - if (!dev->platform_data) {
> + if (!dev->platform_data && !pdev->dev.of_node) {
> /* small (so we copy it) but critical! */
> DBG("missing platform_data\n");
> return -ENODEV;
>
--
Nicolas Ferre
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: changing usbnet's API to better deal with cdc-ncm
> -Original Message- > From: Ming Lei [mailto:tom.leim...@gmail.com] > Sent: Friday, September 07, 2012 2:58 PM > > Several issues need to be improved: > > Tx path: > > 1. IP packets must be accumulated in one NTB. Currently it's done via > data copy. > > Preferred way would be a possibility to have a list of skb-s in > resulting frame sent down. > > Looks the copy is inevitable because the buffer length of each skb in > the list is not possible to be max endpoint packet aligned, so one > short packet may terminate the current tx urb transfer. Zero or short packet shall not be added if resulting NTB is max negotiated size. It's yet another problem with usbnet, where we had to add some workarounds to avoid redundant short/zero packets. Regards, Alexey -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: cdc-ncm: struct usb_cdc_ncm_ndp_input_size not initialized
On Fri, Sep 07, 2012 at 10:55:27AM +0200, Frank Hoffmann wrote: > Hello, > > the mail address in the MAINTAINERS file is not correct, so I send > the patch again. > I hope it finds the way in the official kernel sources. Please send the patch in a format that it can be applied in (hint, you need a signed-off-by: line as described in the Documentation/SubmittingPatches file). thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: PLAT_ORION fulfils USB_ARCH_HAS_EHCI
On Mon, Sep 03, 2012 at 08:21:15PM +0200, Andrew Lunn wrote: > The various Orion SoCs, i.e. orion5x, kirkwood, dove, mv78xx0 > and 370/XP have EHCI. Make sure USB_ARCH_HAS_EHCI reflects this. > > Reported-by: Sebastian Hesselbarth > Signed-off-by: Andrew Lunn oops, almost lost this thread. We can add _LEGACY later, once we are making more productive use of it. So, Acked-by: Jason Cooper thx, Jason. > --- > drivers/usb/Kconfig |1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig > index a7773a3..723efcc 100644 > --- a/drivers/usb/Kconfig > +++ b/drivers/usb/Kconfig > @@ -48,6 +48,7 @@ config USB_ARCH_HAS_EHCI > default y if SPARC_LEON > default y if ARCH_MMP > default y if MACH_LOONGSON1 > + default y if PLAT_ORION > default PCI > > # some non-PCI HCDs implement xHCI > -- > 1.7.10.4 > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: changing usbnet's API to better deal with cdc-ncm
> -Original Message- > From: Oliver Neukum [mailto:oneu...@suse.de] > Sent: Friday, September 07, 2012 3:09 PM > > > > Until we do something with network device framework in order to get > > access to upper layer Tx queue we need to utilize timer. > > Could you explain your reasoning? From what you say we must reduce the > number of transfers, thus use them efficiently, but why by means of a > timer? > What was omitted in the previous mail is the fact that in addition to reducing amount of transfers we need to fill in NTB to the maximum size. It's quite important especially for embedded systems. As an example, on device (modem) side, DMA job for max NTB is initially set. - If device receives full NTB, only one interrupt (per NTB) is generated at the completion of DMA job and NCM function can immediately parse incoming NTB frame. - In case of "skinny" (not full) NTB, device gets INT for the short packet, then original DMA job for full NTB shall be cancelled, then DMA job to get short packet shall be set and after that we get INT for its completion. Only at this point all data is received and NTB parsing can be done by NCM function. All above takes quite more time than first case. There is a temptation to send full NTBs even with a single IP packet, But it will lead to wasted USB bandwidth and reduced ability to send real data for other functions in the device or other devices on the same root hub. Most important it will also harm IN direction. The challenge is to ensure that an acceptable timeout value is used. Too long and latency is introduced. Too short and too many padded NTBs will go out and that reduces the available throughput. The ideal timer value depends on the throughput available in the network. Something that is not really explicitly known to the NCM layer nor to the layer above. Alternate methods exist to achieve the same result without using a timer. But an optimal implementation requires that the amount of IP packets "in progress" or queued up is known to NCM so NCM can decide to send short or padded NTB or aggregate and send one or more full NTBs plus short or padded NTB. An implementation where NCM only knows if there is more data available or not can be shown to have side-effects that are not easily circumvented. And likewise shown to limit throughput compared to a timer-based solution. Regards, Alexey -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL] USB fixes for v3.6-rc4
On Thu, Sep 06, 2012 at 08:05:01PM +0300, Felipe Balbi wrote: > Hi Greg, > > Here's my previous pull request now rebased on top of v3.6-rc4. Sorry > for the inconvenience my "never rebase" methodology :-) > > Note that I'm rebased on top of v3.6-rc4, so you should probably merge > that tag on your tree. > > After merging v3.6-rc4 on your tree, you can merge my tag and you should > get the diffstat below. Let me know if there any other issues. Ick, well, I didn't have -rc4 in my tree, so the diffstat didn't match, but I think all is well. I can't send out the "I applied this patch" emails, as it would have done so for all of the patches between -rc3 and -rc4, but I've now pulled and pushed this out. Please verify I didn't mess anything up. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] How to handle delays in isochronous transfers?
On Fri, 7 Sep 2012, Peter Chen wrote: > > > > Here's the problem we face: The class driver submits packets 1, 2, 3, > > and 4. They get sent properly, but the completion interrupt is > > delayed. As a result, the class driver's completion handler doesn't > > get called until too late; the frames for packets 5 - 44 have already > > expired. The data that should have been sent during those frames is > > lost. > > > > But the class driver doesn't know this, so when the completion handler > > does get called, it submits packets 5, 6, 7, and 8. They end up > > getting sent in frames 45, 46, 47, and 48. This means the data is now > > out of synchronization by 40 frames. > > > > That's the problem I want to solve. > > > Can I understand this problem like below: > > It is OUT ISO transfer, at each completion handler, it prepares > 4 iTDs for 4 packets. > > When the above problem occurs, the class driver will submit packet > from 5 - 48, and the controller driver will prepare 44 iTDs. (or the class > driver still submits packet 5-8?) The class driver submits packets 5-8, because each time the completion handler runs it prepares 4 iTDs. > If the feedback is supported, the device will know host sends data slowly, > it will give speed up feedback information after it receives packet 5 or other > packets depends on its interval at descriptor. At this information, it can > tell > the host to increase the packet size, then the transaction length and > transaction > numbers at iTD can be increased(Assume it was not maximum). Clemens pointed out that this won't work if the delay is too long. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Wrong SSD sector count with current F17 (right with live)
On Fri, 7 Sep 2012, Michael J Gruber wrote: > On Fri, Sep 7, 2012 at 11:30 AM, Michael J Gruber > wrote: > > [Originally posted to fedora general, where Alan Cox suspected a usb > > issue and suggested this list.] > > > > F17 with current updates gets the sector count wrong (too large by 5) > > for my SSD (Jetflash USB), resulting in read errors (during boot, fdisk > > reading the disk etc.) for those sectors. While everything "seems to > > work" this is not OK, of course. I'd be grateful for any clue. > > > > [The device provides USB to S-ATA, is USB3, connected to a USB2 port.] > > > > Test systems > > > > F16 with updates (kernel 3.4.9-2) > > F17 live (kernel 3.3.4-5) > > F17 with updates (kernels 3.3.4-5, 3.5.0-2, 3.5.2-3, 3.5.3-1) > > > > F16 and F17 live are OK, F17 with updates is not (no matter which > > kernel), where "OK" means no read errors. So the relevant difference is > > not in the kernel but something else which has been updated since release. > > Some additional info on the confusing part about 3.3.4-5 being OK with > live (everything release), not with an otherwise updated install: > The real issue here seems to be the difference between a (soft) reboot > and a hard shutdown, then boot. More specifically: > > Boot with 3.3.4-5: OK > Reboot into 3.5.3-1: not OK > Reboot into 3.3.4-5: not OK > Shutdown, then boot into 3.3.4-5: OK > > So, something in the newer kernels seems to put my SSD into a state > which is cleared by a power off only. Smells like HPA?? This proves the problem is not a USB issue, because USB sends only the data it is told to send by the SCSI layer. Try posting your problem on the linux-scsi mailing list. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] EHCI: Update qTD next pointer in QH overlay region during unlink
On Fri, 7 Sep 2012, Pavankumar Kondeti wrote:
> There is a possibility of QH overlay region having reference to a stale
> qTD pointer during unlink.
>
> Consider an endpoint having two pending qTD before unlink process begins.
> The endpoint's QH queue looks like this.
>
> qTD1 --> qTD2 --> Dummy
>
> To unlink qTD2, QH is removed from asynchronous list and Asynchronous
> Advance Doorbell is programmed. The qTD1's next qTD pointer is set to
> qTD2'2 next qTD pointer and qTD2 is retired upon controller's doorbell
> interrupt. If QH's current qTD pointer points to qTD1, transfer overlay
> region still have reference to qTD2. But qtD2 is just unlinked and freed.
> This may cause EHCI system error. Fix this by updating qTD next pointer
> in QH overlay region with the qTD next pointer of the current qTD.
>
> Signed-off-by: Pavankumar Kondeti
> ---
> drivers/usb/host/ehci-q.c | 12 ++--
> 1 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c
> index 9bc39ca..4b66374 100644
> --- a/drivers/usb/host/ehci-q.c
> +++ b/drivers/usb/host/ehci-q.c
> @@ -128,9 +128,17 @@ qh_refresh (struct ehci_hcd *ehci, struct ehci_qh *qh)
> else {
> qtd = list_entry (qh->qtd_list.next,
> struct ehci_qtd, qtd_list);
> - /* first qtd may already be partially processed */
> - if (cpu_to_hc32(ehci, qtd->qtd_dma) == qh->hw->hw_current)
> + /*
> + * first qtd may already be partially processed.
> + * If we come here during unlink, the QH overlay region
> + * might have reference to the just unlinked qtd. The
> + * qtd is updated in qh_completions(). Update the QH
> + * overlay here.
> + */
> + if (cpu_to_hc32(ehci, qtd->qtd_dma) == qh->hw->hw_current) {
> + qh->hw->hw_qtd_next = qtd->hw_next;
> qtd = NULL;
> + }
> }
>
> if (qtd)
Acked-by: Alan Stern
Have you been able to determine that this eliminates your host system
errors?
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] EHCI: Update qTD next pointer in QH overlay region during unlink
On 9/7/2012 9:22 PM, Alan Stern wrote:
> On Fri, 7 Sep 2012, Pavankumar Kondeti wrote:
>
>> There is a possibility of QH overlay region having reference to a stale
>> qTD pointer during unlink.
>>
>> Consider an endpoint having two pending qTD before unlink process begins.
>> The endpoint's QH queue looks like this.
>>
>> qTD1 --> qTD2 --> Dummy
>>
>> To unlink qTD2, QH is removed from asynchronous list and Asynchronous
>> Advance Doorbell is programmed. The qTD1's next qTD pointer is set to
>> qTD2'2 next qTD pointer and qTD2 is retired upon controller's doorbell
>> interrupt. If QH's current qTD pointer points to qTD1, transfer overlay
>> region still have reference to qTD2. But qtD2 is just unlinked and freed.
>> This may cause EHCI system error. Fix this by updating qTD next pointer
>> in QH overlay region with the qTD next pointer of the current qTD.
>>
>> Signed-off-by: Pavankumar Kondeti
>> ---
>> drivers/usb/host/ehci-q.c | 12 ++--
>> 1 files changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c
>> index 9bc39ca..4b66374 100644
>> --- a/drivers/usb/host/ehci-q.c
>> +++ b/drivers/usb/host/ehci-q.c
>> @@ -128,9 +128,17 @@ qh_refresh (struct ehci_hcd *ehci, struct ehci_qh *qh)
>> else {
>> qtd = list_entry (qh->qtd_list.next,
>> struct ehci_qtd, qtd_list);
>> -/* first qtd may already be partially processed */
>> -if (cpu_to_hc32(ehci, qtd->qtd_dma) == qh->hw->hw_current)
>> +/*
>> + * first qtd may already be partially processed.
>> + * If we come here during unlink, the QH overlay region
>> + * might have reference to the just unlinked qtd. The
>> + * qtd is updated in qh_completions(). Update the QH
>> + * overlay here.
>> + */
>> +if (cpu_to_hc32(ehci, qtd->qtd_dma) == qh->hw->hw_current) {
>> +qh->hw->hw_qtd_next = qtd->hw_next;
>> qtd = NULL;
>> +}
>> }
>>
>> if (qtd)
>
> Acked-by: Alan Stern
>
Thanks Alan for reviewing the patch.
> Have you been able to determine that this eliminates your host system
> errors?
Yes. We are able to determine that this patch is fixing the EHCI system
error.
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: When enabling usb autosuspend, new usb devices aren't recognized
On Thu, 6 Sep 2012, Florian Merz wrote: > I've attached the dmesg (USB_DEBUG enabled) output from boot to > enabling a usb port manually (380) until plugging in a device (404). I think the problem shows up in these lines: [3.520138] xhci_hcd :00:14.0: power state changed by ACPI to D2 [5.542872] ehci_hcd :00:1a.0: power state changed by ACPI to D2 [5.642683] ehci_hcd :00:1d.0: power state changed by ACPI to D2 It may be that your host controllers do not support PCI wakeup from D2. What does the "lspci -vv" output show for the host controllers (you have to run it as root)? On Fri, 7 Sep 2012, Florian Merz wrote: > This morning I noticed that first new USB device I plug in after my > system resumed from suspend gets recognized, nut only the first. > I reproduced this after a fresh boot. The dmesg output is attached. > The USB drive was pluged in three times: > 1. After booting (around 30) > 2. After resuming from suspend (around 80) which worked > 3. After removing the stick i waited a few seconds and inserted it again. I don't know why the first device plugged in after resume was recognized. For some reason the xHCI controller did wake up, even though it was in D2. However I can tell you that if you plugged in a second device into a USB-3 port before unplugging the USB drive, the second device would have been recognized. But once all the USB-3 ports were unplugged, the controller went back into D2 and then would not wake up. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] EHCI: Update qTD next pointer in QH overlay region during unlink
On Fri, 7 Sep 2012, Alan Stern wrote:
> On Fri, 7 Sep 2012, Pavankumar Kondeti wrote:
>
> > There is a possibility of QH overlay region having reference to a stale
> > qTD pointer during unlink.
> >
> > Consider an endpoint having two pending qTD before unlink process begins.
> > The endpoint's QH queue looks like this.
> >
> > qTD1 --> qTD2 --> Dummy
> >
> > To unlink qTD2, QH is removed from asynchronous list and Asynchronous
> > Advance Doorbell is programmed. The qTD1's next qTD pointer is set to
> > qTD2'2 next qTD pointer and qTD2 is retired upon controller's doorbell
> > interrupt. If QH's current qTD pointer points to qTD1, transfer overlay
> > region still have reference to qTD2. But qtD2 is just unlinked and freed.
> > This may cause EHCI system error. Fix this by updating qTD next pointer
> > in QH overlay region with the qTD next pointer of the current qTD.
> >
> > Signed-off-by: Pavankumar Kondeti
> > ---
> > drivers/usb/host/ehci-q.c | 12 ++--
> > 1 files changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c
> > index 9bc39ca..4b66374 100644
> > --- a/drivers/usb/host/ehci-q.c
> > +++ b/drivers/usb/host/ehci-q.c
> > @@ -128,9 +128,17 @@ qh_refresh (struct ehci_hcd *ehci, struct ehci_qh *qh)
> > else {
> > qtd = list_entry (qh->qtd_list.next,
> > struct ehci_qtd, qtd_list);
> > - /* first qtd may already be partially processed */
> > - if (cpu_to_hc32(ehci, qtd->qtd_dma) == qh->hw->hw_current)
> > + /*
> > +* first qtd may already be partially processed.
> > +* If we come here during unlink, the QH overlay region
> > +* might have reference to the just unlinked qtd. The
> > +* qtd is updated in qh_completions(). Update the QH
> > +* overlay here.
> > +*/
> > + if (cpu_to_hc32(ehci, qtd->qtd_dma) == qh->hw->hw_current) {
> > + qh->hw->hw_qtd_next = qtd->hw_next;
> > qtd = NULL;
> > + }
> > }
> >
> > if (qtd)
>
> Acked-by: Alan Stern
I forgot to mention: This patch should be included in the next 3.6-rc
release and marked for -stable.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: When enabling usb autosuspend, new usb devices aren't recognized
> I think the problem shows up in these lines: > > [3.520138] xhci_hcd :00:14.0: power state changed by ACPI to D2 > [5.542872] ehci_hcd :00:1a.0: power state changed by ACPI to D2 > [5.642683] ehci_hcd :00:1d.0: power state changed by ACPI to D2 > > It may be that your host controllers do not support PCI wakeup from D2. > What does the "lspci -vv" output show for the host controllers (you > have to run it as root)? > I've attached the output of "lspci -vv" > > I don't know why the first device plugged in after resume was > recognized. For some reason the xHCI controller did wake up, even > though it was in D2. > Yes, the first devie was recognized. Was there a change from 3.5 to 3.6 concerning this? With kernel 3.4 and 3.5 there are no problems. Florian Merz 00:00.0 Host bridge: Intel Corporation 3rd Gen Core processor DRAM Controller (rev 09) Subsystem: Lenovo Device 21fa Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- SERR- 00:02.0 VGA compatible controller: Intel Corporation 3rd Gen Core processor Graphics Controller (rev 09) (prog-if 00 [VGA controller]) Subsystem: Lenovo Device 21fa Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+ Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- SERR- [disabled] Capabilities: [90] MSI: Enable+ Count=1/1 Maskable- 64bit- Address: fee00018 Data: Capabilities: [d0] Power Management version 2 Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-) Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME- Capabilities: [a4] PCI Advanced Features AFCap: TP+ FLR+ AFCtrl: FLR- AFStatus: TP- Kernel driver in use: i915 00:14.0 USB controller: Intel Corporation 7 Series/C210 Series Chipset Family USB xHCI Host Controller (rev 04) (prog-if 30 [XHCI]) Subsystem: Lenovo Device 21fa Control: I/O- Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+ Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- SERR- TAbort- SERR- TAbort- SERR- TAbort- SERR- TAbort- SERR- TAbort- SERR- TAbort- Reset- FastB2B- PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn- Capabilities: [40] Express (v2) Root Port (Slot+), MSI 00 DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s <64ns, L1 <1us ExtTag- RBE+ FLReset- DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported- RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop- MaxPayload 128 bytes, MaxReadReq 128 bytes DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr+ TransPend- LnkCap: Port #1, Speed 5GT/s, Width x1, ASPM L0s L1, Latency L0 <512ns, L1 <16us ClockPM- Surprise- LLActRep+ BwNot- LnkCtl: ASPM L1 Enabled; RCB 64 bytes Disabled- Retrain- CommClk+ ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt- LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive+ BWMgmt+ ABWMgmt- SltCap: AttnBtn- PwrCtrl- MRL- AttnInd- PwrInd- HotPlug+ Surprise+ Slot #0, PowerLimit 10.000W; Interlock- NoCompl+ SltCtl: Enable: AttnBtn- PwrFlt- MRL- PresDet+ CmdCplt- HPIrq- LinkChg- Control: AttnInd Unknown, PwrInd Unknown, Power- Interlock- SltSta: Status: AttnBtn- PowerFlt- MRL- CmdCplt- PresDet+ Interlock- Changed: MRL- PresDet- LinkState- RootCtl: ErrCorrectable- ErrNon-Fatal- ErrFatal- PMEIntEna- CRSVisible- RootCap: CRSVisible- RootSta: PME ReqID , PMEStatus- PMEPending- DevCap2: Completion Timeout: Range BC, TimeoutDis+, LTR-, OBFF Not Supported ARIFwd- DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR-, OBFF Disabled ARIFwd- LnkCtl2: Target Link Speed: 5GT/s, EnterCompliance- SpeedDis- Transmit Margin: Normal Operating Range, EnterModifiedCompliance- ComplianceSOS- Compliance De-emphasis: -6dB LnkSta2: Current De-emphasis Level: -3.5dB, EqualizationComplete-, EqualizationPhase1- EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest- Capabilities: [80] MSI: Enable- Count=1/1 Maskable- 64bit- Address: Data: Capabilities: [90] Subsystem: Lenovo Device 21fa Capabilities: [a0] Power Management version 2 Flags: PMEClk- DSI
Re: When enabling usb autosuspend, new usb devices aren't recognized
Alan Stern writes: > On Thu, 6 Sep 2012, Florian Merz wrote: > >> I've attached the dmesg (USB_DEBUG enabled) output from boot to >> enabling a usb port manually (380) until plugging in a device (404). > > I think the problem shows up in these lines: > > [3.520138] xhci_hcd :00:14.0: power state changed by ACPI to D2 > [5.542872] ehci_hcd :00:1a.0: power state changed by ACPI to D2 > [5.642683] ehci_hcd :00:1d.0: power state changed by ACPI to D2 > > It may be that your host controllers do not support PCI wakeup from D2. > What does the "lspci -vv" output show for the host controllers (you > have to run it as root)? This sounds terribly similar to the issues I had with linux-next a while ago. And those issues are still present in mainline. Florian could try the fixes available in http://git.kernel.org/?p=linux/kernel/git/helgaas/pci.git;a=shortlog;h=refs/heads/for-linus or maybe just wait for -rc5. I see that Bjorn Helgaas just sent a pull request for that branch. Bjørn -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: When enabling usb autosuspend, new usb devices aren't recognized
Hi Bjørn, If I find the time, I'll try the fixes this evening. Thanks for the hint. Florian Merz > > This sounds terribly similar to the issues I had with linux-next a while > ago. And those issues are still present in mainline. Florian could try > the fixes available in > > > http://git.kernel.org/?p=linux/kernel/git/helgaas/pci.git;a=shortlog;h=refs/heads/for-linus > > or maybe just wait for -rc5. I see that Bjorn Helgaas just sent a pull > request for that branch. > > > Bjørn -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: MBIM
On Fri, 2012-09-07 at 10:53 +0200, Bjørn Mork wrote: > Oliver Neukum writes: > > On Thursday 06 September 2012 10:17:46 Bjørn Mork wrote: > >> Not really related, but I am still worrying how MBIM is going to fit > >> into the usbnet model where you have a strict relation between one > >> netdev and one USB interface. Do you see any way usbnet could be > >> extended to manage a list of netdevs? > >> > >> All the netdev related functions doing > >> > >> struct usbnet *dev = netdev_priv(net); > >> > >> would still work. But all the USB related functions using dev->net to > >> get the netdev would fail. Maybe this will be too difficult to > >> implement within the usbnet framework at all? > > > > It depends on how much support we get from upper layers. > > You can give two IPs to an ethernet card as well. > > Yes, of course. But I don't think that fits the MBIM model, where > multiple independent connections (Internet, VoIP, MMS, VPN1, VPN2, etc) > to different APNs will be multiplexed over the same USB endpoints. And Not just multiple APNs each with different IP addressing... but QoS too. Each WWAN bearer you create may have different QoS settings. That bearer may also have *both* IPv4 and IPv6 addresses. We also may want to put the interface that bearer is tied to into a different network namespace or container or something like that, so that the process that requested the specific QoS doesn't have to share it with other apps, possibly malicious ones. Can't do that if they are all on the same kernel network interface, AFAIK. Dan > to make this not fly at all: Some of these may be IPv4, some IPv6 and > some dual stack. You cannot support that on a single netdev. Mixing > them all together on the host will not do. When the driver sees an IPv6 > multicast packet (RS, ND, whatever) on the netdev, which MBIM session > should it forward that packet too? > > If I understand MBIM correctly, we will see device configurations with > _one_ CDC data interface carrying all data traffic for multiple device > functions. A MBIM driver must de-multiplex this, and each IP function > should be routed to its own netdev IMHO. > > > It would be best if you could come up with some preliminary code > > at least. > > Yup. Finally got a device. Will start playing. > > > Bjørn > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] - USB3 bluetooth device not working properly?
On Fri, Sep 07, 2012 at 01:22:43AM +0200, Miroslav Sabljic wrote: > On 09/06/2012 08:12 PM, Sarah Sharp wrote: > >So, try compiling a kernel from the for-usb-linus-plus-set-addr branch > >in the same git repo, and see if that helps. If it doesn't, please turn > >on CONFIG_USB_XHCI_HCD_DEBUGGING and send me the dmesg. > > > >Vikas, can you do the same thing and see if this helps with your Set > >Address issues as well? > > Hi Sarah! > > Big improvment with the latest patches from > for-usb-linus-plus-set-addr branch. My bluetooth device now works as > expected, I can suspend and device isn't disconnected after a > resume. Although those timeout and not accepting address errors are > still present in logs but as far as I could test device now works. > I'm attaching dmesg output with debugging enabled. Great! I will queue these for 3.6. I may need to investigate some interesting behavior that your host controller displays. The xHCI driver handles these spurious events fine (by ignoring them), but I'd like to sort out the root cause of warnings like this: > [ 74.302671] xhci_hcd :00:14.0: Port Status Change Event for port 3 > [ 74.302687] xhci_hcd :00:14.0: ERROR Transfer event for disabled > endpoint or incorrect stream ring > [ 74.302690] xhci_hcd :00:14.0: @37007540 > 0c00 03018000 I may need you to run some additional tests later. Thanks for testing! Sarah Sharp -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net-next 0/2] qmi_wwan changes intended for 3.7
Just a couple of minor improvements for 3.7. The QMI message size issue is not considered important enough for stable. It does not affect the primary device function at all, and I consider it more of a feature enhancement than a fix. Bjørn Mork (2): net: qmi_wwan: increase max QMI message size to 4096 net: qmi_wwan: use a single bind function for all device types drivers/net/usb/qmi_wwan.c | 47 1 file changed, 17 insertions(+), 30 deletions(-) -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net-next 1/2] net: qmi_wwan: increase max QMI message size to 4096
QMI requests exceeding 1500 bytes are possible and
device firmware does not handle fragmented messages
very well. It is therefore necessary to increase
the maximum message size from the current 512 bytes.
The protocol message size limit is not documented
in any publicly known source, but the out of tree
driver from CodeAurora use 4 kB. This is therefore
chosen as the new arbitrary default until the real
limit is known.
This should allow any QMI message to be transmitted
without fragmentation, fixing known issues with GPS
assistance data upload.
Signed-off-by: Bjørn Mork
---
drivers/net/usb/qmi_wwan.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 328397c..516653f 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -108,7 +108,7 @@ static int qmi_wwan_register_subdriver(struct usbnet *dev)
atomic_set(&info->pmcount, 0);
/* register subdriver */
- subdriver = usb_cdc_wdm_register(info->control, &dev->status->desc,
512, &qmi_wwan_cdc_wdm_manage_power);
+ subdriver = usb_cdc_wdm_register(info->control, &dev->status->desc,
4096, &qmi_wwan_cdc_wdm_manage_power);
if (IS_ERR(subdriver)) {
dev_err(&info->control->dev, "subdriver registration failed\n");
rv = PTR_ERR(subdriver);
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net-next 2/2] net: qmi_wwan: use a single bind function for all device types
Refactoring the bind code lets us use a common driver_info struct
for all supported devices, simplifying the code a bit. The
real advantage is that devices using the CDC ECM interface
layout now also can be added dynamically using the new_id sysfs
interface. This simplifies testing of new devices.
Signed-off-by: Bjørn Mork
---
drivers/net/usb/qmi_wwan.c | 45
1 file changed, 16 insertions(+), 29 deletions(-)
diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 516653f..8492333 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -139,10 +139,18 @@ static int qmi_wwan_bind(struct usbnet *dev, struct
usb_interface *intf)
BUILD_BUG_ON((sizeof(((struct usbnet *)0)->data) < sizeof(struct
qmi_wwan_state)));
- /* require a single interrupt status endpoint for subdriver */
+ /* control and data is shared? */
+ if (intf->cur_altsetting->desc.bNumEndpoints == 3) {
+ info->control = intf;
+ info->data = intf;
+ goto shared;
+ }
+
+ /* else require a single interrupt status endpoint on control intf */
if (intf->cur_altsetting->desc.bNumEndpoints != 1)
goto err;
+ /* and a number of CDC descriptors */
while (len > 3) {
struct usb_descriptor_header *h = (void *)buf;
@@ -231,8 +239,9 @@ next_desc:
if (status < 0)
goto err;
+shared:
status = qmi_wwan_register_subdriver(dev);
- if (status < 0) {
+ if (status < 0 && info->control != info->data) {
usb_set_intfdata(info->data, NULL);
usb_driver_release_interface(driver, info->data);
}
@@ -241,20 +250,6 @@ err:
return status;
}
-/* Some devices combine the "control" and "data" functions into a
- * single interface with all three endpoints: interrupt + bulk in and
- * out
- */
-static int qmi_wwan_bind_shared(struct usbnet *dev, struct usb_interface *intf)
-{
- struct qmi_wwan_state *info = (void *)&dev->data;
-
- /* control and data is shared */
- info->control = intf;
- info->data = intf;
- return qmi_wwan_register_subdriver(dev);
-}
-
static void qmi_wwan_unbind(struct usbnet *dev, struct usb_interface *intf)
{
struct qmi_wwan_state *info = (void *)&dev->data;
@@ -330,20 +325,12 @@ static const struct driver_info qmi_wwan_info = {
.manage_power = qmi_wwan_manage_power,
};
-static const struct driver_infoqmi_wwan_shared = {
- .description= "WWAN/QMI device",
- .flags = FLAG_WWAN,
- .bind = qmi_wwan_bind_shared,
- .unbind = qmi_wwan_unbind,
- .manage_power = qmi_wwan_manage_power,
-};
-
#define HUAWEI_VENDOR_ID 0x12D1
/* map QMI/wwan function by a fixed interface number */
#define QMI_FIXED_INTF(vend, prod, num) \
USB_DEVICE_INTERFACE_NUMBER(vend, prod, num), \
- .driver_info = (unsigned long)&qmi_wwan_shared
+ .driver_info = (unsigned long)&qmi_wwan_info
/* Gobi 1000 QMI/wwan interface number is 3 according to qcserial */
#define QMI_GOBI1K_DEVICE(vend, prod) \
@@ -367,15 +354,15 @@ static const struct usb_device_id products[] = {
/* 2. Combined interface devices matching on class+protocol */
{ /* Huawei E392, E398 and possibly others in "Windows mode" */
USB_VENDOR_AND_INTERFACE_INFO(HUAWEI_VENDOR_ID,
USB_CLASS_VENDOR_SPEC, 1, 17),
- .driver_info= (unsigned long)&qmi_wwan_shared,
+ .driver_info= (unsigned long)&qmi_wwan_info,
},
{ /* Pantech UML290 */
USB_DEVICE_AND_INTERFACE_INFO(0x106c, 0x3718,
USB_CLASS_VENDOR_SPEC, 0xf0, 0xff),
- .driver_info= (unsigned long)&qmi_wwan_shared,
+ .driver_info= (unsigned long)&qmi_wwan_info,
},
{ /* Pantech UML290 - newer firmware */
USB_DEVICE_AND_INTERFACE_INFO(0x106c, 0x3718,
USB_CLASS_VENDOR_SPEC, 0xf1, 0xff),
- .driver_info= (unsigned long)&qmi_wwan_shared,
+ .driver_info= (unsigned long)&qmi_wwan_info,
},
/* 3. Combined interface devices matching on interface number */
@@ -457,7 +444,7 @@ static int qmi_wwan_probe(struct usb_interface *intf, const
struct usb_device_id
*/
if (!id->driver_info) {
dev_dbg(&intf->dev, "setting defaults for dynamic device id\n");
- id->driver_info = (unsigned long)&qmi_wwan_shared;
+ id->driver_info = (unsigned long)&qmi_wwan_info;
}
return usbnet_probe(intf, id);
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/10] drivers/net/usb/sierra_net.c: removes unnecessary semicolon
From: Peter Senna Tschudin Date: Thu, 6 Sep 2012 18:09:08 +0200 > From: Peter Senna Tschudin > > removes unnecessary semicolon > > Found by Coccinelle: http://coccinelle.lip6.fr/ > > Signed-off-by: Peter Senna Tschudin Applied, thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: changing usbnet's API to better deal with cdc-ncm
On Friday 07 September 2012 17:23:33 Alexey ORISHKO wrote: > There is a temptation to send full NTBs even with a single IP packet, > But it will lead to wasted USB bandwidth and reduced ability to send > real data for other functions in the device or other devices on the > same root hub. Most important it will also harm IN direction. Well, we will eventually need to do so. In that case, shall we send a short package or not? > The challenge is to ensure that an acceptable timeout value is used. Too > long and latency is introduced. Too short and too many padded NTBs will > go out and that reduces the available throughput. The ideal timer value > depends on the throughput available in the network. Something that is not > really explicitly known to the NCM layer nor to the layer above. Well, we know how many packages are available to the device because they have already been scheduled. It seems to be clear that we need to batch while enough are on the way. > Alternate methods exist to achieve the same result without using a timer. > But an optimal implementation requires that the amount of IP packets "in > progress" or queued up is known to NCM so NCM can decide to send short or > padded NTB or aggregate and send one or more full NTBs plus short or > padded NTB. What exactly means "in progress"? > An implementation where NCM only knows if there is more data available or > not can be shown to have side-effects that are not easily circumvented. > And likewise shown to limit throughput compared to a timer-based solution. Well, I must say that the timer is ugly. If absolutely necessary we can keep it, but then I'd much prefer to put it into usbnet and have a generic batching functionality. However, I'd like to explore aternatives. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net-next 0/2] qmi_wwan changes intended for 3.7
From: Bjørn Mork Date: Fri, 7 Sep 2012 19:36:05 +0200 > Just a couple of minor improvements for 3.7. The QMI > message size issue is not considered important enough for > stable. It does not affect the primary device function > at all, and I consider it more of a feature enhancement > than a fix. > > > Bjørn Mork (2): > net: qmi_wwan: increase max QMI message size to 4096 > net: qmi_wwan: use a single bind function for all device types Both applied, thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: Add Interface Association Descriptor to ECM
On Wed, Jan 25, 2012 at 11:02:03AM +0100, Linus Walleij wrote:
> --- a/drivers/usb/gadget/f_ecm.c
> +++ b/drivers/usb/gadget/f_ecm.c
> @@ -97,6 +97,20 @@ static inline unsigned ecm_bitrate(struct usb_gadget *g)
>
> /* interface descriptor: */
>
> +static struct usb_interface_assoc_descriptor
> +ecm_iad_descriptor = {
> + .bLength = sizeof ecm_iad_descriptor,
> + .bDescriptorType = USB_DT_INTERFACE_ASSOCIATION,
> +
> + /* .bFirstInterface = DYNAMIC, */
> + .bInterfaceCount = 2, /* control + data */
> + .bFunctionClass = USB_CLASS_COMM,
> + .bFunctionSubClass =USB_CDC_SUBCLASS_ETHERNET,
> + .bFunctionProtocol =USB_CDC_PROTO_NONE,
> + /* .iFunction = DYNAMIC */
> +};
> +
> +
> static struct usb_interface_descriptor ecm_control_intf = {
> .bLength = sizeof ecm_control_intf,
> .bDescriptorType = USB_DT_INTERFACE,
> @@ -199,6 +213,7 @@ static struct usb_endpoint_descriptor fs_ecm_out_desc = {
>
> static struct usb_descriptor_header *ecm_fs_function[] = {
> /* CDC ECM control descriptors */
> + (struct usb_descriptor_header *) &ecm_iad_descriptor,
> (struct usb_descriptor_header *) &ecm_control_intf,
> (struct usb_descriptor_header *) &ecm_header_desc,
> (struct usb_descriptor_header *) &ecm_union_desc,
> @@ -247,6 +262,7 @@ static struct usb_endpoint_descriptor hs_ecm_out_desc = {
>
> static struct usb_descriptor_header *ecm_hs_function[] = {
> /* CDC ECM control descriptors */
> + (struct usb_descriptor_header *) &ecm_iad_descriptor,
> (struct usb_descriptor_header *) &ecm_control_intf,
> (struct usb_descriptor_header *) &ecm_header_desc,
> (struct usb_descriptor_header *) &ecm_union_desc,
Why did you add this IAD thingy to FS and HS and not SS descriptors? I don't
see a reason so I would guess you just forgot :)
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] combine / merge usb descriptor generation
I've been looking how to avoid '#include "[fu]_*.c"' and I think I start with common code for the creation / handling of descriptors. I don't understand why each / most gadget set f->hs_descriptors and f->ss_descriptors depending on the gadget speed. This makes no sense as composite does not "leak" SS descriptors if the device operates has HS speed. So removing these speed checks wouldn't hurt and should make the code easier. The next thing is what we do with each descriptor. They are global and yet we update some of them: We set string ids, interface id and endpoint address. This is something each gadget does on its own. Looking closer at those descriptors there is a common pattern: The SS descriptors contain all possible USB descriptors for a given subset. The HS descriptors are the same as SS except they don't have a composite descriptor and the max packet size is smaller. The FS descriptors are the same as HS except for the max packet size. The endpoint address is the same for each endpoint of the same class (read as the first in the FS-set, HS-set, SS-set share the same endpoint address). The address can be also shared across alternative settings since both can't be used at the same time. I've been looking at this for a while but it seems like a good idea to provide one descriptor set per function and let composite assign the dynamic values like endpoint address,…. This would get rid of the global variables and make creation of descriptors easier / with less code. Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH resend2] Input: usbtouchscreen - initialize eGalax devices
Hi Dmitry,
On Tue, Sep 04, 2012 at 11:07:04PM -0700, Dmitry Torokhov wrote:
> On Mon, Sep 03, 2012 at 01:33:50PM -0400, Forest Bond wrote:
> > From: Forest Bond
> >
> > Certain eGalax devices expose an interface with class HID and protocol
> > None. Some work with usbhid and some work with usbtouchscreen, but
> > there is no easy way to differentiate. Sending an eGalax diagnostic
> > packet seems to kick them all into using the right protocol for
> > usbtouchscreen, so we can continue to bind them all there (as opposed to
> > handing some off to usbhid).
> >
> > This fixes a regression for devices that were claimed by (and worked
> > with) usbhid prior to commit 139ebe8dc80dd74cb2ac9f5603d18fbf5cff049f
> > ("Input: usbtouchscreen - fix eGalax HID ignoring"), which made
> > usbtouchscreen claim them instead. With this patch they will still be
> > claimed by usbtouchscreen, but they will actually report events
> > usbtouchscreen can understand. Note that these devices will be limited
> > to the usbtouchscreen feature set so e.g. dual touch features are not
> > supported.
> >
> > I have the distinct pleasure of needing to support devices of both types
> > and have tested accordingly.
> >
> > Signed-off-by: Forest Bond
>
> Applied, thank you Forest.
Great, thanks a lot.
The other piece to this puzzle is that usbhid should blacklist these devices to
avoid binding if it happens to be loaded before usbtouchscreen. To do this,
usbhid needs to be able to blacklist devices based on interface protocol (right
now it only supports blacklist on VID + PID).
Would you accept a patch set that implements this?
Thanks,
Forest
--
Forest Bond
http://www.alittletooquiet.net
http://www.rapidrollout.com
signature.asc
Description: Digital signature
Re: [PATCH] usb: gadget: Add Interface Association Descriptor to ECM
On Fri, Sep 7, 2012 at 8:51 PM, Sebastian Andrzej Siewior
wrote:
> On Wed, Jan 25, 2012 at 11:02:03AM +0100, Linus Walleij wrote:
>> static struct usb_descriptor_header *ecm_fs_function[] = {
>> /* CDC ECM control descriptors */
>> + (struct usb_descriptor_header *) &ecm_iad_descriptor,
>> (struct usb_descriptor_header *) &ecm_control_intf,
>> (struct usb_descriptor_header *) &ecm_header_desc,
>> (struct usb_descriptor_header *) &ecm_union_desc,
>> @@ -247,6 +262,7 @@ static struct usb_endpoint_descriptor hs_ecm_out_desc = {
>>
>> static struct usb_descriptor_header *ecm_hs_function[] = {
>> /* CDC ECM control descriptors */
>> + (struct usb_descriptor_header *) &ecm_iad_descriptor,
>> (struct usb_descriptor_header *) &ecm_control_intf,
>> (struct usb_descriptor_header *) &ecm_header_desc,
>> (struct usb_descriptor_header *) &ecm_union_desc,
>
> Why did you add this IAD thingy to FS and HS and not SS descriptors? I don't
> see a reason so I would guess you just forgot :)
Argh, Praveena, save me here ... I was being patch secretary
this time, but to my untrained USB-eyes it looks like you're right,
interested in patching it?
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] How to handle delays in isochronous transfers?
On Fri, Sep 7, 2012 at 11:44 PM, Alan Stern wrote: > On Fri, 7 Sep 2012, Peter Chen wrote: > >> > >> > Here's the problem we face: The class driver submits packets 1, 2, 3, >> > and 4. They get sent properly, but the completion interrupt is >> > delayed. As a result, the class driver's completion handler doesn't >> > get called until too late; the frames for packets 5 - 44 have already >> > expired. The data that should have been sent during those frames is >> > lost. >> > >> > But the class driver doesn't know this, so when the completion handler >> > does get called, it submits packets 5, 6, 7, and 8. They end up >> > getting sent in frames 45, 46, 47, and 48. This means the data is now >> > out of synchronization by 40 frames. >> > >> > That's the problem I want to solve. >> > >> Can I understand this problem like below: >> >> It is OUT ISO transfer, at each completion handler, it prepares >> 4 iTDs for 4 packets. >> >> When the above problem occurs, the class driver will submit packet >> from 5 - 48, and the controller driver will prepare 44 iTDs. (or the class >> driver still submits packet 5-8?) > > The class driver submits packets 5-8, because each time the completion > handler runs it prepares 4 iTDs. > >> If the feedback is supported, the device will know host sends data slowly, >> it will give speed up feedback information after it receives packet 5 or >> other >> packets depends on its interval at descriptor. At this information, it can >> tell >> the host to increase the packet size, then the transaction length and >> transaction >> numbers at iTD can be increased(Assume it was not maximum). > > Clemens pointed out that this won't work if the delay is too long. > Clements said "In such a situation, the delay is much bigger than the device's buffer, so just sending more samples afterwards will not help." before. I don't understand what will not be helped? And what will happen when the thing goes to wrong? > Alan Stern > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: Add Interface Association Descriptor to ECM
On Sat, Sep 08, 2012 at 12:04:18AM +0200, Linus Walleij wrote: > Argh, Praveena, save me here ... I was being patch secretary > this time, but to my untrained USB-eyes it looks like you're right, > interested in patching it? Don't worry, I take care of this. Just wanted to get sure that this was not on purpose. > Yours, > Linus Walleij Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
