diff mbox series

[3/4] usb: gadget: u_audio: remove struct uac_req

Message ID 20201221173531.215169-4-jbrunet@baylibre.com
State Superseded
Headers show
Series usb: gadget: audio fixes and clean ups | expand

Commit Message

Jerome Brunet Dec. 21, 2020, 5:35 p.m. UTC 
'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(-)

-- 
2.29.2

Comments

Greg Kroah-Hartman Dec. 28, 2020, 3:01 p.m. UTC | #1 
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
Jack Pham Dec. 29, 2020, 10:30 p.m. UTC | #2 
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
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
Jerome Brunet Jan. 4, 2021, 2:08 p.m. UTC | #3 
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
Greg Kroah-Hartman Jan. 4, 2021, 3:36 p.m. UTC | #4 
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 mbox series

Patch

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);