linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] usb: gadget: audio fixes and clean ups
@ 2020-12-21 17:35 Jerome Brunet
  2020-12-21 17:35 ` [PATCH 1/4] usb: gadget: f_uac2: reset wMaxPacketSize Jerome Brunet
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Jerome Brunet @ 2020-12-21 17:35 UTC (permalink / raw)
  To: Felipe Balbi, Greg Kroah-Hartman
  Cc: Jerome Brunet, Ruslan Bilovol, linux-usb, linux-kernel

This patchset is a collection of fixes and clean ups found while
working on the uac2 gadget. Details are provided in each change.

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/

Jerome Brunet (4):
  usb: gadget: f_uac2: reset wMaxPacketSize
  usb: gadget: u_audio: factorize ssize to alsa fmt conversion
  usb: gadget: u_audio: remove struct uac_req
  usb: gadget: u_audio: clean up locking

 drivers/usb/gadget/function/f_uac2.c  |  69 ++++++++++++---
 drivers/usb/gadget/function/u_audio.c | 122 +++++++++++---------------
 2 files changed, 105 insertions(+), 86 deletions(-)

-- 
2.29.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/4] usb: gadget: f_uac2: reset wMaxPacketSize
  2020-12-21 17:35 [PATCH 0/4] usb: gadget: audio fixes and clean ups Jerome Brunet
@ 2020-12-21 17:35 ` Jerome Brunet
  2020-12-21 17:35 ` [PATCH 2/4] usb: gadget: u_audio: factorize ssize to alsa fmt conversion Jerome Brunet
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Jerome Brunet @ 2020-12-21 17:35 UTC (permalink / raw)
  To: Felipe Balbi, Greg Kroah-Hartman
  Cc: Jerome Brunet, Ruslan Bilovol, linux-usb, linux-kernel

With commit 913e4a90b6f9 ("usb: gadget: f_uac2: finalize wMaxPacketSize according to bandwidth")
wMaxPacketSize is computed dynamically but the value is never reset.

Because of this, the actual maximum packet size can only decrease each time
the audio gadget is instantiated.

Reset the endpoint maximum packet size and mark wMaxPacketSize as dynamic
to solve the problem.

Fixes: 913e4a90b6f9 ("usb: gadget: f_uac2: finalize wMaxPacketSize according to bandwidth")
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/usb/gadget/function/f_uac2.c | 69 ++++++++++++++++++++++------
 1 file changed, 55 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/gadget/function/f_uac2.c b/drivers/usb/gadget/function/f_uac2.c
index 3633df6d7610..5d960b6603b6 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -271,7 +271,7 @@ static struct usb_endpoint_descriptor fs_epout_desc = {
 
 	.bEndpointAddress = USB_DIR_OUT,
 	.bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
-	.wMaxPacketSize = cpu_to_le16(1023),
+	/* .wMaxPacketSize = DYNAMIC */
 	.bInterval = 1,
 };
 
@@ -280,7 +280,7 @@ static struct usb_endpoint_descriptor hs_epout_desc = {
 	.bDescriptorType = USB_DT_ENDPOINT,
 
 	.bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
-	.wMaxPacketSize = cpu_to_le16(1024),
+	/* .wMaxPacketSize = DYNAMIC */
 	.bInterval = 4,
 };
 
@@ -348,7 +348,7 @@ static struct usb_endpoint_descriptor fs_epin_desc = {
 
 	.bEndpointAddress = USB_DIR_IN,
 	.bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
-	.wMaxPacketSize = cpu_to_le16(1023),
+	/* .wMaxPacketSize = DYNAMIC */
 	.bInterval = 1,
 };
 
@@ -357,7 +357,7 @@ static struct usb_endpoint_descriptor hs_epin_desc = {
 	.bDescriptorType = USB_DT_ENDPOINT,
 
 	.bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
-	.wMaxPacketSize = cpu_to_le16(1024),
+	/* .wMaxPacketSize = DYNAMIC */
 	.bInterval = 4,
 };
 
@@ -444,12 +444,28 @@ struct cntrl_range_lay3 {
 	__le32	dRES;
 } __packed;
 
-static void set_ep_max_packet_size(const struct f_uac2_opts *uac2_opts,
+static int set_ep_max_packet_size(const struct f_uac2_opts *uac2_opts,
 	struct usb_endpoint_descriptor *ep_desc,
-	unsigned int factor, bool is_playback)
+	enum usb_device_speed speed, bool is_playback)
 {
 	int chmask, srate, ssize;
-	u16 max_packet_size;
+	u16 max_size_bw, max_size_ep;
+	unsigned int factor;
+
+	switch (speed) {
+	case USB_SPEED_FULL:
+		max_size_ep = 1023;
+		factor = 1000;
+		break;
+
+	case USB_SPEED_HIGH:
+		max_size_ep = 1024;
+		factor = 8000;
+		break;
+
+	default:
+		return -EINVAL;
+	}
 
 	if (is_playback) {
 		chmask = uac2_opts->p_chmask;
@@ -461,10 +477,12 @@ static void set_ep_max_packet_size(const struct f_uac2_opts *uac2_opts,
 		ssize = uac2_opts->c_ssize;
 	}
 
-	max_packet_size = num_channels(chmask) * ssize *
+	max_size_bw = num_channels(chmask) * ssize *
 		DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval - 1)));
-	ep_desc->wMaxPacketSize = cpu_to_le16(min_t(u16, max_packet_size,
-				le16_to_cpu(ep_desc->wMaxPacketSize)));
+	ep_desc->wMaxPacketSize = cpu_to_le16(min_t(u16, max_size_bw,
+						    max_size_ep));
+
+	return 0;
 }
 
 /* Use macro to overcome line length limitation */
@@ -670,10 +688,33 @@ afunc_bind(struct usb_configuration *cfg, struct usb_function *fn)
 	}
 
 	/* Calculate wMaxPacketSize according to audio bandwidth */
-	set_ep_max_packet_size(uac2_opts, &fs_epin_desc, 1000, true);
-	set_ep_max_packet_size(uac2_opts, &fs_epout_desc, 1000, false);
-	set_ep_max_packet_size(uac2_opts, &hs_epin_desc, 8000, true);
-	set_ep_max_packet_size(uac2_opts, &hs_epout_desc, 8000, false);
+	ret = set_ep_max_packet_size(uac2_opts, &fs_epin_desc, USB_SPEED_FULL,
+				     true);
+	if (ret < 0) {
+		dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
+		return ret;
+	}
+
+	ret = set_ep_max_packet_size(uac2_opts, &fs_epout_desc, USB_SPEED_FULL,
+				     false);
+	if (ret < 0) {
+		dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
+		return ret;
+	}
+
+	ret = set_ep_max_packet_size(uac2_opts, &hs_epin_desc, USB_SPEED_HIGH,
+				     true);
+	if (ret < 0) {
+		dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
+		return ret;
+	}
+
+	ret = set_ep_max_packet_size(uac2_opts, &hs_epout_desc, USB_SPEED_HIGH,
+				     false);
+	if (ret < 0) {
+		dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
+		return ret;
+	}
 
 	if (EPOUT_EN(uac2_opts)) {
 		agdev->out_ep = usb_ep_autoconfig(gadget, &fs_epout_desc);
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/4] usb: gadget: u_audio: factorize ssize to alsa fmt conversion
  2020-12-21 17:35 [PATCH 0/4] usb: gadget: audio fixes and clean ups Jerome Brunet
  2020-12-21 17:35 ` [PATCH 1/4] usb: gadget: f_uac2: reset wMaxPacketSize Jerome Brunet
@ 2020-12-21 17:35 ` Jerome Brunet
  2020-12-21 17:35 ` [PATCH 3/4] usb: gadget: u_audio: remove struct uac_req Jerome Brunet
  2020-12-21 17:35 ` [PATCH 4/4] usb: gadget: u_audio: clean up locking Jerome Brunet
  3 siblings, 0 replies; 9+ messages in thread
From: Jerome Brunet @ 2020-12-21 17:35 UTC (permalink / raw)
  To: Felipe Balbi, Greg Kroah-Hartman
  Cc: Jerome Brunet, Ruslan Bilovol, linux-usb, linux-kernel

Factorize format related code common to the capture and playback path.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/usb/gadget/function/u_audio.c | 43 +++++++++++++--------------
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
index 71dd9f16c246..2f69bd572ed7 100644
--- a/drivers/usb/gadget/function/u_audio.c
+++ b/drivers/usb/gadget/function/u_audio.c
@@ -244,6 +244,25 @@ static snd_pcm_uframes_t uac_pcm_pointer(struct snd_pcm_substream *substream)
 	return bytes_to_frames(substream->runtime, prm->hw_ptr);
 }
 
+static unsigned long uac_ssize_to_fmt(int ssize)
+{
+	unsigned long ret;
+
+	switch (ssize) {
+	case 3:
+		ret = SNDRV_PCM_FMTBIT_S24_3LE;
+		break;
+	case 4:
+		ret = SNDRV_PCM_FMTBIT_S32_LE;
+		break;
+	default:
+		ret = SNDRV_PCM_FMTBIT_S16_LE;
+		break;
+	}
+
+	return ret;
+}
+
 static int uac_pcm_open(struct snd_pcm_substream *substream)
 {
 	struct snd_uac_chip *uac = snd_pcm_substream_chip(substream);
@@ -269,34 +288,14 @@ static int uac_pcm_open(struct snd_pcm_substream *substream)
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
 		spin_lock_init(&uac->p_prm.lock);
 		runtime->hw.rate_min = p_srate;
-		switch (p_ssize) {
-		case 3:
-			runtime->hw.formats = SNDRV_PCM_FMTBIT_S24_3LE;
-			break;
-		case 4:
-			runtime->hw.formats = SNDRV_PCM_FMTBIT_S32_LE;
-			break;
-		default:
-			runtime->hw.formats = SNDRV_PCM_FMTBIT_S16_LE;
-			break;
-		}
+		runtime->hw.formats = uac_ssize_to_fmt(p_ssize);
 		runtime->hw.channels_min = num_channels(p_chmask);
 		runtime->hw.period_bytes_min = 2 * uac->p_prm.max_psize
 						/ runtime->hw.periods_min;
 	} else {
 		spin_lock_init(&uac->c_prm.lock);
 		runtime->hw.rate_min = c_srate;
-		switch (c_ssize) {
-		case 3:
-			runtime->hw.formats = SNDRV_PCM_FMTBIT_S24_3LE;
-			break;
-		case 4:
-			runtime->hw.formats = SNDRV_PCM_FMTBIT_S32_LE;
-			break;
-		default:
-			runtime->hw.formats = SNDRV_PCM_FMTBIT_S16_LE;
-			break;
-		}
+		runtime->hw.formats = uac_ssize_to_fmt(c_ssize);
 		runtime->hw.channels_min = num_channels(c_chmask);
 		runtime->hw.period_bytes_min = 2 * uac->c_prm.max_psize
 						/ runtime->hw.periods_min;
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/4] usb: gadget: u_audio: remove struct uac_req
  2020-12-21 17:35 [PATCH 0/4] usb: gadget: audio fixes and clean ups Jerome Brunet
  2020-12-21 17:35 ` [PATCH 1/4] usb: gadget: f_uac2: reset wMaxPacketSize Jerome Brunet
  2020-12-21 17:35 ` [PATCH 2/4] usb: gadget: u_audio: factorize ssize to alsa fmt conversion Jerome Brunet
@ 2020-12-21 17:35 ` Jerome Brunet
  2020-12-28 15:01   ` Greg Kroah-Hartman
  2020-12-21 17:35 ` [PATCH 4/4] usb: gadget: u_audio: clean up locking Jerome Brunet
  3 siblings, 1 reply; 9+ messages in thread
