[PATCH] OHCI: add a quirk for ULi M5237 blocking on reset
From: Arseny Solokha
Commit 8dccddbc2368 ("OHCI: final fix for NVIDIA problems (I hope)")
introduced into 3.1.9 broke boot on e.g. Freescale P2020DS development
board. The code path that was previously specific to NVIDIA controllers
had then become taken for all chips.
However, the M5237 installed on the board wedges solid when accessing
its base+OHCI_FMINTERVAL register, making it impossible to boot any
kernel newer than 3.1.8 on this particular and apparently other similar
machines.
Don't readl() and writel() base+OHCI_FMINTERVAL on PCI ID 10b9:5237.
The patch is suitable for the -next tree as well as all maintained
kernels up to 3.2 inclusive.
Signed-off-by: Arseny Solokha
---
drivers/usb/host/pci-quirks.c | 14 +++---
include/linux/pci_ids.h | 1 +
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 2f3aceb..3b29478 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -571,7 +571,7 @@ static void quirk_usb_handoff_ohci(struct pci_dev *pdev)
{
void __iomem *base;
u32 control;
- u32 fminterval;
+ u32 fminterval = 0;
int cnt;
if (!mmio_resource_enabled(pdev, 0))
@@ -619,7 +619,13 @@ static void quirk_usb_handoff_ohci(struct pci_dev *pdev)
}
/* software reset of the controller, preserving HcFmInterval */
- fminterval = readl(base + OHCI_FMINTERVAL);
+ if (!(pdev->vendor == PCI_VENDOR_ID_AL &&
+ pdev->device == PCI_DEVICE_ID_AL_M5237)) {
+ /* ULi M5237 OHCI controller (10b9:5237) locks the whole system
+* when accessing the OHCI_FMINTERVAL offset.
+*/
+ fminterval = readl(base + OHCI_FMINTERVAL);
+ }
writel(OHCI_HCR, base + OHCI_CMDSTATUS);
/* reset requires max 10 us delay */
@@ -628,7 +634,9 @@ static void quirk_usb_handoff_ohci(struct pci_dev *pdev)
break;
udelay(1);
}
- writel(fminterval, base + OHCI_FMINTERVAL);
+ if (!(pdev->vendor == PCI_VENDOR_ID_AL &&
+ pdev->device == PCI_DEVICE_ID_AL_M5237))
+ writel(fminterval, base + OHCI_FMINTERVAL);
/* Now the controller is safely in SUSPEND and nothing can wake it up */
iounmap(base);
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 1fa99a3..266fc5c 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -1107,6 +1107,7 @@
#define PCI_DEVICE_ID_AL_M5219 0x5219
#define PCI_DEVICE_ID_AL_M5228 0x5228
#define PCI_DEVICE_ID_AL_M5229 0x5229
+#define PCI_DEVICE_ID_AL_M5237 0x5237
#define PCI_DEVICE_ID_AL_M5451 0x5451
#define PCI_DEVICE_ID_AL_M7101 0x7101
--
2.2.0
--
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: Is this 32-bit NCM?y
Kevin Zhu writes: > Regarding the location of NDP, it should be easy to fix. It can be added > to the end of the NTB only after it's ready to send. Yes, but this will require a bit of redesign. Note that this code is shared with cdc_mbim, which might have to add multiple NDPs for multiplexed sessions. In theory up to 512 different, but that is so unlinkely that we can ignore it. We could set a fixed limit significantly lower than that if necessary. I'd really like to refactor this code to simply queue the skbs, creating the linear NTB only when the queue is flushed. Then we'll have all the info we need and can order the contents any way we want without adding unnecessary padding. > Regarding the > concern to other devices, as there's a particular driver for Huawei > devices in kernel, which is huawei_cdc_ncm, maybe we can just fix the TX > function there to avoid breaking other devices. Sure. That is definitely an alternative if we don't want to touch the generic driver. But making the generic driver flexible enough to accommodate any device would be preferable. I do hope we can do that. Bjørn -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Is this 32-bit NCM?y
Yes Bjorn: I should say that's true. and - in general, touching the device driver to make it flexible is surely important, especially to make it more easy to fix / "expand" in the future. I was referring to modifying the huawei_cdc_ncm.c driver only for specific huawei workarounds / firmware misbehaviours. thank to everyone, Enrico -- 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: USB 3.0 drive crashes system when plugged in - regression
Hi, On 12/05/2014 12:35 AM, Marcin Zajączkowski wrote: On 2014-12-04 20:21, Hans de Goede wrote: Hi, On 12/03/2014 10:22 PM, Marcin Zajączkowski wrote: On 2014-12-03 10:53, Oliver Neukum wrote: On Wed, 2014-12-03 at 10:41 +0100, Marcin Zajączkowski wrote: 2014-12-03 Oliver Neukum wrote: On Wed, 2014-12-03 at 00:29 +0100, Marcin Zajączkowski wrote: Hi, After upgrade to Fedora 21 with 3.17.3-300.fc21.x86_64 (from 3.14.x in Fedora 19) my USB 3.0 drive (Seagate Backup+ 1TB) stopped working with USB 3.0 port (works fine with USB 2.0 port). When plug in for the first time (USB 3.0 port) I see in log: kernel: xhci_hcd :04:00.0: ERROR Transfer event for disabled endpoint or incorrect stream ring kernel: xhci_hcd :04:00.0: @000241eec570 11979000 0002 0500 01078001 Are you using the UAS driver? Probably yes. How can check that and/or switch to not UAS driver for a test? Sysfs holds the information. I can be comfortably queried with the "usb-devices" script. To switch drivers you can use the "no UAS" quirk which can be given as a module parameter like other quirks. Thanks Olivier for your reply. UAS keyword helped me to ignore UAS for my drive (via `options usb-storage quirks=vendorId:productId:u`) and my drive can again be used via USB 3.0 port. (...) What would you propose to do with the issue with that Seagate drive (family?)? To either track it or to temporary disable UAS for them globally (to not produce system crashes). If you have a seagate driver you may need a US_FL_NO_ATA_1X quirk to disable ata pass through commands, as all (current) seagate devices How can I do that from the modprobe level? With the quirk line I've provided below (and you've already tested). seem to bork on this. What is the usb-id of your seagate device ? idVendor=0bc2, idProduct=a013 Hmm, that one does not yet have the US_FL_NO_ATA_1X quirk most seagate devices seem to need. That is not the problem in your case (probing does not get so far that it matters), but this does seem to be one of the devices which need one looking at the model-name and usb-id. So I'll add a quirk for it to help out other users. And can you try with options usb-storage quirks=vendorId:productId:t ? I tried "quirks=vendorId:productId:t" and it failed. The system reported errors when a drive was connected: Ok, I already suspected as much, since this seems to be an xhci controller problem, not an uas problem. 21:37:32 kernel: usb 4-1: new SuperSpeed USB device number 2 using xhci_hcd 21:37:32 kernel: usb 4-1: New USB device found, idVendor=0bc2, idProduct=a013 21:37:32 kernel: usb 4-1: New USB device strings: Mfr=2, Product=3, SerialNumber=1 21:37:32 kernel: usb 4-1: Product: Backup+ BK 21:37:32 kernel: usb 4-1: Manufacturer: Seagate 21:37:32 kernel: usb 4-1: SerialNumber: 21:37:33 mtp-probe[2777]: checking bus 4, device 2: "/sys/devices/pci:00/:00:1c.3/:04:00.0/usb4/4-1" 21:37:33 mtp-probe[2777]: bus: 4, device: 2 was not an MTP device 21:37:33 kernel: usbcore: registered new interface driver usb-storage 21:37:33 kernel: scsi host6: uas 21:37:33 kernel: usbcore: registered new interface driver uas 21:37:33 kernel: xhci_hcd :04:00.0: ERROR Transfer event for disabled endpoint or incorrect stream ring 21:37:33 kernel: xhci_hcd :04:00.0: @368b3570 9067b000 0500 01078001 21:37:33 kernel: xhci_hcd :04:00.0: ERROR Transfer event for disabled endpoint or incorrect stream ring 21:37:33 kernel: xhci_hcd :04:00.0: @368b3580 9067b400 0500 01038001 21:37:53 kernel: scsi 6:0:0:0: uas_eh_abort_handler 88003653ee80 tag -1, inflight: CMD IN 21:37:53 kernel: xhci_hcd :04:00.0: WARN waiting for error on ep to be cleared 21:37:53 kernel: scsi 6:0:0:0: uas_submit_sense_urb 88003653ee80 tag -1, inflight: CMD IN abort 21:37:53 kernel: scsi host6: sense urb submission error -22 stream 32 21:37:53 kernel: scsi host6: uas_eh_task_mgmt: ABORT TASK: submit sense urb failed 21:37:53 kernel: scsi 6:0:0:0: uas_eh_device_reset_handler 21:37:53 kernel: xhci_hcd :04:00.0: WARN waiting for error on ep to be cleared 21:37:53 kernel: scsi 6:0:0:0: uas_submit_sense_urb 88003653ee80 tag -1, inflight: CMD IN abort 21:37:53 kernel: scsi host6: sense urb submission error -22 stream 32 21:37:53 kernel: scsi host6: uas_eh_task_mgmt: LOGICAL UNIT RESET: submit sense urb failed 21:37:53 kernel: scsi host6: uas_eh_bus_reset_handler start 21:37:53 kernel: usb 4-1: stat urb: killed, stream 1 21:37:53 kernel: scsi 6:0:0:0: uas_data_cmplt 88003653ee80 tag -1, inflight: CMD abort 21:37:53 kernel: scsi 6:0:0:0: data cmplt err -2 stream 1 21:37:53 kernel: scsi 6:0:0:0: uas_zap_dead 88003653ee80 tag -1, inflight: CMD abort 21:37:53 kernel: scsi 6:0:0:0: abort completed 21:37:54 kernel: usb 4-1: reset SuperSpeed USB device number 2 using xhci_hcd 21:37:54 kernel: xhci_hcd :04:00.0: xHCI xhci_drop_endpoint called with disabled
[PATCH 1/2] xhci: Add broken-streams quirk for Fresco Logic FL1000G xhci controllers
Streams do not work reliabe on Fresco Logic FL1000G xhci controllers, trying to use them results in errors like this: 21:37:33 kernel: xhci_hcd :04:00.0: ERROR Transfer event for disabled endpoint or incorrect stream ring 21:37:33 kernel: xhci_hcd :04:00.0: @368b3570 9067b000 0500 01078001 21:37:33 kernel: xhci_hcd :04:00.0: ERROR Transfer event for disabled endpoint or incorrect stream ring 21:37:33 kernel: xhci_hcd :04:00.0: @368b3580 9067b400 0500 01038001 As always I've ordered a pci-e addon card with a Fresco Logic controller for myself to see if I can come up with a better fix then the big hammer, in the mean time this will make uas devices work again (in usb-storage mode) for FL1000G users. Reported-by: Marcin Zajączkowski Cc: sta...@vger.kernel.org # 3.15 Signed-off-by: Hans de Goede --- drivers/usb/host/xhci-pci.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index 9a69b1f..e1071ce 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -82,6 +82,8 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci) "must be suspended extra slowly", pdev->revision); } + if (pdev->device == PCI_DEVICE_ID_FRESCO_LOGIC_PDK) + xhci->quirks |= XHCI_BROKEN_STREAMS; /* Fresco Logic confirms: all revisions of this chip do not * support MSI, even though some of them claim to in their PCI * capabilities. -- 2.1.0 -- 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 2/2] uas: Add US_FL_NO_ATA_1X for Seagate devices with usb-id 0bc2:a013
This is yet another Seagate device which needs the US_FL_NO_ATA_1X quirk Reported-by: Marcin Zajączkowski Cc: sta...@vger.kernel.org # 3.16 Signed-off-by: Hans de Goede --- drivers/usb/storage/unusual_uas.h | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/usb/storage/unusual_uas.h b/drivers/usb/storage/unusual_uas.h index 18a283d..2918376 100644 --- a/drivers/usb/storage/unusual_uas.h +++ b/drivers/usb/storage/unusual_uas.h @@ -68,6 +68,13 @@ UNUSUAL_DEV(0x0bc2, 0xa003, 0x, 0x, USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_NO_ATA_1X), +/* Reported-by: Marcin Zajączkowski */ +UNUSUAL_DEV(0x0bc2, 0xa013, 0x, 0x, + "Seagate", + "Backup Plus", + USB_SC_DEVICE, USB_PR_DEVICE, NULL, + US_FL_NO_ATA_1X), + /* https://bbs.archlinux.org/viewtopic.php?id=183190 */ UNUSUAL_DEV(0x0bc2, 0xab20, 0x, 0x, "Seagate", -- 2.1.0 -- 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] mfd: dln2: add suspend/resume functionality
On Thu, Nov 27, 2014 at 01:57:14PM +0200, Octavian Purdila wrote:
> @@ -753,11 +759,42 @@ static const struct usb_device_id dln2_table[] = {
>
> MODULE_DEVICE_TABLE(usb, dln2_table);
Place the new callbacks above the device id table.
> +static int dln2_suspend(struct usb_interface *iface, pm_message_t message)
> +{
> + struct dln2_dev *dln2 = usb_get_intfdata(iface);
> +
> + dln2_stop(dln2);
You should also stop the reads urbs here.
> + return 0;
> +}
> +
> +static int dln2_resume(struct usb_interface *iface)
> +{
> + struct dln2_dev *dln2 = usb_get_intfdata(iface);
> +
> + dln2->disconnect = false;
And surely you need to resubmit the read urbs in resume, or you will
never receive any more data.
How did you test this patch?
> + return 0;
> +}
> +
> +static int dln2_reset_resume(struct usb_interface *iface)
> +{
> + struct dln2_dev *dln2 = usb_get_intfdata(iface);
> + int ret;
> +
> + dln2_free_rx_urbs(dln2);
> + ret = dln2_setup_rx_urbs(dln2, iface->cur_altsetting);
This doesn't make much sense. Why would you ever want to reallocate the
urbs and their buffers here?
If the device does not lose its state as you claim, then all you need to
do is to resubmit the read urbs (as in resume).
Johan
--
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] xhci: Add broken-streams quirk for Fresco Logic FL1000G xhci controllers
Hello. On 12/5/2014 1:11 PM, Hans de Goede wrote: Streams do not work reliabe on Fresco Logic FL1000G xhci controllers, s/reliabe/reliably/? trying to use them results in errors like this: 21:37:33 kernel: xhci_hcd :04:00.0: ERROR Transfer event for disabled endpoint or incorrect stream ring 21:37:33 kernel: xhci_hcd :04:00.0: @368b3570 9067b000 0500 01078001 21:37:33 kernel: xhci_hcd :04:00.0: ERROR Transfer event for disabled endpoint or incorrect stream ring 21:37:33 kernel: xhci_hcd :04:00.0: @368b3580 9067b400 0500 01038001 As always I've ordered a pci-e addon card with a Fresco Logic controller for myself to see if I can come up with a better fix then the big hammer, in the mean time this will make uas devices work again (in usb-storage mode) for FL1000G users. Reported-by: Marcin Zajączkowski Cc: sta...@vger.kernel.org # 3.15 Signed-off-by: Hans de Goede WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: f_mass_storage: restore address range on exit
At the start of the thread we are changing the address limit, restoring it
to the default while exiting.
Signed-off-by: Sanjay Singh Rawat
---
drivers/usb/gadget/function/f_mass_storage.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/usb/gadget/function/f_mass_storage.c
b/drivers/usb/gadget/function/f_mass_storage.c
index 811929c..c4a2ded 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -2489,6 +2489,7 @@ static void handle_exception(struct fsg_common *common)
static int fsg_main_thread(void *common_)
{
struct fsg_common *common = common_;
+ mm_segment_t fs = get_fs();
/*
* Allow the thread to be killed by a signal, but set the signal mask
@@ -2567,6 +2568,7 @@ static int fsg_main_thread(void *common_)
up_write(&common->filesem);
}
+ set_fs(fs);
/* Let fsg_unbind() know the thread has exited */
complete_and_exit(&common->thread_notifier, 0);
}
--
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] USB: mos7720: delete some unneeded code
On Sat, Nov 29, 2014 at 03:49:54PM +0300, Dan Carpenter wrote: > The "status" is uninitialized so this creates a static checker warning. > But it's harmless, we can just delete it. > > Signed-off-by: Dan Carpenter Thanks, Dan. I'll queue it up and apply once 3.19-rc1 is out. Johan -- 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] mfd: dln2: add suspend/resume functionality
On Fri, Dec 5, 2014 at 12:17 PM, Johan Hovold wrote:
> On Thu, Nov 27, 2014 at 01:57:14PM +0200, Octavian Purdila wrote:
>
>> @@ -753,11 +759,42 @@ static const struct usb_device_id dln2_table[] = {
>>
>> MODULE_DEVICE_TABLE(usb, dln2_table);
>
> Place the new callbacks above the device id table.
>
>> +static int dln2_suspend(struct usb_interface *iface, pm_message_t message)
>> +{
>> + struct dln2_dev *dln2 = usb_get_intfdata(iface);
>> +
>> + dln2_stop(dln2);
>
> You should also stop the reads urbs here.
Do you mean usb_kill_urb()? I thought that was not necessary unless
the device is reset. However, going throught
Documentation/usb/power-management.txt again looks like it must be
done in suspend.
>
>> + return 0;
>> +}
>> +
>> +static int dln2_resume(struct usb_interface *iface)
>> +{
>> + struct dln2_dev *dln2 = usb_get_intfdata(iface);
>> +
>> + dln2->disconnect = false;
>
> And surely you need to resubmit the read urbs in resume, or you will
> never receive any more data.
>
> How did you test this patch?
>
The resume cb is not called in my setup (kvm), only reset_resume. But
I assume since the port is not reset when resume is called the URBs
are still queued.
>> + return 0;
>> +}
>> +
>> +static int dln2_reset_resume(struct usb_interface *iface)
>> +{
>> + struct dln2_dev *dln2 = usb_get_intfdata(iface);
>> + int ret;
>> +
>> + dln2_free_rx_urbs(dln2);
>> + ret = dln2_setup_rx_urbs(dln2, iface->cur_altsetting);
>
> This doesn't make much sense. Why would you ever want to reallocate the
> urbs and their buffers here?
>
> If the device does not lose its state as you claim, then all you need to
> do is to resubmit the read urbs (as in resume).
>
The device itself does not lose state as it does not lose power and
does not react to USB port reset AFAICS (e.g. GPIO settings are
preserved). However the USB port is reset and I assumed I must
reallocate the URBs.
I just found out that usb-serial uses usb_poisoin_urb and
usb_unpoison_urb for suspend/resume and this two looks like just what
I need. Should I use that instead?
--
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] mfd: dln2: add suspend/resume functionality
On Fri, Dec 05, 2014 at 01:51:17PM +0200, Octavian Purdila wrote:
> On Fri, Dec 5, 2014 at 12:17 PM, Johan Hovold wrote:
> > On Thu, Nov 27, 2014 at 01:57:14PM +0200, Octavian Purdila wrote:
> >
> >> @@ -753,11 +759,42 @@ static const struct usb_device_id dln2_table[] = {
> >>
> >> MODULE_DEVICE_TABLE(usb, dln2_table);
> >
> > Place the new callbacks above the device id table.
> >
> >> +static int dln2_suspend(struct usb_interface *iface, pm_message_t message)
> >> +{
> >> + struct dln2_dev *dln2 = usb_get_intfdata(iface);
> >> +
> >> + dln2_stop(dln2);
> >
> > You should also stop the reads urbs here.
>
> Do you mean usb_kill_urb()? I thought that was not necessary unless
> the device is reset. However, going throught
> Documentation/usb/power-management.txt again looks like it must be
> done in suspend.
Yes, you should kill them explicitly. Any outstanding urbs will be
killed by usb core if you fail to do this.
> >> + return 0;
> >> +}
> >> +
> >> +static int dln2_resume(struct usb_interface *iface)
> >> +{
> >> + struct dln2_dev *dln2 = usb_get_intfdata(iface);
> >> +
> >> + dln2->disconnect = false;
> >
> > And surely you need to resubmit the read urbs in resume, or you will
> > never receive any more data.
> >
> > How did you test this patch?
> >
>
> The resume cb is not called in my setup (kvm), only reset_resume.
Please make sure to test your patches on proper hardware.
> But I assume since the port is not reset when resume is called the
> URBs are still queued.
No, they will have been killed by usb core even if you forget to do it,
so this would prevent any further reads.
> >> + return 0;
> >> +}
> >> +
> >> +static int dln2_reset_resume(struct usb_interface *iface)
> >> +{
> >> + struct dln2_dev *dln2 = usb_get_intfdata(iface);
> >> + int ret;
> >> +
> >> + dln2_free_rx_urbs(dln2);
> >> + ret = dln2_setup_rx_urbs(dln2, iface->cur_altsetting);
> >
> > This doesn't make much sense. Why would you ever want to reallocate the
> > urbs and their buffers here?
> >
> > If the device does not lose its state as you claim, then all you need to
> > do is to resubmit the read urbs (as in resume).
> >
> The device itself does not lose state as it does not lose power and
> does not react to USB port reset AFAICS (e.g. GPIO settings are
> preserved). However the USB port is reset and I assumed I must
> reallocate the URBs.
You don't and should not.
> I just found out that usb-serial uses usb_poisoin_urb and
> usb_unpoison_urb for suspend/resume and this two looks like just what
> I need.
Why do you think so?
> Should I use that instead?
No.
Johan
--
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 v2 3/5] usb: dwc2: Add generic PHY framework support for dwc2 usb controler platform driver.
Get PHY parameters from devicetree and power off usb PHY during
system suspend.
Signed-off-by: Yunzhi Li
---
drivers/usb/dwc2/gadget.c | 33 -
drivers/usb/dwc2/platform.c | 34 ++
2 files changed, 46 insertions(+), 21 deletions(-)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 200168e..2601c61 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3410,8 +3410,6 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
{
struct device *dev = hsotg->dev;
struct s3c_hsotg_plat *plat = dev->platform_data;
- struct phy *phy;
- struct usb_phy *uphy;
struct s3c_hsotg_ep *eps;
int epnum;
int ret;
@@ -3421,30 +3419,23 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
hsotg->phyif = GUSBCFG_PHYIF16;
/*
-* Attempt to find a generic PHY, then look for an old style
-* USB PHY, finally fall back to pdata
+* If platform probe couldn't find a generic PHY or an old style
+* USB PHY, fall back to pdata
*/
- phy = devm_phy_get(dev, "usb2-phy");
- if (IS_ERR(phy)) {
- uphy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
- if (IS_ERR(uphy)) {
- /* Fallback for pdata */
- plat = dev_get_platdata(dev);
- if (!plat) {
- dev_err(dev,
- "no platform data or transceiver defined\n");
- return -EPROBE_DEFER;
- }
- hsotg->plat = plat;
- } else
- hsotg->uphy = uphy;
- } else {
- hsotg->phy = phy;
+ if (IS_ERR_OR_NULL(hsotg->phy) && IS_ERR_OR_NULL(hsotg->uphy)) {
+ plat = dev_get_platdata(dev);
+ if (!plat) {
+ dev_err(dev,
+ "no platform data or transceiver defined\n");
+ return -EPROBE_DEFER;
+ }
+ hsotg->plat = plat;
+ } else if (hsotg->phy) {
/*
* If using the generic PHY framework, check if the PHY bus
* width is 8-bit and set the phyif appropriately.
*/
- if (phy_get_bus_width(phy) == 8)
+ if (phy_get_bus_width(hsotg->phy) == 8)
hsotg->phyif = GUSBCFG_PHYIF8;
}
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index 6a795aa..739d14f 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -155,6 +155,8 @@ static int dwc2_driver_probe(struct platform_device *dev)
struct dwc2_core_params defparams;
struct dwc2_hsotg *hsotg;
struct resource *res;
+ struct phy *phy;
+ struct usb_phy *uphy;
int retval;
int irq;
@@ -212,6 +214,24 @@ static int dwc2_driver_probe(struct platform_device *dev)
hsotg->dr_mode = of_usb_get_dr_mode(dev->dev.of_node);
+ /*
+* Attempt to find a generic PHY, then look for an old style
+* USB PHY
+*/
+ phy = devm_phy_get(&dev->dev, "usb2-phy");
+ if (IS_ERR(phy)) {
+ hsotg->phy = NULL;
+ uphy = devm_usb_get_phy(&dev->dev, USB_PHY_TYPE_USB2);
+ if (IS_ERR(uphy))
+ hsotg->uphy = NULL;
+ else
+ hsotg->uphy = uphy;
+ } else {
+ hsotg->phy = phy;
+ phy_power_on(hsotg->phy);
+ phy_init(hsotg->phy);
+ }
+
spin_lock_init(&hsotg->lock);
mutex_init(&hsotg->init_mutex);
retval = dwc2_gadget_init(hsotg, irq);
@@ -233,6 +253,14 @@ static int __maybe_unused dwc2_suspend(struct device *dev)
if (dwc2_is_device_mode(dwc2))
ret = s3c_hsotg_suspend(dwc2);
+ else {
+ if (dwc2->lx_state == DWC2_L0)
+ return 0;
+ if (dwc2->phy) {
+ phy_exit(dwc2->phy);
+ phy_power_off(dwc2->phy);
+ }
+ }
return ret;
}
@@ -243,6 +271,12 @@ static int __maybe_unused dwc2_resume(struct device *dev)
if (dwc2_is_device_mode(dwc2))
ret = s3c_hsotg_resume(dwc2);
+ else {
+ if (dwc2->phy) {
+ phy_power_on(dwc2->phy);
+ phy_init(dwc2->phy);
+ }
+ }
return ret;
}
--
2.0.0
--
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 v2 0/5]
Patches to add support for Rockchip usb phys.Add a new Rockchip usb phy driver and modify dwc2 controller driver to make dwc2 platform devices support a generic PHY framework driver. This patch set has been tested on my rk3288-evb and power off the usb phys would reduce about 60mW power budget in total during sustem suspend. Yunzhi Li (5): phy: add a driver for the Rockchip SoC internal USB2.0 PHY. Documentation: bindings: add doc for the Rockchip usb PHY usb: dwc2: Add generic PHY framework support for dwc2 usb controler platform driver. ARM: dts: add rk3288 usb PHY ARM: dts: Enable usb PHY on rk3288-evb board .../devicetree/bindings/phy/rockchip-usb-phy.txt | 22 +++ arch/arm/boot/dts/rk3288-evb.dtsi | 4 + arch/arm/boot/dts/rk3288.dtsi | 13 ++ drivers/phy/Kconfig| 7 + drivers/phy/Makefile | 1 + drivers/phy/phy-rockchip-usb.c | 179 + drivers/usb/dwc2/gadget.c | 33 ++-- drivers/usb/dwc2/platform.c| 34 8 files changed, 272 insertions(+), 21 deletions(-) create mode 100644 Documentation/devicetree/bindings/phy/rockchip-usb-phy.txt create mode 100644 drivers/phy/phy-rockchip-usb.c -- 2.0.0 -- 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] mfd: dln2: add suspend/resume functionality
On Fri, Dec 5, 2014 at 2:06 PM, Johan Hovold wrote:
> On Fri, Dec 05, 2014 at 01:51:17PM +0200, Octavian Purdila wrote:
>> On Fri, Dec 5, 2014 at 12:17 PM, Johan Hovold wrote:
>> > On Thu, Nov 27, 2014 at 01:57:14PM +0200, Octavian Purdila wrote:
>> >
>> >> @@ -753,11 +759,42 @@ static const struct usb_device_id dln2_table[] = {
>> >>
>> >> MODULE_DEVICE_TABLE(usb, dln2_table);
>> >
>> > Place the new callbacks above the device id table.
>> >
>> >> +static int dln2_suspend(struct usb_interface *iface, pm_message_t
>> >> message)
>> >> +{
>> >> + struct dln2_dev *dln2 = usb_get_intfdata(iface);
>> >> +
>> >> + dln2_stop(dln2);
>> >
>> > You should also stop the reads urbs here.
>>
>> Do you mean usb_kill_urb()? I thought that was not necessary unless
>> the device is reset. However, going throught
>> Documentation/usb/power-management.txt again looks like it must be
>> done in suspend.
>
> Yes, you should kill them explicitly. Any outstanding urbs will be
> killed by usb core if you fail to do this.
>
>> >> + return 0;
>> >> +}
>> >> +
>> >> +static int dln2_resume(struct usb_interface *iface)
>> >> +{
>> >> + struct dln2_dev *dln2 = usb_get_intfdata(iface);
>> >> +
>> >> + dln2->disconnect = false;
>> >
>> > And surely you need to resubmit the read urbs in resume, or you will
>> > never receive any more data.
>> >
>> > How did you test this patch?
>> >
>>
>> The resume cb is not called in my setup (kvm), only reset_resume.
>
> Please make sure to test your patches on proper hardware.
It was tested with the proper hardware, I was just testing it in the
setup we use, which is kvm with USB pass through.
>
>> But I assume since the port is not reset when resume is called the
>> URBs are still queued.
>
> No, they will have been killed by usb core even if you forget to do it,
> so this would prevent any further reads.
>
>> >> + return 0;
>> >> +}
>> >> +
>> >> +static int dln2_reset_resume(struct usb_interface *iface)
>> >> +{
>> >> + struct dln2_dev *dln2 = usb_get_intfdata(iface);
>> >> + int ret;
>> >> +
>> >> + dln2_free_rx_urbs(dln2);
>> >> + ret = dln2_setup_rx_urbs(dln2, iface->cur_altsetting);
>> >
>> > This doesn't make much sense. Why would you ever want to reallocate the
>> > urbs and their buffers here?
>> >
>> > If the device does not lose its state as you claim, then all you need to
>> > do is to resubmit the read urbs (as in resume).
>> >
>> The device itself does not lose state as it does not lose power and
>> does not react to USB port reset AFAICS (e.g. GPIO settings are
>> preserved). However the USB port is reset and I assumed I must
>> reallocate the URBs.
>
> You don't and should not.
>
>> I just found out that usb-serial uses usb_poisoin_urb and
>> usb_unpoison_urb for suspend/resume and this two looks like just what
>> I need.
>
> Why do you think so?
>
Hmm, never mind, now I understand that usb_poisoin_urb is just used
defensive in usb-serial to prevent the drivers to submit the URB until
resume time.
Thanks for the review Johan, I will send v2 with usb_kill_urb in
suspend() and usb_submit_urb() in resume and resume_reset.
--
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 v4] usb: core: buffer: smallest buffer should start at ARCH_DMA_MINALIGN
the following error pops up during "testusb -a -t 10"
| musb-hdrc musb-hdrc.1.auto: dma_pool_free buffer-128, f134e000/be842000 (bad
dma)
hcd_buffer_create() creates a few buffers, the smallest has 32 bytes of
size. ARCH_KMALLOC_MINALIGN is set to 64 bytes. This combo results in
hcd_buffer_alloc() returning memory which is 32 bytes aligned and it
might by identified by buffer_offset() as another buffer. This means the
buffer which is on a 32 byte boundary will not get freed, instead it
tries to free another buffer with the error message.
This patch fixes the issue by creating the smallest DMA buffer with the
size of ARCH_KMALLOC_MINALIGN (or 32 in case ARCH_KMALLOC_MINALIGN is
smaller). This might be 32, 64 or even 128 bytes. The next three pools
will have the size 128, 512 and 2048.
In case the smallest pool is 128 bytes then we have only three pools
instead of four (and zero the first entry in the array).
The last pool size is always 2048 bytes which is the assumed PAGE_SIZE /
2 of 4096. I doubt it makes sense to continue using PAGE_SIZE / 2 where
we would end up with 8KiB buffer in case we have 16KiB pages.
Instead I think it makes sense to have a common size(s) and extend them
if there is need to.
There is a BUILD_BUG_ON() now in case someone has a minalign of more than
128 bytes.
Cc: sta...@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior
---
v3…v4: rewrite as suggested per Alan Stern so it is less confusing.
v2…v3:
- rewrite and use usb_init_pool_max() instead. Albeit the value
__alignof__(x) is known at compile time it can't be used in #if
statement by the CPP. The max_t and if statment in this patch is
optimized away by the compiler
- replace PAGE_SIZE / 2 by 2048.
v1…v2: rewrite pool_max so it is less confusing.
drivers/usb/core/buffer.c | 26 +-
drivers/usb/core/usb.c| 1 +
include/linux/usb/hcd.h | 1 +
3 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c
index 684ef70dc09d..506b969ea7fd 100644
--- a/drivers/usb/core/buffer.c
+++ b/drivers/usb/core/buffer.c
@@ -22,17 +22,25 @@
*/
/* FIXME tune these based on pool statistics ... */
-static const size_tpool_max[HCD_BUFFER_POOLS] = {
- /* platforms without dma-friendly caches might need to
-* prevent cacheline sharing...
-*/
- 32,
- 128,
- 512,
- PAGE_SIZE / 2
- /* bigger --> allocate pages */
+static size_t pool_max[HCD_BUFFER_POOLS] = {
+ 32, 128, 512, 2048,
};
+void __init usb_init_pool_max(void)
+{
+ /*
+* The pool_max values must never be smaller than
+* ARCH_KMALLOC_MINALIGN.
+*/
+ if (ARCH_KMALLOC_MINALIGN <= 32)
+ ; /* Original value is okay */
+ else if (ARCH_KMALLOC_MINALIGN <= 64)
+ pool_max[0] = 64;
+ else if (ARCH_KMALLOC_MINALIGN <= 128)
+ pool_max[0] = 0;/* Don't use this pool */
+ else
+ BUILD_BUG();/* We don't allow this */
+}
/* SETUP primitives */
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 2dd2362198d2..29ee9363faa5 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -1051,6 +1051,7 @@ static int __init usb_init(void)
pr_info("%s: USB support disabled\n", usbcore_name);
return 0;
}
+ usb_init_pool_max();
retval = usb_debugfs_init();
if (retval)
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index cd96a2bc3388..9e8d161bf2db 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -450,6 +450,7 @@ extern const struct dev_pm_ops usb_hcd_pci_pm_ops;
#endif /* CONFIG_PCI */
/* pci-ish (pdev null is ok) buffer alloc/mapping support */
+void usb_init_pool_max(void);
int hcd_buffer_create(struct usb_hcd *hcd);
void hcd_buffer_destroy(struct usb_hcd *hcd);
--
2.1.3
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 0/3] Equivalent of g_webcam with configfs.
This series aims at integrating configfs into uvc, the way it has been done for acm, ncm, ecm, eem, ecm subset, rndis, obex, phonet, mass_storage, FunctionFS, loopback, sourcesink, uac1, uac2, hid and midi. v1..v2: - sorted header fiels alphabetically - applied style corrections - factored out verifying uvc descriptors presence into a separate patch - added mutex_destroy() - added more comments for not-obvious parts - eliminated copy-paste errors (assinment to opts->fs_control, opts->ss_control - actually used the refcnt member of struct f_uvc_opts Below is an example listing of gadget's function directory. The attributes whose access permissions are -r--r--r-- are there only for inspection by the user but not for modifications. For color matching, camera terminal, processing unit and output terminal only default descriptors are provided, that is, creating user's own is not supported at this moment. drwxr-xr-x . drwxr-xr-x ./streaming drwxr-xr-x ./streaming/class drwxr-xr-x ./streaming/class/ss drwxr-xr-x ./streaming/class/hs lrwxrwxrwx ./streaming/class/hs/h -> \ ../../../../../../../usb_gadget/g1/functions/uvc.usb0/streaming/header/h drwxr-xr-x ./streaming/class/fs lrwxrwxrwx ./streaming/class/fs/h -> \ ../../../../../../../usb_gadget/g1/functions/uvc.usb0/streaming/header/h drwxr-xr-x ./streaming/color_matching drwxr-xr-x ./streaming/color_matching/default -r--r--r-- ./streaming/color_matching/default/bMatrixCoefficients -r--r--r-- ./streaming/color_matching/default/bTransferCharacteristics -r--r--r-- ./streaming/color_matching/default/bColorPrimaries drwxr-xr-x ./streaming/uncompressed drwxr-xr-x ./streaming/uncompressed/u drwxr-xr-x ./streaming/uncompressed/u/360p -rw-r--r-- ./streaming/uncompressed/u/360p/dwFrameInterval -rw-r--r-- ./streaming/uncompressed/u/360p/dwDefaultFrameInterval -rw-r--r-- ./streaming/uncompressed/u/360p/dwMaxVideoFrameBufferSize -rw-r--r-- ./streaming/uncompressed/u/360p/dwMaxBitRate -rw-r--r-- ./streaming/uncompressed/u/360p/dwMinBitRate -rw-r--r-- ./streaming/uncompressed/u/360p/wHeight -rw-r--r-- ./streaming/uncompressed/u/360p/wWidth -rw-r--r-- ./streaming/uncompressed/u/360p/bmCapabilities -rw-r--r-- ./streaming/uncompressed/u/bmaControls -r--r--r-- ./streaming/uncompressed/u/bmInterfaceFlags -r--r--r-- ./streaming/uncompressed/u/bAspectRatioY -r--r--r-- ./streaming/uncompressed/u/bAspectRatioX -rw-r--r-- ./streaming/uncompressed/u/bDefaultFrameIndex -rw-r--r-- ./streaming/uncompressed/u/bBitsPerPixel -rw-r--r-- ./streaming/uncompressed/u/guidFormat drwxr-xr-x ./streaming/header drwxr-xr-x ./streaming/header/h lrwxrwxrwx ./streaming/header/h/u -> \ ../../../../../../../usb_gadget/g1/functions/uvc.usb0/streaming/uncompressed/u -r--r--r-- ./streaming/header/h/bTriggerUsage -r--r--r-- ./streaming/header/h/bTriggerSupport -r--r--r-- ./streaming/header/h/bStillCaptureMethod -r--r--r-- ./streaming/header/h/bTerminalLink -r--r--r-- ./streaming/header/h/bmInfo drwxr-xr-x ./control drwxr-xr-x ./control/class drwxr-xr-x ./control/class/ss lrwxrwxrwx ./control/class/ss/h -> \ ../../../../../../../usb_gadget/g1/functions/uvc.usb0/control/header/h drwxr-xr-x ./control/class/fs lrwxrwxrwx ./control/class/fs/h -> \ ../../../../../../../usb_gadget/g1/functions/uvc.usb0/control/header/h drwxr-xr-x ./control/terminal drwxr-xr-x ./control/terminal/output drwxr-xr-x ./control/terminal/output/default -r--r--r-- ./control/terminal/output/default/iTerminal -r--r--r-- ./control/terminal/output/default/bSourceID -r--r--r-- ./control/terminal/output/default/bAssocTerminal -r--r--r-- ./control/terminal/output/default/wTerminalType -r--r--r-- ./control/terminal/output/default/bTerminalID drwxr-xr-x ./control/terminal/camera drwxr-xr-x ./control/terminal/camera/default -r--r--r-- ./control/terminal/camera/default/bmControls -r--r--r-- ./control/terminal/camera/default/wOcularFocalLength -r--r--r-- ./control/terminal/camera/default/wObjectiveFocalLengthMax -r--r--r-- ./control/terminal/camera/default/wObjectiveFocalLengthMin -r--r--r-- ./control/terminal/camera/default/iTerminal -r--r--r-- ./control/terminal/camera/default/bAssocTerminal -r--r--r-- ./control/terminal/camera/default/wTerminalType -r--r--r-- ./control/terminal/camera/default/bTerminalID drwxr-xr-x ./control/processing drwxr-xr-x ./control/processing/default -r--r--r-- ./control/processing/default/iProcessing -r--r--r-- ./control/processing/default/bmControls -r--r--r-- ./control/processing/default/wMaxMultiplier -r--r--r-- ./control/processing/default/bSourceID -r--r--r-- ./control/processing/default/bUnitID drwxr-xr-x ./control/header drwxr-xr-x ./control/header/h -rw-r--r-- ./control/header/h/dwClockFrequency -rw-r--r-- ./control/header/h/bcdUVC -rw-r--r-- ./streaming_maxburst -rw-r--r-- ./streaming_maxpacket -rw-r--r-- ./streaming_interval BACKWARD COMPATIBILITY == P
[PATCHv2 3/3] usb: gadget: uvc: configfs support in uvc function
Add support for using the uvc function as a component of USB gadgets composed with configfs. Signed-off-by: Andrzej Pietrasiewicz --- Documentation/ABI/testing/configfs-usb-gadget-uvc | 265 +++ drivers/usb/gadget/Kconfig| 11 + drivers/usb/gadget/function/Makefile |2 +- drivers/usb/gadget/function/f_uvc.c | 107 +- drivers/usb/gadget/function/u_uvc.h | 46 + drivers/usb/gadget/function/uvc_configfs.c| 2439 + drivers/usb/gadget/function/uvc_configfs.h| 22 + 7 files changed, 2888 insertions(+), 4 deletions(-) create mode 100644 Documentation/ABI/testing/configfs-usb-gadget-uvc create mode 100644 drivers/usb/gadget/function/uvc_configfs.c create mode 100644 drivers/usb/gadget/function/uvc_configfs.h diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc b/Documentation/ABI/testing/configfs-usb-gadget-uvc new file mode 100644 index 000..2f4a005 --- /dev/null +++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc @@ -0,0 +1,265 @@ +What: /config/usb-gadget/gadget/functions/uvc.name +Date: Dec 2014 +KernelVersion: 3.20 +Description: UVC function directory + + streaming_maxburst - 0..15 (ss only) + streaming_maxpacket - 1..1023 (fs), 1..3072 (hs/ss) + streaming_interval - 1..16 + +What: /config/usb-gadget/gadget/functions/uvc.name/control +Date: Dec 2014 +KernelVersion: 3.20 +Description: Control descriptors + +What: /config/usb-gadget/gadget/functions/uvc.name/control/class +Date: Dec 2014 +KernelVersion: 3.20 +Description: Class descriptors + +What: /config/usb-gadget/gadget/functions/uvc.name/control/class/ss +Date: Dec 2014 +KernelVersion: 3.20 +Description: Super speed control class descriptors + +What: /config/usb-gadget/gadget/functions/uvc.name/control/class/fs +Date: Dec 2014 +KernelVersion: 3.20 +Description: Full speed control class descriptors + +What: /config/usb-gadget/gadget/functions/uvc.name/control/terminal +Date: Dec 2014 +KernelVersion: 3.20 +Description: Terminal descriptors + +What: /config/usb-gadget/gadget/functions/uvc.name/control/terminal/output +Date: Dec 2014 +KernelVersion: 3.20 +Description: Output terminal descriptors + +What: /config/usb-gadget/gadget/functions/uvc.name/control/terminal/output/default +Date: Dec 2014 +KernelVersion: 3.20 +Description: Default output terminal descriptors + + All attributes read only: + iTerminal - index of string descriptor + bSourceID - id of the terminal to which this terminal + is connected + bAssocTerminal - id of the input terminal to which this output + terminal is associated + wTerminalType - terminal type + bTerminalID - a non-zero id of this terminal + +What: /config/usb-gadget/gadget/functions/uvc.name/control/terminal/camera +Date: Dec 2014 +KernelVersion: 3.20 +Description: Camera terminal descriptors + +What: /config/usb-gadget/gadget/functions/uvc.name/control/terminal/camera/default +Date: Dec 2014 +KernelVersion: 3.20 +Description: Default camera terminal descriptors + + All attributes read only: + bmControls - bitmap specifying which controls are + supported for the video stream + wOcularFocalLength - the value of Locular + wObjectiveFocalLengthMax- the value of Lmin + wObjectiveFocalLengthMin- the value of Lmax + iTerminal - index of string descriptor + bAssocTerminal - id of the output terminal to which + this terminal is connected + wTerminalType - terminal type + bTerminalID - a non-zero id of this terminal + +What: /config/usb-gadget/gadget/functions/uvc.name/control/processing +Date: Dec 2014 +KernelVersion: 3.20 +Description: Processing unit descriptors + +What: /config/usb-gadget/gadget/functions/uvc.name/control/processing/default +Date: Dec 2014 +KernelVersion: 3.20 +Description: Default processing unit descriptors + + All attributes read only: + iProcessing - index of string descriptor + bmControls - bitmap specifying which controls are + supported for the video stream + wMaxMultiplier - maximum digital magnification x100 + bSourceID - id of the terminal to which this unit is + connected + bUnitID - a non-
[PATCHv2 2/3] usb: gadget: uvc: verify descriptors presence
If the caller of uvc_alloc() does not provide enough
descriptors, binding the function should fail, so appropriate
code is returned from uvc_copy_descriptors().
uvc_function_bind() is modified accordingly to account for possible
errors from uvc_copy_descriptors().
Signed-off-by: Andrzej Pietrasiewicz
---
drivers/usb/gadget/function/f_uvc.c | 23 +--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/function/f_uvc.c
b/drivers/usb/gadget/function/f_uvc.c
index 5b4ab39..b14c599 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -508,6 +508,8 @@ uvc_copy_descriptors(struct uvc_device *uvc, enum
usb_device_speed speed)
uvc_streaming_std = uvc_fs_streaming;
break;
}
+ if (!uvc_control_desc || !uvc_streaming_cls)
+ return ERR_PTR(-ENODEV);
/* Descriptors layout
*
@@ -700,10 +702,27 @@ uvc_function_bind(struct usb_configuration *c, struct
usb_function *f)
/* Copy descriptors */
f->fs_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_FULL);
- if (gadget_is_dualspeed(cdev->gadget))
+ if (IS_ERR(f->fs_descriptors)) {
+ ret = PTR_ERR(f->fs_descriptors);
+ f->fs_descriptors = NULL;
+ goto error;
+ }
+ if (gadget_is_dualspeed(cdev->gadget)) {
f->hs_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_HIGH);
- if (gadget_is_superspeed(c->cdev->gadget))
+ if (IS_ERR(f->hs_descriptors)) {
+ ret = PTR_ERR(f->hs_descriptors);
+ f->hs_descriptors = NULL;
+ goto error;
+ }
+ }
+ if (gadget_is_superspeed(c->cdev->gadget)) {
f->ss_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_SUPER);
+ if (IS_ERR(f->ss_descriptors)) {
+ ret = PTR_ERR(f->ss_descriptors);
+ f->ss_descriptors = NULL;
+ goto error;
+ }
+ }
/* Preallocate control endpoint request. */
uvc->control_req = usb_ep_alloc_request(cdev->gadget->ep0, GFP_KERNEL);
--
1.9.1
--
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
[PATCHv2 1/3] usb: gadget: f_uvc: rename a macro to avoid conflicts
When configfs is integrated, CONFIGFS_ATTR_STRUCT and CONFIGFS_ATTR_OPS
macros should be used, but the latter expects that tere is a to_f_uvc_opts
function accepting a config_item, whereas the macro being changed
can be applied to a different type of argument.
Signed-off-by: Andrzej Pietrasiewicz
---
drivers/usb/gadget/function/f_uvc.c | 6 +++---
drivers/usb/gadget/function/u_uvc.h | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/function/f_uvc.c
b/drivers/usb/gadget/function/f_uvc.c
index 945b3bd..5b4ab39 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -605,7 +605,7 @@ uvc_function_bind(struct usb_configuration *c, struct
usb_function *f)
INFO(cdev, "uvc_function_bind\n");
- opts = to_f_uvc_opts(f->fi);
+ opts = fi_to_f_uvc_opts(f->fi);
/* Sanity check the streaming endpoint module parameters.
*/
opts->streaming_interval = clamp(opts->streaming_interval, 1U, 16U);
@@ -766,7 +766,7 @@ error:
static void uvc_free_inst(struct usb_function_instance *f)
{
- struct f_uvc_opts *opts = to_f_uvc_opts(f);
+ struct f_uvc_opts *opts = fi_to_f_uvc_opts(f);
kfree(opts);
}
@@ -818,7 +818,7 @@ static struct usb_function *uvc_alloc(struct
usb_function_instance *fi)
return ERR_PTR(-ENOMEM);
uvc->state = UVC_STATE_DISCONNECTED;
- opts = to_f_uvc_opts(fi);
+ opts = fi_to_f_uvc_opts(fi);
uvc->desc.fs_control = opts->fs_control;
uvc->desc.ss_control = opts->ss_control;
diff --git a/drivers/usb/gadget/function/u_uvc.h
b/drivers/usb/gadget/function/u_uvc.h
index 2a8dfdf..c0706a3 100644
--- a/drivers/usb/gadget/function/u_uvc.h
+++ b/drivers/usb/gadget/function/u_uvc.h
@@ -18,7 +18,7 @@
#include
-#define to_f_uvc_opts(f) container_of(f, struct f_uvc_opts, func_inst)
+#define fi_to_f_uvc_opts(f)container_of(f, struct f_uvc_opts, func_inst)
struct f_uvc_opts {
struct usb_function_instancefunc_inst;
--
1.9.1
--
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: gadget: uvc: configfs support in uvc function
Hi Laurent,
thank you for your review.
I generally agree with your comments but please see inline
for some explanations.
W dniu 28.11.2014 o 00:40, Laurent Pinchart pisze:
Hi Andrzej,
Thank you for the patch.
On Wednesday 24 September 2014 15:26:43 Andrzej Pietrasiewicz wrote:
Add support for using the uvc function as a component of USB gadgets
composed with configfs.
Signed-off-by: Andrzej Pietrasiewicz
---
This should be 3.19. My fault, sorry :-/
I'm glad you have managed to review.
Now it looks like
all other functions which are available as separate f_xyz.c files are
already converted, so it would be nice if uvc made its way into 3.20.
+What: /config/usb-gadget/gadget/functions/uvc.name/streaming/mjpeg
+Date: Nov 2014
+KernelVersion: 3.18
+Description: MJPEG format descriptors
+
+What:
/config/usb-gadget/gadget/functions/uvc.name/streaming/mjpeg/name
I'm not too familiar with configfs, should the attribute description specify
what values are recommended and/or acceptable for "name" ?
Anything? The video format is determined by the parent directory name
(mjpeg in this case). The name can be pretty much anything a filesystem
permits. The association of formats is done with symbolic links.
@@ -778,6 +873,7 @@ static struct usb_function *uvc_alloc(struct
usb_function_instance *fi) {
struct uvc_device *uvc;
struct f_uvc_opts *opts;
+ struct uvc_descriptor_header **strm_cls;
uvc = kzalloc(sizeof(*uvc), GFP_KERNEL);
if (uvc == NULL)
@@ -786,11 +882,29 @@ static struct usb_function *uvc_alloc(struct
usb_function_instance *fi) uvc->state = UVC_STATE_DISCONNECTED;
opts = fi_to_f_uvc_opts(fi);
+ mutex_lock(&opts->lock);
What does the mutex protect against ? Modifications of opts-
uvc_*_streaming_cls by configfs ? You only copy pointers around, what
prevents the descriptors from being modified after you unlock the mutex ?
+ if (opts->uvc_fs_streaming_cls) {
+ strm_cls = opts->uvc_fs_streaming_cls;
+ opts->fs_streaming =
+ (const struct uvc_descriptor_header * const *)strm_cls;
+ }
+ if (opts->uvc_hs_streaming_cls) {
+ strm_cls = opts->uvc_hs_streaming_cls;
+ opts->hs_streaming =
+ (const struct uvc_descriptor_header * const *)strm_cls;
+ }
+ if (opts->uvc_ss_streaming_cls) {
+ strm_cls = opts->uvc_ss_streaming_cls;
+ opts->ss_streaming =
+ (const struct uvc_descriptor_header * const *)strm_cls;
+ }
+
Also modifications of opts->fs_streaming & friends.
diff --git a/drivers/usb/gadget/function/uvc_configfs.c
b/drivers/usb/gadget/function/uvc_configfs.c new file mode 100644
index 000..33d92ab
--- /dev/null
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -0,0 +1,2439 @@
I can't really review this in a reasonable time frame at the moment I'm afraid
:-/ We'll have to fix bugs as we go.
[snip]
You never know with a company what you are assigned to, but I'm really
hoping to stick around usb gadgets next year so I intend to fix any bugs
that are discovered, especially if they are found in my code ;-) So I
think your suggestion seems reasonable, given the amount of time which
has passed since we first talked in New Orleans about the idea of adding
configfs support to the uvc function.
AP
--
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
storage: Revise/fix quirk for 04E6:000F SCM USB-SCSI converter
I recently posted a patch ("storage: Add quirk for another SCM-based
USB-SCSI converter") to add a quirk for the converter with ID 04E6:000F,
which is listed along with 04E6:000B in the Windows INF file for the
Startech ICUSBSCSI2 as "eUSB SCSI Adapter (Bus Powered)".
The already-present quirk for 04E6:000B has USB_SC_SCSI/USB_PR_BULK, not
USB_SC_DEVICE/USB_PR_DEVICE. Change the 04E6:000F quirk to match that.
Signed-off-by: Mark Knibbs
---
While I don't have one to check, later SCM converters probably use
vendor-specific class and protocol, thus specifying USB_SC_SCSI/USB_PR_BULK
is necessary. Some speculation as to the reason for the difference...
I'm not sure whether the change was for all later products, or is related
to the later products being bus-powered. Windows 98 was the current
mainstream OS when the eUSCSI converter was first introduced. That OS
didn't ship with a USB mass storage driver, thus in order to use the
converter the SCM driver had to be installed.
Later Windows 2000 was released, which included a USB mass storage driver.
If the user were to connect the converter before installing the SCM driver,
Windows would install its default mass storage driver which only works with
the SCSI device at ID 0. Perhaps SCM changed to a vendor-specific class/
protocol to prevent the Windows generic driver binding to it, and reduce
driver-related technical support issues?
Patch is based on 3.18-rc7.
diff --git a/drivers/usb/storage/unusual_devs.h
b/drivers/usb/storage/unusual_devs.h
index 11c7a96..bfc4a6a 100644
--- a/drivers/usb/storage/unusual_devs.h
+++ b/drivers/usb/storage/unusual_devs.h
@@ -507,7 +507,7 @@ UNUSUAL_DEV( 0x04e6, 0x000c, 0x0100, 0x0100,
UNUSUAL_DEV( 0x04e6, 0x000f, 0x, 0x,
"SCM Microsystems",
"eUSB SCSI Adapter (Bus Powered)",
- USB_SC_DEVICE, USB_PR_DEVICE, usb_stor_euscsi_init,
+ USB_SC_SCSI, USB_PR_BULK, usb_stor_euscsi_init,
US_FL_SCM_MULT_TARG ),
UNUSUAL_DEV( 0x04e6, 0x0101, 0x0200, 0x0200,
--
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 3/5] usb: dwc2: Add generic PHY framework support for dwc2 usb controler platform driver.
> From: Yunzhi Li [mailto:l...@rock-chips.com]
> Sent: Friday, December 05, 2014 4:52 AM
>
> Get PHY parameters from devicetree and power off usb PHY during
> system suspend.
>
> Signed-off-by: Yunzhi Li
> ---
>
> drivers/usb/dwc2/gadget.c | 33 -
> drivers/usb/dwc2/platform.c | 34 ++
> 2 files changed, 46 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 200168e..2601c61 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -3410,8 +3410,6 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
> {
> struct device *dev = hsotg->dev;
> struct s3c_hsotg_plat *plat = dev->platform_data;
> - struct phy *phy;
> - struct usb_phy *uphy;
> struct s3c_hsotg_ep *eps;
> int epnum;
> int ret;
> @@ -3421,30 +3419,23 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int
> irq)
> hsotg->phyif = GUSBCFG_PHYIF16;
>
> /*
> - * Attempt to find a generic PHY, then look for an old style
> - * USB PHY, finally fall back to pdata
> + * If platform probe couldn't find a generic PHY or an old style
> + * USB PHY, fall back to pdata
>*/
> - phy = devm_phy_get(dev, "usb2-phy");
> - if (IS_ERR(phy)) {
> - uphy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
> - if (IS_ERR(uphy)) {
> - /* Fallback for pdata */
> - plat = dev_get_platdata(dev);
> - if (!plat) {
> - dev_err(dev,
> - "no platform data or transceiver defined\n");
> - return -EPROBE_DEFER;
> - }
> - hsotg->plat = plat;
> - } else
> - hsotg->uphy = uphy;
> - } else {
> - hsotg->phy = phy;
> + if (IS_ERR_OR_NULL(hsotg->phy) && IS_ERR_OR_NULL(hsotg->uphy)) {
> + plat = dev_get_platdata(dev);
> + if (!plat) {
> + dev_err(dev,
> + "no platform data or transceiver defined\n");
> + return -EPROBE_DEFER;
> + }
> + hsotg->plat = plat;
> + } else if (hsotg->phy) {
You have changed the behavior here. Previously, the driver would work
even if there were no phys or pdata defined. Now it will return
-EPROBE_DEFER instead. Are you sure that won't break any existing
platforms?
> /*
>* If using the generic PHY framework, check if the PHY bus
>* width is 8-bit and set the phyif appropriately.
>*/
> - if (phy_get_bus_width(phy) == 8)
> + if (phy_get_bus_width(hsotg->phy) == 8)
> hsotg->phyif = GUSBCFG_PHYIF8;
> }
>
> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
> index 6a795aa..739d14f 100644
> --- a/drivers/usb/dwc2/platform.c
> +++ b/drivers/usb/dwc2/platform.c
> @@ -155,6 +155,8 @@ static int dwc2_driver_probe(struct platform_device *dev)
> struct dwc2_core_params defparams;
> struct dwc2_hsotg *hsotg;
> struct resource *res;
> + struct phy *phy;
> + struct usb_phy *uphy;
> int retval;
> int irq;
>
> @@ -212,6 +214,24 @@ static int dwc2_driver_probe(struct platform_device *dev)
>
> hsotg->dr_mode = of_usb_get_dr_mode(dev->dev.of_node);
>
> + /*
> + * Attempt to find a generic PHY, then look for an old style
> + * USB PHY
> + */
> + phy = devm_phy_get(&dev->dev, "usb2-phy");
> + if (IS_ERR(phy)) {
> + hsotg->phy = NULL;
> + uphy = devm_usb_get_phy(&dev->dev, USB_PHY_TYPE_USB2);
> + if (IS_ERR(uphy))
> + hsotg->uphy = NULL;
> + else
> + hsotg->uphy = uphy;
> + } else {
> + hsotg->phy = phy;
> + phy_power_on(hsotg->phy);
> + phy_init(hsotg->phy);
> + }
> +
> spin_lock_init(&hsotg->lock);
> mutex_init(&hsotg->init_mutex);
> retval = dwc2_gadget_init(hsotg, irq);
> @@ -233,6 +253,14 @@ static int __maybe_unused dwc2_suspend(struct device
> *dev)
>
> if (dwc2_is_device_mode(dwc2))
> ret = s3c_hsotg_suspend(dwc2);
> + else {
Kernel style says that both branches of the if() should have braces
here.
> + if (dwc2->lx_state == DWC2_L0)
> + return 0;
> + if (dwc2->phy) {
> + phy_exit(dwc2->phy);
> + phy_power_off(dwc2->phy);
> + }
Minor nit: no need to test dwc2->phy for NULL here, the phy functions
handle a NULL pointer just fine.
> + }
> return ret;
> }
>
> @@ -243,6 +271,12 @@ static int __maybe_unused dwc2_resume(struct device *dev)
>
> if (dwc2_is_device_mode(dwc2))
> ret
[PATCH] usb: hcd: get/put device and hcd for hcd_buffers()
Consider the following scenario:
- plugin a webcam
- play the stream via gst-launch-0.10 v4l2src device=/dev/video0…
- remove the USB-HCD during playback via "rmmod $HCD"
and now wait for the crash
|musb-hdrc musb-hdrc.2.auto: remove, state 1
|usb usb2: USB disconnect, device number 1
|usb 2-1: USB disconnect, device number 3
|uvcvideo: Failed to resubmit video URB (-19).
|musb-hdrc musb-hdrc.2.auto: USB bus 2 deregistered
|musb-hdrc musb-hdrc.1.auto: remove, state 4
|usb usb1: USB disconnect, device number 1
|musb-hdrc musb-hdrc.1.auto: USB bus 1 deregistered
|Unable to handle kernel paging request at virtual address 6b6b6b6f
|pgd = c0004000
|[6b6b6b6f] *pgd=
|Internal error: Oops: 5 [#1] ARM
|Modules linked in: uvcvideo]
|CPU: 0 PID: 2613 Comm: gst-launch-0.10 Tainted: GW3.14.25+ #40
|task: ec2b8100 ti: ec38e000 task.ti: ec38e000
|PC is at hcd_buffer_free+0x64/0xc0
|LR is at uvc_free_urb_buffers+0x34/0x50 [uvcvideo]
|Process gst-launch-0.10 (pid: 2613, stack limit = 0xec38e240)
|[] (hcd_buffer_free)
|[] (uvc_free_urb_buffers [uvcvideo])
|[] (uvc_video_enable [uvcvideo])
|[] (uvc_v4l2_release [uvcvideo])
|[] (v4l2_release [videodev])
|[] (__fput)
|[] (task_work_run)
|[] (do_exit)
|[] (do_group_exit)
as part of the device-removal the HCD removes its dma-buffers, the HCD
structure itself and even the struct device is gone. That means if UVC
removes its URBs after its last user (/dev/videoX) is gone and not from
the ->disconnect() callback then it is too late because the HCD might
gone.
First, I fixed by in the UVC driver by ignoring the UVC_SC_VIDEOSTREAMING
in its ->disconnect callback and calling uvc_video_enable(, 0) in
uvc_unregister_video(). But then I though that it might not be clever to
release that memory if there is userspace using it.
So instead, I hold the device struct in the HCD and the HCD struct on
every USB-buf-alloc. That means after a disconnect we still have a
refcount on usb_hcd and device and it will be cleaned "later" once the
last USB-buffer is released.
Signed-off-by: Sebastian Andrzej Siewior
---
With this applied, I only see this three times (which is not new)
| [ cut here ]
| WARNING: CPU: 0 PID: 1755 at fs/sysfs/group.c:219
sysfs_remove_group+0x88/0x98()
| sysfs group c08a70d4 not found for kobject 'event4'
| Modules linked in: uvcvideo videobuf2_vmalloc videobuf2_memops videobuf2_core
v4l2_common videodev ipv6 musb_hdrc udc_core us]
| CPU: 0 PID: 1755 Comm: gst-launch-0.10 Not tainted
3.18.0-rc7-00065-gabcefb342fbf-dirty #1813
| [] (unwind_backtrace) from [] (show_stack+0x10/0x14)
| [] (show_stack) from [] (dump_stack+0x80/0x9c)
| [] (dump_stack) from [] (warn_slowpath_common+0x68/0x8c)
| [] (warn_slowpath_common) from []
(warn_slowpath_fmt+0x30/0x40)
| [] (warn_slowpath_fmt) from []
(sysfs_remove_group+0x88/0x98)
| [] (sysfs_remove_group) from [] (device_del+0x34/0x198)
| [] (device_del) from [] (evdev_disconnect+0x18/0x44)
| [] (evdev_disconnect) from []
(__input_unregister_device+0xa4/0x148)
| [] (__input_unregister_device) from []
(input_unregister_device+0x40/0x74)
| [] (input_unregister_device) from []
(uvc_delete+0x20/0x10c [uvcvideo])
| [] (uvc_delete [uvcvideo]) from []
(v4l2_device_release+0x9c/0xc4 [videodev])
| [] (v4l2_device_release [videodev]) from []
(device_release+0x2c/0x90)
| [] (device_release) from [] (kobject_release+0x48/0x7c)
| [] (kobject_release) from [] (v4l2_release+0x50/0x78
[videodev])
| [] (v4l2_release [videodev]) from [] (__fput+0x80/0x1c4)
| [] (__fput) from [] (task_work_run+0xb4/0xe4)
| [] (task_work_run) from [] (do_exit+0x2dc/0x92c)
| [] (do_exit) from [] (do_group_exit+0x3c/0xb0)
| [] (do_group_exit) from [] (__wake_up_parent+0x0/0x18)
| ---[ end trace b54a8f3c8129180e ]---
anyone an idea?
drivers/usb/core/buffer.c | 30 +-
drivers/usb/core/hcd.c| 2 ++
2 files changed, 27 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c
index 506b969ea7fd..01e080a61519 100644
--- a/drivers/usb/core/buffer.c
+++ b/drivers/usb/core/buffer.c
@@ -107,7 +107,7 @@ void hcd_buffer_destroy(struct usb_hcd *hcd)
* better sharing and to leverage mm/slab.c intelligence.
*/
-void *hcd_buffer_alloc(
+static void *_hcd_buffer_alloc(
struct usb_bus *bus,
size_t size,
gfp_t mem_flags,
@@ -131,7 +131,19 @@ void *hcd_buffer_alloc(
return dma_alloc_coherent(hcd->self.controller, size, dma, mem_flags);
}
-void hcd_buffer_free(
+void *hcd_buffer_alloc(struct usb_bus *bus, size_t size, gfp_t mem_flags,
+ dma_addr_t *dma)
+{
+ struct usb_hcd *hcd = bus_to_hcd(bus);
+ void *ret;
+
+ ret = _hcd_buffer_alloc(bus, size, mem_flags, dma);
+ if (ret)
+ usb_get_hcd(hcd);
+ return ret;
+}
+
+static void _hcd_buffer_free(
struct usb_bus *bus,
size_t size,
Re: [PATCH] usb: gadget: f_mass_storage: restore address range on exit
On Fri, 5 Dec 2014, Sanjay Singh Rawat wrote: > At the start of the thread we are changing the address limit, restoring it > to the default while exiting. What for? The thread is about to exit anyway. Who cares what the address limit is set to? 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] OHCI: add a quirk for ULi M5237 blocking on reset
On Fri, 5 Dec 2014, Arseny Solokha wrote:
> From: Arseny Solokha
>
> Commit 8dccddbc2368 ("OHCI: final fix for NVIDIA problems (I hope)")
> introduced into 3.1.9 broke boot on e.g. Freescale P2020DS development
> board. The code path that was previously specific to NVIDIA controllers
> had then become taken for all chips.
>
> However, the M5237 installed on the board wedges solid when accessing
> its base+OHCI_FMINTERVAL register, making it impossible to boot any
> kernel newer than 3.1.8 on this particular and apparently other similar
> machines.
>
> Don't readl() and writel() base+OHCI_FMINTERVAL on PCI ID 10b9:5237.
I seem to recall seeing something like this before, but I can't
remember where or when. At any rate, there's nothing like it in the
current code...
> The patch is suitable for the -next tree as well as all maintained
> kernels up to 3.2 inclusive.
>
> Signed-off-by: Arseny Solokha
> ---
> drivers/usb/host/pci-quirks.c | 14 +++---
> include/linux/pci_ids.h | 1 +
> 2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
> index 2f3aceb..3b29478 100644
> --- a/drivers/usb/host/pci-quirks.c
> +++ b/drivers/usb/host/pci-quirks.c
> @@ -571,7 +571,7 @@ static void quirk_usb_handoff_ohci(struct pci_dev *pdev)
> {
> void __iomem *base;
> u32 control;
> - u32 fminterval;
> + u32 fminterval = 0;
> int cnt;
>
> if (!mmio_resource_enabled(pdev, 0))
> @@ -619,7 +619,13 @@ static void quirk_usb_handoff_ohci(struct pci_dev *pdev)
> }
>
> /* software reset of the controller, preserving HcFmInterval */
> - fminterval = readl(base + OHCI_FMINTERVAL);
> + if (!(pdev->vendor == PCI_VENDOR_ID_AL &&
> + pdev->device == PCI_DEVICE_ID_AL_M5237)) {
> + /* ULi M5237 OHCI controller (10b9:5237) locks the whole system
> + * when accessing the OHCI_FMINTERVAL offset.
> + */
You don't need to specify the vendor and device IDs in the comment.
That's what #defines are for.
Also, the accepted format for multi-line comments is:
/*
* Blah, blah, blah...
* Blah, blah, blah...
*/
> + fminterval = readl(base + OHCI_FMINTERVAL);
> + }
> writel(OHCI_HCR, base + OHCI_CMDSTATUS);
>
> /* reset requires max 10 us delay */
> @@ -628,7 +634,9 @@ static void quirk_usb_handoff_ohci(struct pci_dev *pdev)
> break;
> udelay(1);
> }
> - writel(fminterval, base + OHCI_FMINTERVAL);
> + if (!(pdev->vendor == PCI_VENDOR_ID_AL &&
> + pdev->device == PCI_DEVICE_ID_AL_M5237))
> + writel(fminterval, base + OHCI_FMINTERVAL);
This is very ugly, with these repeated tests. You should set a
no_fminterval flag at the start of the function and then use it instead
of doing the same tests over again.
> /* Now the controller is safely in SUSPEND and nothing can wake it up */
> iounmap(base);
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 1fa99a3..266fc5c 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -1107,6 +1107,7 @@
> #define PCI_DEVICE_ID_AL_M5219 0x5219
> #define PCI_DEVICE_ID_AL_M5228 0x5228
> #define PCI_DEVICE_ID_AL_M5229 0x5229
> +#define PCI_DEVICE_ID_AL_M5237 0x5237
> #define PCI_DEVICE_ID_AL_M5451 0x5451
> #define PCI_DEVICE_ID_AL_M7101 0x7101
There's no reason to add an ID to pci_ids.h if it's only going to be
used in one source file. Put the ID in the source file instead.
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 v4] usb: core: buffer: smallest buffer should start at ARCH_DMA_MINALIGN
On Fri, 5 Dec 2014, Sebastian Andrzej Siewior wrote: > the following error pops up during "testusb -a -t 10" > | musb-hdrc musb-hdrc.1.auto: dma_pool_free buffer-128, > f134e000/be842000 (bad dma) > hcd_buffer_create() creates a few buffers, the smallest has 32 bytes of > size. ARCH_KMALLOC_MINALIGN is set to 64 bytes. This combo results in > hcd_buffer_alloc() returning memory which is 32 bytes aligned and it > might by identified by buffer_offset() as another buffer. This means the > buffer which is on a 32 byte boundary will not get freed, instead it > tries to free another buffer with the error message. > > This patch fixes the issue by creating the smallest DMA buffer with the > size of ARCH_KMALLOC_MINALIGN (or 32 in case ARCH_KMALLOC_MINALIGN is > smaller). This might be 32, 64 or even 128 bytes. The next three pools > will have the size 128, 512 and 2048. > In case the smallest pool is 128 bytes then we have only three pools > instead of four (and zero the first entry in the array). > The last pool size is always 2048 bytes which is the assumed PAGE_SIZE / > 2 of 4096. I doubt it makes sense to continue using PAGE_SIZE / 2 where > we would end up with 8KiB buffer in case we have 16KiB pages. > Instead I think it makes sense to have a common size(s) and extend them > if there is need to. > There is a BUILD_BUG_ON() now in case someone has a minalign of more than > 128 bytes. > > Cc: sta...@vger.kernel.org > Signed-off-by: Sebastian Andrzej Siewior > --- > v3…v4: rewrite as suggested per Alan Stern so it is less confusing. Acked-by: Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: hcd: get/put device and hcd for hcd_buffers()
On Fri, Dec 05, 2014 at 09:03:57PM +0100, Sebastian Andrzej Siewior wrote: > Consider the following scenario: > - plugin a webcam > - play the stream via gst-launch-0.10 v4l2src device=/dev/video0… > - remove the USB-HCD during playback via "rmmod $HCD" > > and now wait for the crash Which you deserve, why did you ever remove a kernel module? That's racy and _never_ recommended, which is why it never happens automatically and only root can do it. > |musb-hdrc musb-hdrc.2.auto: remove, state 1 > |usb usb2: USB disconnect, device number 1 > |usb 2-1: USB disconnect, device number 3 > |uvcvideo: Failed to resubmit video URB (-19). > |musb-hdrc musb-hdrc.2.auto: USB bus 2 deregistered > |musb-hdrc musb-hdrc.1.auto: remove, state 4 > |usb usb1: USB disconnect, device number 1 > |musb-hdrc musb-hdrc.1.auto: USB bus 1 deregistered > |Unable to handle kernel paging request at virtual address 6b6b6b6f > |pgd = c0004000 > |[6b6b6b6f] *pgd= > |Internal error: Oops: 5 [#1] ARM > |Modules linked in: uvcvideo] > |CPU: 0 PID: 2613 Comm: gst-launch-0.10 Tainted: GW3.14.25+ #40 > |task: ec2b8100 ti: ec38e000 task.ti: ec38e000 > |PC is at hcd_buffer_free+0x64/0xc0 > |LR is at uvc_free_urb_buffers+0x34/0x50 [uvcvideo] > |Process gst-launch-0.10 (pid: 2613, stack limit = 0xec38e240) > |[] (hcd_buffer_free) > |[] (uvc_free_urb_buffers [uvcvideo]) > |[] (uvc_video_enable [uvcvideo]) > |[] (uvc_v4l2_release [uvcvideo]) > |[] (v4l2_release [videodev]) > |[] (__fput) > |[] (task_work_run) > |[] (do_exit) > |[] (do_group_exit) > > as part of the device-removal the HCD removes its dma-buffers, the HCD > structure itself and even the struct device is gone. That means if UVC > removes its URBs after its last user (/dev/videoX) is gone and not from > the ->disconnect() callback then it is too late because the HCD might > gone. > > First, I fixed by in the UVC driver by ignoring the UVC_SC_VIDEOSTREAMING > in its ->disconnect callback and calling uvc_video_enable(, 0) in > uvc_unregister_video(). But then I though that it might not be clever to > release that memory if there is userspace using it. > > So instead, I hold the device struct in the HCD and the HCD struct on > every USB-buf-alloc. That means after a disconnect we still have a > refcount on usb_hcd and device and it will be cleaned "later" once the > last USB-buffer is released. > > Signed-off-by: Sebastian Andrzej Siewior > --- > With this applied, I only see this three times (which is not new) > > | [ cut here ] > | WARNING: CPU: 0 PID: 1755 at fs/sysfs/group.c:219 > sysfs_remove_group+0x88/0x98() > | sysfs group c08a70d4 not found for kobject 'event4' > | Modules linked in: uvcvideo videobuf2_vmalloc videobuf2_memops > videobuf2_core v4l2_common videodev ipv6 musb_hdrc udc_core us] > | CPU: 0 PID: 1755 Comm: gst-launch-0.10 Not tainted > 3.18.0-rc7-00065-gabcefb342fbf-dirty #1813 > | [] (unwind_backtrace) from [] (show_stack+0x10/0x14) > | [] (show_stack) from [] (dump_stack+0x80/0x9c) > | [] (dump_stack) from [] (warn_slowpath_common+0x68/0x8c) > | [] (warn_slowpath_common) from [] > (warn_slowpath_fmt+0x30/0x40) > | [] (warn_slowpath_fmt) from [] > (sysfs_remove_group+0x88/0x98) > | [] (sysfs_remove_group) from [] (device_del+0x34/0x198) > | [] (device_del) from [] (evdev_disconnect+0x18/0x44) > | [] (evdev_disconnect) from [] > (__input_unregister_device+0xa4/0x148) > | [] (__input_unregister_device) from [] > (input_unregister_device+0x40/0x74) > | [] (input_unregister_device) from [] > (uvc_delete+0x20/0x10c [uvcvideo]) > | [] (uvc_delete [uvcvideo]) from [] > (v4l2_device_release+0x9c/0xc4 [videodev]) > | [] (v4l2_device_release [videodev]) from [] > (device_release+0x2c/0x90) > | [] (device_release) from [] (kobject_release+0x48/0x7c) > | [] (kobject_release) from [] (v4l2_release+0x50/0x78 > [videodev]) > | [] (v4l2_release [videodev]) from [] (__fput+0x80/0x1c4) > | [] (__fput) from [] (task_work_run+0xb4/0xe4) > | [] (task_work_run) from [] (do_exit+0x2dc/0x92c) > | [] (do_exit) from [] (do_group_exit+0x3c/0xb0) > | [] (do_group_exit) from [] (__wake_up_parent+0x0/0x18) > | ---[ end trace b54a8f3c8129180e ]--- > anyone an idea? > > drivers/usb/core/buffer.c | 30 +- > drivers/usb/core/hcd.c| 2 ++ > 2 files changed, 27 insertions(+), 5 deletions(-) > > diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c > index 506b969ea7fd..01e080a61519 100644 > --- a/drivers/usb/core/buffer.c > +++ b/drivers/usb/core/buffer.c > @@ -107,7 +107,7 @@ void hcd_buffer_destroy(struct usb_hcd *hcd) > * better sharing and to leverage mm/slab.c intelligence. > */ > > -void *hcd_buffer_alloc( > +static void *_hcd_buffer_alloc( Looks like this isn't really needed here, right? > struct usb_bus *bus, > size_t size, > gfp_t mem_flags, > @@ -131,7 +131,19 @@ void *hcd_buffer_alloc( > return dma_alloc_coh
Re: [PATCH] usb: hcd: get/put device and hcd for hcd_buffers()
On Fri, 5 Dec 2014, Sebastian Andrzej Siewior wrote: > Consider the following scenario: > - plugin a webcam > - play the stream via gst-launch-0.10 v4l2src device=/dev/video0… > - remove the USB-HCD during playback via "rmmod $HCD" > > and now wait for the crash > > |musb-hdrc musb-hdrc.2.auto: remove, state 1 > |usb usb2: USB disconnect, device number 1 > |usb 2-1: USB disconnect, device number 3 > |uvcvideo: Failed to resubmit video URB (-19). > |musb-hdrc musb-hdrc.2.auto: USB bus 2 deregistered > |musb-hdrc musb-hdrc.1.auto: remove, state 4 > |usb usb1: USB disconnect, device number 1 > |musb-hdrc musb-hdrc.1.auto: USB bus 1 deregistered > |Unable to handle kernel paging request at virtual address 6b6b6b6f > |pgd = c0004000 > |[6b6b6b6f] *pgd= > |Internal error: Oops: 5 [#1] ARM > |Modules linked in: uvcvideo] > |CPU: 0 PID: 2613 Comm: gst-launch-0.10 Tainted: GW3.14.25+ #40 > |task: ec2b8100 ti: ec38e000 task.ti: ec38e000 > |PC is at hcd_buffer_free+0x64/0xc0 > |LR is at uvc_free_urb_buffers+0x34/0x50 [uvcvideo] > |Process gst-launch-0.10 (pid: 2613, stack limit = 0xec38e240) > |[] (hcd_buffer_free) > |[] (uvc_free_urb_buffers [uvcvideo]) > |[] (uvc_video_enable [uvcvideo]) > |[] (uvc_v4l2_release [uvcvideo]) > |[] (v4l2_release [videodev]) > |[] (__fput) > |[] (task_work_run) > |[] (do_exit) > |[] (do_group_exit) > > as part of the device-removal the HCD removes its dma-buffers, the HCD > structure itself and even the struct device is gone. That means if UVC > removes its URBs after its last user (/dev/videoX) is gone and not from > the ->disconnect() callback then it is too late because the HCD might > gone. > > First, I fixed by in the UVC driver by ignoring the UVC_SC_VIDEOSTREAMING > in its ->disconnect callback and calling uvc_video_enable(, 0) in > uvc_unregister_video(). But then I though that it might not be clever to > release that memory if there is userspace using it. > > So instead, I hold the device struct in the HCD and the HCD struct on > every USB-buf-alloc. That means after a disconnect we still have a > refcount on usb_hcd and device and it will be cleaned "later" once the > last USB-buffer is released. This is not a valid solution. Notice that your _hcd_buffer_free still dereferences hcd->driver; that will not point to anything useful if you rmmod the HCD. Also, you neglected to move the calls to hcd_buffer_destroy from usb_remove_hcd to hcd_release. On the whole, it would be easier if the UVC driver could release its coherent DMA buffers during the disconnect callback. If that's not feasible we'll have to find some other solution. 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
[PATCH 2/2] usb: ehci-platform: add support for multiple phys per controller
From: Arun Ramamurthy
Added support for cases where one controller is connected
to multiple phys.
Signed-off-by: Arun Ramamurthy
Reviewed-by: Ray Jui
Reviewed-by: Scott Branden
---
drivers/usb/host/ehci-platform.c | 70
1 file changed, 49 insertions(+), 21 deletions(-)
diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
index 2f5b9ce..a1a7d82 100644
--- a/drivers/usb/host/ehci-platform.c
+++ b/drivers/usb/host/ehci-platform.c
@@ -43,7 +43,8 @@
struct ehci_platform_priv {
struct clk *clks[EHCI_MAX_CLKS];
struct reset_control *rst;
- struct phy *phy;
+ struct phy *phys;
+ int num_phys;
};
static const char hcd_name[] = "ehci-platform";
@@ -78,7 +79,7 @@ static int ehci_platform_power_on(struct platform_device *dev)
{
struct usb_hcd *hcd = platform_get_drvdata(dev);
struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd);
- int clk, ret;
+ int clk, ret, phy_num;
for (clk = 0; clk < EHCI_MAX_CLKS && priv->clks[clk]; clk++) {
ret = clk_prepare_enable(priv->clks[clk]);
@@ -86,20 +87,28 @@ static int ehci_platform_power_on(struct platform_device
*dev)
goto err_disable_clks;
}
- if (priv->phy) {
- ret = phy_init(priv->phy);
- if (ret)
- goto err_disable_clks;
-
- ret = phy_power_on(priv->phy);
- if (ret)
+ for (phy_num = 0; phy_num < priv->num_phys; phy_num++) {
+ ret = phy_init(&priv->phys[phy_num]);
+ if (ret) {
+ if (phy_num == 0)
+ goto err_disable_clks;
+ else
+ goto err_exit_phy;
+ }
+ ret = phy_power_on(&priv->phys[phy_num]);
+ if (ret) {
+ phy_exit(&priv->phys[phy_num]);
goto err_exit_phy;
+ }
}
return 0;
err_exit_phy:
- phy_exit(priv->phy);
+ while (--phy_num >= 0) {
+ phy_power_off(&priv->phys[phy_num]);
+ phy_exit(&priv->phys[phy_num]);
+ }
err_disable_clks:
while (--clk >= 0)
clk_disable_unprepare(priv->clks[clk]);
@@ -111,11 +120,11 @@ static void ehci_platform_power_off(struct
platform_device *dev)
{
struct usb_hcd *hcd = platform_get_drvdata(dev);
struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd);
- int clk;
+ int clk, phy_num;
- if (priv->phy) {
- phy_power_off(priv->phy);
- phy_exit(priv->phy);
+ for (phy_num = 0; phy_num < priv->num_phys; phy_num++) {
+ phy_power_off(&priv->phys[phy_num]);
+ phy_exit(&priv->phys[phy_num]);
}
for (clk = EHCI_MAX_CLKS - 1; clk >= 0; clk--)
@@ -143,7 +152,9 @@ static int ehci_platform_probe(struct platform_device *dev)
struct usb_ehci_pdata *pdata = dev_get_platdata(&dev->dev);
struct ehci_platform_priv *priv;
struct ehci_hcd *ehci;
- int err, irq, clk = 0;
+ struct phy *temp_phy;
+ const char *phy_name;
+ int err, irq, phy_num, clk = 0;
if (usb_disabled())
return -ENODEV;
@@ -190,12 +201,29 @@ static int ehci_platform_probe(struct platform_device
*dev)
if (of_property_read_bool(dev->dev.of_node, "big-endian"))
ehci->big_endian_mmio = ehci->big_endian_desc = 1;
- priv->phy = devm_phy_get(&dev->dev, "usb");
- if (IS_ERR(priv->phy)) {
- err = PTR_ERR(priv->phy);
- if (err == -EPROBE_DEFER)
- goto err_put_hcd;
- priv->phy = NULL;
+ priv->num_phys = of_count_phandle_with_args
+ (dev->dev.of_node, "phys", "#phy-cells");
+ if (priv->num_phys > 0) {
+ priv->phys = devm_kcalloc(&dev->dev, priv->num_phys,
+ sizeof(struct phy), GFP_KERNEL);
+ for (phy_num = 0; phy_num < priv->num_phys; phy_num++) {
+ err = of_property_read_string_index(
+ dev->dev.of_node,
+ "phy-names", phy_num,
+ &phy_name);
+ if (err < 0) {
+ dev_err(&dev->dev, "Phy-names not
provided");
+ goto err_put_hcd;
+ }
+
+ temp_phy = devm_of_phy_get(&dev->dev,
+ dev->dev.of_node, phy_name);
+ if (IS_ERR(temp_ph
[PATCH 0/2] Added Multiple Phy support for ehci and ohci drivers
From: Arun Ramamurthy Broadcom has a chip where one ehci and ohci controller are connected to three separate phys. This patch allows each phy to be controlled separately. Arun Ramamurthy (2): usb: ohci-platform: add support for multiple phys per controller usb: ehci-platform: add support for multiple phys per controller drivers/usb/host/ehci-platform.c | 70 drivers/usb/host/ohci-platform.c | 70 2 files changed, 98 insertions(+), 42 deletions(-) -- 2.2.0 -- 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: ohci-platform: add support for multiple phys per controller
From: Arun Ramamurthy
Added support for cases where one controller is connected
to multiple phys
Signed-off-by: Arun Ramamurthy
Reviewed-by: Ray Jui
Reviewed-by: Scott Branden
---
drivers/usb/host/ohci-platform.c | 70
k 1 file changed, 49 insertions(+), 21 deletions(-)
diff --git a/drivers/usb/host/ohci-platform.c b/drivers/usb/host/ohci-platform.c
index 4369299..eef82f1 100644
--- a/drivers/usb/host/ohci-platform.c
+++ b/drivers/usb/host/ohci-platform.c
@@ -38,7 +38,8 @@
struct ohci_platform_priv {
struct clk *clks[OHCI_MAX_CLKS];
struct reset_control *rst;
- struct phy *phy;x
+ struct phy *phys;
+ int num_phys;
};
static const char hcd_name[] = "ohci-platform";
@@ -61,7 +62,7 @@ static int ohci_platform_power_on(struct platform_device *dev)
{
struct usb_hcd *hcd = platform_get_drvdata(dev);
struct ohci_platform_priv *priv = hcd_to_ohci_priv(hcd);
- int clk, ret;
+ int clk, ret, phy_num;
for (clk = 0; clk < OHCI_MAX_CLKS && priv->clks[clk]; clk++) {
ret = clk_prepare_enable(priv->clks[clk]);
@@ -69,20 +70,28 @@ static int ohci_platform_power_on(struct platform_device
*dev)
goto err_disable_clks;
}
- if (priv->phy) {
- ret = phy_init(priv->phy);
- if (ret)
- goto err_disable_clks;
-
- ret = phy_power_on(priv->phy);
- if (ret)
+ for (phy_num = 0; phy_num < priv->num_phys; phy_num++) {
+ ret = phy_init(&priv->phys[phy_num]);
+ if (ret) {
+ if (phy_num == 0)
+ goto err_disable_clks;
+ else
+ goto err_exit_phy;
+ }
+ ret = phy_power_on(&priv->phys[phy_num]);
+ if (ret) {
+ phy_exit(&priv->phys[phy_num]);
goto err_exit_phy;
+ }
}
return 0;
err_exit_phy:
- phy_exit(priv->phy);
+ while (--phy_num >= 0) {
+ phy_power_off(&priv->phys[phy_num]);
+ phy_exit(&priv->phys[phy_num]);
+ };
err_disable_clks:
while (--clk >= 0)
clk_disable_unprepare(priv->clks[clk]);
@@ -94,11 +103,11 @@ static void ohci_platform_power_off(struct platform_device
*dev)
{
struct usb_hcd *hcd = platform_get_drvdata(dev);
struct ohci_platform_priv *priv = hcd_to_ohci_priv(hcd);
- int clk;
+ int clk, phy_num;
- if (priv->phy) {
- phy_power_off(priv->phy);
- phy_exit(priv->phy);
+ for (phy_num = 0; phy_num < priv->num_phys; phy_num++) {
+ phy_power_off(&priv->phys[phy_num]);
+ phy_exit(&priv->phys[phy_num]);
}
for (clk = OHCI_MAX_CLKS - 1; clk >= 0; clk--)
@@ -127,7 +136,9 @@ static int ohci_platform_probe(struct platform_device *dev)
struct usb_ohci_pdata *pdata = dev_get_platdata(&dev->dev);
struct ohci_platform_priv *priv;
struct ohci_hcd *ohci;
- int err, irq, clk = 0;
+ struct phy *temp_phy;
+ const char *phy_name;
+ int err, irq, phy_num, clk = 0;
if (usb_disabled())
return -ENODEV;
@@ -175,12 +186,29 @@ static int ohci_platform_probe(struct platform_device
*dev)
if (of_property_read_bool(dev->dev.of_node, "big-endian"))
ohci->flags |= OHCI_QUIRK_BE_MMIO | OHCI_QUIRK_BE_DESC;
- priv->phy = devm_phy_get(&dev->dev, "usb");
- if (IS_ERR(priv->phy)) {
- err = PTR_ERR(priv->phy);
- if (err == -EPROBE_DEFER)
- goto err_put_hcd;
- priv->phy = NULL;
+ priv->num_phys = of_count_phandle_with_args
+ (dev->dev.of_node, "phys", "#phy-cells");
+ if (priv->num_phys > 0) {
+ priv->phys = devm_kcalloc(&dev->dev, priv->num_phys,
+ sizeof(struct phy), GFP_KERNEL);
+ for (phy_num = 0; phy_num < priv->num_phys; phy_num++) {
+ err = of_property_read_string_index(
+ dev->dev.of_node,
+ "phy-names", phy_num,
+ &phy_name);
+ if (err < 0) {
+ dev_err(&dev->dev, "Phy-names not
provided");
+ goto err_put_hcd;
+ }
+
+ temp_phy = devm_of_phy_get(&dev->dev,
+ dev->dev.of_node, phy_name);
+ if (IS_ERR(t
Re: USB 3.0 drive crashes system when plugged in - regression
On 2014-12-05 11:00, Hans de Goede wrote: > Hi, > > On 12/05/2014 12:35 AM, Marcin Zajączkowski wrote: >> On 2014-12-04 20:21, Hans de Goede wrote: >>> Hi, >>> >>> On 12/03/2014 10:22 PM, Marcin Zajączkowski wrote: On 2014-12-03 10:53, Oliver Neukum wrote: > On Wed, 2014-12-03 at 10:41 +0100, Marcin Zajączkowski wrote: >> 2014-12-03 Oliver Neukum wrote: >>> On Wed, 2014-12-03 at 00:29 +0100, Marcin Zajączkowski wrote: Hi, After upgrade to Fedora 21 with 3.17.3-300.fc21.x86_64 (from 3.14.x in Fedora 19) my USB 3.0 drive (Seagate Backup+ 1TB) stopped working with USB 3.0 port (works fine with USB 2.0 port). When plug in for the first time (USB 3.0 port) I see in log: > kernel: xhci_hcd :04:00.0: ERROR Transfer event for disabled endpoint or incorrect stream ring > kernel: xhci_hcd :04:00.0: @000241eec570 11979000 0002 0500 01078001 >>> >>> Are you using the UAS driver? >> >> Probably yes. How can check that and/or switch to not UAS driver for >> a test? > > Sysfs holds the information. I can be comfortably queried with the > "usb-devices" script. > > To switch drivers you can use the "no UAS" quirk which can be given as > a module parameter like other quirks. Thanks Olivier for your reply. UAS keyword helped me to ignore UAS for my drive (via `options usb-storage quirks=vendorId:productId:u`) and my drive can again be used via USB 3.0 port. >> (...) >> What would you propose to do with the issue with that Seagate drive (family?)? To either track it or to temporary disable UAS for them globally (to not produce system crashes). >>> >>> If you have a seagate driver you may need a US_FL_NO_ATA_1X quirk >>> to disable ata pass through commands, as all (current) seagate devices >> >> How can I do that from the modprobe level? > > With the quirk line I've provided below (and you've already tested). > >>> seem to bork on this. What is the usb-id of your seagate device ? >> >> idVendor=0bc2, idProduct=a013 > > Hmm, that one does not yet have the US_FL_NO_ATA_1X quirk most seagate > devices > seem to need. That is not the problem in your case (probing does not get so > far that it matters), but this does seem to be one of the devices which > need > one looking at the model-name and usb-id. So I'll add a quirk for it to > help > out other users. > >>> And can you try with options usb-storage quirks=vendorId:productId:t ? >> >> I tried "quirks=vendorId:productId:t" and it failed. The system reported >> errors when a drive was connected: > > Ok, I already suspected as much, since this seems to be an xhci controller > problem, not an uas problem. > >>> 21:37:32 kernel: usb 4-1: new SuperSpeed USB device number 2 using >>> xhci_hcd >>> 21:37:32 kernel: usb 4-1: New USB device found, idVendor=0bc2, >>> idProduct=a013 >>> 21:37:32 kernel: usb 4-1: New USB device strings: Mfr=2, Product=3, >>> SerialNumber=1 >>> 21:37:32 kernel: usb 4-1: Product: Backup+ BK >>> 21:37:32 kernel: usb 4-1: Manufacturer: Seagate >>> 21:37:32 kernel: usb 4-1: SerialNumber: >>> 21:37:33 mtp-probe[2777]: checking bus 4, device 2: >>> "/sys/devices/pci:00/:00:1c.3/:04:00.0/usb4/4-1" >>> 21:37:33 mtp-probe[2777]: bus: 4, device: 2 was not an MTP device >>> 21:37:33 kernel: usbcore: registered new interface driver usb-storage >>> 21:37:33 kernel: scsi host6: uas >>> 21:37:33 kernel: usbcore: registered new interface driver uas >>> 21:37:33 kernel: xhci_hcd :04:00.0: ERROR Transfer event for >>> disabled endpoint or incorrect stream ring >>> 21:37:33 kernel: xhci_hcd :04:00.0: @368b3570 9067b000 >>> 0500 01078001 >>> 21:37:33 kernel: xhci_hcd :04:00.0: ERROR Transfer event for >>> disabled endpoint or incorrect stream ring >>> 21:37:33 kernel: xhci_hcd :04:00.0: @368b3580 9067b400 >>> 0500 01038001 >>> 21:37:53 kernel: scsi 6:0:0:0: uas_eh_abort_handler 88003653ee80 >>> tag -1, inflight: CMD IN >>> 21:37:53 kernel: xhci_hcd :04:00.0: WARN waiting for error on ep >>> to be cleared >>> 21:37:53 kernel: scsi 6:0:0:0: uas_submit_sense_urb 88003653ee80 >>> tag -1, inflight: CMD IN abort >>> 21:37:53 kernel: scsi host6: sense urb submission error -22 stream 32 >>> 21:37:53 kernel: scsi host6: uas_eh_task_mgmt: ABORT TASK: submit >>> sense urb failed >>> 21:37:53 kernel: scsi 6:0:0:0: uas_eh_device_reset_handler >>> 21:37:53 kernel: xhci_hcd :04:00.0: WARN waiting for error on ep >>> to be cleared >>> 21:37:53 kernel: scsi 6:0:0:0: uas_submit_sense_urb 88003653ee80 >>> tag -1, inflight: CMD IN abort >>> 21:37:53 kernel: scsi host6: sense urb submission error -22 stream 32 >>> 21:37:53 kernel: scsi host6: uas_eh_task_mgmt: LOGICAL UNIT RESET: >>> submit sense urb failed >>> 21:37:53 kernel: scsi host6: uas_eh_bus_reset_ha
Re: [PATCH] usb: hcd: get/put device and hcd for hcd_buffers()
* Greg Kroah-Hartman | 2014-12-05 13:19:32 [-0800]:
>On Fri, Dec 05, 2014 at 09:03:57PM +0100, Sebastian Andrzej Siewior wrote:
>> Consider the following scenario:
>> - plugin a webcam
>> - play the stream via gst-launch-0.10 v4l2src device=/dev/video0…
>> - remove the USB-HCD during playback via "rmmod $HCD"
>>
>> and now wait for the crash
>
>Which you deserve, why did you ever remove a kernel module? That's racy
its been found by the testing team and looks legitimate.
>and _never_ recommended, which is why it never happens automatically and
>only root can do it.
I beg your pardon. So it is okay to remove the UVC-driver / plug the
cable and expect that things continue to work but removing the HCD is a
no no? I always assumed that kernel should BUG() no matter what the user
does unless he really begs for it. If there is a race then it is a bug
that deserves to be fixed, right?
>> diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c
>> index 506b969ea7fd..01e080a61519 100644
>> --- a/drivers/usb/core/buffer.c
>> +++ b/drivers/usb/core/buffer.c
>> @@ -107,7 +107,7 @@ void hcd_buffer_destroy(struct usb_hcd *hcd)
>> * better sharing and to leverage mm/slab.c intelligence.
>> */
>>
>> -void *hcd_buffer_alloc(
>> +static void *_hcd_buffer_alloc(
>
>Looks like this isn't really needed here, right?
either this or I would have the tree callers if the allocation succeded
or not in order not to take a reference if the allocation failed.
>> struct usb_bus *bus,
>> size_t size,
>> gfp_t mem_flags,
>> @@ -131,7 +131,19 @@ void *hcd_buffer_alloc(
>> return dma_alloc_coherent(hcd->self.controller, size, dma, mem_flags);
>> }
>>
>> -void hcd_buffer_free(
>> +void *hcd_buffer_alloc(struct usb_bus *bus, size_t size, gfp_t mem_flags,
>> + dma_addr_t *dma)
>> +{
>> +struct usb_hcd *hcd = bus_to_hcd(bus);
>> +void *ret;
>> +
>> +ret = _hcd_buffer_alloc(bus, size, mem_flags, dma);
>> +if (ret)
>> +usb_get_hcd(hcd);
>
>I'm all for some good reference counting, but this is going to cause a
>_lot_ of churn on this reference count, what is the performance issue
>with doing this for every buffer?
The UVC allocates the buffers once and reuses them. If a driver does
any kind of high-performance transfers and allocates new buffers on each
transfer then I would expect this kref_get() is in the noise area. But
if you want real numbers I would have to go ahead and test it.
A single get() on first allocation and its counter part on cleanup would
be enough if you are too concerned about it on every allocation (it
would be transparent to the user).
>thanks,
>
>greg k-h
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: hcd: get/put device and hcd for hcd_buffers()
* Alan Stern | 2014-12-05 16:21:02 [-0500]: >On Fri, 5 Dec 2014, Sebastian Andrzej Siewior wrote: >> So instead, I hold the device struct in the HCD and the HCD struct on >> every USB-buf-alloc. That means after a disconnect we still have a >> refcount on usb_hcd and device and it will be cleaned "later" once the >> last USB-buffer is released. > >This is not a valid solution. Notice that your _hcd_buffer_free still >dereferences hcd->driver; that will not point to anything useful if you >rmmod the HCD. Hmm. You're right, that one is gone. >Also, you neglected to move the calls to hcd_buffer_destroy from >usb_remove_hcd to hcd_release. I add them, I didn't move them. >On the whole, it would be easier if the UVC driver could release its >coherent DMA buffers during the disconnect callback. If that's not >feasible we'll have to find some other solution. I had one patch doing that. Let me grab it out on Monday. >Alan Stern > Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: hcd: get/put device and hcd for hcd_buffers()
On Sat, Dec 06, 2014 at 12:13:13AM +0100, Sebastian Andrzej Siewior wrote: > * Greg Kroah-Hartman | 2014-12-05 13:19:32 [-0800]: > > >On Fri, Dec 05, 2014 at 09:03:57PM +0100, Sebastian Andrzej Siewior wrote: > >> Consider the following scenario: > >> - plugin a webcam > >> - play the stream via gst-launch-0.10 v4l2src device=/dev/video0… > >> - remove the USB-HCD during playback via "rmmod $HCD" > >> > >> and now wait for the crash > > > >Which you deserve, why did you ever remove a kernel module? That's racy > its been found by the testing team and looks legitimate. > > >and _never_ recommended, which is why it never happens automatically and > >only root can do it. > I beg your pardon. So it is okay to remove the UVC-driver / plug the > cable and expect that things continue to work but removing the HCD is a > no no? If you hot unplug the HCD and this is an issue, yes, that's something to fix. If you can only trigger this by unloading a kernel module, no, it's not a big issue at all. It's pretty trivial to cause kernel oopses by unloading kernel modules if you know what you are doing. > >> diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c > >> index 506b969ea7fd..01e080a61519 100644 > >> --- a/drivers/usb/core/buffer.c > >> +++ b/drivers/usb/core/buffer.c > >> @@ -107,7 +107,7 @@ void hcd_buffer_destroy(struct usb_hcd *hcd) > >> * better sharing and to leverage mm/slab.c intelligence. > >> */ > >> > >> -void *hcd_buffer_alloc( > >> +static void *_hcd_buffer_alloc( > > > >Looks like this isn't really needed here, right? > > either this or I would have the tree callers if the allocation succeded > or not in order not to take a reference if the allocation failed. My point is this isn't needed for this patch. 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 0/2] Added Multiple Phy support for ehci and ohci drivers
On Fri, 5 Dec 2014 arun.ramamur...@broadcom.com wrote: > From: Arun Ramamurthy > > Broadcom has a chip where one ehci and ohci controller are connected > to three separate phys. This patch allows each phy to be controlled > separately. > > Arun Ramamurthy (2): > usb: ohci-platform: add support for multiple phys per controller > usb: ehci-platform: add support for multiple phys per controller > > drivers/usb/host/ehci-platform.c | 70 > > drivers/usb/host/ohci-platform.c | 70 > > 2 files changed, 98 insertions(+), 42 deletions(-) I'm going on vacation starting tomorrow, with limited internet connectivity. I probably won't review these patches for at least a week. 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
[PATCH v2] OHCI: add a quirk for ULi M5237 blocking on reset
From: Arseny Solokha
Commit 8dccddbc2368 ("OHCI: final fix for NVIDIA problems (I hope)")
introduced into 3.1.9 broke boot on e.g. Freescale P2020DS development
board. The code path that was previously specific to NVIDIA controllers
had then become taken for all chips.
However, the M5237 installed on the board wedges solid when accessing
its base+OHCI_FMINTERVAL register, making it impossible to boot any
kernel newer than 3.1.8 on this particular and apparently other similar
machines.
Don't readl() and writel() base+OHCI_FMINTERVAL on PCI ID 10b9:5237.
The patch is suitable for the -next tree as well as all maintained
kernels up to 3.2 inclusive.
Signed-off-by: Arseny Solokha
---
Changes in v2:
- review comments applied
---
drivers/usb/host/pci-quirks.c | 18 +++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 2f3aceb..f4e6b94 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -571,7 +571,8 @@ static void quirk_usb_handoff_ohci(struct pci_dev *pdev)
{
void __iomem *base;
u32 control;
- u32 fminterval;
+ u32 fminterval = 0;
+ bool no_fminterval = false;
int cnt;
if (!mmio_resource_enabled(pdev, 0))
@@ -581,6 +582,13 @@ static void quirk_usb_handoff_ohci(struct pci_dev *pdev)
if (base == NULL)
return;
+ /*
+* ULi M5237 OHCI controller locks the whole system when accessing
+* the OHCI_FMINTERVAL offset.
+*/
+ if (pdev->vendor == PCI_VENDOR_ID_AL && pdev->device == 0x5237)
+ no_fminterval = true;
+
control = readl(base + OHCI_CONTROL);
/* On PA-RISC, PDC can leave IR set incorrectly; ignore it there. */
@@ -619,7 +627,9 @@ static void quirk_usb_handoff_ohci(struct pci_dev *pdev)
}
/* software reset of the controller, preserving HcFmInterval */
- fminterval = readl(base + OHCI_FMINTERVAL);
+ if (!no_fminterval)
+ fminterval = readl(base + OHCI_FMINTERVAL);
+
writel(OHCI_HCR, base + OHCI_CMDSTATUS);
/* reset requires max 10 us delay */
@@ -628,7 +638,9 @@ static void quirk_usb_handoff_ohci(struct pci_dev *pdev)
break;
udelay(1);
}
- writel(fminterval, base + OHCI_FMINTERVAL);
+
+ if (!no_fminterval)
+ writel(fminterval, base + OHCI_FMINTERVAL);
/* Now the controller is safely in SUSPEND and nothing can wake it up */
iounmap(base);
--
2.2.0
--
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
