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

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:

quoted

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[off-list ref]  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<redacted>
Cc: Benoit Goby<redacted>
Cc: Colin Cross<redacted>
Cc: Arve Hjønnevåg<redacted>
Cc: Peter Oh<poh-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
Cc: Greg Hackmann<redacted>
Cc: Badhri Jagan Sridharan<redacted>
Cc: Android Kernel Team<redacted>
Cc: Greg Kroah-Hartman<redacted>
Cc: Jonathan Corbet<redacted>
Cc: Felipe Balbi<redacted>
Cc: Andrzej Pietrasiewicz<redacted>
Cc: Laurent Pinchart<laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
Cc: Yegor Yefremov<yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Cc: Philippe Reynes<redacted>
Cc: John Stultz<redacted>
Cc: Sumit Semwal<redacted>
Signed-off-by: Amit Pundir<redacted>

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