linux-kernel - Re: [PATCH v2] usb: gadget: f_uac2: uevent changes for uac2

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Tue, 8 Aug 2023 10:29:47 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Akash Kumar <quic_akakum@...cinc.com>
Cc:     Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        Jing Leng <jleng@...arella.com>,
        Felipe Balbi <balbi@...nel.org>,
        Pratham Pratap  <quic_ppratap@...cinc.com>,
        Jack Pham <quic_jackp@...cinc.com>, linux-usb@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] usb: gadget: f_uac2: uevent changes for uac2

On Mon, Jul 24, 2023 at 12:35:38AM +0530, Akash Kumar wrote:
> Adding uevent 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.

How do you keep the events in sync then?  If you get a uevent, and then
wait a while before reading sysfs, how do you know that a different
uevent didn't happen inbetween?

> /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@...cinc.com>
> ---
>  .../ABI/testing/configfs-usb-gadget-uac2      |  4 ++
>  drivers/usb/gadget/function/f_uac2.c          | 47 +++++++++++++++++++
>  drivers/usb/gadget/function/u_uac2.h          |  5 ++
>  3 files changed, 56 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uac2 b/Documentation/ABI/testing/configfs-usb-gadget-uac2
> index 2bfdd4efa9bd..196c6921a163 100644
> --- a/Documentation/ABI/testing/configfs-usb-gadget-uac2
> +++ b/Documentation/ABI/testing/configfs-usb-gadget-uac2
> @@ -10,3 +10,7 @@ Description:
>  		p_chmask - playback channel mask
>  		p_srate - playback sampling rate
>  		p_ssize - playback sample size (bytes)
> +		c_status - usb uac2 audio capture state
> +			   (0: capture stopped, 1: capture started)
> +		p_status - usb uac2 audio playback state
> +			   (0: playback stopped, 1: playback started)
> diff --git a/drivers/usb/gadget/function/f_uac2.c b/drivers/usb/gadget/function/f_uac2.c
> index db2d4980cb35..f1f7631e9380 100644
> --- a/drivers/usb/gadget/function/f_uac2.c
> +++ b/drivers/usb/gadget/function/f_uac2.c
> @@ -739,6 +739,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) {
> @@ -762,6 +764,8 @@ 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;
> +		schedule_work(&audio_opts->work);
>  	} else if (intf == uac2->as_in_intf) {
>  		uac2->as_in_alt = alt;
>  
> @@ -769,6 +773,8 @@ 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;
> +		schedule_work(&audio_opts->work);
>  	} else {
>  		dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
>  		return -EINVAL;
> @@ -801,11 +807,16 @@ 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;
>  	u_audio_stop_capture(&uac2->g_audio);
>  	u_audio_stop_playback(&uac2->g_audio);
> +	schedule_work(&audio_opts->work);
>  }
>  
>  static int
> @@ -1036,6 +1047,25 @@ UAC2_ATTRIBUTE(c_srate);
>  UAC2_ATTRIBUTE(c_ssize);
>  UAC2_ATTRIBUTE(req_number);
>  
> +#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,
> @@ -1044,6 +1074,8 @@ static struct configfs_attribute *f_uac2_attrs[] = {
>  	&f_uac2_opts_attr_c_srate,
>  	&f_uac2_opts_attr_c_ssize,
>  	&f_uac2_opts_attr_req_number,
> +	&f_uac2_opts_attr_c_status,
> +	&f_uac2_opts_attr_p_status,

You didn't document these new attributes either in Documentation/ABI/

Also, what userspace tool is going to use these?

thanks,

greg k-h