From: Jerome Brunet @ 2020-12-21 17:35 UTC (permalink / raw)
  To: Felipe Balbi, Greg Kroah-Hartman
  Cc: Jerome Brunet, Ruslan Bilovol, linux-usb, linux-kernel

'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(-)

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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/4] usb: gadget: u_audio: clean up locking
  2020-12-21 17:35 [PATCH 0/4] usb: gadget: audio fixes and clean ups Jerome Brunet
                   ` (2 preceding siblings ...)
  2020-12-21 17:35 ` [PATCH 3/4] usb: gadget: u_audio: remove struct uac_req Jerome Brunet
@ 2020-12-21 17:35 ` Jerome Brunet
  3 siblings, 0 replies; 9+ messages in thread
From: Jerome Brunet @ 2020-12-21 17:35 UTC (permalink / raw)
  To: Felipe Balbi, Greg Kroah-Hartman
  Cc: Jerome Brunet, Ruslan Bilovol, linux-usb, linux-kernel

snd_pcm_stream_lock() is held when the ALSA .trigger() callback is called.
The lock of 'struct uac_rtd_params' is not necessary since all its locking
operation are done under the snd_pcm_stream_lock() too.

Also, usb_request .complete() is called with irqs disabled, so saving and
restoring the irqs is not necessary.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/usb/gadget/function/u_audio.c | 23 ++++-------------------
 1 file changed, 4 insertions(+), 19 deletions(-)

diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
index 3eba31b8ebcb..d94f95edca40 100644
--- a/drivers/usb/gadget/function/u_audio.c
+++ b/drivers/usb/gadget/function/u_audio.c
@@ -36,9 +36,8 @@ struct uac_rtd_params {
 	void *rbuf;
 
 	unsigned int max_psize;	/* MaxPacketSize of endpoint */
-	struct usb_request **reqs;
 
-	spinlock_t lock;
+	struct usb_request **reqs;
 };
 
 struct snd_uac_chip {
@@ -74,7 +73,6 @@ static const struct snd_pcm_hardware uac_pcm_hardware = {
 static void u_audio_iso_complete(struct usb_ep *ep, struct usb_request *req)
 {
 	unsigned int pending;
-	unsigned long flags, flags2;
 	unsigned int hw_ptr;
 	int status = req->status;
 	struct snd_pcm_substream *substream;
@@ -105,16 +103,14 @@ static void u_audio_iso_complete(struct usb_ep *ep, struct usb_request *req)
 	if (!substream)
 		goto exit;
 
-	snd_pcm_stream_lock_irqsave(substream, flags2);
+	snd_pcm_stream_lock(substream);
 
 	runtime = substream->runtime;
 	if (!runtime || !snd_pcm_running(substream)) {
-		snd_pcm_stream_unlock_irqrestore(substream, flags2);
+		snd_pcm_stream_unlock(substream);
 		goto exit;
 	}
 
-	spin_lock_irqsave(&prm->lock, flags);
-
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
 		/*
 		 * For each IN packet, take the quotient of the current data
@@ -141,8 +137,6 @@ static void u_audio_iso_complete(struct usb_ep *ep, struct usb_request *req)
 
 	hw_ptr = prm->hw_ptr;
 
-	spin_unlock_irqrestore(&prm->lock, flags);
-
 	/* Pack USB load in ALSA ring buffer */
 	pending = runtime->dma_bytes - hw_ptr;
 
@@ -166,12 +160,10 @@ 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);
-	snd_pcm_stream_unlock_irqrestore(substream, flags2);
+	snd_pcm_stream_unlock(substream);
 
 	if ((hw_ptr % snd_pcm_lib_period_bytes(substream)) < req->actual)
 		snd_pcm_period_elapsed(substream);
@@ -187,7 +179,6 @@ static int uac_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
 	struct uac_rtd_params *prm;
 	struct g_audio *audio_dev;
 	struct uac_params *params;
-	unsigned long flags;
 	int err = 0;
 
 	audio_dev = uac->audio_dev;
@@ -198,8 +189,6 @@ static int uac_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
 	else
 		prm = &uac->c_prm;
 
-	spin_lock_irqsave(&prm->lock, flags);
-
 	/* Reset */
 	prm->hw_ptr = 0;
 
@@ -216,8 +205,6 @@ static int uac_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
 		err = -EINVAL;
 	}
 
-	spin_unlock_irqrestore(&prm->lock, flags);
-
 	/* Clear buffer after Play stops */
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK && !prm->ss)
 		memset(prm->rbuf, 0, prm->max_psize * params->req_number);
@@ -280,14 +267,12 @@ static int uac_pcm_open(struct snd_pcm_substream *substream)
 	runtime->hw = uac_pcm_hardware;
 
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-		spin_lock_init(&uac->p_prm.lock);
 		runtime->hw.rate_min = p_srate;
 		runtime->hw.formats = uac_ssize_to_fmt(p_ssize);
 		runtime->hw.channels_min = num_channels(p_chmask);
 		runtime->hw.period_bytes_min = 2 * uac->p_prm.max_psize
 						/ runtime->hw.periods_min;
 	} else {
-		spin_lock_init(&uac->c_prm.lock);
 		runtime->hw.rate_min = c_srate;
 		runtime->hw.formats = uac_ssize_to_fmt(c_ssize);
 		runtime->hw.channels_min = num_channels(c_chmask);
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/4] usb: gadget: u_audio: remove struct uac_req
  2020-12-21 17:35 ` [PATCH 3/4] usb: gadget: u_audio: remove struct uac_req Jerome Brunet
