| Message ID | 20201221173531.215169-4-jbrunet@baylibre.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: gadget: audio fixes and clean ups | expand |
On Mon, Dec 21, 2020 at 06:35:30PM +0100, Jerome Brunet wrote: > 'struct uac_req' purpose is to link 'struct usb_request' to the > corresponding 'struct uac_rtd_params'. However member req is never > used. Using the context of the usb request, we can keep track of the > corresponding 'struct uac_rtd_params' just as well, without allocating > extra memory. > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > --- > drivers/usb/gadget/function/u_audio.c | 58 ++++++++++++--------------- > 1 file changed, 26 insertions(+), 32 deletions(-) This patch doesn't apply, so I can't apply patches 3 or 4 of this series :( Can you rebase against my usb-testing branch and resend? thanks, greg k-h
Hi Greg and Jerome, On Mon, Dec 28, 2020 at 04:01:46PM +0100, Greg Kroah-Hartman wrote: > On Mon, Dec 21, 2020 at 06:35:30PM +0100, Jerome Brunet wrote: > > 'struct uac_req' purpose is to link 'struct usb_request' to the > > corresponding 'struct uac_rtd_params'. However member req is never > > used. Using the context of the usb request, we can keep track of the > > corresponding 'struct uac_rtd_params' just as well, without allocating > > extra memory. > > > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > > --- > > drivers/usb/gadget/function/u_audio.c | 58 ++++++++++++--------------- > > 1 file changed, 26 insertions(+), 32 deletions(-) > > This patch doesn't apply, so I can't apply patches 3 or 4 of this series > :( > > Can you rebase against my usb-testing branch and resend? From the cover letter: On Mon, Dec 21, 2020 at 06:35:27PM +0100, Jerome Brunet wrote: > The series depends on this fix [0] by Jack Pham to apply cleanly > > [0]: https://lore.kernel.org/linux-usb/20201029175949.6052-1-jackp@codeaurora.org/ My patch hadn't been picked up by Felipe, so it's not in your tree either, Greg. Should I just resend it to you first? Or shall I invite Jerome to just include it in v2 of this series? Thanks, Jack
On Tue 29 Dec 2020 at 23:30, Jack Pham <jackp@codeaurora.org> wrote: > Hi Greg and Jerome, > > On Mon, Dec 28, 2020 at 04:01:46PM +0100, Greg Kroah-Hartman wrote: >> On Mon, Dec 21, 2020 at 06:35:30PM +0100, Jerome Brunet wrote: >> > 'struct uac_req' purpose is to link 'struct usb_request' to the >> > corresponding 'struct uac_rtd_params'. However member req is never >> > used. Using the context of the usb request, we can keep track of the >> > corresponding 'struct uac_rtd_params' just as well, without allocating >> > extra memory. >> > >> > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> >> > --- >> > drivers/usb/gadget/function/u_audio.c | 58 ++++++++++++--------------- >> > 1 file changed, 26 insertions(+), 32 deletions(-) >> >> This patch doesn't apply, so I can't apply patches 3 or 4 of this series >> :( >> >> Can you rebase against my usb-testing branch and resend? > > From the cover letter: > > On Mon, Dec 21, 2020 at 06:35:27PM +0100, Jerome Brunet wrote: >> The series depends on this fix [0] by Jack Pham to apply cleanly >> >> [0]: https://lore.kernel.org/linux-usb/20201029175949.6052-1-jackp@codeaurora.org/ > > My patch hadn't been picked up by Felipe, so it's not in your tree > either, Greg. Should I just resend it to you first? Or shall I invite > Jerome to just include it in v2 of this series? Indeed. I rebased on usb-testing and the series applies cleanly with Jack's changes, as decribed in the cover-letter. If it is easier, I'm happy to include Jack's change in the v2, along with the fixed PATCH 2 fixed. Greg, would it be OK with you ? > > Thanks, > Jack
On Mon, Jan 04, 2021 at 03:08:13PM +0100, Jerome Brunet wrote: > > On Tue 29 Dec 2020 at 23:30, Jack Pham <jackp@codeaurora.org> wrote: > > > Hi Greg and Jerome, > > > > On Mon, Dec 28, 2020 at 04:01:46PM +0100, Greg Kroah-Hartman wrote: > >> On Mon, Dec 21, 2020 at 06:35:30PM +0100, Jerome Brunet wrote: > >> > 'struct uac_req' purpose is to link 'struct usb_request' to the > >> > corresponding 'struct uac_rtd_params'. However member req is never > >> > used. Using the context of the usb request, we can keep track of the > >> > corresponding 'struct uac_rtd_params' just as well, without allocating > >> > extra memory. > >> > > >> > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > >> > --- > >> > drivers/usb/gadget/function/u_audio.c | 58 ++++++++++++--------------- > >> > 1 file changed, 26 insertions(+), 32 deletions(-) > >> > >> This patch doesn't apply, so I can't apply patches 3 or 4 of this series > >> :( > >> > >> Can you rebase against my usb-testing branch and resend? > > > > From the cover letter: > > > > On Mon, Dec 21, 2020 at 06:35:27PM +0100, Jerome Brunet wrote: > >> The series depends on this fix [0] by Jack Pham to apply cleanly > >> > >> [0]: https://lore.kernel.org/linux-usb/20201029175949.6052-1-jackp@codeaurora.org/ > > > > My patch hadn't been picked up by Felipe, so it's not in your tree > > either, Greg. Should I just resend it to you first? Or shall I invite > > Jerome to just include it in v2 of this series? > > Indeed. I rebased on usb-testing and the series applies cleanly with > Jack's changes, as decribed in the cover-letter. > > If it is easier, I'm happy to include Jack's change in the v2, along > with the fixed PATCH 2 fixed. > > Greg, would it be OK with you ? That's fine with me.
diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c index 2f69bd572ed7..3eba31b8ebcb 100644 --- a/drivers/usb/gadget/function/u_audio.c +++ b/drivers/usb/gadget/function/u_audio.c @@ -23,11 +23,6 @@ #define PRD_SIZE_MAX PAGE_SIZE #define MIN_PERIODS 4 -struct uac_req { - struct uac_rtd_params *pp; /* parent param */ - struct usb_request *req; -}; - /* Runtime data params for one stream */ struct uac_rtd_params { struct snd_uac_chip *uac; /* parent chip */ @@ -41,7 +36,7 @@ struct uac_rtd_params { void *rbuf; unsigned int max_psize; /* MaxPacketSize of endpoint */ - struct uac_req *ureq; + struct usb_request **reqs; spinlock_t lock; }; @@ -82,10 +77,9 @@ static void u_audio_iso_complete(struct usb_ep *ep, struct usb_request *req) unsigned long flags, flags2; unsigned int hw_ptr; 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 uac_rtd_params *prm = req->context; struct snd_uac_chip *uac = prm->uac; /* i/f shutting down */ @@ -339,11 +333,11 @@ static inline void free_ep(struct uac_rtd_params *prm, struct usb_ep *ep) params = &audio_dev->params; for (i = 0; i < params->req_number; i++) { - if (prm->ureq[i].req) { - if (usb_ep_dequeue(ep, prm->ureq[i].req)) - usb_ep_free_request(ep, prm->ureq[i].req); + if (prm->reqs[i]) { + if (usb_ep_dequeue(ep, prm->reqs[i])) + usb_ep_free_request(ep, prm->reqs[i]); /* else will be freed in u_audio_iso_complete() */ - prm->ureq[i].req = NULL; + prm->reqs[i] = NULL; } } @@ -372,22 +366,21 @@ int u_audio_start_capture(struct g_audio *audio_dev) usb_ep_enable(ep); for (i = 0; i < params->req_number; i++) { - if (!prm->ureq[i].req) { + if (!prm->reqs[i]) { req = usb_ep_alloc_request(ep, GFP_ATOMIC); if (req == NULL) return -ENOMEM; - prm->ureq[i].req = req; - prm->ureq[i].pp = prm; + prm->reqs[i] = req; req->zero = 0; - req->context = &prm->ureq[i]; + req->context = prm; req->length = req_len; req->complete = u_audio_iso_complete; req->buf = prm->rbuf + i * ep->maxpacket; } - if (usb_ep_queue(ep, prm->ureq[i].req, GFP_ATOMIC)) + if (usb_ep_queue(ep, prm->reqs[i], GFP_ATOMIC)) dev_err(dev, "%s:%d Error!\n", __func__, __LINE__); } @@ -450,22 +443,21 @@ int u_audio_start_playback(struct g_audio *audio_dev) usb_ep_enable(ep); for (i = 0; i < params->req_number; i++) { - if (!prm->ureq[i].req) { + if (!prm->reqs[i]) { req = usb_ep_alloc_request(ep, GFP_ATOMIC); if (req == NULL) return -ENOMEM; - prm->ureq[i].req = req; - prm->ureq[i].pp = prm; + prm->reqs[i] = req; req->zero = 0; - req->context = &prm->ureq[i]; + req->context = prm; req->length = req_len; req->complete = u_audio_iso_complete; req->buf = prm->rbuf + i * ep->maxpacket; } - if (usb_ep_queue(ep, prm->ureq[i].req, GFP_ATOMIC)) + if (usb_ep_queue(ep, prm->reqs[i], GFP_ATOMIC)) dev_err(dev, "%s:%d Error!\n", __func__, __LINE__); } @@ -510,9 +502,10 @@ int g_audio_setup(struct g_audio *g_audio, const char *pcm_name, uac->c_prm.uac = uac; prm->max_psize = g_audio->out_ep_maxpsize; - prm->ureq = kcalloc(params->req_number, sizeof(struct uac_req), - GFP_KERNEL); - if (!prm->ureq) { + prm->reqs = kcalloc(params->req_number, + sizeof(struct usb_request *), + GFP_KERNEL); + if (!prm->reqs) { err = -ENOMEM; goto fail; } @@ -532,9 +525,10 @@ int g_audio_setup(struct g_audio *g_audio, const char *pcm_name, uac->p_prm.uac = uac; prm->max_psize = g_audio->in_ep_maxpsize; - prm->ureq = kcalloc(params->req_number, sizeof(struct uac_req), - GFP_KERNEL); - if (!prm->ureq) { + prm->reqs = kcalloc(params->req_number, + sizeof(struct usb_request *), + GFP_KERNEL); + if (!prm->reqs) { err = -ENOMEM; goto fail; } @@ -587,8 +581,8 @@ int g_audio_setup(struct g_audio *g_audio, const char *pcm_name, snd_fail: snd_card_free(card); fail: - kfree(uac->p_prm.ureq); - kfree(uac->c_prm.ureq); + kfree(uac->p_prm.reqs); + kfree(uac->c_prm.reqs); kfree(uac->p_prm.rbuf); kfree(uac->c_prm.rbuf); kfree(uac); @@ -610,8 +604,8 @@ void g_audio_cleanup(struct g_audio *g_audio) if (card) snd_card_free(card); - kfree(uac->p_prm.ureq); - kfree(uac->c_prm.ureq); + kfree(uac->p_prm.reqs); + kfree(uac->c_prm.reqs); kfree(uac->p_prm.rbuf); kfree(uac->c_prm.rbuf); kfree(uac);
'struct uac_req' purpose is to link 'struct usb_request' to the corresponding 'struct uac_rtd_params'. However member req is never used. Using the context of the usb request, we can keep track of the corresponding 'struct uac_rtd_params' just as well, without allocating extra memory. Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> --- drivers/usb/gadget/function/u_audio.c | 58 ++++++++++++--------------- 1 file changed, 26 insertions(+), 32 deletions(-)