From 40d829fb2ec636b6b4b0cc95e2546ab9aca04cc9 Mon Sep 17 00:00:00 2001 From: Manu Gautam Date: Wed, 19 Jul 2017 17:07:10 +0530 Subject: usb: dwc3: gadget: Correct ISOC DATA PIDs for short packets The PIDs for Isochronous data transfers are incorrect for high bandwidth IN endpoints when the request length is less than EP wMaxPacketSize. As per spec correct PIDs for ISOC data transfers are: 1) For request length <= maxpacket - DATA0, 2) For maxpacket < length <= (2 * maxpacket) - DATA1, DATA0 3) For (2 * maxpacket) < length <= (3 * maxpacket) - DATA2, DATA1, DATA0. But driver always sets PCM fields based on wMaxPacketSize due to which DATA2 happens even for small requests. Fix this by setting the PCM field of trb->size depending on request length rather than fixing it to the value depending on wMaxPacketSize. Ideally it shouldn't give any issues as dwc3 will send 0-length packet for next IN token if host sends (even after receiving a short packet). Windows seems to ignore this but with MacOS frame loss observed when using f_uvc. Signed-off-by: Manu Gautam Signed-off-by: Felipe Balbi --- drivers/usb/dwc3/gadget.c | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) (limited to 'drivers/usb') diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 6b299c7b7656..f064f1549333 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -896,9 +896,40 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep, struct dwc3_trb *trb, if (!node) { trb->ctrl = DWC3_TRBCTL_ISOCHRONOUS_FIRST; + /* + * USB Specification 2.0 Section 5.9.2 states that: "If + * there is only a single transaction in the microframe, + * only a DATA0 data packet PID is used. If there are + * two transactions per microframe, DATA1 is used for + * the first transaction data packet and DATA0 is used + * for the second transaction data packet. If there are + * three transactions per microframe, DATA2 is used for + * the first transaction data packet, DATA1 is used for + * the second, and DATA0 is used for the third." + * + * IOW, we should satisfy the following cases: + * + * 1) length <= maxpacket + * - DATA0 + * + * 2) maxpacket < length <= (2 * maxpacket) + * - DATA1, DATA0 + * + * 3) (2 * maxpacket) < length <= (3 * maxpacket) + * - DATA2, DATA1, DATA0 + */ if (speed == USB_SPEED_HIGH) { struct usb_ep *ep = &dep->endpoint; - trb->size |= DWC3_TRB_SIZE_PCM1(ep->mult - 1); + unsigned int mult = ep->mult - 1; + unsigned int maxp = usb_endpoint_maxp(ep->desc); + + if (length <= (2 * maxp)) + mult--; + + if (length <= maxp) + mult--; + + trb->size |= DWC3_TRB_SIZE_PCM1(mult); } } else { trb->ctrl = DWC3_TRBCTL_ISOCHRONOUS; -- cgit v1.2.3-55-g7522 From aca5b9ebd096039657417c321a9252c696b359c2 Mon Sep 17 00:00:00 2001 From: Yoshihiro Shimoda Date: Wed, 2 Aug 2017 21:06:35 +0900 Subject: usb: gadget: udc: renesas_usb3: Fix usb_gadget_giveback_request() calling According to the gadget.h, a "complete" function will always be called with interrupts disabled. However, sometimes usb3_request_done() function is called with interrupts enabled. So, this function should be held by spin_lock_irqsave() to disable interruption. Also, this driver has to call spin_unlock() to avoid spinlock recursion by this driver before calling usb_gadget_giveback_request(). Reported-by: Kazuya Mizuguchi Tested-by: Kazuya Mizuguchi Fixes: 746bfe63bba3 ("usb: gadget: renesas_usb3: add support for Renesas USB3.0 peripheral controller") Cc: # v4.5+ Signed-off-by: Yoshihiro Shimoda Signed-off-by: Felipe Balbi --- drivers/usb/gadget/udc/renesas_usb3.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) (limited to 'drivers/usb') diff --git a/drivers/usb/gadget/udc/renesas_usb3.c b/drivers/usb/gadget/udc/renesas_usb3.c index 62dc9c7798e7..e1de8fe599a3 100644 --- a/drivers/usb/gadget/udc/renesas_usb3.c +++ b/drivers/usb/gadget/udc/renesas_usb3.c @@ -838,21 +838,32 @@ static struct renesas_usb3_request *usb3_get_request(struct renesas_usb3_ep return usb3_req; } -static void usb3_request_done(struct renesas_usb3_ep *usb3_ep, - struct renesas_usb3_request *usb3_req, int status) +static void __usb3_request_done(struct renesas_usb3_ep *usb3_ep, + struct renesas_usb3_request *usb3_req, + int status) { struct renesas_usb3 *usb3 = usb3_ep_to_usb3(usb3_ep); - unsigned long flags; dev_dbg(usb3_to_dev(usb3), "giveback: ep%2d, %u, %u, %d\n", usb3_ep->num, usb3_req->req.length, usb3_req->req.actual, status); usb3_req->req.status = status; - spin_lock_irqsave(&usb3->lock, flags); usb3_ep->started = false; list_del_init(&usb3_req->queue); - spin_unlock_irqrestore(&usb3->lock, flags); + spin_unlock(&usb3->lock); usb_gadget_giveback_request(&usb3_ep->ep, &usb3_req->req); + spin_lock(&usb3->lock); +} + +static void usb3_request_done(struct renesas_usb3_ep *usb3_ep, + struct renesas_usb3_request *usb3_req, int status) +{ + struct renesas_usb3 *usb3 = usb3_ep_to_usb3(usb3_ep); + unsigned long flags; + + spin_lock_irqsave(&usb3->lock, flags); + __usb3_request_done(usb3_ep, usb3_req, status); + spin_unlock_irqrestore(&usb3->lock, flags); } static void usb3_irq_epc_pipe0_status_end(struct renesas_usb3 *usb3) -- cgit v1.2.3-55-g7522 From 5a8141bd41f0e7f7758956e2340e10cdf5f2b0b9 Mon Sep 17 00:00:00 2001 From: Rajendra Nayak Date: Wed, 2 Aug 2017 13:15:42 +0530 Subject: usb: phy: phy-msm-usb: Fix usage of devm_regulator_bulk_get() The regulator_bulk_data pointer passed to devm_regulator_bulk_get() is used to store the client handles for the regulators, which is later used by devm_regulator_bulk_release() to free the regulators. Passing a local array as is done here means the memory used to store the handles is freed causing the handles to be corrupted, resulting in a crash when devm_regulator_bulk_release() tries to free them. Fix this my moving the array inside of the msm_otg structure. Signed-off-by: Rajendra Nayak Signed-off-by: Felipe Balbi --- drivers/usb/phy/phy-msm-usb.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) (limited to 'drivers/usb') diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c index 8fb86a5f458e..3d0dd2f97415 100644 --- a/drivers/usb/phy/phy-msm-usb.c +++ b/drivers/usb/phy/phy-msm-usb.c @@ -197,6 +197,7 @@ struct msm_otg { struct regulator *v3p3; struct regulator *v1p8; struct regulator *vddcx; + struct regulator_bulk_data supplies[3]; struct reset_control *phy_rst; struct reset_control *link_rst; @@ -1731,7 +1732,6 @@ static int msm_otg_reboot_notify(struct notifier_block *this, static int msm_otg_probe(struct platform_device *pdev) { - struct regulator_bulk_data regs[3]; int ret = 0; struct device_node *np = pdev->dev.of_node; struct msm_otg_platform_data *pdata; @@ -1817,17 +1817,18 @@ static int msm_otg_probe(struct platform_device *pdev) return motg->irq; } - regs[0].supply = "vddcx"; - regs[1].supply = "v3p3"; - regs[2].supply = "v1p8"; + motg->supplies[0].supply = "vddcx"; + motg->supplies[1].supply = "v3p3"; + motg->supplies[2].supply = "v1p8"; - ret = devm_regulator_bulk_get(motg->phy.dev, ARRAY_SIZE(regs), regs); + ret = devm_regulator_bulk_get(motg->phy.dev, ARRAY_SIZE(motg->supplies), + motg->supplies); if (ret) return ret; - motg->vddcx = regs[0].consumer; - motg->v3p3 = regs[1].consumer; - motg->v1p8 = regs[2].consumer; + motg->vddcx = motg->supplies[0].consumer; + motg->v3p3 = motg->supplies[1].consumer; + motg->v1p8 = motg->supplies[2].consumer; clk_set_rate(motg->clk, 60000000); -- cgit v1.2.3-55-g7522 From 2acecd58969897795cf015c9057ebd349a3fda8a Mon Sep 17 00:00:00 2001 From: Yoshihiro Shimoda Date: Wed, 2 Aug 2017 13:21:45 +0900 Subject: usb: renesas_usbhs: Fix UGCTRL2 value for R-Car Gen3 The latest HW manual (Rev.0.55) shows us this UGCTRL2.VBUSSEL bit. If the bit sets to 1, the VBUS drive is controlled by phy related registers (called "UCOM Registers" on the manual). Since R-Car Gen3 environment will control VBUS by phy-rcar-gen3-usb2 driver, the UGCTRL2.VBUSSEL bit should be set to 1. So, this patch fixes the register's value. Otherwise, even if the ID pin indicates to peripheral, the R-Car will output USBn_PWEN to 1 when a host driver is running. Fixes: de18757e272d ("usb: renesas_usbhs: add R-Car Gen3 power control" Cc: # v4.6+ Signed-off-by: Yoshihiro Shimoda Signed-off-by: Felipe Balbi --- drivers/usb/renesas_usbhs/rcar3.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'drivers/usb') diff --git a/drivers/usb/renesas_usbhs/rcar3.c b/drivers/usb/renesas_usbhs/rcar3.c index d544b331c9f2..02b67abfc2a1 100644 --- a/drivers/usb/renesas_usbhs/rcar3.c +++ b/drivers/usb/renesas_usbhs/rcar3.c @@ -20,9 +20,13 @@ /* Low Power Status register (LPSTS) */ #define LPSTS_SUSPM 0x4000 -/* USB General control register 2 (UGCTRL2), bit[31:6] should be 0 */ +/* + * USB General control register 2 (UGCTRL2) + * Remarks: bit[31:11] and bit[9:6] should be 0 + */ #define UGCTRL2_RESERVED_3 0x00000001 /* bit[3:0] should be B'0001 */ #define UGCTRL2_USB0SEL_OTG 0x00000030 +#define UGCTRL2_VBUSSEL 0x00000400 static void usbhs_write32(struct usbhs_priv *priv, u32 reg, u32 data) { @@ -34,7 +38,8 @@ static int usbhs_rcar3_power_ctrl(struct platform_device *pdev, { struct usbhs_priv *priv = usbhs_pdev_to_priv(pdev); - usbhs_write32(priv, UGCTRL2, UGCTRL2_RESERVED_3 | UGCTRL2_USB0SEL_OTG); + usbhs_write32(priv, UGCTRL2, UGCTRL2_RESERVED_3 | UGCTRL2_USB0SEL_OTG | + UGCTRL2_VBUSSEL); if (enable) { usbhs_bset(priv, LPSTS, LPSTS_SUSPM, LPSTS_SUSPM); -- cgit v1.2.3-55-g7522 From b7d44c36a6f6d956e1539e0dd42f98b26e5a4684 Mon Sep 17 00:00:00 2001 From: Yoshihiro Shimoda Date: Fri, 28 Jul 2017 19:28:57 +0900 Subject: usb: renesas_usbhs: gadget: fix unused-but-set-variable warning The commit b8b9c974afee ("usb: renesas_usbhs: gadget: disable all eps when the driver stops") causes the unused-but-set-variable warning. But, if the usbhsg_ep_disable() will return non-zero value, udc/core.c doesn't clear the ep->enabled flag. So, this driver should not return non-zero value, if the pipe is zero because this means the pipe is already disabled. Otherwise, the ep->enabled flag is never cleared when the usbhsg_ep_disable() is called by the renesas_usbhs driver first. Fixes: b8b9c974afee ("usb: renesas_usbhs: gadget: disable all eps when the driver stops") Fixes: 11432050f070 ("usb: renesas_usbhs: gadget: fix NULL pointer dereference in ep_disable()") Signed-off-by: Yoshihiro Shimoda Signed-off-by: Felipe Balbi --- drivers/usb/renesas_usbhs/mod_gadget.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'drivers/usb') diff --git a/drivers/usb/renesas_usbhs/mod_gadget.c b/drivers/usb/renesas_usbhs/mod_gadget.c index 93fba9033b00..2c8161bcf5b5 100644 --- a/drivers/usb/renesas_usbhs/mod_gadget.c +++ b/drivers/usb/renesas_usbhs/mod_gadget.c @@ -639,14 +639,11 @@ static int usbhsg_ep_disable(struct usb_ep *ep) struct usbhsg_uep *uep = usbhsg_ep_to_uep(ep); struct usbhs_pipe *pipe; unsigned long flags; - int ret = 0; spin_lock_irqsave(&uep->lock, flags); pipe = usbhsg_uep_to_pipe(uep); - if (!pipe) { - ret = -EINVAL; + if (!pipe) goto out; - } usbhsg_pipe_disable(uep); usbhs_pipe_free(pipe); -- cgit v1.2.3-55-g7522