* [PATCH v1 1/3] Revert "usb: gadget: u_audio: add real feedback implementation"
@ 2021-08-24 20:14 Ferry Toth
2021-08-24 20:14 ` [PATCH v1 2/3] Revert "usb: gadget: f_uac2: add adaptive sync support for capture" Ferry Toth
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Ferry Toth @ 2021-08-24 20:14 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jerome Brunet, Ruslan Bilovol, Oded Gabbay,
Cezary Rojewski, Ferry Toth, Mauro Carvalho Chehab,
Pawel Laszczak, Felipe Balbi, Jack Pham, linux-kernel, linux-usb,
linux-doc
Cc: Jonathan Corbet, Lorenzo Colitti, Wesley Cheng, robh+dt, agross,
bjorn.andersson, frowand.list, devicetree, linux-arm-msm,
heikki.krogerus, Thinh Nguyen, Andy Shevchenko, Pavel Hofman,
Ferry Toth
This reverts commit e89bb4288378b85c82212b60dc98ecda6b3d3a70.
The commit is part of a series with commit
24f779dac8f3efb9629adc0e486914d93dc45517 causing a BUG on dwc3
hardware, at least on Intel Merrifield platform when configured
through configfs:
BUG: kernel NULL pointer dereference, address: 0000000000000008
...
RIP: 0010:dwc3_gadget_del_and_unmap_request+0x19/0xe0
...
Call Trace:
dwc3_remove_requests.constprop.0+0x12f/0x170
__dwc3_gadget_ep_disable+0x7a/0x160
dwc3_gadget_ep_disable+0x3d/0xd0
usb_ep_disable+0x1c/0x70
u_audio_stop_capture+0x79/0x120 [u_audio]
afunc_set_alt+0x73/0x80 [usb_f_uac2]
composite_setup+0x224/0x1b90 [libcomposite]
Pavel's suggestion to add
`echo "adaptive" > functions/uac2.usb0/c_sync` to the configfs script
resolves the issue.
Thinh suggests "the crash is probably because of f_uac2 prematurely
freeing feedback request before its completion. usb_ep_dequeue() is
asynchronous. dwc2() may treat it as a synchronous call so you didn't
get a crash."
Revert as this is a regression and the kernel shouldn't crash depending
on configuration parameters.
Reported-by: Ferry Toth <fntoth@gmail.com>
---
.../ABI/testing/configfs-usb-gadget-uac2 | 1 -
Documentation/usb/gadget-testing.rst | 1 -
drivers/usb/gadget/function/f_uac2.c | 9 +-
drivers/usb/gadget/function/u_audio.c | 124 ++----------------
drivers/usb/gadget/function/u_audio.h | 9 --
drivers/usb/gadget/function/u_uac2.h | 2 -
6 files changed, 10 insertions(+), 136 deletions(-)
diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uac2 b/Documentation/ABI/testing/configfs-usb-gadget-uac2
index 26fb8e9b4e61..e7e59d7fb12f 100644
--- a/Documentation/ABI/testing/configfs-usb-gadget-uac2
+++ b/Documentation/ABI/testing/configfs-usb-gadget-uac2
@@ -9,7 +9,6 @@ Description:
c_srate capture sampling rate
c_ssize capture sample size (bytes)
c_sync capture synchronization type (async/adaptive)
- fb_max maximum extra bandwidth in async mode
p_chmask playback channel mask
p_srate playback sampling rate
p_ssize playback sample size (bytes)
diff --git a/Documentation/usb/gadget-testing.rst b/Documentation/usb/gadget-testing.rst
index 9d6276f82774..f5a12667bd41 100644
--- a/Documentation/usb/gadget-testing.rst
+++ b/Documentation/usb/gadget-testing.rst
@@ -729,7 +729,6 @@ The uac2 function provides these attributes in its function directory:
c_srate capture sampling rate
c_ssize capture sample size (bytes)
c_sync capture synchronization type (async/adaptive)
- fb_max maximum extra bandwidth in async mode
p_chmask playback channel mask
p_srate playback sampling rate
p_ssize playback sample size (bytes)
diff --git a/drivers/usb/gadget/function/f_uac2.c b/drivers/usb/gadget/function/f_uac2.c
index ae29ff2b2b68..321e6c05ba93 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -584,11 +584,8 @@ static int set_ep_max_packet_size(const struct f_uac2_opts *uac2_opts,
ssize = uac2_opts->c_ssize;
}
- if (!is_playback && (uac2_opts->c_sync == USB_ENDPOINT_SYNC_ASYNC))
- srate = srate * (1000 + uac2_opts->fb_max) / 1000;
-
max_size_bw = num_channels(chmask) * ssize *
- DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval - 1)));
+ ((srate / (factor / (1 << (ep_desc->bInterval - 1)))) + 1);
ep_desc->wMaxPacketSize = cpu_to_le16(min_t(u16, max_size_bw,
max_size_ep));
@@ -960,7 +957,6 @@ afunc_bind(struct usb_configuration *cfg, struct usb_function *fn)
agdev->params.c_srate = uac2_opts->c_srate;
agdev->params.c_ssize = uac2_opts->c_ssize;
agdev->params.req_number = uac2_opts->req_number;
- agdev->params.fb_max = uac2_opts->fb_max;
ret = g_audio_setup(agdev, "UAC2 PCM", "UAC2_Gadget");
if (ret)
goto err_free_descs;
@@ -1333,7 +1329,6 @@ UAC2_ATTRIBUTE(c_srate);
UAC2_ATTRIBUTE_SYNC(c_sync);
UAC2_ATTRIBUTE(c_ssize);
UAC2_ATTRIBUTE(req_number);
-UAC2_ATTRIBUTE(fb_max);
static struct configfs_attribute *f_uac2_attrs[] = {
&f_uac2_opts_attr_p_chmask,
@@ -1344,7 +1339,6 @@ static struct configfs_attribute *f_uac2_attrs[] = {
&f_uac2_opts_attr_c_ssize,
&f_uac2_opts_attr_c_sync,
&f_uac2_opts_attr_req_number,
- &f_uac2_opts_attr_fb_max,
NULL,
};
@@ -1384,7 +1378,6 @@ static struct usb_function_instance *afunc_alloc_inst(void)
opts->c_ssize = UAC2_DEF_CSSIZE;
opts->c_sync = UAC2_DEF_CSYNC;
opts->req_number = UAC2_DEF_REQ_NUM;
- opts->fb_max = UAC2_DEF_FB_MAX;
return &opts->func_inst;
}
diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
index 018dd0978995..f637ebec80b0 100644
--- a/drivers/usb/gadget/function/u_audio.c
+++ b/drivers/usb/gadget/function/u_audio.c
@@ -16,7 +16,6 @@
#include <sound/core.h>
#include <sound/pcm.h>
#include <sound/pcm_params.h>
-#include <sound/control.h>
#include "u_audio.h"
@@ -36,12 +35,12 @@ struct uac_rtd_params {
void *rbuf;
- unsigned int pitch; /* Stream pitch ratio to 1000000 */
unsigned int max_psize; /* MaxPacketSize of endpoint */
struct usb_request **reqs;
struct usb_request *req_fback; /* Feedback endpoint request */
+ unsigned int ffback; /* Real frequency reported by feedback endpoint */
bool fb_ep_enabled; /* if the ep is enabled */
};
@@ -76,29 +75,18 @@ static const struct snd_pcm_hardware uac_pcm_hardware = {
};
static void u_audio_set_fback_frequency(enum usb_device_speed speed,
- unsigned long long freq,
- unsigned int pitch,
- void *buf)
+ unsigned int freq, void *buf)
{
u32 ff = 0;
- /*
- * Because the pitch base is 1000000, the final divider here
- * will be 1000 * 1000000 = 1953125 << 9
- *
- * Instead of dealing with big numbers lets fold this 9 left shift
- */
-
if (speed == USB_SPEED_FULL) {
/*
* Full-speed feedback endpoints report frequency
- * in samples/frame
+ * in samples/microframe
* Format is encoded in Q10.10 left-justified in the 24 bits,
* so that it has a Q10.14 format.
- *
- * ff = (freq << 14) / 1000
*/
- freq <<= 5;
+ ff = DIV_ROUND_UP((freq << 14), 1000);
} else {
/*
* High-speed feedback endpoints report frequency
@@ -106,14 +94,9 @@ static void u_audio_set_fback_frequency(enum usb_device_speed speed,
* Format is encoded in Q12.13 fitted into four bytes so that
* the binary point is located between the second and the third
* byte fromat (that is Q16.16)
- *
- * ff = (freq << 16) / 8000
*/
- freq <<= 4;
+ ff = DIV_ROUND_UP((freq << 13), 1000);
}
-
- ff = DIV_ROUND_CLOSEST_ULL((freq * pitch), 1953125);
-
*(__le32 *)buf = cpu_to_le32(ff);
}
@@ -226,8 +209,8 @@ static void u_audio_iso_fback_complete(struct usb_ep *ep,
struct uac_rtd_params *prm = req->context;
struct snd_uac_chip *uac = prm->uac;
struct g_audio *audio_dev = uac->audio_dev;
- struct uac_params *params = &audio_dev->params;
int status = req->status;
+ unsigned long flags;
/* i/f shutting down */
if (!prm->fb_ep_enabled || req->status == -ESHUTDOWN)
@@ -242,8 +225,7 @@ static void u_audio_iso_fback_complete(struct usb_ep *ep,
__func__, status, req->actual, req->length);
u_audio_set_fback_frequency(audio_dev->gadget->speed,
- params->c_srate, prm->pitch,
- req->buf);
+ prm->ffback, req->buf);
if (usb_ep_queue(ep, req, GFP_ATOMIC))
dev_err(uac->card->dev, "%d Error!\n", __LINE__);
@@ -498,10 +480,9 @@ int u_audio_start_capture(struct g_audio *audio_dev)
* Always start with original frequency since its deviation can't
* be meauserd at start of playback
*/
- prm->pitch = 1000000;
+ prm->ffback = params->c_srate;
u_audio_set_fback_frequency(audio_dev->gadget->speed,
- params->c_srate, prm->pitch,
- req_fback->buf);
+ prm->ffback, req_fback->buf);
if (usb_ep_queue(ep_fback, req_fback, GFP_ATOMIC))
dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
@@ -597,82 +578,12 @@ void u_audio_stop_playback(struct g_audio *audio_dev)
}
EXPORT_SYMBOL_GPL(u_audio_stop_playback);
-static int u_audio_pitch_info(struct snd_kcontrol *kcontrol,
- struct snd_ctl_elem_info *uinfo)
-{
- struct uac_rtd_params *prm = snd_kcontrol_chip(kcontrol);
- struct snd_uac_chip *uac = prm->uac;
- struct g_audio *audio_dev = uac->audio_dev;
- struct uac_params *params = &audio_dev->params;
- unsigned int pitch_min, pitch_max;
-
- pitch_min = (1000 - FBACK_SLOW_MAX) * 1000;
- pitch_max = (1000 + params->fb_max) * 1000;
-
- uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
- uinfo->count = 1;
- uinfo->value.integer.min = pitch_min;
- uinfo->value.integer.max = pitch_max;
- uinfo->value.integer.step = 1;
- return 0;
-}
-
-static int u_audio_pitch_get(struct snd_kcontrol *kcontrol,
- struct snd_ctl_elem_value *ucontrol)
-{
- struct uac_rtd_params *prm = snd_kcontrol_chip(kcontrol);
-
- ucontrol->value.integer.value[0] = prm->pitch;
-
- return 0;
-}
-
-static int u_audio_pitch_put(struct snd_kcontrol *kcontrol,
- struct snd_ctl_elem_value *ucontrol)
-{
- struct uac_rtd_params *prm = snd_kcontrol_chip(kcontrol);
- struct snd_uac_chip *uac = prm->uac;
- struct g_audio *audio_dev = uac->audio_dev;
- struct uac_params *params = &audio_dev->params;
- unsigned int val;
- unsigned int pitch_min, pitch_max;
- int change = 0;
-
- pitch_min = (1000 - FBACK_SLOW_MAX) * 1000;
- pitch_max = (1000 + params->fb_max) * 1000;
-
- val = ucontrol->value.integer.value[0];
-
- if (val < pitch_min)
- val = pitch_min;
- if (val > pitch_max)
- val = pitch_max;
-
- if (prm->pitch != val) {
- prm->pitch = val;
- change = 1;
- }
-
- return change;
-}
-
-static const struct snd_kcontrol_new u_audio_controls[] = {
-{
- .iface = SNDRV_CTL_ELEM_IFACE_PCM,
- .name = "Capture Pitch 1000000",
- .info = u_audio_pitch_info,
- .get = u_audio_pitch_get,
- .put = u_audio_pitch_put,
-},
-};
-
int g_audio_setup(struct g_audio *g_audio, const char *pcm_name,
const char *card_name)
{
struct snd_uac_chip *uac;
struct snd_card *card;
struct snd_pcm *pcm;
- struct snd_kcontrol *kctl;
struct uac_params *params;
int p_chmask, c_chmask;
int err;
@@ -760,23 +671,6 @@ int g_audio_setup(struct g_audio *g_audio, const char *pcm_name,
snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, &uac_pcm_ops);
snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, &uac_pcm_ops);
- if (c_chmask && g_audio->in_ep_fback) {
- strscpy(card->mixername, card_name, sizeof(card->driver));
-
- kctl = snd_ctl_new1(&u_audio_controls[0], &uac->c_prm);
- if (!kctl) {
- err = -ENOMEM;
- goto snd_fail;
- }
-
- kctl->id.device = pcm->device;
- kctl->id.subdevice = 0;
-
- err = snd_ctl_add(card, kctl);
- if (err < 0)
- goto snd_fail;
- }
-
strscpy(card->driver, card_name, sizeof(card->driver));
strscpy(card->shortname, card_name, sizeof(card->shortname));
sprintf(card->longname, "%s %i", card_name, card->dev->id);
diff --git a/drivers/usb/gadget/function/u_audio.h b/drivers/usb/gadget/function/u_audio.h
index a218cdf771fe..53e6baf55cbf 100644
--- a/drivers/usb/gadget/function/u_audio.h
+++ b/drivers/usb/gadget/function/u_audio.h
@@ -11,14 +11,6 @@
#include <linux/usb/composite.h>
-/*
- * Same maximum frequency deviation on the slower side as in
- * sound/usb/endpoint.c. Value is expressed in per-mil deviation.
- * The maximum deviation on the faster side will be provided as
- * parameter, as it impacts the endpoint required bandwidth.
- */
-#define FBACK_SLOW_MAX 250
-
struct uac_params {
/* playback */
int p_chmask; /* channel mask */
@@ -31,7 +23,6 @@ struct uac_params {
int c_ssize; /* sample size */
int req_number; /* number of preallocated requests */
- int fb_max; /* upper frequency drift feedback limit per-mil */
};
struct g_audio {
diff --git a/drivers/usb/gadget/function/u_uac2.h b/drivers/usb/gadget/function/u_uac2.h
index 179d3ef6a195..13589c3c805c 100644
--- a/drivers/usb/gadget/function/u_uac2.h
+++ b/drivers/usb/gadget/function/u_uac2.h
@@ -23,7 +23,6 @@
#define UAC2_DEF_CSSIZE 2
#define UAC2_DEF_CSYNC USB_ENDPOINT_SYNC_ASYNC
#define UAC2_DEF_REQ_NUM 2
-#define UAC2_DEF_FB_MAX 5
struct f_uac2_opts {
struct usb_function_instance func_inst;
@@ -35,7 +34,6 @@ struct f_uac2_opts {
int c_ssize;
int c_sync;
int req_number;
- int fb_max;
bool bound;
struct mutex lock;
--
2.30.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v1 2/3] Revert "usb: gadget: f_uac2: add adaptive sync support for capture"
2021-08-24 20:14 [PATCH v1 1/3] Revert "usb: gadget: u_audio: add real feedback implementation" Ferry Toth
@ 2021-08-24 20:14 ` Ferry Toth
2021-08-25 5:53 ` Felipe Balbi
2021-08-24 20:14 ` [PATCH v1 3/3] Revert "usb: gadget: f_uac2/u_audio: add feedback endpoint support" Ferry Toth
2021-08-25 5:53 ` [PATCH v1 1/3] Revert "usb: gadget: u_audio: add real feedback implementation" Felipe Balbi
2 siblings, 1 reply; 7+ messages in thread
From: Ferry Toth @ 2021-08-24 20:14 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jerome Brunet, Ruslan Bilovol, Oded Gabbay,
Cezary Rojewski, Ferry Toth, Mauro Carvalho Chehab,
Pawel Laszczak, Felipe Balbi, Jack Pham, linux-kernel, linux-usb,
linux-doc
Cc: Jonathan Corbet, Lorenzo Colitti, Wesley Cheng, robh+dt, agross,
bjorn.andersson, frowand.list, devicetree, linux-arm-msm,
heikki.krogerus, Thinh Nguyen, Andy Shevchenko, Pavel Hofman,
Ferry Toth
This reverts commit 40c73b30546e759bedcec607fedc2d4be954508f.
The commit is part of a series with commit
24f779dac8f3efb9629adc0e486914d93dc45517 causing a BUG on dwc3
hardware, at least on Intel Merrifield platform when configured
through configfs:
BUG: kernel NULL pointer dereference, address: 0000000000000008
...
RIP: 0010:dwc3_gadget_del_and_unmap_request+0x19/0xe0
...
Call Trace:
dwc3_remove_requests.constprop.0+0x12f/0x170
__dwc3_gadget_ep_disable+0x7a/0x160
dwc3_gadget_ep_disable+0x3d/0xd0
usb_ep_disable+0x1c/0x70
u_audio_stop_capture+0x79/0x120 [u_audio]
afunc_set_alt+0x73/0x80 [usb_f_uac2]
composite_setup+0x224/0x1b90 [libcomposite]
Pavel's suggestion to add
`echo "adaptive" > functions/uac2.usb0/c_sync` to the configfs script
resolves the issue.
Thinh suggests "the crash is probably because of f_uac2 prematurely
freeing feedback request before its completion. usb_ep_dequeue() is
asynchronous. dwc2() may treat it as a synchronous call so you didn't
get a crash."
Revert as this is a regression and the kernel shouldn't crash depending
on configuration parameters.
Reported-by: Ferry Toth <fntoth@gmail.com>
---
.../ABI/testing/configfs-usb-gadget-uac2 | 1 -
Documentation/usb/gadget-testing.rst | 1 -
drivers/usb/gadget/function/f_uac2.c | 100 ++----------------
drivers/usb/gadget/function/u_uac2.h | 2 -
4 files changed, 9 insertions(+), 95 deletions(-)
diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uac2 b/Documentation/ABI/testing/configfs-usb-gadget-uac2
index e7e59d7fb12f..d4356c8b8cd6 100644
--- a/Documentation/ABI/testing/configfs-usb-gadget-uac2
+++ b/Documentation/ABI/testing/configfs-usb-gadget-uac2
@@ -8,7 +8,6 @@ Description:
c_chmask capture channel mask
c_srate capture sampling rate
c_ssize capture sample size (bytes)
- c_sync capture synchronization type (async/adaptive)
p_chmask playback channel mask
p_srate playback sampling rate
p_ssize playback sample size (bytes)
diff --git a/Documentation/usb/gadget-testing.rst b/Documentation/usb/gadget-testing.rst
index f5a12667bd41..2085e7b24eeb 100644
--- a/Documentation/usb/gadget-testing.rst
+++ b/Documentation/usb/gadget-testing.rst
@@ -728,7 +728,6 @@ The uac2 function provides these attributes in its function directory:
c_chmask capture channel mask
c_srate capture sampling rate
c_ssize capture sample size (bytes)
- c_sync capture synchronization type (async/adaptive)
p_chmask playback channel mask
p_srate playback sampling rate
p_ssize playback sample size (bytes)
diff --git a/drivers/usb/gadget/function/f_uac2.c b/drivers/usb/gadget/function/f_uac2.c
index 321e6c05ba93..5d63244ba319 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -44,7 +44,6 @@
#define EPIN_EN(_opts) ((_opts)->p_chmask != 0)
#define EPOUT_EN(_opts) ((_opts)->c_chmask != 0)
-#define EPOUT_FBACK_IN_EN(_opts) ((_opts)->c_sync == USB_ENDPOINT_SYNC_ASYNC)
struct f_uac2 {
struct g_audio g_audio;
@@ -241,7 +240,7 @@ static struct usb_interface_descriptor std_as_out_if1_desc = {
.bDescriptorType = USB_DT_INTERFACE,
.bAlternateSetting = 1,
- .bNumEndpoints = 1,
+ .bNumEndpoints = 2,
.bInterfaceClass = USB_CLASS_AUDIO,
.bInterfaceSubClass = USB_SUBCLASS_AUDIOSTREAMING,
.bInterfaceProtocol = UAC_VERSION_2,
@@ -274,7 +273,7 @@ static struct usb_endpoint_descriptor fs_epout_desc = {
.bDescriptorType = USB_DT_ENDPOINT,
.bEndpointAddress = USB_DIR_OUT,
- /* .bmAttributes = DYNAMIC */
+ .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
/* .wMaxPacketSize = DYNAMIC */
.bInterval = 1,
};
@@ -283,7 +282,7 @@ static struct usb_endpoint_descriptor hs_epout_desc = {
.bLength = USB_DT_ENDPOINT_SIZE,
.bDescriptorType = USB_DT_ENDPOINT,
- /* .bmAttributes = DYNAMIC */
+ .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
/* .wMaxPacketSize = DYNAMIC */
.bInterval = 4,
};
@@ -293,7 +292,7 @@ static struct usb_endpoint_descriptor ss_epout_desc = {
.bDescriptorType = USB_DT_ENDPOINT,
.bEndpointAddress = USB_DIR_OUT,
- /* .bmAttributes = DYNAMIC */
+ .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
/* .wMaxPacketSize = DYNAMIC */
.bInterval = 4,
};
@@ -650,9 +649,7 @@ static void setup_headers(struct f_uac2_opts *opts,
headers[i++] = USBDHDR(epout_desc_comp);
headers[i++] = USBDHDR(&as_iso_out_desc);
-
- if (EPOUT_FBACK_IN_EN(opts))
- headers[i++] = USBDHDR(epin_fback_desc);
+ headers[i++] = USBDHDR(epin_fback_desc);
}
if (EPIN_EN(opts)) {
headers[i++] = USBDHDR(&std_as_in_if0_desc);
@@ -823,23 +820,6 @@ afunc_bind(struct usb_configuration *cfg, struct usb_function *fn)
std_as_out_if1_desc.bInterfaceNumber = ret;
uac2->as_out_intf = ret;
uac2->as_out_alt = 0;
-
- if (EPOUT_FBACK_IN_EN(uac2_opts)) {
- fs_epout_desc.bmAttributes =
- USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC;
- hs_epout_desc.bmAttributes =
- USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC;
- ss_epout_desc.bmAttributes =
- USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC;
- std_as_out_if1_desc.bNumEndpoints++;
- } else {
- fs_epout_desc.bmAttributes =
- USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ADAPTIVE;
- hs_epout_desc.bmAttributes =
- USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ADAPTIVE;
- ss_epout_desc.bmAttributes =
- USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ADAPTIVE;
- }
}
if (EPIN_EN(uac2_opts)) {
@@ -903,14 +883,11 @@ afunc_bind(struct usb_configuration *cfg, struct usb_function *fn)
dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
return -ENODEV;
}
- if (EPOUT_FBACK_IN_EN(uac2_opts)) {
- agdev->in_ep_fback = usb_ep_autoconfig(gadget,
+ agdev->in_ep_fback = usb_ep_autoconfig(gadget,
&fs_epin_fback_desc);
- if (!agdev->in_ep_fback) {
- dev_err(dev, "%s:%d Error!\n",
- __func__, __LINE__);
- return -ENODEV;
- }
+ if (!agdev->in_ep_fback) {
+ dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
+ return -ENODEV;
}
}
@@ -1265,68 +1242,11 @@ end: \
\
CONFIGFS_ATTR(f_uac2_opts_, name)
-#define UAC2_ATTRIBUTE_SYNC(name) \
-static ssize_t f_uac2_opts_##name##_show(struct config_item *item, \
- char *page) \
-{ \
- struct f_uac2_opts *opts = to_f_uac2_opts(item); \
- int result; \
- char *str; \
- \
- mutex_lock(&opts->lock); \
- switch (opts->name) { \
- case USB_ENDPOINT_SYNC_ASYNC: \
- str = "async"; \
- break; \
- case USB_ENDPOINT_SYNC_ADAPTIVE: \
- str = "adaptive"; \
- break; \
- default: \
- str = "unknown"; \
- break; \
- } \
- result = sprintf(page, "%s\n", str); \
- mutex_unlock(&opts->lock); \
- \
- return result; \
-} \
- \
-static ssize_t f_uac2_opts_##name##_store(struct config_item *item, \
- const char *page, size_t len) \
-{ \
- struct f_uac2_opts *opts = to_f_uac2_opts(item); \
- int ret = 0; \
- \
- mutex_lock(&opts->lock); \
- if (opts->refcnt) { \
- ret = -EBUSY; \
- goto end; \
- } \
- \
- if (!strncmp(page, "async", 5)) \
- opts->name = USB_ENDPOINT_SYNC_ASYNC; \
- else if (!strncmp(page, "adaptive", 8)) \
- opts->name = USB_ENDPOINT_SYNC_ADAPTIVE; \
- else { \
- ret = -EINVAL; \
- goto end; \
- } \
- \
- ret = len; \
- \
-end: \
- mutex_unlock(&opts->lock); \
- return ret; \
-} \
- \
-CONFIGFS_ATTR(f_uac2_opts_, name)
-
UAC2_ATTRIBUTE(p_chmask);
UAC2_ATTRIBUTE(p_srate);
UAC2_ATTRIBUTE(p_ssize);
UAC2_ATTRIBUTE(c_chmask);
UAC2_ATTRIBUTE(c_srate);
-UAC2_ATTRIBUTE_SYNC(c_sync);
UAC2_ATTRIBUTE(c_ssize);
UAC2_ATTRIBUTE(req_number);
@@ -1337,7 +1257,6 @@ static struct configfs_attribute *f_uac2_attrs[] = {
&f_uac2_opts_attr_c_chmask,
&f_uac2_opts_attr_c_srate,
&f_uac2_opts_attr_c_ssize,
- &f_uac2_opts_attr_c_sync,
&f_uac2_opts_attr_req_number,
NULL,
};
@@ -1376,7 +1295,6 @@ static struct usb_function_instance *afunc_alloc_inst(void)
opts->c_chmask = UAC2_DEF_CCHMASK;
opts->c_srate = UAC2_DEF_CSRATE;
opts->c_ssize = UAC2_DEF_CSSIZE;
- opts->c_sync = UAC2_DEF_CSYNC;
opts->req_number = UAC2_DEF_REQ_NUM;
return &opts->func_inst;
}
diff --git a/drivers/usb/gadget/function/u_uac2.h b/drivers/usb/gadget/function/u_uac2.h
index 13589c3c805c..b5035711172d 100644
--- a/drivers/usb/gadget/function/u_uac2.h
+++ b/drivers/usb/gadget/function/u_uac2.h
@@ -21,7 +21,6 @@
#define UAC2_DEF_CCHMASK 0x3
#define UAC2_DEF_CSRATE 64000
#define UAC2_DEF_CSSIZE 2
-#define UAC2_DEF_CSYNC USB_ENDPOINT_SYNC_ASYNC
#define UAC2_DEF_REQ_NUM 2
struct f_uac2_opts {
@@ -32,7 +31,6 @@ struct f_uac2_opts {
int c_chmask;
int c_srate;
int c_ssize;
- int c_sync;
int req_number;
bool bound;
--
2.30.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v1 3/3] Revert "usb: gadget: f_uac2/u_audio: add feedback endpoint support"
2021-08-24 20:14 [PATCH v1 1/3] Revert "usb: gadget: u_audio: add real feedback implementation" Ferry Toth
2021-08-24 20:14 ` [PATCH v1 2/3] Revert "usb: gadget: f_uac2: add adaptive sync support for capture" Ferry Toth
@ 2021-08-24 20:14 ` Ferry Toth
2021-08-25 5:54 ` Felipe Balbi
2021-08-25 5:53 ` [PATCH v1 1/3] Revert "usb: gadget: u_audio: add real feedback implementation" Felipe Balbi
2 siblings, 1 reply; 7+ messages in thread
From: Ferry Toth @ 2021-08-24 20:14 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jerome Brunet, Ruslan Bilovol, Oded Gabbay,
Cezary Rojewski, Ferry Toth, Mauro Carvalho Chehab,
Pawel Laszczak, Felipe Balbi, Jack Pham, linux-kernel, linux-usb,
linux-doc
Cc: Jonathan Corbet, Lorenzo Colitti, Wesley Cheng, robh+dt, agross,
bjorn.andersson, frowand.list, devicetree, linux-arm-msm,
heikki.krogerus, Thinh Nguyen, Andy Shevchenko, Pavel Hofman,
Ferry Toth
This reverts commit 24f779dac8f3efb9629adc0e486914d93dc45517.
The commit is part of a series with commit
24f779dac8f3efb9629adc0e486914d93dc45517 causing a BUG on dwc3
hardware, at least on Intel Merrifield platform when configured
through configfs:
BUG: kernel NULL pointer dereference, address: 0000000000000008
...
RIP: 0010:dwc3_gadget_del_and_unmap_request+0x19/0xe0
...
Call Trace:
dwc3_remove_requests.constprop.0+0x12f/0x170
__dwc3_gadget_ep_disable+0x7a/0x160
dwc3_gadget_ep_disable+0x3d/0xd0
usb_ep_disable+0x1c/0x70
u_audio_stop_capture+0x79/0x120 [u_audio]
afunc_set_alt+0x73/0x80 [usb_f_uac2]
composite_setup+0x224/0x1b90 [libcomposite]
Pavel's suggestion to add
`echo "adaptive" > functions/uac2.usb0/c_sync` to the configfs script
resolves the issue.
Thinh suggests "the crash is probably because of f_uac2 prematurely
freeing feedback request before its completion. usb_ep_dequeue() is
asynchronous. dwc2() may treat it as a synchronous call so you didn't
get a crash."
Revert as this is a regression and the kernel shouldn't crash depending
on configuration parameters.
Reported-by: Ferry Toth <fntoth@gmail.com>
---
drivers/usb/gadget/function/f_uac2.c | 49 +----------
drivers/usb/gadget/function/u_audio.c | 119 +-------------------------
drivers/usb/gadget/function/u_audio.h | 3 -
3 files changed, 3 insertions(+), 168 deletions(-)
diff --git a/drivers/usb/gadget/function/f_uac2.c b/drivers/usb/gadget/function/f_uac2.c
index 5d63244ba319..7aa4c8bc5a1a 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -240,7 +240,7 @@ static struct usb_interface_descriptor std_as_out_if1_desc = {
.bDescriptorType = USB_DT_INTERFACE,
.bAlternateSetting = 1,
- .bNumEndpoints = 2,
+ .bNumEndpoints = 1,
.bInterfaceClass = USB_CLASS_AUDIO,
.bInterfaceSubClass = USB_SUBCLASS_AUDIOSTREAMING,
.bInterfaceProtocol = UAC_VERSION_2,
@@ -317,37 +317,6 @@ static struct uac2_iso_endpoint_descriptor as_iso_out_desc = {
.wLockDelay = 0,
};
-/* STD AS ISO IN Feedback Endpoint */
-static struct usb_endpoint_descriptor fs_epin_fback_desc = {
- .bLength = USB_DT_ENDPOINT_SIZE,
- .bDescriptorType = USB_DT_ENDPOINT,
-
- .bEndpointAddress = USB_DIR_IN,
- .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_USAGE_FEEDBACK,
- .wMaxPacketSize = cpu_to_le16(3),
- .bInterval = 1,
-};
-
-static struct usb_endpoint_descriptor hs_epin_fback_desc = {
- .bLength = USB_DT_ENDPOINT_SIZE,
- .bDescriptorType = USB_DT_ENDPOINT,
-
- .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_USAGE_FEEDBACK,
- .wMaxPacketSize = cpu_to_le16(4),
- .bInterval = 4,
-};
-
-static struct usb_endpoint_descriptor ss_epin_fback_desc = {
- .bLength = USB_DT_ENDPOINT_SIZE,
- .bDescriptorType = USB_DT_ENDPOINT,
-
- .bEndpointAddress = USB_DIR_IN,
- .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_USAGE_FEEDBACK,
- .wMaxPacketSize = cpu_to_le16(4),
- .bInterval = 4,
-};
-
-
/* Audio Streaming IN Interface - Alt0 */
static struct usb_interface_descriptor std_as_in_if0_desc = {
.bLength = sizeof std_as_in_if0_desc,
@@ -462,7 +431,6 @@ static struct usb_descriptor_header *fs_audio_desc[] = {
(struct usb_descriptor_header *)&as_out_fmt1_desc,
(struct usb_descriptor_header *)&fs_epout_desc,
(struct usb_descriptor_header *)&as_iso_out_desc,
- (struct usb_descriptor_header *)&fs_epin_fback_desc,
(struct usb_descriptor_header *)&std_as_in_if0_desc,
(struct usb_descriptor_header *)&std_as_in_if1_desc,
@@ -493,7 +461,6 @@ static struct usb_descriptor_header *hs_audio_desc[] = {
(struct usb_descriptor_header *)&as_out_fmt1_desc,
(struct usb_descriptor_header *)&hs_epout_desc,
(struct usb_descriptor_header *)&as_iso_out_desc,
- (struct usb_descriptor_header *)&hs_epin_fback_desc,
(struct usb_descriptor_header *)&std_as_in_if0_desc,
(struct usb_descriptor_header *)&std_as_in_if1_desc,
@@ -525,7 +492,6 @@ static struct usb_descriptor_header *ss_audio_desc[] = {
(struct usb_descriptor_header *)&ss_epout_desc,
(struct usb_descriptor_header *)&ss_epout_desc_comp,
(struct usb_descriptor_header *)&as_iso_out_desc,
- (struct usb_descriptor_header *)&ss_epin_fback_desc,
(struct usb_descriptor_header *)&std_as_in_if0_desc,
(struct usb_descriptor_header *)&std_as_in_if1_desc,
@@ -602,26 +568,22 @@ static void setup_headers(struct f_uac2_opts *opts,
struct usb_ss_ep_comp_descriptor *epin_desc_comp = NULL;
struct usb_endpoint_descriptor *epout_desc;
struct usb_endpoint_descriptor *epin_desc;
- struct usb_endpoint_descriptor *epin_fback_desc;
int i;
switch (speed) {
case USB_SPEED_FULL:
epout_desc = &fs_epout_desc;
epin_desc = &fs_epin_desc;
- epin_fback_desc = &fs_epin_fback_desc;
break;
case USB_SPEED_HIGH:
epout_desc = &hs_epout_desc;
epin_desc = &hs_epin_desc;
- epin_fback_desc = &hs_epin_fback_desc;
break;
default:
epout_desc = &ss_epout_desc;
epin_desc = &ss_epin_desc;
epout_desc_comp = &ss_epout_desc_comp;
epin_desc_comp = &ss_epin_desc_comp;
- epin_fback_desc = &ss_epin_fback_desc;
}
i = 0;
@@ -649,7 +611,6 @@ static void setup_headers(struct f_uac2_opts *opts,
headers[i++] = USBDHDR(epout_desc_comp);
headers[i++] = USBDHDR(&as_iso_out_desc);
- headers[i++] = USBDHDR(epin_fback_desc);
}
if (EPIN_EN(opts)) {
headers[i++] = USBDHDR(&std_as_in_if0_desc);
@@ -883,12 +844,6 @@ afunc_bind(struct usb_configuration *cfg, struct usb_function *fn)
dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
return -ENODEV;
}
- agdev->in_ep_fback = usb_ep_autoconfig(gadget,
- &fs_epin_fback_desc);
- if (!agdev->in_ep_fback) {
- dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
- return -ENODEV;
- }
}
if (EPIN_EN(uac2_opts)) {
@@ -912,10 +867,8 @@ afunc_bind(struct usb_configuration *cfg, struct usb_function *fn)
le16_to_cpu(ss_epout_desc.wMaxPacketSize));
hs_epout_desc.bEndpointAddress = fs_epout_desc.bEndpointAddress;
- hs_epin_fback_desc.bEndpointAddress = fs_epin_fback_desc.bEndpointAddress;
hs_epin_desc.bEndpointAddress = fs_epin_desc.bEndpointAddress;
ss_epout_desc.bEndpointAddress = fs_epout_desc.bEndpointAddress;
- ss_epin_fback_desc.bEndpointAddress = fs_epin_fback_desc.bEndpointAddress;
ss_epin_desc.bEndpointAddress = fs_epin_desc.bEndpointAddress;
setup_descriptor(uac2_opts);
diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
index f637ebec80b0..5fbceee897a3 100644
--- a/drivers/usb/gadget/function/u_audio.c
+++ b/drivers/usb/gadget/function/u_audio.c
@@ -38,10 +38,6 @@ struct uac_rtd_params {
unsigned int max_psize; /* MaxPacketSize of endpoint */
struct usb_request **reqs;
-
- struct usb_request *req_fback; /* Feedback endpoint request */
- unsigned int ffback; /* Real frequency reported by feedback endpoint */
- bool fb_ep_enabled; /* if the ep is enabled */
};
struct snd_uac_chip {
@@ -74,32 +70,6 @@ static const struct snd_pcm_hardware uac_pcm_hardware = {
.periods_min = MIN_PERIODS,
};
-static void u_audio_set_fback_frequency(enum usb_device_speed speed,
- unsigned int freq, void *buf)
-{
- u32 ff = 0;
-
- if (speed == USB_SPEED_FULL) {
- /*
- * Full-speed feedback endpoints report frequency
- * in samples/microframe
- * Format is encoded in Q10.10 left-justified in the 24 bits,
- * so that it has a Q10.14 format.
- */
- ff = DIV_ROUND_UP((freq << 14), 1000);
- } else {
- /*
- * High-speed feedback endpoints report frequency
- * in samples/microframe.
- * Format is encoded in Q12.13 fitted into four bytes so that
- * the binary point is located between the second and the third
- * byte fromat (that is Q16.16)
- */
- ff = DIV_ROUND_UP((freq << 13), 1000);
- }
- *(__le32 *)buf = cpu_to_le32(ff);
-}
-
static void u_audio_iso_complete(struct usb_ep *ep, struct usb_request *req)
{
unsigned int pending;
@@ -203,34 +173,6 @@ static void u_audio_iso_complete(struct usb_ep *ep, struct usb_request *req)
dev_err(uac->card->dev, "%d Error!\n", __LINE__);
}
-static void u_audio_iso_fback_complete(struct usb_ep *ep,
- struct usb_request *req)
-{
- struct uac_rtd_params *prm = req->context;
- struct snd_uac_chip *uac = prm->uac;
- struct g_audio *audio_dev = uac->audio_dev;
- int status = req->status;
- unsigned long flags;
-
- /* i/f shutting down */
- if (!prm->fb_ep_enabled || req->status == -ESHUTDOWN)
- return;
-
- /*
- * We can't really do much about bad xfers.
- * Afterall, the ISOCH xfers could fail legitimately.
- */
- if (status)
- pr_debug("%s: iso_complete status(%d) %d/%d\n",
- __func__, status, req->actual, req->length);
-
- u_audio_set_fback_frequency(audio_dev->gadget->speed,
- prm->ffback, req->buf);
-
- if (usb_ep_queue(ep, req, GFP_ATOMIC))
- dev_err(uac->card->dev, "%d Error!\n", __LINE__);
-}
-
static int uac_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
{
struct snd_uac_chip *uac = snd_pcm_substream_chip(substream);
@@ -393,33 +335,14 @@ static inline void free_ep(struct uac_rtd_params *prm, struct usb_ep *ep)
dev_err(uac->card->dev, "%s:%d Error!\n", __func__, __LINE__);
}
-static inline void free_ep_fback(struct uac_rtd_params *prm, struct usb_ep *ep)
-{
- struct snd_uac_chip *uac = prm->uac;
-
- if (!prm->fb_ep_enabled)
- return;
-
- prm->fb_ep_enabled = false;
-
- if (prm->req_fback) {
- usb_ep_dequeue(ep, prm->req_fback);
- kfree(prm->req_fback->buf);
- usb_ep_free_request(ep, prm->req_fback);
- prm->req_fback = NULL;
- }
-
- if (usb_ep_disable(ep))
- dev_err(uac->card->dev, "%s:%d Error!\n", __func__, __LINE__);
-}
int u_audio_start_capture(struct g_audio *audio_dev)
{
struct snd_uac_chip *uac = audio_dev->uac;
struct usb_gadget *gadget = audio_dev->gadget;
struct device *dev = &gadget->dev;
- struct usb_request *req, *req_fback;
- struct usb_ep *ep, *ep_fback;
+ struct usb_request *req;
+ struct usb_ep *ep;
struct uac_rtd_params *prm;
struct uac_params *params = &audio_dev->params;
int req_len, i;
@@ -451,42 +374,6 @@ int u_audio_start_capture(struct g_audio *audio_dev)
dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
}
- ep_fback = audio_dev->in_ep_fback;
- if (!ep_fback)
- return 0;
-
- /* Setup feedback endpoint */
- config_ep_by_speed(gadget, &audio_dev->func, ep_fback);
- prm->fb_ep_enabled = true;
- usb_ep_enable(ep_fback);
- req_len = ep_fback->maxpacket;
-
- req_fback = usb_ep_alloc_request(ep_fback, GFP_ATOMIC);
- if (req_fback == NULL)
- return -ENOMEM;
-
- prm->req_fback = req_fback;
- req_fback->zero = 0;
- req_fback->context = prm;
- req_fback->length = req_len;
- req_fback->complete = u_audio_iso_fback_complete;
-
- req_fback->buf = kzalloc(req_len, GFP_ATOMIC);
- if (!req_fback->buf)
- return -ENOMEM;
-
- /*
- * Configure the feedback endpoint's reported frequency.
- * Always start with original frequency since its deviation can't
- * be meauserd at start of playback
- */
- prm->ffback = params->c_srate;
- u_audio_set_fback_frequency(audio_dev->gadget->speed,
- prm->ffback, req_fback->buf);
-
- if (usb_ep_queue(ep_fback, req_fback, GFP_ATOMIC))
- dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
-
return 0;
}
EXPORT_SYMBOL_GPL(u_audio_start_capture);
@@ -495,8 +382,6 @@ void u_audio_stop_capture(struct g_audio *audio_dev)
{
struct snd_uac_chip *uac = audio_dev->uac;
- if (audio_dev->in_ep_fback)
- free_ep_fback(&uac->c_prm, audio_dev->in_ep_fback);
free_ep(&uac->c_prm, audio_dev->out_ep);
}
EXPORT_SYMBOL_GPL(u_audio_stop_capture);
diff --git a/drivers/usb/gadget/function/u_audio.h b/drivers/usb/gadget/function/u_audio.h
index 53e6baf55cbf..5ea6b86f1fda 100644
--- a/drivers/usb/gadget/function/u_audio.h
+++ b/drivers/usb/gadget/function/u_audio.h
@@ -30,10 +30,7 @@ struct g_audio {
struct usb_gadget *gadget;
struct usb_ep *in_ep;
-
struct usb_ep *out_ep;
- /* feedback IN endpoint corresponding to out_ep */
- struct usb_ep *in_ep_fback;
/* Max packet size for all in_ep possible speeds */
unsigned int in_ep_maxpsize;
--
2.30.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v1 1/3] Revert "usb: gadget: u_audio: add real feedback implementation"
2021-08-24 20:14 [PATCH v1 1/3] Revert "usb: gadget: u_audio: add real feedback implementation" Ferry Toth
2021-08-24 20:14 ` [PATCH v1 2/3] Revert "usb: gadget: f_uac2: add adaptive sync support for capture" Ferry Toth
2021-08-24 20:14 ` [PATCH v1 3/3] Revert "usb: gadget: f_uac2/u_audio: add feedback endpoint support" Ferry Toth
@ 2021-08-25 5:53 ` Felipe Balbi
2021-08-25 7:16 ` Ferry Toth
2 siblings, 1 reply; 7+ messages in thread
From: Felipe Balbi @ 2021-08-25 5:53 UTC (permalink / raw)
To: Ferry Toth
Cc: Greg Kroah-Hartman, Jerome Brunet, Ruslan Bilovol, Oded Gabbay,
Cezary Rojewski, Mauro Carvalho Chehab, Pawel Laszczak,
Jack Pham, linux-kernel, linux-usb, linux-doc, Jonathan Corbet,
Lorenzo Colitti, Wesley Cheng, robh+dt, agross, bjorn.andersson,
frowand.list, devicetree, linux-arm-msm, heikki.krogerus,
Thinh Nguyen, Andy Shevchenko, Pavel Hofman, Ferry Toth
Ferry Toth <ftoth@exalondelft.nl> writes:
> This reverts commit e89bb4288378b85c82212b60dc98ecda6b3d3a70.
>
> The commit is part of a series with commit
> 24f779dac8f3efb9629adc0e486914d93dc45517 causing a BUG on dwc3
> hardware, at least on Intel Merrifield platform when configured
> through configfs:
> BUG: kernel NULL pointer dereference, address: 0000000000000008
> ...
> RIP: 0010:dwc3_gadget_del_and_unmap_request+0x19/0xe0
> ...
> Call Trace:
> dwc3_remove_requests.constprop.0+0x12f/0x170
> __dwc3_gadget_ep_disable+0x7a/0x160
> dwc3_gadget_ep_disable+0x3d/0xd0
> usb_ep_disable+0x1c/0x70
> u_audio_stop_capture+0x79/0x120 [u_audio]
> afunc_set_alt+0x73/0x80 [usb_f_uac2]
> composite_setup+0x224/0x1b90 [libcomposite]
>
> Pavel's suggestion to add
> `echo "adaptive" > functions/uac2.usb0/c_sync` to the configfs script
> resolves the issue.
> Thinh suggests "the crash is probably because of f_uac2 prematurely
> freeing feedback request before its completion. usb_ep_dequeue() is
> asynchronous. dwc2() may treat it as a synchronous call so you didn't
> get a crash."
>
> Revert as this is a regression and the kernel shouldn't crash depending
> on configuration parameters.
>
> Reported-by: Ferry Toth <fntoth@gmail.com>
this should be Signed-off-by ;-)
--
balbi
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 2/3] Revert "usb: gadget: f_uac2: add adaptive sync support for capture"
2021-08-24 20:14 ` [PATCH v1 2/3] Revert "usb: gadget: f_uac2: add adaptive sync support for capture" Ferry Toth
@ 2021-08-25 5:53 ` Felipe Balbi
0 siblings, 0 replies; 7+ messages in thread
From: Felipe Balbi @ 2021-08-25 5:53 UTC (permalink / raw)
To: Ferry Toth
Cc: Greg Kroah-Hartman, Jerome Brunet, Ruslan Bilovol, Oded Gabbay,
Cezary Rojewski, Mauro Carvalho Chehab, Pawel Laszczak,
Jack Pham, linux-kernel, linux-usb, linux-doc, Jonathan Corbet,
Lorenzo Colitti, Wesley Cheng, robh+dt, agross, bjorn.andersson,
frowand.list, devicetree, linux-arm-msm, heikki.krogerus,
Thinh Nguyen, Andy Shevchenko, Pavel Hofman, Ferry Toth
Ferry Toth <ftoth@exalondelft.nl> writes:
> This reverts commit 40c73b30546e759bedcec607fedc2d4be954508f.
>
> The commit is part of a series with commit
> 24f779dac8f3efb9629adc0e486914d93dc45517 causing a BUG on dwc3
> hardware, at least on Intel Merrifield platform when configured
> through configfs:
> BUG: kernel NULL pointer dereference, address: 0000000000000008
> ...
> RIP: 0010:dwc3_gadget_del_and_unmap_request+0x19/0xe0
> ...
> Call Trace:
> dwc3_remove_requests.constprop.0+0x12f/0x170
> __dwc3_gadget_ep_disable+0x7a/0x160
> dwc3_gadget_ep_disable+0x3d/0xd0
> usb_ep_disable+0x1c/0x70
> u_audio_stop_capture+0x79/0x120 [u_audio]
> afunc_set_alt+0x73/0x80 [usb_f_uac2]
> composite_setup+0x224/0x1b90 [libcomposite]
>
> Pavel's suggestion to add
> `echo "adaptive" > functions/uac2.usb0/c_sync` to the configfs script
> resolves the issue.
> Thinh suggests "the crash is probably because of f_uac2 prematurely
> freeing feedback request before its completion. usb_ep_dequeue() is
> asynchronous. dwc2() may treat it as a synchronous call so you didn't
> get a crash."
>
> Revert as this is a regression and the kernel shouldn't crash depending
> on configuration parameters.
>
> Reported-by: Ferry Toth <fntoth@gmail.com>
Signed-off-by?
--
balbi
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 3/3] Revert "usb: gadget: f_uac2/u_audio: add feedback endpoint support"
2021-08-24 20:14 ` [PATCH v1 3/3] Revert "usb: gadget: f_uac2/u_audio: add feedback endpoint support" Ferry Toth
@ 2021-08-25 5:54 ` Felipe Balbi
0 siblings, 0 replies; 7+ messages in thread
From: Felipe Balbi @ 2021-08-25 5:54 UTC (permalink / raw)
To: Ferry Toth
Cc: Greg Kroah-Hartman, Jerome Brunet, Ruslan Bilovol, Oded Gabbay,
Cezary Rojewski, Mauro Carvalho Chehab, Pawel Laszczak,
Jack Pham, linux-kernel, linux-usb, linux-doc, Jonathan Corbet,
Lorenzo Colitti, Wesley Cheng, robh+dt, agross, bjorn.andersson,
frowand.list, devicetree, linux-arm-msm, heikki.krogerus,
Thinh Nguyen, Andy Shevchenko, Pavel Hofman, Ferry Toth
Ferry Toth <ftoth@exalondelft.nl> writes:
> This reverts commit 24f779dac8f3efb9629adc0e486914d93dc45517.
>
> The commit is part of a series with commit
> 24f779dac8f3efb9629adc0e486914d93dc45517 causing a BUG on dwc3
> hardware, at least on Intel Merrifield platform when configured
> through configfs:
> BUG: kernel NULL pointer dereference, address: 0000000000000008
> ...
> RIP: 0010:dwc3_gadget_del_and_unmap_request+0x19/0xe0
> ...
> Call Trace:
> dwc3_remove_requests.constprop.0+0x12f/0x170
> __dwc3_gadget_ep_disable+0x7a/0x160
> dwc3_gadget_ep_disable+0x3d/0xd0
> usb_ep_disable+0x1c/0x70
> u_audio_stop_capture+0x79/0x120 [u_audio]
> afunc_set_alt+0x73/0x80 [usb_f_uac2]
> composite_setup+0x224/0x1b90 [libcomposite]
>
> Pavel's suggestion to add
> `echo "adaptive" > functions/uac2.usb0/c_sync` to the configfs script
> resolves the issue.
> Thinh suggests "the crash is probably because of f_uac2 prematurely
> freeing feedback request before its completion. usb_ep_dequeue() is
> asynchronous. dwc2() may treat it as a synchronous call so you didn't
> get a crash."
>
> Revert as this is a regression and the kernel shouldn't crash depending
> on configuration parameters.
>
> Reported-by: Ferry Toth <fntoth@gmail.com>
Signed-off-by?
--
balbi
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 1/3] Revert "usb: gadget: u_audio: add real feedback implementation"
2021-08-25 5:53 ` [PATCH v1 1/3] Revert "usb: gadget: u_audio: add real feedback implementation" Felipe Balbi
@ 2021-08-25 7:16 ` Ferry Toth
0 siblings, 0 replies; 7+ messages in thread
From: Ferry Toth @ 2021-08-25 7:16 UTC (permalink / raw)
To: Felipe Balbi, Ferry Toth
Cc: Greg Kroah-Hartman, Jerome Brunet, Ruslan Bilovol, Oded Gabbay,
Cezary Rojewski, Mauro Carvalho Chehab, Pawel Laszczak,
Jack Pham, linux-kernel, linux-usb, linux-doc, Jonathan Corbet,
Lorenzo Colitti, Wesley Cheng, robh+dt, agross, bjorn.andersson,
frowand.list, devicetree, linux-arm-msm, heikki.krogerus,
Thinh Nguyen, Andy Shevchenko, Pavel Hofman
Hi,
Op 25-08-2021 om 07:53 schreef Felipe Balbi:
>
> Ferry Toth <ftoth@exalondelft.nl> writes:
>
>> This reverts commit e89bb4288378b85c82212b60dc98ecda6b3d3a70.
>>
>> The commit is part of a series with commit
>> 24f779dac8f3efb9629adc0e486914d93dc45517 causing a BUG on dwc3
>> hardware, at least on Intel Merrifield platform when configured
>> through configfs:
>> BUG: kernel NULL pointer dereference, address: 0000000000000008
>> ...
>> RIP: 0010:dwc3_gadget_del_and_unmap_request+0x19/0xe0
>> ...
>> Call Trace:
>> dwc3_remove_requests.constprop.0+0x12f/0x170
>> __dwc3_gadget_ep_disable+0x7a/0x160
>> dwc3_gadget_ep_disable+0x3d/0xd0
>> usb_ep_disable+0x1c/0x70
>> u_audio_stop_capture+0x79/0x120 [u_audio]
>> afunc_set_alt+0x73/0x80 [usb_f_uac2]
>> composite_setup+0x224/0x1b90 [libcomposite]
>>
>> Pavel's suggestion to add
>> `echo "adaptive" > functions/uac2.usb0/c_sync` to the configfs script
>> resolves the issue.
>> Thinh suggests "the crash is probably because of f_uac2 prematurely
>> freeing feedback request before its completion. usb_ep_dequeue() is
>> asynchronous. dwc2() may treat it as a synchronous call so you didn't
>> get a crash."
>>
>> Revert as this is a regression and the kernel shouldn't crash depending
>> on configuration parameters.
>>
>> Reported-by: Ferry Toth <fntoth@gmail.com>
>
> this should be Signed-off-by ;-)
>
Indeed. It probably was until I re-worded the commit text.
Will resend tonight v2.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-08-25 7:16 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-24 20:14 [PATCH v1 1/3] Revert "usb: gadget: u_audio: add real feedback implementation" Ferry Toth
2021-08-24 20:14 ` [PATCH v1 2/3] Revert "usb: gadget: f_uac2: add adaptive sync support for capture" Ferry Toth
2021-08-25 5:53 ` Felipe Balbi
2021-08-24 20:14 ` [PATCH v1 3/3] Revert "usb: gadget: f_uac2/u_audio: add feedback endpoint support" Ferry Toth
2021-08-25 5:54 ` Felipe Balbi
2021-08-25 5:53 ` [PATCH v1 1/3] Revert "usb: gadget: u_audio: add real feedback implementation" Felipe Balbi
2021-08-25 7:16 ` Ferry Toth
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).