[3.8-rc3 -> 3.8-rc4 regression] Re: [PATCH] module, async: async_synchronize_full() on module init iff async is used
Hi, Tejun Heo wrote: > This avoids the described deadlock because iosched module doesn't use > async and thus wouldn't invoke async_synchronize_full(). This is > hacky and incomplete. It will deadlock if async module loading nests; > however, this works around the known problem case and seems to be the > best of bad options. > > For more details, please refer to the following thread. > > http://thread.gmane.org/gmane.linux.kernel/1420814 My laptop fails to boot[1] with the message 'Volume group "data" not found'. Bisects to v3.8-rc4~17 (the above commit). Reverting that commit on top of current "master" (d92581fcad18, 2013-08-10) produces a working kernel. dmesg output from that working kernel attached. More details, including .config, at [2]. Any ideas for tracking this down? Thanks, Jonathan [1] Screenshot: http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=5;filename=bad_3.10.3-1.jpg;att=1;bug=719464 Screenshot in recovery mode: http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=5;filename=bad_3.10.3-1_recovery.jpg;att=2;bug=719464 [2] http://bugs.debian.org/719464 [0.00] Initializing cgroup subsys cpuset [0.00] Initializing cgroup subsys cpu [0.00] Initializing cgroup subsys cpuacct [0.00] Linux version 3.11.0-rc4+ (jrn@elie) (gcc version 4.7.3 (Debian 4.7.3-6) ) #2 SMP Sun Aug 11 23:20:20 PDT 2013 [0.00] Command line: BOOT_IMAGE=/vmlinuz-3.11.0-rc4+ root=/dev/mapper/data-root ro quiet [0.00] e820: BIOS-provided physical RAM map: [0.00] BIOS-e820: [mem 0x-0x0009fbff] usable [0.00] BIOS-e820: [mem 0x0009fc00-0x0009] reserved [0.00] BIOS-e820: [mem 0x000e-0x000f] reserved [0.00] BIOS-e820: [mem 0x0010-0x6e545fff] usable [0.00] BIOS-e820: [mem 0x6e546000-0x6e745fff] ACPI NVS [0.00] BIOS-e820: [mem 0x6e746000-0x6fd3efff] usable [0.00] BIOS-e820: [mem 0x6fd3f000-0x6fdbefff] reserved [0.00] BIOS-e820: [mem 0x6fdbf000-0x6febefff] ACPI NVS [0.00] BIOS-e820: [mem 0x6febf000-0x6fef6fff] ACPI data [0.00] BIOS-e820: [mem 0x6fef7000-0x6fef] usable [0.00] BIOS-e820: [mem 0xf800-0xf8ff] reserved [0.00] BIOS-e820: [mem 0xfec0-0xfec00fff] reserved [0.00] BIOS-e820: [mem 0xfec1-0xfec10fff] reserved [0.00] BIOS-e820: [mem 0xfee0-0xfee00fff] reserved [0.00] BIOS-e820: [mem 0xffe0-0x] reserved [0.00] NX (Execute Disable) protection: active [0.00] SMBIOS 2.6 present. [0.00] DMI: TOSHIBA Satellite C650D/Portable PC, BIOS 1.60 09/02/2010 [0.00] e820: update [mem 0x-0x0fff] usable ==> reserved [0.00] e820: remove [mem 0x000a-0x000f] usable [0.00] No AGP bridge found [0.00] e820: last_pfn = 0x6ff00 max_arch_pfn = 0x4 [0.00] MTRR default type: uncachable [0.00] MTRR fixed ranges enabled: [0.00] 0-9 write-back [0.00] A-B uncachable [0.00] C-F write-through [0.00] MTRR variable ranges enabled: [0.00] 0 base mask C000 write-back [0.00] 1 base 4000 mask E000 write-back [0.00] 2 base 6000 mask F000 write-back [0.00] 3 base 6FEBE000 mask F000 uncachable [0.00] 4 base FFE0 mask FFE0 write-protect [0.00] 5 disabled [0.00] 6 disabled [0.00] 7 disabled [0.00] x86 PAT enabled: cpu 0, old 0x7040600070406, new 0x7010600070106 [0.00] Base memory trampoline at [88099000] 99000 size 24576 [0.00] Using GB pages for direct mapping [0.00] init_memory_mapping: [mem 0x-0x000f] [0.00] [mem 0x-0x000f] page 4k [0.00] BRK [0x0184c000, 0x0184cfff] PGTABLE [0.00] BRK [0x0184d000, 0x0184dfff] PGTABLE [0.00] BRK [0x0184e000, 0x0184efff] PGTABLE [0.00] init_memory_mapping: [mem 0x6fa0-0x6fbf] [0.00] [mem 0x6fa0-0x6fbf] page 2M [0.00] BRK [0x0184f000, 0x0184] PGTABLE [0.00] init_memory_mapping: [mem 0x6c00-0x6e545fff] [0.00] [mem 0x6c00-0x6e3f] page 2M [0.00] [mem 0x6e40-0x6e545fff] page 4k [0.00] BRK [0x0185, 0x01850fff] PGTABLE [0.00] init_memory_mapping: [mem 0x6e746000-0x6f9f] [0.00] [mem 0x6e746000-0x6e7f] page 4k [0.00] [mem 0x6e80-0x6f9f] page 2M [0.00] init_memory_mapping: [mem 0x0010-0x6bff] [0.00] [mem 0x0010-0x001f] page 4k [0.00] [mem 0x0020-0x6bff] page 2M [0.00] init
Re: [PATCH] usb: chipidea: enable to build host support as module
Hi Peter, On Mon, Aug 12, 2013 at 09:28:28AM +0800, Peter Chen wrote: > On Sun, Aug 11, 2013 at 01:00:49PM +0200, Luka Perkov wrote: > > USB_CHIPIDEA_HOST does not need to depend on USB=y, USB_CHIPIDEA_HOST will > > work > > just fine even if USB=m is used. The depends line can be safely removed > > since > > USB_CHIPIDEA already depends on USB. ... > Lothar has already posted a solution for it. > > http://marc.info/?l=linux-usb&m=137533651927159&w=2 Great, didn't know that. Luka -- 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 2/3] staging: dwc2: add NAK holdoff patch from downstream Pi kernel
Hi Paul, > Add the NAK holdoff patch from the downstream Raspberry Pi kernel. > This allows the transfer scheduler to better handle "cheeky devices > that just hold off using NAKs". > @@ -365,6 +366,7 @@ struct dwc2_hsotg { > u8 otg_port; > u32 *frame_list; > dma_addr_t frame_list_dma; > + int next_sched_frame; This variable is still not really used, I think. Most of the mentions in the patch are assignments, except for these two: > + if (list_empty(&hsotg->periodic_sched_inactive) || > + dwc2_frame_num_le(qh->sched_frame, hsotg->next_sched_frame)) > + hsotg->next_sched_frame = qh->sched_frame; > + ... > + if (!dwc2_frame_num_le(hsotg->next_sched_frame, > +qh->sched_frame)) > + hsotg->next_sched_frame = > + qh->sched_frame; However, these two "uses" of the variable have only the single purpose of updating the variable itself, no other behaviour is influenced by it. In effect, the variable does not influence the code at all and can be dropped, significantly simplifying this patch. Gr. Matthijs -- 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 3/3] staging: dwc2: add microframe scheduler from downstream Pi kernel
Hi Paul & Dom, I haven't got time to look closely right now, but at first glance most of my comments have been resolved. One thing that is still in there, is this piece of code for which I'm not sure if it is really related to the topic of the patch: > @@ -780,6 +784,10 @@ static void dwc2_assign_and_init_hc(struct dwc2_hsotg > *hsotg, > chan->data_pid_start = qh->data_toggle; > chan->multi_count = 1; > > + if ((urb->actual_length < 0 || urb->actual_length > urb->length) && > + !dwc2_hcd_is_pipe_in(&urb->pipe_info)) > + urb->actual_length = urb->length; > + > if (hsotg->core_params->dma_enable > 0) { > chan->xfer_dma = urb->dma + urb->actual_length; > Paul, did you try the patch without this hunk? Dom, can you tell use why this hunk is needed? Gr. Matthijs -- 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 0/3] staging: dwc2: 2nd try at uframe scheduler patch
Hi Paul, > Matthijs' concern about periodic endpoints with bInterval=1 seems > to be unfounded. I tried a webcam, which uses a bInterval=1 isoc > endpoint, on my PCI-based dev board, and it still works fine with > this patch applied. For the record, I still think this concern actually exists, but you will need to completely fill up the available bandwidth to notice the problem, since the scheduler is now too optimistic about how much bandwidth is availble when devices with bInterval < 8 are present (but pessimistic for devices with bInterval > 8). However, this is something I think we can easily fix later, so getting this series in is a step forward anyway. I haven't got time to properly review and test these patches now, since I'm about to leave for a two-week vacation though. I did a quick scan just now and replied with some comments that occured to me. Gr. Matthijs -- 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 03/13] staging: dwc2: unshift non-bool register value constants
Hey Paul, > OK, I'm kind of on the fence about this one, so if you prefer it this way > I guess it's OK with me. > > Acked-by: Paul Zimmerman Thanks. > What platforms have you tested this on, BTW? You said you have a Raspberry > Pi, so I'd like you to test these patches there before resubmitting them. > And I'll test them on my PCI-based platform. I mostly tested the patches on the Ralink RT3052 SoC. I think I did some basic testing on the Pi at some point, but I'll have to look again to do some proper testing on there as well. I had hoped to get around to this last week, but unfortunately that didn't work out. Since I'm leaving on a vacation today, I won't be able to resubmit this series until over two weeks at the earliest. Gr. Matthijs -- 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: FUSB200 xhci issue
Am 10.08.2013 13:57, schrieb Alan Stern: On Sat, 10 Aug 2013, Oleksij Rempel wrote: usb reset do not affect behaviour of firmware. At least after i remove all attempts to reboot FW from driver. If adapter will got reset signal, FW will be notified about it. Then FW will remove reset flag and will just continue to work. After usb reset, lsusb show correct, update information - EP3 and EP4 was updated from INT to BULK. I assume, no i need to add to the driver some kind of firmware check. What is the proper way to do it? The simplest way is to put a new value for the device descriptor's bcdDevice value in the firmware. Then all you have to do is check that value. Since adding fw check will need fw version update. I would like to do quick fix for current kernel and firmware version. I will revert EP3 and EP4 from bulk back to interrupt. But, before sending patch to the list i would like to know, how to reproduce the bug which was fixed by converting this endpoints from interrupt to bulk. -- Regards, Oleksij -- Regards, Oleksij -- 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 3/3] staging: dwc2: add microframe scheduler from downstream Pi kernel
On Sun, Aug 11, 2013 at 12:50:19PM -0700, Paul Zimmerman wrote: > +static int dwc2_find_single_uframe(struct dwc2_hsotg *hsotg, struct dwc2_qh > *qh) > +{ > + unsigned short utime = qh->usecs; > + int done = 0; > + int i = 0; > + int ret = -1; > + > + while (!done) { I don't care for these while (!done) loops. If there is nothing else to use as a limitter then just do while (1). In this case, we are count from 0 to 7 so it should be: static int dwc2_find_single_uframe(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh) { unsigned short utime = qh->usecs; int i; for (i = 0; i < 8; i++) { if (utime <= hsotg->frame_usecs[i]) { hsotg->frame_usecs[i] -= utime; qh->frame_usecs[i] += utime; return i; } } return -1; } Notice how I've separated out the success and failure paths? Now we don't it's immediately clear what the success and return values are. The error code is returned to dwc2_schedule_periodic() which has some spaghetti code and then prints a misleading error message. > + /* At the start hsotg->frame_usecs[i] = max_uframe_usecs[i] */ > + if (utime <= hsotg->frame_usecs[i]) { > + hsotg->frame_usecs[i] -= utime; > + qh->frame_usecs[i] += utime; > + ret = i; > + done = 1; > + } else { > + i++; > + if (i == 8) > + done = 1; > + } > + } > + > + return ret; > +} > + > +/* > + * use this for FS apps that can span multiple uframes > + */ > +static int dwc2_find_multi_uframe(struct dwc2_hsotg *hsotg, struct dwc2_qh > *qh) > +{ > + unsigned short utime = qh->usecs; > + unsigned short xtime; > + int t_left = utime; No need to set t_left here. > + int done = 0; > + int i = 0; > + int j; > + int ret = -1; > + > + while (!done) { for (i = 0; i < 8; i++) { > + if (hsotg->frame_usecs[i] <= 0) { This test is worrying because ->frame_usecs[] is unsigned and can't be less than zero. > + i++; > + if (i == 8) { > + ret = -1; > + done = 1; > + } > + continue; > + } This chunk becomes: if (hsotg->frame_usecs[i] <= 0) continue; > + > + /* > + * we need n consecutive slots so use j as a start slot > + * j plus j+1 must be enough time (for now) > + */ > + xtime = hsotg->frame_usecs[i]; > + for (j = i + 1; j < 8; j++) { > + /* > + * if we add this frame remaining time to xtime we may > + * be OK, if not we need to test j for a complete frame > + */ > + if (xtime + hsotg->frame_usecs[j] < utime) { > + if (hsotg->frame_usecs[j] < > + max_uframe_usecs[j]) { > + ret = -1; > + break; > + } > + } > + if (xtime >= utime) { > + ret = i; I would like to move the code from after the loop to here but we would run into the 80 character limit. One option is to add a variable and then test it when we have fewer indents, but a better option is to create a new function. Anyway, here is what it looks like: t_left = utime; for (j = i; j < 8; j++) { t_left -= hsotg->frame_usecs[j]; if (t_left <= 0) { qh->frame_usecs[j] += hsotg->frame_usecs[j] + t_left; hsotg->frame_usecs[j] = -t_left; return i; } qh->frame_usecs[j] += hsotg->frame_usecs[j]; hsotg->frame_usecs[j] = 0; } I'm not sure we should be saying "j = i" or if it should be "j = i + 1". I removed the "t_left > 0" because we just return directly now. > + break; > + } > + /* add the frame time to x time */ > + xtime += hsotg->frame_usecs[j]; > + /* we must have a fully available next frame or break */ > + if (xtime < utime && > +
Re: [PATCH v2 3/6] usb: phy: msm: Migrate to Managed Device Resource allocation
Hi, On Mon, 2013-07-29 at 15:25 +0300, Felipe Balbi wrote: > Hi, > > On Mon, Jul 29, 2013 at 10:04:21AM +0300, Ivan T. Ivanov wrote: > > motg->irq = platform_get_irq(pdev, 0); > > - if (!motg->irq) { > > + if (motg->irq < 0) { > > looks like this particular hunk isn't part of $subject. > Indeed. Will remove it. Thanks, Ivan -- 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: Non-enumerable devices on USB and other enumerable buses
On Sun, Aug 11, 2013 at 09:53:01PM -0400, Alan Stern wrote: > On Sun, 11 Aug 2013, Mark Brown wrote: > > One example that's bugging me right now is that on the Insignal Arndale > > platform there's a USB hub connected to one of the USB ports on the SoC > > (not as a PHY, it seems we also need the internal PHY running to talk to > > the device). The hub needs to be "plugged" into the SoC after the SoC > > USB controller has started with some GPIOs so we need to tell the system > > that the hub exists and needs to be synchronised with the USB controller. > On the surface, this seems like a particularly simple case. Why wait > until the SoC's USB controller has started? Why not "plug in" the hub > via the GPIOs right from the beginning? I tried that, it doesn't seem to work - for some reason it seems that the hub is only successfully enumerated if it starts after its parent is running. I don't know enough about USB to speculate on why that might be, the GPIOs are brining the device out of reset not applying power or anything. > > Another case that's going to be problematic once it's in mainline is > > Slimbus - this is a bus used in some embedded audio subsystems which is > > enumerable in a similar manner to USB but where the devices on the bus > > are normally powered up only on demand (causing them to hotplug when > > used and unplug when idle) and have at least interrupt lines wired to > > the SoC using a normal interrupt outside the enumerable bus. > That is indeed more difficult, because it requires geniune cooperation > between the bus and platform subsystems. Yeah. You might want to do the same with for example a USB network controller when you're in flight only mode, that seems to be one of the more common reasons for doing this sort of thing with USB. signature.asc Description: Digital signature
[PATCH 2/2] USB: chipidea: i.MX: simplify usbmisc
The chipidea i.MX driver is split into two drivers. The ci_hdrc_imx driver handles the chipidea cores and the usbmisc_imx driver handles the noncore registers common to all chipidea cores (but SoC specific). Current flow is: - usbmisc sets an ops pointer in the ci_hdrc_imx driver during probe - ci_hdrc_imx checks if the pointer is valid during probe, if yes calls the functions in the ops pointer. - usbmisc_imx calls back into the ci_hdrc_imx driver to get additional data This is overly complicated and has problems if the drivers are compiled as modules. In this case the usbmisc_imx driver can be unloaded even if the ci_hdrc_imx driver still needs usbmisc functionality. This patch changes this by letting the ci_hdrc_imx driver calling functions from the usbmisc_imx driver. This way the symbol resolving during module load makes sure the ci_hdrc_imx driver depends on the usbmisc_imx driver. Also instead of letting the usbmisc_imx driver call back into the ci_hdrc_imx driver, pass the needed data in the first place. Signed-off-by: Sascha Hauer --- drivers/usb/chipidea/ci_hdrc_imx.c | 71 +--- drivers/usb/chipidea/ci_hdrc_imx.h | 17 ++- drivers/usb/chipidea/usbmisc_imx.c | 95 +- 3 files changed, 76 insertions(+), 107 deletions(-) diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c index 11ed423..b9464ff 100644 --- a/drivers/usb/chipidea/ci_hdrc_imx.c +++ b/drivers/usb/chipidea/ci_hdrc_imx.c @@ -29,57 +29,48 @@ struct ci_hdrc_imx_data { struct platform_device *ci_pdev; struct clk *clk; struct regulator *reg_vbus; + struct imx_usbmisc_data *usbmisc_data; }; -static const struct usbmisc_ops *usbmisc_ops; - /* Common functions shared by usbmisc drivers */ -int usbmisc_set_ops(const struct usbmisc_ops *ops) -{ - if (usbmisc_ops) - return -EBUSY; - - usbmisc_ops = ops; - - return 0; -} -EXPORT_SYMBOL_GPL(usbmisc_set_ops); - -void usbmisc_unset_ops(const struct usbmisc_ops *ops) -{ - usbmisc_ops = NULL; -} -EXPORT_SYMBOL_GPL(usbmisc_unset_ops); - -int usbmisc_get_init_data(struct device *dev, struct usbmisc_usb_device *usbdev) +static struct imx_usbmisc_data *usbmisc_get_init_data(struct device *dev) { struct device_node *np = dev->of_node; struct of_phandle_args args; + struct imx_usbmisc_data *data; int ret; - usbdev->dev = dev; + /* +* In case the fsl,usbmisc property is not present this device doesn't +* need usbmisc. Return NULL (which is no error here) +*/ + if (!of_get_property(np, "fsl,usbmisc", NULL)) + return NULL; + + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); + if (!data) + return ERR_PTR(-ENOMEM); ret = of_parse_phandle_with_args(np, "fsl,usbmisc", "#index-cells", 0, &args); if (ret) { dev_err(dev, "Failed to parse property fsl,usbmisc, errno %d\n", ret); - memset(usbdev, 0, sizeof(*usbdev)); - return ret; + return ERR_PTR(ret); } - usbdev->index = args.args[0]; + + data->index = args.args[0]; of_node_put(args.np); if (of_find_property(np, "disable-over-current", NULL)) - usbdev->disable_oc = 1; + data->disable_oc = 1; if (of_find_property(np, "external-vbus-divider", NULL)) - usbdev->evdo = 1; + data->evdo = 1; - return 0; + return data; } -EXPORT_SYMBOL_GPL(usbmisc_get_init_data); /* End of common functions shared by usbmisc drivers*/ @@ -96,10 +87,6 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev) struct resource *res; int ret; - if (of_find_property(pdev->dev.of_node, "fsl,usbmisc", NULL) - && !usbmisc_ops) - return -EPROBE_DEFER; - data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); if (!data) { dev_err(&pdev->dev, "Failed to allocate ci_hdrc-imx data!\n"); @@ -112,6 +99,10 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev) return -ENOENT; } + data->usbmisc_data = usbmisc_get_init_data(&pdev->dev); + if (IS_ERR(data->usbmisc_data)) + return PTR_ERR(data->usbmisc_data); + data->clk = devm_clk_get(&pdev->dev, NULL); if (IS_ERR(data->clk)) { dev_err(&pdev->dev, @@ -159,11 +150,11 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev) if (!pdev->dev.coherent_dma_mask) pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32); - if (usbmisc_ops && usbmisc_ops->init) { - ret = usbmisc_ops->init(&pdev->dev); + if (data->usbmisc_data) { + ret = imx_usbmisc_init(data->usbmisc_data);
[PATCH v2] USB chipidea i.MX: Fix module loading
The Chipidea i.MX driver has some issues with module loading dependencies. This fixes this. It is an alternative approach to the one Peter Chen suggested that does without changing the dt binding. Sascha changes since v1: - make usbmisc optional when fsl,usbmisc property is not present. Needed for i.MX23/28 as suggested by Peter Chen Sascha Hauer (2): USB: chipidea: i.MX: remove unused define USB: chipidea: i.MX: simplify usbmisc drivers/usb/chipidea/ci_hdrc_imx.c | 74 + drivers/usb/chipidea/ci_hdrc_imx.h | 17 ++- drivers/usb/chipidea/usbmisc_imx.c | 95 +- 3 files changed, 76 insertions(+), 110 deletions(-) -- 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 1/2] USB: chipidea: i.MX: remove unused define
Signed-off-by: Sascha Hauer --- drivers/usb/chipidea/ci_hdrc_imx.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c index 14362c0..11ed423 100644 --- a/drivers/usb/chipidea/ci_hdrc_imx.c +++ b/drivers/usb/chipidea/ci_hdrc_imx.c @@ -24,9 +24,6 @@ #include "ci.h" #include "ci_hdrc_imx.h" -#define pdev_to_phy(pdev) \ - ((struct usb_phy *)platform_get_drvdata(pdev)) - struct ci_hdrc_imx_data { struct usb_phy *phy; struct platform_device *ci_pdev; -- 1.8.4.rc1 -- 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: Non-enumerable devices on USB and other enumerable buses
[Adding Olof] On Mon, Aug 12, 2013 at 10:51:36AM +0100, Mark Brown wrote: > On Sun, Aug 11, 2013 at 09:53:01PM -0400, Alan Stern wrote: > > On Sun, 11 Aug 2013, Mark Brown wrote: > > > > One example that's bugging me right now is that on the Insignal Arndale > > > platform there's a USB hub connected to one of the USB ports on the SoC > > > (not as a PHY, it seems we also need the internal PHY running to talk to > > > the device). The hub needs to be "plugged" into the SoC after the SoC > > > USB controller has started with some GPIOs so we need to tell the system > > > that the hub exists and needs to be synchronised with the USB controller. > > > On the surface, this seems like a particularly simple case. Why wait > > until the SoC's USB controller has started? Why not "plug in" the hub > > via the GPIOs right from the beginning? > > I tried that, it doesn't seem to work - for some reason it seems that > the hub is only successfully enumerated if it starts after its parent is > running. I don't know enough about USB to speculate on why that might > be, the GPIOs are brining the device out of reset not applying power or > anything. > > > > Another case that's going to be problematic once it's in mainline is > > > Slimbus - this is a bus used in some embedded audio subsystems which is > > > enumerable in a similar manner to USB but where the devices on the bus > > > are normally powered up only on demand (causing them to hotplug when > > > used and unplug when idle) and have at least interrupt lines wired to > > > the SoC using a normal interrupt outside the enumerable bus. > > > That is indeed more difficult, because it requires geniune cooperation > > between the bus and platform subsystems. > > Yeah. You might want to do the same with for example a USB network > controller when you're in flight only mode, that seems to be one of the > more common reasons for doing this sort of thing with USB. As I understand it, the wifi chip on the Snow Chromebook has a similar issue -- it hangs off of a probeable SDIO bus, but needs a regulator poked for it to turn on and become probeable (see exynos_wifi_bt_set_power in [1]). Thanks, Mark. [1] http://git.chromium.org/gitweb/?p=chromiumos/third_party/kernel.git;a=blob;f=arch/arm/mach-exynos/mach-exynos5-dt.c;h=d131241ea78b7d89df21e676e284ddfd369b4da0;hb=refs/heads/chromeos-3.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: Non-enumerable devices on USB and other enumerable buses
On Sun, Aug 11, 2013 at 07:02:57PM -0700, Greg Kroah-Hartman wrote: > On Sun, Aug 11, 2013 at 08:08:26PM +0100, Mark Brown wrote: > > I know there's been some discussion of this topic but do we have any > > general consensus on how to handle such things both from a Linux driver > > model point of view and from a DT/ACPI point of view? > As these are usually bus-specific things, and really, platform-specific I don't think they're bus specific - the main issue with a lot of this is that they're outside the infrastructure that the bus standardises so we should have a general way of understanding this. There will be bits where the bus needs to get involved but the overall pattern is generic. > things, I'd fall back and just recommend a "platform driver" that > "knows" about where these gpios are, and how/when to enable them, which > will cause the discoverable bus logic to kick in now that it notices a > device is present/removed. This doesn't work in general. These things aren't really platform specific at all, they're features of the devices that apply to any system using them and while for some use cases (like WiFi and BT power) an external thing that just manually applies power will be enough in other cases we want to know about the device even while it's off and the driver might be able to do extra things at runtime if it knows about for example having some control over signals to the device. For example in the Slimbus case we're normally talking about audio CODECs. In order to preserve the userspace API the device has to appear to be present at all times, the driver will remember the settings the user is making to be applied when it actually powers up and indeed the powering up should be kicked off as a result of userspac acting on the apparently present device. Another example here (including USB devices) is interrupt signals wired directly back to the processor, completely outside of the bus. > Perhaps a semi-generic "platform" driver could be created, that knows > how to handle these settings in the DT, but odds are, that might be > difficult to make generic enough to cover a wide range of boards, but > without specifics, it's hard to tell. There's things like the rfkill stuff already, and the reset controller on the way, but again this only covers a fairly small subset of the issues. signature.asc Description: Digital signature
Re: Non-enumerable devices on USB and other enumerable buses
On Mon, Aug 12, 2013 at 12:07:14PM +0100, Mark Rutland wrote: > As I understand it, the wifi chip on the Snow Chromebook has a similar > issue -- it hangs off of a probeable SDIO bus, but needs a regulator > poked for it to turn on and become probeable (see > exynos_wifi_bt_set_power in [1]). Yes, that's another example - this stuff is really common in practical systems, especially the power control for radios since that's how rfkill tends to end up being implemented. signature.asc Description: Digital signature
kernel Oops: 0003 on usbhid_submit_report
When communicating with AT90USB1287, at random intervals (1/25 boots) the linux hid_output_field Oopses and kills the communicating thread. The AT90USB1287 microcontroller uses LUFA library for usb/hid communication. It is trigged by a ioctl call from userspace and fails in a kernel paging request. The system is after the oops in a state where no hid commands is sent anymore and only a boot can fix the system. Keywords: usbhid hid Kernel version: Linux version 3.8.13-03081301-generic (apw@gomeisa) (gcc version 4.6.3 (Ubuntu/Linaro 4.6.3-1ubuntu5) ) #201305311535 SMP Fri May 31 19:44:30 UTC 2013 Oopses: http://paste.debian.net/24305 http://paste.debian.net/24306 http://paste.debian.net/24307 Code: The error is triggered by: ioctl(fd,HIDIOCSUSAGES, &ref_multi_u); ioctl(fd,HIDIOCSREPORT, &rep_info_u); Notes: It is very hard to reproduce so seems like race condition… Any tips to resolve/workaround this issue is appreciated and please let me know if my information is incomplete (This is my first kernel bug report) Andreas Lillebø Holm -- 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: [alsa-devel] [PATCH] usb-audio: Fix invalid volume resolution for Logitech HD Webcam C525
At Sat, 10 Aug 2013 12:20:02 +0400, Maksim A. Boyko wrote: > > Add the volume control quirk for avoiding the kernel warning > for the Logitech HD Webcam C525 > as in the similar commit 36691e1be6ec551eef4a5225f126a281f8c051c2 > for the Logitech HD Webcam C310. > > Reported-by: Maksim Boyko > Tested-by: Maksim Boyko > Cc: # 3.10.5+ > Signed-off-by: Maksim Boyko Thanks, applied. Takashi > -- > sound/usb/mixer.c |1 + > 1 file changed, 1 insertion(+) > > --- linux-3.10.5-orig/sound/usb/mixer.c 2013-08-04 12:51:49.0 > +0400 > +++ linux-3.10.5/sound/usb/mixer.c2013-08-06 21:12:42.274197287 > +0400 > @@ -888,6 +888,7 @@ static void volume_control_quirks(struct > case USB_ID(0x046d, 0x081b): /* HD Webcam c310 */ > case USB_ID(0x046d, 0x081d): /* HD Webcam c510 */ > case USB_ID(0x046d, 0x0825): /* HD Webcam c270 */ > + case USB_ID(0x046d, 0x0826): /* HD Webcam c525 */ > case USB_ID(0x046d, 0x0991): > /* Most audio usb devices lie about volume resolution. >* Most Logitech webcams have res = 384. > > > -- > WBR, Maksim > > P.S. Please, use this patch due to it should have correct tab symbols. > > > ___ > Alsa-devel mailing list > alsa-de...@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel > -- 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 v3 5/6] usb: phy: tegra: Program new PHY parameters
The Tegra30 TRM recommends configuration of certain PHY parameters for optimal quality. Program the following registers based on device tree parameters: - UTMIP_XCVR_HSSLEW: HS slew rate control. - UTMIP_HSSQUELCH_LEVEL: HS squelch detector level - UTMIP_HSDISCON_LEVEL: HS disconnect detector level. These registers exist in Tegra20, but programming them hasn't been necessary, so these parameters won't be set on Tegra20 to keep the device trees backward compatible. Additionally, the UTMIP_XCVR_SETUP parameter can be set from fuses instead of a software-programmed value, as the optimal value can vary between invidual boards. The boolean property nvidia,xcvr-setup-use-fuses can be used to enable this behaviour. Signed-off-by: Tuomas Tynkkynen Tested-by: Stephen Warren Reviewed-by: Stephen Warren --- v2: Don't touch new PHY params under T20 v2: Change register bitmasks to not contain shifted-out bits drivers/usb/phy/phy-tegra-usb.c | 81 ++- include/linux/usb/tegra_usb_phy.h | 4 ++ 2 files changed, 67 insertions(+), 18 deletions(-) diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c index 1ad184a..3bfb3d1 100644 --- a/drivers/usb/phy/phy-tegra-usb.c +++ b/drivers/usb/phy/phy-tegra-usb.c @@ -99,11 +99,15 @@ #define UTMIP_FORCE_PD2_POWERDOWN(1 << 16) #define UTMIP_FORCE_PDZI_POWERDOWN (1 << 18) #define UTMIP_XCVR_LSBIAS_SEL(1 << 21) -#define UTMIP_XCVR_HSSLEW_MSB(x) (((x) & 0x7f) << 25) +#define UTMIP_XCVR_HSSLEW(x) (((x) & 0x3) << 4) +#define UTMIP_XCVR_HSSLEW_MSB(x) x) & 0x1fc) >> 2) << 25) #define UTMIP_BIAS_CFG00x80c #define UTMIP_OTGPD (1 << 11) #define UTMIP_BIASPD (1 << 10) +#define UTMIP_HSSQUELCH_LEVEL(x) (((x) & 0x3) << 0) +#define UTMIP_HSDISCON_LEVEL(x) (((x) & 0x3) << 2) +#define UTMIP_HSDISCON_LEVEL_MSB(x) x) & 0x4) >> 2) << 24) #define UTMIP_HSRX_CFG00x810 #define UTMIP_ELASTIC_LIMIT(x) (((x) & 0x1f) << 10) @@ -255,6 +259,7 @@ static void utmip_pad_power_on(struct tegra_usb_phy *phy) { unsigned long val, flags; void __iomem *base = phy->pad_regs; + struct tegra_utmip_config *config = phy->config; clk_prepare_enable(phy->pad_clk); @@ -263,6 +268,16 @@ static void utmip_pad_power_on(struct tegra_usb_phy *phy) if (utmip_pad_count++ == 0) { val = readl(base + UTMIP_BIAS_CFG0); val &= ~(UTMIP_OTGPD | UTMIP_BIASPD); + + if (phy->soc_config->requires_extra_tuning_parameters) { + val &= ~(UTMIP_HSSQUELCH_LEVEL(~0) | + UTMIP_HSDISCON_LEVEL(~0) | + UTMIP_HSDISCON_LEVEL_MSB(~0)); + + val |= UTMIP_HSSQUELCH_LEVEL(config->hssquelch_level); + val |= UTMIP_HSDISCON_LEVEL(config->hsdiscon_level); + val |= UTMIP_HSDISCON_LEVEL_MSB(config->hsdiscon_level); + } writel(val, base + UTMIP_BIAS_CFG0); } @@ -431,12 +446,20 @@ static int utmi_phy_power_on(struct tegra_usb_phy *phy) val &= ~(UTMIP_FORCE_PD_POWERDOWN | UTMIP_FORCE_PD2_POWERDOWN | UTMIP_FORCE_PDZI_POWERDOWN | UTMIP_XCVR_LSBIAS_SEL | UTMIP_XCVR_SETUP(~0) | UTMIP_XCVR_SETUP_MSB(~0) | -UTMIP_XCVR_LSFSLEW(~0) | UTMIP_XCVR_LSRSLEW(~0) | -UTMIP_XCVR_HSSLEW_MSB(~0)); - val |= UTMIP_XCVR_SETUP(config->xcvr_setup); - val |= UTMIP_XCVR_SETUP_MSB(config->xcvr_setup); +UTMIP_XCVR_LSFSLEW(~0) | UTMIP_XCVR_LSRSLEW(~0)); + + if (!config->xcvr_setup_use_fuses) { + val |= UTMIP_XCVR_SETUP(config->xcvr_setup); + val |= UTMIP_XCVR_SETUP_MSB(config->xcvr_setup); + } val |= UTMIP_XCVR_LSFSLEW(config->xcvr_lsfslew); val |= UTMIP_XCVR_LSRSLEW(config->xcvr_lsrslew); + + if (phy->soc_config->requires_extra_tuning_parameters) { + val &= ~(UTMIP_XCVR_HSSLEW(~0) | UTMIP_XCVR_HSSLEW_MSB(~0)); + val |= UTMIP_XCVR_HSSLEW(config->xcvr_hsslew); + val |= UTMIP_XCVR_HSSLEW_MSB(config->xcvr_hsslew); + } writel(val, base + UTMIP_XCVR_CFG0); val = readl(base + UTMIP_XCVR_CFG1); @@ -450,14 +473,14 @@ static int utmi_phy_power_on(struct tegra_usb_phy *phy) val |= UTMIP_BIAS_PDTRK_COUNT(0x5); writel(val, base + UTMIP_BIAS_CFG1); - if (phy->is_legacy_phy) { - val = readl(base + UTMIP_SPARE_CFG0); - if (phy->mode == USB_DR_MODE_PERIPHERAL) - val &= ~FUSE_SETUP_SEL; - else - val |= FUSE_SETUP_SEL; - writel(val, base + UTMIP_SPARE_CFG0); - } else { + val = readl(base + UTMIP_SPARE_CFG0); +
[PATCH v3 6/6] usb: host: tegra: Tegra30 support
The Tegra30 EHCI controller is mostly compatible with the Tegra20 controller, except Tegra30 includes the HOSTPC register extension. The has_hostpc capability bit must be set in the ehci_hcd structure if the controller has such extensions. The new tegra_ehci_soc_config structure is added to describe the differences between the SoCs. Signed-off-by: Tuomas Tynkkynen Tested-by: Stephen Warren Reviewed-by: Stephen Warren --- v3: Removed useless cast drivers/usb/host/ehci-tegra.c | 34 +- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c index db8031f..78fa76d 100644 --- a/drivers/usb/host/ehci-tegra.c +++ b/drivers/usb/host/ehci-tegra.c @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -50,6 +51,10 @@ static struct hc_driver __read_mostly tegra_ehci_hc_driver; +struct tegra_ehci_soc_config { + bool has_hostpc; +}; + static int (*orig_hub_control)(struct usb_hcd *hcd, u16 typeReq, u16 wValue, u16 wIndex, char *buf, u16 wLength); @@ -320,8 +325,24 @@ static void tegra_ehci_unmap_urb_for_dma(struct usb_hcd *hcd, struct urb *urb) free_dma_aligned_buffer(urb); } +static const struct tegra_ehci_soc_config tegra30_soc_config = { + .has_hostpc = true, +}; + +static const struct tegra_ehci_soc_config tegra20_soc_config = { + .has_hostpc = false, +}; + +static struct of_device_id tegra_ehci_of_match[] = { + { .compatible = "nvidia,tegra30-ehci", .data = &tegra30_soc_config }, + { .compatible = "nvidia,tegra20-ehci", .data = &tegra20_soc_config }, + { }, +}; + static int tegra_ehci_probe(struct platform_device *pdev) { + const struct of_device_id *match; + const struct tegra_ehci_soc_config *soc_config; struct resource *res; struct usb_hcd *hcd; struct ehci_hcd *ehci; @@ -330,6 +351,13 @@ static int tegra_ehci_probe(struct platform_device *pdev) int irq; struct usb_phy *u_phy; + match = of_match_device(tegra_ehci_of_match, &pdev->dev); + if (!match) { + dev_err(&pdev->dev, "Error: No device match found\n"); + return -ENODEV; + } + soc_config = match->data; + /* Right now device-tree probed devices don't get dma_mask set. * Since shared usb code relies on it, set it here for now. * Once we have dma capability bindings this can go away. @@ -391,6 +419,7 @@ static int tegra_ehci_probe(struct platform_device *pdev) goto cleanup_clk_en; } ehci->caps = hcd->regs + 0x100; + ehci->has_hostpc = soc_config->has_hostpc; err = usb_phy_init(hcd->phy); if (err) { @@ -468,11 +497,6 @@ static void tegra_ehci_hcd_shutdown(struct platform_device *pdev) hcd->driver->shutdown(hcd); } -static struct of_device_id tegra_ehci_of_match[] = { - { .compatible = "nvidia,tegra20-ehci", }, - { }, -}; - static struct platform_driver tegra_ehci_driver = { .probe = tegra_ehci_probe, .remove = tegra_ehci_remove, -- 1.8.1.5 -- 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 v3 4/6] Documentation: New DT parameters for tegra30-usb-phy
Document the new device tree parameters for Tegra30 USB PHY. Signed-off-by: Tuomas Tynkkynen Tested-by: Stephen Warren Reviewed-by: Stephen Warren --- .../devicetree/bindings/usb/nvidia,tegra20-usb-phy.txt| 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra20-usb-phy.txt b/Documentation/devicetree/bindings/usb/nvidia,tegra20-usb-phy.txt index 4c8ade8..ba797d3 100644 --- a/Documentation/devicetree/bindings/usb/nvidia,tegra20-usb-phy.txt +++ b/Documentation/devicetree/bindings/usb/nvidia,tegra20-usb-phy.txt @@ -3,7 +3,7 @@ Tegra SOC USB PHY The device node for Tegra SOC USB PHY: Required properties : - - compatible : Should be "nvidia,tegra20-usb-phy". + - compatible : Should be "nvidia,tegra-usb-phy". - reg : Defines the following set of registers, in the order listed: - The PHY's own register set. Always present. @@ -24,17 +24,26 @@ Required properties : Required properties for phy_type == ulpi: - nvidia,phy-reset-gpio : The GPIO used to reset the PHY. -Required PHY timing params for utmi phy: +Required PHY timing params for utmi phy, for all chips: - nvidia,hssync-start-delay : Number of 480 Mhz clock cycles to wait before start of sync launches RxActive - nvidia,elastic-limit : Variable FIFO Depth of elastic input store - nvidia,idle-wait-delay : Number of 480 Mhz clock cycles of idle to wait before declare IDLE. - nvidia,term-range-adj : Range adjusment on terminations - - nvidia,xcvr-setup : HS driver output control + - Either one of the following for HS driver output control: +- nvidia,xcvr-setup : integer, uses the provided value. +- nvidia,xcvr-setup-use-fuses : boolean, indicates that the value is read + from the on-chip fuses +If both are provided, nvidia,xcvr-setup-use-fuses takes precedence. - nvidia,xcvr-lsfslew : LS falling slew rate control. - nvidia,xcvr-lsrslew : LS rising slew rate control. +Required PHY timing params for utmi phy, only on Tegra30 and above: + - nvidia,xcvr-hsslew : HS slew rate control. + - nvidia,hssquelch-level : HS squelch detector level. + - nvidia,hsdiscon-level : HS disconnect detector level. + Optional properties: - nvidia,has-legacy-mode : boolean indicates whether this controller can operate in legacy mode (as APX 2500 / 2600). In legacy mode some -- 1.8.1.5 -- 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 v3 2/6] usb: phy: tegra: Fix wrong PHY parameters
Some of the PHY parameters are not set according to the TRMs: - UTMIP_FS_PREABMLE_J should be set, not cleared - UTMIP_XCVR_LSBIAS_SEL should be cleared, not set - UTMIP_PD_CHRG should be set in host mode and cleared in device mode - UTMIP_XCVR_SETUP is a two-part field; the upper bits were not set properly Signed-off-by: Tuomas Tynkkynen Tested-by: Stephen Warren Reviewed-by: Stephen Warren --- v2: Change register bitmasks to not contain shifted-out bits drivers/usb/phy/phy-tegra-usb.c | 20 ++-- 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c index 49fa2da..ebbf85f 100644 --- a/drivers/usb/phy/phy-tegra-usb.c +++ b/drivers/usb/phy/phy-tegra-usb.c @@ -86,11 +86,13 @@ #define UTMIP_XCVR_CFG00x808 #define UTMIP_XCVR_SETUP(x) (((x) & 0xf) << 0) +#define UTMIP_XCVR_SETUP_MSB(x) x) & 0x70) >> 4) << 22) #define UTMIP_XCVR_LSRSLEW(x)(((x) & 0x3) << 8) #define UTMIP_XCVR_LSFSLEW(x)(((x) & 0x3) << 10) #define UTMIP_FORCE_PD_POWERDOWN (1 << 14) #define UTMIP_FORCE_PD2_POWERDOWN(1 << 16) #define UTMIP_FORCE_PDZI_POWERDOWN (1 << 18) +#define UTMIP_XCVR_LSBIAS_SEL(1 << 21) #define UTMIP_XCVR_HSSLEW_MSB(x) (((x) & 0x7f) << 25) #define UTMIP_BIAS_CFG00x80c @@ -342,7 +344,7 @@ static int utmi_phy_power_on(struct tegra_usb_phy *phy) } val = readl(base + UTMIP_TX_CFG0); - val &= ~UTMIP_FS_PREABMLE_J; + val |= UTMIP_FS_PREABMLE_J; writel(val, base + UTMIP_TX_CFG0); val = readl(base + UTMIP_HSRX_CFG0); @@ -381,16 +383,26 @@ static int utmi_phy_power_on(struct tegra_usb_phy *phy) val = readl(base + USB_SUSP_CTRL); val &= ~(USB_WAKE_ON_CNNT_EN_DEV | USB_WAKE_ON_DISCON_EN_DEV); writel(val, base + USB_SUSP_CTRL); + + val = readl(base + UTMIP_BAT_CHRG_CFG0); + val &= ~UTMIP_PD_CHRG; + writel(val, base + UTMIP_BAT_CHRG_CFG0); + } else { + val = readl(base + UTMIP_BAT_CHRG_CFG0); + val |= UTMIP_PD_CHRG; + writel(val, base + UTMIP_BAT_CHRG_CFG0); } utmip_pad_power_on(phy); val = readl(base + UTMIP_XCVR_CFG0); val &= ~(UTMIP_FORCE_PD_POWERDOWN | UTMIP_FORCE_PD2_POWERDOWN | -UTMIP_FORCE_PDZI_POWERDOWN | UTMIP_XCVR_SETUP(~0) | +UTMIP_FORCE_PDZI_POWERDOWN | UTMIP_XCVR_LSBIAS_SEL | +UTMIP_XCVR_SETUP(~0) | UTMIP_XCVR_SETUP_MSB(~0) | UTMIP_XCVR_LSFSLEW(~0) | UTMIP_XCVR_LSRSLEW(~0) | UTMIP_XCVR_HSSLEW_MSB(~0)); val |= UTMIP_XCVR_SETUP(config->xcvr_setup); + val |= UTMIP_XCVR_SETUP_MSB(config->xcvr_setup); val |= UTMIP_XCVR_LSFSLEW(config->xcvr_lsfslew); val |= UTMIP_XCVR_LSRSLEW(config->xcvr_lsrslew); writel(val, base + UTMIP_XCVR_CFG0); @@ -401,10 +413,6 @@ static int utmi_phy_power_on(struct tegra_usb_phy *phy) val |= UTMIP_XCVR_TERM_RANGE_ADJ(config->term_range_adj); writel(val, base + UTMIP_XCVR_CFG1); - val = readl(base + UTMIP_BAT_CHRG_CFG0); - val &= ~UTMIP_PD_CHRG; - writel(val, base + UTMIP_BAT_CHRG_CFG0); - val = readl(base + UTMIP_BIAS_CFG1); val &= ~UTMIP_BIAS_PDTRK_COUNT(~0); val |= UTMIP_BIAS_PDTRK_COUNT(0x5); -- 1.8.1.5 -- 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 v3 0/6] USB tree changes for Tegra30 and Tegra114 USB Host support
Hi all, Here are the patches for the USB tree to enable USB Host support on Tegra30 and Tegra114. These are based on my and Mikko's cleanup patches that just got merged to Felipe's tree. The first one touches the core hub code to prevent certain (non-standard) clock disable features as our controller doesn't support those. The rest are changes to our PHY and EHCI drivers to deal with a few changed register addresses and a few additional PHY parameters that need to be set for proper signal quality. v2 diff: Patch 1: Added Alan's ack Patch 2 & 5: Change register bitmasks to not contain shifted-out bits Changed the order of patches 4 & 5 Patch 5: Don't touch new PHY params under T20 v3 diff: Patch 6: Removed useless cast Added Stephen's Reviewed-by & Tested-by to all patches Tuomas Tynkkynen (6): usb: host: add has_tdi_phy_lpm capability bit usb: phy: tegra: Fix wrong PHY parameters usb: phy: tegra: Tegra30 support Documentation: New DT parameters for tegra30-usb-phy usb: phy: tegra: Program new PHY parameters usb: host: tegra: Tegra30 support .../bindings/usb/nvidia,tegra20-usb-phy.txt| 15 +- drivers/usb/chipidea/host.c| 1 + drivers/usb/host/ehci-hub.c| 14 +- drivers/usb/host/ehci-tegra.c | 34 +++- drivers/usb/host/ehci.h| 1 + drivers/usb/phy/phy-tegra-usb.c| 220 - include/linux/usb/tegra_usb_phy.h | 23 +++ 7 files changed, 242 insertions(+), 66 deletions(-) -- 1.8.1.5 -- 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 v3 1/6] usb: host: add has_tdi_phy_lpm capability bit
The has_hostpc capability bit indicates that the host controller has the HOSTPC register extensions, but at the same time enables clock disabling power saving features with the PHY Low Power Clock Disable (PHCD) bit. However, some host controllers have the HOSTPC extensions but don't support the low-power feature, so the PHCD bit must not be set on those controllers. Add a separate capability bit for the low-power feature instead, and change all existing users of has_hostpc to use this new capability bit. The idea for this commit is taken from an old 2012 commit that never got merged ("disociate chipidea PHY low power suspend control from hostpc") Inspired-by: Matthieu CASTET Signed-off-by: Tuomas Tynkkynen Acked-by: Alan Stern Tested-by: Stephen Warren Reviewed-by: Stephen Warren --- v2: Add Alan's ack drivers/usb/chipidea/host.c | 1 + drivers/usb/host/ehci-hub.c | 14 +++--- drivers/usb/host/ehci.h | 1 + 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c index 40d0fda..9b3aaf1 100644 --- a/drivers/usb/chipidea/host.c +++ b/drivers/usb/chipidea/host.c @@ -63,6 +63,7 @@ static int host_start(struct ci_hdrc *ci) ehci = hcd_to_ehci(hcd); ehci->caps = ci->hw_bank.cap; ehci->has_hostpc = ci->hw_bank.lpm; + ehci->has_tdi_phy_lpm = ci->hw_bank.lpm; ret = usb_add_hcd(hcd, 0, 0); if (ret) diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c index 6e69ee1..6d5ad37 100644 --- a/drivers/usb/host/ehci-hub.c +++ b/drivers/usb/host/ehci-hub.c @@ -183,7 +183,7 @@ static void ehci_adjust_port_wakeup_flags(struct ehci_hcd *ehci, spin_lock_irq(&ehci->lock); /* clear phy low-power mode before changing wakeup flags */ - if (ehci->has_hostpc) { + if (ehci->has_tdi_phy_lpm) { port = HCS_N_PORTS(ehci->hcs_params); while (port--) { u32 __iomem *hostpc_reg = &ehci->regs->hostpc[port]; @@ -217,7 +217,7 @@ static void ehci_adjust_port_wakeup_flags(struct ehci_hcd *ehci, } /* enter phy low-power mode again */ - if (ehci->has_hostpc) { + if (ehci->has_tdi_phy_lpm) { port = HCS_N_PORTS(ehci->hcs_params); while (port--) { u32 __iomem *hostpc_reg = &ehci->regs->hostpc[port]; @@ -309,7 +309,7 @@ static int ehci_bus_suspend (struct usb_hcd *hcd) } } - if (changed && ehci->has_hostpc) { + if (changed && ehci->has_tdi_phy_lpm) { spin_unlock_irq(&ehci->lock); msleep(5); /* 5 ms for HCD to enter low-power mode */ spin_lock_irq(&ehci->lock); @@ -435,7 +435,7 @@ static int ehci_bus_resume (struct usb_hcd *hcd) goto shutdown; /* clear phy low-power mode before resume */ - if (ehci->bus_suspended && ehci->has_hostpc) { + if (ehci->bus_suspended && ehci->has_tdi_phy_lpm) { i = HCS_N_PORTS(ehci->hcs_params); while (i--) { if (test_bit(i, &ehci->bus_suspended)) { @@ -788,7 +788,7 @@ static int ehci_hub_control ( goto error; /* clear phy low-power mode before resume */ - if (ehci->has_hostpc) { + if (ehci->has_tdi_phy_lpm) { temp1 = ehci_readl(ehci, hostpc_reg); ehci_writel(ehci, temp1 & ~HOSTPC_PHCD, hostpc_reg); @@ -1025,12 +1025,12 @@ static int ehci_hub_control ( /* After above check the port must be connected. * Set appropriate bit thus could put phy into low power -* mode if we have hostpc feature +* mode if we have tdi_phy_lpm feature */ temp &= ~PORT_WKCONN_E; temp |= PORT_WKDISC_E | PORT_WKOC_E; ehci_writel(ehci, temp | PORT_SUSPEND, status_reg); - if (ehci->has_hostpc) { + if (ehci->has_tdi_phy_lpm) { spin_unlock_irqrestore(&ehci->lock, flags); msleep(5);/* 5ms for HCD enter low pwr mode */ spin_lock_irqsave(&ehci->lock, flags); diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h index 64f9a08..d034d94 100644 --- a/drivers/usb/host/ehci.h +++ b/drivers/usb/host/ehci.h @@ -210,6 +210,7 @@ struct ehci_hcd { /* one per controller */ #define OHCI_HCCTRL_LEN 0x4 __hc32 *ohci_hcctrl_reg; unsignedhas_hostpc:1; + unsignedhas_tdi_phy_lpm:1; unsignedhas_ppc
[PATCH v3 3/6] usb: phy: tegra: Tegra30 support
The Tegra30 USB PHY is a bit different than the Tegra20 PHY: - The EHCI controller supports the HOSTPC register extension, and some of the fields that the PHY needs to modify (PHCD and PTS) have moved to the new HOSTPC register. - Some of the UTMI PLL configuration registers have moved from the USB register space to the Clock-And-Reset controller space. In Tegra30 the clock driver is responsible for configuring the UTMI PLL. - The USBMODE register must be explicitly written to enter host mode. - Certain PHY parameters need to be programmed for optimal signal quality. Support for this will be added in the next patch. The new tegra_phy_soc_config structure is added to describe the differences between the SoCs. Signed-off-by: Tuomas Tynkkynen Tested-by: Stephen Warren Reviewed-by: Stephen Warren --- drivers/usb/phy/phy-tegra-usb.c | 121 +- include/linux/usb/tegra_usb_phy.h | 19 ++ 2 files changed, 112 insertions(+), 28 deletions(-) diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c index ebbf85f..1ad184a 100644 --- a/drivers/usb/phy/phy-tegra-usb.c +++ b/drivers/usb/phy/phy-tegra-usb.c @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -39,11 +40,16 @@ #define ULPI_VIEWPORT 0x170 -/* PORTSC registers */ +/* PORTSC PTS/PHCD bits, Tegra20 only */ #define TEGRA_USB_PORTSC1 0x184 #define TEGRA_USB_PORTSC1_PTS(x) (((x) & 0x3) << 30) #define TEGRA_USB_PORTSC1_PHCD (1 << 23) +/* HOSTPC1 PTS/PHCD bits, Tegra30 and above */ +#define TEGRA_USB_HOSTPC1_DEVLC0x1b4 +#define TEGRA_USB_HOSTPC1_DEVLC_PTS(x) (((x) & 0x7) << 29) +#define TEGRA_USB_HOSTPC1_DEVLC_PHCD (1 << 22) + /* Bits of PORTSC1, which will get cleared by writing 1 into them */ #define TEGRA_PORTSC1_RWC_BITS (PORT_CSC | PORT_PEC | PORT_OCC) @@ -141,6 +147,12 @@ #define UTMIP_BIAS_CFG10x83c #define UTMIP_BIAS_PDTRK_COUNT(x)(((x) & 0x1f) << 3) +/* For Tegra30 and above only, the address is different in Tegra20 */ +#define USB_USBMODE0x1f8 +#define USB_USBMODE_MASK (3 << 0) +#define USB_USBMODE_HOST (3 << 0) +#define USB_USBMODE_DEVICE (2 << 0) + static DEFINE_SPINLOCK(utmip_pad_lock); static int utmip_pad_count; @@ -193,10 +205,17 @@ static void set_pts(struct tegra_usb_phy *phy, u8 pts_val) void __iomem *base = phy->regs; unsigned long val; - val = readl(base + TEGRA_USB_PORTSC1) & ~TEGRA_PORTSC1_RWC_BITS; - val &= ~TEGRA_USB_PORTSC1_PTS(3); - val |= TEGRA_USB_PORTSC1_PTS(pts_val & 3); - writel(val, base + TEGRA_USB_PORTSC1); + if (phy->soc_config->has_hostpc) { + val = readl(base + TEGRA_USB_HOSTPC1_DEVLC); + val &= ~TEGRA_USB_HOSTPC1_DEVLC_PTS(~0); + val |= TEGRA_USB_HOSTPC1_DEVLC_PTS(pts_val); + writel(val, base + TEGRA_USB_HOSTPC1_DEVLC); + } else { + val = readl(base + TEGRA_USB_PORTSC1) & ~TEGRA_PORTSC1_RWC_BITS; + val &= ~TEGRA_USB_PORTSC1_PTS(~0); + val |= TEGRA_USB_PORTSC1_PTS(pts_val); + writel(val, base + TEGRA_USB_PORTSC1); + } } static void set_phcd(struct tegra_usb_phy *phy, bool enable) @@ -204,12 +223,21 @@ static void set_phcd(struct tegra_usb_phy *phy, bool enable) void __iomem *base = phy->regs; unsigned long val; - val = readl(base + TEGRA_USB_PORTSC1) & ~TEGRA_PORTSC1_RWC_BITS; - if (enable) - val |= TEGRA_USB_PORTSC1_PHCD; - else - val &= ~TEGRA_USB_PORTSC1_PHCD; - writel(val, base + TEGRA_USB_PORTSC1); + if (phy->soc_config->has_hostpc) { + val = readl(base + TEGRA_USB_HOSTPC1_DEVLC); + if (enable) + val |= TEGRA_USB_HOSTPC1_DEVLC_PHCD; + else + val &= ~TEGRA_USB_HOSTPC1_DEVLC_PHCD; + writel(val, base + TEGRA_USB_HOSTPC1_DEVLC); + } else { + val = readl(base + TEGRA_USB_PORTSC1) & ~PORT_RWC_BITS; + if (enable) + val |= TEGRA_USB_PORTSC1_PHCD; + else + val &= ~TEGRA_USB_PORTSC1_PHCD; + writel(val, base + TEGRA_USB_PORTSC1); + } } static int utmip_pad_open(struct tegra_usb_phy *phy) @@ -367,17 +395,21 @@ static int utmi_phy_power_on(struct tegra_usb_phy *phy) val &= ~UTMIP_SUSPEND_EXIT_ON_EDGE; writel(val, base + UTMIP_MISC_CFG0); - val = readl(base + UTMIP_MISC_CFG1); - val &= ~(UTMIP_PLL_ACTIVE_DLY_COUNT(~0) | UTMIP_PLLU_STABLE_COUNT(~0)); - val |= UTMIP_PLL_ACTIVE_DLY_COUNT(phy->freq->active_delay) | - UTMIP_PLLU_STABLE_COUNT(phy->freq->stable_count); - writel(val, base + UTMIP_MISC_CFG1); -
Re: [PATCH] dwc3: dwc3-pci: Use SIMPLE_DEV_PM_OPS
On Wed, Aug 07, 2013 at 08:51:27AM -0300, Fabio Estevam wrote: > From: Fabio Estevam > > Using SIMPLE_DEV_PM_OPS can make the code simpler and cleaner. > > Signed-off-by: Fabio Estevam Reviewed-by: Mark Brown signature.asc Description: Digital signature
dealing with enclosures that don't handle READ_CAPACITY10
Hi, I got a bug report about an enclosure that mishandles READ_CAPACITY10. I don't want to introduce yet another quirk. So what about basing this on USB version? Regards Oliver >From 58cb3cd4b8a58d986b74e0a41d52de989dbe64bc Mon Sep 17 00:00:00 2001 From: Oliver Neukum Date: Mon, 12 Aug 2013 14:49:02 +0200 Subject: [PATCH] storage: use READ_CAPACITY16 first on 3.00 devices Some old devices crash when confronted with READ_CAPACITY16. Therefore READ_CAPACITY10 is used first and we depend on devices with more than 2TB correctly reporting an overflow. However some 3.0 devices do not report an overflow but capacity % 2TB. The value reported thus is entirely plausible. As these devices also work with another OS without partition tables, that should copy the expected behavior. Signed-off-by: Oliver Neukum --- drivers/usb/storage/scsiglue.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c index 92b05d9..04bf91d 100644 --- a/drivers/usb/storage/scsiglue.c +++ b/drivers/usb/storage/scsiglue.c @@ -108,6 +108,7 @@ static int slave_alloc (struct scsi_device *sdev) static int slave_configure(struct scsi_device *sdev) { struct us_data *us = host_to_us(sdev->host); + u16 usb_version; /* Many devices have trouble transferring more than 32KB at a time, * while others have trouble with more than 64K. At this time we @@ -211,8 +212,12 @@ static int slave_configure(struct scsi_device *sdev) /* * Many devices do not respond properly to READ_CAPACITY_16. * Tell the SCSI layer to try READ_CAPACITY_10 first. + * However some USB 3.0 drive enclosures return capacity + * modulo 2TB */ - sdev->try_rc_10_first = 1; + usb_version = le16_to_cpu(us->pusb_dev->descriptor.bcdUSB); + if (usb_version < '3' * 0x100) + sdev->try_rc_10_first = 1; /* assume SPC3 or latter devices support sense size > 18 */ if (sdev->scsi_level > SCSI_SPC_2) -- 1.8.3.1
Re: [alsa-devel] Buffer size for ALSA USB PCM audio
At Thu, 1 Aug 2013 13:37:45 -0400 (EDT), Alan Stern wrote: > > On Mon, 29 Jul 2013, Clemens Ladisch wrote: > > > Alan Stern wrote: > > > Clemens remarked some time ago that keeping the queue full would be > > > trivial, if only he knew how full it needed to be. The answer to that > > > is given above. I have been trying to make the appropriate changes, > > > but I'm not finding it "trivial". > > > > What I meant was that it would be trivial to change the lower bound of > > the period size (from which many queueing parameters are derived). > > Here's what I've got. In turns out the predicting the optimum number > of URBs needed is extremely difficult. I decided it would be better to > make an overestimate and then to submit URBs as needed, rather than > keeping all of them active all the time. > > I ended up with this patch (untested). It is certainly incomplete > because it doesn't address endpoints with implicit sync. It also > suffers from a race between snd_submit_urbs() and deactivate_urbs(): > an URB may be resubmitted after it has been deactivated. What's the reason to clear ep->active_mask at the beginning of snd_complete_urb()? > (In all > fairness, I think this race probably exists in the current code too -- > there are no spinlocks for synchronization.) The calls of usb_submit_urb() and usb_unlink_urb() might race indeed. The current code somehow assumed that the USB accepts such concurrent calls. deactivate_urbs() just kills the pending urbs and it doesn't change ep->active_mask bit by itself, and the synchronization is done in wait_clear_urbs(). So, if the concurrent calls of usb_submit_urb() and usb_unlink_urbs() were allowed, it should work as is (in the worst case, the complete will be called once again, but then it goes to exit_clear). thanks, Takashi > The patch also fixes a couple of unrelated items: MAX_PACKS vs. > MAX_PACKS_HS, and the maxsize calculation should realize that a packet > can't contain a partial frame. > > What do you think of this approach? > > Alan Stern > > > > sound/usb/card.h |7 + > sound/usb/endpoint.c | 185 > +-- > sound/usb/pcm.c |3 > 3 files changed, 114 insertions(+), 81 deletions(-) > > Index: usb-3.11/sound/usb/card.h > === > --- usb-3.11.orig/sound/usb/card.h > +++ usb-3.11/sound/usb/card.h > @@ -4,7 +4,7 @@ > #define MAX_NR_RATES 1024 > #define MAX_PACKS20 > #define MAX_PACKS_HS (MAX_PACKS * 8) /* in high speed mode */ > -#define MAX_URBS 8 > +#define MAX_URBS 11 > #define SYNC_URBS4 /* always four urbs for sync */ > #define MAX_QUEUE24 /* try not to exceed this queue length, in ms */ > > @@ -43,6 +43,7 @@ struct snd_urb_ctx { > struct snd_usb_endpoint *ep; > int index; /* index for urb array */ > int packets;/* number of packets per urb */ > + int data_packets; /* current number of data packets */ > int packet_size[MAX_PACKS_HS]; /* size of packets for next submission */ > struct list_head ready_list; > }; > @@ -99,6 +100,10 @@ struct snd_usb_endpoint { > int skip_packets; /* quirks for devices to ignore the > first n packets > in a stream */ > > + unsigned int min_queued_packs; /* USB hardware queue requirement */ > + unsigned int queued_packs; /* number of packets currently queued */ > + unsigned int queued_urbs; /* number of URBs currently queued */ > + int next_urb; /* next to submit */ > spinlock_t lock; > struct list_head list; > }; > Index: usb-3.11/sound/usb/pcm.c > === > --- usb-3.11.orig/sound/usb/pcm.c > +++ usb-3.11/sound/usb/pcm.c > @@ -1328,7 +1328,7 @@ static void prepare_playback_urb(struct > stride = runtime->frame_bits >> 3; > > frames = 0; > - urb->number_of_packets = 0; > + ctx->data_packets = urb->number_of_packets = 0; > spin_lock_irqsave(&subs->lock, flags); > for (i = 0; i < ctx->packets; i++) { > if (ctx->packet_size[i]) > @@ -1341,6 +1341,7 @@ static void prepare_playback_urb(struct > urb->iso_frame_desc[i].length = counts * ep->stride; > frames += counts; > urb->number_of_packets++; > + ctx->data_packets++; > subs->transfer_done += counts; > if (subs->transfer_done >= runtime->period_size) { > subs->transfer_done -= runtime->period_size; > Index: usb-3.11/sound/usb/endpoint.c > === > --- usb-3.11.orig/sound/usb/endpoint.c > +++ usb-3.11/sound/usb/endpoint.c > @@ -217,6 +217,7 @@ static void prepare_outbound_urb(struct > } > > urb-
Re: [PATCH 1/2] USB: chipidea: i.MX: remove unused define
On Mon, Aug 12, 2013 at 12:29:41PM +0200, Sascha Hauer wrote: > Signed-off-by: Sascha Hauer > --- > drivers/usb/chipidea/ci_hdrc_imx.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c > b/drivers/usb/chipidea/ci_hdrc_imx.c > index 14362c0..11ed423 100644 > --- a/drivers/usb/chipidea/ci_hdrc_imx.c > +++ b/drivers/usb/chipidea/ci_hdrc_imx.c > @@ -24,9 +24,6 @@ > #include "ci.h" > #include "ci_hdrc_imx.h" > > -#define pdev_to_phy(pdev) \ > - ((struct usb_phy *)platform_get_drvdata(pdev)) > - This one has already submitted by me, and queued by alex. -- Best Regards, Peter Chen -- 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 2/2] USB: chipidea: i.MX: simplify usbmisc
On Mon, Aug 12, 2013 at 12:29:42PM +0200, Sascha Hauer wrote: > The chipidea i.MX driver is split into two drivers. The ci_hdrc_imx driver > handles the chipidea cores and the usbmisc_imx driver handles the noncore > registers common to all chipidea cores (but SoC specific). Current flow is: > > - usbmisc sets an ops pointer in the ci_hdrc_imx driver during probe > - ci_hdrc_imx checks if the pointer is valid during probe, if yes calls > the functions in the ops pointer. > - usbmisc_imx calls back into the ci_hdrc_imx driver to get additional > data > > This is overly complicated and has problems if the drivers are compiled > as modules. In this case the usbmisc_imx driver can be unloaded even if > the ci_hdrc_imx driver still needs usbmisc functionality. > > This patch changes this by letting the ci_hdrc_imx driver calling functions > from the usbmisc_imx driver. This way the symbol resolving during module > load makes sure the ci_hdrc_imx driver depends on the usbmisc_imx driver. > > Also instead of letting the usbmisc_imx driver call back into the ci_hdrc_imx > driver, pass the needed data in the first place. > > Signed-off-by: Sascha Hauer > --- > drivers/usb/chipidea/ci_hdrc_imx.c | 71 +--- > drivers/usb/chipidea/ci_hdrc_imx.h | 17 ++- > drivers/usb/chipidea/usbmisc_imx.c | 95 > +- > 3 files changed, 76 insertions(+), 107 deletions(-) > > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c > b/drivers/usb/chipidea/ci_hdrc_imx.c > index 11ed423..b9464ff 100644 > --- a/drivers/usb/chipidea/ci_hdrc_imx.c > +++ b/drivers/usb/chipidea/ci_hdrc_imx.c > @@ -29,57 +29,48 @@ struct ci_hdrc_imx_data { > struct platform_device *ci_pdev; > struct clk *clk; > struct regulator *reg_vbus; > + struct imx_usbmisc_data *usbmisc_data; > }; > > -static const struct usbmisc_ops *usbmisc_ops; > - > /* Common functions shared by usbmisc drivers */ > > -int usbmisc_set_ops(const struct usbmisc_ops *ops) > -{ > - if (usbmisc_ops) > - return -EBUSY; > - > - usbmisc_ops = ops; > - > - return 0; > -} > -EXPORT_SYMBOL_GPL(usbmisc_set_ops); > - > -void usbmisc_unset_ops(const struct usbmisc_ops *ops) > -{ > - usbmisc_ops = NULL; > -} > -EXPORT_SYMBOL_GPL(usbmisc_unset_ops); > - > -int usbmisc_get_init_data(struct device *dev, struct usbmisc_usb_device > *usbdev) > +static struct imx_usbmisc_data *usbmisc_get_init_data(struct device *dev) > { > struct device_node *np = dev->of_node; > struct of_phandle_args args; > + struct imx_usbmisc_data *data; > int ret; > > - usbdev->dev = dev; > + /* > + * In case the fsl,usbmisc property is not present this device doesn't > + * need usbmisc. Return NULL (which is no error here) > + */ > + if (!of_get_property(np, "fsl,usbmisc", NULL)) > + return NULL; > + > + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); > + if (!data) > + return ERR_PTR(-ENOMEM); > > ret = of_parse_phandle_with_args(np, "fsl,usbmisc", "#index-cells", > 0, &args); > if (ret) { > dev_err(dev, "Failed to parse property fsl,usbmisc, errno %d\n", > ret); > - memset(usbdev, 0, sizeof(*usbdev)); > - return ret; > + return ERR_PTR(ret); > } > - usbdev->index = args.args[0]; > + > + data->index = args.args[0]; > of_node_put(args.np); > > if (of_find_property(np, "disable-over-current", NULL)) > - usbdev->disable_oc = 1; > + data->disable_oc = 1; > > if (of_find_property(np, "external-vbus-divider", NULL)) > - usbdev->evdo = 1; > + data->evdo = 1; > > - return 0; > + return data; > } > -EXPORT_SYMBOL_GPL(usbmisc_get_init_data); > > /* End of common functions shared by usbmisc drivers*/ > > @@ -96,10 +87,6 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev) > struct resource *res; > int ret; > > - if (of_find_property(pdev->dev.of_node, "fsl,usbmisc", NULL) > - && !usbmisc_ops) > - return -EPROBE_DEFER; > - > data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); > if (!data) { > dev_err(&pdev->dev, "Failed to allocate ci_hdrc-imx data!\n"); > @@ -112,6 +99,10 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev) > return -ENOENT; > } > > + data->usbmisc_data = usbmisc_get_init_data(&pdev->dev); > + if (IS_ERR(data->usbmisc_data)) > + return PTR_ERR(data->usbmisc_data); > + > data->clk = devm_clk_get(&pdev->dev, NULL); > if (IS_ERR(data->clk)) { > dev_err(&pdev->dev, > @@ -159,11 +150,11 @@ static int ci_hdrc_imx_probe(struct platform_device > *pdev) > if (!pdev->dev.coherent_dma_mask) > pdev->dev.coherent_dma_mask = D
Re: [PATCH] usb: gadget: at91_udc: add usb_clk for transition to common clk framework
On 01/08/2013 08:18, Boris BREZILLON : The AT91 PMC (Power Management Controller) provides an USB clock used by USB Full Speed host (ohci) and USB Full Speed device (udc). The usb drivers (ohci and udc) must configure this clock to 48Mhz. This configuration was formely done in mach-at91/clock.c, but this implementation will be removed when moving to common clk framework. This patch adds support for usb clock retrieval and configuration, and is backward compatible with the current at91 clk implementation (if usb clk is not found, it does not configure/enable it). Signed-off-by: Boris BREZILLON Acked-by: Nicolas Ferre --- drivers/usb/gadget/at91_udc.c | 17 - drivers/usb/gadget/at91_udc.h |2 +- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/drivers/usb/gadget/at91_udc.c b/drivers/usb/gadget/at91_udc.c index fce8e4e..ae06585 100644 --- a/drivers/usb/gadget/at91_udc.c +++ b/drivers/usb/gadget/at91_udc.c @@ -870,6 +870,11 @@ static void clk_on(struct at91_udc *udc) if (udc->clocked) return; udc->clocked = 1; + + if (!IS_ERR(udc->uclk)) { + clk_set_rate(udc->uclk, 4800); + clk_prepare_enable(udc->uclk); + } clk_prepare_enable(udc->iclk); clk_prepare_enable(udc->fclk); } @@ -882,6 +887,8 @@ static void clk_off(struct at91_udc *udc) udc->gadget.speed = USB_SPEED_UNKNOWN; clk_disable_unprepare(udc->fclk); clk_disable_unprepare(udc->iclk); + if (!IS_ERR(udc->uclk)) + clk_disable_unprepare(udc->uclk); } /* @@ -1774,10 +1781,10 @@ static int at91udc_probe(struct platform_device *pdev) /* get interface and function clocks */ udc->iclk = clk_get(dev, "udc_clk"); udc->fclk = clk_get(dev, "udpck"); + udc->uclk = clk_get(dev, "usb_clk"); if (IS_ERR(udc->iclk) || IS_ERR(udc->fclk)) { DBG("clocks missing\n"); retval = -ENODEV; - /* NOTE: we "know" here that refcounts on these are NOPs */ goto fail1; } @@ -1851,6 +1858,12 @@ fail3: fail2: free_irq(udc->udp_irq, udc); fail1: + if (!IS_ERR(udc->uclk)) + clk_put(udc->uclk); + if (!IS_ERR(udc->fclk)) + clk_put(udc->fclk); + if (!IS_ERR(udc->iclk)) + clk_put(udc->iclk); iounmap(udc->udp_baseaddr); fail0a: if (cpu_is_at91rm9200()) @@ -1894,6 +1907,8 @@ static int __exit at91udc_remove(struct platform_device *pdev) clk_put(udc->iclk); clk_put(udc->fclk); + if (!IS_ERR(udc->uclk)) + clk_put(udc->uclk); return 0; } diff --git a/drivers/usb/gadget/at91_udc.h b/drivers/usb/gadget/at91_udc.h index e647d1c..0175246 100644 --- a/drivers/usb/gadget/at91_udc.h +++ b/drivers/usb/gadget/at91_udc.h @@ -126,7 +126,7 @@ struct at91_udc { unsignedactive_suspend:1; u8 addr; struct at91_udc_databoard; - struct clk *iclk, *fclk; + struct clk *iclk, *fclk, *uclk; struct platform_device *pdev; struct proc_dir_entry *pde; void __iomem*udp_baseaddr; -- 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: [PATCH v2] USB: ohci-at91: add usb_clk for transition to common clk framework
On 01/08/2013 19:09, Boris BREZILLON : The AT91 PMC (Power Management Controller) provides an USB clock used by USB Full Speed host (ohci) and USB Full Speed device (udc). The usb drivers (ohci and udc) must configure this clock to 48Mhz. This configuration was formely done in mach-at91/clock.c, but this implementation will be removed when moving to common clk framework. This patch adds support for usb clock retrieval and configuration, and is backward compatible with the current at91 clk implementation (if usb clk is not found, it does not configure/enable it). Changes since v1: - use IS_ENABLED(CONFIG_COMMON_CLK) to isolate new at91 clk support Signed-off-by: Boris BREZILLON Well, for this v2 as well: Acked-by: Nicolas Ferre (even if it is too late) Bye, --- drivers/usb/host/ohci-at91.c | 23 +-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c index 9677f68..db60048 100644 --- a/drivers/usb/host/ohci-at91.c +++ b/drivers/usb/host/ohci-at91.c @@ -31,8 +31,8 @@ #define at91_for_each_port(index) \ for ((index) = 0; (index) < AT91_MAX_USBH_PORTS; (index)++) -/* interface and function clocks; sometimes also an AHB clock */ -static struct clk *iclk, *fclk, *hclk; +/* interface, function and usb clocks; sometimes also an AHB clock */ +static struct clk *iclk, *fclk, *uclk, *hclk; static int clocked; extern int usb_disabled(void); @@ -41,6 +41,10 @@ extern int usb_disabled(void); static void at91_start_clock(void) { + if (IS_ENABLED(CONFIG_COMMON_CLK)) { + clk_set_rate(uclk, 4800); + clk_prepare_enable(uclk); + } clk_prepare_enable(hclk); clk_prepare_enable(iclk); clk_prepare_enable(fclk); @@ -52,6 +56,8 @@ static void at91_stop_clock(void) clk_disable_unprepare(fclk); clk_disable_unprepare(iclk); clk_disable_unprepare(hclk); + if (IS_ENABLED(CONFIG_COMMON_CLK)) + clk_disable_unprepare(uclk); clocked = 0; } @@ -162,6 +168,14 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver, retval = PTR_ERR(hclk); goto err5; } + if (IS_ENABLED(CONFIG_COMMON_CLK)) { + uclk = clk_get(&pdev->dev, "usb_clk"); + if (IS_ERR(uclk)) { + dev_err(&pdev->dev, "failed to get uclk\n"); + retval = PTR_ERR(uclk); + goto err6; + } + } at91_start_hc(pdev); ohci_hcd_init(hcd_to_ohci(hcd)); @@ -173,6 +187,9 @@ static int usb_hcd_at91_probe(const struct hc_driver *driver, /* Error handling */ at91_stop_hc(pdev); + if (IS_ENABLED(CONFIG_COMMON_CLK)) + clk_put(uclk); + err6: clk_put(hclk); err5: clk_put(fclk); @@ -212,6 +229,8 @@ static void usb_hcd_at91_remove(struct usb_hcd *hcd, release_mem_region(hcd->rsrc_start, hcd->rsrc_len); usb_put_hcd(hcd); + if (IS_ENABLED(CONFIG_COMMON_CLK)) + clk_put(uclk); clk_put(hclk); clk_put(fclk); clk_put(iclk); -- 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: dealing with enclosures that don't handle READ_CAPACITY10
On Mon, 12 Aug 2013, Oliver Neukum wrote: > Hi, > > I got a bug report about an enclosure that mishandles READ_CAPACITY10. > I don't want to introduce yet another quirk. So what about basing this > on USB version? > @@ -211,8 +212,12 @@ static int slave_configure(struct scsi_device *sdev) > /* > * Many devices do not respond properly to READ_CAPACITY_16. > * Tell the SCSI layer to try READ_CAPACITY_10 first. > +* However some USB 3.0 drive enclosures return capacity > +* modulo 2TB > */ > - sdev->try_rc_10_first = 1; > + usb_version = le16_to_cpu(us->pusb_dev->descriptor.bcdUSB); > + if (usb_version < '3' * 0x100) Don't you mean 0x300? '3' * 0x100 is really equal to 0x3300. > + sdev->try_rc_10_first = 1; This may be a reasonable thing to do, although there's always a risk that it will cause a regression. I have definitely seen one or two reports about drives doing this, but I don't remember whether they claimed to be USB-2 or USB-3. 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: dealing with enclosures that don't handle READ_CAPACITY10
On Mon, 2013-08-12 at 10:36 -0400, Alan Stern wrote: > On Mon, 12 Aug 2013, Oliver Neukum wrote: > > > Hi, > > > > I got a bug report about an enclosure that mishandles READ_CAPACITY10. > > I don't want to introduce yet another quirk. So what about basing this > > on USB version? > > > @@ -211,8 +212,12 @@ static int slave_configure(struct scsi_device *sdev) > > /* > > * Many devices do not respond properly to READ_CAPACITY_16. > > * Tell the SCSI layer to try READ_CAPACITY_10 first. > > +* However some USB 3.0 drive enclosures return capacity > > +* modulo 2TB > > */ > > - sdev->try_rc_10_first = 1; > > + usb_version = le16_to_cpu(us->pusb_dev->descriptor.bcdUSB); > > + if (usb_version < '3' * 0x100) > > Don't you mean 0x300? '3' * 0x100 is really equal to 0x3300. Thanks. > > + sdev->try_rc_10_first = 1; > > This may be a reasonable thing to do, although there's always a risk > that it will cause a regression. I have definitely seen one or two > reports about drives doing this, but I don't remember whether they > claimed to be USB-2 or USB-3. Well, if it is USB2 we may need a quirk in addition. 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: [alsa-devel] Buffer size for ALSA USB PCM audio
On Mon, 12 Aug 2013, Takashi Iwai wrote: > > Here's what I've got. In turns out the predicting the optimum number > > of URBs needed is extremely difficult. I decided it would be better to > > make an overestimate and then to submit URBs as needed, rather than > > keeping all of them active all the time. > > > > I ended up with this patch (untested). It is certainly incomplete > > because it doesn't address endpoints with implicit sync. It also > > suffers from a race between snd_submit_urbs() and deactivate_urbs(): > > an URB may be resubmitted after it has been deactivated. > > What's the reason to clear ep->active_mask at the beginning of > snd_complete_urb()? In order to keep ep->active_mask accurate. snd_complete_urb() might not resubmit the URB. > > (In all > > fairness, I think this race probably exists in the current code too -- > > there are no spinlocks for synchronization.) > > The calls of usb_submit_urb() and usb_unlink_urb() might race indeed. > The current code somehow assumed that the USB accepts such concurrent > calls. The USB API does indeed allow concurrent calls of usb_submit_urb() and usb_unlink_urb(). They are serialized by a spinlock in the host controller driver, and if the usb_unlink_urb() call ends up coming first, it will fail. (Ironically, in the EHCI driver, trying to unlink an isochronous URB doesn't do anything at all. The URB will complete in the usual way, when all its time slots expire.) > deactivate_urbs() just kills the pending urbs and it doesn't > change ep->active_mask bit by itself, and the synchronization is done > in wait_clear_urbs(). Oh, so that's where ep->active_mask gets used. Why call bitmap_weight()? Why not simply see if the mask value is equal to 0? > So, if the concurrent calls of usb_submit_urb() > and usb_unlink_urbs() were allowed, it should work as is (in the worst > case, the complete will be called once again, but then it goes to > exit_clear). I see. Assuming there's always at least one active URB while the endpoint is running, wait_clear_urbs() should work okay. The patch won't violate this assumption, so it's good. 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: [alsa-devel] Buffer size for ALSA USB PCM audio
At Mon, 12 Aug 2013 10:53:36 -0400 (EDT), Alan Stern wrote: > > On Mon, 12 Aug 2013, Takashi Iwai wrote: > > > > Here's what I've got. In turns out the predicting the optimum number > > > of URBs needed is extremely difficult. I decided it would be better to > > > make an overestimate and then to submit URBs as needed, rather than > > > keeping all of them active all the time. > > > > > > I ended up with this patch (untested). It is certainly incomplete > > > because it doesn't address endpoints with implicit sync. It also > > > suffers from a race between snd_submit_urbs() and deactivate_urbs(): > > > an URB may be resubmitted after it has been deactivated. > > > > What's the reason to clear ep->active_mask at the beginning of > > snd_complete_urb()? > > In order to keep ep->active_mask accurate. snd_complete_urb() might > not resubmit the URB. > > > > (In all > > > fairness, I think this race probably exists in the current code too -- > > > there are no spinlocks for synchronization.) > > > > The calls of usb_submit_urb() and usb_unlink_urb() might race indeed. > > The current code somehow assumed that the USB accepts such concurrent > > calls. > > The USB API does indeed allow concurrent calls of usb_submit_urb() and > usb_unlink_urb(). They are serialized by a spinlock in the host > controller driver, and if the usb_unlink_urb() call ends up coming > first, it will fail. > > (Ironically, in the EHCI driver, trying to unlink an isochronous URB > doesn't do anything at all. The URB will complete in the usual way, > when all its time slots expire.) > > > deactivate_urbs() just kills the pending urbs and it doesn't > > change ep->active_mask bit by itself, and the synchronization is done > > in wait_clear_urbs(). > > Oh, so that's where ep->active_mask gets used. Why call > bitmap_weight()? Why not simply see if the mask value is equal to 0? Yeah, it can be so. Though, the number of pending urbs is printed in the error message, thus bitmap_weight() is needed there instead. > > So, if the concurrent calls of usb_submit_urb() > > and usb_unlink_urbs() were allowed, it should work as is (in the worst > > case, the complete will be called once again, but then it goes to > > exit_clear). > > I see. Assuming there's always at least one active URB while the > endpoint is running, wait_clear_urbs() should work okay. The patch > won't violate this assumption, so it's good. OK. So... Clemens, Daniel, Eldad, could you guys review the latest version of Alan's patch? I'd love to sort this out for 3.12. thanks, Takashi -- 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: [3.8-rc3 -> 3.8-rc4 regression] Re: [PATCH] module, async: async_synchronize_full() on module init iff async is used
Hello, Jonathan. On Mon, Aug 12, 2013 at 12:04:11AM -0700, Jonathan Nieder wrote: > My laptop fails to boot[1] with the message 'Volume group "data" not > found'. Bisects to v3.8-rc4~17 (the above commit). Reverting that > commit on top of current "master" (d92581fcad18, 2013-08-10) produces > a working kernel. dmesg output from that working kernel attached. > More details, including .config, at [2]. > > Any ideas for tracking this down? Which initrd / boot script are you using? It looks like lvm assemble scripts are running before sdX are detected leading to volume assembly failure. Before the patch, any module loading would end up synchronizing async probes but after the patch modprobe invocations which don't schedule them won't be. Does your boot script happen to run multiple modprobes in parallel and proceed to configure lvm without waiting for modprobes of libata drivers to finish? Thanks. -- tejun -- 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: Non-enumerable devices on USB and other enumerable buses
On Sun, Aug 11, 2013 at 11:08:37PM +0100, Grant Likely wrote: > full enumerating like that with either ACPI or FDT, but we could allow > for sparse population of devices when something is fixed like a > soldered down USB hub or USB Ethernet MAC. I agree, there's no point in listing things that can be done automatically - it's just introducing potential for error. > To make it work would probably require a hook in the USB enumeration > path to look for matching nodes in DT/ACPI and attach it to the struct > device. Yes, that was where I was heading too. Have a mechanism for matching up hotplugged devices with pre-registered ones which appear from firmware or wherever if their IDs match. It'd need some mechanism for drivers to opt into being bound to devices that aren't physically there I think so that normal drivers for enumerated devices don't get confused. signature.asc Description: Digital signature
[PATCH] wusbcore: clean up list locking in urb enqueue
wa_urb_enqueue_run locks and unlocks its list lock as it traverses the list of queued transfers. This was done to prevent deadlocking due to acquiring locks in reverse order in different places. The problem is that releasing the lock during the list traversal could allow the dequeue routine to corrupt the list while it is being iterated over. This patch moves all list entries to a temp list while holding the list lock, then traverses the temp list with no lock held. Signed-off-by: Thomas Pugliese diff --git a/drivers/usb/wusbcore/wa-xfer.c b/drivers/usb/wusbcore/wa-xfer.c index d3493ca..cef940f 100644 --- a/drivers/usb/wusbcore/wa-xfer.c +++ b/drivers/usb/wusbcore/wa-xfer.c @@ -928,7 +928,7 @@ static void wa_xfer_delayed_run(struct wa_rpipe *rpipe) spin_lock_irqsave(&rpipe->seg_lock, flags); while (atomic_read(&rpipe->segs_available) > 0 && !list_empty(&rpipe->seg_list)) { - seg = list_entry(rpipe->seg_list.next, struct wa_seg, + seg = list_first_entry(&(rpipe->seg_list), struct wa_seg, list_node); list_del(&seg->list_node); xfer = seg->xfer; @@ -1093,30 +1093,35 @@ error_xfer_submit: * * We need to be careful here, as dequeue() could be called in the * middle. That's why we do the whole thing under the - * wa->xfer_list_lock. If dequeue() jumps in, it first locks urb->lock + * wa->xfer_list_lock. If dequeue() jumps in, it first locks xfer->lock * and then checks the list -- so as we would be acquiring in inverse - * order, we just drop the lock once we have the xfer and reacquire it - * later. + * order, we move the delayed list to a separate list while locked and then + * submit them without the list lock held. */ void wa_urb_enqueue_run(struct work_struct *ws) { struct wahc *wa = container_of(ws, struct wahc, xfer_work); struct wa_xfer *xfer, *next; struct urb *urb; + LIST_HEAD(tmp_list); + /* Create a copy of the wa->xfer_delayed_list while holding the lock */ spin_lock_irq(&wa->xfer_list_lock); - list_for_each_entry_safe(xfer, next, &wa->xfer_delayed_list, -list_node) { + list_cut_position(&tmp_list, &wa->xfer_delayed_list, + wa->xfer_delayed_list.prev); + spin_unlock_irq(&wa->xfer_list_lock); + + /* +* enqueue from temp list without list lock held since wa_urb_enqueue_b +* can take xfer->lock as well as lock mutexes. +*/ + list_for_each_entry_safe(xfer, next, &tmp_list, list_node) { list_del_init(&xfer->list_node); - spin_unlock_irq(&wa->xfer_list_lock); urb = xfer->urb; wa_urb_enqueue_b(xfer); usb_put_urb(urb); /* taken when queuing */ - - spin_lock_irq(&wa->xfer_list_lock); } - spin_unlock_irq(&wa->xfer_list_lock); } EXPORT_SYMBOL_GPL(wa_urb_enqueue_run); -- 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: Disable runtime PM suspend for quirky controllers.
Hi Sarah, I will resubmit the patch with these changes shortly. On Fri, Aug 9, 2013 at 10:22 AM, Sarah Sharp wrote: > Hi Shawn, > > I noticed that the ChromeOS kernel tree is still using this particular > patch, and thought it was probably time to revisit it. > > On Sat, May 25, 2013 at 09:57:57AM -0700, Shawn Nematbakhsh wrote: >> Hi Sarah and Alan, >> >> Thanks for the comments. I will make the following revisions: >> >> 1. Call pm_runtime_get_noresume only when the first device is connected, >> and call pm_runtime_put when the last device is disconnected. >> 2. Wrap the calls in an ifdef CONFIG_USB_DEFAULT_PERSIST. >> 3. Make sure that all pm_runtime_get_noresume calls have a corresponding >> pm_runtime_put somewhere (originally the intent was to disable runtime >> suspend "forever", but no longer). >> >> In principle, would the patch be acceptable with these revisions? > > The thread petered off with all other options turning out to be > dead-ends, so yes, if you made those changes, you could get that patch > upstream. I would like the ChromeOS kernel to be as close to upstream > as possible, so please resubmit this patch with those changes. > > Thanks, > Sarah Sharp > >> >> On Sat, May 25, 2013 at 7:11 AM, Alan Stern wrote: >> >> > On Fri, 24 May 2013, Sarah Sharp wrote: >> > >> > > On Fri, May 24, 2013 at 11:12:57AM -0700, Shawn Nematbakhsh wrote: >> > > > If a USB controller with XHCI_RESET_ON_RESUME goes to runtime suspend, >> > > > a reset will be performed upon runtime resume. Any previously suspended >> > > > devices attached to the controller will be re-enumerated at this time. >> > > > This will cause problems, for example, if an open system call on the >> > > > device triggered the resume (the open call will fail). >> > > >> > > That's ugly, but disabling runtime PM is going to have a big impact on >> > > the power consumption of those systems. >> > > >> > > Alan, do you think this is really the right thing to be doing here? It >> > > feels like userspace should just be able to deal with devices >> > > disconnecting on resume. After all, there are lots of USB devices that >> > > can't handle USB device suspend at all. >> > >> > This is a complicated issue. It depends on the runtime PM settings for >> > both the device and the host controller. >> > >> > As just one aspect, consider the fact that if it wants to, userspace >> > can already prevent the controller from going into runtime suspend. >> > Always preventing this at the kernel level, even when no devices are >> > plugged in, does seem too heavy-handed. >> > >> > > Shouldn't userspace just disable runtime PM for the USB device classes >> > > that don't have a reset resume callback? >> > >> > That's not so easy, because the kernel changes over time. Userspace >> > has no general way to tell which drivers have reset-resume support. >> > >> > > > Note that this change is only relevant when persist_enabled is not set >> > > > for USB devices. >> > > >> > > Could we at least wrap the call in an ifdef CONFIG_USB_DEFAULT_PERSIST? >> > > That way if people have USB persist turned off in their configuration, >> > > their host will still be able to suspend. >> > >> > Not just that; the patch is incorrect on the face of it... >> > >> > > > @@ -4687,6 +4687,12 @@ int xhci_gen_setup(struct usb_hcd *hcd, >> > xhci_get_quirks_t get_quirks) >> > > > >> > > > get_quirks(dev, xhci); >> > > > >> > > > + /* If we are resetting upon resume, we must disable runtime PM. >> > > > +* Otherwise, an open() syscall to a device on our runtime >> > suspended >> > > > +* controller will trigger controller reset and device >> > re-enumeration */ >> > > > + if (xhci->quirks & XHCI_RESET_ON_RESUME) >> > > > + pm_runtime_get_noresume(dev); >> > > > + >> > >> > It adds a pm_runtime_get call with no corresponding pm_runtime_put. >> > >> > 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: [alsa-devel] Buffer size for ALSA USB PCM audio
On Mon, 12 Aug 2013, Takashi Iwai wrote: > So... Clemens, Daniel, Eldad, could you guys review the latest version > of Alan's patch? I'd love to sort this out for 3.12. Here's a revised version of the patch (still untested). The difference is that this version tries always to keep a period's worth of bytes in the USB hardware queue. This will provide better protection against underruns when the period is larger than the queue's minimum requirement. Alan Stern sound/usb/card.h |9 ++ sound/usb/endpoint.c | 200 ++- sound/usb/pcm.c |4 - 3 files changed, 132 insertions(+), 81 deletions(-) Index: usb-3.11/sound/usb/card.h === --- usb-3.11.orig/sound/usb/card.h +++ usb-3.11/sound/usb/card.h @@ -4,7 +4,7 @@ #define MAX_NR_RATES 1024 #define MAX_PACKS 20 #define MAX_PACKS_HS (MAX_PACKS * 8) /* in high speed mode */ -#define MAX_URBS 8 +#define MAX_URBS 11 #define SYNC_URBS 4 /* always four urbs for sync */ #define MAX_QUEUE 24 /* try not to exceed this queue length, in ms */ @@ -43,6 +43,8 @@ struct snd_urb_ctx { struct snd_usb_endpoint *ep; int index; /* index for urb array */ int packets;/* number of packets per urb */ + int data_packets; /* current number of data packets */ + int data_frames;/* current number of data frames */ int packet_size[MAX_PACKS_HS]; /* size of packets for next submission */ struct list_head ready_list; }; @@ -99,6 +101,11 @@ struct snd_usb_endpoint { int skip_packets; /* quirks for devices to ignore the first n packets in a stream */ + unsigned int min_queued_packs; /* USB hardware queue requirement */ + unsigned int queued_packs; /* number of packets on the queue */ + unsigned int queued_urbs; /* number of URBs on the queue */ + unsigned int queued_frames; /* number of data frames on the queue */ + int next_urb; /* next to submit */ spinlock_t lock; struct list_head list; }; Index: usb-3.11/sound/usb/pcm.c === --- usb-3.11.orig/sound/usb/pcm.c +++ usb-3.11/sound/usb/pcm.c @@ -1328,7 +1328,7 @@ static void prepare_playback_urb(struct stride = runtime->frame_bits >> 3; frames = 0; - urb->number_of_packets = 0; + ctx->data_packets = urb->number_of_packets = 0; spin_lock_irqsave(&subs->lock, flags); for (i = 0; i < ctx->packets; i++) { if (ctx->packet_size[i]) @@ -1341,6 +1341,7 @@ static void prepare_playback_urb(struct urb->iso_frame_desc[i].length = counts * ep->stride; frames += counts; urb->number_of_packets++; + ctx->data_packets++; subs->transfer_done += counts; if (subs->transfer_done >= runtime->period_size) { subs->transfer_done -= runtime->period_size; @@ -1370,6 +1371,7 @@ static void prepare_playback_urb(struct !snd_usb_endpoint_implicit_feedback_sink(subs->data_endpoint)) /* finish at the period boundary */ break; } + ctx->data_frames = frames; bytes = frames * ep->stride; if (unlikely(subs->pcm_format == SNDRV_PCM_FORMAT_DSD_U16_LE && Index: usb-3.11/sound/usb/endpoint.c === --- usb-3.11.orig/sound/usb/endpoint.c +++ usb-3.11/sound/usb/endpoint.c @@ -203,6 +203,7 @@ static void prepare_outbound_urb(struct } else { /* no data provider, so send silence */ unsigned int offs = 0; + for (i = 0; i < ctx->packets; ++i) { int counts; @@ -217,6 +218,8 @@ static void prepare_outbound_urb(struct } urb->number_of_packets = ctx->packets; + ctx->data_packets = ctx->packets; + ctx->data_frames = offs; urb->transfer_buffer_length = offs * ep->stride; memset(urb->transfer_buffer, ep->silence_value, offs * ep->stride); @@ -273,6 +276,7 @@ static inline void prepare_inbound_urb(s urb->transfer_buffer_length = offs; urb->number_of_packets = urb_ctx->packets; + urb_ctx->data_packets = urb_ctx->packets; break; case SND_USB_ENDPOINT_TYPE_SYNC: @@ -341,6 +345,55 @@ static void queue_pending_output_urbs(st } } +/** + * snd_submit_urbs: Submit data URBs for endpoints without implicit feedback + * + * @ep: The endpoint to use
[PATCH 1/1] USB-Serial: Fix error handling of usb_wwan
This fixes an issue where the bulk-in urb used for incoming data transfer is not resubmitted if the packet recieved contains an error status. This results in the driver locking until the port is closed and re-opened. Tested on a custom board with a Cinterion GSM module. Signed-off-by: Matt Burtch --- drivers/usb/serial/usb_wwan.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c index 8257d30..9eefc33 100644 --- a/drivers/usb/serial/usb_wwan.c +++ b/drivers/usb/serial/usb_wwan.c @@ -291,18 +291,18 @@ static void usb_wwan_indat_callback(struct urb *urb) tty_flip_buffer_push(&port->port); } else dev_dbg(dev, "%s: empty read urb received\n", __func__); - - /* Resubmit urb so we continue receiving */ - err = usb_submit_urb(urb, GFP_ATOMIC); - if (err) { - if (err != -EPERM) { - dev_err(dev, "%s: resubmit read urb failed. (%d)\n", __func__, err); - /* busy also in error unless we are killed */ - usb_mark_last_busy(port->serial->dev); - } - } else { + } + /* Resubmit urb so we continue receiving */ + err = usb_submit_urb(urb, GFP_ATOMIC); + if (err) { + if (err != -EPERM) { + dev_err(dev, "%s: resubmit read urb failed. (%d)\n", + __func__, err); + /* busy also in error unless we are killed */ usb_mark_last_busy(port->serial->dev); } + } else { + usb_mark_last_busy(port->serial->dev); } } -- 1.7.9.5 -- 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/1] USB-Serial: Fix error handling of usb_wwan
On Mon, Aug 12, 2013 at 10:11:39AM -0700, Matt Burtch wrote: > This fixes an issue where the bulk-in urb used for incoming data transfer > is not resubmitted if the packet recieved contains an error status. This > results in the driver locking until the port is closed and re-opened. > > Tested on a custom board with a Cinterion GSM module. > > Signed-off-by: Matt Burtch Ah, nice catch, thanks for this, I'll also mark it for the stable tree as it's something that those kernels would want to pick up. 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/PATCH 2/2] usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET
Hi, On Fri, Aug 09, 2013 at 11:04:48AM -0400, Alan Stern wrote: > > > > heh, it doesn't need to be entirely in the core. Core could have the > > > > generic calls and HCDs could implement some callbacks, but I think quite > > > > a bit of the code will be similar if we implement the same thing on all > > > > HCDs. > > > > > > What generic calls and callbacks would you suggest? I assume you want > > > enough to cover not just this one test but the entire USB-CV suite. > > > > maybe a single callback for supporting 'testmodes' ? which receives the > > test mode as argument ? > > I don't have a clear picture of how you would apply such an approach to > this case. There would have to be a way to tell the HCD to insert a > 15-second delay between the Setup and Data stages of a particular > control URB. How would you do that? Whatever method you choose, each test is started after enumerating a known Vid/Pid pair. Based on that, you *know* which test to run. > implementing it in every HCD would be a huge amount of work. sure, we can support different HCDs with time, but once SINGLE_STEP is implemented in e.g. EHCI, it should be simple to port it to OHCI/xHCI/MUSB/etc. > What other test modes would you want to support? anything that USB[23]0CV supports today. There are even link layer tests for USB3 and a bunch of others. This SINGLE_STEP_SET_FEATURE is but one of a large(-ish) test suite which needs to be supported. > Is it worth adding this support to the standard host controller > drivers, or should there be a special version (a Kconfig option like > CONFIG_RCU_TORTURE_TEST) to enable it? Putting a lot of testing code > in distribution kernels where it will never be used seems like a big > waste. right, I think it should be hidden by Kconfig, not arguing against that. -- balbi signature.asc Description: Digital signature
Re: DWC3 role switch cause IRQ request failed
On Fri, Aug 09, 2013 at 03:16:20PM +, Wang, Yu Y wrote: > > On Fri, Aug 09, 2013 at 01:34:09PM +, Wang, Yu Y wrote: > > > > On Wed, Aug 07, 2013 at 12:03:34PM +, Wang, Yu Y wrote: > > > > > Hi Balbi, > > > > > > > > > > Because dwc3 driver request_threaded_irq with flags IRQF_ONESHOT > > > > > and IRQF_SHARED. > > > > > But xHCI driver will not set IRQF_ONESHOT. Then will met IRQ > > > > > request failed if use same IRQ number. > > > > > > > > > > > > > > > <4>[1.019248] Call Trace: > > > > > <4>[1.019270] [] dump_stack+0x16/0x18 > > > > > <4>[1.019292] [] __schedule_bug+0x65/0x77 > > > > > <4>[1.019313] [] __schedule+0x7ad/0x830 > > > > > <4>[1.019335] [] ? rest_init+0xc0/0xc0 > > > > > <4>[1.019358] [] ? printk_address+0x32/0x40 > > > > > <4>[1.019380] [] ? __module_text_address+0x10/0x60 > > > > > <4>[1.019402] [] ? is_module_text_address+0x2b/0x50 > > > > > <4>[1.019424] [] ? __kernel_text_address+0x4f/0x70 > > > > > <4>[1.019444] [] schedule+0x23/0x60 > > > > > <4>[1.019466] [] schedule_timeout+0x125/0x1f0 > > > > > <4>[1.019488] [] ? wait_for_completion+0x2b/0xe0 > > > > > <4>[1.019509] [] ? wait_for_completion+0x98/0xe0 > > > > > <4>[1.019530] [] wait_for_completion+0x9f/0xe0 > > > > > <4>[1.019551] [] ? try_to_wake_up+0x280/0x280 > > > > > <4>[1.019572] [] kthread_stop+0x41/0x100 > > > > > <4>[1.019595] [] __setup_irq+0xd6/0x460 > > > > > <4>[1.019617] [] ? > > > > dwc3_gadget_set_selfpowered+0x60/0x60 > > > > > <4>[1.019639] [] request_threaded_irq+0xa2/0x120 > > > > > <4>[1.019660] [] ? > > > > dwc3_gadget_reset_interrupt+0x1a0/0x1a0 > > > > > <4>[1.019681] [] dwc3_gadget_start+0x1a4/0x1f0 > > > > > <4>[1.019703] [] udc_bind_to_driver+0x57/0x100 > > > > > <4>[1.019724] [] usb_gadget_probe_driver+0xa1/0xe0 > > > > > <4>[1.019745] [] ? device_create_file+0x38/0xb0 > > > > > <4>[1.019766] [] usb_composite_probe+0x6f/0x90 > > > > > <4>[1.019788] [] init+0x147/0x19f > > > > > <4>[1.019809] [] ? acmmod_init+0xf/0xf > > > > > <4>[1.019829] [] do_one_initcall+0xba/0x170 > > > > > <4>[1.019853] [] kernel_init_freeable+0x119/0x1b8 > > > > > <4>[1.019875] [] ? do_early_param+0x7a/0x7a > > > > > <4>[1.019896] [] kernel_init+0x10/0xd0 > > > > > <4>[1.019917] [] ret_from_kernel_thread+0x1b/0x28 > > > > > <4>[1.019937] [] ? rest_init+0xc0/0xc0 > > > > > <3>[1.020082] dwc3 dwc3.0.auto: failed to request irq #34 --> -16 > > > > > > > > > > This is one known issue or new issue? > > > > > > > > I have already removed IRQF_ONESHOT, it'll be on v3.12 > > > > > > > > > > [Yu:] Thanks balbi. Can you please share the patch link? > > > > http://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?h=next&id=e8 > > adfc30ff9282a728fd8b666b6418308164c415 > > > > as usual, it's in my 'next' branch. you really need to fix your mailer btw, > > I got > > many copies of this same message. > > [Yu:] Thanks balbi. Don't know the reason. I send the email to you, and always > get send failed response. So I tried more times. Sorry about that. > > BTW, Where can I find your git server link? I want to clone it to get latest > code base. on git.kernel.org, in the link above, click on 'summary' and scroll down to the end of the page. -- balbi signature.asc Description: Digital signature
Re: [RFC PATCH v2 1/3] usb: dwc3: msm: Add device tree binding information
On Fri, Aug 09, 2013 at 10:31:58AM -0500, Kumar Gala wrote: > > On Aug 9, 2013, at 4:53 AM, Ivan T. Ivanov wrote: > > > From: "Ivan T. Ivanov" > > > > MSM USB3.0 core wrapper consist of USB3.0 IP (SNPS) > > probably good to spell out Synopsys rather than SNPS Synopsys (the company) has always used snps in their bindings though. > > +Required properities : > > +- compatible : sould be "qcom,dwc3-hsphy"; > > +- reg : offset and length of the register set in the memory map > > +- clocks : phandles to clock instances of the device tree nodes > > +- clock-names : > > + "xo" : External reference clock 19 MHz > > + "sleep_a_clk" : Sleep clock, used when USB3 core goes into low > > + power mode (U3). > > +-supply : phandle to the regulator device tree node > > +Required "supply-name" are: > > + "v1p8" : 1.8v supply for HSPHY > > + "v3p3" : 3.3v supply for HSPHY > > + "vbus" : vbus supply for host mode > > + "vddcx" : vdd supply for HS-PHY digital circuit operation I believe these regulators belong to the PHY, not DWC3. Please write a PHY driver. > > +Required properities : > > +- compatible : sould be "qcom,dwc3-ssphy"; > > +- reg : offset and length of the register set in the memory map > > +- clocks : phandles to clock instances of the device tree nodes > > +- clock-names : > > + "xo" : External reference clock 19 MHz > > + "ref_clk" : Reference clock - used in host mode. > > +-supply : phandle to the regulator device tree node > > +Required "supply-name" are: > > + "v1p8" : 1.8v supply for SS-PHY > > + "vddcx" : vdd supply for SS-PHY digital circuit operation likewise, these regulators should be moved to the PHY driver. > > +Required properties : > > +- compatible : should be "qcom,dwc3" > > +- reg : offset and length of the register set in the memory map > > + offset and length of the TCSR register for routing USB > > + signals to either picoPHY0 or picoPHY1. > > +- clocks : phandles to clock instances of the device tree nodes > > +- clock-names : > > + "core_clk" : Master/Core clock, have to be >= 125 MHz for SS > > + operation and >= 60MHz for HS operation > > + "iface_clk" : System bus AXI clock > > + "sleep_clk" : Sleep clock, used when USB3 core goes into low > > + power mode (U3). > > + "utmi_clk" : Generated by HS-PHY. Used to clock the low power > > + parts of thr HS Link layer. > > + > > +Optional properties : > > +- gdsc-supply : phandle to the globally distributed switch controller > > + regulator node to the USB controller. > > + > > +Sub nodes: > > +- Sub node for "DWC3 USB3 controller". > > + This sub node is required property for device node. The properties > > + of this subnode are specified in dwc3.txt. > > Is tx-fifo-resize required for qcom,dwc3? if so we should call that > out in the binding. AFAICT that's only needed for OMAP5 ES1.0. Unless Qualcomm also screwed up default TX FIFO sizes :-p -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb: dwc3: core: clarify usb-phy array binding
On Fri, Aug 09, 2013 at 01:42:15PM -0500, Kumar Gala wrote: > > On Aug 9, 2013, at 11:28 AM, Mark Rutland wrote: > > > On Fri, Aug 09, 2013 at 04:40:32PM +0100, Kumar Gala wrote: > >> The binding spec wasn't clear that the order of the phandles in the > >> usb-phy array has meaning. Clarify this point in the binding that > >> it should be . > >> > >> Signed-off-by: Kumar Gala > >> --- > >> Documentation/devicetree/bindings/usb/dwc3.txt | 4 +++- > >> 1 file changed, 3 insertions(+), 1 deletion(-) > >> > >> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt > >> b/Documentation/devicetree/bindings/usb/dwc3.txt > >> index 7a95c65..8a9770b 100644 > >> --- a/Documentation/devicetree/bindings/usb/dwc3.txt > >> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt > >> @@ -6,7 +6,9 @@ Required properties: > >> - compatible: must be "synopsys,dwc3" > >> - reg : Address and length of the register set for the device > >> - interrupts: Interrupts used by the dwc3 controller. > >> - - usb-phy : array of phandle for the PHY device > >> + - usb-phy : array of phandle for the PHY device. The first element > >> + in the array is expected to be a handle to the USB2/HS PHY and > >> + the second element is expected to be a handle to the USB3/SS PHY > > > > Just to check - it's not valid to have a USB3 phy without a USB2 phy? > > Don't know, hopefully Felipe will chime in on that. that'd be a really non-standard implementation. Per-spec, USB3 is *always* backwards compatible with USB2. -- balbi signature.asc Description: Digital signature
Re: Non-enumerable devices on USB and other enumerable buses
On 08/12/2013 05:07 AM, Mark Rutland wrote: > [Adding Olof] > > On Mon, Aug 12, 2013 at 10:51:36AM +0100, Mark Brown wrote: >> On Sun, Aug 11, 2013 at 09:53:01PM -0400, Alan Stern wrote: >>> On Sun, 11 Aug 2013, Mark Brown wrote: >> One example that's bugging me right now is that on the Insignal Arndale platform there's a USB hub connected to one of the USB ports on the SoC (not as a PHY, it seems we also need the internal PHY running to talk to the device). The hub needs to be "plugged" into the SoC after the SoC USB controller has started with some GPIOs so we need to tell the system that the hub exists and needs to be synchronised with the USB controller. ... > As I understand it, the wifi chip on the Snow Chromebook has a similar > issue -- it hangs off of a probeable SDIO bus, but needs a regulator > poked for it to turn on and become probeable (see > exynos_wifi_bt_set_power in [1]). In this case, I wonder if it makes sense to model the extra requirements as part of the bus/socket/... itself rather than as part of the device that's attached to the bus. It seems quite common for SDIO-based WiFi devices to need a few things: * Regulator * 32KHz clock * Enable GPIO (-> rfkill?) * Perhaps a reset GPIO Physically, these are provided to the socket into which the WiFi device plugs in. Perhaps we should try to explicitly model the socket (which I guess is really the SDIO bus in a way) rather than attaching these new resources to the controller or the device itself. In a similar way, I wonder if the USB case can be considered the same way? This seems less like a good fit since I don't expect the resources are always so similar there, and also there's the case of the bus being potentially behind a few levels of USB hub. And of course it all gets a little more messy when you get board-specific logic that needs setup, rather than something more common across multiple devices. -- 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/composite : Avoid crash with bad gadget drivers
On Fri, Aug 09, 2013 at 09:23:08PM +0300, Philippe De Swert wrote: > Some bad gadget drivers do not check the return status of usb_add_config. fix the gadget driver > Thus they get a not correctly initialized config and when this gadget gets > deactivated the whole kernel crashes. Since on initialization failure cdev > is set to NULL it can be used to detect this problem situation. It can be > argued that the faulty gadget driver should be fixed, but imho it is better > to avoid crashing the kernel and letting the gadget developer know he/she > is making a mistake. And have the developer of said gadget driver then fix > the problem of course. > > Signed-off-by: Philippe De Swert > --- > drivers/usb/gadget/composite.c | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c > index 55f4df6..e019bb5 100644 > --- a/drivers/usb/gadget/composite.c > +++ b/drivers/usb/gadget/composite.c > @@ -848,12 +848,18 @@ void usb_remove_config(struct usb_composite_dev *cdev, > { > unsigned long flags; > > + if (config->cdev == NULL) { > + pr_warn("Calling usb_remove_config without a matching > usb_add_config!\n"); > + goto end; > + } I would take a WARN() only, but let the crash happen and fix the gadget driver. -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb: gadget: at91_udc: add usb_clk for transition to common clk framework
On Mon, Aug 12, 2013 at 03:52:01PM +0200, Nicolas Ferre wrote: > On 01/08/2013 08:18, Boris BREZILLON : > >The AT91 PMC (Power Management Controller) provides an USB clock used by > >USB Full Speed host (ohci) and USB Full Speed device (udc). > >The usb drivers (ohci and udc) must configure this clock to 48Mhz. > >This configuration was formely done in mach-at91/clock.c, but this > >implementation will be removed when moving to common clk framework. > > > >This patch adds support for usb clock retrieval and configuration, and is > >backward compatible with the current at91 clk implementation (if usb clk > >is not found, it does not configure/enable it). > > > >Signed-off-by: Boris BREZILLON > > Acked-by: Nicolas Ferre I don't have the original, where is it ? Was it not Cced to linux-usb ? Can someone resend with Nicolas' Acked-by in place ? -- balbi signature.asc Description: Digital signature
[PATCH v3] usb: rh_call_control tbuf overflow fix
rh_call_control() contains a buffer, tbuf, which it uses to hold USB descriptors. These discriptors are eventually copied into the transfer_buffer in the URB. The buffer in the URB is dynamically defined and is always large enough to hold the amount of data it requests. tbuf is currently statically allocated on the stack with a size of 15 bytes, regardless of the size specified in the URB. This patch dynamically allocates tbuf, and ensures that tbuf is at least as big as the buffer in the URB. If an hcd attempts to write a descriptor containing more than 15 bytes ( such as the Standard BOS Descriptor for hubs, defined in the USB3.0 Spec, section 10.13.1 ) the write would overflow the buffer and corrupt the stack. This patch addresses this behavior. Acked-by: Alan Stern Signed-off-by: Sean O. Stalley --- drivers/usb/core/hcd.c | 24 +--- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 014dc99..88d6e6e 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -464,17 +464,13 @@ static int rh_call_control (struct usb_hcd *hcd, struct urb *urb) struct usb_ctrlrequest *cmd; u16 typeReq, wValue, wIndex, wLength; u8 *ubuf = urb->transfer_buffer; - /* -* tbuf should be as big as the BOS descriptor and -* the USB hub descriptor. -*/ - u8 tbuf[USB_DT_BOS_SIZE + USB_DT_USB_SS_CAP_SIZE] - __attribute__((aligned(4))); - const u8*bufp = tbuf; unsignedlen = 0; int status; u8 patch_wakeup = 0; u8 patch_protocol = 0; + u16 tbuf_size; + u8 *tbuf; + const u8*bufp; might_sleep(); @@ -494,6 +490,18 @@ static int rh_call_control (struct usb_hcd *hcd, struct urb *urb) if (wLength > urb->transfer_buffer_length) goto error; + /* +* tbuf should be at least as big as the +* USB hub descriptor. +*/ + tbuf_size = max_t(u16, sizeof(struct usb_hub_descriptor), wLength); + tbuf = kzalloc(tbuf_size, GFP_KERNEL); + if (!tbuf) + return -ENOMEM; + + bufp = tbuf; + + urb->actual_length = 0; switch (typeReq) { @@ -691,6 +699,8 @@ error: bDeviceProtocol = USB_HUB_PR_HS_SINGLE_TT; } + kfree(tbuf); + /* any errors get returned through the urb completion */ spin_lock_irq(&hcd_root_hub_lock); usb_hcd_unlink_urb_from_ep(hcd, urb); -- 1.8.1.2 -- 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 11/11] usb: misc: usb3503: Support operation with no I2C control
Reviewed-by: Dongjin Kim I like this patch series, I dropped my changes to support non-i2c based operation. On Fri, Aug 9, 2013 at 7:41 PM, Mark Brown wrote: > From: Mark Brown > > Refactor so that register writes for configuration are only performed if > the device has a regmap provided and also register as a platform driver. > This allows the driver to be used to manage GPIO based control of the > device. > > Signed-off-by: Mark Brown > Cc: devicet...@vger.kernel.org > --- > Documentation/devicetree/bindings/usb/usb3503.txt | 3 +- > drivers/usb/misc/usb3503.c| 93 > ++- > 2 files changed, 78 insertions(+), 18 deletions(-) > > diff --git a/Documentation/devicetree/bindings/usb/usb3503.txt > b/Documentation/devicetree/bindings/usb/usb3503.txt > index 44a03f3..a018da4 100644 > --- a/Documentation/devicetree/bindings/usb/usb3503.txt > +++ b/Documentation/devicetree/bindings/usb/usb3503.txt > @@ -2,9 +2,10 @@ SMSC USB3503 High-Speed Hub Controller > > Required properties: > - compatible: Should be "smsc,usb3503" or "smsc,usb3503a". > -- reg: Specifies the i2c slave address, it should be 0x08. > > Optional properties: > +- reg: Specifies the i2c slave address, it is required and should be 0x08 > + if I2C is used. > - connect-gpios: Should specify GPIO for connect. > - disabled-ports: Should specify the ports unused. > '1' or '2' or '3' are availe for this property to describe the port > diff --git a/drivers/usb/misc/usb3503.c b/drivers/usb/misc/usb3503.c > index da45ed9..a31641e 100644 > --- a/drivers/usb/misc/usb3503.c > +++ b/drivers/usb/misc/usb3503.c > @@ -78,22 +78,21 @@ static int usb3503_reset(struct usb3503 *hub, int state) > return 0; > } > > -static int usb3503_switch_mode(struct usb3503 *hub, enum usb3503_mode mode) > +static int usb3503_connect(struct usb3503 *hub) > { > struct device *dev = hub->dev; > - int err = 0; > + int err; > > - switch (mode) { > - case USB3503_MODE_HUB: > - usb3503_reset(hub, 1); > + usb3503_reset(hub, 1); > > + if (hub->regmap) { > /* SP_ILOCK: set connect_n, config_n for config */ > err = regmap_write(hub->regmap, USB3503_SP_ILOCK, > - (USB3503_SPILOCK_CONNECT > + (USB3503_SPILOCK_CONNECT > | USB3503_SPILOCK_CONFIG)); > if (err < 0) { > dev_err(dev, "SP_ILOCK failed (%d)\n", err); > - goto err_hubmode; > + return err; > } > > /* PDS : Disable For Self Powered Operation */ > @@ -103,7 +102,7 @@ static int usb3503_switch_mode(struct usb3503 *hub, enum > usb3503_mode mode) > hub->port_off_mask); > if (err < 0) { > dev_err(dev, "PDS failed (%d)\n", err); > - goto err_hubmode; > + return err; > } > } > > @@ -113,7 +112,7 @@ static int usb3503_switch_mode(struct usb3503 *hub, enum > usb3503_mode mode) > USB3503_SELF_BUS_PWR); > if (err < 0) { > dev_err(dev, "CFG1 failed (%d)\n", err); > - goto err_hubmode; > + return err; > } > > /* SP_LOCK: clear connect_n, config_n for hub connect */ > @@ -122,14 +121,27 @@ static int usb3503_switch_mode(struct usb3503 *hub, > enum usb3503_mode mode) > | USB3503_SPILOCK_CONFIG), 0); > if (err < 0) { > dev_err(dev, "SP_ILOCK failed (%d)\n", err); > - goto err_hubmode; > + return err; > } > + } > > - if (gpio_is_valid(hub->gpio_connect)) > - gpio_set_value_cansleep(hub->gpio_connect, 1); > + if (gpio_is_valid(hub->gpio_connect)) > + gpio_set_value_cansleep(hub->gpio_connect, 1); > > - hub->mode = mode; > - dev_info(dev, "switched to HUB mode\n"); > + hub->mode = USB3503_MODE_HUB; > + dev_info(dev, "switched to HUB mode\n"); > + > + return 0; > +} > + > +static int usb3503_switch_mode(struct usb3503 *hub, enum usb3503_mode mode) > +{ > + struct device *dev = hub->dev; > + int err = 0; > + > + switch (mode) { > + case USB3503_MODE_HUB: > + err = usb3503_connect(hub); > break; > > case USB3503_MODE_STANDBY: > @@ -145,7 +157,6 @@ static int usb3503_switch_mode(struct usb3503 *hub, enum > usb3503_mode mode) > break; > } > > -err_hubmode: > return err; > } > > @@ -198,6 +209,9 @@ static int
RE: [PATCH 3/3] staging: dwc2: add microframe scheduler from downstream Pi kernel
> From: popcornmix [mailto:popcorn...@gmail.com] > Sent: Monday, August 12, 2013 6:39 AM > > On Mon, Aug 12, 2013 at 8:40 AM, Matthijs Kooijman wrote: > > > Paul, did you try the patch without this hunk? > > Dom, can you tell use why this hunk is needed? > > The microframe patch originally came from here: > > http://git.denx.de/?p=linux-denx.git;a=commit;h=661a47d4cec90545dce32926757ee0a95b9100d2 > which should be acknowledged. I don't see the that hunk in this patch, or our > implementation of it: > > https://github.com/raspberrypi/linux/commit/5cc98c1e75700e51e4a31d8a05b4a55a7d29648c > Can you point at the raspberry pi commit it came from? Hi Greg, Is it necessary to also get signed-off-bys from whoever at Denx did the original work on the patch? Or can I just acknowledge that in the commit message? Hi Dom, Please use plain text when replying to messages that are CCed to any of the Linux mailing lists. They reject all emails that are not plain text. It look like the part of the patch in question came from an update to the Synopsys driver, and is not part of the microframe scheduler. So I will split that into a separate patch. And I will add an acknowledgement of denx.de in the commit message for the microframe scheduler patch, or try to get signed-off-bys if necessary. -- Paul -- 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/2] usb: dwc3: Add Qualcomm DWC3 glue layer driver
On Fri, Aug 09, 2013 at 07:09:18PM +0300, Ivan T. Ivanov wrote: > Hi, > > On Fri, 2013-08-09 at 16:23 +0300, Felipe Balbi wrote: > > Hi, > > > > On Tue, Aug 06, 2013 at 02:53:11PM +0300, Ivan T. Ivanov wrote: > > > diff --git a/drivers/usb/dwc3/dwc3-msm.c b/drivers/usb/dwc3/dwc3-msm.c > > > new file mode 100644 > > > index 000..e509abc > > > --- /dev/null > > > +++ b/drivers/usb/dwc3/dwc3-msm.c > > > @@ -0,0 +1,175 @@ > > > +#undef CONFIG_REGULATOR > > > > why ?? > > > > 1. This shows that driver is still not fully tested >Regulators support is still missing in the MSM > 2. Helps me to load driver successfully. Then remove all the regulator-related code from this. > > > + clk_prepare_enable(mdwc->core_clk); > > > + clk_prepare_enable(mdwc->iface_clk); > > > + clk_prepare_enable(mdwc->sleep_clk); > > > + clk_prepare_enable(mdwc->utmi_clk); > > > > do you really need to enable your clocks here ? Why don't you enable > > them on runtime_resume and disable on runtime_suspend ? > > I will like to make it working first and then will improve > power management. alright, makes sense. > > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > > + tcsr = devm_ioremap_resource(&pdev->dev, res); > > > + if (!tcsr) { > > > + dev_dbg(&pdev->dev, "tcsr ioremap failed\n"); > > > > no need to ioremap, also you're likely leaking clocks and regulators > > enabled here. > > > > Make sure to have something like: > > > > if (!tcsr) > > goto err_disable_clocks; > > > > /* TODO This has to be revised */\ > > > > [...] > > > > Sure. just to make it clear, I meant to say that you don't need to print the error message :-) -- balbi signature.asc Description: Digital signature
Re: [PATCH 3/3] staging: dwc2: add microframe scheduler from downstream Pi kernel
On Mon, Aug 12, 2013 at 06:18:00PM +, Paul Zimmerman wrote: > > From: popcornmix [mailto:popcorn...@gmail.com] > > Sent: Monday, August 12, 2013 6:39 AM > > > > On Mon, Aug 12, 2013 at 8:40 AM, Matthijs Kooijman > > wrote: > > > > > Paul, did you try the patch without this hunk? > > > Dom, can you tell use why this hunk is needed? > > > > The microframe patch originally came from here: > > > > http://git.denx.de/?p=linux-denx.git;a=commit;h=661a47d4cec90545dce32926757ee0a95b9100d2 > > which should be acknowledged. I don't see the that hunk in this patch, or > > our implementation of it: > > > > https://github.com/raspberrypi/linux/commit/5cc98c1e75700e51e4a31d8a05b4a55a7d29648c > > Can you point at the raspberry pi commit it came from? > > Hi Greg, > > Is it necessary to also get signed-off-bys from whoever at Denx did the > original work on the patch? Or can I just acknowledge that in the commit > message? You can acknowledge it, if there's no way to get a signed-off-by from them (and it looks like it will be hard.). 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: Commit 09fc7d22b0 (usb: musb: fix incorrect usage of resource pointer) questions
On Mon, Aug 12, 2013 at 12:35:25AM +0400, Sergei Shtylyov wrote: > Hello. > > On 08/11/2013 03:08 AM, Sergei Shtylyov wrote: > > >>>I have basically two questions on this change: > > [...] > > >>>2) why you omitted am35x.c from this commit? > > >>mistake > > >Are you going to fix it, or should I? > >I'm just not sure how many resources there are with all these > hwmod complications... tell me about it. Kishon was going to send a patch which would allocate and copy those resources dynamically based on pdev->num_resources. I haven't seen the patch yet. -- balbi signature.asc Description: Digital signature
Re: [RFC PATCH v2 1/3] usb: dwc3: msm: Add device tree binding information
On Aug 12, 2013, at 1:04 PM, Felipe Balbi wrote: > On Fri, Aug 09, 2013 at 10:31:58AM -0500, Kumar Gala wrote: >> >> On Aug 9, 2013, at 4:53 AM, Ivan T. Ivanov wrote: >> >>> From: "Ivan T. Ivanov" >>> >>> MSM USB3.0 core wrapper consist of USB3.0 IP (SNPS) >> >> probably good to spell out Synopsys rather than SNPS > > Synopsys (the company) has always used snps in their bindings though. I just meant in the commit message as SNPS wasn't obvious to me and I think when someone comes back and looks the commit message doing "Synopsys (SNPS)" is a little more readable. - k -- Employee of Qualcomm Innovation Center, Inc. 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: [RFC/PATCH 2/2] usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET
On Mon, 12 Aug 2013, Felipe Balbi wrote: > > > maybe a single callback for supporting 'testmodes' ? which receives the > > > test mode as argument ? > > > > I don't have a clear picture of how you would apply such an approach to > > this case. There would have to be a way to tell the HCD to insert a > > 15-second delay between the Setup and Data stages of a particular > > control URB. How would you do that? Whatever method you choose, > > each test is started after enumerating a known Vid/Pid pair. Based on > that, you *know* which test to run. That's not what I meant. Yes, the test-device driver knows what test it wants to run, based on the VID/PID. I was asking how it would communicate this knowledge to the HCD. For example, it doesn't make sense to have a callback that means "Insert a 15-second delay into the next URB that I submit", because the HCD doesn't know where URBs come from. > > What other test modes would you want to support? > > anything that USB[23]0CV supports today. There are even link layer tests > for USB3 and a bunch of others. This SINGLE_STEP_SET_FEATURE is but one > of a large(-ish) test suite which needs to be supported. That's what I'm trying to find out. What are the special features that we would need to implement in order to support the entire test suite? > > Is it worth adding this support to the standard host controller > > drivers, or should there be a special version (a Kconfig option like > > CONFIG_RCU_TORTURE_TEST) to enable it? Putting a lot of testing code > > in distribution kernels where it will never be used seems like a big > > waste. > > right, I think it should be hidden by Kconfig, not arguing against that. Therefore we both agree the $SUBJECT patch should not be accepted in its current form. At the very least, the new code in ehci-hcd should be conditional on a CONFIG_USB_HCD_TEST_MODE option. In addition, we may want some of the work (though at this point I'm not still clear on exactly what parts) to be moved into usbcore. 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 v5 0/5] USB: HCD/EHCI: giveback of URB in tasklet context
On Wed, Jul 03, 2013 at 10:53:06PM +0800, Ming Lei wrote: > Hi, > > The patchset supports to run giveback of URB in tasklet context, so that > DMA unmapping/mapping on transfer buffer and compelte() callback can be > run with interrupt enabled, then time of HCD interrupt handler(IRQs > disabled time) can be saved much. Also this approach may simplify HCD > since HCD lock needn't be released any more before calling > usb_hcd_giveback_urb(). > > The patchset enables the mechanism on EHCI HCD now. > > In the commit log of patch 5/5, detailed test result on three machines > (ARM A9/A15 dual core, X86) are provided about transfer performance and > ehci irq handling time. From the result, basically no transfer performance > loss is found and ehci irq handling time drops much with the patchset. All now applied, sorry for the delay. 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
[PATCH] usb: musb: dsps: make it depend on OF_IRQ
musb_dsps.c utilizes a symbol which is only available when CONFIG_OF_IRQ is set, so make it depend on that. Signed-off-by: Felipe Balbi --- drivers/usb/musb/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig index 04658d7..c64ee09a7 100644 --- a/drivers/usb/musb/Kconfig +++ b/drivers/usb/musb/Kconfig @@ -84,6 +84,7 @@ config USB_MUSB_AM35X config USB_MUSB_DSPS tristate "TI DSPS platforms" select USB_MUSB_AM335X_CHILD + depends on OF_IRQ config USB_MUSB_BLACKFIN tristate "Blackfin" -- 1.8.3.4.840.g6a90778 -- 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/4] usb: config->desc.bLength may not exceed amount of data returned by the device
On Sat, Aug 03, 2013 at 04:37:48PM +0200, Hans de Goede wrote: > While reading the config parsing code I noticed this check is missing, without > this check config->desc.wTotalLength can end up with a value larger then the > dev->rawdescriptors length for the config, and when userspace then tries to > get the rawdescriptors bad things may happen. This would require a "broken" device configuration as the length is determined from the descriptors themselves, not from userspace, but yes, I agree it's good to have this check. 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 4/4] Documentation sysfs-bus-usb: Document all files used by libusb
On Sat, Aug 03, 2013 at 04:37:51PM +0200, Hans de Goede wrote: > Signed-off-by: Hans de Goede > --- > Documentation/ABI/testing/sysfs-bus-usb | 38 > + > 1 file changed, 38 insertions(+) This patch doesn't apply at all, can you redo it against the usb-next branch and resend? 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 V2] USB: EHCI: make ehci-w90X900 a separate driver
On Sat, Aug 10, 2013 at 01:04:33PM +0530, Manjunath Goudar wrote: > Separate the W90X900(W90P910) on-chip host controller driver from > ehci-hcd host code so that it can be built as a separate driver module. > This work is part of enabling multi-platform kernels on ARM; > however, note that other changes are still needed before W90X900(W90P910) > can be booted with a multi-platform kernel > > and an ehci driver that only works on one of them. > > With the infrastructure added by Alan Stern in patch 3e0232039 > "USB: EHCI: prepare to make ehci-hcd a library module", we can > avoid this problem by turning a bus glue into a separate > module, as we do here for the w90X900 bus glue. > > Signed-off-by: Manjunath Goudar > Signed-off-by: Deepak Saxena > Acked-by: Arnd Bergmann > Acked-by: Wan ZongShun > Acked-by: Alan Stern > Cc: Greg KH > Cc: linux-usb@vger.kernel.org > Cc: linux-ker...@vger.kernel.org > > V2: > -Arranged #include's in alphabetical order. > -Replaced "w90p910" by "w90x900" because it is supports > all series of w90x900. > > drivers/usb/host/Kconfig|2 +- > drivers/usb/host/Makefile |1 + > drivers/usb/host/ehci-hcd.c |5 --- > drivers/usb/host/ehci-w90x900.c | 85 > --- > 4 files changed, 37 insertions(+), 56 deletions(-) This patch fails to apply: checking file drivers/usb/host/Kconfig Hunk #1 succeeded at 236 with fuzz 1 (offset -8 lines). checking file drivers/usb/host/Makefile Hunk #1 FAILED at 35. 1 out of 1 hunk FAILED checking file drivers/usb/host/ehci-hcd.c Hunk #1 succeeded at 1246 (offset -3 lines). checking file drivers/usb/host/ehci-w90x900.c Hunk #2 FAILED at 114. 1 out of 3 hunks FAILED Please redo it against the latest usb-next branch and resend. 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 V3 1/2] USB: OHCI: make ohci-ep93xx a separate driver
On Sat, Aug 10, 2013 at 01:11:39PM +0530, Manjunath Goudar wrote: > Separate the OHCI EP93XX host controller driver from ohci-hcd > host code so that it can be built as a separate driver module. > This work is part of enabling multi-platform kernels on ARM. > > Signed-off-by: Manjunath Goudar > Signed-off-by: Deepak Saxena > Acked-by: Alan Stern > Cc: Arnd Bergmann > Cc: Greg KH > Cc: linux-usb@vger.kernel.org > > V2: > -ohci_hcd_init() statements are removed, > because by default it is called in ohci_setup(). > > V3: > -Unused *ohci variable has been removed. > --- > drivers/usb/host/Kconfig |8 > drivers/usb/host/Makefile |1 + > drivers/usb/host/ohci-ep93xx.c | 80 > +--- > drivers/usb/host/ohci-hcd.c| 18 - > 4 files changed, 43 insertions(+), 64 deletions(-) This too fails to apply: $ p1 < ../s1 checking file drivers/usb/host/Kconfig Hunk #1 FAILED at 414. 1 out of 1 hunk FAILED checking file drivers/usb/host/Makefile Hunk #1 FAILED at 54. 1 out of 1 hunk FAILED checking file drivers/usb/host/ohci-ep93xx.c Hunk #2 FAILED at 92. Hunk #3 FAILED at 120. Hunk #4 succeeded at 179 (offset -40 lines). Hunk #5 succeeded at 193 (offset -40 lines). 2 out of 5 hunks FAILED checking file drivers/usb/host/ohci-hcd.c Reversed (or previously applied) patch detected! Assume -R? [n] Apply anyway? [n] Skipping patch. 3 out of 3 hunks ignored Can you respin _all_ of your pending patches you have sent me in a format that I can apply them, and resend them in a series that shows what should be applied in which order. As it is, I have 3-4 different series of patches from you, with no obvious order which should be applied in which order. Because of that, that might be why these aren't applying. 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 V5 4/6] USB: OHCI: make ohci-spear a separate driver
On Mon, Aug 12, 2013 at 12:26:09PM +0530, Viresh Kumar wrote: > On 12 August 2013 12:16, Manjunath Goudar wrote: > > This work is part of enabling multi-platform kernels on ARM; > > it would be nice to have in 3.11. > > You really want it in 3.11, we are already at 3.11-rc5 :) > > Whatever you write here gets in git history with your patch and I > am sure you never wanted this to be commited. :) > > So, both the above lines must be removed from each of > your patches and must be present only once in the cover-letter. > > Don't resend your patches now with this.. Unless Greg wants you > to. Yes, they all need to be resent as they don't seem to apply :( Please just delete that part of the sentance from all of the patches in this series. It does not matter which tree it would be "nice" to have them in, once they are accepted the changelog will show where it was accepted. 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 V2] USB: OHCI: make ohci-nxp a separate driver
On Sat, Aug 10, 2013 at 01:14:15PM +0530, Manjunath Goudar wrote: > Separate the OHCI NXP host controller driver from ohci-hcd > host code so that it can be built as a separate driver module. > This work is part of enabling multi-platform kernels on ARM. > > Many place function name and struct name started with usb, > current scenario replaced usb with ohci for proper naming. > > Signed-off-by: Manjunath Goudar > Signed-off-by: Deepak Saxena > Acked-by: Alan Stern > Cc: Arnd Bergmann > Cc: Greg KH > Cc: linux-usb@vger.kernel.org > > V2: > - Directive check NXP_PLATFORM_DRIVER block has been removed. > --- > drivers/usb/host/Kconfig|8 +++ > drivers/usb/host/Makefile |1 + > drivers/usb/host/ohci-hcd.c | 18 --- > drivers/usb/host/ohci-nxp.c | 124 > +-- > 4 files changed, 58 insertions(+), 93 deletions(-) Also failed to apply: checking file drivers/usb/host/Kconfig Hunk #1 FAILED at 398. 1 out of 1 hunk FAILED checking file drivers/usb/host/Makefile Hunk #1 FAILED at 50. 1 out of 1 hunk FAILED checking file drivers/usb/host/ohci-hcd.c Hunk #1 succeeded at 1212 with fuzz 1 (offset 18 lines). Hunk #2 succeeded at 1349 with fuzz 1 (offset 53 lines). Hunk #3 FAILED at 1309. Hunk #4 FAILED at 1357. 2 out of 4 hunks FAILED checking file drivers/usb/host/ohci-nxp.c {sigh} That's why I required a senior Linaro developer to approve these patches. You just wasted a bunch of my time, and it was obvious that no one else actually tried these patches out, as they can't even be applied! Deepak, come on, you know better :( 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 v3] usb: rh_call_control tbuf potential future overflow fix
On Mon, Aug 12, 2013 at 11:09:25AM -0700, Sean O. Stalley wrote: > rh_call_control() contains a buffer, tbuf, which it uses to hold > USB descriptors. These discriptors are eventually copied into the > transfer_buffer in the URB. The buffer in the URB is dynamically > defined and is always large enough to hold the amount of data it > requests. > > tbuf is currently statically allocated on the stack with a size > of 15 bytes, regardless of the size specified in the URB. > This patch dynamically allocates tbuf, and ensures that tbuf is > at least as big as the buffer in the URB. > > If an hcd attempts to write a descriptor containing more than > 15 bytes ( such as the Standard BOS Descriptor for hubs, defined > in the USB3.0 Spec, section 10.13.1 ) the write would overflow > the buffer and corrupt the stack. This patch addresses this > behavior. > > Acked-by: Alan Stern > Signed-off-by: Sean O. Stalley > --- > drivers/usb/core/hcd.c | 24 +--- > 1 file changed, 17 insertions(+), 7 deletions(-) Applying this patch produces a complier warning that shows the patch is buggy: drivers/usb/core/hcd.c: In function ‘usb_hcd_submit_urb’: drivers/usb/core/hcd.c:704:7: warning: ‘tbuf’ may be used uninitialized in this function [-Wmaybe-uninitialized] drivers/usb/core/hcd.c:474:7: note: ‘tbuf’ was declared here gcc is correct here, please fix this, and _never_ ignore complier warnings. 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: Non-enumerable devices on USB and other enumerable buses
On Mon, Aug 12, 2013 at 12:08:17PM -0600, Stephen Warren wrote: > In a similar way, I wonder if the USB case can be considered the same > way? This seems less like a good fit since I don't expect the resources > are always so similar there, and also there's the case of the bus being > potentially behind a few levels of USB hub. That won't work for the hub on Arndale in a system where it's fully wired up - it's got an I2C interface for control and the idea is that you bring it out of reset, write the configuration out via I2C and then you can complete startup using a GPIO or another I2C command. > And of course it all gets a little more messy when you get > board-specific logic that needs setup, rather than something more common > across multiple devices. I think we could probably cover many cases by putting common things onto the bus but we still need to confront the cases where the device driver really does need to get involved in what's going on. Things like the SDIO WiFi cards seem very interchangable as you say and would probably benefit a lot but there's other applications that are less regular. signature.asc Description: Digital signature
Re: [RFC 0/2] USB port power off bug fixes
On Tue, Aug 06, 2013 at 01:16:50PM -0700, Sarah Sharp wrote: > On Tue, Aug 06, 2013 at 02:48:57PM -0400, Alan Stern wrote: > > On Tue, 6 Aug 2013, Sarah Sharp wrote: > > > > > On Tue, Aug 06, 2013 at 10:16:15AM -0400, Alan Stern wrote: > > > > Patch 1 has been rendered out of date by intervening changes (commit > > > > 4fae6f0fa86f in Greg's usb-next tree). > > > > > > Greg, could that be marked for stable? I'm not sure why it's in > > > usb-next; it should really be in usb-linus, since it's a bug fix. The > > > USB port power off mechanism doesn't work for suspended USB devices > > > without it. > > > > That commit does not do exactly the same thing as your patch 1. It > > keeps the NO_POWER_OFF check in usb_port_suspend(), among other things. > > I didn't mean to imply that patch 1 was unnecessary, just that it was > > out of date and would cause a merge conflict when Greg tried to apply > > it to the current tree. > > Ah, ok. It sounds like we need both patches for stable, and I should > rebase patch 1 against yours. I can either queue the rebased patch for > usb-next and mention that it (and your patch) should be queued for > stable, or Greg could revert yours from usb-next, move it to usb-linus, > and I could send my patch to usb-linus. > > Greg, how do you want to handle this? I tried to "cherry pick" this patch from my usb-next branch to my usb-linus branch, but it fails :( So, could you "port" it, add a cc: stable line, and then resend these 3 for me to apply? Or pull, either is fine. 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: Non-enumerable devices on USB and other enumerable buses
On Mon, Aug 12, 2013 at 12:23:44PM +0100, Mark Brown wrote: > On Sun, Aug 11, 2013 at 07:02:57PM -0700, Greg Kroah-Hartman wrote: > > On Sun, Aug 11, 2013 at 08:08:26PM +0100, Mark Brown wrote: > > > > I know there's been some discussion of this topic but do we have any > > > general consensus on how to handle such things both from a Linux driver > > > model point of view and from a DT/ACPI point of view? > > > As these are usually bus-specific things, and really, platform-specific > > I don't think they're bus specific - the main issue with a lot of this > is that they're outside the infrastructure that the bus standardises so > we should have a general way of understanding this. There will be bits > where the bus needs to get involved but the overall pattern is generic. The "pattern" is generic, yes, we've been dealing with that for well over a decade now (pci hotplug controllers are "out of line" and control when/if a PCI is device is added/removed.) I'd argue that each bus needs special logic to handle this, just like PCI hotplug did it with their hotplug controller logic. Due to the nature of this type of thing, it's all out-of-line hardware that is custom to each device type / bus. > > things, I'd fall back and just recommend a "platform driver" that > > "knows" about where these gpios are, and how/when to enable them, which > > will cause the discoverable bus logic to kick in now that it notices a > > device is present/removed. > > This doesn't work in general. These things aren't really platform > specific at all, they're features of the devices that apply to any > system using them and while for some use cases (like WiFi and BT power) > an external thing that just manually applies power will be enough in > other cases we want to know about the device even while it's off and the > driver might be able to do extra things at runtime if it knows about for > example having some control over signals to the device. No, the device isn't platform specific, but the logic needed to turn on/off is platform specific, otherwise the device would "just work" properly as the bus is self-discoverable. Much like rfkill is, as you point out. Those are all platform specific. > For example in the Slimbus case we're normally talking about audio > CODECs. In order to preserve the userspace API the device has to appear > to be present at all times, the driver will remember the settings the > user is making to be applied when it actually powers up and indeed the > powering up should be kicked off as a result of userspac acting on the > apparently present device. That's some horrible hardware, who thought of that? > Another example here (including USB devices) is interrupt signals wired > directly back to the processor, completely outside of the bus. Like rfkill switches that just power on/off a USB device built onto the laptop motherboard :) > > Perhaps a semi-generic "platform" driver could be created, that knows > > how to handle these settings in the DT, but odds are, that might be > > difficult to make generic enough to cover a wide range of boards, but > > without specifics, it's hard to tell. > > There's things like the rfkill stuff already, and the reset controller > on the way, but again this only covers a fairly small subset of the > issues. "small subset"? How common is this type of thing? 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: gadget: at91_udc: add usb_clk for transition to common clk framework
Hello Nicolas, On 12/08/2013 15:52, Nicolas Ferre wrote: On 01/08/2013 08:18, Boris BREZILLON : The AT91 PMC (Power Management Controller) provides an USB clock used by USB Full Speed host (ohci) and USB Full Speed device (udc). The usb drivers (ohci and udc) must configure this clock to 48Mhz. This configuration was formely done in mach-at91/clock.c, but this implementation will be removed when moving to common clk framework. This patch adds support for usb clock retrieval and configuration, and is backward compatible with the current at91 clk implementation (if usb clk is not found, it does not configure/enable it). Signed-off-by: Boris BREZILLON Acked-by: Nicolas Ferre Is there a reason you acked this version but not the 3rd one (https://lkml.org/lkml/2013/8/2/102). --- drivers/usb/gadget/at91_udc.c | 17 - drivers/usb/gadget/at91_udc.h |2 +- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/drivers/usb/gadget/at91_udc.c b/drivers/usb/gadget/at91_udc.c index fce8e4e..ae06585 100644 --- a/drivers/usb/gadget/at91_udc.c +++ b/drivers/usb/gadget/at91_udc.c @@ -870,6 +870,11 @@ static void clk_on(struct at91_udc *udc) if (udc->clocked) return; udc->clocked = 1; + +if (!IS_ERR(udc->uclk)) { +clk_set_rate(udc->uclk, 4800); +clk_prepare_enable(udc->uclk); +} clk_prepare_enable(udc->iclk); clk_prepare_enable(udc->fclk); } @@ -882,6 +887,8 @@ static void clk_off(struct at91_udc *udc) udc->gadget.speed = USB_SPEED_UNKNOWN; clk_disable_unprepare(udc->fclk); clk_disable_unprepare(udc->iclk); +if (!IS_ERR(udc->uclk)) +clk_disable_unprepare(udc->uclk); } /* @@ -1774,10 +1781,10 @@ static int at91udc_probe(struct platform_device *pdev) /* get interface and function clocks */ udc->iclk = clk_get(dev, "udc_clk"); udc->fclk = clk_get(dev, "udpck"); +udc->uclk = clk_get(dev, "usb_clk"); if (IS_ERR(udc->iclk) || IS_ERR(udc->fclk)) { DBG("clocks missing\n"); retval = -ENODEV; -/* NOTE: we "know" here that refcounts on these are NOPs */ goto fail1; } @@ -1851,6 +1858,12 @@ fail3: fail2: free_irq(udc->udp_irq, udc); fail1: +if (!IS_ERR(udc->uclk)) +clk_put(udc->uclk); +if (!IS_ERR(udc->fclk)) +clk_put(udc->fclk); +if (!IS_ERR(udc->iclk)) +clk_put(udc->iclk); iounmap(udc->udp_baseaddr); fail0a: if (cpu_is_at91rm9200()) @@ -1894,6 +1907,8 @@ static int __exit at91udc_remove(struct platform_device *pdev) clk_put(udc->iclk); clk_put(udc->fclk); +if (!IS_ERR(udc->uclk)) +clk_put(udc->uclk); return 0; } diff --git a/drivers/usb/gadget/at91_udc.h b/drivers/usb/gadget/at91_udc.h index e647d1c..0175246 100644 --- a/drivers/usb/gadget/at91_udc.h +++ b/drivers/usb/gadget/at91_udc.h @@ -126,7 +126,7 @@ struct at91_udc { unsignedactive_suspend:1; u8addr; struct at91_udc_databoard; -struct clk*iclk, *fclk; +struct clk*iclk, *fclk, *uclk; struct platform_device*pdev; struct proc_dir_entry*pde; void __iomem*udp_baseaddr; -- 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: add usb_clk for transition to common clk framework
Hello Felipe On 12/08/2013 20:08, Felipe Balbi wrote: On Mon, Aug 12, 2013 at 03:52:01PM +0200, Nicolas Ferre wrote: On 01/08/2013 08:18, Boris BREZILLON : The AT91 PMC (Power Management Controller) provides an USB clock used by USB Full Speed host (ohci) and USB Full Speed device (udc). The usb drivers (ohci and udc) must configure this clock to 48Mhz. This configuration was formely done in mach-at91/clock.c, but this implementation will be removed when moving to common clk framework. This patch adds support for usb clock retrieval and configuration, and is backward compatible with the current at91 clk implementation (if usb clk is not found, it does not configure/enable it). Signed-off-by: Boris BREZILLON Acked-by: Nicolas Ferre I don't have the original, where is it ? Was it not Cced to linux-usb ? Can someone resend with Nicolas' Acked-by in place ? This mail was sent to linux-usb@vger.kernel.org and to you (ba...@ti.com). It may have been filtered. Anyway, you asked me to rework the 2nd version (which I did). I think it's better to review the 3rd one (https://lkml.org/lkml/2013/8/2/102). Best Regards, Boris -- 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: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET" added to usb tree
On Mon, 12 Aug 2013 gre...@linuxfoundation.org wrote: > This is a note to let you know that I've just added the patch titled > > usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET > > to my usb git tree which can be found at > git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git > in the usb-next branch. > > The patch will show up in the next release of the linux-next tree > (usually sometime within the next 24 hours during the week.) > > The patch will also be merged in the next major kernel release > during the merge window. > > If you have any questions about this process, please let me know. > > > From 9841f37a1cca5357c1fd198b1068c12955aa632f Mon Sep 17 00:00:00 2001 > From: Manu Gautam > Date: Thu, 8 Aug 2013 16:49:24 -0700 > Subject: usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET > > The USB Embedded High-speed Host Electrical Test (EHSET) defines the > SINGLE_STEP_SET_FEATURE test as follows: > > 1) The host enumerates the test device with VID:0x1A0A, PID:0x0108 > 2) The host sends the SETUP stage of a GetDescriptor(Device) > 3) The device ACKs the request > 4) The host issues SOFs for 15 seconds allowing the test operator to >raise the scope trigger just above the SOF voltage level > 5) The host sends the IN packet > 6) The device sends data in response, triggering the scope > 7) The host sends an ACK in response to the data > > This patch adds additional handling to the EHCI hub driver and allows > the EHSET driver to initiate this test mode by issuing a a SetFeature > request to the root hub with a Test Selector value of 0x06. From there > it mimics ehci_urb_enqueue() but separately submits QTDs for the > SETUP and DATA/STATUS stages in order to insert a delay in between. > > Signed-off-by: Manu Gautam > Acked-by: Alan Stern > [ja...@codeaurora.org: imported from commit c2084930 on codeaurora.org; > minor cleanup and updated author email] > Signed-off-by: Jack Pham > Signed-off-by: Greg Kroah-Hartman Okay, I was a little too late and Greg has merged the patch. Jack or Manu, can you write a follow-on patch that adds a Kconfig option to enable test modes in the HCDs, and then avoids compiling the new code added to ehci-hcd if the config option is disabled? 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 1/2] ACPI / PCI: don't allow PCI devices without irq to be enabled
On Mon, Aug 12, 2013 at 2:14 PM, Bjorn Helgaas wrote: > On Thu, Aug 8, 2013 at 7:57 AM, Rafael J. Wysocki wrote: >> On Thursday, August 08, 2013 03:35:13 PM Heikki Krogerus wrote: >>> If there is no ACPI entry for the irq, returning error from >>> acpi_pci_enable_irq() if the irq is 0. >>> >>> Prarit Bhargava reported an issue where he noticed that his >>> Dell PowerEdge 840 has buggy BIOS that does not supply ACPI >>> entries for irq with some devices. That lead into kernel >>> generating a warning "genirq: Flags mismatch irq 0...". This >>> will fix that issue. >>> >>> Signed-off-by: Heikki Krogerus >>> Tested-by: Prarit Bhargava >> >> Hi Bjorn, >> >> Any objections to this? > > Acked-by: Bjorn Helgaas some USB3 host controller does not have intx configured, but still work with MSI? Yinghai -- 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: Non-enumerable devices on USB and other enumerable buses
On Mon, Aug 12, 2013 at 01:50:07PM -0700, Greg Kroah-Hartman wrote: > On Mon, Aug 12, 2013 at 12:23:44PM +0100, Mark Brown wrote: > > I don't think they're bus specific - the main issue with a lot of this > > is that they're outside the infrastructure that the bus standardises so > > we should have a general way of understanding this. There will be bits > > where the bus needs to get involved but the overall pattern is generic. > The "pattern" is generic, yes, we've been dealing with that for well > over a decade now (pci hotplug controllers are "out of line" and control > when/if a PCI is device is added/removed.) > I'd argue that each bus needs special logic to handle this, just like > PCI hotplug did it with their hotplug controller logic. Due to the > nature of this type of thing, it's all out-of-line hardware that is > custom to each device type / bus. I agree that the bus will need some logic to handle this and I also think Stephen is right that there are going to be some common patterns for some device classes. However I do think that this is a common problem for pretty much all buses so we should be factoring out as much as possible from the buses so they only have to deal with their bit of things. Most of the stuff that's worrying me is not anything the bus should have any idea about beyond tunnelling the data to the device. > > This doesn't work in general. These things aren't really platform > > specific at all, they're features of the devices that apply to any > > system using them and while for some use cases (like WiFi and BT power) > > an external thing that just manually applies power will be enough in > > other cases we want to know about the device even while it's off and the > > driver might be able to do extra things at runtime if it knows about for > > example having some control over signals to the device. > No, the device isn't platform specific, but the logic needed to turn > on/off is platform specific, otherwise the device would "just work" > properly as the bus is self-discoverable. > Much like rfkill is, as you point out. Those are all platform specific. This is not just about power control - that's one of the simpler applications but you're talking about essentially anything that might be handled by platform data (or by the configuration SEPROMs on a freestanding device for that matter, why would you put down an extra part to configure the device when you can just load things from the AP). > > For example in the Slimbus case we're normally talking about audio > > CODECs. In order to preserve the userspace API the device has to appear > > to be present at all times, the driver will remember the settings the > > user is making to be applied when it actually powers up and indeed the > > powering up should be kicked off as a result of userspac acting on the > > apparently present device. > That's some horrible hardware, who thought of that? It's actually pretty nice hardware in many regards; the bus is a bit underspecified for maximum bandwidth in modern systems but it gives you standardised multi-drop data streaming over two wires muxed with control which helps with the pin counts and with simplifying the system hardware design. The disappearing from the bus thing is because you're aiming to hit low power idle (but still functioning) states consuming very small numbers of microamps, even having a clock running is measurable. > > > Perhaps a semi-generic "platform" driver could be created, that knows > > > how to handle these settings in the DT, but odds are, that might be > > > difficult to make generic enough to cover a wide range of boards, but > > > without specifics, it's hard to tell. > > There's things like the rfkill stuff already, and the reset controller > > on the way, but again this only covers a fairly small subset of the > > issues. > "small subset"? How common is this type of thing? Well, for Slimbus it's absolutely the norm, I've never seen a Slimbus device that didn't have a need for platform data. The small subset there is of the problems rather than the devices - you'll cover a large set of devices with a common helper like rfkill but trying to handle everything with that pattern doesn't seem like it'll work. signature.asc Description: Digital signature
[RFC 1/3] xhci: trace debug statements for urb cancellation
This patch defines a new trace event, which is called xhci_dbg_cancel_urb and belongs to the event class xhci_log_msg, and adds tracepoints that trace the debug messages related to the removal of a cancelled URB from the endpoint's transfer ring. Signed-off-by: Xenia Ragiadakou --- drivers/usb/host/xhci-ring.c | 64 +++ drivers/usb/host/xhci-trace.h | 5 drivers/usb/host/xhci.c | 13 + 3 files changed, 54 insertions(+), 28 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 0bb275e..3a2277b 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -556,7 +556,8 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci, return; } state->new_cycle_state = 0; - xhci_dbg(xhci, "Finding segment containing stopped TRB.\n"); + xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, + "Finding segment containing stopped TRB."); state->new_deq_seg = find_trb_seg(cur_td->start_seg, dev->eps[ep_index].stopped_trb, &state->new_cycle_state); @@ -566,12 +567,14 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci, } /* Dig out the cycle state saved by the xHC during the stop ep cmd */ - xhci_dbg(xhci, "Finding endpoint context\n"); + xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, + "Finding endpoint context"); ep_ctx = xhci_get_ep_ctx(xhci, dev->out_ctx, ep_index); state->new_cycle_state = 0x1 & le64_to_cpu(ep_ctx->deq); state->new_deq_ptr = cur_td->last_trb; - xhci_dbg(xhci, "Finding segment containing last TRB in TD.\n"); + xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, + "Finding segment containing last TRB in TD."); state->new_deq_seg = find_trb_seg(state->new_deq_seg, state->new_deq_ptr, &state->new_cycle_state); @@ -598,13 +601,16 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci, if (ep_ring->first_seg == ep_ring->first_seg->next && state->new_deq_ptr < dev->eps[ep_index].stopped_trb) state->new_cycle_state ^= 0x1; - xhci_dbg(xhci, "Cycle state = 0x%x\n", state->new_cycle_state); + xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, + "Cycle state = 0x%x", state->new_cycle_state); /* Don't update the ring cycle state for the producer (us). */ - xhci_dbg(xhci, "New dequeue segment = %p (virtual)\n", + xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, + "New dequeue segment = %p (virtual)", state->new_deq_seg); addr = xhci_trb_virt_to_dma(state->new_deq_seg, state->new_deq_ptr); - xhci_dbg(xhci, "New dequeue pointer = 0x%llx (DMA)\n", + xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, + "New dequeue pointer = 0x%llx (DMA)", (unsigned long long) addr); } @@ -632,9 +638,11 @@ static void td_to_noop(struct xhci_hcd *xhci, struct xhci_ring *ep_ring, if (flip_cycle) cur_trb->generic.field[3] ^= cpu_to_le32(TRB_CYCLE); - xhci_dbg(xhci, "Cancel (unchain) link TRB\n"); - xhci_dbg(xhci, "Address = %p (0x%llx dma); " - "in seg %p (0x%llx dma)\n", + xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, + "Cancel (unchain) link TRB"); + xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, + "Address = %p (0x%llx dma); " + "in seg %p (0x%llx dma)", cur_trb, (unsigned long long)xhci_trb_virt_to_dma(cur_seg, cur_trb), cur_seg, @@ -652,7 +660,8 @@ static void td_to_noop(struct xhci_hcd *xhci, struct xhci_ring *ep_ring, cpu_to_le32(TRB_CYCLE); cur_trb->generic.field[3] |= cpu_to_le32( TRB_TYPE(TRB_TR_NOOP)); - xhci_dbg(xhci, "TRB to noop at offset 0x%llx\n", + xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, + "TRB to noop at offset 0x%llx", (unsigned long long) xhci_trb_virt_to_dma(cur_seg, cur_trb)); } @@ -673,8 +682,9 @@ void xhci_queue_new_dequeue_state(struct xhci_hcd *xhci, { struct xhci_virt_ep *ep = &xhci->devs[slot_id]->eps[ep_index]; - xhci_dbg(xhci, "Set TR Deq Ptr cmd, new
Re: patch "usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET" added to usb tree
On Mon, Aug 12, 2013 at 05:03:17PM -0400, Alan Stern wrote: > Okay, I was a little too late and Greg has merged the patch. > > Jack or Manu, can you write a follow-on patch that adds a Kconfig > option to enable test modes in the HCDs, and then avoids compiling the > new code added to ehci-hcd if the config option is disabled? Doh! In fact Manu's original patch did have such #ifdefs which I neglected to preserve in the patch I sent. Will send a follow-on patch. Jack -- 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
[RFC 2/3] xhci: trace debug messages related to driver initialization and unload
This patch defines a new trace event, which is called xhci_dbg_init and belongs to the event class xhci_log_msg, and adds tracepoints that trace the debug statements in the functions used to start and stop the xhci-hcd driver. Also, it removes an unnecessary cast of variable val to unsigned int in xhci_mem_init(), since val is already declared as unsigned int. Signed-off-by: Xenia Ragiadakou --- drivers/usb/host/xhci-mem.c | 110 ++ drivers/usb/host/xhci-trace.h | 5 ++ drivers/usb/host/xhci.c | 66 +++-- 3 files changed, 113 insertions(+), 68 deletions(-) diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index b1bb59b..ef27470 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -1545,7 +1545,8 @@ static int scratchpad_alloc(struct xhci_hcd *xhci, gfp_t flags) struct device *dev = xhci_to_hcd(xhci)->self.controller; int num_sp = HCS_MAX_SCRATCHPAD(xhci->hcs_params2); - xhci_dbg(xhci, "Allocating %d scratchpad buffers\n", num_sp); + xhci_dbg_trace(xhci, trace_xhci_dbg_init, + "Allocating %d scratchpad buffers", num_sp); if (!num_sp) return 0; @@ -1702,11 +1703,11 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci) dma_free_coherent(&pdev->dev, size, xhci->erst.entries, xhci->erst.erst_dma_addr); xhci->erst.entries = NULL; - xhci_dbg(xhci, "Freed ERST\n"); + xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed ERST"); if (xhci->event_ring) xhci_ring_free(xhci, xhci->event_ring); xhci->event_ring = NULL; - xhci_dbg(xhci, "Freed event ring\n"); + xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed event ring"); if (xhci->lpm_command) xhci_free_command(xhci, xhci->lpm_command); @@ -1714,7 +1715,7 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci) if (xhci->cmd_ring) xhci_ring_free(xhci, xhci->cmd_ring); xhci->cmd_ring = NULL; - xhci_dbg(xhci, "Freed command ring\n"); + xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed command ring"); list_for_each_entry_safe(cur_cd, next_cd, &xhci->cancel_cmd_list, cancel_cmd_list) { list_del(&cur_cd->cancel_cmd_list); @@ -1727,22 +1728,24 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci) if (xhci->segment_pool) dma_pool_destroy(xhci->segment_pool); xhci->segment_pool = NULL; - xhci_dbg(xhci, "Freed segment pool\n"); + xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed segment pool"); if (xhci->device_pool) dma_pool_destroy(xhci->device_pool); xhci->device_pool = NULL; - xhci_dbg(xhci, "Freed device context pool\n"); + xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed device context pool"); if (xhci->small_streams_pool) dma_pool_destroy(xhci->small_streams_pool); xhci->small_streams_pool = NULL; - xhci_dbg(xhci, "Freed small stream array pool\n"); + xhci_dbg_trace(xhci, trace_xhci_dbg_init, + "Freed small stream array pool"); if (xhci->medium_streams_pool) dma_pool_destroy(xhci->medium_streams_pool); xhci->medium_streams_pool = NULL; - xhci_dbg(xhci, "Freed medium stream array pool\n"); + xhci_dbg_trace(xhci, trace_xhci_dbg_init, + "Freed medium stream array pool"); if (xhci->dcbaa) dma_free_coherent(&pdev->dev, sizeof(*xhci->dcbaa), @@ -1968,8 +1971,9 @@ static void xhci_set_hc_event_deq(struct xhci_hcd *xhci) * there might be more events to service. */ temp &= ~ERST_EHB; - xhci_dbg(xhci, "// Write event ring dequeue pointer, " - "preserving EHB bit\n"); + xhci_dbg_trace(xhci, trace_xhci_dbg_init, + "// Write event ring dequeue pointer, " + "preserving EHB bit"); xhci_write_64(xhci, ((u64) deq & (u64) ~ERST_PTR_MASK) | temp, &xhci->ir_set->erst_dequeue); } @@ -1992,8 +1996,9 @@ static void xhci_add_in_port(struct xhci_hcd *xhci, unsigned int num_ports, temp = xhci_readl(xhci, addr + 2); port_offset = XHCI_EXT_PORT_OFF(temp); port_count = XHCI_EXT_PORT_COUNT(temp); - xhci_dbg(xhci, "Ext Cap %p, port offset = %u, " - "count = %u, revision = 0x%x\n", + xhci_dbg_trace(xhci, trace_xhci_dbg_init, + "Ext Cap %p, port offset = %u, " + "count = %u, revision = 0x%x", addr, port_offset, port_count, major_revision); /* Port count includes the current port offset */ if (port_offset == 0 || (port_offset + port_count - 1) > num_ports) @@ -2007,15 +2012,18 @@ static
[RFC 3/3] xhci: trace debug statements related to ring expansion
This patch defines a new trace event, which is called xhci_dbg_ring_expansion and belongs to the event class xhci_log_msg, and adds tracepoints that trace the debug messages associated with the expansion of endpoint ring when there is not enough space allocated to hold all pending TRBs. Signed-off-by: Xenia Ragiadakou --- drivers/usb/host/xhci-mem.c | 3 ++- drivers/usb/host/xhci-ring.c | 4 ++-- drivers/usb/host/xhci-trace.h | 5 + 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index ef27470..b150360 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -348,7 +348,8 @@ int xhci_ring_expansion(struct xhci_hcd *xhci, struct xhci_ring *ring, return -ENOMEM; xhci_link_rings(xhci, ring, first, last, num_segs); - xhci_dbg(xhci, "ring expansion succeed, now has %d segments\n", + xhci_dbg_trace(xhci, trace_xhci_dbg_ring_expansion, + "ring expansion succeed, now has %d segments", ring->num_segs); return 0; diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 3a2277b..a8f9e05 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -2904,8 +2904,8 @@ static int prepare_ring(struct xhci_hcd *xhci, struct xhci_ring *ep_ring, return -ENOMEM; } - xhci_dbg(xhci, "ERROR no room on ep ring, " - "try ring expansion\n"); + xhci_dbg_trace(xhci, trace_xhci_dbg_ring_expansion, + "ERROR no room on ep ring, try ring expansion"); num_trbs_needed = num_trbs - ep_ring->num_trbs_free; if (xhci_ring_expansion(xhci, ep_ring, num_trbs_needed, mem_flags)) { diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h index 11025fe..20364cc 100644 --- a/drivers/usb/host/xhci-trace.h +++ b/drivers/usb/host/xhci-trace.h @@ -62,6 +62,11 @@ DEFINE_EVENT(xhci_log_msg, xhci_dbg_init, TP_ARGS(vaf) ); +DEFINE_EVENT(xhci_log_msg, xhci_dbg_ring_expansion, + TP_PROTO(struct va_format *vaf), + TP_ARGS(vaf) +); + DECLARE_EVENT_CLASS(xhci_log_ctx, TP_PROTO(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx, unsigned int ep_num), -- 1.8.3.2 -- 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/2] Revert "Revert "HID: Fix logitech-dj: missing Unifying device issue""
On Thursday 18 July 2013 16:28:01 Peter Hurley wrote: > Before we revert to using the workaround, I'd like to suggest that > this new "hidden" problem may be an interaction with the xhci_hcd host > controller driver only. > > Looking at the related bug, the OP indicates the machine only has > USB3 ports. Additionally, comments #7, #100, and #104 of the original > bug report add additional information that would seem to confirm > this suspicion. > > Let me add I have this USB device running on the uhci_hcd driver > with or without this workaround on v3.10. This problem does not seem specific to xhci, uhci seems also effected. Today I upgraded a system (running Arch Linux) from kernel 3.9.9 to 3.10.5. After a reboot to 3.10.5, things broke. The setup: - There are two USB receivers plugged into USB 1.1 ports (different buses according to lsusb, uhci), each receiver is paired to a K360 keyboard. - One of the receivers are passed to a QEMU guest with -usbdevice host:$busid. $devid. This keyboard is working (probably because QEMU performed a reset). - Since 3.10.5, the keyboard that is *not* passed to the QEMU guest is not functioning on reboot. After closing the QEMU guest, the USB bus gets reset(?) after which the other keyboard suddenly gets detected. I had only booted 3.10.5 twice before rolling back to 3.9.9, both boots triggered the issue. Do I need to provide a usbmon, lsusb, dmesg and/ or other details from 3.10.5? Note that there are other Arch Linux users who have reported issues[1][2] since upgrading to 3.10.z. Triggering a re-enumeration by writing the magic HID++ message[3] makes the paired devices appear again (as reported in forums[1], I haven't tried this on the affected UHCI machine). While the underlying bug is fixed, can this patch be forwarded to stable? I see that 3.10.6 has been released, but still without this patch. Regards, Peter [1]: https://bbs.archlinux.org/viewtopic.php?id=167210 [2]: https://bugs.archlinux.org/task/35991 [3]: https://bbs.archlinux.org/viewtopic.php?pid=1309535#p1309535 -- 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 2/4] staging: ozwpan: Increment port number for new device.
On Mon, Aug 05, 2013 at 06:40:13PM +0100, Rupesh Gujare wrote: > This patch fixes crash issue when there is quick cycle of > de-enumeration & enumeration due to loss of wireless link. > > It is found that sometimes new device (or coming back device) > returns very fast, even before USB core read out hub status, > resulting in allocation of same port, which results in unstable > system & crash. > > Above issue is resolved by making sure that we always assign > new port to new device, making sure that USB core reads correct > hub status. > > Signed-off-by: Rupesh Gujare > --- > drivers/staging/ozwpan/ozhcd.c | 16 +++- > 1 file changed, 11 insertions(+), 5 deletions(-) No, don't paper over this, fix it correctly with reference counting as Dan has suggested. 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 1/2] ACPI / PCI: don't allow PCI devices without irq to be enabled
On Mon, Aug 12, 2013 at 3:32 PM, Yinghai Lu wrote: > On Mon, Aug 12, 2013 at 2:14 PM, Bjorn Helgaas wrote: >> On Thu, Aug 8, 2013 at 7:57 AM, Rafael J. Wysocki wrote: >>> On Thursday, August 08, 2013 03:35:13 PM Heikki Krogerus wrote: If there is no ACPI entry for the irq, returning error from acpi_pci_enable_irq() if the irq is 0. Prarit Bhargava reported an issue where he noticed that his Dell PowerEdge 840 has buggy BIOS that does not supply ACPI entries for irq with some devices. That lead into kernel generating a warning "genirq: Flags mismatch irq 0...". This will fix that issue. Signed-off-by: Heikki Krogerus Tested-by: Prarit Bhargava >>> >>> Hi Bjorn, >>> >>> Any objections to this? >> >> Acked-by: Bjorn Helgaas > > some USB3 host controller does not have intx configured, but still > work with MSI? Huh, OK, I take back my ack, at least while we investigate this. Bjorn -- 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 2/3] staging: dwc2: add NAK holdoff patch from downstream Pi kernel
> From: Matthijs Kooijman [mailto:matth...@stdin.nl] > Sent: Monday, August 12, 2013 12:36 AM > To: Paul Zimmerman > Cc: gre...@linuxfoundation.org; linux-usb@vger.kernel.org; > de...@linuxdriverproject.org; > swar...@wwwdotorg.org; gordon.hollingwo...@gmail.com; sk...@netbsd.org; Dom > Cobley > Subject: Re: [PATCH 2/3] staging: dwc2: add NAK holdoff patch from downstream > Pi kernel > > Hi Paul, > > > Add the NAK holdoff patch from the downstream Raspberry Pi kernel. > > This allows the transfer scheduler to better handle "cheeky devices > > that just hold off using NAKs". > > > @@ -365,6 +366,7 @@ struct dwc2_hsotg { > > u8 otg_port; > > u32 *frame_list; > > dma_addr_t frame_list_dma; > > + int next_sched_frame; > > This variable is still not really used, I think. Most of the mentions in > the patch are assignments, except for these two: > > > + if (list_empty(&hsotg->periodic_sched_inactive) || > > + dwc2_frame_num_le(qh->sched_frame, hsotg->next_sched_frame)) > > + hsotg->next_sched_frame = qh->sched_frame; > > + > ... > > + if (!dwc2_frame_num_le(hsotg->next_sched_frame, > > + qh->sched_frame)) > > + hsotg->next_sched_frame = > > + qh->sched_frame; > > However, these two "uses" of the variable have only the single purpose > of updating the variable itself, no other behaviour is influenced by it. > In effect, the variable does not influence the code at all and can be > dropped, significantly simplifying this patch. Yep, that's kind of embarrassing. I will have to go back and read the downstream driver code again to figure out how this code is supposed to work. Thanks for the review. -- Paul -- 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: host: add Kconfig option for EHSET
commit 9841f37a1c ("usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET") added additional code to the EHCI hub driver but it is anticipated to only have a limited audience (e.g. embedded silicon vendors and integrators). Avoid subjecting all EHCI (and in the future maybe xHCI/OHCI, etc.) HCD users to code bloat by conditionally compiling the EHSET-specific additions with a new Kconfig option, CONFIG_USB_HCD_TEST_MODE. Cc: Alan Stern Signed-off-by: Jack Pham --- drivers/usb/host/Kconfig| 15 +++ drivers/usb/host/ehci-hub.c |8 +++- drivers/usb/host/ehci-q.c |2 ++ 3 files changed, 24 insertions(+), 1 deletions(-) diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index cf521d6..b009368 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -722,3 +722,18 @@ config USB_HCD_SSB for ehci and ohci. If unsure, say N. + +config USB_HCD_TEST_MODE + bool "HCD test mode support" + ---help--- + Say 'Y' to enable additional software test modes that may be + supported by the host controller drivers. + + One such test mode is the Embedded High-speed Host Electrical Test + (EHSET) for EHCI host controller hardware, specifically the "Single + Step Set Feature" test. Typically this will be enabled for On-the-Go + or embedded hosts that need to undergo USB-IF compliance testing with + the aid of a special test fixture. In the future, this may expand to + include other tests that require support from a HCD driver. + + If unsure, say N. diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c index 9b222e4..d0edf53 100644 --- a/drivers/usb/host/ehci-hub.c +++ b/drivers/usb/host/ehci-hub.c @@ -711,6 +711,8 @@ ehci_hub_descriptor ( } /*-*/ +#ifdef CONFIG_USB_HCD_TEST_MODE + #define EHSET_TEST_SINGLE_STEP_SET_FEATURE 0x06 static void usb_ehset_completion(struct urb *urb) @@ -846,6 +848,7 @@ cleanup: kfree(buf); return retval; } +#endif /* CONFIG_USB_HCD_TEST_MODE */ /*-*/ static int ehci_hub_control ( @@ -1221,13 +1224,16 @@ static int ehci_hub_control ( * about the EHCI-specific stuff. */ case USB_PORT_FEAT_TEST: +#ifdef CONFIG_USB_HCD_TEST_MODE if (selector == EHSET_TEST_SINGLE_STEP_SET_FEATURE) { spin_unlock_irqrestore(&ehci->lock, flags); retval = ehset_single_step_set_feature(hcd, wIndex); spin_lock_irqsave(&ehci->lock, flags); break; - } else if (!selector || selector > 5) + } +#endif + if (!selector || selector > 5) goto error; spin_unlock_irqrestore(&ehci->lock, flags); ehci_quiesce(ehci); diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c index 702a0e9..c8d432f 100644 --- a/drivers/usb/host/ehci-q.c +++ b/drivers/usb/host/ehci-q.c @@ -1144,6 +1144,7 @@ submit_async ( } /*-*/ +#ifdef CONFIG_USB_HCD_TEST_MODE /* * This function creates the qtds and submits them for the * SINGLE_STEP_SET_FEATURE Test. @@ -1243,6 +1244,7 @@ cleanup: qtd_list_free(ehci, urb, head); return -1; } +#endif /* CONFIG_USB_HCD_TEST_MODE */ /*-*/ -- 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: [PATCH v3] usb: rh_call_control tbuf potential future overflow fix
> -Original Message- > From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org] > Sent: Monday, August 12, 2013 1:32 PM > To: Stalley, Sean > Cc: linux-usb@vger.kernel.org; Sarah Sharp; Ismail, Abdul R; Alan Stern > Subject: Re: [PATCH v3] usb: rh_call_control tbuf potential future overflow > fix > > On Mon, Aug 12, 2013 at 11:09:25AM -0700, Sean O. Stalley wrote: > > rh_call_control() contains a buffer, tbuf, which it uses to hold USB > > descriptors. These discriptors are eventually copied into the > > transfer_buffer in the URB. The buffer in the URB is dynamically > > defined and is always large enough to hold the amount of data it > > requests. > > > > tbuf is currently statically allocated on the stack with a size of 15 > > bytes, regardless of the size specified in the URB. > > This patch dynamically allocates tbuf, and ensures that tbuf is at > > least as big as the buffer in the URB. > > > > If an hcd attempts to write a descriptor containing more than > > 15 bytes ( such as the Standard BOS Descriptor for hubs, defined in > > the USB3.0 Spec, section 10.13.1 ) the write would overflow the buffer > > and corrupt the stack. This patch addresses this behavior. > > > > Acked-by: Alan Stern > > Signed-off-by: Sean O. Stalley > > --- > > drivers/usb/core/hcd.c | 24 +--- > > 1 file changed, 17 insertions(+), 7 deletions(-) > > Applying this patch produces a complier warning that shows the patch is > buggy: > > drivers/usb/core/hcd.c: In function ‘usb_hcd_submit_urb’: I should note that tbuf isn't in function 'usb_hcd_submit_urb', it is in the static function 'rh_call_control'. 'usb_hcd_submit_urb' calls 'rh_urb_enqueue', which calls 'rh_call_control' > drivers/usb/core/hcd.c:704:7: warning: ‘tbuf’ may be used uninitialized in > this > function [-Wmaybe-uninitialized] This line is kfree(tbuf); > drivers/usb/core/hcd.c:474:7: note: ‘tbuf’ was declared here I do not see how tbuf could not be initialized by the time it gets to 'kfree'. Unless 'kzalloc' fails, 'kzalloc' and 'kfree' are unconditional and sequential, so I'm not sure how you could get one without the other. All of the goto's in this function are below the 'kzalloc' and above 'kfree', so allocation (and release) cannot be jumped over. > > gcc is correct here, please fix this, and _never_ ignore complier warnings. If I set tbuf to NULL when I declare it, the warning goes away. Is this an acceptable solution? > > thanks, > > greg k-h Thanks, Sean O. Stalley
RTL8192CU module device HW:ID support
Hello ! Is it possible to add the "Hercules mini 300m v2" in the device list of rtl8192cu. The ID of this one is "0x06F8, 0xE035". 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 v3] usb: rh_call_control tbuf potential future overflow fix
On Mon, Aug 12, 2013 at 11:16:24PM +, Stalley, Sean wrote: > > > > -Original Message- > > From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org] > > Sent: Monday, August 12, 2013 1:32 PM > > To: Stalley, Sean > > Cc: linux-usb@vger.kernel.org; Sarah Sharp; Ismail, Abdul R; Alan Stern > > Subject: Re: [PATCH v3] usb: rh_call_control tbuf potential future overflow > > fix > > > > On Mon, Aug 12, 2013 at 11:09:25AM -0700, Sean O. Stalley wrote: > > > rh_call_control() contains a buffer, tbuf, which it uses to hold USB > > > descriptors. These discriptors are eventually copied into the > > > transfer_buffer in the URB. The buffer in the URB is dynamically > > > defined and is always large enough to hold the amount of data it > > > requests. > > > > > > tbuf is currently statically allocated on the stack with a size of 15 > > > bytes, regardless of the size specified in the URB. > > > This patch dynamically allocates tbuf, and ensures that tbuf is at > > > least as big as the buffer in the URB. > > > > > > If an hcd attempts to write a descriptor containing more than > > > 15 bytes ( such as the Standard BOS Descriptor for hubs, defined in > > > the USB3.0 Spec, section 10.13.1 ) the write would overflow the buffer > > > and corrupt the stack. This patch addresses this behavior. > > > > > > Acked-by: Alan Stern > > > Signed-off-by: Sean O. Stalley > > > --- > > > drivers/usb/core/hcd.c | 24 +--- > > > 1 file changed, 17 insertions(+), 7 deletions(-) > > > > Applying this patch produces a complier warning that shows the patch is > > buggy: > > > > drivers/usb/core/hcd.c: In function ‘usb_hcd_submit_urb’: > > I should note that tbuf isn't in function 'usb_hcd_submit_urb', it is > in the static function 'rh_call_control'. > 'usb_hcd_submit_urb' calls 'rh_urb_enqueue', which calls 'rh_call_control' Odd, the joys of inline functions? Very strange, but the warning is still correct. > > drivers/usb/core/hcd.c:704:7: warning: ‘tbuf’ may be used uninitialized in > > this > > function [-Wmaybe-uninitialized] > > This line is kfree(tbuf); > > > drivers/usb/core/hcd.c:474:7: note: ‘tbuf’ was declared here > > I do not see how tbuf could not be initialized by the time it gets to 'kfree'. Look closer... > Unless 'kzalloc' fails, 'kzalloc' and 'kfree' are unconditional and > sequential, so I'm not sure how you could get one without the other. > All of the goto's in this function are below the 'kzalloc' and above > 'kfree', so allocation (and release) cannot be jumped over. Look earlier in the function, you could jump to error, which would eventually call kfree(tbuf) with tbuf being undefined. > > gcc is correct here, please fix this, and _never_ ignore complier warnings. > > If I set tbuf to NULL when I declare it, the warning goes away. > Is this an acceptable solution? Yes, that's the correct solution. 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: RTL8192CU module device HW:ID support
On Tue, Aug 13, 2013 at 01:50:55AM +0200, srenau...@free.fr wrote: > Hello ! > Is it possible to add the "Hercules mini 300m v2" in the device list of > rtl8192cu. > The ID of this one is "0x06F8, 0xE035". Does the driver work with these new device ids? If so, great, can you send us a patch adding them? The instructions on how to do this are 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: RTL8192CU module device HW:ID support
On Mon, Aug 12, 2013 at 8:50 PM, wrote: > Hello ! > Is it possible to add the "Hercules mini 300m v2" in the device list of > rtl8192cu. > The ID of this one is "0x06F8, 0xE035". Does this work for you? diff --git a/drivers/net/wireless/rtlwifi/rtl8192cu/sw.c b/drivers/net/wireless/rtlwifi/rtl8192cu/sw.c index 2bd5985..89da62c 100644 --- a/drivers/net/wireless/rtlwifi/rtl8192cu/sw.c +++ b/drivers/net/wireless/rtlwifi/rtl8192cu/sw.c @@ -300,6 +300,7 @@ static struct usb_device_id rtl8192c_usb_ids[] = { {RTL_USB_DEVICE(0x050d, 0x1102, rtl92cu_hal_cfg)}, /*Belkin - Edimax*/ {RTL_USB_DEVICE(0x050d, 0x11f2, rtl92cu_hal_cfg)}, /*Belkin - ISY*/ {RTL_USB_DEVICE(0x06f8, 0xe033, rtl92cu_hal_cfg)}, /*Hercules - Edimax*/ + {RTL_USB_DEVICE(0x06f8, 0xe035, rtl92cu_hal_cfg)}, /*Hercules - Mini*/ {RTL_USB_DEVICE(0x07b8, 0x8188, rtl92cu_hal_cfg)}, /*Abocom - Abocom*/ {RTL_USB_DEVICE(0x07b8, 0x8189, rtl92cu_hal_cfg)}, /*Funai - Abocom*/ {RTL_USB_DEVICE(0x0846, 0x9041, rtl92cu_hal_cfg)}, /*NetGear WNA1000M*/ -- 1.8.1.2 -- 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: Non-enumerable devices on USB and other enumerable buses
On Mon, 12 Aug 2013, Mark Brown wrote: > On Mon, Aug 12, 2013 at 01:50:07PM -0700, Greg Kroah-Hartman wrote: > > On Mon, Aug 12, 2013 at 12:23:44PM +0100, Mark Brown wrote: > > > > I don't think they're bus specific - the main issue with a lot of this > > > is that they're outside the infrastructure that the bus standardises so > > > we should have a general way of understanding this. There will be bits > > > where the bus needs to get involved but the overall pattern is generic. > > > The "pattern" is generic, yes, we've been dealing with that for well > > over a decade now (pci hotplug controllers are "out of line" and control > > when/if a PCI is device is added/removed.) > > > I'd argue that each bus needs special logic to handle this, just like > > PCI hotplug did it with their hotplug controller logic. Due to the > > nature of this type of thing, it's all out-of-line hardware that is > > custom to each device type / bus. > > I agree that the bus will need some logic to handle this and I also > think Stephen is right that there are going to be some common patterns > for some device classes. However I do think that this is a common > problem for pretty much all buses so we should be factoring out as much > as possible from the buses so they only have to deal with their bit of > things. Most of the stuff that's worrying me is not anything the bus > should have any idea about beyond tunnelling the data to the device. In theory, the matching of device IDs can be done in the driver core, provided the IDs are assigned based on the bus topology (as is the case for USB). But what should happen when there's a match? The bus code would need hooks installed wherever the platform wants to do something extra. This could end up growing to a lot of hooks. How can the whole thing be done in a reasonable fashion? 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: DWC3 role switch cause IRQ request failed
> On Fri, Aug 09, 2013 at 03:16:20PM +, Wang, Yu Y wrote: > > > On Fri, Aug 09, 2013 at 01:34:09PM +, Wang, Yu Y wrote: > > > > > On Wed, Aug 07, 2013 at 12:03:34PM +, Wang, Yu Y wrote: > > > > > > Hi Balbi, > > > > > > > > > > > > Because dwc3 driver request_threaded_irq with flags > > > > > > IRQF_ONESHOT and IRQF_SHARED. > > > > > > But xHCI driver will not set IRQF_ONESHOT. Then will met IRQ > > > > > > request failed if use same IRQ number. > > > > > > > > > > > > > > > > > > <4>[1.019248] Call Trace: > > > > > > <4>[1.019270] [] dump_stack+0x16/0x18 > > > > > > <4>[1.019292] [] __schedule_bug+0x65/0x77 > > > > > > <4>[1.019313] [] __schedule+0x7ad/0x830 > > > > > > <4>[1.019335] [] ? rest_init+0xc0/0xc0 > > > > > > <4>[1.019358] [] ? printk_address+0x32/0x40 > > > > > > <4>[1.019380] [] ? > __module_text_address+0x10/0x60 > > > > > > <4>[1.019402] [] ? > is_module_text_address+0x2b/0x50 > > > > > > <4>[1.019424] [] ? > __kernel_text_address+0x4f/0x70 > > > > > > <4>[1.019444] [] schedule+0x23/0x60 > > > > > > <4>[1.019466] [] schedule_timeout+0x125/0x1f0 > > > > > > <4>[1.019488] [] ? wait_for_completion+0x2b/0xe0 > > > > > > <4>[1.019509] [] ? wait_for_completion+0x98/0xe0 > > > > > > <4>[1.019530] [] wait_for_completion+0x9f/0xe0 > > > > > > <4>[1.019551] [] ? try_to_wake_up+0x280/0x280 > > > > > > <4>[1.019572] [] kthread_stop+0x41/0x100 > > > > > > <4>[1.019595] [] __setup_irq+0xd6/0x460 > > > > > > <4>[1.019617] [] ? > > > > > dwc3_gadget_set_selfpowered+0x60/0x60 > > > > > > <4>[1.019639] [] > request_threaded_irq+0xa2/0x120 > > > > > > <4>[1.019660] [] ? > > > > > dwc3_gadget_reset_interrupt+0x1a0/0x1a0 > > > > > > <4>[1.019681] [] dwc3_gadget_start+0x1a4/0x1f0 > > > > > > <4>[1.019703] [] udc_bind_to_driver+0x57/0x100 > > > > > > <4>[1.019724] [] > usb_gadget_probe_driver+0xa1/0xe0 > > > > > > <4>[1.019745] [] ? device_create_file+0x38/0xb0 > > > > > > <4>[1.019766] [] usb_composite_probe+0x6f/0x90 > > > > > > <4>[1.019788] [] init+0x147/0x19f > > > > > > <4>[1.019809] [] ? acmmod_init+0xf/0xf > > > > > > <4>[1.019829] [] do_one_initcall+0xba/0x170 > > > > > > <4>[1.019853] [] kernel_init_freeable+0x119/0x1b8 > > > > > > <4>[1.019875] [] ? do_early_param+0x7a/0x7a > > > > > > <4>[1.019896] [] kernel_init+0x10/0xd0 > > > > > > <4>[1.019917] [] > ret_from_kernel_thread+0x1b/0x28 > > > > > > <4>[1.019937] [] ? rest_init+0xc0/0xc0 > > > > > > <3>[1.020082] dwc3 dwc3.0.auto: failed to request irq #34 --> > > > > > > -16 > > > > > > > > > > > > This is one known issue or new issue? > > > > > > > > > > I have already removed IRQF_ONESHOT, it'll be on v3.12 > > > > > > > > > > > > > [Yu:] Thanks balbi. Can you please share the patch link? > > > > > > http://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?h= > > > next&id=e8 > > > adfc30ff9282a728fd8b666b6418308164c415 > > > > > > as usual, it's in my 'next' branch. you really need to fix your > > > mailer btw, I got many copies of this same message. > > > > [Yu:] Thanks balbi. Don't know the reason. I send the email to you, > > and always get send failed response. So I tried more times. Sorry about > > that. > > > > BTW, Where can I find your git server link? I want to clone it to get latest > code base. > > on git.kernel.org, in the link above, click on 'summary' and scroll down to > the > end of the page. > [Yu:] Get it. Thanks. > -- > balbi -- 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
Bug 60738 - USB 3.0 connection delay seems to be too short for HP USB 3.0 hard drive
Hi, As requested by Greg Kroah, please see bug 60738 - Summary: USB 3.0 connection delay seems to be too short for HP USB 3.0 hard drive. I'll be pleased to help you as much as possible. -- Alexandre Demers -- 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 60738 - USB 3.0 connection delay seems to be too short for HP USB 3.0 hard drive
On Tue, Aug 13, 2013 at 12:26:25AM -0400, Alexandre Demers wrote: > Hi, > > As requested by Greg Kroah, please see bug 60738 - Summary: USB 3.0 > connection delay seems to be too short for HP USB 3.0 hard drive. Please just post the information you posted there (at the least, a url would be good to have, some of us deal with multiple bug tracking databases, and we don't know which one you are pointing to...) Email on the mailing list is the best way to handle bugs for the USB subsystem at this point in time. 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: Bug 60738 - USB 3.0 connection delay seems to be too short for HP USB 3.0 hard drive
Sorry, here are more details. I'm refering to https://bugzilla.kernel.org/show_bug.cgi?id=60738 To sum up: Most of the time, my HP USB 3.0 hard drive will not be properly mounted when booting or when connecting it. I usually end up with something similar to the following in dmesg: [ 204.081693] usb usb9: usb wakeup-resume [ 204.081700] usb usb9: usb auto-resume [ 204.081711] hub 9-0:1.0: hub_resume [ 204.081722] hub 9-0:1.0: port 1: status 02c1 change 0001 [ 204.090114] usb usb8: usb wakeup-resume [ 204.090119] usb usb8: usb auto-resume [ 204.090131] hub 8-0:1.0: hub_resume [ 204.090140] hub 8-0:1.0: port 1: status 0101 change 0001 [ 204.182172] hub 9-0:1.0: state 7 ports 2 chg 0002 evt [ 204.182192] hub 9-0:1.0: port 1, status 02a0, change 0001, 5.0 Gb/s [ 204.286239] hub 9-0:1.0: debounce: port 1: total 100ms stable 100ms status 0x2a0 [ 204.286250] hub 8-0:1.0: state 7 ports 2 chg 0002 evt 0002 [ 204.286262] hub 8-0:1.0: port 1, status 0101, change 0001, 12 Mb/s [ 204.286285] hub 9-0:1.0: hub_suspend [ 204.286291] usb usb9: bus auto-suspend, wakeup 1 [ 204.390320] hub 8-0:1.0: debounce: port 1: total 100ms stable 100ms status 0x101 [ 204.492391] usb 8-1: new high-speed USB device number 8 using xhci_hcd [ 204.697607] usb 8-1: Device not responding to set address. [ 204.898877] usb 8-1: Device not responding to set address. [ 205.099841] usb 8-1: device not accepting address 8, error -71 [ 205.099868] kobject: '8-1' (88020d7fe098): kobject_cleanup [ 205.099870] kobject: '8-1' (88020d7fe098): calling ktype release [ 205.099883] kobject: '8-1': free name [ 205.150907] kobject: '8-1' (88020a605898): kobject_cleanup [ 205.150918] kobject: '8-1' (88020a605898): calling ktype release [ 205.150921] kobject: '8-1': free name [ 205.150929] hub 8-0:1.0: state 7 ports 2 chg evt 0002 [ 205.150939] hub 8-0:1.0: port 1, status 0100, change 0001, 12 Mb/s [ 205.254964] hub 8-0:1.0: debounce: port 1: total 100ms stable 100ms status 0x100 [ 205.254978] hub 8-0:1.0: hub_suspend [ 205.254983] usb usb8: bus auto-suspend, wakeup 1 To make it work, when the USB drive is already plugged in, if I can disconnect it and reconnect it in the USB 3.0 port fast enough, it works most of the time. This problem is not seen under Windows. According to Windows Hardware Certification Requirements (part 1): "Next, the time between the time that RxDetect succeeds and the time that the device enters U0 can be up to 300 ms. With an extra margin for hub control transfers and other delays, the total comes to 500 ms." Also, when the hard drive is plugged in a USB 2.0 port, it works like a charm. Are we too aggressive when we are "waking up" the device? Alexandre Demers On 08/13/2013 12:26 AM, Alexandre Demers wrote: > Hi, > > As requested by Greg Kroah, please see bug 60738 - Summary: USB 3.0 connection delay seems to be too short for HP USB 3.0 hard drive. > > I'll be pleased to help you as much as possible. -- 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/2] usb: core: don't try to reset_device() a port that got just disconnected
On Tue, Aug 06, 2013 at 08:40:56PM -0700, Sarah Sharp wrote: > On Tue, Aug 06, 2013 at 06:04:36AM +0800, Greg Kroah-Hartman wrote: > > On Mon, Aug 05, 2013 at 09:46:46AM -0700, Sarah Sharp wrote: > > > I wanted to test it for a kernel release to make sure it didn't cause > > > any issues before sending it off to stable. I'm sure it fixes Julius' > > > issues on his Intel xHCI host, but I want to make sure it doesn't cause > > > issues on other host controllers. > > > > Ok, so you will remember to send it to stable@ after a bit? > > Yes. It's in my todo list. No need to, I've picked it up now, 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: Regression: USB memory stick stalls and no longer automounts under 3.11-rc1
On Mon, Aug 05, 2013 at 09:07:51PM -0400, Alan Stern wrote: > On Tue, 6 Aug 2013, Greg KH wrote: > > > > Have you tried this patch? > > > > > > http://marc.info/?l=linux-usb&m=137523956310060&w=2 > > > > What is the status of this patch, I'm starting to get lots of complaints > > about this :( > > James Bottomley has merged it into his Fixes branch, but I don't think > he has sent it to Linus yet. It's now merged. 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: Bug 60738 - USB 3.0 connection delay seems to be too short for HP USB 3.0 hard drive
Hi Alexandre, I'm new to kernel development. I want to explore this bug. From you dmesg output I'm unable to get the driver your device is using. can you tell me which driver are you using? regards Kumar Gaurav On Tuesday 13 August 2013 11:15 AM, Alexandre Demers wrote: Sorry, here are more details. I'm refering to https://bugzilla.kernel.org/show_bug.cgi?id=60738 To sum up: Most of the time, my HP USB 3.0 hard drive will not be properly mounted when booting or when connecting it. I usually end up with something similar to the following in dmesg: [ 204.081693] usb usb9: usb wakeup-resume [ 204.081700] usb usb9: usb auto-resume [ 204.081711] hub 9-0:1.0: hub_resume [ 204.081722] hub 9-0:1.0: port 1: status 02c1 change 0001 [ 204.090114] usb usb8: usb wakeup-resume [ 204.090119] usb usb8: usb auto-resume [ 204.090131] hub 8-0:1.0: hub_resume [ 204.090140] hub 8-0:1.0: port 1: status 0101 change 0001 [ 204.182172] hub 9-0:1.0: state 7 ports 2 chg 0002 evt [ 204.182192] hub 9-0:1.0: port 1, status 02a0, change 0001, 5.0 Gb/s [ 204.286239] hub 9-0:1.0: debounce: port 1: total 100ms stable 100ms status 0x2a0 [ 204.286250] hub 8-0:1.0: state 7 ports 2 chg 0002 evt 0002 [ 204.286262] hub 8-0:1.0: port 1, status 0101, change 0001, 12 Mb/s [ 204.286285] hub 9-0:1.0: hub_suspend [ 204.286291] usb usb9: bus auto-suspend, wakeup 1 [ 204.390320] hub 8-0:1.0: debounce: port 1: total 100ms stable 100ms status 0x101 [ 204.492391] usb 8-1: new high-speed USB device number 8 using xhci_hcd [ 204.697607] usb 8-1: Device not responding to set address. [ 204.898877] usb 8-1: Device not responding to set address. [ 205.099841] usb 8-1: device not accepting address 8, error -71 [ 205.099868] kobject: '8-1' (88020d7fe098): kobject_cleanup [ 205.099870] kobject: '8-1' (88020d7fe098): calling ktype release [ 205.099883] kobject: '8-1': free name [ 205.150907] kobject: '8-1' (88020a605898): kobject_cleanup [ 205.150918] kobject: '8-1' (88020a605898): calling ktype release [ 205.150921] kobject: '8-1': free name [ 205.150929] hub 8-0:1.0: state 7 ports 2 chg evt 0002 [ 205.150939] hub 8-0:1.0: port 1, status 0100, change 0001, 12 Mb/s [ 205.254964] hub 8-0:1.0: debounce: port 1: total 100ms stable 100ms status 0x100 [ 205.254978] hub 8-0:1.0: hub_suspend [ 205.254983] usb usb8: bus auto-suspend, wakeup 1 To make it work, when the USB drive is already plugged in, if I can disconnect it and reconnect it in the USB 3.0 port fast enough, it works most of the time. This problem is not seen under Windows. According to Windows Hardware Certification Requirements (part 1): "Next, the time between the time that RxDetect succeeds and the time that the device enters U0 can be up to 300 ms. With an extra margin for hub control transfers and other delays, the total comes to 500 ms." Also, when the hard drive is plugged in a USB 2.0 port, it works like a charm. Are we too aggressive when we are "waking up" the device? Alexandre Demers On 08/13/2013 12:26 AM, Alexandre Demers wrote: > Hi, > > As requested by Greg Kroah, please see bug 60738 - Summary: USB 3.0 connection delay seems to be too short for HP USB 3.0 hard drive. > > I'll be pleased to help you as much as possible. -- 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
Re: Bug 60738 - USB 3.0 connection delay seems to be too short for HP USB 3.0 hard drive
lsusb gives me the following: sudo lsusb --verbose -t /: Bus 13.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 5000M /: Bus 12.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 480M /: Bus 11.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 5000M /: Bus 10.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 480M /: Bus 09.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 5000M |__ Port 1: Dev 2, If 0, Class=Mass Storage, Driver=usb-storage, 5000M /: Bus 08.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 480M /: Bus 07.Port 1: Dev 1, Class=root_hub, Driver=ohci_hcd/4p, 12M |__ Port 1: Dev 2, If 0, Class=Human Interface Device, Driver=usbhid, 12M |__ Port 1: Dev 2, If 1, Class=Human Interface Device, Driver=usbhid, 12M |__ Port 1: Dev 2, If 2, Class=Human Interface Device, Driver=usbhid, 12M /: Bus 06.Port 1: Dev 1, Class=root_hub, Driver=ohci_hcd/2p, 12M /: Bus 05.Port 1: Dev 1, Class=root_hub, Driver=ohci_hcd/5p, 12M |__ Port 4: Dev 2, If 0, Class=Human Interface Device, Driver=usbhid, 1.5M /: Bus 04.Port 1: Dev 1, Class=root_hub, Driver=ohci_hcd/5p, 12M /: Bus 03.Port 1: Dev 1, Class=root_hub, Driver=ehci-pci/4p, 480M /: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=ehci-pci/5p, 480M /: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=ehci-pci/5p, 480M |__ Port 1: Dev 2, If 0, Class=Vendor Specific Class, Driver=r8712u, 480M So I assume it is a combinaision of usb-storage over xhci_hcd (the hp hard drive is working right now and is on Bus 09). Alex On Tue 13 Aug 2013 02:23:50 AM EDT, Kumar Gaurav wrote: Hi Alexandre, I'm new to kernel development. I want to explore this bug. From you dmesg output I'm unable to get the driver your device is using. can you tell me which driver are you using? regards Kumar Gaurav On Tuesday 13 August 2013 11:15 AM, Alexandre Demers wrote: Sorry, here are more details. I'm refering to https://bugzilla.kernel.org/show_bug.cgi?id=60738 To sum up: Most of the time, my HP USB 3.0 hard drive will not be properly mounted when booting or when connecting it. I usually end up with something similar to the following in dmesg: [ 204.081693] usb usb9: usb wakeup-resume [ 204.081700] usb usb9: usb auto-resume [ 204.081711] hub 9-0:1.0: hub_resume [ 204.081722] hub 9-0:1.0: port 1: status 02c1 change 0001 [ 204.090114] usb usb8: usb wakeup-resume [ 204.090119] usb usb8: usb auto-resume [ 204.090131] hub 8-0:1.0: hub_resume [ 204.090140] hub 8-0:1.0: port 1: status 0101 change 0001 [ 204.182172] hub 9-0:1.0: state 7 ports 2 chg 0002 evt [ 204.182192] hub 9-0:1.0: port 1, status 02a0, change 0001, 5.0 Gb/s [ 204.286239] hub 9-0:1.0: debounce: port 1: total 100ms stable 100ms status 0x2a0 [ 204.286250] hub 8-0:1.0: state 7 ports 2 chg 0002 evt 0002 [ 204.286262] hub 8-0:1.0: port 1, status 0101, change 0001, 12 Mb/s [ 204.286285] hub 9-0:1.0: hub_suspend [ 204.286291] usb usb9: bus auto-suspend, wakeup 1 [ 204.390320] hub 8-0:1.0: debounce: port 1: total 100ms stable 100ms status 0x101 [ 204.492391] usb 8-1: new high-speed USB device number 8 using xhci_hcd [ 204.697607] usb 8-1: Device not responding to set address. [ 204.898877] usb 8-1: Device not responding to set address. [ 205.099841] usb 8-1: device not accepting address 8, error -71 [ 205.099868] kobject: '8-1' (88020d7fe098): kobject_cleanup [ 205.099870] kobject: '8-1' (88020d7fe098): calling ktype release [ 205.099883] kobject: '8-1': free name [ 205.150907] kobject: '8-1' (88020a605898): kobject_cleanup [ 205.150918] kobject: '8-1' (88020a605898): calling ktype release [ 205.150921] kobject: '8-1': free name [ 205.150929] hub 8-0:1.0: state 7 ports 2 chg evt 0002 [ 205.150939] hub 8-0:1.0: port 1, status 0100, change 0001, 12 Mb/s [ 205.254964] hub 8-0:1.0: debounce: port 1: total 100ms stable 100ms status 0x100 [ 205.254978] hub 8-0:1.0: hub_suspend [ 205.254983] usb usb8: bus auto-suspend, wakeup 1 To make it work, when the USB drive is already plugged in, if I can disconnect it and reconnect it in the USB 3.0 port fast enough, it works most of the time. This problem is not seen under Windows. According to Windows Hardware Certification Requirements (part 1): "Next, the time between the time that RxDetect succeeds and the time that the device enters U0 can be up to 300 ms. With an extra margin for hub control transfers and other delays, the total comes to 500 ms." Also, when the hard drive is plugged in a USB 2.0 port, it works like a charm. Are we too aggressive when we are "waking up" the device? Alexandre Demers On 08/13/2013 12:26 AM, Alexandre Demers wrote: > Hi, > > As requested by Greg Kroah, please see bug 60738 - Summary: USB 3.0 connection delay seems to be too short for HP USB 3.0 hard drive. > > I'll be pleased to help you as much as possible. -- 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 -- Alexandre Demers -- To uns