2023-05-08 12:29:04

by AKASH KUMAR

[permalink] [raw]
Subject: [PATCH] usb: gadget: f_uac2: uevent changes for uac2

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 <[email protected]>
---
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

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
--
2.17.1


2023-05-08 12:29:37

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] usb: gadget: f_uac2: uevent changes for uac2

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 <[email protected]>

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

2023-05-08 15:27:26

by AKASH KUMAR

[permalink] [raw]
Subject: Re: [PATCH] usb: gadget: f_uac2: uevent changes for uac2

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 <[email protected]>
> 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