Discussion:
[PATCH v2 01/10] usb: dwc2/gadget: report disconnect event from 'end session' irq
Marek Szyprowski
2014-10-20 10:45:31 UTC
Permalink
This patch adds a call to s3c_hsotg_disconnect() from 'end session'
interrupt (GOTGINT_SES_END_DET) to correctly notify gadget subsystem
about unplugged usb cable. 'disconnected' interrupt (DISCONNINT) might
look a bit more suitable for this event, but it is asserted only in
host mode, so in device mode we need to use something else.

Signed-off-by: Marek Szyprowski <***@samsung.com>
---
drivers/usb/dwc2/gadget.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 7b5856fadd93..119c8a3effc2 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2279,6 +2279,12 @@ irq_retry:
dev_info(hsotg->dev, "OTGInt: %08x\n", otgint);

writel(otgint, hsotg->regs + GOTGINT);
+
+ if (otgint & GOTGINT_SES_END_DET) {
+ if (hsotg->gadget.speed != USB_SPEED_UNKNOWN)
+ s3c_hsotg_disconnect(hsotg);
+ hsotg->gadget.speed = USB_SPEED_UNKNOWN;
+ }
}

if (gintsts & GINTSTS_SESSREQINT) {
--
1.9.2
Marek Szyprowski
2014-10-20 10:45:36 UTC
Permalink
This patch changes s3c_hsotg_core_init function to leave hardware in
soft disconnect mode, so the moment of coupling the hardware to the usb
bus can be later controlled by the separate functions for enabling and
disabling soft disconnect mode. This patch is a preparation to rework
pullup() method.

Signed-off-by: Marek Szyprowski <m.szyprowski-***@public.gmane.org>
---
drivers/usb/dwc2/gadget.c | 22 +++++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index c1dad46bbbdd..5eb2473031c4 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2124,7 +2124,7 @@ static int s3c_hsotg_corereset(struct s3c_hsotg *hsotg)
*
* Issue a soft reset to the core, and await the core finishing it.
*/
-static void s3c_hsotg_core_init(struct s3c_hsotg *hsotg)
+static void s3c_hsotg_core_init_disconnected(struct s3c_hsotg *hsotg)
{
s3c_hsotg_corereset(hsotg);

@@ -2241,14 +2241,23 @@ static void s3c_hsotg_core_init(struct s3c_hsotg *hsotg)
readl(hsotg->regs + DOEPCTL0));

/* clear global NAKs */
- writel(DCTL_CGOUTNAK | DCTL_CGNPINNAK,
+ writel(DCTL_CGOUTNAK | DCTL_CGNPINNAK | DCTL_SFTDISCON,
hsotg->regs + DCTL);

/* must be at-least 3ms to allow bus to see disconnect */
mdelay(3);

hsotg->last_rst = jiffies;
+}
+
+static void s3c_hsotg_core_disconnect(struct s3c_hsotg *hsotg)
+{
+ /* set the soft-disconnect bit */
+ __orr32(hsotg->regs + DCTL, DCTL_SFTDISCON);
+}

+static void s3c_hsotg_core_connect(struct s3c_hsotg *hsotg)
+{
/* remove the soft-disconnect and let's go */
__bic32(hsotg->regs + DCTL, DCTL_SFTDISCON);
}
@@ -2348,7 +2357,8 @@ irq_retry:
kill_all_requests(hsotg, &hsotg->eps[0],
-ECONNRESET, true);

- s3c_hsotg_core_init(hsotg);
+ s3c_hsotg_core_init_disconnected(hsotg);
+ s3c_hsotg_core_connect(hsotg);
}
}
}
@@ -2981,7 +2991,8 @@ static int s3c_hsotg_pullup(struct usb_gadget *gadget, int is_on)
if (is_on) {
s3c_hsotg_phy_enable(hsotg);
clk_enable(hsotg->clk);
- s3c_hsotg_core_init(hsotg);
+ s3c_hsotg_core_init_disconnected(hsotg);
+ s3c_hsotg_core_connect(hsotg);
} else {
clk_disable(hsotg->clk);
s3c_hsotg_phy_disable(hsotg);
@@ -3668,7 +3679,8 @@ static int s3c_hsotg_resume(struct platform_device *pdev)

spin_lock_irqsave(&hsotg->lock, flags);
s3c_hsotg_phy_enable(hsotg);
- s3c_hsotg_core_init(hsotg);
+ s3c_hsotg_core_init_disconnected(hsotg);
+ s3c_hsotg_core_connect(hsotg);
spin_unlock_irqrestore(&hsotg->lock, flags);

return ret;
--
1.9.2

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Marek Szyprowski
2014-10-20 10:45:33 UTC
Permalink
udc_stop() should clear ->driver pointer unconditionally to let the UDC
framework to work correctly with both registering/unregistering gadgets
and enabling/disabling gadgets by writing to
/sys/class/udc/*hsotg/soft_connect interface.

Signed-off-by: Marek Szyprowski <m.szyprowski-***@public.gmane.org>
---
drivers/usb/dwc2/gadget.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 8870e38c1d82..a4b4def23afd 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2940,9 +2940,7 @@ static int s3c_hsotg_udc_stop(struct usb_gadget *gadget,

spin_lock_irqsave(&hsotg->lock, flags);

- if (!driver)
- hsotg->driver = NULL;
Marek Szyprowski
2014-10-20 10:45:34 UTC
Permalink
This patch fixes probe function to match the pattern used elsewhere in
the driver, where power regulators are turned off as the last element in
the device shutdown procedure.

Signed-off-by: Marek Szyprowski <m.szyprowski-***@public.gmane.org>
---
drivers/usb/dwc2/gadget.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index a4b4def23afd..fd52a8b23649 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3571,6 +3571,7 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
s3c_hsotg_initep(hsotg, &hsotg->eps[epnum], epnum);

/* disable power and clock */
+ s3c_hsotg_phy_disable(hsotg);

ret = regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies),
hsotg->supplies);
@@ -3579,8 +3580,6 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
goto err_ep_mem;
}

