Re: [RFC/PATCH 1/2] usb: gadget: introduce gadget status tracking

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 19 Dec 2011, Felipe Balbi wrote:

> that's useful information to expose to userland.
> 
> NYET-Signed-off-by: Felipe Balbi <balbi@xxxxxx>

> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
> index 7d64f89..6aa9fc2 100644
> --- a/include/linux/usb/gadget.h
> +++ b/include/linux/usb/gadget.h
> @@ -478,6 +478,53 @@ struct usb_gadget_ops {
>  	int	(*stop)(struct usb_gadget_driver *);
>  };
>  
> +enum usb_device_status {
> +	USB_STATUS_IDLE,

IDLE is not the best name.  It suggests the gadget isn't doing
anything, whereas in fact it's supposed to mean the gadget isn't
getting any bus power.  How about USB_STATUS_NOTATTACHED instead?

> +	USB_STATUS_CONNECTED,
> +	USB_STATUS_RESETTING,
> +
> +	USB_STATUS_DEFAULT,
> +	USB_STATUS_ADDRESSED,
> +	USB_STATUS_UNCONFIGURED,
> +	USB_STATUS_CONFIGURED,

As pointed out earlier, there is no difference between ADDRESSED and 
UNCONFIGURED.

Furthermore, UDC drivers don't know whether the gadget is configured or 
not.  This means gadget drivers would have to make the call to set the 
status to or from CONFIGURED, which is awkward.

> +
> +	USB_STATUS_SUSPENDING,
> +	USB_STATUS_SUSPENDED,
> +	USB_STATUS_RESUMING,
> +	USB_STATUS_RESUMED,

As pointed out earlier, there is no RESUMED state.  The gadget has to 
go back to the state it had before resuming -- which could be either 
DEFAULT, ADDRESSED, or CONFIGURED.  Somebody needs to remember this; 
the UDC core is probably the best place.  But it means adding a second 
usb_device_status field to the gadget structure.

> +};

How about USB_STATUS_HOST for use by OTG gadgets?  Or does the entire 
gadget structure get deallocated when the controller goes into host 
mode?

> +
> +static inline const char *
> +usb_device_status_string(enum usb_device_status status)
> +{
> +	switch (status) {
> +	case USB_STATUS_IDLE:
> +		return "idle";
> +	case USB_STATUS_CONNECTED:
> +		return "connected";
> +	case USB_STATUS_RESETTING:
> +		return "resetting";
> +	case USB_STATUS_DEFAULT:
> +		return "default";
> +	case USB_STATUS_ADDRESSED:
> +		return "addressed";
> +	case USB_STATUS_UNCONFIGURED:
> +		return "unconfigured";
> +	case USB_STATUS_CONFIGURED:
> +		return "configured";
> +	case USB_STATUS_SUSPENDING:
> +		return "suspending";
> +	case USB_STATUS_SUSPENDED:
> +		return "suspended";
> +	case USB_STATUS_RESUMING:
> +		return "resuming";
> +	case USB_STATUS_RESUMED:
> +		return "resumed";
> +	default:
> +		return "UNKNOWN";
> +	}
> +}

This should not be an inline routine.  It needs to go into udc-core.c
(which is where it gets used, after all).

Alan Stern


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux