diff mbox series

usb: gadget: f_uac2: uevent changes for uac2

Message ID 20230508120535.31472-1-quic_akakum@quicinc.com (mailing list archive)
State New, archived
Headers show
Series usb: gadget: f_uac2: uevent changes for uac2 | expand

Commit Message

AKASH KUMAR May 8, 2023, 12:05 p.m. UTC
Adding uvent from usb audio gadget driver for
uac2 playback/capture events, which userspace reads
and later reads sysfs entry to know if playback
or capture has stopped or started by host
application.

/config/usb_gadget/g1/functions/uac2.0 # cat c_status
1  --> capture started
0  --> capture stopped
/config/usb_gadget/g1/functions/uac2.0 # cat p_status
1 --> playback started
0 --> playback stopped

Signed-off-by: AKASH KUMAR <quic_akakum@quicinc.com>
---
 drivers/usb/gadget/function/f_uac2.c | 57 ++++++++++++++++++++++++++++
 drivers/usb/gadget/function/u_uac2.h |  7 ++++
 2 files changed, 64 insertions(+)

Comments

Greg KH May 8, 2023, 12:14 p.m. UTC | #1
On Mon, May 08, 2023 at 05:35:35PM +0530, AKASH KUMAR wrote:
> Adding uvent from usb audio gadget driver for

What is a "uvent"?

> uac2 playback/capture events, which userspace reads
> and later reads sysfs entry to know if playback
> or capture has stopped or started by host
> application.

Please use your full 72 columns please.

> 
> /config/usb_gadget/g1/functions/uac2.0 # cat c_status
> 1  --> capture started
> 0  --> capture stopped
> /config/usb_gadget/g1/functions/uac2.0 # cat p_status
> 1 --> playback started
> 0 --> playback stopped

You need to document these new files in Documentation/ABI/ right?

> 
> Signed-off-by: AKASH KUMAR <quic_akakum@quicinc.com>

No need to UPPER CASE your name :)

> ---
>  drivers/usb/gadget/function/f_uac2.c | 57 ++++++++++++++++++++++++++++
>  drivers/usb/gadget/function/u_uac2.h |  7 ++++
>  2 files changed, 64 insertions(+)
> 
> diff --git a/drivers/usb/gadget/function/f_uac2.c b/drivers/usb/gadget/function/f_uac2.c
> index 0219cd79493a..d0a5fa6b49b8 100644
> --- a/drivers/usb/gadget/function/f_uac2.c
> +++ b/drivers/usb/gadget/function/f_uac2.c
> @@ -1423,6 +1423,8 @@ afunc_set_alt(struct usb_function *fn, unsigned intf, unsigned alt)
>  	struct usb_gadget *gadget = cdev->gadget;
>  	struct device *dev = &gadget->dev;
>  	int ret = 0;
> +	struct f_uac2_opts *audio_opts =
> +		container_of(fn->fi, struct f_uac2_opts, func_inst);
>  
>  	/* No i/f has more than 2 alt settings */
>  	if (alt > 1) {
> @@ -1454,6 +1456,7 @@ afunc_set_alt(struct usb_function *fn, unsigned intf, unsigned alt)
>  			ret = u_audio_start_capture(&uac2->g_audio);
>  		else
>  			u_audio_stop_capture(&uac2->g_audio);
> +		audio_opts->c_status = alt;
>  	} else if (intf == uac2->as_in_intf) {
>  		uac2->as_in_alt = alt;
>  
> @@ -1461,10 +1464,14 @@ afunc_set_alt(struct usb_function *fn, unsigned intf, unsigned alt)
>  			ret = u_audio_start_playback(&uac2->g_audio);
>  		else
>  			u_audio_stop_playback(&uac2->g_audio);
> +		audio_opts->p_status = alt;
>  	} else {
>  		dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
>  		return -EINVAL;
>  	}
> +#ifdef CONFIG_USB_CONFIGFS_UEVENT
> +	schedule_work(&audio_opts->work);
> +#endif

Why the #ifdef?  Please never do that in .c files if at all possible.

