[PATCH] usb: gadget: f_uac1: add set requests support

[PATCH] usb: gadget: f_uac1: add set requests support

[3090101217]

From: Jing Leng <jleng@ambarella.com>

Currently the f_uac1 driver only supports UAC_SET_CUR request.

But when uac1 device is plugged to Ubuntu 20.04 PC, at the stage
of setup, the PC will send UAC_SET_RES request, If the device
doesn't respond to the request, the PC will abort the setup process
and uac1 device can't be recognized on Ubuntu 20.04 PC.

So f_uac1 driver should handle other set requests.

Signed-off-by: Jing Leng <jleng@ambarella.com>
---
 drivers/usb/gadget/function/f_uac1.c | 44 +++++++++++++++++++++++-----
 1 file changed, 36 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/gadget/function/f_uac1.c b/drivers/usb/gadget/function/f_uac1.c
index 03f50643fbba..c9d8ec4fdf22 100644
--- a/drivers/usb/gadget/function/f_uac1.c
+++ b/drivers/usb/gadget/function/f_uac1.c
@@ -589,7 +589,7 @@ in_rq_res(struct usb_function *fn, const struct usb_ctrlrequest *cr)
 }
 
 static void
-out_rq_cur_complete(struct usb_ep *ep, struct usb_request *req)
+out_rq_complete(struct usb_ep *ep, struct usb_request *req)
 {
 	struct g_audio *audio = req->context;
 	struct usb_composite_dev *cdev = audio->func.config->cdev;
@@ -614,9 +614,11 @@ out_rq_cur_complete(struct usb_ep *ep, struct usb_request *req)
 			is_playback = 1;
 
 		if (control_selector == UAC_FU_MUTE) {
-			u8 mute = *(u8 *)req->buf;
+			if (cr->bRequest == UAC_SET_CUR) {
+				u8 mute = *(u8 *)req->buf;
 
-			u_audio_set_mute(audio, is_playback, mute);
+				u_audio_set_mute(audio, is_playback, mute);
+			}
 
 			return;
 		} else if (control_selector == UAC_FU_VOLUME) {
@@ -624,7 +626,34 @@ out_rq_cur_complete(struct usb_ep *ep, struct usb_request *req)
 			s16 volume;
 
 			volume = le16_to_cpu(*c);
-			u_audio_set_volume(audio, is_playback, volume);
+
+			switch (cr->bRequest) {
+			case UAC_SET_CUR:
+				u_audio_set_volume(audio, is_playback, volume);
+				break;
+			case UAC_SET_MIN:
+				if (is_playback)
+					opts->p_volume_min = volume;
+				else
+					opts->c_volume_min = volume;
+				break;
+			case UAC_SET_MAX:
+				if (is_playback)
+					opts->p_volume_max = volume;
+				else
+					opts->c_volume_max = volume;
+				break;
+			case UAC_SET_RES:
+				if (is_playback)
+					opts->p_volume_res = volume;
+				else
+					opts->c_volume_res = volume;
+				break;
+			case UAC_SET_MEM:
+				break;
+			default:
+				break;
+			}
 
 			return;
 		} else {
@@ -643,7 +672,7 @@ out_rq_cur_complete(struct usb_ep *ep, struct usb_request *req)
 }
 
 static int
-out_rq_cur(struct usb_function *fn, const struct usb_ctrlrequest *cr)
+ac_rq_out(struct usb_function *fn, const struct usb_ctrlrequest *cr)
 {
 	struct usb_request *req = fn->config->cdev->req;
 	struct g_audio *audio = func_to_g_audio(fn);
@@ -659,7 +688,7 @@ out_rq_cur(struct usb_function *fn, const struct usb_ctrlrequest *cr)
 			(FUOUT_EN(opts) && (entity_id == USB_OUT_FU_ID))) {
 		memcpy(&uac1->setup_cr, cr, sizeof(*cr));
 		req->context = audio;
-		req->complete = out_rq_cur_complete;
+		req->complete = out_rq_complete;
 
 		return w_length;
 	} else {
@@ -789,8 +818,7 @@ f_audio_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
 		value = audio_get_endpoint_req(f, ctrl);
 		break;
 	case USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE:
-		if (ctrl->bRequest == UAC_SET_CUR)
-			value = out_rq_cur(f, ctrl);
+		value = ac_rq_out(f, ctrl);
 		break;
 	case USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE:
 		value = ac_rq_in(f, ctrl);
-- 
2.17.1

Re: [PATCH] usb: gadget: f_uac1: add set requests support

[Greg KH]

On Wed, Feb 16, 2022 at 05:43:01PM +0800, 3090101217@zju.edu.cn wrote:
> From: Jing Leng <jleng@ambarella.com>
> 
> Currently the f_uac1 driver only supports UAC_SET_CUR request.
> 
> But when uac1 device is plugged to Ubuntu 20.04 PC, at the stage
> of setup, the PC will send UAC_SET_RES request, If the device
> doesn't respond to the request, the PC will abort the setup process
> and uac1 device can't be recognized on Ubuntu 20.04 PC.

So is this a bug in the Host side to not do stuff like this?  Why not
fix it there instead?

Where is the requirement that this command must be handled by the
device?

thanks,

greg k-h

Re: [PATCH] usb: gadget: f_uac1: add set requests support

[Jing Leng]

Hi Greg KH,

> So is this a bug in the Host side to not do stuff like this?  Why not
> fix it there instead?
> 
> Where is the requirement that this command must be handled by the
> device?
> 

First we need to clarify two issues.

1. Does the Ubuntu go beyond the UAC1 specification?
No. 
On page 66 of the UAC1 specification (
https://www.usb.org/sites/default/files/audio10.pdf):
The bRequest can be SET_CUR, SET_MIN, SET_MAX, SET_RES or SET_MEM.
In most cases, only the CUR and MEM attribute will be supported for
the Set request. However, this specification does not prevent a
designer from making other attributes programmable.
Supplement: Windows 10 only sends SET_CUR request.

2. Does the old version kernel have the problem on the Ubuntu?
NO. (e.g. linux-5.10)
The problem is introduced by the following modification:
    commit 0356e6283c7177391d144612f4b12986ed5c4f6e
    Author: Ruslan Bilovol <ruslan.bilovol@gmail.com>
    Date:   Mon Jul 12 14:55:29 2021 +0200

        usb: gadget: f_uac1: add volume and mute support

Since Ubuntu doesn't go beyond the UAC1 specification and the problem
is introduced by new version kernel, Why don't we perfect it on 
kernel side?

Thanks
Jing Leng

Re: [PATCH] usb: gadget: f_uac1: add set requests support

[Greg KH]

On Thu, Feb 17, 2022 at 09:42:00AM +0800, Jing Leng wrote:
> Hi Greg KH,
> 
> > So is this a bug in the Host side to not do stuff like this?  Why not
> > fix it there instead?
> > 
> > Where is the requirement that this command must be handled by the
> > device?
> > 
> 
> First we need to clarify two issues.
> 
> 1. Does the Ubuntu go beyond the UAC1 specification?
> No. 
> On page 66 of the UAC1 specification (
> https://www.usb.org/sites/default/files/audio10.pdf):
> The bRequest can be SET_CUR, SET_MIN, SET_MAX, SET_RES or SET_MEM.
> In most cases, only the CUR and MEM attribute will be supported for
> the Set request. However, this specification does not prevent a
> designer from making other attributes programmable.
> Supplement: Windows 10 only sends SET_CUR request.
> 
> 2. Does the old version kernel have the problem on the Ubuntu?
> NO. (e.g. linux-5.10)
> The problem is introduced by the following modification:
>     commit 0356e6283c7177391d144612f4b12986ed5c4f6e
>     Author: Ruslan Bilovol <ruslan.bilovol@gmail.com>
>     Date:   Mon Jul 12 14:55:29 2021 +0200
> 
>         usb: gadget: f_uac1: add volume and mute support

Then please add this commit id as a "Fixes:" tag in the changelog area.

thanks,

greg k-h

[PATCH v2] usb: gadget: f_uac1: add set requests support

[3090101217]

From: Jing Leng <jleng@ambarella.com>

Currently the f_uac1 driver only supports UAC_SET_CUR request.

But when uac1 device is plugged to Ubuntu 20.04 PC, at the stage
of setup, the PC will send UAC_SET_RES request, If the device
doesn't respond to the request, the PC will abort the setup process
and uac1 device can't be recognized on Ubuntu 20.04 PC.

So f_uac1 driver should handle other set requests.

Fixes: 0356e6283c71 ("usb: gadget: f_uac1: add volume and mute support")
Signed-off-by: Jing Leng <jleng@ambarella.com>
---
ChangeLog v1->v2:
- Add "Fixes:" tag in the changelog area
---
 drivers/usb/gadget/function/f_uac1.c | 44 +++++++++++++++++++++++-----
 1 file changed, 36 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/gadget/function/f_uac1.c b/drivers/usb/gadget/function/f_uac1.c
index 03f50643fbba..c9d8ec4fdf22 100644
--- a/drivers/usb/gadget/function/f_uac1.c
+++ b/drivers/usb/gadget/function/f_uac1.c
@@ -589,7 +589,7 @@ in_rq_res(struct usb_function *fn, const struct usb_ctrlrequest *cr)
 }
 
 static void
-out_rq_cur_complete(struct usb_ep *ep, struct usb_request *req)
+out_rq_complete(struct usb_ep *ep, struct usb_request *req)
 {
 	struct g_audio *audio = req->context;
 	struct usb_composite_dev *cdev = audio->func.config->cdev;
@@ -614,9 +614,11 @@ out_rq_cur_complete(struct usb_ep *ep, struct usb_request *req)
 			is_playback = 1;
 
 		if (control_selector == UAC_FU_MUTE) {
-			u8 mute = *(u8 *)req->buf;
+			if (cr->bRequest == UAC_SET_CUR) {
+				u8 mute = *(u8 *)req->buf;
 
-			u_audio_set_mute(audio, is_playback, mute);
+				u_audio_set_mute(audio, is_playback, mute);
+			}
 
 			return;
 		} else if (control_selector == UAC_FU_VOLUME) {
@@ -624,7 +626,34 @@ out_rq_cur_complete(struct usb_ep *ep, struct usb_request *req)
 			s16 volume;
 
 			volume = le16_to_cpu(*c);
-			u_audio_set_volume(audio, is_playback, volume);
+
+			switch (cr->bRequest) {
+			case UAC_SET_CUR:
+				u_audio_set_volume(audio, is_playback, volume);
+				break;
+			case UAC_SET_MIN:
+				if (is_playback)
+					opts->p_volume_min = volume;
+				else
+					opts->c_volume_min = volume;
+				break;
+			case UAC_SET_MAX:
+				if (is_playback)
+					opts->p_volume_max = volume;
+				else
+					opts->c_volume_max = volume;
+				break;
+			case UAC_SET_RES:
+				if (is_playback)
+					opts->p_volume_res = volume;
+				else
+					opts->c_volume_res = volume;
+				break;
+			case UAC_SET_MEM:
+				break;
+			default:
+				break;
+			}
 
 			return;
 		} else {
@@ -643,7 +672,7 @@ out_rq_cur_complete(struct usb_ep *ep, struct usb_request *req)
 }
 
 static int
-out_rq_cur(struct usb_function *fn, const struct usb_ctrlrequest *cr)
+ac_rq_out(struct usb_function *fn, const struct usb_ctrlrequest *cr)
 {
 	struct usb_request *req = fn->config->cdev->req;
 	struct g_audio *audio = func_to_g_audio(fn);
@@ -659,7 +688,7 @@ out_rq_cur(struct usb_function *fn, const struct usb_ctrlrequest *cr)
 			(FUOUT_EN(opts) && (entity_id == USB_OUT_FU_ID))) {
 		memcpy(&uac1->setup_cr, cr, sizeof(*cr));
 		req->context = audio;
-		req->complete = out_rq_cur_complete;
+		req->complete = out_rq_complete;
 
 		return w_length;
 	} else {
@@ -789,8 +818,7 @@ f_audio_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
 		value = audio_get_endpoint_req(f, ctrl);
 		break;
 	case USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE:
-		if (ctrl->bRequest == UAC_SET_CUR)
-			value = out_rq_cur(f, ctrl);
+		value = ac_rq_out(f, ctrl);
 		break;
 	case USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE:
 		value = ac_rq_in(f, ctrl);
-- 
2.17.1