Hi Greg,
On 12/24/2022 12:54 AM, Greg KH wrote:
On Fri, Dec 23, 2022 at 03:31:52PM -0800, Wesley Cheng wrote:
For USB HCDs that can support multiple USB interrupters, expose functions
that class drivers can utilize for setting up secondary interrupters.
Class drivers can pass this information to its respective clients, i.e.
a dedicated DSP.
Where is the locking here that seems to be required when a hcd is
removed from the system and you have data in flight? What am I missing
here in the design of this?
The XHCI driver is the one that maintains the list of interrupters that
are available, so the locking was placed in the XHCI driver versus
adding it in the core hcd layer.
And yes, HCDs get removed all the time, and will happen more and more in
the future with the design of more systems moving to Thunderbolt/PCIe
designs due to the simplicity of it.
As part of the HCD removal, it has to first ensure that class driver
interfaces, and the connected udevs are removed first. qc_audio_offload
will first handle the transfer cleanup and stopping of the audio stream
before returning from the disconnect callback. (this includes ensuring
that the interrupter is released)
This concept is how all usb class drivers are currently implemented.
When the HCD remove occurs, the class drivers are the ones responsible
for ensuring that all URBs are stopped, and removed before it returns
from its respective disconnect callback.
+/**
+ * usb_set_interruper - Reserve an interrupter
Where is an "interrupter" defined? I don't know what this term means
sorry, is this in the USB specification somewhere?
Interrupter is defined in the XHCI spec, refer to "section 4.17
Interrupters"
+ * @udev: usb device which requested the interrupter
+ * @intr_num: interrupter number to reserve
+ * @dma: iova address to event ring
+ *
+ * Request for a specific interrupter to be reserved for a USB class driver.
+ * This will return the base address to the event ring that was allocated to
+ * the specific interrupter.
+ **/
+phys_addr_t usb_set_interruper(struct usb_device *udev, int intr_num,
+ dma_addr_t *dma)
+{
+ struct usb_hcd *hcd;
+ phys_addr_t pa = 0;
+
+ hcd = bus_to_hcd(udev->bus);
+ if (hcd->driver->update_interrupter)
+ pa = hcd->driver->update_interrupter(hcd, intr_num, dma);
+
+ return pa;
Wait, you return a physical address? What are you going to do with
that? And what guarantees that the address is valid after you return it
(again, remember memory and devices can be removed at any point in time.
The interrupter is basically another event ring which is the buffer
allocated for the controller to copy events into. Since the audio dsp
now takes over handling of the endpoint events, then it needs to know
where the buffer resides. Will fix the interruper typo as well.
The allocation and freeing of this event ring follows how XHCI allocates
and frees the main event ring as well. This API just reserves the
interrupter for the class driver, and return the previously allocated
(during XHCI init) memory address.
Thanks
Wesley Cheng