>  
>  	return ret;
>  }
> @@ -1493,9 +1500,17 @@ static void
>  afunc_disable(struct usb_function *fn)
>  {
>  	struct f_uac2 *uac2 = func_to_uac2(fn);
> +	struct f_uac2_opts *audio_opts =
> +		container_of(fn->fi, struct f_uac2_opts, func_inst);
>  
>  	uac2->as_in_alt = 0;
>  	uac2->as_out_alt = 0;
> +	audio_opts->p_status = 0;//alt;
> +	audio_opts->c_status = 0; //alt;
> +
> +#ifdef CONFIG_USB_CONFIGFS_UEVENT
> +	schedule_work(&audio_opts->work);
> +#endif

Same here and elsewhere in this patch.

thanks,

greg k-h
AKASH KUMAR May 8, 2023, 3:16 p.m. UTC | #2
Thanks Greg for review, will fix the issues in version 2 patchset.

Thanks,

Akash

On 5/8/2023 5:44 PM, Greg Kroah-Hartman wrote:
> On Mon, May 08, 2023 at 05:35:35PM +0530, AKASH KUMAR wrote:
>> Adding uvent from usb audio gadget driver for
> What is a "uvent"?
>
>> uac2 playback/capture events, which userspace reads
>> and later reads sysfs entry to know if playback
>> or capture has stopped or started by host
>> application.
> Please use your full 72 columns please.
>
>> /config/usb_gadget/g1/functions/uac2.0 # cat c_status
>> 1  --> capture started
>> 0  --> capture stopped
>> /config/usb_gadget/g1/functions/uac2.0 # cat p_status
>> 1 --> playback started
>> 0 --> playback stopped
> You need to document these new files in Documentation/ABI/ right?
>
>> Signed-off-by: AKASH KUMAR <quic_akakum@quicinc.com>
> No need to UPPER CASE your name :)
>
>> ---
>>   drivers/usb/gadget/function/f_uac2.c | 57 ++++++++++++++++++++++++++++
>>   drivers/usb/gadget/function/u_uac2.h |  7 ++++
>>   2 files changed, 64 insertions(+)
>>
>> diff --git a/drivers/usb/gadget/function/f_uac2.c b/drivers/usb/gadget/function/f_uac2.c
>> index 0219cd79493a..d0a5fa6b49b8 100644
>> --- a/drivers/usb/gadget/function/f_uac2.c
>> +++ b/drivers/usb/gadget/function/f_uac2.c
>> @@ -1423,6 +1423,8 @@ afunc_set_alt(struct usb_function *fn, unsigned intf, unsigned alt)
>>   	struct usb_gadget *gadget = cdev->gadget;
>>   	struct device *dev = &gadget->dev;
>>   	int ret = 0;
>> +	struct f_uac2_opts *audio_opts =
>> +		container_of(fn->fi, struct f_uac2_opts, func_inst);
>>   
>>   	/* No i/f has more than 2 alt settings */
>>   	if (alt > 1) {
>> @@ -1454,6 +1456,7 @@ afunc_set_alt(struct usb_function *fn, unsigned intf, unsigned alt)
>>   			ret = u_audio_start_capture(&uac2->g_audio);
>>   		else
>>   			u_audio_stop_capture(&uac2->g_audio);
>> +		audio_opts->c_status = alt;
>>   	} else if (intf == uac2->as_in_intf) {
>>   		uac2->as_in_alt = alt;
>>   
>> @@ -1461,10 +1464,14 @@ afunc_set_alt(struct usb_function *fn, unsigned intf, unsigned alt)
>>   			ret = u_audio_start_playback(&uac2->g_audio);
>>   		else
>>   			u_audio_stop_playback(&uac2->g_audio);
>> +		audio_opts->p_status = alt;
>>   	} else {
>>   		dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
>>   		return -EINVAL;
>>   	}
>> +#ifdef CONFIG_USB_CONFIGFS_UEVENT
>> +	schedule_work(&audio_opts->work);
>> +#endif
> Why the #ifdef?  Please never do that in .c files if at all possible.
>
>>   
>>   	return ret;
>>   }
>> @@ -1493,9 +1500,17 @@ static void
>>   afunc_disable(struct usb_function *fn)
>>   {
>>   	struct f_uac2 *uac2 = func_to_uac2(fn);
>> +	struct f_uac2_opts *audio_opts =
>> +		container_of(fn->fi, struct f_uac2_opts, func_inst);
>>   
>>   	uac2->as_in_alt = 0;
>>   	uac2->as_out_alt = 0;
>> +	audio_opts->p_status = 0;//alt;
>> +	audio_opts->c_status = 0; //alt;
>> +
>> +#ifdef CONFIG_USB_CONFIGFS_UEVENT
>> +	schedule_work(&audio_opts->work);
>> +#endif
> Same here and elsewhere in this patch.
>
> thanks,
>
> greg k-h
diff mbox series

Patch

diff --git a/drivers/usb/gadget/function/f_uac2.c b/drivers/usb/gadget/function/f_uac2.c
index 0219cd79493a..d0a5fa6b49b8 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -1423,6 +1423,8 @@  afunc_set_alt(struct usb_function *fn, unsigned intf, unsigned alt)
 	struct usb_gadget *gadget = cdev->gadget;
 	struct device *dev = &gadget->dev;
 	int ret = 0;
+	struct f_uac2_opts *audio_opts =
+		container_of(fn->fi, struct f_uac2_opts, func_inst);
 
 	/* No i/f has more than 2 alt settings */
 	if (alt > 1) {
@@ -1454,6 +1456,7 @@  afunc_set_alt(struct usb_function *fn, unsigned intf, unsigned alt)
 			ret = u_audio_start_capture(&uac2->g_audio);
 		else
 			u_audio_stop_capture(&uac2->g_audio);
+		audio_opts->c_status = alt;
 	} else if (intf == uac2->as_in_intf) {
 		uac2->as_in_alt = alt;
 
@@ -1461,10 +1464,14 @@  afunc_set_alt(struct usb_function *fn, unsigned intf, unsigned alt)
 			ret = u_audio_start_playback(&uac2->g_audio);
 		else
 			u_audio_stop_playback(&uac2->g_audio);
+		audio_opts->p_status = alt;
 	} else {
 		dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
 		return -EINVAL;
 	}
+#ifdef CONFIG_USB_CONFIGFS_UEVENT
+	schedule_work(&audio_opts->work);
+#endif
 
 	return ret;
 }
