Re: [PATCH] usb: udc: add gadget state kobject uevent

[Felipe Balbi]

Hi,

On Wed, Jul 17, 2013 at 11:37:35AM -0400, Alan Stern wrote:
> On Wed, 17 Jul 2013, Felipe Balbi wrote:
> 
> > On Wed, Jul 17, 2013 at 09:04:54PM +0800, Rong Wang wrote:
> > > Hi Felipe,
> > > 
> > > On Wed, Jul 17, 2013 at 3:57 PM, Felipe Balbi <balbi@xxxxxx> wrote:
> > > > Hi,
> > > >
> > > > On Mon, Jul 15, 2013 at 11:31:17PM -0700, Greg KH wrote:
> > > >> > The question is since we default GADGET, so the g_mass_storage.ko is
> > > >> > installed early but connecting to a host PC
> > > >> > is randomly, But the udev has no idea when a host PC connects our device.
> > > >> >
> > > >> > So we consider it's reasonable to let the udev know the GADGET device state.
> > > >> > Is there any alternative to our question?
> > > >>
> > > >> I thought we already export events for gadget device states, have you
> > > >> looked for them?  I can't dig through the code at the moment, but this
> > > >> seems like a pretty common issue...
> > > >>
> > > >> Felipe, any ideas?
> > > >
> > > > we already expose that in sysfs. IIRC udev can act on sysfs changes,
> > > > no ?
> > > 
> > > I do not know if udev can polling sysfs file content change. I'll study this.
> > > 
> > > But the change is triggered by calling usb_gadget_set_state, and I find
> > > composite framework do not call this. Then we should do this common work
> > > in every udc driver?
> > 
> > yes. Only the UDC driver knows when the controller is moving among those
> > states.
> 
> Not quite.  Only the gadget driver knows when the transition between 
> ADDRESS and CONFIGURED occurs.  This should be added to composite.c.

that's not entirely true :-) See how we handle that in dwc3:

| static int dwc3_ep0_set_config(struct dwc3 *dwc, struct usb_ctrlrequest *ctrl)
| {
| 	enum usb_device_state state = dwc->gadget.state;
| 	u32 cfg;
| 	int ret;
| 	u32 reg;
| 
| 	dwc->start_config_issued = false;
| 	cfg = le16_to_cpu(ctrl->wValue);
| 
| 	switch (state) {
| 	case USB_STATE_DEFAULT:
| 		return -EINVAL;
| 		break;
| 
| 	case USB_STATE_ADDRESS:
| 		ret = dwc3_ep0_delegate_req(dwc, ctrl);
| 		/* if the cfg matches and the cfg is non zero */
| 		if (cfg && (!ret || (ret == USB_GADGET_DELAYED_STATUS))) {
| 			usb_gadget_set_state(&dwc->gadget,
| 					USB_STATE_CONFIGURED);
| 
| 			/*
| 			 * Enable transition to U1/U2 state when
| 			 * nothing is pending from application.
| 			 */
| 			reg = dwc3_readl(dwc->regs, DWC3_DCTL);
| 			reg |= (DWC3_DCTL_ACCEPTU1ENA | DWC3_DCTL_ACCEPTU2ENA);
| 			dwc3_writel(dwc->regs, DWC3_DCTL, reg);
| 
| 			dwc->resize_fifos = true;
| 			dev_dbg(dwc->dev, "resize fifos flag SET\n");
| 		}
| 		break;
| 
| 	case USB_STATE_CONFIGURED:
| 		ret = dwc3_ep0_delegate_req(dwc, ctrl);
| 		if (!cfg)
| 			usb_gadget_set_state(&dwc->gadget,
| 					USB_STATE_ADDRESS);
| 		break;
| 	default:
| 		ret = -EINVAL;
| 	}
| 	return ret;
| }

Also, until other gadget drivers add notifications to the other cases, I
don't think it's wise to add a transition from NOTATTACHED to
CONFIGURED.

But I have one change I'll send for the gadget notifications, I'm just
trying to get a new OMAP5 board to test because the FTDI chip on mine
died and I have no console :-)

-- 
balbi

Attachment: signature.asc
Description: Digital signature