RE: UAC2 gadget not recognized on Windows 10
Hi, Robert Bielik writes: >> Enabling SOF interrupts will be a big pain :-) Well, enabling the >> interrupt itself is a no-brainer, but it'll cause terrible CPU overload. > > Oh, I see. Hmm... would it be possible to allow upper levels to config > this dynamically ? I.e. for the ALSA subsystem there is no need for > the SOF timestamps, whereas for my proposal they would be needed. > > And what kind of CPU overhead are we talking about ? The IRQs > shouldn't come more often than every 125 us, and all that is needed > is to take a timestamp value 😊 But I'm probably overlooking a lot of > stuff... that's exactly the problem. Every 125us you're gonna interrupt the cpu just to take a timestamp. Are you gonna use every timestamp? Probably not :) One thing you could do is enable SOF interrupt and look at cpu utilization. -- balbi signature.asc Description: PGP signature
Re: [PATCH] usb: dwc3: of-simple: fix use-after-free on remove
On 21/06/18 01:55, Rafael J. Wysocki wrote: > On Thu, Jun 21, 2018 at 12:32 AM, Rafael J. Wysocki wrote: >> On Wed, Jun 20, 2018 at 5:46 PM, Johan Hovold wrote: >>> On Wed, Jun 20, 2018 at 02:54:10PM +0200, Rafael J. Wysocki wrote: On Wednesday, June 20, 2018 2:23:46 PM CEST Johan Hovold wrote: > On Wed, Jun 20, 2018 at 02:16:59AM -0700, Tony Lindgren wrote: >> Hi, >> >> Adding Rafael and linux-pm to Cc as well. >> >> * Felipe Balbi [180619 01:23]: >>> This is a direct consequence of not paying attention to the order of >>> things. If driver were to assume that pm_domain->activate() would do the >>> right thing for the device -- meaning that probe would run with an >>> active device --, then we wouldn't need that pm_runtime_get() call on >>> probe at all. Rather we would follow the sequence: >>> >>> pm_runtime_forbid() >>> pm_runtime_set_active() >>> pm_runtime_enable() >>> >>> /* do your probe routine */ >>> >>> pm_runtime_put_noidle() >>> >>> Then you remove you would need to call pm_runtime_get_noresume() to >>> balance out the pm_runtime_put_noidle() there. > >>> (If you need to know why the pm_runtime_put_noidle(), remember that >>> pm_runtime_set_active() increments the usage counter, so >>> pm_runtime_put_noidle is basically allowing pm_runtime to happen as soon >>> as userspace writes "auto" to /sys//power/control) > > That's not correct; pm_runtime_set_active() only increments the usage > counter of a parent (under some circumstances), so unless you have bus > code incrementing the usage counter before probe, the above > pm_runtime_put_noidle() would actually introduce an imbalance. No, it wouldn't. It balances the incrementation in pm_runtime_forbid(). >>> >>> Right, but even if you take the whole sequence, which included >>> pm_runtime_forbid(), consider what happens when pm_runtime_allow() is >>> later called through sysfs (see below). >>> > And note that that's also the case even if you meant to say that > *pm_runtime_forbid()* increments the usage counter (which it does). Why is it? Surely, after pm_runtime_forbid(dev); pm_runtime_put_noidle(dev); the runtime PM usage counter of dev will be the same as before, won't it? >>> >>> Sure, but the imbalance, or rather inconsistent state, has already been >>> introduced. >>> >>> Consider the following sequence of events: >>> >>> usage count >>> 0 >>> probe() >>> pm_runtime_forbid() 1 Can you call pm_runtime_forbid() before pm_runtime_enable()? Wouldn't it fail with -EACCES as dev->power.disable_depth > 0? >>> pm_runtime_set_active() >>> pm_runtime_enable() >>> pm_runtime_put_noidle() 0 >>> >>> Here nothing is preventing the device from runtime suspending, despite >>> runtime PM being forbidden. In fact, it will typically be suspended due >>> to the pm_request_idle() in driver_probe_device(). If later we have: >>> >>> echo auto > power/control >>> pm_runtime_allow() -1 >> >> OK, you have a point. >> >> After calling pm_runtime_forbid() the driver should allow user space >> to unblock runtime PM for the device - or call pm_runtime_allow() >> itself. > > The confusion regarding the pm_runtime_put_noidle() at the end may > come from the special requirement of the PCI bus type as per the > comment in local_pci_probe(). > OK. So it is the PCI bus which is behaving odd here and pm_runtime_put_noidle() needs to be done only if its a PCI device, correct? -- cheers, -roger Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki -- 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: dwc3: of-simple: fix use-after-free on remove
On Thu, Jun 21, 2018 at 12:32:59AM +0200, Rafael J. Wysocki wrote: > On Wed, Jun 20, 2018 at 5:46 PM, Johan Hovold wrote: > > On Wed, Jun 20, 2018 at 02:54:10PM +0200, Rafael J. Wysocki wrote: > >> On Wednesday, June 20, 2018 2:23:46 PM CEST Johan Hovold wrote: > >> > On Wed, Jun 20, 2018 at 02:16:59AM -0700, Tony Lindgren wrote: > >> > > Hi, > >> > > > >> > > Adding Rafael and linux-pm to Cc as well. > >> > > > >> > > * Felipe Balbi [180619 01:23]: > >> > > > This is a direct consequence of not paying attention to the order of > >> > > > things. If driver were to assume that pm_domain->activate() would do > >> > > > the > >> > > > right thing for the device -- meaning that probe would run with an > >> > > > active device --, then we wouldn't need that pm_runtime_get() call on > >> > > > probe at all. Rather we would follow the sequence: > >> > > > > >> > > > pm_runtime_forbid() > >> > > > pm_runtime_set_active() > >> > > > pm_runtime_enable() > >> > > > > >> > > > /* do your probe routine */ > >> > > > > >> > > > pm_runtime_put_noidle() > >> > > > > >> > > > Then you remove you would need to call pm_runtime_get_noresume() to > >> > > > balance out the pm_runtime_put_noidle() there. > >> > > >> > > > (If you need to know why the pm_runtime_put_noidle(), remember that > >> > > > pm_runtime_set_active() increments the usage counter, so > >> > > > pm_runtime_put_noidle is basically allowing pm_runtime to happen as > >> > > > soon > >> > > > as userspace writes "auto" to /sys//power/control) > >> > > >> > That's not correct; pm_runtime_set_active() only increments the usage > >> > counter of a parent (under some circumstances), so unless you have bus > >> > code incrementing the usage counter before probe, the above > >> > pm_runtime_put_noidle() would actually introduce an imbalance. > >> > >> No, it wouldn't. It balances the incrementation in pm_runtime_forbid(). > > > > Right, but even if you take the whole sequence, which included > > pm_runtime_forbid(), consider what happens when pm_runtime_allow() is > > later called through sysfs (see below). > > > >> > And note that that's also the case even if you meant to say that > >> > *pm_runtime_forbid()* increments the usage counter (which it does). > >> > >> Why is it? > >> > >> Surely, after > >> > >> pm_runtime_forbid(dev); > >> pm_runtime_put_noidle(dev); > >> > >> the runtime PM usage counter of dev will be the same as before, won't it? > > > > Sure, but the imbalance, or rather inconsistent state, has already been > > introduced. > > > > Consider the following sequence of events: > > > > usage count > > 0 > > probe() > > pm_runtime_forbid() 1 > > pm_runtime_set_active() > > pm_runtime_enable() > > pm_runtime_put_noidle() 0 > > > > Here nothing is preventing the device from runtime suspending, despite > > runtime PM being forbidden. In fact, it will typically be suspended due > > to the pm_request_idle() in driver_probe_device(). If later we have: > > > > echo auto > power/control > > pm_runtime_allow() -1 > > OK, you have a point. > > After calling pm_runtime_forbid() the driver should allow user space > to unblock runtime PM for the device - or call pm_runtime_allow() > itself. Right, the usage count increment done by forbid() should only be balanced by allow(). > [cut] > > > > > And if runtime pm is later again forbidden: > > > > echo on > power/control > > pm_runtime_forbid() 0 > > > > then the device will not be resumed. > > But I don't quite see why that will be the case. rpm_resume() will > still be called and it doesn't look at the usage counter. You're right, I left out some important details and jumped to the conclusion. As you point out, the device will be unconditionally resumed, but due to the usage counter being zero the idle notification queued by rpm_resume() will typically suspend it straight away despite runtime pm being forbidden (cf. the idle notification issued by driver core after probe() returns). 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: usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers
+Lars-Peter
Vincent,
On 14/06/18 16:23, Sam Protsenko wrote:
> + Roger Quadros
> + Praneeth Bajjuri
>
> Tested-by: Sam Protsenko
>
> I've tested it on X15 board (DWC3 controller) on Android master, by
> doing "adb root". Without this patch I see backtrace and kernel panic
> (the same error as described in commit message). When this patch is
> applied, everything works fine.
>
> Can we please merge this patch, it fixes major bug for us?
>
> Thanks!
>
>
> On 13 June 2018 at 14:05, Vincent Pelletier wrote:
>> This bug happens only when the UDC needs to sleep during usb_ep_dequeue,
>> as is the case for (at least) dwc3.
>>
>> [ 382.200896] BUG: scheduling while atomic: screen/1808/0x0100
>> [ 382.207124] 4 locks held by screen/1808:
>> [ 382.211266] #0: (rcu_callback){}, at: []
>> rcu_process_callbacks+0x260/0x440
>> [ 382.219949] #1: (rcu_read_lock_sched){}, at: []
>> percpu_ref_switch_to_atomic_rcu+0xb0/0x130
>> [ 382.230034] #2: (&(&ctx->ctx_lock)->rlock){}, at: []
>> free_ioctx_users+0x23/0xd0
>> [ 382.230096] #3: (&(&ffs->eps_lock)->rlock){}, at: []
>> ffs_aio_cancel+0x20/0x60 [usb_f_fs]
>> [ 382.230160] Modules linked in: usb_f_fs libcomposite configfs bnep btsdio
>> bluetooth ecdh_generic brcmfmac brcmutil intel_powerclamp coretemp dwc3
>> kvm_intel ulpi udc_core kvm irqbypass crc32_pclmul crc32c_intel pcbc
>> dwc3_pci aesni_intel aes_i586 crypto_simd cryptd ehci_pci ehci_hcd gpio_keys
>> usbcore basincove_gpadc industrialio usb_common
>> [ 382.230407] CPU: 1 PID: 1808 Comm: screen Not tainted 4.14.0-edison+ #117
>> [ 382.230416] Hardware name: Intel Corporation Merrifield/BODEGA BAY, BIOS
>> 542 2015.01.21:18.19.48
>> [ 382.230425] Call Trace:
>> [ 382.230438]
>> [ 382.230466] dump_stack+0x47/0x62
>> [ 382.230498] __schedule_bug+0x61/0x80
>> [ 382.230522] __schedule+0x43/0x7a0
>> [ 382.230587] schedule+0x5f/0x70
>> [ 382.230625] dwc3_gadget_ep_dequeue+0x14c/0x270 [dwc3]
>> [ 382.230669] ? do_wait_intr_irq+0x70/0x70
>> [ 382.230724] usb_ep_dequeue+0x19/0x90 [udc_core]
>> [ 382.230770] ffs_aio_cancel+0x37/0x60 [usb_f_fs]
>> [ 382.230798] kiocb_cancel+0x31/0x40
>> [ 382.230822] free_ioctx_users+0x4d/0xd0
>> [ 382.230858] percpu_ref_switch_to_atomic_rcu+0x10a/0x130
>> [ 382.230881] ? percpu_ref_exit+0x40/0x40
>> [ 382.230904] rcu_process_callbacks+0x2b3/0x440
>> [ 382.230965] __do_softirq+0xf8/0x26b
>> [ 382.231011] ? __softirqentry_text_start+0x8/0x8
>> [ 382.231033] do_softirq_own_stack+0x22/0x30
>> [ 382.231042]
>> [ 382.231071] irq_exit+0x45/0xc0
>> [ 382.231089] smp_apic_timer_interrupt+0x13c/0x150
>> [ 382.231118] apic_timer_interrupt+0x35/0x3c
>> [ 382.231132] EIP: __copy_user_ll+0xe2/0xf0
>> [ 382.231142] EFLAGS: 00210293 CPU: 1
>> [ 382.231154] EAX: bfd4508c EBX: 0004 ECX: 0003 EDX: f3d8fe50
>> [ 382.231165] ESI: f3d8fe51 EDI: bfd4508d EBP: f3d8fe14 ESP: f3d8fe08
>> [ 382.231176] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
>> [ 382.231265] core_sys_select+0x25f/0x320
>> [ 382.231346] ? __wake_up_common_lock+0x62/0x80
>> [ 382.231399] ? tty_ldisc_deref+0x13/0x20
>> [ 382.231438] ? ldsem_up_read+0x1b/0x40
>> [ 382.231459] ? tty_ldisc_deref+0x13/0x20
>> [ 382.231479] ? tty_write+0x29f/0x2e0
>> [ 382.231514] ? n_tty_ioctl+0xe0/0xe0
>> [ 382.231541] ? tty_write_unlock+0x30/0x30
>> [ 382.231566] ? __vfs_write+0x22/0x110
>> [ 382.231604] ? security_file_permission+0x2f/0xd0
>> [ 382.231635] ? rw_verify_area+0xac/0x120
>> [ 382.231677] ? vfs_write+0x103/0x180
>> [ 382.231711] SyS_select+0x87/0xc0
>> [ 382.231739] ? SyS_write+0x42/0x90
>> [ 382.231781] do_fast_syscall_32+0xd6/0x1a0
>> [ 382.231836] entry_SYSENTER_32+0x47/0x71
>> [ 382.231848] EIP: 0xb7f75b05
>> [ 382.231857] EFLAGS: 0246 CPU: 1
>> [ 382.231868] EAX: ffda EBX: 0400 ECX: bfd4508c EDX: bfd4510c
>> [ 382.231878] ESI: EDI: EBP: ESP: bfd45020
>> [ 382.231889] DS: 007b ES: 007b FS: GS: 0033 SS: 007b
>> [ 382.232281] softirq: huh, entered softirq 9 RCU c10b4d90 with
>> preempt_count 0100, exited with ?
>>
>> Signed-off-by: Vincent Pelletier
>> ---
>> drivers/usb/gadget/function/f_fs.c | 26 ++
>> 1 file changed, 18 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/function/f_fs.c
>> b/drivers/usb/gadget/function/f_fs.c
>> index 199d25700050..01841fdb3144 100644
>> --- a/drivers/usb/gadget/function/f_fs.c
>> +++ b/drivers/usb/gadget/function/f_fs.c
>> @@ -215,6 +215,7 @@ struct ffs_io_data {
>>
>> struct mm_struct *mm;
>> struct work_struct work;
>> + struct work_struct cancellation_work;
>>
>> struct usb_ep *ep;
>> struct usb_request *req;
>> @@ -1072,22 +1073,31 @@ ffs_epfile_open(struct inode *inode, struct file
>> *file)
>> return 0;
>> }
>>
>> +static void ffs_aio_cancel_worker(struct work_struct *work)
>> +{
>> + struct ffs_io_data *io_d
[PATCH] usb: cdc-acm: Decrement tty port's refcount if probe() fail
The cdc-acm driver does not have a refcount of itself, but uses a
tty_port's refcount. That is, if the refcount of tty_port is '0', we
can clean up the cdc-acm driver by calling the .destruct()
callback function of struct tty_port_operations.
The problem is the destruct() callback function is not called if
the probe() fails, because tty_port's refcount is not zero. So,
add tty_port_put() when probe() fails.
Signed-off-by: Jaejoong Kim
---
drivers/usb/class/cdc-acm.c | 35 +--
1 file changed, 17 insertions(+), 18 deletions(-)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 8e0636c..e8d4cb7 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -1375,6 +1375,9 @@ static int acm_probe(struct usb_interface *intf,
if (acm == NULL)
goto alloc_fail;
+ tty_port_init(&acm->port);
+ acm->port.ops = &acm_port_ops;
+
minor = acm_alloc_minor(acm);
if (minor < 0)
goto alloc_fail1;
@@ -1410,22 +1413,20 @@ static int acm_probe(struct usb_interface *intf,
acm->out = usb_sndintpipe(usb_dev, epwrite->bEndpointAddress);
else
acm->out = usb_sndbulkpipe(usb_dev, epwrite->bEndpointAddress);
- tty_port_init(&acm->port);
- acm->port.ops = &acm_port_ops;
init_usb_anchor(&acm->delayed);
acm->quirks = quirks;
buf = usb_alloc_coherent(usb_dev, ctrlsize, GFP_KERNEL, &acm->ctrl_dma);
if (!buf)
- goto alloc_fail2;
+ goto alloc_fail1;
acm->ctrl_buffer = buf;
if (acm_write_buffers_alloc(acm) < 0)
- goto alloc_fail4;
+ goto alloc_fail2;
acm->ctrlurb = usb_alloc_urb(0, GFP_KERNEL);
if (!acm->ctrlurb)
- goto alloc_fail5;
+ goto alloc_fail3;
for (i = 0; i < num_rx_buf; i++) {
struct acm_rb *rb = &(acm->read_buffers[i]);
@@ -1434,13 +1435,13 @@ static int acm_probe(struct usb_interface *intf,
rb->base = usb_alloc_coherent(acm->dev, readsize, GFP_KERNEL,
&rb->dma);
if (!rb->base)
- goto alloc_fail6;
+ goto alloc_fail4;
rb->index = i;
rb->instance = acm;
urb = usb_alloc_urb(0, GFP_KERNEL);
if (!urb)
- goto alloc_fail6;
+ goto alloc_fail4;
urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
urb->transfer_dma = rb->dma;
@@ -1462,7 +1463,7 @@ static int acm_probe(struct usb_interface *intf,
snd->urb = usb_alloc_urb(0, GFP_KERNEL);
if (snd->urb == NULL)
- goto alloc_fail7;
+ goto alloc_fail5;
if (usb_endpoint_xfer_int(epwrite))
usb_fill_int_urb(snd->urb, usb_dev, acm->out,
@@ -1480,7 +1481,7 @@ static int acm_probe(struct usb_interface *intf,
i = device_create_file(&intf->dev, &dev_attr_bmCapabilities);
if (i < 0)
- goto alloc_fail7;
+ goto alloc_fail5;
if (h.usb_cdc_country_functional_desc) { /* export the country data */
struct usb_cdc_country_functional_desc * cfd =
@@ -1539,7 +1540,7 @@ static int acm_probe(struct usb_interface *intf,
&control_interface->dev);
if (IS_ERR(tty_dev)) {
rv = PTR_ERR(tty_dev);
- goto alloc_fail8;
+ goto alloc_fail6;
}
if (quirks & CLEAR_HALT_CONDITIONS) {
@@ -1548,7 +1549,7 @@ static int acm_probe(struct usb_interface *intf,
}
return 0;
-alloc_fail8:
+alloc_fail6:
if (acm->country_codes) {
device_remove_file(&acm->control->dev,
&dev_attr_wCountryCodes);
@@ -1557,23 +1558,21 @@ static int acm_probe(struct usb_interface *intf,
kfree(acm->country_codes);
}
device_remove_file(&acm->control->dev, &dev_attr_bmCapabilities);
-alloc_fail7:
+alloc_fail5:
usb_set_intfdata(intf, NULL);
for (i = 0; i < ACM_NW; i++)
usb_free_urb(acm->wb[i].urb);
-alloc_fail6:
+alloc_fail4:
for (i = 0; i < num_rx_buf; i++)
usb_free_urb(acm->read_urbs[i]);
acm_read_buffers_free(acm);
usb_free_urb(acm->ctrlurb);
-alloc_fail5:
+alloc_fail3:
acm_write_buffers_free(acm);
-alloc_fail4:
- usb_free_coherent(usb_dev, ctrlsize, acm->ctrl_buffer, acm->ctrl_dma);
alloc_fail2:
- acm_release_minor(acm);
+ usb_free_coherent(usb_dev, ctrlsize, acm->ctrl_buffer, acm->ctrl_dma);
alloc_fail1:
- kfree(acm);
+ tty_port_put(&acm->port);
alloc_fail:
return rv;
}
--
2.7.4
--
To unsubscribe from this list: se
Re: [PATCH] usb: cdc-acm: Decrement tty port's refcount if probe() fail
On Do, 2018-06-21 at 17:45 +0900, Jaejoong Kim wrote: > The cdc-acm driver does not have a refcount of itself, but uses a > tty_port's refcount. That is, if the refcount of tty_port is '0', we > can clean up the cdc-acm driver by calling the .destruct() > callback function of struct tty_port_operations. > > The problem is the destruct() callback function is not called if > the probe() fails, because tty_port's refcount is not zero. So, > add tty_port_put() when probe() fails. > > Signed-off-by: Jaejoong Kim Acked-by: Oliver Neukum -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: cdc-acm: Decrement tty port's refcount if probe() fail
On Do, 2018-06-21 at 17:45 +0900, Jaejoong Kim wrote: > The cdc-acm driver does not have a refcount of itself, but uses a > tty_port's refcount. That is, if the refcount of tty_port is '0', we > can clean up the cdc-acm driver by calling the .destruct() > callback function of struct tty_port_operations. > > The problem is the destruct() callback function is not called if > the probe() fails, because tty_port's refcount is not zero. So, > add tty_port_put() when probe() fails. > > Signed-off-by: Jaejoong Kim Acked-by: Oliver Neukum -- 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: make a confirm for [usb: dwc3: gadget: skip Set/Clear Halt when invalid]
Hi,
that patch is not 100% correct. You can revert it in your tree. I added
that because of a problem I found when running adb against macOS.
It's actually okay to send Clear Halt at any time, but for some reason
dwc3 was hanging when running adb against macOS.
If you can revert the patch and make sure it works against all 3 major
OSes (linux, windows and mac) I'd be really glad.
liangshengjun writes:
> Hi felipe,
>
> I have met a case about set/clear Halt patch
> Version: linux v4.16,
> Case: usb uvc run with bulk-mode connect to Windows 7 PC. When PC stop
> camera application , it would send clearHalt request to uvc device to
> streaming-off video transfer.
>But with v4.16 dwc3 drivers, it would skip handling this clear
> Halt request , because dep->flags is not DWC3_EP_STALL status, then it causes
> PC restart camera application , uvc transfer fail.
>And I have confirmed v3.18 dwc3 drivers is OK.
>
> So how to balance for handling clear Halt without first setHalt ??
>
> PS:
> commit ffb80fc672c3a7b6afd0cefcb1524fb99917b2f3
> Author: Felipe Balbi
> Date: Thu Jan 19 13:38:42 2017 +0200
>
> usb: dwc3: gadget: skip Set/Clear Halt when invalid
>
> At least macOS seems to be sending
> ClearFeature(ENDPOINT_HALT) to endpoints which
> aren't Halted. This makes DWC3's CLEARSTALL command
> time out which causes several issues for the driver.
>
> Instead, let's just return 0 and bail out early.
>
> Cc:
> Signed-off-by: Felipe Balbi
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 6faf484..0a664d8 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -1379,6 +1379,9 @@ int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int
> value, int protocol)
> unsigned transfer_in_flight;
> unsigned started;
>
> + if (dep->flags & DWC3_EP_STALL)
> + return 0;
> +
> if (dep->number > 1)
> trb = dwc3_ep_prev_trb(dep, dep->trb_enqueue);
> else
> @@ -1400,6 +1403,8 @@ int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int
> value, int protocol)
> else
> dep->flags |= DWC3_EP_STALL;
> } else {
> + if (!(dep->flags & DWC3_EP_STALL))
> + return 0;
>
> ret = dwc3_send_clear_stall_ep_cmd(dep);
> if (ret)
>
>
> Liang Shengjun
> [cid:image001.png@01D40971.9265B340]
> HISILICON TECHNOLOGIES CO., LTD.
> New R&D Center, Wuhe Road, Bantian,
> Longgang District, Shenzhen 518129 P.R. China
>
--
balbi
signature.asc
Description: PGP signature
Re: [PATCH] usb: dwc3: of-simple: fix use-after-free on remove
On Thu, Jun 21, 2018 at 12:55:26AM +0200, Rafael J. Wysocki wrote: > On Thu, Jun 21, 2018 at 12:32 AM, Rafael J. Wysocki wrote: > > On Wed, Jun 20, 2018 at 5:46 PM, Johan Hovold wrote: > >> On Wed, Jun 20, 2018 at 02:54:10PM +0200, Rafael J. Wysocki wrote: > >>> On Wednesday, June 20, 2018 2:23:46 PM CEST Johan Hovold wrote: > >>> > On Wed, Jun 20, 2018 at 02:16:59AM -0700, Tony Lindgren wrote: > >>> > > Hi, > >>> > > > >>> > > Adding Rafael and linux-pm to Cc as well. > >>> > > > >>> > > * Felipe Balbi [180619 01:23]: > >>> > > > This is a direct consequence of not paying attention to the order of > >>> > > > things. If driver were to assume that pm_domain->activate() would > >>> > > > do the > >>> > > > right thing for the device -- meaning that probe would run with an > >>> > > > active device --, then we wouldn't need that pm_runtime_get() call > >>> > > > on > >>> > > > probe at all. Rather we would follow the sequence: > >>> > > > > >>> > > > pm_runtime_forbid() > >>> > > > pm_runtime_set_active() > >>> > > > pm_runtime_enable() > >>> > > > > >>> > > > /* do your probe routine */ > >>> > > > > >>> > > > pm_runtime_put_noidle() > >>> > > > > >>> > > > Then you remove you would need to call pm_runtime_get_noresume() to > >>> > > > balance out the pm_runtime_put_noidle() there. > >>> > > >>> > > > (If you need to know why the pm_runtime_put_noidle(), remember that > >>> > > > pm_runtime_set_active() increments the usage counter, so > >>> > > > pm_runtime_put_noidle is basically allowing pm_runtime to happen as > >>> > > > soon > >>> > > > as userspace writes "auto" to /sys//power/control) > >>> > > >>> > That's not correct; pm_runtime_set_active() only increments the usage > >>> > counter of a parent (under some circumstances), so unless you have bus > >>> > code incrementing the usage counter before probe, the above > >>> > pm_runtime_put_noidle() would actually introduce an imbalance. > The confusion regarding the pm_runtime_put_noidle() at the end may > come from the special requirement of the PCI bus type as per the > comment in local_pci_probe(). That seems to be the case, but I'm not sure of much of what PCI is doing that can be applied here (e.g. OMAP platform devices), where unbound devices should always be powered off and were I assume we want to have runtime pm allowed by default, for example. 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] usb: dwc3: of-simple: fix use-after-free on remove
On Thu, Jun 21, 2018 at 11:17:36AM +0300, Roger Quadros wrote: > On 21/06/18 01:55, Rafael J. Wysocki wrote: > > On Thu, Jun 21, 2018 at 12:32 AM, Rafael J. Wysocki > > wrote: > >> On Wed, Jun 20, 2018 at 5:46 PM, Johan Hovold wrote: > >>> On Wed, Jun 20, 2018 at 02:54:10PM +0200, Rafael J. Wysocki wrote: > On Wednesday, June 20, 2018 2:23:46 PM CEST Johan Hovold wrote: > > On Wed, Jun 20, 2018 at 02:16:59AM -0700, Tony Lindgren wrote: > >> Hi, > >> > >> Adding Rafael and linux-pm to Cc as well. > >> > >> * Felipe Balbi [180619 01:23]: > >>> This is a direct consequence of not paying attention to the order of > >>> things. If driver were to assume that pm_domain->activate() would do > >>> the > >>> right thing for the device -- meaning that probe would run with an > >>> active device --, then we wouldn't need that pm_runtime_get() call on > >>> probe at all. Rather we would follow the sequence: > >>> > >>> pm_runtime_forbid() > >>> pm_runtime_set_active() > >>> pm_runtime_enable() > >>> > >>> /* do your probe routine */ > >>> > >>> pm_runtime_put_noidle() > >>> > >>> Then you remove you would need to call pm_runtime_get_noresume() to > >>> balance out the pm_runtime_put_noidle() there. > > > >>> (If you need to know why the pm_runtime_put_noidle(), remember that > >>> pm_runtime_set_active() increments the usage counter, so > >>> pm_runtime_put_noidle is basically allowing pm_runtime to happen as > >>> soon > >>> as userspace writes "auto" to /sys//power/control) > > > > That's not correct; pm_runtime_set_active() only increments the usage > > counter of a parent (under some circumstances), so unless you have bus > > code incrementing the usage counter before probe, the above > > pm_runtime_put_noidle() would actually introduce an imbalance. > > No, it wouldn't. It balances the incrementation in pm_runtime_forbid(). > >>> > >>> Right, but even if you take the whole sequence, which included > >>> pm_runtime_forbid(), consider what happens when pm_runtime_allow() is > >>> later called through sysfs (see below). > >>> > > And note that that's also the case even if you meant to say that > > *pm_runtime_forbid()* increments the usage counter (which it does). > > Why is it? > > Surely, after > > pm_runtime_forbid(dev); > pm_runtime_put_noidle(dev); > > the runtime PM usage counter of dev will be the same as before, won't it? > >>> > >>> Sure, but the imbalance, or rather inconsistent state, has already been > >>> introduced. > >>> > >>> Consider the following sequence of events: > >>> > >>> usage count > >>> 0 > >>> probe() > >>> pm_runtime_forbid() 1 > > Can you call pm_runtime_forbid() before pm_runtime_enable()? > Wouldn't it fail with -EACCES as dev->power.disable_depth > 0? No, pm_runtime_forbid() manipulates the usage count directly and doesn't check for errors from rpm_resume(), so this actually "works". That doesn't mean anyone should be doing it, though (e.g. for the below reasons). > >>> pm_runtime_set_active() > >>> pm_runtime_enable() > >>> pm_runtime_put_noidle() 0 > >>> > >>> Here nothing is preventing the device from runtime suspending, despite > >>> runtime PM being forbidden. In fact, it will typically be suspended due > >>> to the pm_request_idle() in driver_probe_device(). If later we have: > >>> > >>> echo auto > power/control > >>> pm_runtime_allow() -1 > >> > >> OK, you have a point. > >> > >> After calling pm_runtime_forbid() the driver should allow user space > >> to unblock runtime PM for the device - or call pm_runtime_allow() > >> itself. > > > > The confusion regarding the pm_runtime_put_noidle() at the end may > > come from the special requirement of the PCI bus type as per the > > comment in local_pci_probe(). > > OK. So it is the PCI bus which is behaving odd here and > pm_runtime_put_noidle() needs to be done only if its a PCI device, correct? Yeah, the pm_runtime_put_noidle() would be used to balance the unconditional runtime resume by the bus code in case a PCI driver implements runtime pm. 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: usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers
On 06/21/2018 10:29 AM, Roger Quadros wrote:
[...]
>>> static int ffs_aio_cancel(struct kiocb *kiocb)
>>> {
>>> struct ffs_io_data *io_data = kiocb->private;
>>> - struct ffs_epfile *epfile = kiocb->ki_filp->private_data;
>>> + struct ffs_data *ffs = io_data->ffs;
>>> int value;
>>>
>>> ENTER();
>>>
>>> - spin_lock_irq(&epfile->ffs->eps_lock);
>>> -
>>> - if (likely(io_data && io_data->ep && io_data->req))
>>> - value = usb_ep_dequeue(io_data->ep, io_data->req);
>>> - else
>>> + if (likely(io_data && io_data->ep && io_data->req)) {
>>> + INIT_WORK(&io_data->cancellation_work,
>>> ffs_aio_cancel_worker);
>>> + queue_work(ffs->io_completion_wq,
>>> &io_data->cancellation_work);
>>> + value = -EINPROGRESS;
>>> + } else {
>>> value = -EINVAL;
>>> -
>>> - spin_unlock_irq(&epfile->ffs->eps_lock);
>>> + }
>
> Can we avoid the spin_lock() and the work-queue and call usb_ep_dequeue()
> directly from here?
>> What is the purpose of the spin_lock()?
I agree that the lock doesn't seem to be necessary. But I believe the whole
thing is already running in non-sleeping context, even before the spinlock
is taken. So this wouldn't help much.
Even the io_cancel() syscall takes a spinlock before invoking the cancel
function. So this issue is not exclusive to program termination.
Are there any documented guidelines on which context usb_ep_dequeue() should
be able to be called in? The sleep in the dwc3 driver seems to be a recent
addition.
>
>>>
>>> return value;
>>> }
>>> --
>>> 2.17.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: usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers
On 21/06/18 13:52, Lars-Peter Clausen wrote:
> On 06/21/2018 10:29 AM, Roger Quadros wrote:
> [...]
static int ffs_aio_cancel(struct kiocb *kiocb)
{
struct ffs_io_data *io_data = kiocb->private;
- struct ffs_epfile *epfile = kiocb->ki_filp->private_data;
+ struct ffs_data *ffs = io_data->ffs;
int value;
ENTER();
- spin_lock_irq(&epfile->ffs->eps_lock);
-
- if (likely(io_data && io_data->ep && io_data->req))
- value = usb_ep_dequeue(io_data->ep, io_data->req);
- else
+ if (likely(io_data && io_data->ep && io_data->req)) {
+ INIT_WORK(&io_data->cancellation_work,
ffs_aio_cancel_worker);
+ queue_work(ffs->io_completion_wq,
&io_data->cancellation_work);
+ value = -EINPROGRESS;
+ } else {
value = -EINVAL;
-
- spin_unlock_irq(&epfile->ffs->eps_lock);
+ }
>>
>> Can we avoid the spin_lock() and the work-queue and call usb_ep_dequeue()
>> directly from here?
>>> What is the purpose of the spin_lock()?
>
> I agree that the lock doesn't seem to be necessary. But I believe the whole
> thing is already running in non-sleeping context, even before the spinlock
> is taken. So this wouldn't help much.
>
> Even the io_cancel() syscall takes a spinlock before invoking the cancel
> function. So this issue is not exclusive to program termination.
>
> Are there any documented guidelines on which context usb_ep_dequeue() should
> be able to be called in? The sleep in the dwc3 driver seems to be a recent
> addition.
drivers/usb/udc/gadget/core.c has the only documentation, but context is not
mentioned there.
Felipe, what do you suggest?
>
>>
return value;
}
--
2.17.1
>>
>
--
cheers,
-roger
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
--
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] Bluetooth: btusb: use irqsave() in URB's complete callback
Hi Sebastian, > The USB completion callback does not disable interrupts while acquiring > the ->lock. We want to remove the local_irq_disable() invocation from > __usb_hcd_giveback_urb() and therefore it is required for the callback > handler to disable the interrupts while acquiring the lock. > The callback may be invoked either in IRQ or BH context depending on the > USB host controller. > Use the _irqsave variant of the locking primitives. > > Cc: Marcel Holtmann > Cc: Johan Hedberg > Cc: linux-blueto...@vger.kernel.org > Signed-off-by: Sebastian Andrzej Siewior > --- > drivers/bluetooth/btusb.c | 20 > 1 file changed, 12 insertions(+), 8 deletions(-) can I get an ACK from someone ensuring that this is the direction we are going with the USB host controllers? Regards Marcel -- 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] Bluetooth: btusb: use irqsave() in URB's complete callback
On 2018-06-21 14:43:41 [+0200], Marcel Holtmann wrote: > Hi Sebastian, Hi Marcel, > > The USB completion callback does not disable interrupts while acquiring > > the ->lock. We want to remove the local_irq_disable() invocation from > > __usb_hcd_giveback_urb() and therefore it is required for the callback > > handler to disable the interrupts while acquiring the lock. > > The callback may be invoked either in IRQ or BH context depending on the > > USB host controller. > > Use the _irqsave variant of the locking primitives. > > > > Cc: Marcel Holtmann > > Cc: Johan Hedberg > > Cc: linux-blueto...@vger.kernel.org > > Signed-off-by: Sebastian Andrzej Siewior > > --- > > drivers/bluetooth/btusb.c | 20 > > 1 file changed, 12 insertions(+), 8 deletions(-) > > can I get an ACK from someone ensuring that this is the direction we are > going with the USB host controllers? +Alan. EHCI completes in BH since v3.12-rc1. In order to get rid of that local_irq_save() in USB core code I need to make sure that the USB device driver(s) use irqsave primitives. See https://lkml.kernel.org/r/pine.lnx.4.44l0.1806011629140.1404-100...@iolanthe.rowland.org > Regards > > Marcel 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
[PATCH 4/5] usb: xhci: tegra: fix runtime PM error handling
From: Stefan Agner
The address-of operator will always evaluate to true. However,
power should be explicitly disabled if no power domain is used.
Remove the address-of operator.
Fixes: 58c38116c6cc ("usb: xhci: tegra: Add support for managing powergates")
Signed-off-by: Stefan Agner
Acked-by: Jon Hunter
Signed-off-by: Mathias Nyman
---
drivers/usb/host/xhci-tegra.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
index d50549f..4b463e5 100644
--- a/drivers/usb/host/xhci-tegra.c
+++ b/drivers/usb/host/xhci-tegra.c
@@ -1223,10 +1223,10 @@ static int tegra_xusb_probe(struct platform_device
*pdev)
pm_runtime_disable(&pdev->dev);
usb_put_hcd(tegra->hcd);
disable_xusbc:
- if (!&pdev->dev.pm_domain)
+ if (!pdev->dev.pm_domain)
tegra_powergate_power_off(TEGRA_POWERGATE_XUSBC);
disable_xusba:
- if (!&pdev->dev.pm_domain)
+ if (!pdev->dev.pm_domain)
tegra_powergate_power_off(TEGRA_POWERGATE_XUSBA);
put_padctl:
tegra_xusb_padctl_put(tegra->padctl);
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] xhci: Fix kernel oops in trace_xhci_free_virt_device
From: Zhengjun Xing
commit 44a182b9d177 ("xhci: Fix use-after-free in xhci_free_virt_device")
set dev->udev pointer to NULL in xhci_free_dev(), it will cause kernel
panic in trace_xhci_free_virt_device. This patch reimplement the trace
function trace_xhci_free_virt_device, remove dev->udev dereference and
added more useful parameters to show in the trace function,it also makes
sure dev->udev is not NULL before calling trace_xhci_free_virt_device.
This issue happened when xhci-hcd trace is enabled and USB devices hot
plug test. Original use-after-free patch went to stable so this needs so
be applied there as well.
[ 1092.022457] usb 2-4: USB disconnect, device number 6
[ 1092.092772] BUG: unable to handle kernel NULL pointer dereference at
[ 1092.101694] PGD 0 P4D 0
[ 1092.104601] Oops: [#1] SMP
[ 1092.207734] Workqueue: usb_hub_wq hub_event
[ 1092.212507] RIP: 0010:trace_event_raw_event_xhci_log_virt_dev+0x6c/0xf0
[ 1092.220050] RSP: 0018:8c252e883d28 EFLAGS: 00010086
[ 1092.226024] RAX: 8c24af86fa84 RBX: 0003 RCX: 8c25255c2a01
[ 1092.234130] RDX: RSI: aef55009 RDI: 8c252e883d28
[ 1092.242242] RBP: 8c252550e2c0 R08: 8c24af86fa84 R09: 0a70
[ 1092.250364] R10: 0a70 R11: R12: 8c251f21a000
[ 1092.258468] R13: 000c R14: 8c251f21a000 R15: 8c251f432f60
[ 1092.266572] FS: () GS:8c252e88()
knlGS:
[ 1092.275757] CS: 0010 DS: ES: CR0: 80050033
[ 1092.282281] CR2: CR3: 000154209001 CR4: 003606e0
[ 1092.290384] Call Trace:
[ 1092.293156]
[ 1092.295439] xhci_free_virt_device.part.34+0x182/0x1a0
[ 1092.301288] handle_cmd_completion+0x7ac/0xfa0
[ 1092.306336] ? trace_event_raw_event_xhci_log_trb+0x6e/0xa0
[ 1092.312661] xhci_irq+0x3e8/0x1f60
[ 1092.316524] __handle_irq_event_percpu+0x75/0x180
[ 1092.321876] handle_irq_event_percpu+0x20/0x50
[ 1092.326922] handle_irq_event+0x36/0x60
[ 1092.331273] handle_edge_irq+0x6d/0x180
[ 1092.335644] handle_irq+0x16/0x20
[ 1092.339417] do_IRQ+0x41/0xc0
[ 1092.342782] common_interrupt+0xf/0xf
[ 1092.346955]
Fixes: 44a182b9d177 ("xhci: Fix use-after-free in xhci_free_virt_device")
Cc:
Signed-off-by: Zhengjun Xing
Signed-off-by: Mathias Nyman
---
drivers/usb/host/xhci-mem.c | 4 ++--
drivers/usb/host/xhci-trace.h | 36 +++-
2 files changed, 33 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index acbd3d7..8a62eee 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -886,12 +886,12 @@ void xhci_free_virt_device(struct xhci_hcd *xhci, int
slot_id)
dev = xhci->devs[slot_id];
- trace_xhci_free_virt_device(dev);
-
xhci->dcbaa->dev_context_ptrs[slot_id] = 0;
if (!dev)
return;
+ trace_xhci_free_virt_device(dev);
+
if (dev->tt_info)
old_active_eps = dev->tt_info->active_eps;
diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index 410544f..88b4274 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -171,6 +171,37 @@ DEFINE_EVENT(xhci_log_trb, xhci_dbc_gadget_ep_queue,
TP_ARGS(ring, trb)
);
+DECLARE_EVENT_CLASS(xhci_log_free_virt_dev,
+ TP_PROTO(struct xhci_virt_device *vdev),
+ TP_ARGS(vdev),
+ TP_STRUCT__entry(
+ __field(void *, vdev)
+ __field(unsigned long long, out_ctx)
+ __field(unsigned long long, in_ctx)
+ __field(u8, fake_port)
+ __field(u8, real_port)
+ __field(u16, current_mel)
+
+ ),
+ TP_fast_assign(
+ __entry->vdev = vdev;
+ __entry->in_ctx = (unsigned long long) vdev->in_ctx->dma;
+ __entry->out_ctx = (unsigned long long) vdev->out_ctx->dma;
+ __entry->fake_port = (u8) vdev->fake_port;
+ __entry->real_port = (u8) vdev->real_port;
+ __entry->current_mel = (u16) vdev->current_mel;
+ ),
+ TP_printk("vdev %p ctx %llx | %llx fake_port %d real_port %d
current_mel %d",
+ __entry->vdev, __entry->in_ctx, __entry->out_ctx,
+ __entry->fake_port, __entry->real_port, __entry->current_mel
+ )
+);
+
+DEFINE_EVENT(xhci_log_free_virt_dev, xhci_free_virt_device,
+ TP_PROTO(struct xhci_virt_device *vdev),
+ TP_ARGS(vdev)
+);
+
DECLARE_EVENT_CLASS(xhci_log_virt_dev,
TP_PROTO(struct xhci_virt_device *vdev),
TP_ARGS(vdev),
@@ -208,11 +239,6 @@ DEFINE_EVENT(xhci_log_virt_dev, xhci_alloc_virt_device,
TP_ARGS(vdev)
);
-DEFINE_EVENT(xhci_log_virt_dev, xhci_free_virt_device,
- TP_PROTO(struct xhci_virt_device *vdev),
- TP_ARGS(vdev)
-);
-
DEFINE_EVENT(xhci_log_virt_dev, xhci_setup_dev
[PATCH 5/5] usb: xhci: increase CRS timeout value
From: Ajay Gupta
Some controllers take almost 55ms to complete controller
restore state (CRS).
There is no timeout limit mentioned in xhci specification so
fixing the issue by increasing the timeout limit to 100ms
[reformat code comment -Mathias]
Signed-off-by: Ajay Gupta
Signed-off-by: Nagaraj Annaiah
Signed-off-by: Mathias Nyman
---
drivers/usb/host/xhci.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index f11ec61..2f4850f 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1078,8 +1078,13 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
command = readl(&xhci->op_regs->command);
command |= CMD_CRS;
writel(command, &xhci->op_regs->command);
+ /*
+* Some controllers take up to 55+ ms to complete the controller
+* restore so setting the timeout to 100ms. Xhci specification
+* doesn't mention any timeout value.
+*/
if (xhci_handshake(&xhci->op_regs->status,
- STS_RESTORE, 0, 10 * 1000)) {
+ STS_RESTORE, 0, 100 * 1000)) {
xhci_warn(xhci, "WARN: xHC restore state timeout\n");
spin_unlock_irq(&xhci->lock);
return -ETIMEDOUT;
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] xhci: Fix perceived dead host due to runtime suspend race with event handler
Don't rely on event interrupt (EINT) bit alone to detect pending port
change in resume. If no change event is detected the host may be suspended
again, oterwise roothubs are resumed.
There is a lag in xHC setting EINT. If we don't notice the pending change
in resume, and the controller is runtime suspeded again, it causes the
event handler to assume host is dead as it will fail to read xHC registers
once PCI puts the controller to D3 state.
[ 268.520969] xhci_hcd: xhci_resume: starting port polling.
[ 268.520985] xhci_hcd: xhci_hub_status_data: stopping port polling.
[ 268.521030] xhci_hcd: xhci_suspend: stopping port polling.
[ 268.521040] xhci_hcd: // Setting command ring address to 0x349bd001
[ 268.521139] xhci_hcd: Port Status Change Event for port 3
[ 268.521149] xhci_hcd: resume root hub
[ 268.521163] xhci_hcd: port resume event for port 3
[ 268.521168] xhci_hcd: xHC is not running.
[ 268.521174] xhci_hcd: handle_port_status: starting port polling.
[ 268.596322] xhci_hcd: xhci_hc_died: xHCI host controller not responding,
assume dead
The EINT lag is described in a additional note in xhci specs 4.19.2:
"Due to internal xHC scheduling and system delays, there will be a lag
between a change bit being set and the Port Status Change Event that it
generated being written to the Event Ring. If SW reads the PORTSC and
sees a change bit set, there is no guarantee that the corresponding Port
Status Change Event has already been written into the Event Ring."
Cc:
Signed-off-by: Mathias Nyman
---
drivers/usb/host/xhci.c | 40 +---
drivers/usb/host/xhci.h | 4
2 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 8c8da2d..f11ec61 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -908,6 +908,41 @@ static void xhci_disable_port_wake_on_bits(struct xhci_hcd
*xhci)
spin_unlock_irqrestore(&xhci->lock, flags);
}
+static bool xhci_pending_portevent(struct xhci_hcd *xhci)
+{
+ struct xhci_port**ports;
+ int port_index;
+ u32 status;
+ u32 portsc;
+
+ status = readl(&xhci->op_regs->status);
+ if (status & STS_EINT)
+ return true;
+ /*
+* Checking STS_EINT is not enough as there is a lag between a change
+* bit being set and the Port Status Change Event that it generated
+* being written to the Event Ring. See note in xhci 1.1 section 4.19.2.
+*/
+
+ port_index = xhci->usb2_rhub.num_ports;
+ ports = xhci->usb2_rhub.ports;
+ while (port_index--) {
+ portsc = readl(ports[port_index]->addr);
+ if (portsc & PORT_CHANGE_MASK ||
+ (portsc & PORT_PLS_MASK) == XDEV_RESUME)
+ return true;
+ }
+ port_index = xhci->usb3_rhub.num_ports;
+ ports = xhci->usb3_rhub.ports;
+ while (port_index--) {
+ portsc = readl(ports[port_index]->addr);
+ if (portsc & PORT_CHANGE_MASK ||
+ (portsc & PORT_PLS_MASK) == XDEV_RESUME)
+ return true;
+ }
+ return false;
+}
+
/*
* Stop HC (not bus-specific)
*
@@ -1009,7 +1044,7 @@ EXPORT_SYMBOL_GPL(xhci_suspend);
*/
int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
{
- u32 command, temp = 0, status;
+ u32 command, temp = 0;
struct usb_hcd *hcd = xhci_to_hcd(xhci);
struct usb_hcd *secondary_hcd;
int retval = 0;
@@ -1134,8 +1169,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
done:
if (retval == 0) {
/* Resume root hubs only when have pending events. */
- status = readl(&xhci->op_regs->status);
- if (status & STS_EINT) {
+ if (xhci_pending_portevent(xhci)) {
usb_hcd_resume_root_hub(xhci->shared_hcd);
usb_hcd_resume_root_hub(hcd);
}
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 939e2f86..841e89f 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -382,6 +382,10 @@ struct xhci_op_regs {
#define PORT_PLC (1 << 22)
/* port configure error change - port failed to configure its link partner */
#define PORT_CEC (1 << 23)
+#define PORT_CHANGE_MASK (PORT_CSC | PORT_PEC | PORT_WRC | PORT_OCC | \
+PORT_RC | PORT_PLC | PORT_CEC)
+
+
/* Cold Attach Status - xHC can set this bit to report device attached during
* Sx state. Warm port reset should be perfomed to clear this bit and move port
* to connected state.
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info
[PATCH 0/5] xhci fixes for usb-linus
Hi Greg A few xhci fixes for usb-linus. Fixing a oops in xhci tracing, a race in S3 resume event handling, tegra runtime PM, and other small fixes. -Mathias Ajay Gupta (1): usb: xhci: increase CRS timeout value Dongjiu Geng (1): usb: xhci: remove the code build warning Mathias Nyman (1): xhci: Fix perceived dead host due to runtime suspend race with event handler Stefan Agner (1): usb: xhci: tegra: fix runtime PM error handling Zhengjun Xing (1): xhci: Fix kernel oops in trace_xhci_free_virt_device drivers/usb/host/xhci-mem.c | 4 ++-- drivers/usb/host/xhci-tegra.c | 6 +++--- drivers/usb/host/xhci-trace.h | 36 - drivers/usb/host/xhci.c | 47 +++ drivers/usb/host/xhci.h | 4 5 files changed, 83 insertions(+), 14 deletions(-) -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] usb: xhci: remove the code build warning
From: Dongjiu Geng
Initialize the 'err' variate to remove the build warning,
the warning is shown as below:
drivers/usb/host/xhci-tegra.c: In function 'tegra_xusb_mbox_thread':
drivers/usb/host/xhci-tegra.c:552:6: warning: 'err' may be used uninitialized
in this function [-Wuninitialized]
drivers/usb/host/xhci-tegra.c:482:6: note: 'err' was declared here
Fixes: e84fce0f8837 ("usb: xhci: Add NVIDIA Tegra XUSB controller driver")
Signed-off-by: Dongjiu Geng
Acked-by: Thierry Reding
Acked-by: Jon Hunter
Signed-off-by: Mathias Nyman
---
drivers/usb/host/xhci-tegra.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
index a8c1d07..d50549f 100644
--- a/drivers/usb/host/xhci-tegra.c
+++ b/drivers/usb/host/xhci-tegra.c
@@ -481,7 +481,7 @@ static void tegra_xusb_mbox_handle(struct tegra_xusb *tegra,
unsigned long mask;
unsigned int port;
bool idle, enable;
- int err;
+ int err = 0;
memset(&rsp, 0, sizeof(rsp));
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] usb: typec: fixes for UCSI issues
Hi, The two first patches provide a workaround for the issue that Paul reported [1], where the driver fails with error: ioremap error for 0x3f799000-0x3f79a000, requested 0x2, got 0x0 [1] https://lkml.org/lkml/2018/5/15/569 Heikki Krogerus (3): acpi: Add helper for deactivating memory region usb: typec: ucsi: acpi: Workaround for cache mode issue usb: typec: ucsi: Fix for incorrect status data issue drivers/acpi/osl.c | 72 ++ drivers/usb/typec/ucsi/ucsi.c | 13 ++ drivers/usb/typec/ucsi/ucsi_acpi.c | 5 +++ include/linux/acpi.h | 3 ++ 4 files changed, 93 insertions(+) -- 2.17.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
[PATCH 1/3] acpi: Add helper for deactivating memory region
Sometimes memory resource may be overlapping with
SystemMemory Operation Region by design, for example if the
memory region is used as a mailbox for communication with a
firmware in the system. One occasion of such mailboxes is
USB Type-C Connector System Software Interface (UCSI).
With regions like that, it is important that the driver is
able to map the memory with the requirements it has. For
example, the driver should be allowed to map the memory as
non-cached memory. However, if the operation region has been
accessed before the driver has mapped the memory, the memory
has been marked as write-back by the time the driver is
loaded. That means the driver will fail to map the memory
if it expects non-cached memory.
To work around the problem, introducing helper that the
drivers can use to temporarily deactivate (unmap)
SystemMemory Operation Regions that overlap with their
IO memory.
Fixes: 8243edf44152 ("usb: typec: ucsi: Add ACPI driver")
Cc: sta...@vger.kernel.org
Reviewed-by: Rafael J. Wysocki
Signed-off-by: Heikki Krogerus
---
drivers/acpi/osl.c | 72
include/linux/acpi.h | 3 ++
2 files changed, 75 insertions(+)
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 7ca41bf023c9..8df9abfa947b 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -45,6 +45,8 @@
#include
#include
+#include "acpica/accommon.h"
+#include "acpica/acnamesp.h"
#include "internal.h"
#define _COMPONENT ACPI_OS_SERVICES
@@ -1490,6 +1492,76 @@ int acpi_check_region(resource_size_t start,
resource_size_t n,
}
EXPORT_SYMBOL(acpi_check_region);
+static acpi_status acpi_deactivate_mem_region(acpi_handle handle, u32 level,
+ void *_res, void **return_value)
+{
+ struct acpi_mem_space_context **mem_ctx;
+ union acpi_operand_object *handler_obj;
+ union acpi_operand_object *region_obj2;
+ union acpi_operand_object *region_obj;
+ struct resource *res = _res;
+ acpi_status status;
+
+ region_obj = acpi_ns_get_attached_object(handle);
+ if (!region_obj)
+ return AE_OK;
+
+ handler_obj = region_obj->region.handler;
+ if (!handler_obj)
+ return AE_OK;
+
+ if (region_obj->region.space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
+ return AE_OK;
+
+ if (!(region_obj->region.flags & AOPOBJ_SETUP_COMPLETE))
+ return AE_OK;
+
+ region_obj2 = acpi_ns_get_secondary_object(region_obj);
+ if (!region_obj2)
+ return AE_OK;
+
+ mem_ctx = (void *)®ion_obj2->extra.region_context;
+
+ if (!(mem_ctx[0]->address >= res->start &&
+ mem_ctx[0]->address < res->end))
+ return AE_OK;
+
+ status = handler_obj->address_space.setup(region_obj,
+ ACPI_REGION_DEACTIVATE,
+ NULL, (void **)mem_ctx);
+ if (ACPI_SUCCESS(status))
+ region_obj->region.flags &= ~(AOPOBJ_SETUP_COMPLETE);
+
+ return status;
+}
+
+/**
+ * acpi_release_memory - Release any mappings done to a memory region
+ * @handle: Handle to namespace node
+ * @res: Memory resource
+ * @level: A level that terminates the search
+ *
+ * Walks through @handle and unmaps all SystemMemory Operation Regions that
+ * overlap with @res and that have already been activated (mapped).
+ *
+ * This is a helper that allows drivers to place special requirements on memory
+ * region that may overlap with operation regions, primarily allowing them to
+ * safely map the region as non-cached memory.
+ *
+ * The unmapped Operation Regions will be automatically remapped next time they
+ * are called, so the drivers do not need to do anything else.
+ */
+acpi_status acpi_release_memory(acpi_handle handle, struct resource *res,
+ u32 level)
+{
+ if (!(res->flags & IORESOURCE_MEM))
+ return AE_TYPE;
+
+ return acpi_walk_namespace(ACPI_TYPE_REGION, handle, level,
+ acpi_deactivate_mem_region, NULL, res, NULL);
+}
+EXPORT_SYMBOL_GPL(acpi_release_memory);
+
/*
* Let drivers know whether the resource checks are effective
*/
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 4b35a66383f9..e54f40974eb0 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -443,6 +443,9 @@ int acpi_check_resource_conflict(const struct resource
*res);
int acpi_check_region(resource_size_t start, resource_size_t n,
const char *name);
+acpi_status acpi_release_memory(acpi_handle handle, struct resource *res,
+ u32 level);
+
int acpi_resources_are_enforced(void);
#ifdef CONFIG_HIBERNATION
--
2.17.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 inf
[PATCH 2/3] usb: typec: ucsi: acpi: Workaround for cache mode issue
This fixes an issue where the driver fails with an error:
ioremap error for 0x3f799000-0x3f79a000, requested 0x2, got 0x0
On some platforms the UCSI ACPI mailbox SystemMemory
Operation Region may be setup before the driver has been
loaded. That will lead into the driver failing to map the
mailbox region, as it has been already marked as write-back
memory. acpi_os_ioremap() for x86 uses ioremap_cache()
unconditionally.
When the issue happens, the embedded controller has a
pending query event for the UCSI notification right after
boot-up which causes the operation region to be setup before
UCSI driver has been loaded.
The fix is to notify acpi core that the driver is about to
access memory region which potentially overlaps with an
operation region right before mapping it.
acpi_release_memory() will check if the memory has already
been setup (mapped) by acpi core, and deactivate it (unmap)
if it has. The driver is then able to map the memory with
ioremap_nocache() and set the memtype to uncached for the
region.
Reported-by: Paul Menzel
Fixes: 8243edf44152 ("usb: typec: ucsi: Add ACPI driver")
Cc: sta...@vger.kernel.org
Signed-off-by: Heikki Krogerus
---
drivers/usb/typec/ucsi/ucsi_acpi.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/usb/typec/ucsi/ucsi_acpi.c
b/drivers/usb/typec/ucsi/ucsi_acpi.c
index 44eb4e1ea817..a18112a83fae 100644
--- a/drivers/usb/typec/ucsi/ucsi_acpi.c
+++ b/drivers/usb/typec/ucsi/ucsi_acpi.c
@@ -79,6 +79,11 @@ static int ucsi_acpi_probe(struct platform_device *pdev)
return -ENODEV;
}
+ /* This will make sure we can use ioremap_nocache() */
+ status = acpi_release_memory(ACPI_HANDLE(&pdev->dev), res, 1);
+ if (ACPI_FAILURE(status))
+ return -ENOMEM;
+
/*
* NOTE: The memory region for the data structures is used also in an
* operation region, which means ACPI has already reserved it. Therefore
--
2.17.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
[PATCH 3/3] usb: typec: ucsi: Fix for incorrect status data issue
According to UCSI Specification, Connector Change Event only
means a change in the Connector Status and Operation Mode
fields of the STATUS data structure. So any other change
should create another event.
Unfortunately on some platforms the firmware acting as PPM
(platform policy manager - usually embedded controller
firmware) still does not report any other status changes if
there is a connector change event. So if the connector power
or data role was changed when a device was plugged to the
connector, the driver does not get any indication about
that. The port will show wrong roles if that happens.
To fix the issue, always checking the data and power role
together with a connector change event.
Fixes: c1b0bc2dabfa ("usb: typec: Add support for UCSI interface")
Signed-off-by: Heikki Krogerus
---
drivers/usb/typec/ucsi/ucsi.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index bd5cca5632b3..8d0a6fe748bd 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -350,6 +350,19 @@ static void ucsi_connector_change(struct work_struct *work)
}
if (con->status.change & UCSI_CONSTAT_CONNECT_CHANGE) {
+ typec_set_pwr_role(con->port, con->status.pwr_dir);
+
+ switch (con->status.partner_type) {
+ case UCSI_CONSTAT_PARTNER_TYPE_UFP:
+ typec_set_data_role(con->port, TYPEC_HOST);
+ break;
+ case UCSI_CONSTAT_PARTNER_TYPE_DFP:
+ typec_set_data_role(con->port, TYPEC_DEVICE);
+ break;
+ default:
+ break;
+ }
+
if (con->status.connected)
ucsi_register_partner(con);
else
--
2.17.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] usb: dwc3: of-simple: fix use-after-free on remove
On Thu, 21 Jun 2018, Roger Quadros wrote: > >>> probe() > >>> pm_runtime_forbid() 1 > > Can you call pm_runtime_forbid() before pm_runtime_enable()? > Wouldn't it fail with -EACCES as dev->power.disable_depth > 0? Look, there has been a lot of confusion in this email thread. Let's get some things straightened out before it goes much farther. There are only a very few reasons for ever calling pm_runtime_forbid() or pm_runtime_allow() at any time other than just before the device is registered. In fact, I can only think of one reason at the moment: If a device belongs to a class which is well known to have excellent support for runtime PM, a driver might want to call pm_runtime_allow(). But in general, a driver should not call these routines. The decision about whether or not a device should be allowed to go into runtime suspend is a policy matter and therefore should be decided by the user, not by the kernel. Furthermore, these calls merely set a default value; the default can be overridden by the user at any time and therefore a driver cannot depend on these functions for anything. Another point of confusion involves balancing pm_runtime_get_* and pm_runtime_put_* calls. They should always end up in balance, and at any moment there never should be more put's than get's except in one very particular circumstance: The bus system may guarantee to invoke probe callbacks after performing an extra get, and to perform an extra put after invoking remove callbacks (the PCI subsystem does this). This can be useful when a lot of drivers don't support runtime PM; the extra get insures that the PM core will never try to suspend the device while such a driver is bound to it. A driver that _does_ support runtime PM would do an extra pm_runtime_put at the end of its probe routine and an extra pm_runtime_get_sync in its remove routine; this will allow runtime PM to work while keeping the counts non-negative. That's the only situation I know of where it's reasonable to have more puts than gets. The PCI subsystem does this, and to be perfectly honest, I do not remember why local_pci_probe() recommends that drivers call pm_runtime_put_noidle rather than pm_runtime_put. On the face of it, this would allow the PM usage counter to go to 0 without triggering an immediate runtime suspend. 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: make a confirm for [usb: dwc3: gadget: skip Set/Clear Halt when invalid]
On Thu, 21 Jun 2018, Felipe Balbi wrote: > > Hi, > > that patch is not 100% correct. You can revert it in your tree. I added > that because of a problem I found when running adb against macOS. > > It's actually okay to send Clear Halt at any time, but for some reason > dwc3 was hanging when running adb against macOS. Note: According to the USB spec it's okay to send Clear-Halt at any time. But there are plenty of devices that get upset if they receive this message when the endpoint isn't actually halted. 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 1/7] usb: gadget: f_uac2: fix error handling in afunc_bind (again)
If usb_ep_autoconfig() fails (i.e. returns a null endpoint descriptor),
we expect afunc_bind() to fail (i.e. return a negative error code).
However, due to v4.10-rc1 commit f1d3861d63a5 ("usb: gadget: f_uac2: fix
error handling at afunc_bind"), afunc_bind() returns zero, telling the
caller that it succeeded. This then generates NULL pointer dereference
in below scenario on Rcar H3-ES20-Salvator-X target:
rcar-gen3:/home/root# modprobe g_audio
[ 626.521155] g_audio gadget: afunc_bind:565 Error!
[ 626.526319] g_audio gadget: Linux USB Audio Gadget, version: Feb 2, 2012
[ 626.533405] g_audio gadget: g_audio ready
rcar-gen3:/home/root#
rcar-gen3:/home/root# modprobe -r g_audio
[ 728.256707]
==
[ 728.264293] BUG: KASAN: null-ptr-deref in u_audio_stop_capture+0x70/0x268
[u_audio]
[ 728.272244] Read of size 8 at addr 00a0 by task modprobe/2545
[ 728.279309]
[ 728.280849] CPU: 0 PID: 2545 Comm: modprobe Tainted: GWC
4.14.47+ #152
[ 728.288778] Hardware name: Renesas Salvator-X board based on r8a7795 ES2.0+
(DT)
[ 728.296454] Call trace:
[ 728.299151] [] dump_backtrace+0x0/0x364
[ 728.304808] [] show_stack+0x14/0x1c
[ 728.310081] [] dump_stack+0x108/0x174
[ 728.315522] [] kasan_report+0x1fc/0x354
[ 728.321134] [] __asan_load8+0x24/0x94
[ 728.326600] [] u_audio_stop_capture+0x70/0x268 [u_audio]
[ 728.333735] [] afunc_disable+0x44/0x60 [usb_f_uac2]
[ 728.340503] [] usb_remove_function+0x9c/0x210
[libcomposite]
[ 728.348060] [] remove_config.isra.2+0x1d8/0x218
[libcomposite]
[ 728.355788] [] __composite_unbind+0x104/0x1f8
[libcomposite]
[ 728.363339] [] composite_unbind+0x10/0x18 [libcomposite]
[ 728.370536] [] usb_gadget_remove_driver+0xc0/0x170
[udc_core]
[ 728.378172] [] usb_gadget_unregister_driver+0x1cc/0x258
[udc_core]
[ 728.386274] [] usb_composite_unregister+0x10/0x18
[libcomposite]
[ 728.394116] [] audio_driver_exit+0x14/0x28 [g_audio]
[ 728.400878] [] SyS_delete_module+0x288/0x32c
[ 728.406935] Exception stack(0x8006cf6c7ec0 to 0x8006cf6c8000)
[ 728.413624] 7ec0: 06136428 0800
d706efe8
[ 728.421718] 7ee0: d706efe9 000a 1999
[ 728.429792] 7f00: 006a 0042c078
0005
[ 728.437870] 7f20: 0004
[ 728.445952] 7f40: 0042bfc8 bc7c8f40
061363c0
[ 728.454035] 7f60: 06136428
06136428
[ 728.462114] 7f80: 0042c000 d7071448 0042c000
[ 728.470190] 7fa0: 061350c0 d7070010 0041129c
d7070010
[ 728.478281] 7fc0: bc7c8f48 6000 06136428
006a
[ 728.486351] 7fe0:
[ 728.494434] [] el0_svc_naked+0x34/0x38
[ 728.499957]
==
[ 728.507801] Unable to handle kernel NULL pointer dereference at virtual
address 00a0
[ 728.517742] Mem abort info:
[ 728.520993] Exception class = DABT (current EL), IL = 32 bits
[ 728.527375] SET = 0, FnV = 0
[ 728.530731] EA = 0, S1PTW = 0
[ 728.534361] Data abort info:
[ 728.537650] ISV = 0, ISS = 0x0006
[ 728.541863] CM = 0, WnR = 0
[ 728.545167] user pgtable: 4k pages, 48-bit VAs, pgd = 8006c610
[ 728.552156] [00a0] *pgd=000716a8d003
[ 728.557519] , *pud=0007116fc003
[ 728.561259] , *pmd=
[ 728.564985] Internal error: Oops: 9606 [#1] PREEMPT SMP
[ 728.570815] Modules linked in:
[ 728.574023] usb_f_uac2
[ 728.576560] u_audio
[ 728.578827] g_audio(-)
[ 728.581361] libcomposite
[ 728.584071] configfs
[ 728.586428] aes_ce_blk
[ 728.588960] sata_rcar
[ 728.591421] crypto_simd
[ 728.594039] cryptd
[ 728.596217] libata
[ 728.598396] aes_ce_cipher
[ 728.601188] crc32_ce
[ 728.603542] ghash_ce
[ 728.605896] gf128mul
[ 728.608250] aes_arm64
[ 728.610692] scsi_mod
[ 728.613046] sha2_ce
[ 728.615313] xhci_plat_hcd
[ 728.618106] sha256_arm64
[ 728.620811] sha1_ce
[ 728.623077] renesas_usbhs
[ 728.625869] xhci_hcd
[ 728.628243] renesas_usb3
[ 728.630948] sha1_generic
[ 728.633670] ravb_streaming(C)
[ 728.636814] udc_core
[ 728.639168] cpufreq_dt
[ 728.641697] rcar_gen3_thermal
[ 728.644840] usb_dmac
[ 728.647194] pwm_rcar
[ 728.649548] thermal_sys
[ 728.652165] virt_dma
[ 728.654519] mch_core(C)
[ 728.657137] pwm_bl
[ 728.659315] snd_soc_rcar
[ 728.662020] snd_aloop
[ 728.664462] snd_soc_generic_card
[ 728.667869] snd_soc_ak4613
[ 728.670749] ipv6
[ 728.672768] autofs4
[ 728.675052] CPU: 0 PID: 2545 Comm: modprobe Tainted: GB WC
4.14.47+ #152
[ 728.682973] Hardware
[PATCH 2/7] usb: gadget: u_audio: fix pcm/card naming in g_audio_setup()
Fix below smatch (v0.5.0-4443-g69e9094e11c1) warnings:
drivers/usb/gadget/function/u_audio.c:607 g_audio_setup() warn: strcpy()
'pcm_name' of unknown size might be too large for 'pcm->name'
drivers/usb/gadget/function/u_audio.c:614 g_audio_setup() warn: strcpy()
'card_name' of unknown size might be too large for 'card->driver'
drivers/usb/gadget/function/u_audio.c:615 g_audio_setup() warn: strcpy()
'card_name' of unknown size might be too large for 'card->shortname'
Below commits performed a similar 's/strcpy/strlcpy/' rework:
* v2.6.31 commit 8372d4980fbc ("ALSA: ctxfi - Fix PCM device naming")
* v4.14 commit 003d3e70dbeb ("ALSA: ad1848: fix format string overflow warning")
* v4.14 commit 6d8b04de87e1 ("ALSA: cs423x: fix format string overflow warning")
Fixes: eb9fecb9e69b ("usb: gadget: f_uac2: split out audio core")
Signed-off-by: Eugeniu Rosca
---
drivers/usb/gadget/function/u_audio.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/function/u_audio.c
b/drivers/usb/gadget/function/u_audio.c
index a72295c953bb..c0a65e110fc9 100644
--- a/drivers/usb/gadget/function/u_audio.c
+++ b/drivers/usb/gadget/function/u_audio.c
@@ -595,15 +595,15 @@ int g_audio_setup(struct g_audio *g_audio, const char
*pcm_name,
if (err < 0)
goto snd_fail;
- strcpy(pcm->name, pcm_name);
+ strlcpy(pcm->name, pcm_name, sizeof(pcm->name));
pcm->private_data = uac;
uac->pcm = pcm;
snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, &uac_pcm_ops);
snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, &uac_pcm_ops);
- strcpy(card->driver, card_name);
- strcpy(card->shortname, card_name);
+ strlcpy(card->driver, card_name, sizeof(card->driver));
+ strlcpy(card->shortname, card_name, sizeof(card->shortname));
sprintf(card->longname, "%s %i", card_name, card->dev->id);
snd_pcm_lib_preallocate_pages_for_all(pcm, SNDRV_DMA_TYPE_CONTINUOUS,
--
2.17.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 3/7] usb: gadget: u_audio: update hw_ptr in iso_complete after data copied
From: Joshua Frkuska
In u_audio_iso_complete, the runtime hw_ptr is updated before the
data is actually copied over to/from the buffer/dma area. When
ALSA uses this hw_ptr, the data may not actually be available to
be used. This causes trash/stale audio to play/record. This
patch updates the hw_ptr after the data has been copied to avoid
this.
Fixes: 132fcb460839 ("usb: gadget: Add Audio Class 2.0 Driver")
Signed-off-by: Joshua Frkuska
Signed-off-by: Eugeniu Rosca
---
drivers/usb/gadget/function/u_audio.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/function/u_audio.c
b/drivers/usb/gadget/function/u_audio.c
index c0a65e110fc9..4c6df7130891 100644
--- a/drivers/usb/gadget/function/u_audio.c
+++ b/drivers/usb/gadget/function/u_audio.c
@@ -143,7 +143,6 @@ static void u_audio_iso_complete(struct usb_ep *ep, struct
usb_request *req)
update_alsa = true;
hw_ptr = prm->hw_ptr;
- prm->hw_ptr = (prm->hw_ptr + req->actual) % prm->dma_bytes;
spin_unlock_irqrestore(&prm->lock, flags);
@@ -168,6 +167,11 @@ static void u_audio_iso_complete(struct usb_ep *ep, struct
usb_request *req)
}
}
+ spin_lock_irqsave(&prm->lock, flags);
+ /* update hw_ptr after data is copied to memory */
+ prm->hw_ptr = (hw_ptr + req->actual) % prm->dma_bytes;
+ spin_unlock_irqrestore(&prm->lock, flags);
+
exit:
if (usb_ep_queue(ep, req, GFP_ATOMIC))
dev_err(uac->card->dev, "%d Error!\n", __LINE__);
--
2.17.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 0/7] Fix issues in UAC2 gadget driver
Hello Felipe and linux-usb community,
This is a collection of UAC fixes, some of which have been written back
in 2016 on top of v3.14. v4.13-rc1 commit eb9fecb9e69b ("usb: gadget:
f_uac2: split out audio core") made forward-porting of these patches to
recent kernels quite painful and time-consuming, so we decided to
re-base them against v4.18-rc1, re-test and share them with community.
These changes have already undergone internal review, which includes:
* static analysis:
- checkpatch --strict, cppcheck, smatch, sparse, coccicheck, make W=1
* dynamic testing:
- Audio playback on host side and recording on gadget side, where the
latter is RCAR Gen3 Salvator-X arm64 target and the former is a
Ubuntu PC or an appropriate USB port on the same RCAR target.
The kernel was compiled with and w/o KA/UB sanitizers.
The "older" patches actually live in real Customer products.
Thank you in advance for any feedback/review comments.
Best regards,
Eugeniu.
Contributors (auto-generated by git)
Andreas Pape (1):
usb: gadget: f_uac2: disable IN/OUT ep if unused
Eugeniu Rosca (2):
usb: gadget: f_uac2: fix error handling in afunc_bind (again)
usb: gadget: u_audio: fix pcm/card naming in g_audio_setup()
Joshua Frkuska (1):
usb: gadget: u_audio: update hw_ptr in iso_complete after data copied
Vladimir Zapolskiy (3):
usb: gadget: u_audio: remove caching of stream buffer parameters
usb: gadget: u_audio: remove cached period bytes value
usb: gadget: u_audio: protect stream runtime fields with stream
spinlock
drivers/usb/gadget/function/f_uac2.c | 216 --
drivers/usb/gadget/function/u_audio.c | 88 ---
2 files changed, 205 insertions(+), 99 deletions(-)
--
2.17.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: usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers
On Thu, 21 Jun 2018, Roger Quadros wrote: > >> Can we avoid the spin_lock() and the work-queue and call usb_ep_dequeue() > >> directly from here? > >>> What is the purpose of the spin_lock()? > > > > I agree that the lock doesn't seem to be necessary. But I believe the whole > > thing is already running in non-sleeping context, even before the spinlock > > is taken. So this wouldn't help much. > > > > Even the io_cancel() syscall takes a spinlock before invoking the cancel > > function. So this issue is not exclusive to program termination. > > > > Are there any documented guidelines on which context usb_ep_dequeue() should > > be able to be called in? The sleep in the dwc3 driver seems to be a recent > > addition. > > drivers/usb/udc/gadget/core.c has the only documentation, but context is not > mentioned there. > Felipe, what do you suggest? As far as I remember, usb_ep_dequeue() is supposed to be more or less analogous to usb_ep_queue(); drivers should be allowed to call either routine in an atomic context. 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] Bluetooth: btusb: use irqsave() in URB's complete callback
On Thu, 21 Jun 2018, Sebastian Andrzej Siewior wrote: > On 2018-06-21 14:43:41 [+0200], Marcel Holtmann wrote: > > Hi Sebastian, > Hi Marcel, > > > > The USB completion callback does not disable interrupts while acquiring > > > the ->lock. We want to remove the local_irq_disable() invocation from > > > __usb_hcd_giveback_urb() and therefore it is required for the callback > > > handler to disable the interrupts while acquiring the lock. > > > The callback may be invoked either in IRQ or BH context depending on the > > > USB host controller. > > > Use the _irqsave variant of the locking primitives. > > > > > > Cc: Marcel Holtmann > > > Cc: Johan Hedberg > > > Cc: linux-blueto...@vger.kernel.org > > > Signed-off-by: Sebastian Andrzej Siewior > > > --- > > > drivers/bluetooth/btusb.c | 20 > > > 1 file changed, 12 insertions(+), 8 deletions(-) > > > > can I get an ACK from someone ensuring that this is the direction we are > > going with the USB host controllers? > +Alan. > > EHCI completes in BH since v3.12-rc1. In order to get rid of that > local_irq_save() in USB core code I need to make sure that the USB > device driver(s) use irqsave primitives. See > > https://lkml.kernel.org/r/pine.lnx.4.44l0.1806011629140.1404-100...@iolanthe.rowland.org Hi, Marcel! Yes, Sebastian is right. We are aiming to make it possible for the USB core to invoke URB completion handlers with interrupts enabled, in order to reduce latency (since USB interrupt processing can take a fairly long time). And of course, this means completion handlers have to work correctly regardless of whether interrupts are enabled or disabled. Currently ehci-hcd supports this possibility. Other host controller drivers may follow along; I'd like to see xhci-hcd do this too. 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 4/7] usb: gadget: u_audio: remove caching of stream buffer parameters
From: Vladimir Zapolskiy
There is no necessity to copy PCM stream ring buffer area and size
properties to UAC private data structure, these values can be got
from substream itself.
The change gives more control on substream and avoid stale caching.
Fixes: 132fcb460839 ("usb: gadget: Add Audio Class 2.0 Driver")
Signed-off-by: Vladimir Zapolskiy
Signed-off-by: Eugeniu Rosca
---
drivers/usb/gadget/function/u_audio.c | 30 ---
1 file changed, 13 insertions(+), 17 deletions(-)
diff --git a/drivers/usb/gadget/function/u_audio.c
b/drivers/usb/gadget/function/u_audio.c
index 4c6df7130891..b8fc2d6d156f 100644
--- a/drivers/usb/gadget/function/u_audio.c
+++ b/drivers/usb/gadget/function/u_audio.c
@@ -32,9 +32,6 @@ struct uac_req {
struct uac_rtd_params {
struct snd_uac_chip *uac; /* parent chip */
bool ep_enabled; /* if the ep is enabled */
- /* Size of the ring buffer */
- size_t dma_bytes;
- unsigned char *dma_area;
struct snd_pcm_substream *ss;
@@ -90,6 +87,7 @@ static void u_audio_iso_complete(struct usb_ep *ep, struct
usb_request *req)
int status = req->status;
struct uac_req *ur = req->context;
struct snd_pcm_substream *substream;
+ struct snd_pcm_runtime *runtime;
struct uac_rtd_params *prm = ur->pp;
struct snd_uac_chip *uac = prm->uac;
@@ -111,6 +109,7 @@ static void u_audio_iso_complete(struct usb_ep *ep, struct
usb_request *req)
if (!substream)
goto exit;
+ runtime = substream->runtime;
spin_lock_irqsave(&prm->lock, flags);
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
@@ -147,29 +146,31 @@ static void u_audio_iso_complete(struct usb_ep *ep,
struct usb_request *req)
spin_unlock_irqrestore(&prm->lock, flags);
/* Pack USB load in ALSA ring buffer */
- pending = prm->dma_bytes - hw_ptr;
+ pending = runtime->dma_bytes - hw_ptr;
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
if (unlikely(pending < req->actual)) {
- memcpy(req->buf, prm->dma_area + hw_ptr, pending);
- memcpy(req->buf + pending, prm->dma_area,
+ memcpy(req->buf, runtime->dma_area + hw_ptr, pending);
+ memcpy(req->buf + pending, runtime->dma_area,
req->actual - pending);
} else {
- memcpy(req->buf, prm->dma_area + hw_ptr, req->actual);
+ memcpy(req->buf, runtime->dma_area + hw_ptr,
+ req->actual);
}
} else {
if (unlikely(pending < req->actual)) {
- memcpy(prm->dma_area + hw_ptr, req->buf, pending);
- memcpy(prm->dma_area, req->buf + pending,
+ memcpy(runtime->dma_area + hw_ptr, req->buf, pending);
+ memcpy(runtime->dma_area, req->buf + pending,
req->actual - pending);
} else {
- memcpy(prm->dma_area + hw_ptr, req->buf, req->actual);
+ memcpy(runtime->dma_area + hw_ptr, req->buf,
+ req->actual);
}
}
spin_lock_irqsave(&prm->lock, flags);
/* update hw_ptr after data is copied to memory */
- prm->hw_ptr = (hw_ptr + req->actual) % prm->dma_bytes;
+ prm->hw_ptr = (hw_ptr + req->actual) % runtime->dma_bytes;
spin_unlock_irqrestore(&prm->lock, flags);
exit:
@@ -251,11 +252,8 @@ static int uac_pcm_hw_params(struct snd_pcm_substream
*substream,
err = snd_pcm_lib_malloc_pages(substream,
params_buffer_bytes(hw_params));
- if (err >= 0) {
- prm->dma_bytes = substream->runtime->dma_bytes;
- prm->dma_area = substream->runtime->dma_area;
+ if (err >= 0)
prm->period_size = params_period_bytes(hw_params);
- }
return err;
}
@@ -270,8 +268,6 @@ static int uac_pcm_hw_free(struct snd_pcm_substream
*substream)
else
prm = &uac->c_prm;
- prm->dma_area = NULL;
- prm->dma_bytes = 0;
prm->period_size = 0;
return snd_pcm_lib_free_pages(substream);
--
2.17.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 6/7] usb: gadget: f_uac2: disable IN/OUT ep if unused
From: Andreas Pape
Via p_chmask/c_chmask the user can define whether uac2 shall support
playback and/or capture. This has only effect on the created ALSA device,
but not on the USB descriptor. This patch adds playback/capture descriptors
dependent on that parameter.
Signed-off-by: Andreas Pape
Signed-off-by: Eugeniu Rosca
---
drivers/usb/gadget/function/f_uac2.c | 216 +--
1 file changed, 172 insertions(+), 44 deletions(-)
diff --git a/drivers/usb/gadget/function/f_uac2.c
b/drivers/usb/gadget/function/f_uac2.c
index 6954db22c2f3..87fa6f14efcf 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -22,12 +22,8 @@
* controlled by two clock sources :
*CLK_5 := c_srate, and CLK_6 := p_srate
*/
-#define USB_OUT_IT_ID 1
-#define IO_IN_IT_ID2
-#define IO_OUT_OT_ID 3
-#define USB_IN_OT_ID 4
-#define USB_OUT_CLK_ID 5
-#define USB_IN_CLK_ID 6
+#define USB_OUT_CLK_ID (out_clk_src_desc.bClockID)
+#define USB_IN_CLK_ID (in_clk_src_desc.bClockID)
#define CONTROL_ABSENT 0
#define CONTROL_RDONLY 1
@@ -43,6 +39,9 @@
#define UNFLW_CTRL 8
#define OVFLW_CTRL 10
+#define EPIN_EN(_opts) ((_opts)->p_chmask != 0)
+#define EPOUT_EN(_opts) ((_opts)->c_chmask != 0)
+
struct f_uac2 {
struct g_audio g_audio;
u8 ac_intf, as_in_intf, as_out_intf;
@@ -135,7 +134,7 @@ static struct uac_clock_source_descriptor in_clk_src_desc =
{
.bDescriptorType = USB_DT_CS_INTERFACE,
.bDescriptorSubtype = UAC2_CLOCK_SOURCE,
- .bClockID = USB_IN_CLK_ID,
+ /* .bClockID = DYNAMIC */
.bmAttributes = UAC_CLOCK_SOURCE_TYPE_INT_FIXED,
.bmControls = (CONTROL_RDONLY << CLK_FREQ_CTRL),
.bAssocTerminal = 0,
@@ -147,7 +146,7 @@ static struct uac_clock_source_descriptor out_clk_src_desc
= {
.bDescriptorType = USB_DT_CS_INTERFACE,
.bDescriptorSubtype = UAC2_CLOCK_SOURCE,
- .bClockID = USB_OUT_CLK_ID,
+ /* .bClockID = DYNAMIC */
.bmAttributes = UAC_CLOCK_SOURCE_TYPE_INT_FIXED,
.bmControls = (CONTROL_RDONLY << CLK_FREQ_CTRL),
.bAssocTerminal = 0,
@@ -159,10 +158,10 @@ static struct uac2_input_terminal_descriptor
usb_out_it_desc = {
.bDescriptorType = USB_DT_CS_INTERFACE,
.bDescriptorSubtype = UAC_INPUT_TERMINAL,
- .bTerminalID = USB_OUT_IT_ID,
+ /* .bTerminalID = DYNAMIC */
.wTerminalType = cpu_to_le16(UAC_TERMINAL_STREAMING),
.bAssocTerminal = 0,
- .bCSourceID = USB_OUT_CLK_ID,
+ /* .bCSourceID = DYNAMIC */
.iChannelNames = 0,
.bmControls = cpu_to_le16(CONTROL_RDWR << COPY_CTRL),
};
@@ -173,10 +172,10 @@ static struct uac2_input_terminal_descriptor
io_in_it_desc = {
.bDescriptorType = USB_DT_CS_INTERFACE,
.bDescriptorSubtype = UAC_INPUT_TERMINAL,
- .bTerminalID = IO_IN_IT_ID,
+ /* .bTerminalID = DYNAMIC */
.wTerminalType = cpu_to_le16(UAC_INPUT_TERMINAL_UNDEFINED),
.bAssocTerminal = 0,
- .bCSourceID = USB_IN_CLK_ID,
+ /* .bCSourceID = DYNAMIC */
.iChannelNames = 0,
.bmControls = cpu_to_le16(CONTROL_RDWR << COPY_CTRL),
};
@@ -187,11 +186,11 @@ static struct uac2_output_terminal_descriptor
usb_in_ot_desc = {
.bDescriptorType = USB_DT_CS_INTERFACE,
.bDescriptorSubtype = UAC_OUTPUT_TERMINAL,
- .bTerminalID = USB_IN_OT_ID,
+ /* .bTerminalID = DYNAMIC */
.wTerminalType = cpu_to_le16(UAC_TERMINAL_STREAMING),
.bAssocTerminal = 0,
- .bSourceID = IO_IN_IT_ID,
- .bCSourceID = USB_IN_CLK_ID,
+ /* .bSourceID = DYNAMIC */
+ /* .bCSourceID = DYNAMIC */
.bmControls = cpu_to_le16(CONTROL_RDWR << COPY_CTRL),
};
@@ -201,11 +200,11 @@ static struct uac2_output_terminal_descriptor
io_out_ot_desc = {
.bDescriptorType = USB_DT_CS_INTERFACE,
.bDescriptorSubtype = UAC_OUTPUT_TERMINAL,
- .bTerminalID = IO_OUT_OT_ID,
+ /* .bTerminalID = DYNAMIC */
.wTerminalType = cpu_to_le16(UAC_OUTPUT_TERMINAL_UNDEFINED),
.bAssocTerminal = 0,
- .bSourceID = USB_OUT_IT_ID,
- .bCSourceID = USB_OUT_CLK_ID,
+ /* .bSourceID = DYNAMIC */
+ /* .bCSourceID = DYNAMIC */
.bmControls = cpu_to_le16(CONTROL_RDWR << COPY_CTRL),
};
@@ -253,7 +252,7 @@ static struct uac2_as_header_descriptor as_out_hdr_desc = {
.bDescriptorType = USB_DT_CS_INTERFACE,
.bDescriptorSubtype = UAC_AS_GENERAL,
- .bTerminalLink = USB_OUT_IT_ID,
+ /* .bTerminalLink = DYNAMIC */
.bmControls = 0,
.bFormatType = UAC_FORMAT_TYPE_I,
.bmFormats = cpu_to_le32(UAC_FORMAT_TYPE_I_PCM),
@@ -330,7 +329,7 @@ static struct uac2_as_header_descriptor as_in_hdr_desc = {
.bDescriptorType = USB_DT_CS_INTERFACE,
.bDescriptorSubtype = UAC_AS_GENERAL,
- .bTerminalLink = USB_IN_OT_ID,
+ /* .bTerminalLink = DYNAMIC */
.bmControls = 0,
[PATCH 7/7] usb: gadget: u_audio: protect stream runtime fields with stream spinlock
From: Vladimir Zapolskiy The change protects almost the whole body of u_audio_iso_complete() function by PCM stream lock, this is mainly sufficient to avoid a race between USB request completion and stream termination, the change prevents a possibility of invalid memory access in interrupt context by memcpy(): Unable to handle kernel paging request at virtual address 4e80 pgd = c0004000 [4e80] *pgd= Internal error: Oops: 817 [#1] PREEMPT SMP ARM CPU: 0 PID: 3 Comm: ksoftirqd/0 Tainted: G C 3.14.54+ #117 task: da180b80 ti: da192000 task.ti: da192000 PC is at memcpy+0x50/0x330 LR is at 0xcdd92b0e pc : []lr : []psr: 2193 sp : da193ce4 ip : dd86ae26 fp : b180 r10: daf81680 r9 : r8 : d58a01ea r7 : 2c0b43e4 r6 : acdfb08b r5 : 01a271cf r4 : 87389377 r3 : 69469782 r2 : 0020 r1 : daf82fe0 r0 : 4e80 Flags: nzCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment kernel Control: 10c5387d Table: 2b70804a DAC: 0015 Process ksoftirqd/0 (pid: 3, stack limit = 0xda192238) Also added a check for potential !runtime condition, commonly it is done by PCM_RUNTIME_CHECK(substream) in the beginning, however this does not completely prevent from oopses in u_audio_iso_complete(), because the proper protection scheme must be implemented in PCM library functions. An example of *not fixed* oops due to substream->runtime->* dereference by snd_pcm_running(substream) from snd_pcm_period_elapsed(), where substream->runtime is gone while waiting the substream lock: Unable to handle kernel paging request at virtual address 6b6b6b6b pgd = db7e4000 [6b6b6b6b] *pgd= CPU: 0 PID: 193 Comm: klogd Tainted: G C 3.14.54+ #118 task: db5ac500 ti: db60c000 task.ti: db60c000 PC is at snd_pcm_period_elapsed+0x48/0xd8 [snd_pcm] LR is at snd_pcm_period_elapsed+0x40/0xd8 [snd_pcm] pc : [<>]lr : [<>]psr: 6193 Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment user Control: 10c5387d Table: 2b7e404a DAC: 0015 Process klogd (pid: 193, stack limit = 0xdb60c238) [<>] (snd_pcm_period_elapsed [snd_pcm]) from [<>] (udc_irq+0x500/0xbbc) [<>] (udc_irq) from [<>] (ci_irq+0x280/0x304) [<>] (ci_irq) from [<>] (handle_irq_event_percpu+0xa4/0x40c) [<>] (handle_irq_event_percpu) from [<>] (handle_irq_event+0x3c/0x5c) [<>] (handle_irq_event) from [<>] (handle_fasteoi_irq+0xc4/0x110) [<>] (handle_fasteoi_irq) from [<>] (generic_handle_irq+0x20/0x30) [<>] (generic_handle_irq) from [<>] (handle_IRQ+0x80/0xc0) [<>] (handle_IRQ) from [<>] (gic_handle_irq+0x3c/0x60) [<>] (gic_handle_irq) from [<>] (__irq_svc+0x44/0x78) Signed-off-by: Vladimir Zapolskiy [erosca: W/o this patch, with minimal instrumentation [1], I can consistently reproduce BUG: KASAN: use-after-free [2]] [1] Instrumentation to reproduce issue [2]: diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c index a72295c953bb..bd0b308024fe 100644 --- a/drivers/usb/gadget/function/u_audio.c +++ b/drivers/usb/gadget/function/u_audio.c @@ -16,6 +16,7 @@ #include #include #include +#include #include "u_audio.h" @@ -147,6 +148,8 @@ static void u_audio_iso_complete(struct usb_ep *ep, struct usb_request *req) spin_unlock_irqrestore(&prm->lock, flags); + udelay(500); //delay here to increase probability of parallel activities + /* Pack USB load in ALSA ring buffer */ pending = prm->dma_bytes - hw_ptr; [2] After applying [1], below BUG occurs on Rcar-H3-Salvator-X board: == BUG: KASAN: use-after-free in u_audio_iso_complete+0x24c/0x520 [u_audio] Read of size 8 at addr 8006cafcc248 by task swapper/0/0 CPU: 0 PID: 0 Comm: swapper/0 Tainted: GWC 4.14.47+ #160 Hardware name: Renesas Salvator-X board based on r8a7795 ES2.0+ (DT) Call trace: [] dump_backtrace+0x0/0x364 [] show_stack+0x14/0x1c [] dump_stack+0x108/0x174 [] print_address_description+0x7c/0x32c [] kasan_report+0x324/0x354 [] __asan_load8+0x24/0x94 [] u_audio_iso_complete+0x24c/0x520 [u_audio] [] usb_gadget_giveback_request+0x480/0x4d0 [udc_core] [] usbhsg_queue_done+0x100/0x130 [renesas_usbhs] [] usbhsf_pkt_handler+0x1a4/0x298 [renesas_usbhs] [] usbhsf_irq_ready+0x128/0x178 [renesas_usbhs] [] usbhs_interrupt+0x440/0x490 [renesas_usbhs] [] __handle_irq_event_percpu+0x594/0xa58 [] handle_irq_event_percpu+0x84/0x12c [] handle_irq_event+0xb0/0x10c [] handle_fasteoi_irq+0x1e0/0x2ec [] generic_handle_irq+0x2c/0x44 [] __handle_domain_irq+0x190/0x194 [] gic_handle_irq+0x80/0xac Exception stack(0x29e97c80 to 0x29e97dc0) 7c80: 0003 28179298 7ca0: 2ae1c180 dfff2000 281f9a88 7cc0: 29eb5960 29e97cf0
[PATCH 5/7] usb: gadget: u_audio: remove cached period bytes value
From: Vladimir Zapolskiy
Substream period size potentially can be changed in runtime, however
this is not accounted in the data copying routine, the change replaces
the cached value with an actual value from substream runtime.
As a side effect the change also removes a potential division by zero
in u_audio_iso_complete() function, if there is a race with
uac_pcm_hw_free(), which sets prm->period_size to 0.
Fixes: 132fcb460839 ("usb: gadget: Add Audio Class 2.0 Driver")
Signed-off-by: Vladimir Zapolskiy
Signed-off-by: Eugeniu Rosca
---
drivers/usb/gadget/function/u_audio.c | 40 ---
1 file changed, 5 insertions(+), 35 deletions(-)
diff --git a/drivers/usb/gadget/function/u_audio.c
b/drivers/usb/gadget/function/u_audio.c
index b8fc2d6d156f..8a63009b7cd5 100644
--- a/drivers/usb/gadget/function/u_audio.c
+++ b/drivers/usb/gadget/function/u_audio.c
@@ -40,8 +40,6 @@ struct uac_rtd_params {
void *rbuf;
- size_t period_size;
-
unsigned max_psize; /* MaxPacketSize of endpoint */
struct uac_req *ureq;
@@ -83,7 +81,6 @@ static void u_audio_iso_complete(struct usb_ep *ep, struct
usb_request *req)
unsigned pending;
unsigned long flags;
unsigned int hw_ptr;
- bool update_alsa = false;
int status = req->status;
struct uac_req *ur = req->context;
struct snd_pcm_substream *substream;
@@ -136,11 +133,6 @@ static void u_audio_iso_complete(struct usb_ep *ep, struct
usb_request *req)
req->actual = req->length;
}
- pending = prm->hw_ptr % prm->period_size;
- pending += req->actual;
- if (pending >= prm->period_size)
- update_alsa = true;
-
hw_ptr = prm->hw_ptr;
spin_unlock_irqrestore(&prm->lock, flags);
@@ -171,14 +163,15 @@ static void u_audio_iso_complete(struct usb_ep *ep,
struct usb_request *req)
spin_lock_irqsave(&prm->lock, flags);
/* update hw_ptr after data is copied to memory */
prm->hw_ptr = (hw_ptr + req->actual) % runtime->dma_bytes;
+ hw_ptr = prm->hw_ptr;
spin_unlock_irqrestore(&prm->lock, flags);
+ if ((hw_ptr % snd_pcm_lib_period_bytes(substream)) < req->actual)
+ snd_pcm_period_elapsed(substream);
+
exit:
if (usb_ep_queue(ep, req, GFP_ATOMIC))
dev_err(uac->card->dev, "%d Error!\n", __LINE__);
-
- if (update_alsa)
- snd_pcm_period_elapsed(substream);
}
static int uac_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
@@ -241,35 +234,12 @@ static snd_pcm_uframes_t uac_pcm_pointer(struct
snd_pcm_substream *substream)
static int uac_pcm_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *hw_params)
{
- struct snd_uac_chip *uac = snd_pcm_substream_chip(substream);
- struct uac_rtd_params *prm;
- int err;
-
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
- prm = &uac->p_prm;
- else
- prm = &uac->c_prm;
-
- err = snd_pcm_lib_malloc_pages(substream,
+ return snd_pcm_lib_malloc_pages(substream,
params_buffer_bytes(hw_params));
- if (err >= 0)
- prm->period_size = params_period_bytes(hw_params);
-
- return err;
}
static int uac_pcm_hw_free(struct snd_pcm_substream *substream)
{
- struct snd_uac_chip *uac = snd_pcm_substream_chip(substream);
- struct uac_rtd_params *prm;
-
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
- prm = &uac->p_prm;
- else
- prm = &uac->c_prm;
-
- prm->period_size = 0;
-
return snd_pcm_lib_free_pages(substream);
}
--
2.17.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] USB: serial: ftdi_sio: Add MTP NVM support
Most of FTDI's devices have an EEPROM which records FTDI devices
configuration setting (e.g. the VID, PID, I/O config...) and user
data. FT230R chip integrates a 128-byte eeprom, FT230X a 2048-byte
eeprom...
This patch adds support for FTDI EEPROM read/write via USB control
transfers and register a new nvm device to the nvmem core.
This permits to expose the eeprom as a sysfs file, allowing userspace
to read/modify FTDI configuration and its user data without having to
rely on a specific userspace USB driver.
Moreover, any upcoming new tentative to add CBUS GPIO support could
integrate CBUS EEPROM configuration reading in order to determine
which of the CBUS pins are available as GPIO.
Signed-off-by: Loic Poulain
---
v2: Use ifdef instead of IS_ENABLED
error message in case of nvmem registering failure
Fix space/tab in Kconfig
drivers/usb/serial/Kconfig| 13 -
drivers/usb/serial/ftdi_sio.c | 111 ++
drivers/usb/serial/ftdi_sio.h | 28 +++
3 files changed, 151 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/serial/Kconfig b/drivers/usb/serial/Kconfig
index 533f127..f05af5f 100644
--- a/drivers/usb/serial/Kconfig
+++ b/drivers/usb/serial/Kconfig
@@ -153,7 +153,7 @@ config USB_SERIAL_CYPRESS_M8
Supported microcontrollers in the CY4601 family are:
CY7C63741 CY7C63742 CY7C63743 CY7C64013
-
+
To compile this driver as a module, choose M here: the
module will be called cypress_m8.
@@ -181,6 +181,17 @@ config USB_SERIAL_FTDI_SIO
To compile this driver as a module, choose M here: the
module will be called ftdi_sio.
+config USB_SERIAL_FTDI_SIO_NVMEM
+ bool "FTDI MTP non-volatile memory support"
+ depends on USB_SERIAL_FTDI_SIO
+ select NVMEM
+ default y
+ help
+ Say yes here to add support for the MTP non-volatile memory
+ present on FTDI. Most of FTDI's devices have an EEPROM which
+ records FTDI device's configuration setting as well as user
+ data.
+
config USB_SERIAL_VISOR
tristate "USB Handspring Visor / Palm m50x / Sony Clie Driver"
help
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 7ea221d..9e242e8 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -40,6 +40,7 @@
#include
#include
#include
+#include
#include "ftdi_sio.h"
#include "ftdi_sio_ids.h"
@@ -73,6 +74,8 @@ struct ftdi_private {
unsigned int latency; /* latency setting in use */
unsigned short max_packet_size;
struct mutex cfg_lock; /* Avoid mess by parallel calls of config
ioctl() and change_speed() */
+
+ struct nvmem_device *eeprom;
};
/* struct ftdi_sio_quirk is used by devices requiring special attention. */
@@ -1529,6 +1532,104 @@ static int get_lsr_info(struct usb_serial_port *port,
return 0;
}
+#ifdef CONFIG_USB_SERIAL_FTDI_SIO_NVMEM
+
+static int write_eeprom(void *priv, unsigned int off, void *_val, size_t bytes)
+{
+ struct usb_serial_port *port = priv;
+ struct usb_serial *serial = port->serial;
+ struct usb_device *udev = serial->dev;
+ unsigned char *buf = _val;
+
+ while (bytes) { /* bytes value is always a multiple of 2 */
+ uint16_t val;
+ int rv;
+
+ val = buf[0] + (buf[1] << 8);
+
+ rv = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+FTDI_SIO_WRITE_EEPROM_REQUEST,
+FTDI_SIO_WRITE_EEPROM_REQUEST_TYPE,
+val, off / 2, NULL, 0, WDR_TIMEOUT);
+ if (rv < 0)
+ return rv;
+
+ off += 2;
+ buf += 2;
+ bytes -= 2;
+ }
+
+ return 0;
+}
+
+static int read_eeprom(void *priv, unsigned int off, void *val, size_t bytes)
+{
+ struct usb_serial_port *port = priv;
+ struct usb_serial *serial = port->serial;
+ struct usb_device *udev = serial->dev;
+ unsigned char *buf = val;
+
+ while (bytes) { /* bytes value is always a multiple of 2 */
+ int rv;
+
+ rv = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
+FTDI_SIO_READ_EEPROM_REQUEST,
+FTDI_SIO_READ_EEPROM_REQUEST_TYPE,
+0, off / 2, buf, 2, WDR_TIMEOUT);
+ if (rv < 0)
+ return rv;
+
+ off += 2;
+ buf += 2;
+ bytes -= 2;
+ }
+
+ return 0;
+}
+
+static int register_eeprom(struct usb_serial_port *port)
+{
+ struct ftdi_private *priv = usb_get_serial_port_data(port);
+ struct nvmem_config nvmconf = {};
+
+ switch (priv->chip_type) {
+ case FTX:
+ nvmconf.size = 2048;
+
Re: [PATCH v2] USB: serial: ftdi_sio: Add MTP NVM support
Hi Loic
On 6/21/18, Loic Poulain wrote:
> Most of FTDI's devices have an EEPROM which records FTDI devices
> configuration setting (e.g. the VID, PID, I/O config...) and user
> data. FT230R chip integrates a 128-byte eeprom, FT230X a 2048-byte
> eeprom...
>
> This patch adds support for FTDI EEPROM read/write via USB control
> transfers and register a new nvm device to the nvmem core.
>
> This permits to expose the eeprom as a sysfs file, allowing userspace
> to read/modify FTDI configuration and its user data without having to
> rely on a specific userspace USB driver.
>
> Moreover, any upcoming new tentative to add CBUS GPIO support could
> integrate CBUS EEPROM configuration reading in order to determine
> which of the CBUS pins are available as GPIO.
>
> Signed-off-by: Loic Poulain
> ---
> v2: Use ifdef instead of IS_ENABLED
> error message in case of nvmem registering failure
> Fix space/tab in Kconfig
>
> drivers/usb/serial/Kconfig| 13 -
> drivers/usb/serial/ftdi_sio.c | 111
> ++
> drivers/usb/serial/ftdi_sio.h | 28 +++
> 3 files changed, 151 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/serial/Kconfig b/drivers/usb/serial/Kconfig
> index 533f127..f05af5f 100644
> --- a/drivers/usb/serial/Kconfig
> +++ b/drivers/usb/serial/Kconfig
> @@ -153,7 +153,7 @@ config USB_SERIAL_CYPRESS_M8
>
> Supported microcontrollers in the CY4601 family are:
> CY7C63741 CY7C63742 CY7C63743 CY7C64013
> -
> +
There is no change here so please remove it.
> To compile this driver as a module, choose M here: the
> module will be called cypress_m8.
>
> @@ -181,6 +181,17 @@ config USB_SERIAL_FTDI_SIO
> To compile this driver as a module, choose M here: the
> module will be called ftdi_sio.
>
> +config USB_SERIAL_FTDI_SIO_NVMEM
> + bool "FTDI MTP non-volatile memory support"
> + depends on USB_SERIAL_FTDI_SIO
> + select NVMEM
> + default y
> + help
> + Say yes here to add support for the MTP non-volatile memory
> + present on FTDI. Most of FTDI's devices have an EEPROM which
> + records FTDI device's configuration setting as well as user
> + data.
> +
> config USB_SERIAL_VISOR
> tristate "USB Handspring Visor / Palm m50x / Sony Clie Driver"
> help
> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
> index 7ea221d..9e242e8 100644
> --- a/drivers/usb/serial/ftdi_sio.c
> +++ b/drivers/usb/serial/ftdi_sio.c
> @@ -40,6 +40,7 @@
> #include
> #include
> #include
> +#include
> #include "ftdi_sio.h"
> #include "ftdi_sio_ids.h"
>
> @@ -73,6 +74,8 @@ struct ftdi_private {
> unsigned int latency; /* latency setting in use */
> unsigned short max_packet_size;
> struct mutex cfg_lock; /* Avoid mess by parallel calls of config ioctl()
> and change_speed() */
> +
> + struct nvmem_device *eeprom;
> };
>
> /* struct ftdi_sio_quirk is used by devices requiring special attention.
> */
> @@ -1529,6 +1532,104 @@ static int get_lsr_info(struct usb_serial_port
> *port,
> return 0;
> }
>
> +#ifdef CONFIG_USB_SERIAL_FTDI_SIO_NVMEM
> +
> +static int write_eeprom(void *priv, unsigned int off, void *_val, size_t
> bytes)
> +{
> + struct usb_serial_port *port = priv;
> + struct usb_serial *serial = port->serial;
> + struct usb_device *udev = serial->dev;
> + unsigned char *buf = _val;
> +
> + while (bytes) { /* bytes value is always a multiple of 2 */
We should add check that 'bytes' is always multiple of 2 otherwise in
case its not then there will be memory overrun due to buf[1] access
below.
while (bytes / 2) {
}
> + uint16_t val;
> + int rv;
> +
> + val = buf[0] + (buf[1] << 8);
> +
> + rv = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
> + FTDI_SIO_WRITE_EEPROM_REQUEST,
> + FTDI_SIO_WRITE_EEPROM_REQUEST_TYPE,
> + val, off / 2, NULL, 0, WDR_TIMEOUT);
> + if (rv < 0)
> + return rv;
> +
> + off += 2;
> + buf += 2;
> + bytes -= 2;
> + }
> +
> + return 0;
> +}
> +
> +static int read_eeprom(void *priv, unsigned int off, void *val, size_t
> bytes)
> +{
> + struct usb_serial_port *port = priv;
> + struct usb_serial *serial = port->serial;
> + struct usb_device *udev = serial->dev;
> + unsigned char *buf = val;
> +
> + while (bytes) { /* bytes value is always a multiple of 2 */
same here
> + int rv;
> +
> + rv = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
> + FTDI_SIO_READ_EEPROM_REQUEST,
> + FTDI_SIO_READ_EEPROM_REQUEST_TYPE,
> + 0, off / 2, buf, 2, WDR_TIMEOUT);
> + if (rv < 0)
> +
Re: [PATCH 5/9] ALSA: usb: caiaq: audio: use irqsave() in USB's complete callback
On Tuesday, June 19, 2018 11:55 PM, Sebastian Andrzej Siewior wrote:
From: John Ogness
The USB completion callback does not disable interrupts while acquiring
the lock. We want to remove the local_irq_disable() invocation from
__usb_hcd_giveback_urb() and therefore it is required for the callback
handler to disable the interrupts while acquiring the lock.
The callback may be invoked either in IRQ or BH context depending on the
USB host controller.
Use the _irqsave() variant of the locking primitives.
Acked-by: Daniel Mack
Cc: Jaroslav Kysela
Cc: Takashi Iwai
Signed-off-by: John Ogness
Signed-off-by: Sebastian Andrzej Siewior
---
sound/usb/caiaq/audio.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/sound/usb/caiaq/audio.c b/sound/usb/caiaq/audio.c
index f35d29f49ffe..15344d39a6cd 100644
--- a/sound/usb/caiaq/audio.c
+++ b/sound/usb/caiaq/audio.c
@@ -636,6 +636,7 @@ static void read_completed(struct urb *urb)
struct device *dev;
struct urb *out = NULL;
int i, frame, len, send_it = 0, outframe = 0;
+ unsigned long flags;
size_t offset = 0;
if (urb->status || !info)
@@ -672,10 +673,10 @@ static void read_completed(struct urb *urb)
offset += len;
if (len > 0) {
- spin_lock(&cdev->spinlock);
+ spin_lock_irqsave(&cdev->spinlock, flags);
fill_out_urb(cdev, out, &out->iso_frame_desc[outframe]);
read_in_urb(cdev, urb, &urb->iso_frame_desc[frame]);
- spin_unlock(&cdev->spinlock);
+ spin_unlock_irqrestore(&cdev->spinlock, flags);
check_for_elapsed_periods(cdev, cdev->sub_playback);
check_for_elapsed_periods(cdev, cdev->sub_capture);
send_it = 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 6/9] ALSA: usb: caiaq: use usb_fill_int_urb()
On Tuesday, June 19, 2018 11:55 PM, Sebastian Andrzej Siewior wrote:
Using usb_fill_int_urb() helps to find code which initializes an
URB. A grep for members of the struct (like ->complete) reveal lots
of other things, too.
Acked-by: Daniel Mack
Cc: Jaroslav Kysela
Cc: Takashi Iwai
Signed-off-by: Sebastian Andrzej Siewior
---
sound/usb/caiaq/audio.c | 23 +++
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/sound/usb/caiaq/audio.c b/sound/usb/caiaq/audio.c
index 15344d39a6cd..e10d5790099f 100644
--- a/sound/usb/caiaq/audio.c
+++ b/sound/usb/caiaq/audio.c
@@ -736,16 +736,17 @@ static struct urb **alloc_urbs(struct snd_usb_caiaqdev
*cdev, int dir, int *ret)
}
for (i = 0; i < N_URBS; i++) {
+ void *buf;
+
urbs[i] = usb_alloc_urb(FRAMES_PER_URB, GFP_KERNEL);
if (!urbs[i]) {
*ret = -ENOMEM;
return urbs;
}
- urbs[i]->transfer_buffer =
- kmalloc_array(BYTES_PER_FRAME, FRAMES_PER_URB,
- GFP_KERNEL);
- if (!urbs[i]->transfer_buffer) {
+ buf = kmalloc_array(BYTES_PER_FRAME, FRAMES_PER_URB,
+ GFP_KERNEL);
+ if (!buf) {
*ret = -ENOMEM;
return urbs;
}
@@ -758,15 +759,13 @@ static struct urb **alloc_urbs(struct snd_usb_caiaqdev
*cdev, int dir, int *ret)
iso->length = BYTES_PER_FRAME;
}
- urbs[i]->dev = usb_dev;
- urbs[i]->pipe = pipe;
- urbs[i]->transfer_buffer_length = FRAMES_PER_URB
- * BYTES_PER_FRAME;
- urbs[i]->context = &cdev->data_cb_info[i];
- urbs[i]->interval = 1;
+ usb_fill_int_urb(urbs[i], usb_dev, pipe, buf,
+FRAMES_PER_URB * BYTES_PER_FRAME,
+(dir == SNDRV_PCM_STREAM_CAPTURE) ?
+read_completed : write_completed,
+&cdev->data_cb_info[i], 1);
+
urbs[i]->number_of_packets = FRAMES_PER_URB;
- urbs[i]->complete = (dir == SNDRV_PCM_STREAM_CAPTURE) ?
- read_completed : write_completed;
}
*ret = 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 6/9] ALSA: usb: caiaq: use usb_fill_int_urb()
On 2018-06-21 22:19:32 [+0200], Daniel Mack wrote: > On Tuesday, June 19, 2018 11:55 PM, Sebastian Andrzej Siewior wrote: > > Using usb_fill_int_urb() helps to find code which initializes an > > URB. A grep for members of the struct (like ->complete) reveal lots > > of other things, too. > > Acked-by: Daniel Mack nope, please don't. Takashi, please ignore the usb_fill_.* patches. I will be doing another spin with usb_fill_iso_urb() instead. 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
