On 10/31/2016 3:51 AM, Felipe Balbi wrote:
> By extracting smaller functions from
> dwc3_ep0_handle_feature(), it becomes far easier to
> understand what's going on. Cleanup only, no
> functional changes.
>
> Signed-off-by: Felipe Balbi <[email protected]>
> ---
> drivers/usb/dwc3/ep0.c | 256
> +++++++++++++++++++++++++++++++------------------
> 1 file changed, 163 insertions(+), 93 deletions(-)
>
> diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
> index c562613ccd1a..5e642d65a3b2 100644
> --- a/drivers/usb/dwc3/ep0.c
> +++ b/drivers/usb/dwc3/ep0.c
> @@ -371,126 +371,196 @@ static int dwc3_ep0_handle_status(struct dwc3 *dwc,
> return __dwc3_gadget_ep0_queue(dep, &dwc->ep0_usb_req);
> }
>
> -static int dwc3_ep0_handle_feature(struct dwc3 *dwc,
> +static int dwc3_ep0_handle_u1(struct dwc3 *dwc, enum usb_device_state state,
> + int set)
> +{
> + u32 reg;
> +
> + if (state != USB_STATE_CONFIGURED)
> + return -EINVAL;
> + if ((dwc->speed != DWC3_DSTS_SUPERSPEED) &&
> + (dwc->speed != DWC3_DSTS_SUPERSPEED_PLUS))
> + return -EINVAL;
> +
> + reg = dwc3_readl(dwc->regs, DWC3_DCTL);
> + if (set)
> + reg |= DWC3_DCTL_INITU1ENA;
> + else
> + reg &= ~DWC3_DCTL_INITU1ENA;
> + dwc3_writel(dwc->regs, DWC3_DCTL, reg);
> +
> + return 0;
> +}
> +
> +static int dwc3_ep0_handle_u2(struct dwc3 *dwc, enum usb_device_state state,
> + int set)
> +{
> + u32 reg;
> +
> +
> + if (state != USB_STATE_CONFIGURED)
> + return -EINVAL;
> + if ((dwc->speed != DWC3_DSTS_SUPERSPEED) &&
> + (dwc->speed != DWC3_DSTS_SUPERSPEED_PLUS))
> + return -EINVAL;
> +
> + reg = dwc3_readl(dwc->regs, DWC3_DCTL);
> + if (set)
> + reg |= DWC3_DCTL_INITU2ENA;
> + else
> + reg &= ~DWC3_DCTL_INITU2ENA;
> + dwc3_writel(dwc->regs, DWC3_DCTL, reg);
> +
> + return 0;
> +}
> +
> +static int dwc3_ep0_handle_test(struct dwc3 *dwc, enum usb_device_state
> state,
> + u32 wIndex, int set)
> +{
> + if ((wIndex & 0xff) != 0)
> + return -EINVAL;
> + if (!set)
> + return -EINVAL;
> +
> + switch (wIndex >> 8) {
> + case TEST_J:
> + case TEST_K:
> + case TEST_SE0_NAK:
> + case TEST_PACKET:
> + case TEST_FORCE_EN:
> + dwc->test_mode_nr = wIndex >> 8;
> + dwc->test_mode = true;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int dwc3_ep0_handle_device(struct dwc3 *dwc,
> struct usb_ctrlrequest *ctrl, int set)
> {
> - struct dwc3_ep *dep;
> - u32 recip;
> + enum usb_device_state state;
> u32 wValue;
> u32 wIndex;
> - u32 reg;
> - int ret;
> - enum usb_device_state state;
> + int ret = 0;
>
> wValue = le16_to_cpu(ctrl->wValue);
> wIndex = le16_to_cpu(ctrl->wIndex);
> - recip = ctrl->bRequestType & USB_RECIP_MASK;
> state = dwc->gadget.state;
>
> - switch (recip) {
> - case USB_RECIP_DEVICE:
> + switch (wValue) {
> + case USB_DEVICE_REMOTE_WAKEUP:
> + break;
> + /*
> + * 9.4.1 says only only for SS, in AddressState only for
> + * default control pipe
> + */
> + case USB_DEVICE_U1_ENABLE:
> + ret = dwc3_ep0_handle_u1(dwc, state, set);
> + break;
> + case USB_DEVICE_U2_ENABLE:
> + ret = dwc3_ep0_handle_u2(dwc, state, set);
> + break;
> + case USB_DEVICE_LTM_ENABLE:
> + ret = -EINVAL;
> + break;
> + case USB_DEVICE_TEST_MODE:
> + ret = dwc3_ep0_handle_test(dwc, state, wIndex, set);
Need a break here.
Found with coverity.
Regards,
John
> + default:
> + ret = -EINVAL;
> + }
>
> - switch (wValue) {
> - case USB_DEVICE_REMOTE_WAKEUP:
> - break;
> - /*
> - * 9.4.1 says only only for SS, in AddressState only for
> - * default control pipe
> - */
> - case USB_DEVICE_U1_ENABLE:
> - if (state != USB_STATE_CONFIGURED)
> - return -EINVAL;
> - if ((dwc->speed != DWC3_DSTS_SUPERSPEED) &&
> - (dwc->speed != DWC3_DSTS_SUPERSPEED_PLUS))
> - return -EINVAL;
> + return ret;
> +}
>
> - reg = dwc3_readl(dwc->regs, DWC3_DCTL);
> - if (set)
> - reg |= DWC3_DCTL_INITU1ENA;
> - else
> - reg &= ~DWC3_DCTL_INITU1ENA;
> - dwc3_writel(dwc->regs, DWC3_DCTL, reg);
> - break;
> +static int dwc3_ep0_handle_intf(struct dwc3 *dwc,
> + struct usb_ctrlrequest *ctrl, int set)
> +{
> + enum usb_device_state state;
> + u32 wValue;
> + u32 wIndex;
> + int ret = 0;
>
> - case USB_DEVICE_U2_ENABLE:
> - if (state != USB_STATE_CONFIGURED)
> - return -EINVAL;
> - if ((dwc->speed != DWC3_DSTS_SUPERSPEED) &&
> - (dwc->speed != DWC3_DSTS_SUPERSPEED_PLUS))
> - return -EINVAL;
> + wValue = le16_to_cpu(ctrl->wValue);
> + wIndex = le16_to_cpu(ctrl->wIndex);
> + state = dwc->gadget.state;
>
> - reg = dwc3_readl(dwc->regs, DWC3_DCTL);
> - if (set)
> - reg |= DWC3_DCTL_INITU2ENA;
> - else
> - reg &= ~DWC3_DCTL_INITU2ENA;
> - dwc3_writel(dwc->regs, DWC3_DCTL, reg);
> - break;
> + switch (wValue) {
> + case USB_INTRF_FUNC_SUSPEND:
> + if (wIndex & USB_INTRF_FUNC_SUSPEND_LP)
> + /* XXX enable Low power suspend */
> + ;
> + if (wIndex & USB_INTRF_FUNC_SUSPEND_RW)
> + /* XXX enable remote wakeup */
> + ;
> + break;
> + default:
> + ret = -EINVAL;
> + }
>
> - case USB_DEVICE_LTM_ENABLE:
> + return ret;
> +}
> +
> +static int dwc3_ep0_handle_endpoint(struct dwc3 *dwc,
> + struct usb_ctrlrequest *ctrl, int set)
> +{
> + struct dwc3_ep *dep;
> + enum usb_device_state state;
> + u32 wValue;
> + u32 wIndex;
> + int ret;
> +
> + wValue = le16_to_cpu(ctrl->wValue);
> + wIndex = le16_to_cpu(ctrl->wIndex);
> + state = dwc->gadget.state;
> +
> + switch (wValue) {
> + case USB_ENDPOINT_HALT:
> + dep = dwc3_wIndex_to_dep(dwc, ctrl->wIndex);
> + if (!dep)
> return -EINVAL;
>
> - case USB_DEVICE_TEST_MODE:
> - if ((wIndex & 0xff) != 0)
> - return -EINVAL;
> - if (!set)
> - return -EINVAL;
> -
> - switch (wIndex >> 8) {
> - case TEST_J:
> - case TEST_K:
> - case TEST_SE0_NAK:
> - case TEST_PACKET:
> - case TEST_FORCE_EN:
> - dwc->test_mode_nr = wIndex >> 8;
> - dwc->test_mode = true;
> - break;
> - default:
> - return -EINVAL;
> - }
> + if (set == 0 && (dep->flags & DWC3_EP_WEDGE))
> break;
> - default:
> +
> + ret = __dwc3_gadget_ep_set_halt(dep, set, true);
> + if (ret)
> return -EINVAL;
> - }
> break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int dwc3_ep0_handle_feature(struct dwc3 *dwc,
> + struct usb_ctrlrequest *ctrl, int set)
> +{
> + u32 recip;
> + int ret;
> + enum usb_device_state state;
> +
> + recip = ctrl->bRequestType & USB_RECIP_MASK;
> + state = dwc->gadget.state;
>
> + switch (recip) {
> + case USB_RECIP_DEVICE:
> + ret = dwc3_ep0_handle_device(dwc, ctrl, set);
> + break;
> case USB_RECIP_INTERFACE:
> - switch (wValue) {
> - case USB_INTRF_FUNC_SUSPEND:
> - if (wIndex & USB_INTRF_FUNC_SUSPEND_LP)
> - /* XXX enable Low power suspend */
> - ;
> - if (wIndex & USB_INTRF_FUNC_SUSPEND_RW)
> - /* XXX enable remote wakeup */
> - ;
> - break;
> - default:
> - return -EINVAL;
> - }
> + ret = dwc3_ep0_handle_intf(dwc, ctrl, set);
> break;
> -
> case USB_RECIP_ENDPOINT:
> - switch (wValue) {
> - case USB_ENDPOINT_HALT:
> - dep = dwc3_wIndex_to_dep(dwc, ctrl->wIndex);
> - if (!dep)
> - return -EINVAL;
> - if (set == 0 && (dep->flags & DWC3_EP_WEDGE))
> - break;
> - ret = __dwc3_gadget_ep_set_halt(dep, set, true);
> - if (ret)
> - return -EINVAL;
> - break;
> - default:
> - return -EINVAL;
> - }
> + ret = dwc3_ep0_handle_endpoint(dwc, ctrl, set);
> break;
> -
> default:
> - return -EINVAL;
> + ret = -EINVAL;
> }
>
> - return 0;
> + return ret;
> }
>
> static int dwc3_ep0_set_address(struct dwc3 *dwc, struct usb_ctrlrequest
> *ctrl)
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
