From: Wesley Cheng <quic_wcheng@quicinc.com> To: Greg KH <gregkh@linuxfoundation.org> Cc: devicetree@vger.kernel.org, alsa-devel@alsa-project.org, linux-usb@vger.kernel.org, bgoswami@quicinc.com, mathias.nyman@intel.com, Thinh.Nguyen@synopsys.com, andersson@kernel.org, tiwai@suse.com, lgirdwood@gmail.com, robh+dt@kernel.org, broonie@kernel.org, srinivas.kandagatla@linaro.org, agross@kernel.org, krzysztof.kozlowski+dt@linaro.org, linux-arm-msm@vger.kernel.org, quic_plai@quicinc.com, linux-kernel@vger.kernel.org, quic_jackp@quicinc.com Subject: Re: [RFC PATCH 06/14] usb: core: hcd: Introduce USB HCD APIs for interrupter management Date: Tue, 27 Dec 2022 13:13:06 -0800 [thread overview] Message-ID: <1c72011b-b80c-7f6f-66d3-0658cfd600d2@quicinc.com> (raw) In-Reply-To: <Y6a+VJ75lRIUE9yD@kroah.com> 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
WARNING: multiple messages have this Message-ID (diff)
From: Wesley Cheng <quic_wcheng@quicinc.com> To: Greg KH <gregkh@linuxfoundation.org> Cc: <srinivas.kandagatla@linaro.org>, <mathias.nyman@intel.com>, <perex@perex.cz>, <broonie@kernel.org>, <lgirdwood@gmail.com>, <andersson@kernel.org>, <krzysztof.kozlowski+dt@linaro.org>, <Thinh.Nguyen@synopsys.com>, <bgoswami@quicinc.com>, <tiwai@suse.com>, <robh+dt@kernel.org>, <agross@kernel.org>, <linux-kernel@vger.kernel.org>, <linux-arm-msm@vger.kernel.org>, <alsa-devel@alsa-project.org>, <devicetree@vger.kernel.org>, <linux-usb@vger.kernel.org>, <quic_jackp@quicinc.com>, <quic_plai@quicinc.com> Subject: Re: [RFC PATCH 06/14] usb: core: hcd: Introduce USB HCD APIs for interrupter management Date: Tue, 27 Dec 2022 13:13:06 -0800 [thread overview] Message-ID: <1c72011b-b80c-7f6f-66d3-0658cfd600d2@quicinc.com> (raw) In-Reply-To: <Y6a+VJ75lRIUE9yD@kroah.com> 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
next prev parent reply other threads:[~2022-12-27 21:14 UTC|newest] Thread overview: 171+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-12-23 23:31 [RFC PATCH 00/14] Introduce QC USB SND audio offloading support Wesley Cheng 2022-12-23 23:31 ` Wesley Cheng 2022-12-23 23:31 ` [RFC PATCH 01/14] ASoC: Add SOC USB APIs for adding an USB backend Wesley Cheng 2022-12-23 23:31 ` Wesley Cheng 2022-12-24 6:48 ` Greg KH 2022-12-24 6:48 ` Greg KH 2022-12-23 23:31 ` [RFC PATCH 02/14] ASoC: qcom: qdsp6: Introduce USB AFE port to q6dsp Wesley Cheng 2022-12-23 23:31 ` Wesley Cheng 2022-12-24 1:37 ` kernel test robot 2023-01-04 23:33 ` Pierre-Louis Bossart 2023-01-06 1:05 ` Wesley Cheng 2023-01-06 1:05 ` Wesley Cheng 2023-01-06 16:09 ` Pierre-Louis Bossart 2023-01-07 0:51 ` Wesley Cheng 2023-01-07 0:51 ` Wesley Cheng 2023-01-05 18:09 ` Krzysztof Kozlowski 2023-01-05 18:09 ` Krzysztof Kozlowski 2023-01-06 1:32 ` Wesley Cheng 2023-01-06 1:32 ` Wesley Cheng 2022-12-23 23:31 ` [RFC PATCH 03/14] ASoC: qcom: Add USB backend ASoC driver for Q6 Wesley Cheng 2022-12-23 23:31 ` Wesley Cheng 2022-12-24 2:17 ` kernel test robot 2022-12-24 2:17 ` kernel test robot 2022-12-24 9:02 ` Greg KH 2022-12-24 9:02 ` Greg KH 2022-12-27 13:04 ` Mark Brown 2022-12-27 13:04 ` Mark Brown 2022-12-27 13:45 ` Greg KH 2022-12-27 13:45 ` Greg KH 2022-12-27 14:02 ` Takashi Iwai 2022-12-27 14:02 ` Takashi Iwai 2022-12-27 14:11 ` Mark Brown 2022-12-27 14:11 ` Mark Brown 2022-12-27 15:11 ` Mark Brown 2022-12-27 15:11 ` Mark Brown 2022-12-27 21:06 ` Wesley Cheng 2022-12-27 21:06 ` Wesley Cheng 2022-12-27 21:07 ` Wesley Cheng 2022-12-27 21:07 ` Wesley Cheng 2023-01-04 23:41 ` Pierre-Louis Bossart 2023-01-06 1:05 ` Wesley Cheng 2023-01-06 1:05 ` Wesley Cheng 2023-01-06 16:16 ` Pierre-Louis Bossart 2022-12-23 23:31 ` [RFC PATCH 04/14] sound: usb: card: Introduce USB SND vendor op callbacks Wesley Cheng 2022-12-23 23:31 ` Wesley Cheng 2022-12-24 11:03 ` Dmitry Baryshkov 2022-12-24 11:03 ` Dmitry Baryshkov 2022-12-27 21:07 ` Wesley Cheng 2022-12-27 21:07 ` Wesley Cheng 2022-12-27 21:33 ` Dmitry Baryshkov 2022-12-27 21:33 ` Dmitry Baryshkov 2022-12-29 13:49 ` Oliver Neukum 2022-12-29 13:49 ` Oliver Neukum 2022-12-29 14:20 ` Takashi Iwai 2022-12-29 14:20 ` Takashi Iwai 2022-12-30 7:10 ` Wesley Cheng 2022-12-30 7:10 ` Wesley Cheng 2023-01-03 12:20 ` Oliver Neukum 2023-01-03 12:20 ` Oliver Neukum 2023-01-03 12:49 ` Takashi Iwai 2023-01-03 12:49 ` Takashi Iwai 2023-01-03 23:45 ` Wesley Cheng 2023-01-03 23:45 ` Wesley Cheng 2022-12-23 23:31 ` [RFC PATCH 05/14] sound: usb: Export USB SND APIs for modules Wesley Cheng 2022-12-23 23:31 ` Wesley Cheng 2022-12-24 6:48 ` Greg KH 2022-12-24 6:48 ` Greg KH 2022-12-23 23:31 ` [RFC PATCH 06/14] usb: core: hcd: Introduce USB HCD APIs for interrupter management Wesley Cheng 2022-12-23 23:31 ` Wesley Cheng 2022-12-24 8:54 ` Greg KH 2022-12-24 8:54 ` Greg KH 2022-12-27 21:13 ` Wesley Cheng [this message] 2022-12-27 21:13 ` Wesley Cheng 2022-12-24 15:29 ` Alan Stern 2022-12-24 15:29 ` Alan Stern 2022-12-27 21:07 ` Wesley Cheng 2022-12-27 21:07 ` Wesley Cheng 2022-12-28 8:59 ` Oliver Neukum 2022-12-28 8:59 ` Oliver Neukum 2022-12-28 15:16 ` Alan Stern 2022-12-28 15:16 ` Alan Stern 2022-12-28 20:31 ` Wesley Cheng 2022-12-28 20:31 ` Wesley Cheng 2022-12-29 1:41 ` Alan Stern 2022-12-29 1:41 ` Alan Stern 2022-12-23 23:31 ` [RFC PATCH 07/14] usb: host: xhci: Add XHCI secondary interrupter support Wesley Cheng 2022-12-23 23:31 ` Wesley Cheng 2022-12-24 5:59 ` kernel test robot 2022-12-24 8:55 ` Greg KH 2022-12-24 8:55 ` Greg KH 2022-12-28 15:47 ` Mathias Nyman 2022-12-28 15:47 ` Mathias Nyman 2022-12-29 21:14 ` Wesley Cheng 2022-12-29 21:14 ` Wesley Cheng 2023-01-02 16:38 ` Mathias Nyman 2023-01-02 16:38 ` Mathias Nyman 2023-01-09 20:24 ` Wesley Cheng 2023-01-09 20:24 ` Wesley Cheng 2023-01-10 19:47 ` Mathias Nyman 2023-01-10 19:47 ` Mathias Nyman 2023-01-10 20:03 ` Wesley Cheng 2023-01-10 20:03 ` Wesley Cheng 2023-01-11 3:11 ` Wesley Cheng 2023-01-11 3:11 ` Wesley Cheng 2023-01-12 9:24 ` Mathias Nyman 2023-01-12 9:24 ` Mathias Nyman 2023-01-13 0:34 ` Wesley Cheng 2023-01-13 0:34 ` Wesley Cheng 2022-12-23 23:31 ` [RFC PATCH 08/14] usb: dwc3: Add DT parameter to specify maximum number of interrupters Wesley Cheng 2022-12-23 23:31 ` Wesley Cheng 2022-12-24 11:13 ` Dmitry Baryshkov 2022-12-24 11:13 ` Dmitry Baryshkov 2022-12-26 12:28 ` Krzysztof Kozlowski 2022-12-26 12:28 ` Krzysztof Kozlowski 2022-12-27 21:06 ` Wesley Cheng 2022-12-27 21:06 ` Wesley Cheng 2022-12-23 23:31 ` [RFC PATCH 09/14] sound: usb: Introduce QC USB SND offloading support Wesley Cheng 2022-12-23 23:31 ` Wesley Cheng 2022-12-24 2:27 ` kernel test robot 2023-01-02 17:28 ` Takashi Iwai 2023-01-02 17:28 ` Takashi Iwai 2023-01-04 22:38 ` Wesley Cheng 2023-01-04 22:38 ` Wesley Cheng 2023-01-04 23:51 ` Pierre-Louis Bossart 2023-01-06 1:06 ` Wesley Cheng 2023-01-06 1:06 ` Wesley Cheng 2022-12-23 23:31 ` [RFC PATCH 10/14] sound: usb: card: Check for support for requested audio format Wesley Cheng 2022-12-23 23:31 ` Wesley Cheng 2022-12-24 8:59 ` Greg KH 2022-12-24 8:59 ` Greg KH 2022-12-27 21:07 ` Wesley Cheng 2022-12-27 21:07 ` Wesley Cheng 2022-12-27 20:31 ` kernel test robot 2022-12-23 23:31 ` [RFC PATCH 11/14] sound: soc: soc-usb: Add PCM format check API for USB backend Wesley Cheng 2022-12-23 23:31 ` Wesley Cheng 2022-12-24 2:17 ` kernel test robot 2022-12-24 2:17 ` kernel test robot 2022-12-23 23:31 ` [RFC PATCH 12/14] sound: soc: qcom: qusb6: Ensure PCM format is supported by USB audio device Wesley Cheng 2022-12-23 23:31 ` Wesley Cheng 2022-12-24 8:19 ` Sergey Shtylyov 2022-12-24 8:19 ` Sergey Shtylyov 2022-12-24 8:50 ` Wesley Cheng 2022-12-24 8:50 ` Wesley Cheng 2023-01-03 17:44 ` Mark Brown 2023-01-03 17:44 ` Mark Brown 2022-12-23 23:31 ` [RFC PATCH 13/14] ASoC: dt-bindings: Add Q6USB backend bindings Wesley Cheng 2022-12-23 23:31 ` Wesley Cheng 2022-12-26 12:25 ` Krzysztof Kozlowski 2022-12-26 12:25 ` Krzysztof Kozlowski 2022-12-23 23:32 ` [RFC PATCH 14/14] ASoC: dt-bindings: Update example for enabling USB offload on SM8250 Wesley Cheng 2022-12-23 23:32 ` Wesley Cheng 2022-12-26 12:27 ` Krzysztof Kozlowski 2022-12-26 12:27 ` Krzysztof Kozlowski 2023-01-03 17:46 ` Mark Brown 2023-01-03 17:46 ` Mark Brown 2023-01-05 18:09 ` Krzysztof Kozlowski 2023-01-05 18:09 ` Krzysztof Kozlowski 2023-01-04 0:46 ` Rob Herring 2023-01-04 0:46 ` Rob Herring 2022-12-24 6:45 ` [RFC PATCH 00/14] Introduce QC USB SND audio offloading support Greg KH 2022-12-24 6:45 ` Greg KH 2022-12-24 8:49 ` Wesley Cheng 2022-12-24 8:49 ` Wesley Cheng 2022-12-27 14:36 ` Mark Brown 2022-12-27 14:36 ` Mark Brown 2023-01-04 23:19 ` Pierre-Louis Bossart 2023-01-06 1:05 ` Wesley Cheng 2023-01-06 1:05 ` Wesley Cheng 2023-01-06 15:57 ` Pierre-Louis Bossart 2023-01-07 0:46 ` Wesley Cheng 2023-01-07 0:46 ` Wesley Cheng
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1c72011b-b80c-7f6f-66d3-0658cfd600d2@quicinc.com \ --to=quic_wcheng@quicinc.com \ --cc=Thinh.Nguyen@synopsys.com \ --cc=agross@kernel.org \ --cc=alsa-devel@alsa-project.org \ --cc=andersson@kernel.org \ --cc=bgoswami@quicinc.com \ --cc=broonie@kernel.org \ --cc=devicetree@vger.kernel.org \ --cc=gregkh@linuxfoundation.org \ --cc=krzysztof.kozlowski+dt@linaro.org \ --cc=lgirdwood@gmail.com \ --cc=linux-arm-msm@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-usb@vger.kernel.org \ --cc=mathias.nyman@intel.com \ --cc=quic_jackp@quicinc.com \ --cc=quic_plai@quicinc.com \ --cc=robh+dt@kernel.org \ --cc=srinivas.kandagatla@linaro.org \ --cc=tiwai@suse.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.