* [PATCH v4 1/7] Revert "USB: chipidea: add vbus detect for udc"
2012-12-27 6:59 [PATCH v4 0/7] Add fully tested id switch and vbus connect detect support for Chipidea Peter Chen
@ 2012-12-27 6:59 ` Peter Chen
2012-12-27 6:59 ` [PATCH v4 2/7] usb: chipidea: add otg file Peter Chen
` (6 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Peter Chen @ 2012-12-27 6:59 UTC (permalink / raw)
To: linux-arm-kernel
vbus register is at otgsc, and vbus detect does not belong
to device function. Revert this patch, and will move
vbus detect function to drivers/usb/chipidea/udc.c
This reverts commit 8c4fc031954b4eb72daf13d3c907a985a3eee208.
Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
Changes for v3:
Add Signed-off-by: Peter Chen <peter.chen@freescale.com>
drivers/usb/chipidea/ci.h | 1 -
drivers/usb/chipidea/udc.c | 39 +--------------------------------------
2 files changed, 1 insertions(+), 39 deletions(-)
diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index e25d126..d738603 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -139,7 +139,6 @@ struct ci13xxx {
enum ci_role role;
bool is_otg;
struct work_struct work;
- struct work_struct vbus_work;
struct workqueue_struct *wq;
struct dma_pool *qh_pool;
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 2f45bba..d214448 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -305,18 +305,6 @@ static u32 hw_test_and_clear_intr_active(struct ci13xxx *ci)
return reg;
}
-static void hw_enable_vbus_intr(struct ci13xxx *ci)
-{
- hw_write(ci, OP_OTGSC, OTGSC_AVVIS, OTGSC_AVVIS);
- hw_write(ci, OP_OTGSC, OTGSC_AVVIE, OTGSC_AVVIE);
- queue_work(ci->wq, &ci->vbus_work);
-}
-
-static void hw_disable_vbus_intr(struct ci13xxx *ci)
-{
- hw_write(ci, OP_OTGSC, OTGSC_AVVIE, 0);
-}
-
/**
* hw_test_and_clear_setup_guard: test & clear setup guard (execute without
* interruption)
@@ -383,16 +371,6 @@ static int hw_usb_reset(struct ci13xxx *ci)
return 0;
}
-static void vbus_work(struct work_struct *work)
-{
- struct ci13xxx *ci = container_of(work, struct ci13xxx, vbus_work);
-
- if (hw_read(ci, OP_OTGSC, OTGSC_AVV))
- usb_gadget_vbus_connect(&ci->gadget);
- else
- usb_gadget_vbus_disconnect(&ci->gadget);
-}
-
/******************************************************************************
* UTIL block
*****************************************************************************/
@@ -1392,7 +1370,6 @@ static int ci13xxx_vbus_session(struct usb_gadget *_gadget, int is_active)
if (is_active) {
pm_runtime_get_sync(&_gadget->dev);
hw_device_reset(ci, USBMODE_CM_DC);
- hw_enable_vbus_intr(ci);
hw_device_state(ci, ci->ep0out->qh.dma);
} else {
hw_device_state(ci, 0);
@@ -1567,10 +1544,8 @@ static int ci13xxx_start(struct usb_gadget *gadget,
pm_runtime_get_sync(&ci->gadget.dev);
if (ci->platdata->flags & CI13XXX_PULLUP_ON_VBUS) {
if (ci->vbus_active) {
- if (ci->platdata->flags & CI13XXX_REGS_SHARED) {
+ if (ci->platdata->flags & CI13XXX_REGS_SHARED)
hw_device_reset(ci, USBMODE_CM_DC);
- hw_enable_vbus_intr(ci);
- }
} else {
pm_runtime_put_sync(&ci->gadget.dev);
goto done;
@@ -1676,13 +1651,6 @@ static irqreturn_t udc_irq(struct ci13xxx *ci)
} else {
retval = IRQ_NONE;
}
-
- intr = hw_read(ci, OP_OTGSC, ~0);
- hw_write(ci, OP_OTGSC, ~0, intr);
-
- if (intr & (OTGSC_AVVIE & OTGSC_AVVIS))
- queue_work(ci->wq, &ci->vbus_work);
-
spin_unlock(&ci->lock);
return retval;
@@ -1758,7 +1726,6 @@ static int udc_start(struct ci13xxx *ci)
retval = hw_device_reset(ci, USBMODE_CM_DC);
if (retval)
goto put_transceiver;
- hw_enable_vbus_intr(ci);
}
retval = device_register(&ci->gadget.dev);
@@ -1821,9 +1788,6 @@ static void udc_stop(struct ci13xxx *ci)
if (ci == NULL)
return;
- hw_disable_vbus_intr(ci);
- cancel_work_sync(&ci->vbus_work);
-
usb_del_gadget_udc(&ci->gadget);
destroy_eps(ci);
@@ -1864,7 +1828,6 @@ int ci_hdrc_gadget_init(struct ci13xxx *ci)
rdrv->irq = udc_irq;
rdrv->name = "gadget";
ci->roles[CI_ROLE_GADGET] = rdrv;
- INIT_WORK(&ci->vbus_work, vbus_work);
return 0;
}
--
1.7.0.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v4 2/7] usb: chipidea: add otg file
2012-12-27 6:59 [PATCH v4 0/7] Add fully tested id switch and vbus connect detect support for Chipidea Peter Chen
2012-12-27 6:59 ` [PATCH v4 1/7] Revert "USB: chipidea: add vbus detect for udc" Peter Chen
@ 2012-12-27 6:59 ` Peter Chen
2012-12-27 10:47 ` Sascha Hauer
2012-12-27 6:59 ` [PATCH v4 3/7] usb: chipidea: add otg id switch and vbus connect/disconnect detect Peter Chen
` (5 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Peter Chen @ 2012-12-27 6:59 UTC (permalink / raw)
To: linux-arm-kernel
Implement struct usb_otg, In that way, calling otg_set_peripheral
will not be failed at udc.c.
Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
Changes for v4:
- Some tiny changes, like delete unused header files
drivers/usb/chipidea/Makefile | 2 +-
drivers/usb/chipidea/ci.h | 2 +
drivers/usb/chipidea/otg.c | 60 +++++++++++++++++++++++++++++++++++++++++
drivers/usb/chipidea/otg.h | 6 ++++
4 files changed, 69 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile
index d92ca32..11f513c 100644
--- a/drivers/usb/chipidea/Makefile
+++ b/drivers/usb/chipidea/Makefile
@@ -2,7 +2,7 @@ ccflags-$(CONFIG_USB_CHIPIDEA_DEBUG) := -DDEBUG
obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc.o
-ci_hdrc-y := core.o
+ci_hdrc-y := core.o otg.o
ci_hdrc-$(CONFIG_USB_CHIPIDEA_UDC) += udc.o
ci_hdrc-$(CONFIG_USB_CHIPIDEA_HOST) += host.o
ci_hdrc-$(CONFIG_USB_CHIPIDEA_DEBUG) += debug.o
diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index d738603..8702871 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -129,6 +129,7 @@ struct hw_bank {
* @vbus_active: is VBUS active
* @transceiver: pointer to USB PHY, if any
* @hcd: pointer to usb_hcd for ehci host driver
+ * @otg: for otg support
*/
struct ci13xxx {
struct device *dev;
@@ -164,6 +165,7 @@ struct ci13xxx {
bool global_phy;
struct usb_phy *transceiver;
struct usb_hcd *hcd;
+ struct usb_otg otg;
};
static inline struct ci_role_driver *ci_role(struct ci13xxx *ci)
diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
new file mode 100644
index 0000000..7dea3b3
--- /dev/null
+++ b/drivers/usb/chipidea/otg.c
@@ -0,0 +1,60 @@
+/*
+ * otg.c - ChipIdea USB IP core OTG driver
+ *
+ * Copyright (C) 2012 Freescale Semiconductor, Inc.
+ *
+ * Author: Peter Chen
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/usb/gadget.h>
+#include <linux/usb/otg.h>
+#include <linux/usb/chipidea.h>
+
+#include "ci.h"
+#include "udc.h"
+#include "bits.h"
+#include "host.h"
+#include "debug.h"
+
+static int ci_otg_set_peripheral(struct usb_otg *otg,
+ struct usb_gadget *periph)
+{
+ otg->gadget = periph;
+
+ return 0;
+}
+
+static int ci_otg_set_host(struct usb_otg *otg, struct usb_bus *host)
+{
+ otg->host = host;
+
+ return 0;
+}
+
+/**
+ * ci_hdrc_otg_init - initialize device related bits
+ * ci: the controller
+ *
+ * This function create otg struct, if the device can switch between
+ * device and host.
+ */
+int ci_hdrc_otg_init(struct ci13xxx *ci)
+{
+ /* Useless at current */
+ ci->otg.set_peripheral = ci_otg_set_peripheral;
+ ci->otg.set_host = ci_otg_set_host;
+ if (!IS_ERR_OR_NULL(ci->transceiver))
+ ci->transceiver->otg = &ci->otg;
+
+ return 0;
+}
diff --git a/drivers/usb/chipidea/otg.h b/drivers/usb/chipidea/otg.h
new file mode 100644
index 0000000..b4c6b3e
--- /dev/null
+++ b/drivers/usb/chipidea/otg.h
@@ -0,0 +1,6 @@
+#ifndef __DRIVERS_USB_CHIPIDEA_OTG_H
+#define __DRIVERS_USB_CHIPIDEA_OTG_H
+
+int ci_hdrc_otg_init(struct ci13xxx *ci);
+
+#endif /* __DRIVERS_USB_CHIPIDEA_OTG_H */
--
1.7.0.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v4 2/7] usb: chipidea: add otg file
2012-12-27 6:59 ` [PATCH v4 2/7] usb: chipidea: add otg file Peter Chen
@ 2012-12-27 10:47 ` Sascha Hauer
2012-12-28 2:14 ` Peter Chen
0 siblings, 1 reply; 17+ messages in thread
From: Sascha Hauer @ 2012-12-27 10:47 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Dec 27, 2012 at 02:59:58PM +0800, Peter Chen wrote:
> Implement struct usb_otg, In that way, calling otg_set_peripheral
> will not be failed at udc.c.
>
> Signed-off-by: Peter Chen <peter.chen@freescale.com>
> ---
> Changes for v4:
> - Some tiny changes, like delete unused header files
>
> drivers/usb/chipidea/Makefile | 2 +-
> drivers/usb/chipidea/ci.h | 2 +
> drivers/usb/chipidea/otg.c | 60 +++++++++++++++++++++++++++++++++++++++++
> drivers/usb/chipidea/otg.h | 6 ++++
> 4 files changed, 69 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile
> index d92ca32..11f513c 100644
> --- a/drivers/usb/chipidea/Makefile
> +++ b/drivers/usb/chipidea/Makefile
> @@ -2,7 +2,7 @@ ccflags-$(CONFIG_USB_CHIPIDEA_DEBUG) := -DDEBUG
>
> obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc.o
>
> -ci_hdrc-y := core.o
> +ci_hdrc-y := core.o otg.o
> ci_hdrc-$(CONFIG_USB_CHIPIDEA_UDC) += udc.o
> ci_hdrc-$(CONFIG_USB_CHIPIDEA_HOST) += host.o
> ci_hdrc-$(CONFIG_USB_CHIPIDEA_DEBUG) += debug.o
> diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> index d738603..8702871 100644
> --- a/drivers/usb/chipidea/ci.h
> +++ b/drivers/usb/chipidea/ci.h
> @@ -129,6 +129,7 @@ struct hw_bank {
> * @vbus_active: is VBUS active
> * @transceiver: pointer to USB PHY, if any
> * @hcd: pointer to usb_hcd for ehci host driver
> + * @otg: for otg support
> */
> struct ci13xxx {
> struct device *dev;
> @@ -164,6 +165,7 @@ struct ci13xxx {
> bool global_phy;
> struct usb_phy *transceiver;
> struct usb_hcd *hcd;
> + struct usb_otg otg;
I'd really like to know how this is going to proceed.
The ULPI driver currently allocates a struct usb_otg and sets
otg->set_host and otg->set_vbus. Since now the chipidea driver uses its
own struct usb_otg, this will not work with ULPI support.
I see some ways out of this:
- Use a pointer above instead of an own struct usb_otg in the ci driver.
- move the set_host and set_vbus callbacks to struct usb_phy.
I'd like to have this solved before we merge this patch.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH v4 2/7] usb: chipidea: add otg file
2012-12-27 10:47 ` Sascha Hauer
@ 2012-12-28 2:14 ` Peter Chen
0 siblings, 0 replies; 17+ messages in thread
From: Peter Chen @ 2012-12-28 2:14 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Dec 27, 2012 at 11:47:58AM +0100, Sascha Hauer wrote:
> On Thu, Dec 27, 2012 at 02:59:58PM +0800, Peter Chen wrote:
> > struct ci13xxx {
> > struct device *dev;
> > @@ -164,6 +165,7 @@ struct ci13xxx {
> > bool global_phy;
> > struct usb_phy *transceiver;
> > struct usb_hcd *hcd;
> > + struct usb_otg otg;
>
> I'd really like to know how this is going to proceed.
>
> The ULPI driver currently allocates a struct usb_otg and sets
> otg->set_host and otg->set_vbus. Since now the chipidea driver uses its
> own struct usb_otg, this will not work with ULPI support.
We discussed before that the otg is not related to phy.
The struct usb_otg is better allocated at the otg driver, not phy driver.
I have a look at other phy drivers(drivers/usb/phy), now, there
is no one to create struct usb_otg at their phy drivers. To solve this
problem, we may need to modify ulpi phy driver.
To integrate ULPI phy, we may need to do:
- Make ulpi as a platform driver
- Override otg.set_host, and otg.set_vbus with ulpi's at
otg_ulpi_create or what else.
- Call otg_ulpi_create at the end of ci13xxx_imx_probe (before
pm_runtime_no_callbacks) if the phy is ulpi.
>
> I see some ways out of this:
>
> - Use a pointer above instead of an own struct usb_otg in the ci driver.
> - move the set_host and set_vbus callbacks to struct usb_phy.
>
> I'd like to have this solved before we merge this patch.
>
> Sascha
>
>
> --
> Pengutronix e.K. | |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
>
--
Best Regards,
Peter Chen
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v4 3/7] usb: chipidea: add otg id switch and vbus connect/disconnect detect
2012-12-27 6:59 [PATCH v4 0/7] Add fully tested id switch and vbus connect detect support for Chipidea Peter Chen
2012-12-27 6:59 ` [PATCH v4 1/7] Revert "USB: chipidea: add vbus detect for udc" Peter Chen
2012-12-27 6:59 ` [PATCH v4 2/7] usb: chipidea: add otg file Peter Chen
@ 2012-12-27 6:59 ` Peter Chen
2012-12-27 7:21 ` Marek Vasut
2012-12-27 7:00 ` [PATCH v4 4/7] usb: chipidea: consolidate ci_role_driver's API for both roles Peter Chen
` (4 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Peter Chen @ 2012-12-27 6:59 UTC (permalink / raw)
To: linux-arm-kernel
The main design flow is the same with msm otg driver, that is the id and
vbus interrupt are handled at core driver, others are handled by
individual drivers.
- At former design, when switch usb role from device->host, it will call
udc_stop, it will remove the gadget driver, so when switch role
from host->device, it can't add gadget driver any more.
At new design, when role switch occurs, the gadget just calls
usb_gadget_vbus_disconnect/usb_gadget_vbus_connect as well as
reset controller, it will not free any device/gadget structure
- Add vbus connect and disconnect to core interrupt handler, it can
notify udc driver by calling usb_gadget_vbus_disconnect
/usb_gadget_vbus_connect.
Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
Changes for v4:
- Change the otg event flag from the bitmask to bool flag
- Add some otg helper functions
- Move some hardware init to hw_device_init
- Move the otg init to one block
Changes for v3:
- Add prefix CI_ for ID and B_SESS_VALID
- Use new API hw_wait_reg which is alexander sugguested to wait vbus
voltage stable
drivers/usb/chipidea/bits.h | 10 +++
drivers/usb/chipidea/ci.h | 8 ++-
drivers/usb/chipidea/core.c | 177 ++++++++++++++++++++++++++++++++++++++----
drivers/usb/chipidea/otg.c | 28 +++++---
drivers/usb/chipidea/otg.h | 3 +
drivers/usb/chipidea/udc.c | 2 +
6 files changed, 200 insertions(+), 28 deletions(-)
diff --git a/drivers/usb/chipidea/bits.h b/drivers/usb/chipidea/bits.h
index 050de85..ba9c6ef 100644
--- a/drivers/usb/chipidea/bits.h
+++ b/drivers/usb/chipidea/bits.h
@@ -65,11 +65,21 @@
#define OTGSC_ASVIS BIT(18)
#define OTGSC_BSVIS BIT(19)
#define OTGSC_BSEIS BIT(20)
+#define OTGSC_1MSIS BIT(21)
+#define OTGSC_DPIS BIT(22)
#define OTGSC_IDIE BIT(24)
#define OTGSC_AVVIE BIT(25)
#define OTGSC_ASVIE BIT(26)
#define OTGSC_BSVIE BIT(27)
#define OTGSC_BSEIE BIT(28)
+#define OTGSC_1MSIE BIT(29)
+#define OTGSC_DPIE BIT(30)
+#define OTGSC_INT_EN_BITS (OTGSC_IDIE | OTGSC_AVVIE | OTGSC_ASVIE \
+ | OTGSC_BSVIE | OTGSC_BSEIE | OTGSC_1MSIE \
+ | OTGSC_DPIE)
+#define OTGSC_INT_STATUS_BITS (OTGSC_IDIS | OTGSC_AVVIS | OTGSC_ASVIS \
+ | OTGSC_BSVIS | OTGSC_BSEIS | OTGSC_1MSIS \
+ | OTGSC_DPIS)
/* USBMODE */
#define USBMODE_CM (0x03UL << 0)
diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index 8702871..325d790 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -130,6 +130,7 @@ struct hw_bank {
* @transceiver: pointer to USB PHY, if any
* @hcd: pointer to usb_hcd for ehci host driver
* @otg: for otg support
+ * @events: events for otg, and handled@ci_role_work
*/
struct ci13xxx {
struct device *dev;
@@ -140,6 +141,7 @@ struct ci13xxx {
enum ci_role role;
bool is_otg;
struct work_struct work;
+ struct delayed_work dwork;
struct workqueue_struct *wq;
struct dma_pool *qh_pool;
@@ -165,7 +167,9 @@ struct ci13xxx {
bool global_phy;
struct usb_phy *transceiver;
struct usb_hcd *hcd;
- struct usb_otg otg;
+ struct usb_otg otg;
+ bool id_event;
+ bool b_sess_valid_event;
};
static inline struct ci_role_driver *ci_role(struct ci13xxx *ci)
@@ -314,4 +318,6 @@ int hw_port_test_set(struct ci13xxx *ci, u8 mode);
u8 hw_port_test_get(struct ci13xxx *ci);
+void ci_handle_vbus_change(struct ci13xxx *ci);
+
#endif /* __DRIVERS_USB_CHIPIDEA_CI_H */
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index aebf695..f8f8484 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -73,6 +73,7 @@
#include "bits.h"
#include "host.h"
#include "debug.h"
+#include "otg.h"
/* Controller register map */
static uintptr_t ci_regs_nolpm[] = {
@@ -199,6 +200,14 @@ static int hw_device_init(struct ci13xxx *ci, void __iomem *base)
if (ci->hw_ep_max > ENDPT_MAX)
return -ENODEV;
+ /* Disable all interrupts bits */
+ hw_write(ci, OP_USBINTR, 0xffffffff, 0);
+ ci_disable_otg_interrupt(ci, OTGSC_INT_EN_BITS);
+
+ /* Clear all interrupts status bits*/
+ hw_write(ci, OP_USBSTS, 0xffffffff, 0xffffffff);
+ ci_clear_otg_interrupt(ci, OTGSC_INT_STATUS_BITS);
+
dev_dbg(ci->dev, "ChipIdea HDRC found, lpm: %d; cap: %p op: %p\n",
ci->hw_bank.lpm, ci->hw_bank.cap, ci->hw_bank.op);
@@ -265,24 +274,124 @@ static enum ci_role ci_otg_role(struct ci13xxx *ci)
}
/**
- * ci_role_work - perform role changing based on ID pin
- * @work: work struct
+ * hw_wait_reg: wait the register value
+ *
+ * Sometimes, it needs to wait register value before going on.
+ * Eg, when switch to device mode, the vbus value should be lower
+ * than OTGSC_BSV before connects to host.
+ *
+ * @ci: the controller
+ * @reg: register index
+ * @mask: mast bit
+ * @value: the bit value to wait
+ * @timeout: timeout to indicate an error
+ *
+ * This function returns an error code if timeout
*/
-static void ci_role_work(struct work_struct *work)
+static int hw_wait_reg(struct ci13xxx *ci, enum ci13xxx_regs reg, u32 mask,
+ u32 value, unsigned long timeout)
+{
+ unsigned long elapse = jiffies + timeout;
+
+ while (hw_read(ci, reg, mask) != value) {
+ if (time_after(jiffies, elapse)) {
+ dev_err(ci->dev, "timeout waiting for %08x in %d\n",
+ mask, reg);
+ return -ETIMEDOUT;
+ }
+ msleep(20);
+ }
+
+ return 0;
+}
+
+#define CI_VBUS_STABLE_TIMEOUT 500
+static void ci_handle_id_switch(struct ci13xxx *ci)
{
- struct ci13xxx *ci = container_of(work, struct ci13xxx, work);
enum ci_role role = ci_otg_role(ci);
if (role != ci->role) {
dev_dbg(ci->dev, "switching from %s to %s\n",
ci_role(ci)->name, ci->roles[role]->name);
- ci_role_stop(ci);
- ci_role_start(ci, role);
- enable_irq(ci->irq);
+ /* 1. Finish the current role */
+ if (ci->role == CI_ROLE_GADGET) {
+ usb_gadget_vbus_disconnect(&ci->gadget);
+ /* host doesn't care B_SESSION_VALID event */
+ ci_clear_otg_interrupt(ci, OTGSC_BSVIS);
+ ci_disable_otg_interrupt(ci, OTGSC_BSVIE);
+ ci->role = CI_ROLE_END;
+ /* reset controller */
+ hw_device_reset(ci, USBMODE_CM_IDLE);
+ } else if (ci->role == CI_ROLE_HOST) {
+ ci_role_stop(ci);
+ /* reset controller */
+ hw_device_reset(ci, USBMODE_CM_IDLE);
+ }
+
+ /* 2. Turn on/off vbus according to coming role */
+ if (ci_otg_role(ci) == CI_ROLE_GADGET) {
+ otg_set_vbus(&ci->otg, false);
+ /* wait vbus lower than OTGSC_BSV */
+ hw_wait_reg(ci, OP_OTGSC, OTGSC_BSV, 0,
+ CI_VBUS_STABLE_TIMEOUT);
+ } else if (ci_otg_role(ci) == CI_ROLE_HOST) {
+ otg_set_vbus(&ci->otg, true);
+ /* wait vbus higher than OTGSC_AVV */
+ hw_wait_reg(ci, OP_OTGSC, OTGSC_AVV, OTGSC_AVV,
+ CI_VBUS_STABLE_TIMEOUT);
+ }
+
+ /* 3. Begin the new role */
+ if (ci_otg_role(ci) == CI_ROLE_GADGET) {
+ ci->role = role;
+ ci_clear_otg_interrupt(ci, OTGSC_BSVIS);
+ ci_enable_otg_interrupt(ci, OTGSC_BSVIE);
+ } else if (ci_otg_role(ci) == CI_ROLE_HOST) {
+ ci_role_start(ci, role);
+ }
}
}
+void ci_handle_vbus_change(struct ci13xxx *ci)
+{
+ u32 otgsc = hw_read(ci, OP_OTGSC, ~0);
+
+ if (otgsc & OTGSC_BSV)
+ usb_gadget_vbus_connect(&ci->gadget);
+ else
+ usb_gadget_vbus_disconnect(&ci->gadget);
+}
+
+/**
+ * ci_otg_work - perform otg (vbus/id) event handle
+ * @work: work struct
+ */
+static void ci_otg_work(struct work_struct *work)
+{
+ struct ci13xxx *ci = container_of(work, struct ci13xxx, work);
+
+ if (ci->id_event) {
+ ci->id_event = false;
+ ci_handle_id_switch(ci);
+ } else if (ci->b_sess_valid_event) {
+ ci->b_sess_valid_event = false;
+ ci_handle_vbus_change(ci);
+ } else
+ dev_err(ci->dev, "unexpected event occurs at %s\n", __func__);
+
+ enable_irq(ci->irq);
+}
+
+static void ci_delayed_work(struct work_struct *work)
+{
+ struct delayed_work *dwork = to_delayed_work(work);
+ struct ci13xxx *ci = container_of(dwork, struct ci13xxx, dwork);
+
+ otg_set_vbus(&ci->otg, true);
+
+}
+
static ssize_t show_role(struct device *dev, struct device_attribute *attr,
char *buf)
{
@@ -321,19 +430,36 @@ static irqreturn_t ci_irq(int irq, void *data)
irqreturn_t ret = IRQ_NONE;
u32 otgsc = 0;
- if (ci->is_otg)
- otgsc = hw_read(ci, OP_OTGSC, ~0);
+ otgsc = hw_read(ci, OP_OTGSC, ~0);
- if (ci->role != CI_ROLE_END)
- ret = ci_role(ci)->irq(ci);
+ /*
+ * Handle id change interrupt, it indicates device/host function
+ * switch.
+ */
+ if (ci->is_otg && (otgsc & OTGSC_IDIE) && (otgsc & OTGSC_IDIS)) {
+ ci->id_event = true;
+ ci_clear_otg_interrupt(ci, OTGSC_IDIS);
+ disable_irq_nosync(ci->irq);
+ queue_work(ci->wq, &ci->work);
+ return IRQ_HANDLED;
+ }
- if (ci->is_otg && (otgsc & OTGSC_IDIS)) {
- hw_write(ci, OP_OTGSC, OTGSC_IDIS, OTGSC_IDIS);
+ /*
+ * Handle vbus change interrupt, it indicates device connection
+ * and disconnection events.
+ */
+ if ((otgsc & OTGSC_BSVIE) && (otgsc & OTGSC_BSVIS)) {
+ ci->b_sess_valid_event = true;
+ ci_clear_otg_interrupt(ci, OTGSC_BSVIS);
disable_irq_nosync(ci->irq);
queue_work(ci->wq, &ci->work);
- ret = IRQ_HANDLED;
+ return IRQ_HANDLED;
}
+ /* Handle device/host interrupt */
+ if (ci->role != CI_ROLE_END)
+ ret = ci_role(ci)->irq(ci);
+
return ret;
}
@@ -398,6 +524,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
struct resource *res;
void __iomem *base;
int ret;
+ u32 otgsc;
if (!dev->platform_data) {
dev_err(dev, "platform data missing\n");
@@ -443,7 +570,8 @@ static int ci_hdrc_probe(struct platform_device *pdev)
return -ENODEV;
}
- INIT_WORK(&ci->work, ci_role_work);
+ INIT_WORK(&ci->work, ci_otg_work);
+ INIT_DELAYED_WORK(&ci->dwork, ci_delayed_work);
ci->wq = create_singlethread_workqueue("ci_otg");
if (!ci->wq) {
dev_err(dev, "can't create workqueue\n");
@@ -466,7 +594,10 @@ static int ci_hdrc_probe(struct platform_device *pdev)
}
if (ci->roles[CI_ROLE_HOST] && ci->roles[CI_ROLE_GADGET]) {
+ dev_dbg(dev, "support otg\n");
ci->is_otg = true;
+ /* if otg is supported, create struct usb_otg */
+ ci_hdrc_otg_init(ci);
/* ID pin needs 1ms debouce time, we delay 2ms for safe */
mdelay(2);
ci->role = ci_otg_role(ci);
@@ -483,6 +614,17 @@ static int ci_hdrc_probe(struct platform_device *pdev)
goto rm_wq;
}
+ otgsc = hw_read(ci, OP_OTGSC, ~0);
+ /*
+ * if it is device mode:
+ * - Enable vbus detect
+ * - If it has already connected to host, notify udc
+ */
+ if (ci->role == CI_ROLE_GADGET) {
+ ci_enable_otg_interrupt(ci, OTGSC_BSVIE);
+ ci_handle_vbus_change(ci);
+ }
+
platform_set_drvdata(pdev, ci);
ret = request_irq(ci->irq, ci_irq, IRQF_SHARED, ci->platdata->name,
ci);
@@ -493,8 +635,9 @@ static int ci_hdrc_probe(struct platform_device *pdev)
if (ret)
goto rm_attr;
- if (ci->is_otg)
- hw_write(ci, OP_OTGSC, OTGSC_IDIE, OTGSC_IDIE);
+ /* Handle the situation that usb device at the MicroB to A cable */
+ if (ci->is_otg && !(otgsc & OTGSC_ID))
+ queue_delayed_work(ci->wq, &ci->dwork, msecs_to_jiffies(500));
return ret;
diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
index 7dea3b3..2986d91 100644
--- a/drivers/usb/chipidea/otg.c
+++ b/drivers/usb/chipidea/otg.c
@@ -10,21 +10,28 @@
* published by the Free Software Foundation.
*/
-#include <linux/platform_device.h>
-#include <linux/module.h>
-#include <linux/io.h>
-#include <linux/irq.h>
-#include <linux/kernel.h>
-#include <linux/slab.h>
-#include <linux/usb/gadget.h>
#include <linux/usb/otg.h>
+#include <linux/usb/gadget.h>
#include <linux/usb/chipidea.h>
#include "ci.h"
-#include "udc.h"
#include "bits.h"
-#include "host.h"
-#include "debug.h"
+
+void ci_clear_otg_interrupt(struct ci13xxx *ci, u32 bits)
+{
+ /* Only clear request bits */
+ hw_write(ci, OP_OTGSC, OTGSC_INT_STATUS_BITS, bits);
+}
+
+void ci_enable_otg_interrupt(struct ci13xxx *ci, u32 bits)
+{
+ hw_write(ci, OP_OTGSC, bits, bits);
+}
+
+void ci_disable_otg_interrupt(struct ci13xxx *ci, u32 bits)
+{
+ hw_write(ci, OP_OTGSC, bits, 0);
+}
static int ci_otg_set_peripheral(struct usb_otg *otg,
struct usb_gadget *periph)
@@ -55,6 +62,7 @@ int ci_hdrc_otg_init(struct ci13xxx *ci)
ci->otg.set_host = ci_otg_set_host;
if (!IS_ERR_OR_NULL(ci->transceiver))
ci->transceiver->otg = &ci->otg;
+ ci_enable_otg_interrupt(ci, OTGSC_IDIE);
return 0;
}
diff --git a/drivers/usb/chipidea/otg.h b/drivers/usb/chipidea/otg.h
index b4c6b3e..fa30428 100644
--- a/drivers/usb/chipidea/otg.h
+++ b/drivers/usb/chipidea/otg.h
@@ -2,5 +2,8 @@
#define __DRIVERS_USB_CHIPIDEA_OTG_H
int ci_hdrc_otg_init(struct ci13xxx *ci);
+void ci_clear_otg_interrupt(struct ci13xxx *ci, u32 bits);
+void ci_enable_otg_interrupt(struct ci13xxx *ci, u32 bits);
+void ci_disable_otg_interrupt(struct ci13xxx *ci, u32 bits);
#endif /* __DRIVERS_USB_CHIPIDEA_OTG_H */
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index d214448..b52cb10 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1371,6 +1371,7 @@ static int ci13xxx_vbus_session(struct usb_gadget *_gadget, int is_active)
pm_runtime_get_sync(&_gadget->dev);
hw_device_reset(ci, USBMODE_CM_DC);
hw_device_state(ci, ci->ep0out->qh.dma);
+ dev_dbg(ci->dev, "Connected to host\n");
} else {
hw_device_state(ci, 0);
if (ci->platdata->notify_event)
@@ -1378,6 +1379,7 @@ static int ci13xxx_vbus_session(struct usb_gadget *_gadget, int is_active)
CI13XXX_CONTROLLER_STOPPED_EVENT);
_gadget_stop_activity(&ci->gadget);
pm_runtime_put_sync(&_gadget->dev);
+ dev_dbg(ci->dev, "disconnected from host\n");
}
}
--
1.7.0.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v4 3/7] usb: chipidea: add otg id switch and vbus connect/disconnect detect
2012-12-27 6:59 ` [PATCH v4 3/7] usb: chipidea: add otg id switch and vbus connect/disconnect detect Peter Chen
@ 2012-12-27 7:21 ` Marek Vasut
2012-12-27 8:20 ` Peter Chen
0 siblings, 1 reply; 17+ messages in thread
From: Marek Vasut @ 2012-12-27 7:21 UTC (permalink / raw)
To: linux-arm-kernel
Dear Peter Chen,
[...]
> +
> +#define CI_VBUS_STABLE_TIMEOUT 500
Shall we not change this to static const int instead ?
[...]
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -1371,6 +1371,7 @@ static int ci13xxx_vbus_session(struct usb_gadget
> *_gadget, int is_active) pm_runtime_get_sync(&_gadget->dev);
> hw_device_reset(ci, USBMODE_CM_DC);
> hw_device_state(ci, ci->ep0out->qh.dma);
> + dev_dbg(ci->dev, "Connected to host\n");
> } else {
> hw_device_state(ci, 0);
> if (ci->platdata->notify_event)
> @@ -1378,6 +1379,7 @@ static int ci13xxx_vbus_session(struct usb_gadget
> *_gadget, int is_active) CI13XXX_CONTROLLER_STOPPED_EVENT);
> _gadget_stop_activity(&ci->gadget);
> pm_runtime_put_sync(&_gadget->dev);
> + dev_dbg(ci->dev, "disconnected from host\n");
Keep the capital letters at the begining of the sentence consistent -- either
start with capital D here or fix the capital C above.
> }
> }
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH v4 3/7] usb: chipidea: add otg id switch and vbus connect/disconnect detect
2012-12-27 7:21 ` Marek Vasut
@ 2012-12-27 8:20 ` Peter Chen
2012-12-27 8:31 ` Marek Vasut
0 siblings, 1 reply; 17+ messages in thread
From: Peter Chen @ 2012-12-27 8:20 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Dec 27, 2012 at 08:21:55AM +0100, Marek Vasut wrote:
> Dear Peter Chen,
>
> [...]
> > +
> > +#define CI_VBUS_STABLE_TIMEOUT 500
>
> Shall we not change this to static const int instead ?
Is it a must?
I find the similar at drivers/usb/core/hub.c
2433 #define PORT_RESET_TRIES 5
2434 #define SET_ADDRESS_TRIES 2
2435 #define GET_DESCRIPTOR_TRIES 2
2436 #define SET_CONFIG_TRIES (2 * (use_both_schemes + 1))
2437 #define USE_NEW_SCHEME(i) ((i) / 2 == (int)old_scheme_first)
2438
2439 #define HUB_ROOT_RESET_TIME 50 /* times are in msec */
2440 #define HUB_SHORT_RESET_TIME 10
2441 #define HUB_BH_RESET_TIME 50
2442 #define HUB_LONG_RESET_TIME 200
2443 #define HUB_RESET_TIMEOUT 500
>
> [...]
>
> > --- a/drivers/usb/chipidea/udc.c
> > +++ b/drivers/usb/chipidea/udc.c
> > @@ -1371,6 +1371,7 @@ static int ci13xxx_vbus_session(struct usb_gadget
> > *_gadget, int is_active) pm_runtime_get_sync(&_gadget->dev);
> > hw_device_reset(ci, USBMODE_CM_DC);
> > hw_device_state(ci, ci->ep0out->qh.dma);
> > + dev_dbg(ci->dev, "Connected to host\n");
> > } else {
> > hw_device_state(ci, 0);
> > if (ci->platdata->notify_event)
> > @@ -1378,6 +1379,7 @@ static int ci13xxx_vbus_session(struct usb_gadget
> > *_gadget, int is_active) CI13XXX_CONTROLLER_STOPPED_EVENT);
> > _gadget_stop_activity(&ci->gadget);
> > pm_runtime_put_sync(&_gadget->dev);
> > + dev_dbg(ci->dev, "disconnected from host\n");
>
> Keep the capital letters at the begining of the sentence consistent -- either
> start with capital D here or fix the capital C above.
Thanks, I will change "d" to "D".
>
> > }
> > }
>
--
Best Regards,
Peter Chen
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH v4 3/7] usb: chipidea: add otg id switch and vbus connect/disconnect detect
2012-12-27 8:20 ` Peter Chen
@ 2012-12-27 8:31 ` Marek Vasut
0 siblings, 0 replies; 17+ messages in thread
From: Marek Vasut @ 2012-12-27 8:31 UTC (permalink / raw)
To: linux-arm-kernel
Dear Peter Chen,
> On Thu, Dec 27, 2012 at 08:21:55AM +0100, Marek Vasut wrote:
> > Dear Peter Chen,
> >
> > [...]
> >
> > > +
> > > +#define CI_VBUS_STABLE_TIMEOUT 500
> >
> > Shall we not change this to static const int instead ?
>
> Is it a must?
> I find the similar at drivers/usb/core/hub.c
[...]
No it's not. I was just pondering now that it's a local constant, it might just
be like that.
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v4 4/7] usb: chipidea: consolidate ci_role_driver's API for both roles
2012-12-27 6:59 [PATCH v4 0/7] Add fully tested id switch and vbus connect detect support for Chipidea Peter Chen
` (2 preceding siblings ...)
2012-12-27 6:59 ` [PATCH v4 3/7] usb: chipidea: add otg id switch and vbus connect/disconnect detect Peter Chen
@ 2012-12-27 7:00 ` Peter Chen
2012-12-27 7:00 ` [PATCH v4 5/7] usb: chipidea: udc: add pullup/pulldown dp at hw_device_state Peter Chen
` (3 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Peter Chen @ 2012-12-27 7:00 UTC (permalink / raw)
To: linux-arm-kernel
- Create init/destroy API for probe and remove
- start/stop API are only used otg id switch process
- Create the gadget at ci_hdrc_probe if the gadget is supported
at that port, the main purpose for this is to avoid gadget module
load fail at init.rc
Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
Changes for v4:
- Move some udc operation from core to udc->init
Changes for v3:
- Create init/destroy API for probe and remove
- start/stop API are only used otg id switch process
drivers/usb/chipidea/ci.h | 19 +++++++++++-
drivers/usb/chipidea/core.c | 65 ++++++++++++++++++------------------------
drivers/usb/chipidea/host.c | 2 +
drivers/usb/chipidea/udc.c | 33 ++++++++++++++++++++-
4 files changed, 78 insertions(+), 41 deletions(-)
diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index 325d790..00939e6 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -67,14 +67,18 @@ enum ci_role {
/**
* struct ci_role_driver - host/gadget role driver
- * start: start this role
- * stop: stop this role
+ * init: init this role (used at module probe)
+ * start: start this role (used at id switch)
+ * stop: stop this role (used at id switch)
+ * destroy: destroy this role (used at module remove)
* irq: irq handler for this role
* name: role name string (host/gadget)
*/
struct ci_role_driver {
+ int (*init)(struct ci13xxx *);
int (*start)(struct ci13xxx *);
void (*stop)(struct ci13xxx *);
+ void (*destroy)(struct ci13xxx *);
irqreturn_t (*irq)(struct ci13xxx *);
const char *name;
};
@@ -206,6 +210,17 @@ static inline void ci_role_stop(struct ci13xxx *ci)
ci->roles[role]->stop(ci);
}
+static inline void ci_role_destroy(struct ci13xxx *ci)
+{
+ enum ci_role role = ci->role;
+
+ if (role == CI_ROLE_END)
+ return;
+
+ ci->role = CI_ROLE_END;
+
+ ci->roles[role]->destroy(ci);
+}
/******************************************************************************
* REGISTERS
*****************************************************************************/
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index f8f8484..a5adf1a 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -315,27 +315,16 @@ static void ci_handle_id_switch(struct ci13xxx *ci)
ci_role(ci)->name, ci->roles[role]->name);
/* 1. Finish the current role */
- if (ci->role == CI_ROLE_GADGET) {
- usb_gadget_vbus_disconnect(&ci->gadget);
- /* host doesn't care B_SESSION_VALID event */
- ci_clear_otg_interrupt(ci, OTGSC_BSVIS);
- ci_disable_otg_interrupt(ci, OTGSC_BSVIE);
- ci->role = CI_ROLE_END;
- /* reset controller */
- hw_device_reset(ci, USBMODE_CM_IDLE);
- } else if (ci->role == CI_ROLE_HOST) {
- ci_role_stop(ci);
- /* reset controller */
- hw_device_reset(ci, USBMODE_CM_IDLE);
- }
+ ci_role_stop(ci);
+ hw_device_reset(ci, USBMODE_CM_IDLE);
/* 2. Turn on/off vbus according to coming role */
- if (ci_otg_role(ci) == CI_ROLE_GADGET) {
+ if (role == CI_ROLE_GADGET) {
otg_set_vbus(&ci->otg, false);
/* wait vbus lower than OTGSC_BSV */
hw_wait_reg(ci, OP_OTGSC, OTGSC_BSV, 0,
CI_VBUS_STABLE_TIMEOUT);
- } else if (ci_otg_role(ci) == CI_ROLE_HOST) {
+ } else if (role == CI_ROLE_HOST) {
otg_set_vbus(&ci->otg, true);
/* wait vbus higher than OTGSC_AVV */
hw_wait_reg(ci, OP_OTGSC, OTGSC_AVV, OTGSC_AVV,
@@ -343,13 +332,7 @@ static void ci_handle_id_switch(struct ci13xxx *ci)
}
/* 3. Begin the new role */
- if (ci_otg_role(ci) == CI_ROLE_GADGET) {
- ci->role = role;
- ci_clear_otg_interrupt(ci, OTGSC_BSVIS);
- ci_enable_otg_interrupt(ci, OTGSC_BSVIE);
- } else if (ci_otg_role(ci) == CI_ROLE_HOST) {
- ci_role_start(ci, role);
- }
+ ci_role_start(ci, role);
}
}
@@ -585,7 +568,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
ret = ci_hdrc_gadget_init(ci);
if (ret)
- dev_info(dev, "doesn't support gadget\n");
+ dev_info(dev, "doesn't support gadget, ret=%d\n", ret);
if (!ci->roles[CI_ROLE_HOST] && !ci->roles[CI_ROLE_GADGET]) {
dev_err(dev, "no supported roles\n");
@@ -607,22 +590,30 @@ static int ci_hdrc_probe(struct platform_device *pdev)
: CI_ROLE_GADGET;
}
- ret = ci_role_start(ci, ci->role);
- if (ret) {
- dev_err(dev, "can't start %s role\n", ci_role(ci)->name);
- ret = -ENODEV;
- goto rm_wq;
- }
-
otgsc = hw_read(ci, OP_OTGSC, ~0);
+
/*
- * if it is device mode:
- * - Enable vbus detect
- * - If it has already connected to host, notify udc
+ * If the gadget is supported, call its init unconditionally,
+ * We need to support load gadget module at init.rc.
*/
- if (ci->role == CI_ROLE_GADGET) {
- ci_enable_otg_interrupt(ci, OTGSC_BSVIE);
- ci_handle_vbus_change(ci);
+ if (ci->roles[CI_ROLE_GADGET]) {
+ ret = ci->roles[CI_ROLE_GADGET]->init(ci);
+ if (ret) {
+ dev_err(dev, "can't init %s role, ret=%d\n",
+ ci_role(ci)->name, ret);
+ ret = -ENODEV;
+ goto rm_wq;
+ }
+ }
+
+ if (ci->role == CI_ROLE_HOST) {
+ ret = ci->roles[CI_ROLE_HOST]->init(ci);
+ if (ret) {
+ dev_err(dev, "can't init %s role, ret=%d\n",
+ ci_role(ci)->name, ret);
+ ret = -ENODEV;
+ goto rm_wq;
+ }
}
platform_set_drvdata(pdev, ci);
@@ -660,7 +651,7 @@ static int ci_hdrc_remove(struct platform_device *pdev)
destroy_workqueue(ci->wq);
device_remove_file(ci->dev, &dev_attr_role);
free_irq(ci->irq, ci);
- ci_role_stop(ci);
+ ci_role_destroy(ci);
return 0;
}
diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
index caecad9..6024a4f 100644
--- a/drivers/usb/chipidea/host.c
+++ b/drivers/usb/chipidea/host.c
@@ -92,8 +92,10 @@ int ci_hdrc_host_init(struct ci13xxx *ci)
if (!rdrv)
return -ENOMEM;
+ rdrv->init = host_start;
rdrv->start = host_start;
rdrv->stop = host_stop;
+ rdrv->destroy = host_stop;
rdrv->irq = host_irq;
rdrv->name = "host";
ci->roles[CI_ROLE_HOST] = rdrv;
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index b52cb10..1ac6321 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -31,6 +31,7 @@
#include "ci.h"
#include "udc.h"
+#include "otg.h"
#include "bits.h"
#include "debug.h"
@@ -1754,6 +1755,16 @@ static int udc_start(struct ci13xxx *ci)
pm_runtime_no_callbacks(&ci->gadget.dev);
pm_runtime_enable(&ci->gadget.dev);
+ /*
+ * if it is device mode:
+ * - Enable vbus detect
+ * - If it has already connected to host, notify udc
+ */
+ if (ci->role == CI_ROLE_GADGET) {
+ ci_enable_otg_interrupt(ci, OTGSC_BSVIE);
+ ci_handle_vbus_change(ci);
+ }
+
return retval;
remove_trans:
@@ -1780,6 +1791,22 @@ free_qh_pool:
return retval;
}
+static int udc_id_switch_for_device(struct ci13xxx *ci)
+{
+ ci_clear_otg_interrupt(ci, OTGSC_BSVIS);
+ ci_enable_otg_interrupt(ci, OTGSC_BSVIE);
+
+ return 0;
+}
+
+static void udc_id_switch_for_host(struct ci13xxx *ci)
+{
+ usb_gadget_vbus_disconnect(&ci->gadget);
+ /* host doesn't care B_SESSION_VALID event */
+ ci_clear_otg_interrupt(ci, OTGSC_BSVIS);
+ ci_disable_otg_interrupt(ci, OTGSC_BSVIE);
+}
+
/**
* udc_remove: parent remove must call this to remove UDC
*
@@ -1825,8 +1852,10 @@ int ci_hdrc_gadget_init(struct ci13xxx *ci)
if (!rdrv)
return -ENOMEM;
- rdrv->start = udc_start;
- rdrv->stop = udc_stop;
+ rdrv->init = udc_start;
+ rdrv->start = udc_id_switch_for_device;
+ rdrv->stop = udc_id_switch_for_host;
+ rdrv->destroy = udc_stop;
rdrv->irq = udc_irq;
rdrv->name = "gadget";
ci->roles[CI_ROLE_GADGET] = rdrv;
--
1.7.0.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v4 5/7] usb: chipidea: udc: add pullup/pulldown dp at hw_device_state
2012-12-27 6:59 [PATCH v4 0/7] Add fully tested id switch and vbus connect detect support for Chipidea Peter Chen
` (3 preceding siblings ...)
2012-12-27 7:00 ` [PATCH v4 4/7] usb: chipidea: consolidate ci_role_driver's API for both roles Peter Chen
@ 2012-12-27 7:00 ` Peter Chen
2012-12-27 7:00 ` [PATCH v4 6/7] usb: chipidea: udc: retire the flag CI13_PULLUP_ON_VBUS Peter Chen
` (2 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Peter Chen @ 2012-12-27 7:00 UTC (permalink / raw)
To: linux-arm-kernel
- During the connect/disconnect host, we need to pullup
and pulldown dp
- Make sure the dp is not pullup until the vbus is on when
flag CI13XXX_PULLUP_ON_VBUS is set
- Using hw_device_state when set run/stop bit
Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
drivers/usb/chipidea/udc.c | 10 ++++++++--
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 1ac6321..c983ce2 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -91,8 +91,10 @@ static int hw_device_state(struct ci13xxx *ci, u32 dma)
/* interrupt, error, port change, reset, sleep/suspend */
hw_write(ci, OP_USBINTR, ~0,
USBi_UI|USBi_UEI|USBi_PCI|USBi_URI|USBi_SLI);
+ hw_write(ci, OP_USBCMD, USBCMD_RS, USBCMD_RS);
} else {
hw_write(ci, OP_USBINTR, ~0, 0);
+ hw_write(ci, OP_USBCMD, USBCMD_RS, 0);
}
return 0;
}
@@ -1424,10 +1426,14 @@ static int ci13xxx_pullup(struct usb_gadget *_gadget, int is_on)
{
struct ci13xxx *ci = container_of(_gadget, struct ci13xxx, gadget);
+ if ((ci->platdata->flags & CI13XXX_PULLUP_ON_VBUS) &&
+ !ci->vbus_active)
+ return -ENOTSUPP;
+
if (is_on)
- hw_write(ci, OP_USBCMD, USBCMD_RS, USBCMD_RS);
+ hw_device_state(ci, ci->ep0out->qh.dma);
else
- hw_write(ci, OP_USBCMD, USBCMD_RS, 0);
+ hw_device_state(ci, 0);
return 0;
}
--
1.7.0.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v4 6/7] usb: chipidea: udc: retire the flag CI13_PULLUP_ON_VBUS
2012-12-27 6:59 [PATCH v4 0/7] Add fully tested id switch and vbus connect detect support for Chipidea Peter Chen
` (4 preceding siblings ...)
2012-12-27 7:00 ` [PATCH v4 5/7] usb: chipidea: udc: add pullup/pulldown dp at hw_device_state Peter Chen
@ 2012-12-27 7:00 ` Peter Chen
2012-12-27 7:00 ` [PATCH v4 7/7] usb: chipidea: imx: add internal vbus regulator control Peter Chen
2013-01-08 15:27 ` [PATCH v4 0/7] Add fully tested id switch and vbus connect detect support for Chipidea Maxime Ripard
7 siblings, 0 replies; 17+ messages in thread
From: Peter Chen @ 2012-12-27 7:00 UTC (permalink / raw)
To: linux-arm-kernel
(change CI13XXX to CI13 to avoid junk email check)
Now, we have handled vbus session in core driver when the
vbus interrupt occurs, so our pullup operations are all
according to vbus.
Of cource, the software can still call .pullup when device connects
to host if it wants to connect/disconnect with host.
Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
drivers/usb/chipidea/ci13xxx_imx.c | 1 -
drivers/usb/chipidea/ci13xxx_msm.c | 1 -
drivers/usb/chipidea/udc.c | 23 ++++++++---------------
include/linux/usb/chipidea.h | 1 -
4 files changed, 8 insertions(+), 18 deletions(-)
diff --git a/drivers/usb/chipidea/ci13xxx_imx.c b/drivers/usb/chipidea/ci13xxx_imx.c
index 8c29122..3b91ff4 100644
--- a/drivers/usb/chipidea/ci13xxx_imx.c
+++ b/drivers/usb/chipidea/ci13xxx_imx.c
@@ -88,7 +88,6 @@ EXPORT_SYMBOL_GPL(usbmisc_get_init_data);
static struct ci13xxx_platform_data ci13xxx_imx_platdata = {
.name = "ci13xxx_imx",
.flags = CI13XXX_REQUIRE_TRANSCEIVER |
- CI13XXX_PULLUP_ON_VBUS |
CI13XXX_DISABLE_STREAMING,
.capoffset = DEF_CAPOFFSET,
};
diff --git a/drivers/usb/chipidea/ci13xxx_msm.c b/drivers/usb/chipidea/ci13xxx_msm.c
index 7d16681..5755ee8 100644
--- a/drivers/usb/chipidea/ci13xxx_msm.c
+++ b/drivers/usb/chipidea/ci13xxx_msm.c
@@ -49,7 +49,6 @@ static struct ci13xxx_platform_data ci13xxx_msm_platdata = {
.name = "ci13xxx_msm",
.flags = CI13XXX_REGS_SHARED |
CI13XXX_REQUIRE_TRANSCEIVER |
- CI13XXX_PULLUP_ON_VBUS |
CI13XXX_DISABLE_STREAMING,
.notify_event = ci13xxx_msm_notify_event,
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index c983ce2..4a246a6 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1360,9 +1360,6 @@ static int ci13xxx_vbus_session(struct usb_gadget *_gadget, int is_active)
unsigned long flags;
int gadget_ready = 0;
- if (!(ci->platdata->flags & CI13XXX_PULLUP_ON_VBUS))
- return -EOPNOTSUPP;
-
spin_lock_irqsave(&ci->lock, flags);
ci->vbus_active = is_active;
if (ci->driver)
@@ -1426,8 +1423,7 @@ static int ci13xxx_pullup(struct usb_gadget *_gadget, int is_on)
{
struct ci13xxx *ci = container_of(_gadget, struct ci13xxx, gadget);
- if ((ci->platdata->flags & CI13XXX_PULLUP_ON_VBUS) &&
- !ci->vbus_active)
+ if (!ci->vbus_active)
return -ENOTSUPP;
if (is_on)
@@ -1551,14 +1547,12 @@ static int ci13xxx_start(struct usb_gadget *gadget,
ci->driver = driver;
pm_runtime_get_sync(&ci->gadget.dev);
- if (ci->platdata->flags & CI13XXX_PULLUP_ON_VBUS) {
- if (ci->vbus_active) {
- if (ci->platdata->flags & CI13XXX_REGS_SHARED)
- hw_device_reset(ci, USBMODE_CM_DC);
- } else {
- pm_runtime_put_sync(&ci->gadget.dev);
- goto done;
- }
+ if (ci->vbus_active) {
+ if (ci->platdata->flags & CI13XXX_REGS_SHARED)
+ hw_device_reset(ci, USBMODE_CM_DC);
+ } else {
+ pm_runtime_put_sync(&ci->gadget.dev);
+ goto done;
}
retval = hw_device_state(ci, ci->ep0out->qh.dma);
@@ -1581,8 +1575,7 @@ static int ci13xxx_stop(struct usb_gadget *gadget,
spin_lock_irqsave(&ci->lock, flags);
- if (!(ci->platdata->flags & CI13XXX_PULLUP_ON_VBUS) ||
- ci->vbus_active) {
+ if (ci->vbus_active) {
hw_device_state(ci, 0);
if (ci->platdata->notify_event)
ci->platdata->notify_event(ci,
diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
index 544825d..37821b3 100644
--- a/include/linux/usb/chipidea.h
+++ b/include/linux/usb/chipidea.h
@@ -17,7 +17,6 @@ struct ci13xxx_platform_data {
unsigned long flags;
#define CI13XXX_REGS_SHARED BIT(0)
#define CI13XXX_REQUIRE_TRANSCEIVER BIT(1)
-#define CI13XXX_PULLUP_ON_VBUS BIT(2)
#define CI13XXX_DISABLE_STREAMING BIT(3)
#define CI13XXX_CONTROLLER_RESET_EVENT 0
--
1.7.0.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v4 7/7] usb: chipidea: imx: add internal vbus regulator control
2012-12-27 6:59 [PATCH v4 0/7] Add fully tested id switch and vbus connect detect support for Chipidea Peter Chen
` (5 preceding siblings ...)
2012-12-27 7:00 ` [PATCH v4 6/7] usb: chipidea: udc: retire the flag CI13_PULLUP_ON_VBUS Peter Chen
@ 2012-12-27 7:00 ` Peter Chen
2013-01-08 15:27 ` [PATCH v4 0/7] Add fully tested id switch and vbus connect detect support for Chipidea Maxime Ripard
7 siblings, 0 replies; 17+ messages in thread
From: Peter Chen @ 2012-12-27 7:00 UTC (permalink / raw)
To: linux-arm-kernel
- For host, the vbus should always be on.
- For otg, the vbus is off defaultly, the vbus needs to be
turned on/off when usb role switches.
Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
Changes for v3:
- Add return value check for regulator_enable/regulator_disable
- typo error
drivers/usb/chipidea/ci.h | 2 +
drivers/usb/chipidea/ci13xxx_imx.c | 80 ++++++++++++++++++++++++++++--------
2 files changed, 64 insertions(+), 18 deletions(-)
diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index 00939e6..342b430 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -135,6 +135,7 @@ struct hw_bank {
* @hcd: pointer to usb_hcd for ehci host driver
* @otg: for otg support
* @events: events for otg, and handled at ci_role_work
+ * @reg_vbus: used to control internal vbus regulator
*/
struct ci13xxx {
struct device *dev;
@@ -174,6 +175,7 @@ struct ci13xxx {
struct usb_otg otg;
bool id_event;
bool b_sess_valid_event;
+ struct regulator *reg_vbus;
};
static inline struct ci_role_driver *ci_role(struct ci13xxx *ci)
diff --git a/drivers/usb/chipidea/ci13xxx_imx.c b/drivers/usb/chipidea/ci13xxx_imx.c
index 3b91ff4..4c9df98 100644
--- a/drivers/usb/chipidea/ci13xxx_imx.c
+++ b/drivers/usb/chipidea/ci13xxx_imx.c
@@ -88,14 +88,47 @@ EXPORT_SYMBOL_GPL(usbmisc_get_init_data);
static struct ci13xxx_platform_data ci13xxx_imx_platdata = {
.name = "ci13xxx_imx",
.flags = CI13XXX_REQUIRE_TRANSCEIVER |
- CI13XXX_DISABLE_STREAMING,
+ CI13XXX_DISABLE_STREAMING |
+ CI13XXX_REGS_SHARED,
.capoffset = DEF_CAPOFFSET,
};
+static int ci13xxx_otg_set_vbus(struct usb_otg *otg, bool enabled)
+{
+ struct ci13xxx *ci = container_of(otg, struct ci13xxx, otg);
+ struct regulator *reg_vbus = ci->reg_vbus;
+ int ret;
+
+ WARN_ON(!reg_vbus);
+
+ if (reg_vbus) {
+ if (enabled) {
+ ret = regulator_enable(reg_vbus);
+ if (ret) {
+ dev_err(ci->dev,
+ "Failed to enable vbus regulator, ret=%d\n",
+ ret);
+ return ret;
+ }
+ } else {
+ ret = regulator_disable(reg_vbus);
+ if (ret) {
+ dev_err(ci->dev,
+ "Failed to disable vbus regulator, ret=%d\n",
+ ret);
+ return ret;
+ }
+ }
+ }
+
+ return 0;
+}
+
static int ci13xxx_imx_probe(struct platform_device *pdev)
{
struct ci13xxx_imx_data *data;
struct platform_device *plat_ci, *phy_pdev;
+ struct ci13xxx *ci;
struct device_node *phy_np;
struct resource *res;
struct regulator *reg_vbus;
@@ -152,20 +185,11 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
}
}
- /* we only support host now, so enable vbus here */
reg_vbus = devm_regulator_get(&pdev->dev, "vbus");
- if (!IS_ERR(reg_vbus)) {
- ret = regulator_enable(reg_vbus);
- if (ret) {
- dev_err(&pdev->dev,
- "Failed to enable vbus regulator, err=%d\n",
- ret);
- goto put_np;
- }
+ if (!IS_ERR(reg_vbus))
data->reg_vbus = reg_vbus;
- } else {
+ else
reg_vbus = NULL;
- }
ci13xxx_imx_platdata.phy = data->phy;
@@ -175,7 +199,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
if (!pdev->dev.dma_mask) {
ret = -ENOMEM;
dev_err(&pdev->dev, "Failed to alloc dma_mask!\n");
- goto err;
+ goto put_np;
}
*pdev->dev.dma_mask = DMA_BIT_MASK(32);
dma_set_coherent_mask(&pdev->dev, *pdev->dev.dma_mask);
@@ -186,7 +210,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
if (ret) {
dev_err(&pdev->dev,
"usbmisc init failed, ret=%d\n", ret);
- goto err;
+ goto put_np;
}
}
@@ -198,24 +222,44 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
dev_err(&pdev->dev,
"Can't register ci_hdrc platform device, err=%d\n",
ret);
- goto err;
+ goto put_np;
}
data->ci_pdev = plat_ci;
platform_set_drvdata(pdev, data);
+ ci = platform_get_drvdata(plat_ci);
+ /*
+ * Internal vbus on/off policy
+ * - Always on for host only function
+ * - Always off for gadget only function
+ * - call otg.set_vbus to control on/off according usb role
+ */
+
+ if (ci->roles[CI_ROLE_HOST] && !ci->roles[CI_ROLE_GADGET]
+ && reg_vbus) {
+ ret = regulator_enable(reg_vbus);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "Failed to enable vbus regulator, ret=%d\n",
+ ret);
+ goto put_np;
+ }
+ } else if (ci->is_otg) {
+ ci->otg.set_vbus = ci13xxx_otg_set_vbus;
+ ci->reg_vbus = data->reg_vbus;
+ }
+
pm_runtime_no_callbacks(&pdev->dev);
pm_runtime_enable(&pdev->dev);
return 0;
-err:
- if (reg_vbus)
- regulator_disable(reg_vbus);
put_np:
if (phy_np)
of_node_put(phy_np);
clk_disable_unprepare(data->clk);
+
return ret;
}
--
1.7.0.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v4 0/7] Add fully tested id switch and vbus connect detect support for Chipidea
2012-12-27 6:59 [PATCH v4 0/7] Add fully tested id switch and vbus connect detect support for Chipidea Peter Chen
` (6 preceding siblings ...)
2012-12-27 7:00 ` [PATCH v4 7/7] usb: chipidea: imx: add internal vbus regulator control Peter Chen
@ 2013-01-08 15:27 ` Maxime Ripard
2013-01-09 3:23 ` Peter Chen
7 siblings, 1 reply; 17+ messages in thread
From: Maxime Ripard @ 2013-01-08 15:27 UTC (permalink / raw)
To: linux-arm-kernel
Hi Peter,
On 27/12/2012 07:59, Peter Chen wrote:
> (Sorry for update slowly due to long time business trip)
>
> Changes for v4 mainly for 2/7, 3/7, 4/7, see individual
> patch commit for detail.
>
> This patchset adds fully tested otg id switch function and
> vbus connect/disconnect detection for chipidea driver.
> The mainly design of id/vbus handling follows msm otg driver.
> I hope the msm usb maintainer can have a look of this patchset,
> and give some comments, and move the whole msm usb driver to
> chipidea framework if possible in the future.
>
> This patchset is fully tested at i.mx6Q saberlite board.
Have you tested it on a 3.8 kernel?
On a 3.7 kernel, it worked flawlessly on an imx28, but I rebased your
patches on top of 3.8, and now, it doesn't work anymore. At probe, it
generates the following panic:
[ 1.257375] ci_hdrc ci_hdrc.0: doesn't support host
[ 1.268937] ci_hdrc ci_hdrc.0: can't init gadget role, ret=-524
[ 1.277062] Unable to handle kernel NULL pointer dereference at virtual address 00000044
[ 1.285343] pgd = c0004000
[ 1.288093] [00000044] *pgd=00000000
[ 1.291718] Internal error: Oops: 5 [#1] ARM
[ 1.296000] Modules linked in:
[ 1.299093] CPU: 0 Not tainted (3.8.0-rc2-00021-ge35aebb #339)
[ 1.305312] PC is at ci13xxx_imx_probe+0x1d8/0x3d0
[ 1.310125] LR is at ci13xxx_imx_probe+0x1d8/0x3d0
[ 1.314906] pc : [<c025124c>] lr : [<c025124c>] psr: a0000013
[ 1.314906] sp : c782fe90 ip : c0507578 fp : 00000000
[ 1.326406] r10: 00000000 r9 : c797e8a0 r8 : c79c26d0
[ 1.331656] r7 : c787f600 r6 : c787f610 r5 : 00000000 r4 : c79e1600
[ 1.338187] r3 : c7871b00 r2 : 20000013 r1 : c79c26d0 r0 : 00000000
[ 1.344718] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel
[ 1.352031] Control: 0005317f Table: 40004000 DAC: 00000017
[ 1.357781] Process swapper (pid: 1, stack limit = 0xc782e1b8)
[ 1.363656] Stack: (0xc782fe90 to 0xc7830000)
[ 1.368031] fe80: c0a776c0 c787f610 c787f610 c0a776c0
[ 1.376218] fea0: c787f644 c05227e4 00000000 c04e5500 c782e000 c02042ec c02042d8 c0203030
[ 1.384437] fec0: c787f610 c05227e4 c787f644 c052fa60 00000000 c0203264 00000000 c05227e4
[ 1.392625] fee0: c02031d0 c02017b4 c7803ca8 c7871b50 c05227e4 c79cdb00 c051a4c8 c020287c
[ 1.400843] ff00: c047021c c05227e4 c05227e4 c04efed0 c052fa60 c052fa60 c04efeb0 c0203730
[ 1.409031] ff20: 00000000 c04f4e0c c04efed0 c052fa60 c052fa60 c04efeb0 c782e000 c0008898
[ 1.417250] ff40: c0470b50 c04b1860 00000006 00000006 ffffffff c0b88cc0 c04f4e0c c04efed0
[ 1.425437] ff60: 00000007 c04efeb0 0000007c c04d421c 00000000 c035ebb0 00000006 00000006
[ 1.433656] ff80: c04d421c 00000000 c035ea9c 00000000 00000000 c035ea9c 00000000 00000000
[ 1.441843] ffa0: 00000000 00000000 00000000 c000ed48 00000000 00000000 00000000 00000000
[ 1.450031] ffc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 1.458250] ffe0: 00000000 00000000 00000000 00000000 00000013 00000000 36060003 012c1285
[ 1.466500] [<c025124c>] (ci13xxx_imx_probe+0x1d8/0x3d0) from [<c02042ec>] (platform_drv_probe+0x14/0x18)
[ 1.476093] [<c02042ec>] (platform_drv_probe+0x14/0x18) from [<c0203030>] (driver_probe_device+0x74/0x214)
[ 1.485781] [<c0203030>] (driver_probe_device+0x74/0x214) from [<c0203264>] (__driver_attach+0x94/0x98)
[ 1.495218] [<c0203264>] (__driver_attach+0x94/0x98) from [<c02017b4>] (bus_for_each_dev+0x50/0x80)
[ 1.504312] [<c02017b4>] (bus_for_each_dev+0x50/0x80) from [<c020287c>] (bus_add_driver+0x17c/0x250)
[ 1.513468] [<c020287c>] (bus_add_driver+0x17c/0x250) from [<c0203730>] (driver_register+0x78/0x14c)
[ 1.522625] [<c0203730>] (driver_register+0x78/0x14c) from [<c0008898>] (do_one_initcall+0x108/0x17c)
[ 1.531875] [<c0008898>] (do_one_initcall+0x108/0x17c) from [<c035ebb0>] (kernel_init+0x114/0x2a4)
[ 1.540875] [<c035ebb0>] (kernel_init+0x114/0x2a4) from [<c000ed48>] (ret_from_fork+0x14/0x2c)
[ 1.549531] Code: e1a00006 ebfec61d e2840010 ebfec5f0 (e5903044)
[ 1.555906] ---[ end trace ad9061e486664e45 ]---
I dug a bit into it, and it seems there's two different bug here:
- The probe function, when calling ci_hdrc_host_init, get ENXIO as
the error code. This is new to 3.8. I don't really see why it would
happen, because, judging from the code, only the first hw_read in
the function returns such code, and this code hasn't change between
the two versions.
- The second one is that since the host mode is not enabled, the
is_otg variable is never set to true, and thus, the function
ci_hdrc_otg_init is never called. This is problematic since it
registers the set_peripheral callback, that is called later in the
function if the gadget role is defined. And I believe that this is
what actually triggers the panic.
Thanks,
Maxime
--
Maxime Ripard, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH v4 0/7] Add fully tested id switch and vbus connect detect support for Chipidea
2013-01-08 15:27 ` [PATCH v4 0/7] Add fully tested id switch and vbus connect detect support for Chipidea Maxime Ripard
@ 2013-01-09 3:23 ` Peter Chen
2013-01-09 11:43 ` Maxime Ripard
0 siblings, 1 reply; 17+ messages in thread
From: Peter Chen @ 2013-01-09 3:23 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jan 08, 2013 at 04:27:21PM +0100, Maxime Ripard wrote:
> Hi Peter,
>
> On 27/12/2012 07:59, Peter Chen wrote:
> > (Sorry for update slowly due to long time business trip)
> >
> > Changes for v4 mainly for 2/7, 3/7, 4/7, see individual
> > patch commit for detail.
> >
> > This patchset adds fully tested otg id switch function and
> > vbus connect/disconnect detection for chipidea driver.
> > The mainly design of id/vbus handling follows msm otg driver.
> > I hope the msm usb maintainer can have a look of this patchset,
> > and give some comments, and move the whole msm usb driver to
> > chipidea framework if possible in the future.
> >
> > This patchset is fully tested at i.mx6Q saberlite board.
>
> Have you tested it on a 3.8 kernel?
Hi Maxime,
I just tested my branch (https://github.com/hzpeterchen/linux-usb)
It works ok at i.mx6Q saberlite board.
The version is Linux version 3.8.0-rc2+, and it is based on greg's
usb-next(top of 102ee001912f67a7701f26a56ef2bcf84fc78028).
>
> On a 3.7 kernel, it worked flawlessly on an imx28, but I rebased your
> patches on top of 3.8, and now, it doesn't work anymore. At probe, it
> generates the following panic:
>
> [ 1.257375] ci_hdrc ci_hdrc.0: doesn't support host
> [ 1.268937] ci_hdrc ci_hdrc.0: can't init gadget role, ret=-524
> [ 1.277062] Unable to handle kernel NULL pointer dereference at virtual address 00000044
> [ 1.285343] pgd = c0004000
> [ 1.288093] [00000044] *pgd=00000000
> [ 1.291718] Internal error: Oops: 5 [#1] ARM
> [ 1.296000] Modules linked in:
> [ 1.299093] CPU: 0 Not tainted (3.8.0-rc2-00021-ge35aebb #339)
> [ 1.305312] PC is at ci13xxx_imx_probe+0x1d8/0x3d0
> [ 1.310125] LR is at ci13xxx_imx_probe+0x1d8/0x3d0
> [ 1.314906] pc : [<c025124c>] lr : [<c025124c>] psr: a0000013
> [ 1.314906] sp : c782fe90 ip : c0507578 fp : 00000000
> [ 1.326406] r10: 00000000 r9 : c797e8a0 r8 : c79c26d0
> [ 1.331656] r7 : c787f600 r6 : c787f610 r5 : 00000000 r4 : c79e1600
> [ 1.338187] r3 : c7871b00 r2 : 20000013 r1 : c79c26d0 r0 : 00000000
> [ 1.344718] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel
> [ 1.352031] Control: 0005317f Table: 40004000 DAC: 00000017
> [ 1.357781] Process swapper (pid: 1, stack limit = 0xc782e1b8)
> [ 1.363656] Stack: (0xc782fe90 to 0xc7830000)
> [ 1.368031] fe80: c0a776c0 c787f610 c787f610 c0a776c0
> [ 1.376218] fea0: c787f644 c05227e4 00000000 c04e5500 c782e000 c02042ec c02042d8 c0203030
> [ 1.384437] fec0: c787f610 c05227e4 c787f644 c052fa60 00000000 c0203264 00000000 c05227e4
> [ 1.392625] fee0: c02031d0 c02017b4 c7803ca8 c7871b50 c05227e4 c79cdb00 c051a4c8 c020287c
> [ 1.400843] ff00: c047021c c05227e4 c05227e4 c04efed0 c052fa60 c052fa60 c04efeb0 c0203730
> [ 1.409031] ff20: 00000000 c04f4e0c c04efed0 c052fa60 c052fa60 c04efeb0 c782e000 c0008898
> [ 1.417250] ff40: c0470b50 c04b1860 00000006 00000006 ffffffff c0b88cc0 c04f4e0c c04efed0
> [ 1.425437] ff60: 00000007 c04efeb0 0000007c c04d421c 00000000 c035ebb0 00000006 00000006
> [ 1.433656] ff80: c04d421c 00000000 c035ea9c 00000000 00000000 c035ea9c 00000000 00000000
> [ 1.441843] ffa0: 00000000 00000000 00000000 c000ed48 00000000 00000000 00000000 00000000
> [ 1.450031] ffc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> [ 1.458250] ffe0: 00000000 00000000 00000000 00000000 00000013 00000000 36060003 012c1285
> [ 1.466500] [<c025124c>] (ci13xxx_imx_probe+0x1d8/0x3d0) from [<c02042ec>] (platform_drv_probe+0x14/0x18)
> [ 1.476093] [<c02042ec>] (platform_drv_probe+0x14/0x18) from [<c0203030>] (driver_probe_device+0x74/0x214)
> [ 1.485781] [<c0203030>] (driver_probe_device+0x74/0x214) from [<c0203264>] (__driver_attach+0x94/0x98)
> [ 1.495218] [<c0203264>] (__driver_attach+0x94/0x98) from [<c02017b4>] (bus_for_each_dev+0x50/0x80)
> [ 1.504312] [<c02017b4>] (bus_for_each_dev+0x50/0x80) from [<c020287c>] (bus_add_driver+0x17c/0x250)
> [ 1.513468] [<c020287c>] (bus_add_driver+0x17c/0x250) from [<c0203730>] (driver_register+0x78/0x14c)
> [ 1.522625] [<c0203730>] (driver_register+0x78/0x14c) from [<c0008898>] (do_one_initcall+0x108/0x17c)
> [ 1.531875] [<c0008898>] (do_one_initcall+0x108/0x17c) from [<c035ebb0>] (kernel_init+0x114/0x2a4)
> [ 1.540875] [<c035ebb0>] (kernel_init+0x114/0x2a4) from [<c000ed48>] (ret_from_fork+0x14/0x2c)
> [ 1.549531] Code: e1a00006 ebfec61d e2840010 ebfec5f0 (e5903044)
> [ 1.555906] ---[ end trace ad9061e486664e45 ]---
>
> I dug a bit into it, and it seems there's two different bug here:
> - The probe function, when calling ci_hdrc_host_init, get ENXIO as
> the error code. This is new to 3.8. I don't really see why it would
> happen, because, judging from the code, only the first hw_read in
> the function returns such code, and this code hasn't change between
> the two versions.
> - The second one is that since the host mode is not enabled, the
> is_otg variable is never set to true, and thus, the function
> ci_hdrc_otg_init is never called. This is problematic since it
> registers the set_peripheral callback, that is called later in the
> function if the gadget role is defined. And I believe that this is
> what actually triggers the panic.
Please always enable both device and host mode for chipidea driver.
Current code still not consider all things for different mode config,
like device-only, host-only, otg. In future, it is supposed to do
at dts file, and judge probe.
>
> Thanks,
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Kernel, drivers, real-time and embedded Linux
> development, consulting, training and support.
> http://free-electrons.com
>
--
Best Regards,
Peter Chen
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v4 0/7] Add fully tested id switch and vbus connect detect support for Chipidea
2013-01-09 3:23 ` Peter Chen
@ 2013-01-09 11:43 ` Maxime Ripard
2013-01-10 1:38 ` Peter Chen
0 siblings, 1 reply; 17+ messages in thread
From: Maxime Ripard @ 2013-01-09 11:43 UTC (permalink / raw)
To: linux-arm-kernel
Hi Peter,
Thanks for your answer.
On 09/01/2013 04:23, Peter Chen wrote:
> On Tue, Jan 08, 2013 at 04:27:21PM +0100, Maxime Ripard wrote:
>> Hi Peter,
>>
>> On 27/12/2012 07:59, Peter Chen wrote:
>>> (Sorry for update slowly due to long time business trip)
>>>
>>> Changes for v4 mainly for 2/7, 3/7, 4/7, see individual
>>> patch commit for detail.
>>>
>>> This patchset adds fully tested otg id switch function and
>>> vbus connect/disconnect detection for chipidea driver.
>>> The mainly design of id/vbus handling follows msm otg driver.
>>> I hope the msm usb maintainer can have a look of this patchset,
>>> and give some comments, and move the whole msm usb driver to
>>> chipidea framework if possible in the future.
>>>
>>> This patchset is fully tested at i.mx6Q saberlite board.
>>
>> Have you tested it on a 3.8 kernel?
> Hi Maxime,
>
> I just tested my branch (https://github.com/hzpeterchen/linux-usb)
> It works ok at i.mx6Q saberlite board.
> The version is Linux version 3.8.0-rc2+, and it is based on greg's
> usb-next(top of 102ee001912f67a7701f26a56ef2bcf84fc78028).
Ok, I finally got what's going on.
Commit 09f6ffde introduced a dependency on USB_EHCI_HCD for host mode,
that in turn depends on USB_ARCH_HAS_EHCI, which isn't selected by MXS.
It was thus removing the host mode support when switching from 3.7 to
3.8, and I somehow overlooked that.
Enabling the host mode solved the panic.
> Please always enable both device and host mode for chipidea driver.
> Current code still not consider all things for different mode config,
> like device-only, host-only, otg. In future, it is supposed to do
> at dts file, and judge probe.
Then isn't the gadget mode configuration option supposed to depends on
the host mode if it's known to be broken otherwise?
Thanks,
Maxime
--
Maxime Ripard, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v4 0/7] Add fully tested id switch and vbus connect detect support for Chipidea
2013-01-09 11:43 ` Maxime Ripard
@ 2013-01-10 1:38 ` Peter Chen
0 siblings, 0 replies; 17+ messages in thread
From: Peter Chen @ 2013-01-10 1:38 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jan 09, 2013 at 12:43:38PM +0100, Maxime Ripard wrote:
> Hi Peter,
>
> Thanks for your answer.
>
>
> > Please always enable both device and host mode for chipidea driver.
> > Current code still not consider all things for different mode config,
> > like device-only, host-only, otg. In future, it is supposed to do
> > at dts file, and judge probe.
>
> Then isn't the gadget mode configuration option supposed to depends on
> the host mode if it's known to be broken otherwise?
At udc, it uses otg_set_peripheral, which depends on otg. OTG depends on
both host and device are enabled.
In fact, we don't need to use otg_set_peripheral at udc if all usb part
is in chipidea driver, but currently, msm also uses chipidea/udc.c, and
its otg and host is out of chipidea/
I am working on suspend/resume, low power mode and wakeup on chipidea driver.
After that, I will work on it.
>
> Thanks,
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Kernel, drivers, real-time and embedded Linux
> development, consulting, training and support.
> http://free-electrons.com
>
--
Best Regards,
Peter Chen
^ permalink raw reply [flat|nested] 17+ messages in thread