Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1217638 > unrolled thread
| Started by | Roger Quadros <rogerq@ti.com> |
|---|---|
| First post | 2015-09-02 16:30 +0200 |
| Last post | 2015-09-04 11:20 +0200 |
| Articles | 20 on this page of 30 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH v4 0/9] usb: dwc3: add dual-role support Roger Quadros <rogerq@ti.com> - 2015-09-02 16:30 +0200
[PATCH v4 1/9] usb: dwc3: add dual-role support Roger Quadros <rogerq@ti.com> - 2015-09-02 16:30 +0200
Re: [PATCH v4 1/9] usb: dwc3: add dual-role support Felipe Balbi <balbi@ti.com> - 2015-09-02 16:40 +0200
Re: [PATCH v4 1/9] usb: dwc3: add dual-role support Roger Quadros <rogerq@ti.com> - 2015-09-03 14:30 +0200
Re: [PATCH v4 1/9] usb: dwc3: add dual-role support Felipe Balbi <balbi@ti.com> - 2015-09-03 18:10 +0200
Re: [PATCH v4 1/9] usb: dwc3: add dual-role support Roger Quadros <rogerq@ti.com> - 2015-09-04 11:10 +0200
[PATCH v4 6/9] usb: dwc3: save/restore OTG registers during suspend/resume Roger Quadros <rogerq@ti.com> - 2015-09-02 16:30 +0200
Re: [PATCH v4 6/9] usb: dwc3: save/restore OTG registers during suspend/resume Felipe Balbi <balbi@ti.com> - 2015-09-02 16:50 +0200
Re: [PATCH v4 6/9] usb: dwc3: save/restore OTG registers during suspend/resume Roger Quadros <rogerq@ti.com> - 2015-09-03 16:00 +0200
[PATCH v4 3/9] usb: dwc3: dwc3-omap: Make the wrapper interrupt shared Roger Quadros <rogerq@ti.com> - 2015-09-02 16:30 +0200
Re: [PATCH v4 3/9] usb: dwc3: dwc3-omap: Make the wrapper interrupt shared Felipe Balbi <balbi@ti.com> - 2015-09-02 16:40 +0200
[PATCH v4 5/9] usb: dwc3: core: make dual-role work with OTG irq Roger Quadros <rogerq@ti.com> - 2015-09-02 16:30 +0200
Re: [PATCH v4 5/9] usb: dwc3: core: make dual-role work with OTG irq Felipe Balbi <balbi@ti.com> - 2015-09-02 16:50 +0200
Re: [PATCH v4 5/9] usb: dwc3: core: make dual-role work with OTG irq Roger Quadros <rogerq@ti.com> - 2015-09-03 16:00 +0200
Re: [PATCH v4 5/9] usb: dwc3: core: make dual-role work with OTG irq Felipe Balbi <balbi@ti.com> - 2015-09-03 18:00 +0200
Re: [PATCH v4 5/9] usb: dwc3: core: make dual-role work with OTG irq Roger Quadros <rogerq@ti.com> - 2015-09-04 11:20 +0200
[PATCH v4 9/9] usb: dwc3: core: don't break during suspend/resume while we're dual-role Roger Quadros <rogerq@ti.com> - 2015-09-02 16:30 +0200
Re: [PATCH v4 9/9] usb: dwc3: core: don't break during suspend/resume while we're dual-role Felipe Balbi <balbi@ti.com> - 2015-09-02 16:50 +0200
Re: [PATCH v4 9/9] usb: dwc3: core: don't break during suspend/resume while we're dual-role Roger Quadros <rogerq@ti.com> - 2015-09-03 16:10 +0200
Re: [PATCH v4 9/9] usb: dwc3: core: don't break during suspend/resume while we're dual-role Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2015-09-02 19:30 +0200
Re: [PATCH v4 9/9] usb: dwc3: core: don't break during suspend/resume while we're dual-role Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2015-09-03 16:10 +0200
Re: [PATCH v4 9/9] usb: dwc3: core: don't break during suspend/resume while we're dual-role Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2015-09-03 16:20 +0200
Re: [PATCH v4 9/9] usb: dwc3: core: don't break during suspend/resume while we're dual-role Roger Quadros <rogerq@ti.com> - 2015-09-03 16:20 +0200
Re: [PATCH v4 9/9] usb: dwc3: core: don't break during suspend/resume while we're dual-role Roger Quadros <rogerq@ti.com> - 2015-09-03 16:10 +0200
[PATCH v4 2/9] usb: dwc3: core.h: add some register definitions Roger Quadros <rogerq@ti.com> - 2015-09-02 16:30 +0200
[PATCH v4 4/9] usb: dwc3: core: Adapt to named interrupts Roger Quadros <rogerq@ti.com> - 2015-09-02 16:30 +0200
Re: [PATCH v4 4/9] usb: dwc3: core: Adapt to named interrupts Felipe Balbi <balbi@ti.com> - 2015-09-02 16:40 +0200
Re: [PATCH v4 4/9] usb: dwc3: core: Adapt to named interrupts Roger Quadros <rogerq@ti.com> - 2015-09-03 14:50 +0200
Re: [PATCH v4 4/9] usb: dwc3: core: Adapt to named interrupts Felipe Balbi <balbi@ti.com> - 2015-09-03 18:00 +0200
Re: [PATCH v4 4/9] usb: dwc3: core: Adapt to named interrupts Roger Quadros <rogerq@ti.com> - 2015-09-04 11:20 +0200
Page 1 of 2 [1] 2 Next page →
| From | Roger Quadros <rogerq@ti.com> |
|---|---|
| Date | 2015-09-02 16:30 +0200 |
| Subject | [PATCH v4 0/9] usb: dwc3: add dual-role support |
| Message-ID | <q4hcZ-7Er-3@gated-at.bofh.it> |
Hi,
This series adds dual role support to dwc3 controller driver.
Series depends on [1] for core OTG/dual-role support.
Patches are based on greg/usb-next.
Tested on am437x-gp-evm and dra7-evm after platform related patches are
applied. Those are available at [2].
[1] core OTG/DRD support - http://thread.gmane.org/gmane.linux.kernel/2025239
[2] platform support - http://thread.gmane.org/gmane.linux.usb.general/127898
cheers,
-roger
Felipe Balbi (1):
usb: dwc3: core: Adapt to named interrupts
Roger Quadros (8):
usb: dwc3: add dual-role support
usb: dwc3: core.h: add some register definitions
usb: dwc3: dwc3-omap: Make the wrapper interrupt shared
usb: dwc3: core: make dual-role work with OTG irq
usb: dwc3: save/restore OTG registers during suspend/resume
usb: dwc3: gadget: Fix suspend/resume during dual-role mode
usb: dwc3: core: Prevent otg events from disabling themselves
usb: dwc3: core: don't break during suspend/resume while we're
dual-role
drivers/usb/dwc3/core.c | 422 +++++++++++++++++++++++++++++++++++++--
drivers/usb/dwc3/core.h | 103 ++++++++++
drivers/usb/dwc3/dwc3-omap.c | 4 +-
drivers/usb/dwc3/gadget.c | 8 +-
drivers/usb/dwc3/platform_data.h | 1 +
5 files changed, 516 insertions(+), 22 deletions(-)
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Roger Quadros <rogerq@ti.com> |
|---|---|
| Date | 2015-09-02 16:30 +0200 |
| Subject | [PATCH v4 1/9] usb: dwc3: add dual-role support |
| Message-ID | <q4hd0-7Er-19@gated-at.bofh.it> |
| In reply to | #1217638 |
Register with the USB OTG core. Since we don't support
OTG yet we just work as a dual-role device even
if device tree says "otg".
Use extcon framework to get VBUS/ID cable events and
kick the OTG state machine.
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
drivers/usb/dwc3/core.c | 174 ++++++++++++++++++++++++++++++++++++++-
drivers/usb/dwc3/core.h | 7 ++
drivers/usb/dwc3/platform_data.h | 1 +
3 files changed, 181 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 064123e..2e36a9b 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -704,6 +704,152 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
return 0;
}
+/* --------------------- Dual-Role management ------------------------------- */
+
+static void dwc3_drd_fsm_sync(struct dwc3 *dwc)
+{
+ int id, vbus;
+
+ /* get ID */
+ id = extcon_get_cable_state(dwc->edev, "USB-HOST");
+ /* Host means ID == 0 */
+ id = !id;
+
+ /* get VBUS */
+ vbus = extcon_get_cable_state(dwc->edev, "USB");
+ dev_dbg(dwc->dev, "id %d vbus %d\n", id, vbus);
+
+ dwc->fsm->id = id;
+ dwc->fsm->b_sess_vld = vbus;
+ usb_otg_sync_inputs(dwc->fsm);
+}
+
+static int dwc3_drd_start_host(struct otg_fsm *fsm, int on)
+{
+ struct device *dev = usb_otg_fsm_to_dev(fsm);
+ struct dwc3 *dwc = dev_get_drvdata(dev);
+
+ dev_dbg(dwc->dev, "%s: %d\n", __func__, on);
+ if (on) {
+ dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
+ /* start the HCD */
+ usb_otg_start_host(fsm, true);
+ } else {
+ /* stop the HCD */
+ usb_otg_start_host(fsm, false);
+ }
+
+ return 0;
+}
+
+static int dwc3_drd_start_gadget(struct otg_fsm *fsm, int on)
+{
+ struct device *dev = usb_otg_fsm_to_dev(fsm);
+ struct dwc3 *dwc = dev_get_drvdata(dev);
+
+ dev_dbg(dwc->dev, "%s: %d\n", __func__, on);
+ if (on) {
+ dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
+ dwc3_event_buffers_setup(dwc);
+
+ /* start the UDC */
+ usb_otg_start_gadget(fsm, true);
+ } else {
+ /* stop the UDC */
+ usb_otg_start_gadget(fsm, false);
+ }
+
+ return 0;
+}
+
+static struct otg_fsm_ops dwc3_drd_ops = {
+ .start_host = dwc3_drd_start_host,
+ .start_gadget = dwc3_drd_start_gadget,
+};
+
+static int dwc3_drd_notifier(struct notifier_block *nb,
+ unsigned long event, void *ptr)
+{
+ struct dwc3 *dwc = container_of(nb, struct dwc3, otg_nb);
+
+ dwc3_drd_fsm_sync(dwc);
+
+ return NOTIFY_DONE;
+}
+
+static int dwc3_drd_init(struct dwc3 *dwc)
+{
+ int ret, id, vbus;
+ struct usb_otg_caps *otgcaps = &dwc->otg_config.otg_caps;
+
+ otgcaps->otg_rev = 0;
+ otgcaps->hnp_support = false;
+ otgcaps->srp_support = false;
+ otgcaps->adp_support = false;
+ dwc->otg_config.fsm_ops = &dwc3_drd_ops;
+
+ if (!dwc->edev) {
+ dev_err(dwc->dev, "No extcon device found for OTG mode\n");
+ return -ENODEV;
+ }
+
+ dwc->otg_nb.notifier_call = dwc3_drd_notifier;
+ ret = extcon_register_notifier(dwc->edev, EXTCON_USB, &dwc->otg_nb);
+ if (ret < 0) {
+ dev_err(dwc->dev, "Couldn't register USB cable notifier\n");
+ return -ENODEV;
+ }
+
+ ret = extcon_register_notifier(dwc->edev, EXTCON_USB_HOST,
+ &dwc->otg_nb);
+ if (ret < 0) {
+ dev_err(dwc->dev, "Couldn't register USB-HOST cable notifier\n");
+ ret = -ENODEV;
+ goto extcon_fail;
+ }
+
+ /* sanity check id & vbus states */
+ id = extcon_get_cable_state(dwc->edev, "USB-HOST");
+ vbus = extcon_get_cable_state(dwc->edev, "USB");
+ if (id < 0 || vbus < 0) {
+ dev_err(dwc->dev, "Invalid USB cable state. id %d, vbus %d\n",
+ id, vbus);
+ ret = -ENODEV;
+ goto fail;
+ }
+
+ /* register as DRD device with OTG core */
+ dwc->fsm = usb_otg_register(dwc->dev, &dwc->otg_config);
+ if (IS_ERR(dwc->fsm)) {
+ ret = PTR_ERR(dwc->fsm);
+ if (ret == -ENOTSUPP)
+ dev_err(dwc->dev, "CONFIG_USB_OTG needed for dual-role\n");
+ else
+ dev_err(dwc->dev, "Failed to register with OTG core\n");
+
+ goto fail;
+ }
+
+ dwc3_drd_fsm_sync(dwc);
+
+ return 0;
+fail:
+ extcon_unregister_notifier(dwc->edev, EXTCON_USB_HOST, &dwc->otg_nb);
+extcon_fail:
+ extcon_unregister_notifier(dwc->edev, EXTCON_USB, &dwc->otg_nb);
+
+ return ret;
+}
+
+static void dwc3_drd_exit(struct dwc3 *dwc)
+{
+ usb_otg_unregister(dwc->dev);
+ extcon_unregister_notifier(dwc->edev, EXTCON_USB_HOST, &dwc->otg_nb);
+ extcon_unregister_notifier(dwc->edev, EXTCON_USB, &dwc->otg_nb);
+}
+
+/* -------------------------------------------------------------------------- */
+
static int dwc3_core_init_mode(struct dwc3 *dwc)
{
struct device *dev = dwc->dev;
@@ -727,13 +873,21 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
}
break;
case USB_DR_MODE_OTG:
- dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG);
+ ret = dwc3_drd_init(dwc);
+ if (ret) {
+ dev_err(dev, "limiting to peripheral only\n");
+ dwc->dr_mode = USB_DR_MODE_PERIPHERAL;
+ dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
+ goto gadget_init;
+ }
+
ret = dwc3_host_init(dwc);
if (ret) {
dev_err(dev, "failed to initialize host\n");
return ret;
}
+gadget_init:
ret = dwc3_gadget_init(dwc);
if (ret) {
dev_err(dev, "failed to initialize gadget\n");
@@ -760,6 +914,7 @@ static void dwc3_core_exit_mode(struct dwc3 *dwc)
case USB_DR_MODE_OTG:
dwc3_host_exit(dwc);
dwc3_gadget_exit(dwc);
+ dwc3_drd_exit(dwc);
break;
default:
/* do nothing */
@@ -843,6 +998,16 @@ static int dwc3_probe(struct platform_device *pdev)
hird_threshold = 12;
if (node) {
+ if (of_property_read_bool(node, "extcon"))
+ dwc->edev = extcon_get_edev_by_phandle(dev, 0);
+ else if (of_property_read_bool(dev->parent->of_node, "extcon"))
+ dwc->edev = extcon_get_edev_by_phandle(dev->parent, 0);
+
+ if (IS_ERR(dwc->edev)) {
+ dev_vdbg(dev, "couldn't get extcon device\n");
+ return -EPROBE_DEFER;
+ }
+
dwc->maximum_speed = of_usb_get_maximum_speed(node);
dwc->has_lpm_erratum = of_property_read_bool(node,
"snps,has-lpm-erratum");
@@ -887,6 +1052,13 @@ static int dwc3_probe(struct platform_device *pdev)
of_property_read_string(node, "snps,hsphy_interface",
&dwc->hsphy_interface);
} else if (pdata) {
+ if (pdata->extcon) {
+ dwc->edev = extcon_get_extcon_dev(pdata->extcon);
+ if (!dwc->edev) {
+ dev_vdbg(dev, "couldn't get extcon device\n");
+ return -EPROBE_DEFER;
+ }
+ }
dwc->maximum_speed = pdata->maximum_speed;
dwc->has_lpm_erratum = pdata->has_lpm_erratum;
if (pdata->lpm_nyet_threshold)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 0447788..5ca2b25 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -31,8 +31,10 @@
#include <linux/usb/gadget.h>
#include <linux/usb/otg.h>
#include <linux/ulpi/interface.h>
+#include <linux/usb/otg-fsm.h>
#include <linux/phy/phy.h>
+#include <linux/extcon.h>
#define DWC3_MSG_MAX 500
@@ -753,6 +755,11 @@ struct dwc3 {
struct ulpi *ulpi;
+ struct extcon_dev *edev; /* USB cable events ID & VBUS */
+ struct notifier_block otg_nb; /* notifier for USB cable events */
+ struct otg_fsm *fsm;
+ struct usb_otg_config otg_config;
+
void __iomem *regs;
size_t regs_size;
diff --git a/drivers/usb/dwc3/platform_data.h b/drivers/usb/dwc3/platform_data.h
index d3614ec..b3b245c 100644
--- a/drivers/usb/dwc3/platform_data.h
+++ b/drivers/usb/dwc3/platform_data.h
@@ -47,4 +47,5 @@ struct dwc3_platform_data {
unsigned tx_de_emphasis:2;
const char *hsphy_interface;
+ const char *extcon; /* extcon name for USB cable events ID/VBUS */
};
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Felipe Balbi <balbi@ti.com> |
|---|---|
| Date | 2015-09-02 16:40 +0200 |
| Subject | Re: [PATCH v4 1/9] usb: dwc3: add dual-role support |
| Message-ID | <q4hmF-7PP-1@gated-at.bofh.it> |
| In reply to | #1217639 |
[Multipart message — attachments visible in raw view] — view raw
Hi,
On Wed, Sep 02, 2015 at 05:24:16PM +0300, Roger Quadros wrote:
> Register with the USB OTG core. Since we don't support
> OTG yet we just work as a dual-role device even
> if device tree says "otg".
>
> Use extcon framework to get VBUS/ID cable events and
> kick the OTG state machine.
>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
> drivers/usb/dwc3/core.c | 174 ++++++++++++++++++++++++++++++++++++++-
> drivers/usb/dwc3/core.h | 7 ++
> drivers/usb/dwc3/platform_data.h | 1 +
> 3 files changed, 181 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 064123e..2e36a9b 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -704,6 +704,152 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
> return 0;
> }
>
> +/* --------------------- Dual-Role management ------------------------------- */
> +
> +static void dwc3_drd_fsm_sync(struct dwc3 *dwc)
> +{
> + int id, vbus;
> +
> + /* get ID */
> + id = extcon_get_cable_state(dwc->edev, "USB-HOST");
> + /* Host means ID == 0 */
> + id = !id;
> +
> + /* get VBUS */
> + vbus = extcon_get_cable_state(dwc->edev, "USB");
> + dev_dbg(dwc->dev, "id %d vbus %d\n", id, vbus);
tracepoint please. We don't want this driver to use dev_(v)?db anymore.
Ditto to all others.
> +
> + dwc->fsm->id = id;
> + dwc->fsm->b_sess_vld = vbus;
> + usb_otg_sync_inputs(dwc->fsm);
> +}
> +
> +static int dwc3_drd_start_host(struct otg_fsm *fsm, int on)
> +{
> + struct device *dev = usb_otg_fsm_to_dev(fsm);
> + struct dwc3 *dwc = dev_get_drvdata(dev);
how about adding a usb_otg_get_drvdata(fsm) ?
> + dev_dbg(dwc->dev, "%s: %d\n", __func__, on);
> + if (on) {
> + dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
> + /* start the HCD */
> + usb_otg_start_host(fsm, true);
> + } else {
> + /* stop the HCD */
> + usb_otg_start_host(fsm, false);
> + }
This can be simplified.
if (on)
dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
usb_otg_start_host(fsm, on);
> +
> + return 0;
> +}
> +
> +static int dwc3_drd_start_gadget(struct otg_fsm *fsm, int on)
> +{
> + struct device *dev = usb_otg_fsm_to_dev(fsm);
> + struct dwc3 *dwc = dev_get_drvdata(dev);
> +
> + dev_dbg(dwc->dev, "%s: %d\n", __func__, on);
> + if (on) {
> + dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
> + dwc3_event_buffers_setup(dwc);
> +
> + /* start the UDC */
> + usb_otg_start_gadget(fsm, true);
> + } else {
> + /* stop the UDC */
> + usb_otg_start_gadget(fsm, false);
> + }
likewise:
if (on) {
dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
dwc3_event_buffers_setup(dwc);
}
usb_otg_start_gadget(fsm, on);
> + return 0;
> +}
> +
> +static struct otg_fsm_ops dwc3_drd_ops = {
> + .start_host = dwc3_drd_start_host,
> + .start_gadget = dwc3_drd_start_gadget,
> +};
> +
> +static int dwc3_drd_notifier(struct notifier_block *nb,
> + unsigned long event, void *ptr)
> +{
> + struct dwc3 *dwc = container_of(nb, struct dwc3, otg_nb);
> +
> + dwc3_drd_fsm_sync(dwc);
> +
> + return NOTIFY_DONE;
> +}
> +
> +static int dwc3_drd_init(struct dwc3 *dwc)
> +{
> + int ret, id, vbus;
> + struct usb_otg_caps *otgcaps = &dwc->otg_config.otg_caps;
> +
> + otgcaps->otg_rev = 0;
> + otgcaps->hnp_support = false;
> + otgcaps->srp_support = false;
> + otgcaps->adp_support = false;
> + dwc->otg_config.fsm_ops = &dwc3_drd_ops;
> +
> + if (!dwc->edev) {
> + dev_err(dwc->dev, "No extcon device found for OTG mode\n");
> + return -ENODEV;
> + }
> +
> + dwc->otg_nb.notifier_call = dwc3_drd_notifier;
> + ret = extcon_register_notifier(dwc->edev, EXTCON_USB, &dwc->otg_nb);
> + if (ret < 0) {
> + dev_err(dwc->dev, "Couldn't register USB cable notifier\n");
> + return -ENODEV;
> + }
> +
> + ret = extcon_register_notifier(dwc->edev, EXTCON_USB_HOST,
> + &dwc->otg_nb);
> + if (ret < 0) {
> + dev_err(dwc->dev, "Couldn't register USB-HOST cable notifier\n");
> + ret = -ENODEV;
> + goto extcon_fail;
> + }
> +
> + /* sanity check id & vbus states */
> + id = extcon_get_cable_state(dwc->edev, "USB-HOST");
> + vbus = extcon_get_cable_state(dwc->edev, "USB");
> + if (id < 0 || vbus < 0) {
> + dev_err(dwc->dev, "Invalid USB cable state. id %d, vbus %d\n",
> + id, vbus);
> + ret = -ENODEV;
> + goto fail;
> + }
> +
> + /* register as DRD device with OTG core */
> + dwc->fsm = usb_otg_register(dwc->dev, &dwc->otg_config);
> + if (IS_ERR(dwc->fsm)) {
> + ret = PTR_ERR(dwc->fsm);
> + if (ret == -ENOTSUPP)
> + dev_err(dwc->dev, "CONFIG_USB_OTG needed for dual-role\n");
> + else
> + dev_err(dwc->dev, "Failed to register with OTG core\n");
do you need to cope with EPROBE_DEFER ?
> +
> + goto fail;
> + }
> +
> + dwc3_drd_fsm_sync(dwc);
> +
> + return 0;
> +fail:
> + extcon_unregister_notifier(dwc->edev, EXTCON_USB_HOST, &dwc->otg_nb);
> +extcon_fail:
> + extcon_unregister_notifier(dwc->edev, EXTCON_USB, &dwc->otg_nb);
> +
> + return ret;
> +}
> +
> +static void dwc3_drd_exit(struct dwc3 *dwc)
> +{
> + usb_otg_unregister(dwc->dev);
> + extcon_unregister_notifier(dwc->edev, EXTCON_USB_HOST, &dwc->otg_nb);
> + extcon_unregister_notifier(dwc->edev, EXTCON_USB, &dwc->otg_nb);
> +}
> +
> +/* -------------------------------------------------------------------------- */
> +
> static int dwc3_core_init_mode(struct dwc3 *dwc)
> {
> struct device *dev = dwc->dev;
> @@ -727,13 +873,21 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
> }
> break;
> case USB_DR_MODE_OTG:
> - dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG);
> + ret = dwc3_drd_init(dwc);
> + if (ret) {
> + dev_err(dev, "limiting to peripheral only\n");
> + dwc->dr_mode = USB_DR_MODE_PERIPHERAL;
> + dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
> + goto gadget_init;
> + }
> +
> ret = dwc3_host_init(dwc);
> if (ret) {
> dev_err(dev, "failed to initialize host\n");
> return ret;
> }
>
> +gadget_init:
> ret = dwc3_gadget_init(dwc);
> if (ret) {
> dev_err(dev, "failed to initialize gadget\n");
> @@ -760,6 +914,7 @@ static void dwc3_core_exit_mode(struct dwc3 *dwc)
> case USB_DR_MODE_OTG:
> dwc3_host_exit(dwc);
> dwc3_gadget_exit(dwc);
> + dwc3_drd_exit(dwc);
> break;
> default:
> /* do nothing */
> @@ -843,6 +998,16 @@ static int dwc3_probe(struct platform_device *pdev)
> hird_threshold = 12;
>
> if (node) {
> + if (of_property_read_bool(node, "extcon"))
> + dwc->edev = extcon_get_edev_by_phandle(dev, 0);
> + else if (of_property_read_bool(dev->parent->of_node, "extcon"))
> + dwc->edev = extcon_get_edev_by_phandle(dev->parent, 0);
why do you need to check the parent ? Why isn't that done on the glue
layer ?
> +
> + if (IS_ERR(dwc->edev)) {
> + dev_vdbg(dev, "couldn't get extcon device\n");
dev_err() ??
> + return -EPROBE_DEFER;
this could make us reschedule probe forever.
> + }
> +
> dwc->maximum_speed = of_usb_get_maximum_speed(node);
> dwc->has_lpm_erratum = of_property_read_bool(node,
> "snps,has-lpm-erratum");
> @@ -887,6 +1052,13 @@ static int dwc3_probe(struct platform_device *pdev)
> of_property_read_string(node, "snps,hsphy_interface",
> &dwc->hsphy_interface);
> } else if (pdata) {
> + if (pdata->extcon) {
> + dwc->edev = extcon_get_extcon_dev(pdata->extcon);
> + if (!dwc->edev) {
> + dev_vdbg(dev, "couldn't get extcon device\n");
> + return -EPROBE_DEFER;
ditto
> + }
> + }
> dwc->maximum_speed = pdata->maximum_speed;
> dwc->has_lpm_erratum = pdata->has_lpm_erratum;
> if (pdata->lpm_nyet_threshold)
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 0447788..5ca2b25 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -31,8 +31,10 @@
> #include <linux/usb/gadget.h>
> #include <linux/usb/otg.h>
> #include <linux/ulpi/interface.h>
> +#include <linux/usb/otg-fsm.h>
>
> #include <linux/phy/phy.h>
> +#include <linux/extcon.h>
>
> #define DWC3_MSG_MAX 500
>
> @@ -753,6 +755,11 @@ struct dwc3 {
>
> struct ulpi *ulpi;
>
> + struct extcon_dev *edev; /* USB cable events ID & VBUS */
> + struct notifier_block otg_nb; /* notifier for USB cable events */
> + struct otg_fsm *fsm;
> + struct usb_otg_config otg_config;
> +
> void __iomem *regs;
> size_t regs_size;
>
> diff --git a/drivers/usb/dwc3/platform_data.h b/drivers/usb/dwc3/platform_data.h
> index d3614ec..b3b245c 100644
> --- a/drivers/usb/dwc3/platform_data.h
> +++ b/drivers/usb/dwc3/platform_data.h
> @@ -47,4 +47,5 @@ struct dwc3_platform_data {
> unsigned tx_de_emphasis:2;
>
> const char *hsphy_interface;
> + const char *extcon; /* extcon name for USB cable events ID/VBUS */
> };
> --
> 2.1.4
>
--
balbi
[toc] | [prev] | [next] | [standalone]
| From | Roger Quadros <rogerq@ti.com> |
|---|---|
| Date | 2015-09-03 14:30 +0200 |
| Subject | Re: [PATCH v4 1/9] usb: dwc3: add dual-role support |
| Message-ID | <q4BOq-3tf-11@gated-at.bofh.it> |
| In reply to | #1217650 |
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
On 02/09/15 17:31, Felipe Balbi wrote:
> Hi,
>
> On Wed, Sep 02, 2015 at 05:24:16PM +0300, Roger Quadros wrote:
>> Register with the USB OTG core. Since we don't support
>> OTG yet we just work as a dual-role device even
>> if device tree says "otg".
>>
>> Use extcon framework to get VBUS/ID cable events and
>> kick the OTG state machine.
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>> drivers/usb/dwc3/core.c | 174 ++++++++++++++++++++++++++++++++++++++-
>> drivers/usb/dwc3/core.h | 7 ++
>> drivers/usb/dwc3/platform_data.h | 1 +
>> 3 files changed, 181 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 064123e..2e36a9b 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -704,6 +704,152 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
>> return 0;
>> }
>>
>> +/* --------------------- Dual-Role management ------------------------------- */
>> +
>> +static void dwc3_drd_fsm_sync(struct dwc3 *dwc)
>> +{
>> + int id, vbus;
>> +
>> + /* get ID */
>> + id = extcon_get_cable_state(dwc->edev, "USB-HOST");
>> + /* Host means ID == 0 */
>> + id = !id;
>> +
>> + /* get VBUS */
>> + vbus = extcon_get_cable_state(dwc->edev, "USB");
>> + dev_dbg(dwc->dev, "id %d vbus %d\n", id, vbus);
>
> tracepoint please. We don't want this driver to use dev_(v)?db anymore.
> Ditto to all others.
OK.
>
>> +
>> + dwc->fsm->id = id;
>> + dwc->fsm->b_sess_vld = vbus;
>> + usb_otg_sync_inputs(dwc->fsm);
>> +}
>> +
>> +static int dwc3_drd_start_host(struct otg_fsm *fsm, int on)
>> +{
>> + struct device *dev = usb_otg_fsm_to_dev(fsm);
>> + struct dwc3 *dwc = dev_get_drvdata(dev);
>
> how about adding a usb_otg_get_drvdata(fsm) ?
You meant for otg core? That can be done.
>
>> + dev_dbg(dwc->dev, "%s: %d\n", __func__, on);
>> + if (on) {
>> + dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
>> + /* start the HCD */
>> + usb_otg_start_host(fsm, true);
>> + } else {
>> + /* stop the HCD */
>> + usb_otg_start_host(fsm, false);
>> + }
>
> This can be simplified.
>
> if (on)
> dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
>
> usb_otg_start_host(fsm, on);
Indeed.
>
>> +
>> + return 0;
>> +}
>> +
>> +static int dwc3_drd_start_gadget(struct otg_fsm *fsm, int on)
>> +{
>> + struct device *dev = usb_otg_fsm_to_dev(fsm);
>> + struct dwc3 *dwc = dev_get_drvdata(dev);
>> +
>> + dev_dbg(dwc->dev, "%s: %d\n", __func__, on);
>> + if (on) {
>> + dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
>> + dwc3_event_buffers_setup(dwc);
>> +
>> + /* start the UDC */
>> + usb_otg_start_gadget(fsm, true);
>> + } else {
>> + /* stop the UDC */
>> + usb_otg_start_gadget(fsm, false);
>> + }
>
> likewise:
>
> if (on) {
> dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
> dwc3_event_buffers_setup(dwc);
> }
>
> usb_otg_start_gadget(fsm, on);
OK.
>
>> + return 0;
>> +}
>> +
>> +static struct otg_fsm_ops dwc3_drd_ops = {
>> + .start_host = dwc3_drd_start_host,
>> + .start_gadget = dwc3_drd_start_gadget,
>> +};
>> +
>> +static int dwc3_drd_notifier(struct notifier_block *nb,
>> + unsigned long event, void *ptr)
>> +{
>> + struct dwc3 *dwc = container_of(nb, struct dwc3, otg_nb);
>> +
>> + dwc3_drd_fsm_sync(dwc);
>> +
>> + return NOTIFY_DONE;
>> +}
>> +
>> +static int dwc3_drd_init(struct dwc3 *dwc)
>> +{
>> + int ret, id, vbus;
>> + struct usb_otg_caps *otgcaps = &dwc->otg_config.otg_caps;
>> +
>> + otgcaps->otg_rev = 0;
>> + otgcaps->hnp_support = false;
>> + otgcaps->srp_support = false;
>> + otgcaps->adp_support = false;
>> + dwc->otg_config.fsm_ops = &dwc3_drd_ops;
>> +
>> + if (!dwc->edev) {
>> + dev_err(dwc->dev, "No extcon device found for OTG mode\n");
>> + return -ENODEV;
>> + }
>> +
>> + dwc->otg_nb.notifier_call = dwc3_drd_notifier;
>> + ret = extcon_register_notifier(dwc->edev, EXTCON_USB, &dwc->otg_nb);
>> + if (ret < 0) {
>> + dev_err(dwc->dev, "Couldn't register USB cable notifier\n");
>> + return -ENODEV;
>> + }
>> +
>> + ret = extcon_register_notifier(dwc->edev, EXTCON_USB_HOST,
>> + &dwc->otg_nb);
>> + if (ret < 0) {
>> + dev_err(dwc->dev, "Couldn't register USB-HOST cable notifier\n");
>> + ret = -ENODEV;
>> + goto extcon_fail;
>> + }
>> +
>> + /* sanity check id & vbus states */
>> + id = extcon_get_cable_state(dwc->edev, "USB-HOST");
>> + vbus = extcon_get_cable_state(dwc->edev, "USB");
>> + if (id < 0 || vbus < 0) {
>> + dev_err(dwc->dev, "Invalid USB cable state. id %d, vbus %d\n",
>> + id, vbus);
>> + ret = -ENODEV;
>> + goto fail;
>> + }
>> +
>> + /* register as DRD device with OTG core */
>> + dwc->fsm = usb_otg_register(dwc->dev, &dwc->otg_config);
>> + if (IS_ERR(dwc->fsm)) {
>> + ret = PTR_ERR(dwc->fsm);
>> + if (ret == -ENOTSUPP)
>> + dev_err(dwc->dev, "CONFIG_USB_OTG needed for dual-role\n");
>> + else
>> + dev_err(dwc->dev, "Failed to register with OTG core\n");
>
> do you need to cope with EPROBE_DEFER ?
Yes, we should.
>
>> +
>> + goto fail;
>> + }
>> +
>> + dwc3_drd_fsm_sync(dwc);
>> +
>> + return 0;
>> +fail:
>> + extcon_unregister_notifier(dwc->edev, EXTCON_USB_HOST, &dwc->otg_nb);
>> +extcon_fail:
>> + extcon_unregister_notifier(dwc->edev, EXTCON_USB, &dwc->otg_nb);
>> +
>> + return ret;
>> +}
>> +
>> +static void dwc3_drd_exit(struct dwc3 *dwc)
>> +{
>> + usb_otg_unregister(dwc->dev);
>> + extcon_unregister_notifier(dwc->edev, EXTCON_USB_HOST, &dwc->otg_nb);
>> + extcon_unregister_notifier(dwc->edev, EXTCON_USB, &dwc->otg_nb);
>> +}
>> +
>> +/* -------------------------------------------------------------------------- */
>> +
>> static int dwc3_core_init_mode(struct dwc3 *dwc)
>> {
>> struct device *dev = dwc->dev;
>> @@ -727,13 +873,21 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
>> }
>> break;
>> case USB_DR_MODE_OTG:
>> - dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG);
>> + ret = dwc3_drd_init(dwc);
>> + if (ret) {
So here if ret is -EPROBE_DEFER, we return.
>> + dev_err(dev, "limiting to peripheral only\n");
>> + dwc->dr_mode = USB_DR_MODE_PERIPHERAL;
>> + dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
>> + goto gadget_init;
>> + }
>> +
>> ret = dwc3_host_init(dwc);
>> if (ret) {
>> dev_err(dev, "failed to initialize host\n");
>> return ret;
>> }
>>
>> +gadget_init:
>> ret = dwc3_gadget_init(dwc);
>> if (ret) {
>> dev_err(dev, "failed to initialize gadget\n");
>> @@ -760,6 +914,7 @@ static void dwc3_core_exit_mode(struct dwc3 *dwc)
>> case USB_DR_MODE_OTG:
>> dwc3_host_exit(dwc);
>> dwc3_gadget_exit(dwc);
>> + dwc3_drd_exit(dwc);
>> break;
>> default:
>> /* do nothing */
>> @@ -843,6 +998,16 @@ static int dwc3_probe(struct platform_device *pdev)
>> hird_threshold = 12;
>>
>> if (node) {
>> + if (of_property_read_bool(node, "extcon"))
>> + dwc->edev = extcon_get_edev_by_phandle(dev, 0);
>> + else if (of_property_read_bool(dev->parent->of_node, "extcon"))
>> + dwc->edev = extcon_get_edev_by_phandle(dev->parent, 0);
>
> why do you need to check the parent ? Why isn't that done on the glue
> layer ?
On DRA7-evm, the extcon device is defined in the glue layer node. But
we need the device both at the glue layer and at the core layer.
We do get the extcon device in dwc3-omap.c
Any suggestion how to pass the extcon device from glue layer to core.c?
Or should I add the extcon property to dwc3 USB node as well in the DT?
>
>> +
>> + if (IS_ERR(dwc->edev)) {
>> + dev_vdbg(dev, "couldn't get extcon device\n");
>
> dev_err() ??
Is it ok to print it even in EPROBE_DEFER case?
>
>> + return -EPROBE_DEFER;
>
> this could make us reschedule probe forever.
Good catch, we must return PTR_ERR(dwc->edev).
>
>> + }
>> +
>> dwc->maximum_speed = of_usb_get_maximum_speed(node);
>> dwc->has_lpm_erratum = of_property_read_bool(node,
>> "snps,has-lpm-erratum");
>> @@ -887,6 +1052,13 @@ static int dwc3_probe(struct platform_device *pdev)
>> of_property_read_string(node, "snps,hsphy_interface",
>> &dwc->hsphy_interface);
>> } else if (pdata) {
>> + if (pdata->extcon) {
>> + dwc->edev = extcon_get_extcon_dev(pdata->extcon);
>> + if (!dwc->edev) {
>> + dev_vdbg(dev, "couldn't get extcon device\n");
>> + return -EPROBE_DEFER;
>
> ditto
OK.
>
>> + }
>> + }
>> dwc->maximum_speed = pdata->maximum_speed;
>> dwc->has_lpm_erratum = pdata->has_lpm_erratum;
>> if (pdata->lpm_nyet_threshold)
>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>> index 0447788..5ca2b25 100644
>> --- a/drivers/usb/dwc3/core.h
>> +++ b/drivers/usb/dwc3/core.h
>> @@ -31,8 +31,10 @@
>> #include <linux/usb/gadget.h>
>> #include <linux/usb/otg.h>
>> #include <linux/ulpi/interface.h>
>> +#include <linux/usb/otg-fsm.h>
>>
>> #include <linux/phy/phy.h>
>> +#include <linux/extcon.h>
>>
>> #define DWC3_MSG_MAX 500
>>
>> @@ -753,6 +755,11 @@ struct dwc3 {
>>
>> struct ulpi *ulpi;
>>
>> + struct extcon_dev *edev; /* USB cable events ID & VBUS */
>> + struct notifier_block otg_nb; /* notifier for USB cable events */
>> + struct otg_fsm *fsm;
>> + struct usb_otg_config otg_config;
>> +
>> void __iomem *regs;
>> size_t regs_size;
>>
>> diff --git a/drivers/usb/dwc3/platform_data.h b/drivers/usb/dwc3/platform_data.h
>> index d3614ec..b3b245c 100644
>> --- a/drivers/usb/dwc3/platform_data.h
>> +++ b/drivers/usb/dwc3/platform_data.h
>> @@ -47,4 +47,5 @@ struct dwc3_platform_data {
>> unsigned tx_de_emphasis:2;
>>
>> const char *hsphy_interface;
>> + const char *extcon; /* extcon name for USB cable events ID/VBUS */
>> };
>> --
>> 2.1.4
>>
>
- --
cheers,
- -roger
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQIcBAEBCAAGBQJV6DtcAAoJENJaa9O+djCTWYcP/j/nseHR49Z1I8ftXv8urDGl
mMjMTzhlMxB7CYCtiqGpQ8Dytr9R7Ahm1llUVC4v6vYC5csJW1IcKxp/LOnaYmOL
d6EVFCSGM5JdEN83JoPfz/BD8qR/pWMmgzAGo60CkESev3E+/KxtcTl8md8CyOJj
DimMFsAmMxiVD2uABaW64Y96sG/lr8+b2AOksa6P1Os5n3GylNNegNHLSVMfx0b8
6lTn5uOa70fFZJTQDfF5idzgTrIT9irvP/GZ6RY1d6xLat4Hos1SWCjrDCHfgvHF
z3z39uW336/dpB56eomUSzctq0gVzzxG8lOIuVPXAJWEmwlx2FbV3M6viul56AGH
vtgWEB2LloLZxuXz60lnj3y8MSccUfMii+KILKtJVVjD7TW1BziW31juq+QZs+T2
yTT3YWuJVXpzob9jbzAenyU2ogDoS3vNA9cMRGOdr8cKg33z70rU/mfPcKlQKgWN
Ngve174czk9ZZgrF+l2r/m3SOqSbTDRqkxarP+KFrzNEn/hFPprSx8Encqg7Xr0t
EzUNQC64LmTxpzJwXzRuMuCqcu+plhuvSK93ymU5LkNqe+Bv5WtJaqXuxRCQ9rrY
Mn34QMFaKLz+9mLRvwGEp70MRafy44eJ5qslyj+0glL9mOB3AV7nyAArzeEJgY4/
vyyf4sA7JD9OL6NpkgVh
=ZhiZ
-----END PGP SIGNATURE-----
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Felipe Balbi <balbi@ti.com> |
|---|---|
| Date | 2015-09-03 18:10 +0200 |
| Subject | Re: [PATCH v4 1/9] usb: dwc3: add dual-role support |
| Message-ID | <q4Ffl-8tV-45@gated-at.bofh.it> |
| In reply to | #1218216 |
[Multipart message — attachments visible in raw view] — view raw
Hi,
On Thu, Sep 03, 2015 at 03:21:48PM +0300, Roger Quadros wrote:
> >> + dwc->fsm->id = id;
> >> + dwc->fsm->b_sess_vld = vbus;
> >> + usb_otg_sync_inputs(dwc->fsm);
> >> +}
> >> +
> >> +static int dwc3_drd_start_host(struct otg_fsm *fsm, int on)
> >> +{
> >> + struct device *dev = usb_otg_fsm_to_dev(fsm);
> >> + struct dwc3 *dwc = dev_get_drvdata(dev);
> >
> > how about adding a usb_otg_get_drvdata(fsm) ?
>
> You meant for otg core? That can be done.
yeah. BTW, I think otg core needs quite a few changes to become actually
useful. Currently it's just too much pointer ping-pong going back and
forth between phy, otg core, udc and hcd.
Also, I caught a ton of issues with it and suspend/resume. You might
want to fix them before adding more users to it.
It's also rather racy and that needs fixing too. On top of all that, I
think there's too much being added to UDC just to get Dual-Role, let's
see if we can improve that too.
> >> @@ -843,6 +998,16 @@ static int dwc3_probe(struct platform_device *pdev)
> >> hird_threshold = 12;
> >>
> >> if (node) {
> >> + if (of_property_read_bool(node, "extcon"))
> >> + dwc->edev = extcon_get_edev_by_phandle(dev, 0);
> >> + else if (of_property_read_bool(dev->parent->of_node, "extcon"))
> >> + dwc->edev = extcon_get_edev_by_phandle(dev->parent, 0);
> >
> > why do you need to check the parent ? Why isn't that done on the glue
> > layer ?
>
> On DRA7-evm, the extcon device is defined in the glue layer node. But
> we need the device both at the glue layer and at the core layer.
why do you need extcon here ? Glue updates core via UTMI about the
states, right ? So you should get proper VBUS and ID status via OTG IRQ.
Is that not working ?
> We do get the extcon device in dwc3-omap.c
>
> Any suggestion how to pass the extcon device from glue layer to core.c?
> Or should I add the extcon property to dwc3 USB node as well in the DT?
GPIO toggles
dwc3-omap extcon event
update status via UTMI STATUS register
OTG IRQ on core
Horray!
:-)
> >> +
> >> + if (IS_ERR(dwc->edev)) {
> >> + dev_vdbg(dev, "couldn't get extcon device\n");
> >
> > dev_err() ??
>
> Is it ok to print it even in EPROBE_DEFER case?
hmm, probably pointless, indeed.
--
balbi
[toc] | [prev] | [next] | [standalone]
| From | Roger Quadros <rogerq@ti.com> |
|---|---|
| Date | 2015-09-04 11:10 +0200 |
| Subject | Re: [PATCH v4 1/9] usb: dwc3: add dual-role support |
| Message-ID | <q4Vap-60u-7@gated-at.bofh.it> |
| In reply to | #1218415 |
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Felipe,
On 03/09/15 18:44, Felipe Balbi wrote:
> Hi,
>
> On Thu, Sep 03, 2015 at 03:21:48PM +0300, Roger Quadros wrote:
>>>> + dwc->fsm->id = id;
>>>> + dwc->fsm->b_sess_vld = vbus;
>>>> + usb_otg_sync_inputs(dwc->fsm);
>>>> +}
>>>> +
>>>> +static int dwc3_drd_start_host(struct otg_fsm *fsm, int on)
>>>> +{
>>>> + struct device *dev = usb_otg_fsm_to_dev(fsm);
>>>> + struct dwc3 *dwc = dev_get_drvdata(dev);
>>>
>>> how about adding a usb_otg_get_drvdata(fsm) ?
>>
>> You meant for otg core? That can be done.
>
> yeah. BTW, I think otg core needs quite a few changes to become actually
> useful. Currently it's just too much pointer ping-pong going back and
> forth between phy, otg core, udc and hcd.
Sure, any inputs for improvement appreciated.
>
> Also, I caught a ton of issues with it and suspend/resume. You might
> want to fix them before adding more users to it.
OK.
>
> It's also rather racy and that needs fixing too. On top of all that, I
> think there's too much being added to UDC just to get Dual-Role, let's
> see if we can improve that too.
Would appreciate if you could give all your inputs on the otg core thread
as early as you can :)
>
>>>> @@ -843,6 +998,16 @@ static int dwc3_probe(struct platform_device *pdev)
>>>> hird_threshold = 12;
>>>>
>>>> if (node) {
>>>> + if (of_property_read_bool(node, "extcon"))
>>>> + dwc->edev = extcon_get_edev_by_phandle(dev, 0);
>>>> + else if (of_property_read_bool(dev->parent->of_node, "extcon"))
>>>> + dwc->edev = extcon_get_edev_by_phandle(dev->parent, 0);
>>>
>>> why do you need to check the parent ? Why isn't that done on the glue
>>> layer ?
>>
>> On DRA7-evm, the extcon device is defined in the glue layer node. But
>> we need the device both at the glue layer and at the core layer.
>
> why do you need extcon here ? Glue updates core via UTMI about the
> states, right ? So you should get proper VBUS and ID status via OTG IRQ.
> Is that not working ?
I didn't even expect that would work. Let me give that a try.
>
>> We do get the extcon device in dwc3-omap.c
>>
>> Any suggestion how to pass the extcon device from glue layer to core.c?
>> Or should I add the extcon property to dwc3 USB node as well in the DT?
>
> GPIO toggles
> dwc3-omap extcon event
> update status via UTMI STATUS register
> OTG IRQ on core
> Horray!
>
> :-)
That's great. Thanks :)
>
>>>> +
>>>> + if (IS_ERR(dwc->edev)) {
>>>> + dev_vdbg(dev, "couldn't get extcon device\n");
>>>
>>> dev_err() ??
>>
>> Is it ok to print it even in EPROBE_DEFER case?
>
> hmm, probably pointless, indeed.
>
cheers,
- -roger
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQIcBAEBCAAGBQJV6V8UAAoJENJaa9O+djCToUwP/1SRrcUG/1KG54c1loxKFvYG
51x17TiVuQc68xtrp56HYJNRpZzZIwBqlfa3LU19bd/p6alYGaaY6jOqW+gFWWt8
tJd+7lyjVuCWkrJBQ6obK4yDPK4r6ZLcFlCTLGsfnd6SSBdsGxNrpcNCwz2rhhE/
uTPQdU4wvgxDFnGFVtTMhM+/ehtJtkKB4dppoFA5Vw86vsKinJ7o5EJMwho/PwJu
4D+mQVi17kQDBx1wkQPBxyVHD6RXfjMBLK+zg46T6lsg1eUodkv22Grf9Xy4i4wT
9Pr3g6SFnczkKiU1Bp7q4TV048SY0KedA7oe1U7K9B+hjHK4Fc2/vxtb3kGyNK7x
VPkN3NbvqSZalcgmdoDnv6VvU5NdnZUyKasVeJQDp9Fzaom9rUmNvic0ZLx9TOTP
4e4R2ovyCFnU1nODDUccYDInPiJ3EUo6D7CX3L7rfud5h6qAXcqC6vzZ3oHcSnBu
S1PXPTjLyk/a+den4gf41ReF5xKXzNocH21cXe7oFwKDaCJ9VgEn4+E5tX0vK778
KGHz3qFEQGkM6Eib0tEZEdHUEgeO8H99rNAELFNTrtS0FPZIEtQPHr2lmFDM/iYC
sEECc/HFP+LvmAzuJLA4XRUeNh6xo0K/ZDOtX1YOCCqnjy/UHNKvB4gUDzVaNti+
DB0wWXP+/A9Qz1PBK1dG
=rFBL
-----END PGP SIGNATURE-----
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Roger Quadros <rogerq@ti.com> |
|---|---|
| Date | 2015-09-02 16:30 +0200 |
| Subject | [PATCH v4 6/9] usb: dwc3: save/restore OTG registers during suspend/resume |
| Message-ID | <q4hd0-7Er-23@gated-at.bofh.it> |
| In reply to | #1217638 |
Without this we loose OTG controller register context and malfunction
after a system suspend-resume.
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
drivers/usb/dwc3/core.c | 17 +++++++++++++++++
drivers/usb/dwc3/core.h | 6 +++++-
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 632ee53..684010c 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -56,6 +56,7 @@ void dwc3_set_mode(struct dwc3 *dwc, u32 mode)
reg = dwc3_readl(dwc->regs, DWC3_GCTL);
reg &= ~(DWC3_GCTL_PRTCAPDIR(DWC3_GCTL_PRTCAP_OTG));
reg |= DWC3_GCTL_PRTCAPDIR(mode);
+ dwc->current_mode = mode;
dwc3_writel(dwc->regs, DWC3_GCTL, reg);
}
@@ -1443,6 +1444,14 @@ static int dwc3_suspend(struct device *dev)
spin_lock_irqsave(&dwc->lock, flags);
+ /* Save OTG state only if we're really using it */
+ if (dwc->current_mode == DWC3_GCTL_PRTCAP_OTG) {
+ dwc->ocfg = dwc3_readl(dwc->regs, DWC3_OCFG);
+ dwc->octl = dwc3_readl(dwc->regs, DWC3_OCTL);
+ dwc->oevt = dwc3_readl(dwc->regs, DWC3_OEVT);
+ dwc->oevten = dwc3_readl(dwc->regs, DWC3_OEVTEN);
+ }
+
switch (dwc->dr_mode) {
case USB_DR_MODE_PERIPHERAL:
case USB_DR_MODE_OTG:
@@ -1486,6 +1495,14 @@ static int dwc3_resume(struct device *dev)
dwc3_event_buffers_setup(dwc);
dwc3_writel(dwc->regs, DWC3_GCTL, dwc->gctl);
+ /* Restore OTG state only if we're really using it */
+ if (dwc->current_mode == DWC3_GCTL_PRTCAP_OTG) {
+ dwc3_writel(dwc->regs, DWC3_OCFG, dwc->ocfg);
+ dwc3_writel(dwc->regs, DWC3_OCTL, dwc->octl);
+ dwc3_writel(dwc->regs, DWC3_OEVT, dwc->oevt);
+ dwc3_writel(dwc->regs, DWC3_OEVTEN, dwc->oevten);
+ }
+
switch (dwc->dr_mode) {
case USB_DR_MODE_PERIPHERAL:
case USB_DR_MODE_OTG:
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 129ef37..1115ce0 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -737,6 +737,7 @@ struct dwc3_scratchpad_array {
* @regs: base address for our registers
* @regs_size: address space size
* @oevten: otg interrupt enable mask copy
+ * @current_mode: current mode of operation written to PRTCAPDIR
* @nr_scratch: number of scratch buffers
* @num_event_buffers: calculated number of event buffers
* @u1u2: only used on revisions <1.83a for workaround
@@ -858,9 +859,12 @@ struct dwc3 {
/* used for suspend/resume */
u32 dcfg;
u32 gctl;
-
+ u32 ocfg;
+ u32 octl;
+ u32 oevt;
u32 oevten;
+ u32 current_mode;
u32 nr_scratch;
u32 num_event_buffers;
u32 u1u2;
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Felipe Balbi <balbi@ti.com> |
|---|---|
| Date | 2015-09-02 16:50 +0200 |
| Subject | Re: [PATCH v4 6/9] usb: dwc3: save/restore OTG registers during suspend/resume |
| Message-ID | <q4hwn-81p-27@gated-at.bofh.it> |
| In reply to | #1217640 |
[Multipart message — attachments visible in raw view] — view raw
On Wed, Sep 02, 2015 at 05:24:21PM +0300, Roger Quadros wrote:
> Without this we loose OTG controller register context and malfunction
> after a system suspend-resume.
>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
> drivers/usb/dwc3/core.c | 17 +++++++++++++++++
> drivers/usb/dwc3/core.h | 6 +++++-
> 2 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 632ee53..684010c 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -56,6 +56,7 @@ void dwc3_set_mode(struct dwc3 *dwc, u32 mode)
> reg = dwc3_readl(dwc->regs, DWC3_GCTL);
> reg &= ~(DWC3_GCTL_PRTCAPDIR(DWC3_GCTL_PRTCAP_OTG));
> reg |= DWC3_GCTL_PRTCAPDIR(mode);
> + dwc->current_mode = mode;
> dwc3_writel(dwc->regs, DWC3_GCTL, reg);
> }
>
> @@ -1443,6 +1444,14 @@ static int dwc3_suspend(struct device *dev)
>
> spin_lock_irqsave(&dwc->lock, flags);
>
> + /* Save OTG state only if we're really using it */
> + if (dwc->current_mode == DWC3_GCTL_PRTCAP_OTG) {
> + dwc->ocfg = dwc3_readl(dwc->regs, DWC3_OCFG);
> + dwc->octl = dwc3_readl(dwc->regs, DWC3_OCTL);
> + dwc->oevt = dwc3_readl(dwc->regs, DWC3_OEVT);
oevt is what you use to clear pending IRQs, which means that ...
> + dwc->oevten = dwc3_readl(dwc->regs, DWC3_OEVTEN);
> + }
> +
> switch (dwc->dr_mode) {
> case USB_DR_MODE_PERIPHERAL:
> case USB_DR_MODE_OTG:
> @@ -1486,6 +1495,14 @@ static int dwc3_resume(struct device *dev)
> dwc3_event_buffers_setup(dwc);
> dwc3_writel(dwc->regs, DWC3_GCTL, dwc->gctl);
>
> + /* Restore OTG state only if we're really using it */
> + if (dwc->current_mode == DWC3_GCTL_PRTCAP_OTG) {
> + dwc3_writel(dwc->regs, DWC3_OCFG, dwc->ocfg);
> + dwc3_writel(dwc->regs, DWC3_OCTL, dwc->octl);
> + dwc3_writel(dwc->regs, DWC3_OEVT, dwc->oevt);
... you could be clearing pending IRQs right here.
> + dwc3_writel(dwc->regs, DWC3_OEVTEN, dwc->oevten);
> + }
> +
> switch (dwc->dr_mode) {
> case USB_DR_MODE_PERIPHERAL:
> case USB_DR_MODE_OTG:
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 129ef37..1115ce0 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -737,6 +737,7 @@ struct dwc3_scratchpad_array {
> * @regs: base address for our registers
> * @regs_size: address space size
> * @oevten: otg interrupt enable mask copy
> + * @current_mode: current mode of operation written to PRTCAPDIR
> * @nr_scratch: number of scratch buffers
> * @num_event_buffers: calculated number of event buffers
> * @u1u2: only used on revisions <1.83a for workaround
> @@ -858,9 +859,12 @@ struct dwc3 {
> /* used for suspend/resume */
> u32 dcfg;
> u32 gctl;
> -
> + u32 ocfg;
> + u32 octl;
> + u32 oevt;
> u32 oevten;
>
> + u32 current_mode;
> u32 nr_scratch;
> u32 num_event_buffers;
> u32 u1u2;
> --
> 2.1.4
>
--
balbi
[toc] | [prev] | [next] | [standalone]
| From | Roger Quadros <rogerq@ti.com> |
|---|---|
| Date | 2015-09-03 16:00 +0200 |
| Subject | Re: [PATCH v4 6/9] usb: dwc3: save/restore OTG registers during suspend/resume |
| Message-ID | <q4Ddw-5mr-19@gated-at.bofh.it> |
| In reply to | #1217673 |
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
On 02/09/15 17:44, Felipe Balbi wrote:
> On Wed, Sep 02, 2015 at 05:24:21PM +0300, Roger Quadros wrote:
>> Without this we loose OTG controller register context and malfunction
>> after a system suspend-resume.
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>> drivers/usb/dwc3/core.c | 17 +++++++++++++++++
>> drivers/usb/dwc3/core.h | 6 +++++-
>> 2 files changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 632ee53..684010c 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -56,6 +56,7 @@ void dwc3_set_mode(struct dwc3 *dwc, u32 mode)
>> reg = dwc3_readl(dwc->regs, DWC3_GCTL);
>> reg &= ~(DWC3_GCTL_PRTCAPDIR(DWC3_GCTL_PRTCAP_OTG));
>> reg |= DWC3_GCTL_PRTCAPDIR(mode);
>> + dwc->current_mode = mode;
>> dwc3_writel(dwc->regs, DWC3_GCTL, reg);
>> }
>>
>> @@ -1443,6 +1444,14 @@ static int dwc3_suspend(struct device *dev)
>>
>> spin_lock_irqsave(&dwc->lock, flags);
>>
>> + /* Save OTG state only if we're really using it */
>> + if (dwc->current_mode == DWC3_GCTL_PRTCAP_OTG) {
>> + dwc->ocfg = dwc3_readl(dwc->regs, DWC3_OCFG);
>> + dwc->octl = dwc3_readl(dwc->regs, DWC3_OCTL);
>> + dwc->oevt = dwc3_readl(dwc->regs, DWC3_OEVT);
>
> oevt is what you use to clear pending IRQs, which means that ...
>
>> + dwc->oevten = dwc3_readl(dwc->regs, DWC3_OEVTEN);
>> + }
>> +
>> switch (dwc->dr_mode) {
>> case USB_DR_MODE_PERIPHERAL:
>> case USB_DR_MODE_OTG:
>> @@ -1486,6 +1495,14 @@ static int dwc3_resume(struct device *dev)
>> dwc3_event_buffers_setup(dwc);
>> dwc3_writel(dwc->regs, DWC3_GCTL, dwc->gctl);
>>
>> + /* Restore OTG state only if we're really using it */
>> + if (dwc->current_mode == DWC3_GCTL_PRTCAP_OTG) {
>> + dwc3_writel(dwc->regs, DWC3_OCFG, dwc->ocfg);
>> + dwc3_writel(dwc->regs, DWC3_OCTL, dwc->octl);
>> + dwc3_writel(dwc->regs, DWC3_OEVT, dwc->oevt);
>
> ... you could be clearing pending IRQs right here.
Good catch. So we can't really restore this register.
I'll remove this line.
>
>> + dwc3_writel(dwc->regs, DWC3_OEVTEN, dwc->oevten);
>> + }
>> +
>> switch (dwc->dr_mode) {
>> case USB_DR_MODE_PERIPHERAL:
>> case USB_DR_MODE_OTG:
>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>> index 129ef37..1115ce0 100644
>> --- a/drivers/usb/dwc3/core.h
>> +++ b/drivers/usb/dwc3/core.h
>> @@ -737,6 +737,7 @@ struct dwc3_scratchpad_array {
>> * @regs: base address for our registers
>> * @regs_size: address space size
>> * @oevten: otg interrupt enable mask copy
>> + * @current_mode: current mode of operation written to PRTCAPDIR
>> * @nr_scratch: number of scratch buffers
>> * @num_event_buffers: calculated number of event buffers
>> * @u1u2: only used on revisions <1.83a for workaround
>> @@ -858,9 +859,12 @@ struct dwc3 {
>> /* used for suspend/resume */
>> u32 dcfg;
>> u32 gctl;
>> -
>> + u32 ocfg;
>> + u32 octl;
>> + u32 oevt;
>> u32 oevten;
>>
>> + u32 current_mode;
>> u32 nr_scratch;
>> u32 num_event_buffers;
>> u32 u1u2;
>> --
>> 2.1.4
>>
>
- --
cheers,
- -roger
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQIcBAEBCAAGBQJV6FEGAAoJENJaa9O+djCTE7gP/0rlQJPL1R11k8hJCM842zjT
SSxfdNQ9kkG008D88As8nLodL79agvD/XBot7iIrTGVFMG/tzzkwQb71gZoCOogb
K0eMMnLNyzLRapNpOe1OJqI1NjO8Yx1o7heoaGkGYERR7m6dwz1LmSPYEtaaveVq
5gb5NbUKfSq0Cs6wM+3+U8CQzAPLf7eRSZwiORkzyn7cWiZDsYHr9Bn35NA24r5v
0YMQvmTp7K/7pFENiFHmOWbKPoY6G8ebkInBSMXQfHcLPtL86l9e+VrlJrIZzBqQ
SJyoQw8JFU9c2ojtKeG9AYn8fSbCQpbsuIFAZtCVy2rK4xMiITmaSsjHF5+KpBt2
cE7eahicYAP3edLqT0R0hYfiI+JrVuOdkiW+W8UWPHrGt/nELQjQxZtIkTOI1b9B
FIxnZbIwp0veEE+vqXiTQizUPdZag0qPG1PZ5Bpu0867vfX6khY2MCNeYTDatzHT
dz9RwWIDH3/K5K7TOug/VZEiqvLY3BYMrMK+W3FreGfagwvYvlLyojsHifrA1x9z
1+zAjkml1kIIP5Jlf3VOYdLsN8mgeMyCNfi5USvVFnzPITAjtwm9+ai7xxxCjSxA
QTvmhQRjAxIPWWZgVuc/Z5PdqgHKFKpCLxG8IE9bqGBQnNSKueZHEhfUCxfPNKV6
ZkkhIq/s6hm6S/YM4WcS
=hUag
-----END PGP SIGNATURE-----
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Roger Quadros <rogerq@ti.com> |
|---|---|
| Date | 2015-09-02 16:30 +0200 |
| Subject | [PATCH v4 3/9] usb: dwc3: dwc3-omap: Make the wrapper interrupt shared |
| Message-ID | <q4hd0-7Er-21@gated-at.bofh.it> |
| In reply to | #1217638 |
The wrapper interrupt is shared with OTG core so mark it IRQF_SHARED.
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
drivers/usb/dwc3/dwc3-omap.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index a5a1b7c..b18f2a3 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -506,8 +506,8 @@ static int dwc3_omap_probe(struct platform_device *pdev)
reg = dwc3_omap_readl(omap->base, USBOTGSS_SYSCONFIG);
omap->dma_status = !!(reg & USBOTGSS_SYSCONFIG_DMADISABLE);
- ret = devm_request_irq(dev, omap->irq, dwc3_omap_interrupt, 0,
- "dwc3-omap", omap);
+ ret = devm_request_irq(dev, omap->irq, dwc3_omap_interrupt, IRQF_SHARED,
+ "dwc3-omap", omap);
if (ret) {
dev_err(dev, "failed to request IRQ #%d --> %d\n",
omap->irq, ret);
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Felipe Balbi <balbi@ti.com> |
|---|---|
| Date | 2015-09-02 16:40 +0200 |
| Subject | Re: [PATCH v4 3/9] usb: dwc3: dwc3-omap: Make the wrapper interrupt shared |
| Message-ID | <q4hmG-7PP-19@gated-at.bofh.it> |
| In reply to | #1217642 |
[Multipart message — attachments visible in raw view] — view raw
On Wed, Sep 02, 2015 at 05:24:18PM +0300, Roger Quadros wrote:
> The wrapper interrupt is shared with OTG core so mark it IRQF_SHARED.
>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
> drivers/usb/dwc3/dwc3-omap.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
> index a5a1b7c..b18f2a3 100644
> --- a/drivers/usb/dwc3/dwc3-omap.c
> +++ b/drivers/usb/dwc3/dwc3-omap.c
> @@ -506,8 +506,8 @@ static int dwc3_omap_probe(struct platform_device *pdev)
> reg = dwc3_omap_readl(omap->base, USBOTGSS_SYSCONFIG);
> omap->dma_status = !!(reg & USBOTGSS_SYSCONFIG_DMADISABLE);
>
> - ret = devm_request_irq(dev, omap->irq, dwc3_omap_interrupt, 0,
> - "dwc3-omap", omap);
> + ret = devm_request_irq(dev, omap->irq, dwc3_omap_interrupt, IRQF_SHARED,
> + "dwc3-omap", omap);
looks like only first line needs to change
> if (ret) {
> dev_err(dev, "failed to request IRQ #%d --> %d\n",
> omap->irq, ret);
> --
> 2.1.4
>
--
balbi
[toc] | [prev] | [next] | [standalone]
| From | Roger Quadros <rogerq@ti.com> |
|---|---|
| Date | 2015-09-02 16:30 +0200 |
| Subject | [PATCH v4 5/9] usb: dwc3: core: make dual-role work with OTG irq |
| Message-ID | <q4hd0-7Er-33@gated-at.bofh.it> |
| In reply to | #1217638 |
If the ID pin event is not available over extcon
then we rely on the OTG controller to provide us ID and VBUS
information.
We still don't support any OTG features but just
dual-role operation.
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
drivers/usb/dwc3/core.c | 217 ++++++++++++++++++++++++++++++++++++++++++++----
drivers/usb/dwc3/core.h | 3 +
2 files changed, 205 insertions(+), 15 deletions(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 38b31df..632ee53 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -704,6 +704,63 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
return 0;
}
+/* Get OTG events and sync it to OTG fsm */
+static void dwc3_otg_fsm_sync(struct dwc3 *dwc)
+{
+ u32 reg;
+ int id, vbus;
+
+ reg = dwc3_readl(dwc->regs, DWC3_OSTS);
+ dev_dbg(dwc->dev, "otgstatus 0x%x\n", reg);
+
+ id = !!(reg & DWC3_OSTS_CONIDSTS);
+ vbus = !!(reg & DWC3_OSTS_BSESVLD);
+
+ if (id != dwc->fsm->id || vbus != dwc->fsm->b_sess_vld) {
+ dev_dbg(dwc->dev, "id %d vbus %d\n", id, vbus);
+ dwc->fsm->id = id;
+ dwc->fsm->b_sess_vld = vbus;
+ usb_otg_sync_inputs(dwc->fsm);
+ }
+}
+
+static irqreturn_t dwc3_otg_thread_irq(int irq, void *_dwc)
+{
+ struct dwc3 *dwc = _dwc;
+ unsigned long flags;
+ irqreturn_t ret = IRQ_NONE;
+
+ spin_lock_irqsave(&dwc->lock, flags);
+ dwc3_otg_fsm_sync(dwc);
+ /* unmask interrupts */
+ dwc3_writel(dwc->regs, DWC3_OEVTEN, dwc->oevten);
+ spin_unlock_irqrestore(&dwc->lock, flags);
+
+ return ret;
+}
+
+static irqreturn_t dwc3_otg_irq(int irq, void *_dwc)
+{
+ struct dwc3 *dwc = _dwc;
+ irqreturn_t ret = IRQ_NONE;
+ u32 reg;
+
+ spin_lock(&dwc->lock);
+
+ reg = dwc3_readl(dwc->regs, DWC3_OEVT);
+ if (reg) {
+ dwc3_writel(dwc->regs, DWC3_OEVT, reg);
+ /* mask interrupts till processed */
+ dwc->oevten = dwc3_readl(dwc->regs, DWC3_OEVTEN);
+ dwc3_writel(dwc->regs, DWC3_OEVTEN, 0);
+ ret = IRQ_WAKE_THREAD;
+ }
+
+ spin_unlock(&dwc->lock);
+
+ return ret;
+}
+
/* --------------------- Dual-Role management ------------------------------- */
static void dwc3_drd_fsm_sync(struct dwc3 *dwc)
@@ -728,15 +785,44 @@ static int dwc3_drd_start_host(struct otg_fsm *fsm, int on)
{
struct device *dev = usb_otg_fsm_to_dev(fsm);
struct dwc3 *dwc = dev_get_drvdata(dev);
+ u32 reg;
dev_dbg(dwc->dev, "%s: %d\n", __func__, on);
+ if (dwc->edev) {
+ if (on) {
+ dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
+ /* start the HCD */
+ usb_otg_start_host(fsm, true);
+ } else {
+ /* stop the HCD */
+ usb_otg_start_host(fsm, false);
+ }
+
+ return 0;
+ }
+
+ /* switch OTG core */
if (on) {
- dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
+ /* OCTL.PeriMode = 0 */
+ reg = dwc3_readl(dwc->regs, DWC3_OCTL);
+ reg &= ~DWC3_OCTL_PERIMODE;
+ dwc3_writel(dwc->regs, DWC3_OCTL, reg);
+ /* unconditionally turn on VBUS */
+ reg |= DWC3_OCTL_PRTPWRCTL;
+ dwc3_writel(dwc->regs, DWC3_OCTL, reg);
/* start the HCD */
usb_otg_start_host(fsm, true);
} else {
/* stop the HCD */
usb_otg_start_host(fsm, false);
+ /* turn off VBUS */
+ reg = dwc3_readl(dwc->regs, DWC3_OCTL);
+ reg &= ~DWC3_OCTL_PRTPWRCTL;
+ dwc3_writel(dwc->regs, DWC3_OCTL, reg);
+ /* OCTL.PeriMode = 1 */
+ reg = dwc3_readl(dwc->regs, DWC3_OCTL);
+ reg |= DWC3_OCTL_PERIMODE;
+ dwc3_writel(dwc->regs, DWC3_OCTL, reg);
}
return 0;
@@ -746,15 +832,48 @@ static int dwc3_drd_start_gadget(struct otg_fsm *fsm, int on)
{
struct device *dev = usb_otg_fsm_to_dev(fsm);
struct dwc3 *dwc = dev_get_drvdata(dev);
+ u32 reg;
dev_dbg(dwc->dev, "%s: %d\n", __func__, on);
- if (on) {
- dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
+ if (on)
dwc3_event_buffers_setup(dwc);
+ if (dwc->edev) {
+ if (on) {
+ dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
+ usb_otg_start_gadget(fsm, true);
+ } else {
+ usb_otg_start_gadget(fsm, false);
+ }
+
+ return 0;
+ }
+
+ /* switch OTG core */
+ if (on) {
+ /* OCTL.PeriMode = 1 */
+ reg = dwc3_readl(dwc->regs, DWC3_OCTL);
+ reg |= DWC3_OCTL_PERIMODE;
+ dwc3_writel(dwc->regs, DWC3_OCTL, reg);
+ /* GUSB2PHYCFG0.SusPHY = 1 */
+ if (!dwc->dis_u2_susphy_quirk) {
+ reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
+ reg |= DWC3_GUSB2PHYCFG_SUSPHY;
+ dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
+ }
/* start the UDC */
usb_otg_start_gadget(fsm, true);
} else {
+ /* GUSB2PHYCFG0.SusPHY=0 */
+ if (!dwc->dis_u2_susphy_quirk) {
+ reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
+ reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
+ dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
+ }
+ /* OCTL.PeriMode = 1 */
+ reg = dwc3_readl(dwc->regs, DWC3_OCTL);
+ reg |= DWC3_OCTL_PERIMODE;
+ dwc3_writel(dwc->regs, DWC3_OCTL, reg);
/* stop the UDC */
usb_otg_start_gadget(fsm, false);
}
@@ -777,10 +896,30 @@ static int dwc3_drd_notifier(struct notifier_block *nb,
return NOTIFY_DONE;
}
+static int dwc3_drd_register(struct dwc3 *dwc)
+{
+ int ret;
+
+ /* register parent as DRD device with OTG core */
+ dwc->fsm = usb_otg_register(dwc->dev, &dwc->otg_config);
+ if (IS_ERR(dwc->fsm)) {
+ ret = PTR_ERR(dwc->fsm);
+ if (ret == -ENOTSUPP)
+ dev_err(dwc->dev, "CONFIG_USB_OTG needed for dual-role\n");
+ else
+ dev_err(dwc->dev, "Failed to register with OTG core\n");
+
+ return ret;
+ }
+
+ return 0;
+}
+
static int dwc3_drd_init(struct dwc3 *dwc)
{
int ret, id, vbus;
struct usb_otg_caps *otgcaps = &dwc->otg_config.otg_caps;
+ u32 reg;
otgcaps->otg_rev = 0;
otgcaps->hnp_support = false;
@@ -788,9 +927,10 @@ static int dwc3_drd_init(struct dwc3 *dwc)
otgcaps->adp_support = false;
dwc->otg_config.fsm_ops = &dwc3_drd_ops;
+ /* If extcon device is not present we rely on OTG core for ID event */
if (!dwc->edev) {
- dev_err(dwc->dev, "No extcon device found for OTG mode\n");
- return -ENODEV;
+ dev_dbg(dwc->dev, "No extcon device found for OTG mode\n");
+ goto try_otg_core;
}
dwc->otg_nb.notifier_call = dwc3_drd_notifier;
@@ -818,17 +958,9 @@ static int dwc3_drd_init(struct dwc3 *dwc)
goto fail;
}
- /* register as DRD device with OTG core */
- dwc->fsm = usb_otg_register(dwc->dev, &dwc->otg_config);
- if (IS_ERR(dwc->fsm)) {
- ret = PTR_ERR(dwc->fsm);
- if (ret == -ENOTSUPP)
- dev_err(dwc->dev, "CONFIG_USB_OTG needed for dual-role\n");
- else
- dev_err(dwc->dev, "Failed to register with OTG core\n");
-
+ ret = dwc3_drd_register(dwc);
+ if (ret)
goto fail;
- }
dwc3_drd_fsm_sync(dwc);
@@ -839,6 +971,61 @@ extcon_fail:
extcon_unregister_notifier(dwc->edev, EXTCON_USB, &dwc->otg_nb);
return ret;
+
+try_otg_core:
+ ret = dwc3_drd_register(dwc);
+ if (ret)
+ return ret;
+
+ /* disable all irqs */
+ dwc3_writel(dwc->regs, DWC3_OEVTEN, 0);
+ /* clear all events */
+ dwc3_writel(dwc->regs, DWC3_OEVT, ~0);
+
+ ret = request_threaded_irq(dwc->otg_irq, dwc3_otg_irq,
+ dwc3_otg_thread_irq,
+ IRQF_SHARED, "dwc3-otg", dwc);
+ if (ret) {
+ dev_err(dwc->dev, "failed to request irq #%d --> %d\n",
+ dwc->otg_irq, ret);
+ ret = -ENODEV;
+ goto error;
+ }
+
+ /* we need to set OTG to get events from OTG core */
+ dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG);
+ /* GUSB2PHYCFG0.SusPHY=0 */
+ if (!dwc->dis_u2_susphy_quirk) {
+ reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
+ reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
+ dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
+ }
+
+ /* Initialize OTG registers */
+ /*
+ * Prevent host/device reset from resetting OTG core.
+ * If we don't do this then xhci_reset (USBCMD.HCRST) will reset
+ * the signal outputs sent to the PHY, the OTG FSM logic of the
+ * core and also the resets to the VBUS filters inside the core.
+ */
+ reg = DWC3_OCFG_SFTRSTMASK;
+ dwc3_writel(dwc->regs, DWC3_OCFG, reg);
+ /* Enable ID event interrupt */
+ dwc3_writel(dwc->regs, DWC3_OEVTEN, DWC3_OEVTEN_CONIDSTSCHNGEN |
+ DWC3_OEVTEN_BDEVVBUSCHNGE |
+ DWC3_OEVTEN_BDEVSESSVLDDETEN);
+ /* OCTL.PeriMode = 1 */
+ dwc3_writel(dwc->regs, DWC3_OCTL, DWC3_OCTL_PERIMODE);
+
+ dwc3_otg_fsm_sync(dwc);
+ usb_otg_sync_inputs(dwc->fsm);
+
+ return 0;
+
+error:
+ usb_otg_unregister(dwc->dev);
+
+ return ret;
}
static void dwc3_drd_exit(struct dwc3 *dwc)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index bd32cb2..129ef37 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -736,6 +736,7 @@ struct dwc3_scratchpad_array {
* @gadget_driver: pointer to the gadget driver
* @regs: base address for our registers
* @regs_size: address space size
+ * @oevten: otg interrupt enable mask copy
* @nr_scratch: number of scratch buffers
* @num_event_buffers: calculated number of event buffers
* @u1u2: only used on revisions <1.83a for workaround
@@ -858,6 +859,8 @@ struct dwc3 {
u32 dcfg;
u32 gctl;
+ u32 oevten;
+
u32 nr_scratch;
u32 num_event_buffers;
u32 u1u2;
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Felipe Balbi <balbi@ti.com> |
|---|---|
| Date | 2015-09-02 16:50 +0200 |
| Subject | Re: [PATCH v4 5/9] usb: dwc3: core: make dual-role work with OTG irq |
| Message-ID | <q4hwn-81p-33@gated-at.bofh.it> |
| In reply to | #1217643 |
[Multipart message — attachments visible in raw view] — view raw
Hi,
On Wed, Sep 02, 2015 at 05:24:20PM +0300, Roger Quadros wrote:
> If the ID pin event is not available over extcon
> then we rely on the OTG controller to provide us ID and VBUS
> information.
>
> We still don't support any OTG features but just
> dual-role operation.
>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
> drivers/usb/dwc3/core.c | 217 ++++++++++++++++++++++++++++++++++++++++++++----
> drivers/usb/dwc3/core.h | 3 +
> 2 files changed, 205 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 38b31df..632ee53 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -704,6 +704,63 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
> return 0;
> }
>
> +/* Get OTG events and sync it to OTG fsm */
> +static void dwc3_otg_fsm_sync(struct dwc3 *dwc)
> +{
> + u32 reg;
> + int id, vbus;
> +
> + reg = dwc3_readl(dwc->regs, DWC3_OSTS);
> + dev_dbg(dwc->dev, "otgstatus 0x%x\n", reg);
> +
> + id = !!(reg & DWC3_OSTS_CONIDSTS);
> + vbus = !!(reg & DWC3_OSTS_BSESVLD);
> +
> + if (id != dwc->fsm->id || vbus != dwc->fsm->b_sess_vld) {
> + dev_dbg(dwc->dev, "id %d vbus %d\n", id, vbus);
> + dwc->fsm->id = id;
> + dwc->fsm->b_sess_vld = vbus;
> + usb_otg_sync_inputs(dwc->fsm);
> + }
this guy shouldn't try to filter events here. That's what the FSM should
be doing.
> +}
> +
> +static irqreturn_t dwc3_otg_thread_irq(int irq, void *_dwc)
> +{
> + struct dwc3 *dwc = _dwc;
> + unsigned long flags;
> + irqreturn_t ret = IRQ_NONE;
this IRQ will be disabled pretty quickly. You *always* return IRQ_NONE
> + spin_lock_irqsave(&dwc->lock, flags);
if you cache current OSTS in dwc3, you can use that instead and change
this to a spin_lock() instead of disabling IRQs here. This device's IRQs
are already masked anyway.
> + dwc3_otg_fsm_sync(dwc);
> + /* unmask interrupts */
> + dwc3_writel(dwc->regs, DWC3_OEVTEN, dwc->oevten);
> + spin_unlock_irqrestore(&dwc->lock, flags);
> +
> + return ret;
> +}
> +
> +static irqreturn_t dwc3_otg_irq(int irq, void *_dwc)
> +{
> + struct dwc3 *dwc = _dwc;
> + irqreturn_t ret = IRQ_NONE;
> + u32 reg;
> +
> + spin_lock(&dwc->lock);
this seems unnecessary, we're already in hardirq with IRQs disabled.
What sort of race could we have ? (in fact, this also needs change in
dwc3/gadget.c).
> +
> + reg = dwc3_readl(dwc->regs, DWC3_OEVT);
> + if (reg) {
> + dwc3_writel(dwc->regs, DWC3_OEVT, reg);
> + /* mask interrupts till processed */
> + dwc->oevten = dwc3_readl(dwc->regs, DWC3_OEVTEN);
> + dwc3_writel(dwc->regs, DWC3_OEVTEN, 0);
> + ret = IRQ_WAKE_THREAD;
> + }
> +
> + spin_unlock(&dwc->lock);
> +
> + return ret;
> +}
> +
> /* --------------------- Dual-Role management ------------------------------- */
>
> static void dwc3_drd_fsm_sync(struct dwc3 *dwc)
> @@ -728,15 +785,44 @@ static int dwc3_drd_start_host(struct otg_fsm *fsm, int on)
> {
> struct device *dev = usb_otg_fsm_to_dev(fsm);
> struct dwc3 *dwc = dev_get_drvdata(dev);
> + u32 reg;
>
> dev_dbg(dwc->dev, "%s: %d\n", __func__, on);
> + if (dwc->edev) {
> + if (on) {
> + dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
> + /* start the HCD */
> + usb_otg_start_host(fsm, true);
> + } else {
> + /* stop the HCD */
> + usb_otg_start_host(fsm, false);
> + }
if (on)
dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
usb_otg_start_host(fsm, on);
> +
> + return 0;
> + }
> +
> + /* switch OTG core */
> if (on) {
> - dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
> + /* OCTL.PeriMode = 0 */
> + reg = dwc3_readl(dwc->regs, DWC3_OCTL);
> + reg &= ~DWC3_OCTL_PERIMODE;
> + dwc3_writel(dwc->regs, DWC3_OCTL, reg);
> + /* unconditionally turn on VBUS */
> + reg |= DWC3_OCTL_PRTPWRCTL;
> + dwc3_writel(dwc->regs, DWC3_OCTL, reg);
> /* start the HCD */
> usb_otg_start_host(fsm, true);
> } else {
> /* stop the HCD */
> usb_otg_start_host(fsm, false);
> + /* turn off VBUS */
> + reg = dwc3_readl(dwc->regs, DWC3_OCTL);
> + reg &= ~DWC3_OCTL_PRTPWRCTL;
> + dwc3_writel(dwc->regs, DWC3_OCTL, reg);
> + /* OCTL.PeriMode = 1 */
> + reg = dwc3_readl(dwc->regs, DWC3_OCTL);
> + reg |= DWC3_OCTL_PERIMODE;
> + dwc3_writel(dwc->regs, DWC3_OCTL, reg);
> }
it looks like you're not really following the fluxchart from SNPS
documentation, see Figure 11-4 on section 11.1.4.5
> @@ -746,15 +832,48 @@ static int dwc3_drd_start_gadget(struct otg_fsm *fsm, int on)
> {
> struct device *dev = usb_otg_fsm_to_dev(fsm);
> struct dwc3 *dwc = dev_get_drvdata(dev);
> + u32 reg;
>
> dev_dbg(dwc->dev, "%s: %d\n", __func__, on);
> - if (on) {
> - dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
> + if (on)
> dwc3_event_buffers_setup(dwc);
>
> + if (dwc->edev) {
> + if (on) {
> + dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
> + usb_otg_start_gadget(fsm, true);
> + } else {
> + usb_otg_start_gadget(fsm, false);
> + }
> +
> + return 0;
> + }
> +
> + /* switch OTG core */
> + if (on) {
> + /* OCTL.PeriMode = 1 */
> + reg = dwc3_readl(dwc->regs, DWC3_OCTL);
> + reg |= DWC3_OCTL_PERIMODE;
> + dwc3_writel(dwc->regs, DWC3_OCTL, reg);
> + /* GUSB2PHYCFG0.SusPHY = 1 */
> + if (!dwc->dis_u2_susphy_quirk) {
> + reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> + reg |= DWC3_GUSB2PHYCFG_SUSPHY;
> + dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> + }
> /* start the UDC */
> usb_otg_start_gadget(fsm, true);
> } else {
> + /* GUSB2PHYCFG0.SusPHY=0 */
> + if (!dwc->dis_u2_susphy_quirk) {
> + reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> + reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
> + dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> + }
> + /* OCTL.PeriMode = 1 */
> + reg = dwc3_readl(dwc->regs, DWC3_OCTL);
> + reg |= DWC3_OCTL_PERIMODE;
> + dwc3_writel(dwc->regs, DWC3_OCTL, reg);
> /* stop the UDC */
> usb_otg_start_gadget(fsm, false);
> }
> @@ -777,10 +896,30 @@ static int dwc3_drd_notifier(struct notifier_block *nb,
> return NOTIFY_DONE;
> }
>
> +static int dwc3_drd_register(struct dwc3 *dwc)
> +{
> + int ret;
> +
> + /* register parent as DRD device with OTG core */
> + dwc->fsm = usb_otg_register(dwc->dev, &dwc->otg_config);
> + if (IS_ERR(dwc->fsm)) {
> + ret = PTR_ERR(dwc->fsm);
> + if (ret == -ENOTSUPP)
> + dev_err(dwc->dev, "CONFIG_USB_OTG needed for dual-role\n");
> + else
> + dev_err(dwc->dev, "Failed to register with OTG core\n");
> +
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> static int dwc3_drd_init(struct dwc3 *dwc)
> {
> int ret, id, vbus;
> struct usb_otg_caps *otgcaps = &dwc->otg_config.otg_caps;
> + u32 reg;
>
> otgcaps->otg_rev = 0;
> otgcaps->hnp_support = false;
> @@ -788,9 +927,10 @@ static int dwc3_drd_init(struct dwc3 *dwc)
> otgcaps->adp_support = false;
> dwc->otg_config.fsm_ops = &dwc3_drd_ops;
>
> + /* If extcon device is not present we rely on OTG core for ID event */
> if (!dwc->edev) {
> - dev_err(dwc->dev, "No extcon device found for OTG mode\n");
> - return -ENODEV;
> + dev_dbg(dwc->dev, "No extcon device found for OTG mode\n");
> + goto try_otg_core;
> }
>
> dwc->otg_nb.notifier_call = dwc3_drd_notifier;
> @@ -818,17 +958,9 @@ static int dwc3_drd_init(struct dwc3 *dwc)
> goto fail;
> }
>
> - /* register as DRD device with OTG core */
> - dwc->fsm = usb_otg_register(dwc->dev, &dwc->otg_config);
> - if (IS_ERR(dwc->fsm)) {
> - ret = PTR_ERR(dwc->fsm);
> - if (ret == -ENOTSUPP)
> - dev_err(dwc->dev, "CONFIG_USB_OTG needed for dual-role\n");
> - else
> - dev_err(dwc->dev, "Failed to register with OTG core\n");
> -
> + ret = dwc3_drd_register(dwc);
> + if (ret)
> goto fail;
> - }
>
> dwc3_drd_fsm_sync(dwc);
>
> @@ -839,6 +971,61 @@ extcon_fail:
> extcon_unregister_notifier(dwc->edev, EXTCON_USB, &dwc->otg_nb);
>
> return ret;
> +
> +try_otg_core:
> + ret = dwc3_drd_register(dwc);
> + if (ret)
> + return ret;
> +
> + /* disable all irqs */
> + dwc3_writel(dwc->regs, DWC3_OEVTEN, 0);
> + /* clear all events */
> + dwc3_writel(dwc->regs, DWC3_OEVT, ~0);
> +
> + ret = request_threaded_irq(dwc->otg_irq, dwc3_otg_irq,
> + dwc3_otg_thread_irq,
> + IRQF_SHARED, "dwc3-otg", dwc);
> + if (ret) {
> + dev_err(dwc->dev, "failed to request irq #%d --> %d\n",
> + dwc->otg_irq, ret);
> + ret = -ENODEV;
> + goto error;
> + }
> +
> + /* we need to set OTG to get events from OTG core */
> + dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG);
> + /* GUSB2PHYCFG0.SusPHY=0 */
> + if (!dwc->dis_u2_susphy_quirk) {
> + reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> + reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
> + dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> + }
> +
> + /* Initialize OTG registers */
> + /*
> + * Prevent host/device reset from resetting OTG core.
> + * If we don't do this then xhci_reset (USBCMD.HCRST) will reset
> + * the signal outputs sent to the PHY, the OTG FSM logic of the
> + * core and also the resets to the VBUS filters inside the core.
> + */
> + reg = DWC3_OCFG_SFTRSTMASK;
> + dwc3_writel(dwc->regs, DWC3_OCFG, reg);
> + /* Enable ID event interrupt */
> + dwc3_writel(dwc->regs, DWC3_OEVTEN, DWC3_OEVTEN_CONIDSTSCHNGEN |
> + DWC3_OEVTEN_BDEVVBUSCHNGE |
> + DWC3_OEVTEN_BDEVSESSVLDDETEN);
> + /* OCTL.PeriMode = 1 */
> + dwc3_writel(dwc->regs, DWC3_OCTL, DWC3_OCTL_PERIMODE);
> +
> + dwc3_otg_fsm_sync(dwc);
> + usb_otg_sync_inputs(dwc->fsm);
> +
> + return 0;
> +
> +error:
> + usb_otg_unregister(dwc->dev);
> +
> + return ret;
> }
>
> static void dwc3_drd_exit(struct dwc3 *dwc)
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index bd32cb2..129ef37 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -736,6 +736,7 @@ struct dwc3_scratchpad_array {
> * @gadget_driver: pointer to the gadget driver
> * @regs: base address for our registers
> * @regs_size: address space size
> + * @oevten: otg interrupt enable mask copy
> * @nr_scratch: number of scratch buffers
> * @num_event_buffers: calculated number of event buffers
> * @u1u2: only used on revisions <1.83a for workaround
> @@ -858,6 +859,8 @@ struct dwc3 {
> u32 dcfg;
> u32 gctl;
>
> + u32 oevten;
> +
> u32 nr_scratch;
> u32 num_event_buffers;
> u32 u1u2;
> --
> 2.1.4
>
--
balbi
[toc] | [prev] | [next] | [standalone]
| From | Roger Quadros <rogerq@ti.com> |
|---|---|
| Date | 2015-09-03 16:00 +0200 |
| Subject | Re: [PATCH v4 5/9] usb: dwc3: core: make dual-role work with OTG irq |
| Message-ID | <q4Ddv-5mr-3@gated-at.bofh.it> |
| In reply to | #1217676 |
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
On 02/09/15 17:43, Felipe Balbi wrote:
> Hi,
>
> On Wed, Sep 02, 2015 at 05:24:20PM +0300, Roger Quadros wrote:
>> If the ID pin event is not available over extcon
>> then we rely on the OTG controller to provide us ID and VBUS
>> information.
>>
>> We still don't support any OTG features but just
>> dual-role operation.
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>> drivers/usb/dwc3/core.c | 217 ++++++++++++++++++++++++++++++++++++++++++++----
>> drivers/usb/dwc3/core.h | 3 +
>> 2 files changed, 205 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 38b31df..632ee53 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -704,6 +704,63 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
>> return 0;
>> }
>>
>> +/* Get OTG events and sync it to OTG fsm */
>> +static void dwc3_otg_fsm_sync(struct dwc3 *dwc)
>> +{
>> + u32 reg;
>> + int id, vbus;
>> +
>> + reg = dwc3_readl(dwc->regs, DWC3_OSTS);
>> + dev_dbg(dwc->dev, "otgstatus 0x%x\n", reg);
>> +
>> + id = !!(reg & DWC3_OSTS_CONIDSTS);
>> + vbus = !!(reg & DWC3_OSTS_BSESVLD);
>> +
>> + if (id != dwc->fsm->id || vbus != dwc->fsm->b_sess_vld) {
>> + dev_dbg(dwc->dev, "id %d vbus %d\n", id, vbus);
>> + dwc->fsm->id = id;
>> + dwc->fsm->b_sess_vld = vbus;
>> + usb_otg_sync_inputs(dwc->fsm);
>> + }
>
> this guy shouldn't try to filter events here. That's what the FSM should
> be doing.
OK. I'll remove the if condition.
>
>> +}
>> +
>> +static irqreturn_t dwc3_otg_thread_irq(int irq, void *_dwc)
>> +{
>> + struct dwc3 *dwc = _dwc;
>> + unsigned long flags;
>> + irqreturn_t ret = IRQ_NONE;
>
> this IRQ will be disabled pretty quickly. You *always* return IRQ_NONE
>
>> + spin_lock_irqsave(&dwc->lock, flags);
>
> if you cache current OSTS in dwc3, you can use that instead and change
> this to a spin_lock() instead of disabling IRQs here. This device's IRQs
> are already masked anyway.
OK.
>
>> + dwc3_otg_fsm_sync(dwc);
>> + /* unmask interrupts */
>> + dwc3_writel(dwc->regs, DWC3_OEVTEN, dwc->oevten);
>> + spin_unlock_irqrestore(&dwc->lock, flags);
>> +
>> + return ret;
>> +}
>> +
>> +static irqreturn_t dwc3_otg_irq(int irq, void *_dwc)
>> +{
>> + struct dwc3 *dwc = _dwc;
>> + irqreturn_t ret = IRQ_NONE;
>> + u32 reg;
>> +
>> + spin_lock(&dwc->lock);
>
> this seems unnecessary, we're already in hardirq with IRQs disabled.
> What sort of race could we have ? (in fact, this also needs change in
> dwc3/gadget.c).
You're right. Will fix at both places.
>
>> +
>> + reg = dwc3_readl(dwc->regs, DWC3_OEVT);
>> + if (reg) {
>> + dwc3_writel(dwc->regs, DWC3_OEVT, reg);
>> + /* mask interrupts till processed */
>> + dwc->oevten = dwc3_readl(dwc->regs, DWC3_OEVTEN);
>> + dwc3_writel(dwc->regs, DWC3_OEVTEN, 0);
>> + ret = IRQ_WAKE_THREAD;
>> + }
>> +
>> + spin_unlock(&dwc->lock);
>> +
>> + return ret;
>> +}
>> +
>> /* --------------------- Dual-Role management ------------------------------- */
>>
>> static void dwc3_drd_fsm_sync(struct dwc3 *dwc)
>> @@ -728,15 +785,44 @@ static int dwc3_drd_start_host(struct otg_fsm *fsm, int on)
>> {
>> struct device *dev = usb_otg_fsm_to_dev(fsm);
>> struct dwc3 *dwc = dev_get_drvdata(dev);
>> + u32 reg;
>>
>> dev_dbg(dwc->dev, "%s: %d\n", __func__, on);
>> + if (dwc->edev) {
>> + if (on) {
>> + dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
>> + /* start the HCD */
>> + usb_otg_start_host(fsm, true);
>> + } else {
>> + /* stop the HCD */
>> + usb_otg_start_host(fsm, false);
>> + }
>
> if (on)
> dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
> usb_otg_start_host(fsm, on);
>
OK.
>> +
>> + return 0;
>> + }
>> +
>> + /* switch OTG core */
>> if (on) {
>> - dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
>> + /* OCTL.PeriMode = 0 */
>> + reg = dwc3_readl(dwc->regs, DWC3_OCTL);
>> + reg &= ~DWC3_OCTL_PERIMODE;
>> + dwc3_writel(dwc->regs, DWC3_OCTL, reg);
>> + /* unconditionally turn on VBUS */
>> + reg |= DWC3_OCTL_PRTPWRCTL;
>> + dwc3_writel(dwc->regs, DWC3_OCTL, reg);
>> /* start the HCD */
>> usb_otg_start_host(fsm, true);
>> } else {
>> /* stop the HCD */
>> usb_otg_start_host(fsm, false);
>> + /* turn off VBUS */
>> + reg = dwc3_readl(dwc->regs, DWC3_OCTL);
>> + reg &= ~DWC3_OCTL_PRTPWRCTL;
>> + dwc3_writel(dwc->regs, DWC3_OCTL, reg);
>> + /* OCTL.PeriMode = 1 */
>> + reg = dwc3_readl(dwc->regs, DWC3_OCTL);
>> + reg |= DWC3_OCTL_PERIMODE;
>> + dwc3_writel(dwc->regs, DWC3_OCTL, reg);
>> }
>
> it looks like you're not really following the fluxchart from SNPS
> documentation, see Figure 11-4 on section 11.1.4.5
Did you mean that I'm ignoring all OTG bits (HNP/SRP/ADP)?
>
>> @@ -746,15 +832,48 @@ static int dwc3_drd_start_gadget(struct otg_fsm *fsm, int on)
>> {
>> struct device *dev = usb_otg_fsm_to_dev(fsm);
>> struct dwc3 *dwc = dev_get_drvdata(dev);
>> + u32 reg;
>>
>> dev_dbg(dwc->dev, "%s: %d\n", __func__, on);
>> - if (on) {
>> - dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
>> + if (on)
>> dwc3_event_buffers_setup(dwc);
>>
>> + if (dwc->edev) {
>> + if (on) {
>> + dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
>> + usb_otg_start_gadget(fsm, true);
>> + } else {
>> + usb_otg_start_gadget(fsm, false);
>> + }
>> +
>> + return 0;
>> + }
>> +
>> + /* switch OTG core */
>> + if (on) {
>> + /* OCTL.PeriMode = 1 */
>> + reg = dwc3_readl(dwc->regs, DWC3_OCTL);
>> + reg |= DWC3_OCTL_PERIMODE;
>> + dwc3_writel(dwc->regs, DWC3_OCTL, reg);
>> + /* GUSB2PHYCFG0.SusPHY = 1 */
>> + if (!dwc->dis_u2_susphy_quirk) {
>> + reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
>> + reg |= DWC3_GUSB2PHYCFG_SUSPHY;
>> + dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>> + }
>> /* start the UDC */
>> usb_otg_start_gadget(fsm, true);
>> } else {
>> + /* GUSB2PHYCFG0.SusPHY=0 */
>> + if (!dwc->dis_u2_susphy_quirk) {
>> + reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
>> + reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
>> + dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>> + }
>> + /* OCTL.PeriMode = 1 */
>> + reg = dwc3_readl(dwc->regs, DWC3_OCTL);
>> + reg |= DWC3_OCTL_PERIMODE;
>> + dwc3_writel(dwc->regs, DWC3_OCTL, reg);
>> /* stop the UDC */
>> usb_otg_start_gadget(fsm, false);
>> }
>> @@ -777,10 +896,30 @@ static int dwc3_drd_notifier(struct notifier_block *nb,
>> return NOTIFY_DONE;
>> }
>>
- --
cheers,
- -roger
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQIcBAEBCAAGBQJV6FCCAAoJENJaa9O+djCTbaYQAME771phZpgr2Xtj1ejnPE8H
Bl84Sam/gWjy4+mqCUw+mQaCuF8M24ExVugHypQ0fF+9w6UMNrDrg+g+kZtQusCt
BTFkvS7g/6LJHEJowIoRZc5y/5bhnLa4Udcw5pYdhZHG7yIUsTs98WePROdOPk6z
i6OXA/wPC9ZJxeavew42HDmNj2IjJppU7bLDo+uMj/vz35dElq/B5w5mAXhshJ9A
R2IbxDevP4SiBYPfx1uFYKO5v9YVHnB3wk+3i3MjKuwO2CqfAVjzt9qWpM1iNThx
hOh+9vOenvttn7WHXP0scZAdBjmp3kKRAlfSELaAowy79X/3QRseZ75yJA8/tz+y
GT0x69fDQDxu0ffC961CY8p0a0F3ByVAqXBmsrCXPj0KxfutOkB8xE1BXY6+oUg/
ciqe0geXabmD9mu+3z8AXWOsFBnyFzsgSa2Dx5CRJ4/w5jhYOIg9/l8GGDlP6p1R
kHfiGYC2OzyxM4IgKYvc5p/VbAA4Ub5aQsWBdMYahAbs+l1xQ7zUEALf5S8c0KHK
k8jJo+oo+ghWzm6ikMfn96Ko/0vQuKG+uZZtzBDVp0uHBEW135GmZV5PCOh89M2k
yMuZQnbcTpEaANvWYJDkH3Can6Afcuki/i9kOK8bDib4Exo3IBijZNsLzpUzeS0m
vnsEqLL9IDRJR54ibQOC
=5n96
-----END PGP SIGNATURE-----
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Felipe Balbi <balbi@ti.com> |
|---|---|
| Date | 2015-09-03 18:00 +0200 |
| Subject | Re: [PATCH v4 5/9] usb: dwc3: core: make dual-role work with OTG irq |
| Message-ID | <q4F5F-83m-37@gated-at.bofh.it> |
| In reply to | #1218259 |
[Multipart message — attachments visible in raw view] — view raw
Hi,
On Thu, Sep 03, 2015 at 04:52:02PM +0300, Roger Quadros wrote:
> >> if (on) {
> >> - dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
> >> + /* OCTL.PeriMode = 0 */
> >> + reg = dwc3_readl(dwc->regs, DWC3_OCTL);
> >> + reg &= ~DWC3_OCTL_PERIMODE;
> >> + dwc3_writel(dwc->regs, DWC3_OCTL, reg);
> >> + /* unconditionally turn on VBUS */
> >> + reg |= DWC3_OCTL_PRTPWRCTL;
> >> + dwc3_writel(dwc->regs, DWC3_OCTL, reg);
> >> /* start the HCD */
> >> usb_otg_start_host(fsm, true);
> >> } else {
> >> /* stop the HCD */
> >> usb_otg_start_host(fsm, false);
> >> + /* turn off VBUS */
> >> + reg = dwc3_readl(dwc->regs, DWC3_OCTL);
> >> + reg &= ~DWC3_OCTL_PRTPWRCTL;
> >> + dwc3_writel(dwc->regs, DWC3_OCTL, reg);
> >> + /* OCTL.PeriMode = 1 */
> >> + reg = dwc3_readl(dwc->regs, DWC3_OCTL);
> >> + reg |= DWC3_OCTL_PERIMODE;
> >> + dwc3_writel(dwc->regs, DWC3_OCTL, reg);
> >> }
> >
> > it looks like you're not really following the fluxchart from SNPS
> > documentation, see Figure 11-4 on section 11.1.4.5
>
> Did you mean that I'm ignoring all OTG bits (HNP/SRP/ADP)?
yes and no :-) There's a rather complex flux chart which details how we
switch from host to peripheral and vice versa. We need to follow that to
the smallest details since that's what IP provider considers to be
correct. If we deviate from that we should have very strong reasons for
doing so and we also want big, fat, long comments in source code
detailing why and how we're deviating :-)
--
balbi
[toc] | [prev] | [next] | [standalone]
| From | Roger Quadros <rogerq@ti.com> |
|---|---|
| Date | 2015-09-04 11:20 +0200 |
| Subject | Re: [PATCH v4 5/9] usb: dwc3: core: make dual-role work with OTG irq |
| Message-ID | <q4Vk6-6ej-7@gated-at.bofh.it> |
| In reply to | #1218397 |
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
On 03/09/15 18:51, Felipe Balbi wrote:
> Hi,
>
> On Thu, Sep 03, 2015 at 04:52:02PM +0300, Roger Quadros wrote:
>>>> if (on) {
>>>> - dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
>>>> + /* OCTL.PeriMode = 0 */
>>>> + reg = dwc3_readl(dwc->regs, DWC3_OCTL);
>>>> + reg &= ~DWC3_OCTL_PERIMODE;
>>>> + dwc3_writel(dwc->regs, DWC3_OCTL, reg);
>>>> + /* unconditionally turn on VBUS */
>>>> + reg |= DWC3_OCTL_PRTPWRCTL;
>>>> + dwc3_writel(dwc->regs, DWC3_OCTL, reg);
>>>> /* start the HCD */
>>>> usb_otg_start_host(fsm, true);
>>>> } else {
>>>> /* stop the HCD */
>>>> usb_otg_start_host(fsm, false);
>>>> + /* turn off VBUS */
>>>> + reg = dwc3_readl(dwc->regs, DWC3_OCTL);
>>>> + reg &= ~DWC3_OCTL_PRTPWRCTL;
>>>> + dwc3_writel(dwc->regs, DWC3_OCTL, reg);
>>>> + /* OCTL.PeriMode = 1 */
>>>> + reg = dwc3_readl(dwc->regs, DWC3_OCTL);
>>>> + reg |= DWC3_OCTL_PERIMODE;
>>>> + dwc3_writel(dwc->regs, DWC3_OCTL, reg);
>>>> }
>>>
>>> it looks like you're not really following the fluxchart from SNPS
>>> documentation, see Figure 11-4 on section 11.1.4.5
>>
>> Did you mean that I'm ignoring all OTG bits (HNP/SRP/ADP)?
>
> yes and no :-) There's a rather complex flux chart which details how we
> switch from host to peripheral and vice versa. We need to follow that to
> the smallest details since that's what IP provider considers to be
> correct. If we deviate from that we should have very strong reasons for
> doing so and we also want big, fat, long comments in source code
> detailing why and how we're deviating :-)
>
Understood. I'll update this accordingly.
- --
cheers,
- -roger
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQIcBAEBCAAGBQJV6WC4AAoJENJaa9O+djCTWSEP/jq0jX2KJVjqKeSk60wyDIEH
AIGXpaupz4XCFRsLFHlEomAoWHEzmbdwQlcI5gyB4N6IgF6xfxX71AZnytw4k4ja
sFxQq/qNSPWa4yiEY9MFbIPuiRMFcLOB5VbemGpcZq4n31yaKlbLtHMDEC8zPmdY
yfnUz7raY2tID/+wOASAM+nDXVS6nFztWXCrz+4TxwDaH0dgwl9OAoD7nvTil3MT
UTuR34GbWcb9rCxJVYgneht1sLbkKGCkwjLRcqzhOajg87GuW38irqD9OOF+pLCm
srhYsntQyBJlvArDQKMZIyGEYHvjICWzJUo/IPA8DFS/qOrRLyrLvVHskb6pr9/y
KZL/1N+5Nbh0N0XGWKwxzVh69DqmQPuY32SN2r1xqCDzzZAnvrl9cz6ixsRq1JGK
9EvYNJlRWsdypfA8tdhPtCgXua+wYSLNewgHQSko10qeAgiaIU5/bQ/9Eb2Ys9QV
6wkhr0Gulf/hnMKXHupErhUsvSBJk2MGaaDIfUSt3mWMTRsthp2NedRE9li9bWW9
gSXcTxs8785Y7FgxFDvvqQ05rbGWNqfxkeq54ziMtH3mZ93inE7me1HbqL1jgEuh
NFrNsAKYc000syvnqunzl8uZMRMsICaY1aKC0riQTdpgagHgGMfadKtWXb8OdcVa
gICj2/o8MpPpv3FEniDz
=L39b
-----END PGP SIGNATURE-----
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Roger Quadros <rogerq@ti.com> |
|---|---|
| Date | 2015-09-02 16:30 +0200 |
| Subject | [PATCH v4 9/9] usb: dwc3: core: don't break during suspend/resume while we're dual-role |
| Message-ID | <q4hd0-7Er-35@gated-at.bofh.it> |
| In reply to | #1217638 |
We can't rely just on dr_mode to decide if we're in host or gadget
mode when we're configured as otg/dual-role. So while dr_mode is
OTG, we find out from the otg state machine if we're in host
or gadget mode and take the necessary actions during suspend/resume.
Also make sure that we disable OTG irq and events during system suspend
so that we don't lockup the system during system suspend/resume.
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
drivers/usb/dwc3/core.c | 27 +++++++++------------------
1 file changed, 9 insertions(+), 18 deletions(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 654aebf..25891e3 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1455,18 +1455,15 @@ static int dwc3_suspend(struct device *dev)
dwc->octl = dwc3_readl(dwc->regs, DWC3_OCTL);
dwc->oevt = dwc3_readl(dwc->regs, DWC3_OEVT);
dwc->oevten = dwc3_readl(dwc->regs, DWC3_OEVTEN);
+ dwc3_writel(dwc->regs, DWC3_OEVTEN, 0);
+ disable_irq(dwc->otg_irq);
}
- switch (dwc->dr_mode) {
- case USB_DR_MODE_PERIPHERAL:
- case USB_DR_MODE_OTG:
+ if (dwc->dr_mode == USB_DR_MODE_PERIPHERAL ||
+ ((dwc->dr_mode == USB_DR_MODE_OTG) && (dwc->fsm->protocol == PROTO_GADGET)))
dwc3_gadget_suspend(dwc);
- /* FALLTHROUGH */
- case USB_DR_MODE_HOST:
- default:
- dwc3_event_buffers_cleanup(dwc);
- break;
- }
+
+ dwc3_event_buffers_cleanup(dwc);
dwc->gctl = dwc3_readl(dwc->regs, DWC3_GCTL);
spin_unlock_irqrestore(&dwc->lock, flags);
@@ -1506,18 +1503,12 @@ static int dwc3_resume(struct device *dev)
dwc3_writel(dwc->regs, DWC3_OCTL, dwc->octl);
dwc3_writel(dwc->regs, DWC3_OEVT, dwc->oevt);
dwc3_writel(dwc->regs, DWC3_OEVTEN, dwc->oevten);
+ enable_irq(dwc->otg_irq);
}
- switch (dwc->dr_mode) {
- case USB_DR_MODE_PERIPHERAL:
- case USB_DR_MODE_OTG:
+ if (dwc->dr_mode == USB_DR_MODE_PERIPHERAL ||
+ ((dwc->dr_mode == USB_DR_MODE_OTG) && (dwc->fsm->protocol == PROTO_GADGET)))
dwc3_gadget_resume(dwc);
- /* FALLTHROUGH */
- case USB_DR_MODE_HOST:
- default:
- /* do nothing */
- break;
- }
spin_unlock_irqrestore(&dwc->lock, flags);
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Felipe Balbi <balbi@ti.com> |
|---|---|
| Date | 2015-09-02 16:50 +0200 |
| Subject | Re: [PATCH v4 9/9] usb: dwc3: core: don't break during suspend/resume while we're dual-role |
| Message-ID | <q4hwn-81p-47@gated-at.bofh.it> |
| In reply to | #1217644 |
[Multipart message — attachments visible in raw view] — view raw
On Wed, Sep 02, 2015 at 05:24:24PM +0300, Roger Quadros wrote:
> We can't rely just on dr_mode to decide if we're in host or gadget
> mode when we're configured as otg/dual-role. So while dr_mode is
> OTG, we find out from the otg state machine if we're in host
> or gadget mode and take the necessary actions during suspend/resume.
>
> Also make sure that we disable OTG irq and events during system suspend
> so that we don't lockup the system during system suspend/resume.
>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
> drivers/usb/dwc3/core.c | 27 +++++++++------------------
> 1 file changed, 9 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 654aebf..25891e3 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1455,18 +1455,15 @@ static int dwc3_suspend(struct device *dev)
> dwc->octl = dwc3_readl(dwc->regs, DWC3_OCTL);
> dwc->oevt = dwc3_readl(dwc->regs, DWC3_OEVT);
> dwc->oevten = dwc3_readl(dwc->regs, DWC3_OEVTEN);
> + dwc3_writel(dwc->regs, DWC3_OEVTEN, 0);
> + disable_irq(dwc->otg_irq);
you don't need disable_irq() here. In fact, it causes problems since
you're calling it with IRQs disabled.
> }
>
> - switch (dwc->dr_mode) {
> - case USB_DR_MODE_PERIPHERAL:
> - case USB_DR_MODE_OTG:
> + if (dwc->dr_mode == USB_DR_MODE_PERIPHERAL ||
> + ((dwc->dr_mode == USB_DR_MODE_OTG) && (dwc->fsm->protocol == PROTO_GADGET)))
> dwc3_gadget_suspend(dwc);
> - /* FALLTHROUGH */
> - case USB_DR_MODE_HOST:
> - default:
> - dwc3_event_buffers_cleanup(dwc);
> - break;
> - }
> +
> + dwc3_event_buffers_cleanup(dwc);
>
> dwc->gctl = dwc3_readl(dwc->regs, DWC3_GCTL);
> spin_unlock_irqrestore(&dwc->lock, flags);
> @@ -1506,18 +1503,12 @@ static int dwc3_resume(struct device *dev)
> dwc3_writel(dwc->regs, DWC3_OCTL, dwc->octl);
> dwc3_writel(dwc->regs, DWC3_OEVT, dwc->oevt);
> dwc3_writel(dwc->regs, DWC3_OEVTEN, dwc->oevten);
> + enable_irq(dwc->otg_irq);
> }
>
> - switch (dwc->dr_mode) {
> - case USB_DR_MODE_PERIPHERAL:
> - case USB_DR_MODE_OTG:
> + if (dwc->dr_mode == USB_DR_MODE_PERIPHERAL ||
> + ((dwc->dr_mode == USB_DR_MODE_OTG) && (dwc->fsm->protocol == PROTO_GADGET)))
> dwc3_gadget_resume(dwc);
> - /* FALLTHROUGH */
> - case USB_DR_MODE_HOST:
> - default:
> - /* do nothing */
> - break;
> - }
>
> spin_unlock_irqrestore(&dwc->lock, flags);
>
> --
> 2.1.4
>
--
balbi
[toc] | [prev] | [next] | [standalone]
| From | Roger Quadros <rogerq@ti.com> |
|---|---|
| Date | 2015-09-03 16:10 +0200 |
| Subject | Re: [PATCH v4 9/9] usb: dwc3: core: don't break during suspend/resume while we're dual-role |
| Message-ID | <q4Dnc-5MW-13@gated-at.bofh.it> |
| In reply to | #1217681 |
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
On 02/09/15 17:48, Felipe Balbi wrote:
> On Wed, Sep 02, 2015 at 05:24:24PM +0300, Roger Quadros wrote:
>> We can't rely just on dr_mode to decide if we're in host or gadget
>> mode when we're configured as otg/dual-role. So while dr_mode is
>> OTG, we find out from the otg state machine if we're in host
>> or gadget mode and take the necessary actions during suspend/resume.
>>
>> Also make sure that we disable OTG irq and events during system suspend
>> so that we don't lockup the system during system suspend/resume.
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>> drivers/usb/dwc3/core.c | 27 +++++++++------------------
>> 1 file changed, 9 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 654aebf..25891e3 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -1455,18 +1455,15 @@ static int dwc3_suspend(struct device *dev)
>> dwc->octl = dwc3_readl(dwc->regs, DWC3_OCTL);
>> dwc->oevt = dwc3_readl(dwc->regs, DWC3_OEVT);
>> dwc->oevten = dwc3_readl(dwc->regs, DWC3_OEVTEN);
>> + dwc3_writel(dwc->regs, DWC3_OEVTEN, 0);
>> + disable_irq(dwc->otg_irq);
>
> you don't need disable_irq() here. In fact, it causes problems since
> you're calling it with IRQs disabled.
OK. will remove it.
>
>> }
>>
>> - switch (dwc->dr_mode) {
>> - case USB_DR_MODE_PERIPHERAL:
>> - case USB_DR_MODE_OTG:
>> + if (dwc->dr_mode == USB_DR_MODE_PERIPHERAL ||
>> + ((dwc->dr_mode == USB_DR_MODE_OTG) && (dwc->fsm->protocol == PROTO_GADGET)))
>> dwc3_gadget_suspend(dwc);
>> - /* FALLTHROUGH */
>> - case USB_DR_MODE_HOST:
>> - default:
>> - dwc3_event_buffers_cleanup(dwc);
>> - break;
>> - }
>> +
>> + dwc3_event_buffers_cleanup(dwc);
>>
>> dwc->gctl = dwc3_readl(dwc->regs, DWC3_GCTL);
>> spin_unlock_irqrestore(&dwc->lock, flags);
>> @@ -1506,18 +1503,12 @@ static int dwc3_resume(struct device *dev)
>> dwc3_writel(dwc->regs, DWC3_OCTL, dwc->octl);
>> dwc3_writel(dwc->regs, DWC3_OEVT, dwc->oevt);
>> dwc3_writel(dwc->regs, DWC3_OEVTEN, dwc->oevten);
>> + enable_irq(dwc->otg_irq);
>> }
>>
>> - switch (dwc->dr_mode) {
>> - case USB_DR_MODE_PERIPHERAL:
>> - case USB_DR_MODE_OTG:
>> + if (dwc->dr_mode == USB_DR_MODE_PERIPHERAL ||
>> + ((dwc->dr_mode == USB_DR_MODE_OTG) && (dwc->fsm->protocol == PROTO_GADGET)))
>> dwc3_gadget_resume(dwc);
>> - /* FALLTHROUGH */
>> - case USB_DR_MODE_HOST:
>> - default:
>> - /* do nothing */
>> - break;
>> - }
>>
>> spin_unlock_irqrestore(&dwc->lock, flags);
>>
>> --
>> 2.1.4
>>
>
- --
cheers,
- -roger
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQIcBAEBCAAGBQJV6FLtAAoJENJaa9O+djCTxnMQANGOTEBjO8E9v32qfrgwL6X+
VyTeGBHhfwv7/u7xOM0im6tfXXaLtjj16mEY8fePC+oXO2Vv3RDxi0HTiF6sePnT
odt8nTOYGq7arPAKrtL8BK/8xyJRDjijXWGdQDAgPT1D5V2y8Ib4AbkM2j5IkL+/
75EGybhxIPNGNEV2eS/OBn2BPWSzn/r6rFMJiuYPY2yoPQqwG1ovdt+K+tvMybvZ
sYdQxu4UPN1z1pKplmCQxmPut9SyCqIAeHXdWGT6kJsleBsv2WNM1ZdM/y4zZmMw
gpyNOs+HYqdd/llfEYFrSRnSM2io2GyKZ73xM+DVfY8ot7Vf/h4x41Dz4V19jqtC
3IoyPzNb+inbsRKs0GhTw3yD9N8b7xRbVF+qZPIvDvn7QGLF2pY8cDiqZE1LiOs2
VpKBRoC0wdmdQ/bjRMt516jFmJiQ2RbR2SenGffIr+PrCb1EQ7Fmwtoya5FPgIz/
nMdL7g1MGHUzWvzXTYO+RH4bHV9cDL66qekOL7PR1yEyAAo09vLc+ds/4z85MGQ6
2JYKYb4Mdtv2EAOzIqNR3+FzcCawpZbhm8bBFz14Kj12pgjwl5A3o+fYaZ4Q07OK
ZfFRAUG4RKFNVcjb7hhiBPdN+mb34/50YreFcNj/MOYwklXwUAmgiTFH7cjO9SA7
XZpbfSRwT4Ec4WWqTeeR
=unNn
-----END PGP SIGNATURE-----
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> |
|---|---|
| Date | 2015-09-02 19:30 +0200 |
| Subject | Re: [PATCH v4 9/9] usb: dwc3: core: don't break during suspend/resume while we're dual-role |
| Message-ID | <q4k1c-3eF-9@gated-at.bofh.it> |
| In reply to | #1217644 |
Hello.
On 09/02/2015 05:24 PM, Roger Quadros wrote:
> We can't rely just on dr_mode to decide if we're in host or gadget
> mode when we're configured as otg/dual-role. So while dr_mode is
> OTG, we find out from the otg state machine if we're in host
> or gadget mode and take the necessary actions during suspend/resume.
>
> Also make sure that we disable OTG irq and events during system suspend
> so that we don't lockup the system during system suspend/resume.
>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
> drivers/usb/dwc3/core.c | 27 +++++++++------------------
> 1 file changed, 9 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 654aebf..25891e3 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1455,18 +1455,15 @@ static int dwc3_suspend(struct device *dev)
> dwc->octl = dwc3_readl(dwc->regs, DWC3_OCTL);
> dwc->oevt = dwc3_readl(dwc->regs, DWC3_OEVT);
> dwc->oevten = dwc3_readl(dwc->regs, DWC3_OEVTEN);
> + dwc3_writel(dwc->regs, DWC3_OEVTEN, 0);
> + disable_irq(dwc->otg_irq);
> }
>
> - switch (dwc->dr_mode) {
> - case USB_DR_MODE_PERIPHERAL:
> - case USB_DR_MODE_OTG:
> + if (dwc->dr_mode == USB_DR_MODE_PERIPHERAL ||
> + ((dwc->dr_mode == USB_DR_MODE_OTG) && (dwc->fsm->protocol == PROTO_GADGET)))
Hm, you're not very consistent about your parens. :-)
[...]
> @@ -1506,18 +1503,12 @@ static int dwc3_resume(struct device *dev)
> dwc3_writel(dwc->regs, DWC3_OCTL, dwc->octl);
> dwc3_writel(dwc->regs, DWC3_OEVT, dwc->oevt);
> dwc3_writel(dwc->regs, DWC3_OEVTEN, dwc->oevten);
> + enable_irq(dwc->otg_irq);
> }
>
> - switch (dwc->dr_mode) {
> - case USB_DR_MODE_PERIPHERAL:
> - case USB_DR_MODE_OTG:
> + if (dwc->dr_mode == USB_DR_MODE_PERIPHERAL ||
> + ((dwc->dr_mode == USB_DR_MODE_OTG) && (dwc->fsm->protocol == PROTO_GADGET)))
Same here...
[...]
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
Page 1 of 2 [1] 2 Next page →
Back to top | Article view | linux.kernel
csiph-web