@ 2020-12-28 15:01   ` Greg Kroah-Hartman
  2020-12-29 22:30     ` Jack Pham
  0 siblings, 1 reply; 9+ messages in thread
From: Greg Kroah-Hartman @ 2020-12-28 15:01 UTC (permalink / raw)
  To: Jerome Brunet; +Cc: Felipe Balbi, Ruslan Bilovol, linux-usb, linux-kernel

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/4] usb: gadget: u_audio: remove struct uac_req
  2020-12-28 15:01   ` Greg Kroah-Hartman
@ 2020-12-29 22:30     ` Jack Pham
  2021-01-04 14:08       ` Jerome Brunet
  0 siblings, 1 reply; 9+ messages in thread
From: Jack Pham @ 2020-12-29 22:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jerome Brunet
  Cc: Felipe Balbi, Ruslan Bilovol, linux-usb, linux-kernel

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/4] usb: gadget: u_audio: remove struct uac_req
  2020-12-29 22:30     ` Jack Pham
@ 2021-01-04 14:08       ` Jerome Brunet
  2021-01-04 15:36         ` Greg Kroah-Hartman
  0 siblings, 1 reply; 9+ messages in thread
From: Jerome Brunet @ 2021-01-04 14:08 UTC (permalink / raw)
  To: Jack Pham, Greg Kroah-Hartman
  Cc: Felipe Balbi, Ruslan Bilovol, linux-usb, linux-kernel


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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/4] usb: gadget: u_audio: remove struct uac_req
  2021-01-04 14:08       ` Jerome Brunet
@ 2021-01-04 15:36         ` Greg Kroah-Hartman
  0 siblings, 0 replies; 9+ messages in thread
From: Greg Kroah-Hartman @ 2021-01-04 15:36 UTC (permalink / raw)
  To: Jerome Brunet
  Cc: Jack Pham, Felipe Balbi, Ruslan Bilovol, linux-usb, linux-kernel

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.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-01-04 15:36 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-21 17:35 [PATCH 0/4] usb: gadget: audio fixes and clean ups Jerome Brunet
2020-12-21 17:35 ` [PATCH 1/4] usb: gadget: f_uac2: reset wMaxPacketSize Jerome Brunet
2020-12-21 17:35 ` [PATCH 2/4] usb: gadget: u_audio: factorize ssize to alsa fmt conversion Jerome Brunet
2020-12-21 17:35 ` [PATCH 3/4] usb: gadget: u_audio: remove struct uac_req Jerome Brunet
2020-12-28 15:01   ` Greg Kroah-Hartman
2020-12-29 22:30     ` Jack Pham
2021-01-04 14:08       ` Jerome Brunet
2021-01-04 15:36         ` Greg Kroah-Hartman
2020-12-21 17:35 ` [PATCH 4/4] usb: gadget: u_audio: clean up locking Jerome Brunet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).