- s3c_hsotg_phy_disable(hsotg);
Marek Szyprowski
2014-10-20 10:45:39 UTC
Permalink
This patch moves calls to phy enable/disable out of spinlock protected
blocks in device suspend/resume to fix incorrect caller context. Phy
related functions must not be called from atomic context. To protect
device internal state from a race during suspend, a call to
s3c_hsotg_core_disconnect() is added under a spinlock, what prevents any
further activity on the usb bus.

Signed-off-by: Marek Szyprowski <m.szyprowski-***@public.gmane.org>
---
drivers/usb/dwc2/gadget.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index e8ffc080e6c7..0d34cfc71bfb 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3653,11 +3653,13 @@ static int s3c_hsotg_suspend(struct platform_device *pdev, pm_message_t state)
hsotg->driver->driver.name);

spin_lock_irqsave(&hsotg->lock, flags);
+ s3c_hsotg_core_disconnect(hsotg);
s3c_hsotg_disconnect(hsotg);
- s3c_hsotg_phy_disable(hsotg);
hsotg->gadget.speed = USB_SPEED_UNKNOWN;
spin_unlock_irqrestore(&hsotg->lock, flags);

+ s3c_hsotg_phy_disable(hsotg);
+
if (hsotg->driver) {
int ep;
for (ep = 0; ep < hsotg->num_of_eps; ep++)
@@ -3686,8 +3688,9 @@ static int s3c_hsotg_resume(struct platform_device *pdev)
hsotg->supplies);
}

- spin_lock_irqsave(&hsotg->lock, flags);
s3c_hsotg_phy_enable(hsotg);
+
+ spin_lock_irqsave(&hsotg->lock, flags);
s3c_hsotg_core_init_disconnected(hsotg);
s3c_hsotg_core_connect(hsotg);
spin_unlock_irqrestore(&hsotg->lock, flags);
--
1.9.2

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Marek Szyprowski
2014-10-20 10:45:38 UTC
Permalink
This patch moves udc initialization from pullup() method to
s3c_hsotg_udc_start(), so that method ends with hardware fully
initialized and left in soft-disconnected state. After this change, the
pullup() method simply clears soft-disconnect start() when called with
is_on=1. For completeness, a call to s3c_hsotg_core_disconnect() has
been added when pullup() method is called with is_on=0, what puts the
udc hardware back to soft-disconnected state.