@@ -1493,9 +1500,17 @@  static void
 afunc_disable(struct usb_function *fn)
 {
 	struct f_uac2 *uac2 = func_to_uac2(fn);
+	struct f_uac2_opts *audio_opts =
+		container_of(fn->fi, struct f_uac2_opts, func_inst);
 
 	uac2->as_in_alt = 0;
 	uac2->as_out_alt = 0;
+	audio_opts->p_status = 0;//alt;
+	audio_opts->c_status = 0; //alt;
+
+#ifdef CONFIG_USB_CONFIGFS_UEVENT
+	schedule_work(&audio_opts->work);
+#endif
 	u_audio_stop_capture(&uac2->g_audio);
 	u_audio_stop_playback(&uac2->g_audio);
 	if (uac2->int_ep)
@@ -2095,6 +2110,25 @@  UAC2_ATTRIBUTE(s16, c_volume_res);
 UAC2_ATTRIBUTE(u32, fb_max);
 UAC2_ATTRIBUTE_STRING(function_name);
 
+#define UAC2_ATTRIBUTE_RO(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;                                                     \
+	\
+	mutex_lock(&opts->lock);                                        \
+	result = scnprintf(page, PAGE_SIZE, "%u\n", opts->name);        \
+	mutex_unlock(&opts->lock);                                      \
+	\
+	return result;                                                  \
+}                                                                       \
+CONFIGFS_ATTR_RO(f_uac2_opts_, name)
+
+UAC2_ATTRIBUTE_RO(c_status);
+UAC2_ATTRIBUTE_RO(p_status);
+
 static struct configfs_attribute *f_uac2_attrs[] = {
 	&f_uac2_opts_attr_p_chmask,
 	&f_uac2_opts_attr_p_srate,
@@ -2119,6 +2153,8 @@  static struct configfs_attribute *f_uac2_attrs[] = {
 	&f_uac2_opts_attr_c_volume_min,
 	&f_uac2_opts_attr_c_volume_max,
 	&f_uac2_opts_attr_c_volume_res,
+	&f_uac2_opts_attr_c_status,
+	&f_uac2_opts_attr_p_status,
 
 	&f_uac2_opts_attr_function_name,
 
@@ -2136,9 +2172,26 @@  static void afunc_free_inst(struct usb_function_instance *f)
 	struct f_uac2_opts *opts;
 
 	opts = container_of(f, struct f_uac2_opts, func_inst);
+#ifdef CONFIG_USB_CONFIGFS_UEVENT
+	device_destroy(opts->device->class, opts->device->devt);
+	cancel_work_sync(&opts->work);
+#endif
 	kfree(opts);
 }
 
+#ifdef CONFIG_USB_CONFIGFS_UEVENT
+static void f_uac2_audio_status_change_work(struct work_struct *data)
+{
+	struct f_uac2_opts *audio_opts =
+		container_of(data, struct f_uac2_opts, work);
+	char *envp[2] = { "UAC2_STATE=Changed", NULL };
+
+	kobject_uevent_env(&audio_opts->device->kobj,
+			KOBJ_CHANGE, envp);
+	pr_debug("%s:uac2 state changed\n", __func__);
+}
+#endif
+
 static struct usb_function_instance *afunc_alloc_inst(void)
 {
 	struct f_uac2_opts *opts;
@@ -2180,6 +2233,10 @@  static struct usb_function_instance *afunc_alloc_inst(void)
 
 	snprintf(opts->function_name, sizeof(opts->function_name), "Source/Sink");
 
+#ifdef CONFIG_USB_CONFIGFS_UEVENT
+	INIT_WORK(&opts->work, f_uac2_audio_status_change_work);
+	opts->device = create_function_device("f_uac2");
+#endif
 	return &opts->func_inst;
 }
 
diff --git a/drivers/usb/gadget/function/u_uac2.h b/drivers/usb/gadget/function/u_uac2.h
index 0510c9bad58d..e05536c13c45 100644
--- a/drivers/usb/gadget/function/u_uac2.h
+++ b/drivers/usb/gadget/function/u_uac2.h
@@ -67,6 +67,13 @@  struct f_uac2_opts {
 
 	struct mutex			lock;
 	int				refcnt;
+	u8				c_status;
+	u8				p_status;
+	struct device                   *device;
+#ifdef CONFIG_USB_CONFIGFS_UEVENT
+	struct work_struct              work;
+#endif
 };
 
+extern struct device *create_function_device(char *name);
 #endif