Re: [RFC][PATCH 2/2] usb: gadget: configfs: notify userspace of usb state changes

[Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 4624 bytes --]

Hi,

On Thu, Aug 13, 2015 at 09:42:17PM +0200, Krzysztof Opasiak wrote:
> Hello,
> 
> On 08/13/2015 09:12 PM, Amit Pundir wrote:
> >This is more of an RFC than an actual submission. There are few
> >scattered #ifdefs..#endifs here and there which still need to be
> >taken care of before going for actual submission.
> >
> >Currently there is no way with the upstream ConfigFS gadget to
> >communicate state changes (connected, disconnected, configured), at
> >the gadget level. Instead such state changes are handled function by
> >function independently I presume. This is problematic, because some
> >coordination between the functions, across the state changes, may be
> >desired at the userspace level. Thus to address this issue, this
> >patch send uevents to allow userspace to be notified of these usb
> >state changes, allowing userspace to respond and configure the
> >configfs gadget appropriately.
> >
> >This patch is based on an Android patchset originaly authored by
> >Badhri Jagan Sridharan<Badhri-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>  to send uevent notifications
> >to Android userpace for USB state changes. I've folded his patches
> >together and modified it enough that I don't want him to be blamed for
> >any mistakes I've made condensing his patches down.
> >
> >This patch introduces USB_CONFIGFS_UEVENT Kconfig to handle userspace
> >notifications of usb state changes, and add setup and disconnect
> >functions to intercept the setup requests from the usb_core. It also
> >creates a sysfs device class entry and a device attribute (state) to
> >read and respond to gadget's current state from userspace. As of now
> >this sysfs device class (/sys/class/android_usb) and gadget device
> >(/sys/class/android_usb/android0) with state attribute
> >(/sys/class/android_usb/android0/state) are strictly tied up to
> >facilitate Android userspace requests. But going forward we may want
> >to bring all function devices (hid, printer etc) under a unified usb
> >gadget device class e.g. /sys/class/usb_gadget/g_{func0,func1} etc..
> >
> >Also I think it make sense to add this state attribute to the configfs
> >usb gadget itself i.e. have something like /config/usb_gadget/g1/state
> >to read USB gadget's current state. Since it is going to be consistent
> >throughout all the functions tied up to that gadget.
> >
> >Again this is just an initial RFC, thoughts and feedback would be
> >greatly appreciated.
> >
> >Cc: Mike Lockwood<lockwood-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
> >Cc: Benoit Goby<benoit-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
> >Cc: Colin Cross<ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
> >Cc: Arve Hjønnevåg<arve-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
> >Cc: Peter Oh<poh-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> >Cc: Greg Hackmann<ghackmann-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> >Cc: Badhri Jagan Sridharan<Badhri-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> >Cc: Android Kernel Team<kernel-team-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
> >Cc: Greg Kroah-Hartman<gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
> >Cc: Jonathan Corbet<corbet-T1hC0tSOHrs@public.gmane.org>
> >Cc: Felipe Balbi<balbi-l0cyMroinI0@public.gmane.org>
> >Cc: Andrzej Pietrasiewicz<andrzej.p-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> >Cc: Laurent Pinchart<laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
> >Cc: Yegor Yefremov<yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> >Cc: Philippe Reynes<tremyfr-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >Cc: John Stultz<john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> >Cc: Sumit Semwal<sumit.semwal-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> >Signed-off-by: Amit Pundir<amit.pundir-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> 
> Generally I agree that there should be some way of notifying userspace about

yes, and we already have a sysfs file for that. See udc-core.c:

static void usb_gadget_state_work(struct work_struct *work)
{
	struct usb_gadget *gadget = work_to_gadget(work);
	struct usb_udc *udc = gadget->udc;

	if (udc)
		sysfs_notify(&udc->dev.kobj, NULL, "state");
}

void usb_gadget_set_state(struct usb_gadget *gadget,
		enum usb_device_state state)
{
	gadget->state = state;
	schedule_work(&gadget->work);
}
EXPORT_SYMBOL_GPL(usb_gadget_set_state);

If it's not working for any UDC, it just means the UDC needs to be
patched and if we're missing any state, it means that either the UDC
can't provide that IRQ, or we need to add more states to that
enumeration (which I find unlikely).

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]