Signed-off-by: Marek Szyprowski <***@samsung.com>
---
drivers/usb/dwc2/gadget.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 98adf8d17493..e8ffc080e6c7 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2883,6 +2883,7 @@ static int s3c_hsotg_udc_start(struct usb_gadget *gadget,
struct usb_gadget_driver *driver)
{
struct s3c_hsotg *hsotg = to_hsotg(gadget);
+ unsigned long flags;
int ret;

if (!hsotg) {
@@ -2921,7 +2922,13 @@ static int s3c_hsotg_udc_start(struct usb_gadget *gadget,

s3c_hsotg_phy_enable(hsotg);

+ spin_lock_irqsave(&hsotg->lock, flags);
+ s3c_hsotg_init(hsotg);
+ s3c_hsotg_core_init_disconnected(hsotg);
+ spin_unlock_irqrestore(&hsotg->lock, flags);
+
dev_info(hsotg->dev, "bound driver %s\n", driver->driver.name);
+
return 0;

err:
@@ -2994,9 +3001,9 @@ static int s3c_hsotg_pullup(struct usb_gadget *gadget, int is_on)
spin_lock_irqsave(&hsotg->lock, flags);
if (is_on) {
clk_enable(hsotg->clk);
- s3c_hsotg_core_init_disconnected(hsotg);
s3c_hsotg_core_connect(hsotg);
} else {
+ s3c_hsotg_core_disconnect(hsotg);
clk_disable(hsotg->clk);
}
--
1.9.2
Marek Szyprowski
2014-10-20 10:45:40 UTC
Permalink
Suspend/resume code assumed that the gadget was always enabled and
connected to usb bus. This means that the actual state of the gadget
(soft-enabled/disabled or connected/disconnected) was not correctly
preserved on suspend/resume cycle. This patch fixes this issue.

Signed-off-by: Marek Szyprowski <m.szyprowski-***@public.gmane.org>
---
drivers/usb/dwc2/core.h | 4 +++-
drivers/usb/dwc2/gadget.c | 43 +++++++++++++++++++++++++++----------------
2 files changed, 30 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index bf015ab3b44c..3648b76a18b4 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -210,7 +210,9 @@ struct s3c_hsotg {
u8 ctrl_buff[8];

struct usb_gadget gadget;
- unsigned int setup;
+ unsigned int setup:1;
+ unsigned int connected:1;
+ unsigned int enabled:1;
unsigned long last_rst;
struct s3c_hsotg_ep *eps;
};
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 0d34cfc71bfb..c6c6cf982c90 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2925,6 +2925,8 @@ static int s3c_hsotg_udc_start(struct usb_gadget *gadget,
spin_lock_irqsave(&hsotg->lock, flags);
s3c_hsotg_init(hsotg);
s3c_hsotg_core_init_disconnected(hsotg);
+ hsotg->enabled = 1;
+ hsotg->connected = 0;
spin_unlock_irqrestore(&hsotg->lock, flags);

dev_info(hsotg->dev, "bound driver %s\n", driver->driver.name);
@@ -2961,6 +2963,8 @@ static int s3c_hsotg_udc_stop(struct usb_gadget *gadget,

hsotg->driver = NULL;
hsotg->gadget.speed = USB_SPEED_UNKNOWN;
+ hsotg->enabled = 0;
+ hsotg->connected = 0;

spin_unlock_irqrestore(&hsotg->lock, flags);

@@ -2999,11 +3003,14 @@ static int s3c_hsotg_pullup(struct usb_gadget *gadget, int is_on)
dev_dbg(hsotg->dev, "%s: is_on: %d\n", __func__, is_on);

spin_lock_irqsave(&hsotg->lock, flags);
+
if (is_on) {
clk_enable(hsotg->clk);
+ hsotg->connected = 1;
s3c_hsotg_core_connect(hsotg);
} else {
s3c_hsotg_core_disconnect(hsotg);
+ hsotg->connected = 0;
clk_disable(hsotg->clk);
}

@@ -3652,16 +3659,18 @@ static int s3c_hsotg_suspend(struct platform_device *pdev, pm_message_t state)
dev_info(hsotg->dev, "suspending usb gadget %s\n",
hsotg->driver->driver.name);

- spin_lock_irqsave(&hsotg->lock, flags);
- s3c_hsotg_core_disconnect(hsotg);
- s3c_hsotg_disconnect(hsotg);
- hsotg->gadget.speed = USB_SPEED_UNKNOWN;
- spin_unlock_irqrestore(&hsotg->lock, flags);
+ if (hsotg->enabled) {
+ int ep;

- s3c_hsotg_phy_disable(hsotg);
+ spin_lock_irqsave(&hsotg->lock, flags);
+ if (hsotg->connected)
+ s3c_hsotg_core_disconnect(hsotg);
+ s3c_hsotg_disconnect(hsotg);
+ hsotg->gadget.speed = USB_SPEED_UNKNOWN;
+ spin_unlock_irqrestore(&hsotg->lock, flags);
+
+ s3c_hsotg_phy_disable(hsotg);

- if (hsotg->driver) {
- int ep;
for (ep = 0; ep < hsotg->num_of_eps; ep++)
s3c_hsotg_ep_disable(&hsotg->eps[ep].ep);

@@ -3679,21 +3688,23 @@ static int s3c_hsotg_resume(struct platform_device *pdev)
unsigned long flags;
int ret = 0;

- if (hsotg->driver) {
+ if (hsotg->driver)
dev_info(hsotg->dev, "resuming usb gadget %s\n",
hsotg->driver->driver.name);

+ if (hsotg->enabled) {
clk_enable(hsotg->clk);
ret = regulator_bulk_enable(ARRAY_SIZE(hsotg->supplies),
- hsotg->supplies);
- }
+ hsotg->supplies);

- s3c_hsotg_phy_enable(hsotg);
+ s3c_hsotg_phy_enable(hsotg);

- spin_lock_irqsave(&hsotg->lock, flags);
- s3c_hsotg_core_init_disconnected(hsotg);
- s3c_hsotg_core_connect(hsotg);
- spin_unlock_irqrestore(&hsotg->lock, flags);
+ spin_lock_irqsave(&hsotg->lock, flags);
+ s3c_hsotg_core_init_disconnected(hsotg);
+ if (hsotg->connected)
+ s3c_hsotg_core_connect(hsotg);
+ spin_unlock_irqrestore(&hsotg->lock, flags);
+ }

return ret;
}
--
1.9.2

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Marek Szyprowski
2014-10-20 10:45:35 UTC
Permalink
This patch removes duplicated code and sets last_rst variable in the
function which does the hardware reset.

Signed-off-by: Marek Szyprowski <***@samsung.com>
---
drivers/usb/dwc2/gadget.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index fd52a8b23649..c1dad46bbbdd 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2247,6 +2247,8 @@ static void s3c_hsotg_core_init(struct s3c_hsotg *hsotg)
/* must be at-least 3ms to allow bus to see disconnect */
mdelay(3);

+ hsotg->last_rst = jiffies;
+
/* remove the soft-disconnect and let's go */
__bic32(hsotg->regs + DCTL, DCTL_SFTDISCON);
}
@@ -2347,7 +2349,6 @@ irq_retry:
-ECONNRESET, true);

s3c_hsotg_core_init(hsotg);
- hsotg->last_rst = jiffies;
}
}
}
@@ -2908,7 +2909,6 @@ static int s3c_hsotg_udc_start(struct usb_gadget *gadget,
goto err;
}

