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

[AKASH KUMAR]

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@xxxxxxxxxxx>
> 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