Re: [PATCH v4 2/3] usb: gadget: introduce gadget state tracking

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

 



Hi,

On Fri, Jan 25, 2013 at 11:29:57AM +0200, Felipe Balbi wrote:
> Hi,
> 
> On Fri, Jan 25, 2013 at 11:12:41AM +0200, Felipe Balbi wrote:
> > that's useful information to expose to userland.
> > 
> > Signed-off-by: Felipe Balbi <balbi@xxxxxx>
> > ---
> >  drivers/usb/gadget/udc-core.c | 23 +++++++++++++++++++++++
> >  include/linux/usb/gadget.h    |  9 +++++++++
> >  2 files changed, 32 insertions(+)
> > 
> > diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c
> > index 40b1d88..8a1eeb2 100644
> > --- a/drivers/usb/gadget/udc-core.c
> > +++ b/drivers/usb/gadget/udc-core.c
> > @@ -101,6 +101,16 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
> >  
> >  /* ------------------------------------------------------------------------- */
> >  
> > +void usb_gadget_set_state(struct usb_gadget *gadget,
> > +		enum usb_device_state state)
> > +{
> > +	gadget->state = state;
> > +	sysfs_notify(&gadget->dev.kobj, NULL, "status");
> > +}
> > +EXPORT_SYMBOL_GPL(usb_gadget_set_state);
> 
> I guess it would be good to have a:
> 
> enum usb_gadget_state usb_gadget_get_state(struct usb_gadget *gadget)
> {
> 	return gadget->state;
> }
> 
> right ?? At least dwc3 can make use of it.

We could use it like this:

From c6c90377581bfcd55fc8a4bf724e0c9d5872944a Mon Sep 17 00:00:00 2001
From: Felipe Balbi <balbi@xxxxxx>
Date: Fri, 25 Jan 2013 11:28:19 +0200
Subject: [PATCH] usb: dwc3: remove our homebrew state mechanism

We can reuse the generic implementation via
our struct usb_gadget.

Signed-off-by: Felipe Balbi <balbi@xxxxxx>
---
 drivers/usb/dwc3/core.h |  7 -------
 drivers/usb/dwc3/ep0.c  | 34 +++++++++++++++++-----------------
 2 files changed, 17 insertions(+), 24 deletions(-)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 4999563..11a77f0 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -487,12 +487,6 @@ enum dwc3_link_state {
 	DWC3_LINK_STATE_MASK		= 0x0f,
 };
 
-enum dwc3_device_state {
-	DWC3_DEFAULT_STATE,
-	DWC3_ADDRESS_STATE,
-	DWC3_CONFIGURED_STATE,
-};
-
 /* TRB Length, PCM and Status */
 #define DWC3_TRB_SIZE_MASK	(0x00ffffff)
 #define DWC3_TRB_SIZE_LENGTH(n)	((n) & DWC3_TRB_SIZE_MASK)
@@ -707,7 +701,6 @@ struct dwc3 {
 	enum dwc3_ep0_next	ep0_next_event;
 	enum dwc3_ep0_state	ep0state;
 	enum dwc3_link_state	link_state;
-	enum dwc3_device_state	dev_state;
 
 	u16			isoch_delay;
 	u16			u2sel;
diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index ec4b563..45847ce 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -394,10 +394,13 @@ static int dwc3_ep0_handle_feature(struct dwc3 *dwc,
 	u32			wIndex;
 	u32			reg;
 	int			ret;
+	enum usb_device_state	state;
 
 	wValue = le16_to_cpu(ctrl->wValue);
 	wIndex = le16_to_cpu(ctrl->wIndex);
 	recip = ctrl->bRequestType & USB_RECIP_MASK;
+	state = usb_gadget_get_state(&dwc->gadget);
+
 	switch (recip) {
 	case USB_RECIP_DEVICE:
 
@@ -409,7 +412,7 @@ static int dwc3_ep0_handle_feature(struct dwc3 *dwc,
 		 * default control pipe
 		 */
 		case USB_DEVICE_U1_ENABLE:
-			if (dwc->dev_state != DWC3_CONFIGURED_STATE)
+			if (state != USB_STATE_CONFIGURED)
 				return -EINVAL;
 			if (dwc->speed != DWC3_DSTS_SUPERSPEED)
 				return -EINVAL;
@@ -423,7 +426,7 @@ static int dwc3_ep0_handle_feature(struct dwc3 *dwc,
 			break;
 
 		case USB_DEVICE_U2_ENABLE:
-			if (dwc->dev_state != DWC3_CONFIGURED_STATE)
+			if (state != USB_STATE_CONFIGURED)
 				return -EINVAL;
 			if (dwc->speed != DWC3_DSTS_SUPERSPEED)
 				return -EINVAL;
@@ -493,6 +496,7 @@ static int dwc3_ep0_handle_feature(struct dwc3 *dwc,
 
 static int dwc3_ep0_set_address(struct dwc3 *dwc, struct usb_ctrlrequest *ctrl)
 {
+	enum usb_device_state state = usb_gadget_get_state(&dwc->gadget);
 	u32 addr;
 	u32 reg;
 
@@ -502,7 +506,7 @@ static int dwc3_ep0_set_address(struct dwc3 *dwc, struct usb_ctrlrequest *ctrl)
 		return -EINVAL;
 	}
 
-	if (dwc->dev_state == DWC3_CONFIGURED_STATE) {
+	if (state == USB_STATE_CONFIGURED) {
 		dev_dbg(dwc->dev, "trying to set address when configured\n");
 		return -EINVAL;
 	}
@@ -512,13 +516,10 @@ static int dwc3_ep0_set_address(struct dwc3 *dwc, struct usb_ctrlrequest *ctrl)
 	reg |= DWC3_DCFG_DEVADDR(addr);
 	dwc3_writel(dwc->regs, DWC3_DCFG, reg);
 
-	if (addr) {
-		dwc->dev_state = DWC3_ADDRESS_STATE;
+	if (addr)
 		usb_gadget_set_state(&dwc->gadget, USB_STATE_ADDRESS);
-	} else {
-		dwc->dev_state = DWC3_DEFAULT_STATE;
+	else
 		usb_gadget_set_state(&dwc->gadget, USB_STATE_DEFAULT);
-	}
 
 	return 0;
 }
@@ -535,6 +536,7 @@ static int dwc3_ep0_delegate_req(struct dwc3 *dwc, struct usb_ctrlrequest *ctrl)
 
 static int dwc3_ep0_set_config(struct dwc3 *dwc, struct usb_ctrlrequest *ctrl)
 {
+	enum usb_device_state state = usb_gadget_get_state(&dwc->gadget);
 	u32 cfg;
 	int ret;
 	u32 reg;
@@ -542,16 +544,15 @@ static int dwc3_ep0_set_config(struct dwc3 *dwc, struct usb_ctrlrequest *ctrl)
 	dwc->start_config_issued = false;
 	cfg = le16_to_cpu(ctrl->wValue);
 
-	switch (dwc->dev_state) {
-	case DWC3_DEFAULT_STATE:
+	switch (state) {
+	case USB_STATE_DEFAULT:
 		return -EINVAL;
 		break;
 
-	case DWC3_ADDRESS_STATE:
+	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))) {
-			dwc->dev_state = DWC3_CONFIGURED_STATE;
 			usb_gadget_set_state(&dwc->gadget,
 					USB_STATE_CONFIGURED);
 
@@ -568,13 +569,11 @@ static int dwc3_ep0_set_config(struct dwc3 *dwc, struct usb_ctrlrequest *ctrl)
 		}
 		break;
 
-	case DWC3_CONFIGURED_STATE:
+	case USB_STATE_CONFIGURED:
 		ret = dwc3_ep0_delegate_req(dwc, ctrl);
-		if (!cfg) {
-			dwc->dev_state = DWC3_ADDRESS_STATE;
+		if (!cfg)
 			usb_gadget_set_state(&dwc->gadget,
 					USB_STATE_ADDRESS);
-		}
 		break;
 	default:
 		ret = -EINVAL;
@@ -629,10 +628,11 @@ static void dwc3_ep0_set_sel_cmpl(struct usb_ep *ep, struct usb_request *req)
 static int dwc3_ep0_set_sel(struct dwc3 *dwc, struct usb_ctrlrequest *ctrl)
 {
 	struct dwc3_ep	*dep;
+	enum usb_device_state state = usb_gadget_get_state(&dwc->gadget);
 	u16		wLength;
 	u16		wValue;
 
-	if (dwc->dev_state == DWC3_DEFAULT_STATE)
+	if (state == USB_STATE_DEFAULT)
 		return -EINVAL;
 
 	wValue = le16_to_cpu(ctrl->wValue);
-- 
1.8.1.rc1.5.g7e0651a


-- 
balbi

Attachment: signature.asc
Description: Digital signature


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

  Powered by Linux