- hsotg->last_rst = jiffies;
dev_info(hsotg->dev, "bound driver %s\n", driver->driver.name);
return 0;

@@ -3667,7 +3667,6 @@ static int s3c_hsotg_resume(struct platform_device *pdev)
}

spin_lock_irqsave(&hsotg->lock, flags);
- hsotg->last_rst = jiffies;
s3c_hsotg_phy_enable(hsotg);
s3c_hsotg_core_init(hsotg);
spin_unlock_irqrestore(&hsotg->lock, flags);
--
1.9.2
Marek Szyprowski
2014-10-20 10:45:37 UTC
Permalink
This patch moves phy enable/disable calls from pullup() method to
udc_start/stop functions. This solves the issue related to limited caller
context for PHY functions, because they cannot be called from non-sleeping
context. This is also a preparation for using soft-disconnect feature of
udc controller in pullup() method.

Signed-off-by: Marek Szyprowski <***@samsung.com>
---
drivers/usb/dwc2/gadget.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 5eb2473031c4..98adf8d17493 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2919,6 +2919,8 @@ static int s3c_hsotg_udc_start(struct usb_gadget *gadget,
goto err;
}

+ s3c_hsotg_phy_enable(hsotg);
+
dev_info(hsotg->dev, "bound driver %s\n", driver->driver.name);
return 0;

@@ -2955,6 +2957,8 @@ static int s3c_hsotg_udc_stop(struct usb_gadget *gadget,

spin_unlock_irqrestore(&hsotg->lock, flags);

+ s3c_hsotg_phy_disable(hsotg);
+
regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies), hsotg->supplies);

clk_disable(hsotg->clk);
@@ -2989,13 +2993,11 @@ static int s3c_hsotg_pullup(struct usb_gadget *gadget, int is_on)

spin_lock_irqsave(&hsotg->lock, flags);
if (is_on) {
- s3c_hsotg_phy_enable(hsotg);
clk_enable(hsotg->clk);
s3c_hsotg_core_init_disconnected(hsotg);
s3c_hsotg_core_connect(hsotg);
} else {
clk_disable(hsotg->clk);
- s3c_hsotg_phy_disable(hsotg);
}

hsotg->gadget.speed = USB_SPEED_UNKNOWN;
--
1.9.2
Marek Szyprowski
2014-10-20 10:45:32 UTC
Permalink
Excessive debug messages might cause timing issues that prevent correct
usb enumeration. This patch hides information about USB bus reset to let
driver enumerate fast enough to avoid making host angry. This fixes
endless enumeration and usb reset loop observed with some Linux hosts.

Signed-off-by: Marek Szyprowski <m.szyprowski-***@public.gmane.org>
Reviewed-by: Felipe Balbi <balbi-***@public.gmane.org>
---
drivers/usb/dwc2/gadget.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 119c8a3effc2..8870e38c1d82 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2333,7 +2333,7 @@ irq_retry:

u32 usb_status = readl(hsotg->regs + GOTGCTL);

- dev_info(hsotg->dev, "%s: USBRst\n", __func__);
+ dev_dbg(hsotg->dev, "%s: USBRst\n", __func__);
dev_dbg(hsotg->dev, "GNPTXSTS=%08x\n",
readl(hsotg->regs + GNPTXSTS));
--
1.9.2

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