chipidea/imx: add otg support and some bug fix

PATCH Superseded Richard Zhao 7 participants 29 messages Jul 12, 2012 – Jul 30, 2012

Subsystems (4)

Device tree bindings USB Arm i.MX and Layerscape Chipidea USB high speed dual role controller

lore

Share & tools

lore

Richard Zhao

author

2012-07-12 07:01 UTC #0 lore raw

cover chipidea/imx: add otg support and some bug fix

The patch set is tested on imx6q_sabrelite board.

The patch can also be found at
https://github.com/riczhao/kernel-imx/commits/topics/usb-driver

For test which merged platform patches:
https://github.com/riczhao/kernel-imx/commits/topics/usb-test

It's better apply after patch I sent out:
usb: chipidea: cleanup dma_pool if udc_start() fails

Richard Zhao (12):
USB: chipidea: imx: add pinctrl support
USB: chipidea: delay 2ms before read ID status at probe time
USB: chipidea: move OTGSC_IDIS clearing from ci_role_work to irq
handler
USB: chipidea: clear gadget struct at udc_start fail path
USB: chipidea: don't let probe fail if otg controller start one role
failed
USB: mxs-phy: add basic otg support
USB: chipidea: add vbus detect for udc
USB: chipidea: convert to use devm_request_irq
USB: chipidea: add -DDEBUG if CONFIG_USB_CHIPIDEA_DEBUG
USB: chipidea: add set_vbus_power support
USB: chipidea: re-order irq handling to avoid unhandled irq
USB: chipidea: add imx usbmisc support

 .../devicetree/bindings/usb/imx-usbmisc.txt        |   15 ++
 drivers/usb/chipidea/Makefile                      |    4 +-
 drivers/usb/chipidea/ci.h                          |    1 +
 drivers/usb/chipidea/ci13xxx_imx.c                 |   50 +++++--
 drivers/usb/chipidea/core.c                        |   36 +++--
 drivers/usb/chipidea/host.c                        |    8 ++
 drivers/usb/chipidea/imx_usbmisc.c                 |  144 ++++++++++++++++++++
 drivers/usb/chipidea/udc.c                         |   40 +++++-
 drivers/usb/otg/mxs-phy.c                          |   21 +++
 include/linux/usb/chipidea.h                       |    2 +
 10 files changed, 293 insertions(+), 28 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/usb/imx-usbmisc.txt
 create mode 100644 drivers/usb/chipidea/imx_usbmisc.c

Signature · 1 line

1.7.9.5

Richard Zhao

author

2012-07-12 07:01 UTC #1 lore raw

PATCH 1/12 USB: chipidea: imx: add pinctrl support

Some controllers may not need to setup pinctrl, so we don't fail the probe if pinctrl get/select failed.

Signed-off-by Richard Zhao <[email protected]>

---
 drivers/usb/chipidea/ci13xxx_imx.c |    7 +++++++
 1 file changed, 7 insertions(+)

drivers/usb/chipidea/ci13xxx_imx.c +7 -0

index ef60d06..c94e30f 100644
@@ -20,6 +20,7 @@
 #include <linux/usb/chipidea.h>
 #include <linux/clk.h>
 #include <linux/regulator/consumer.h>
+#include <linux/pinctrl/consumer.h>
 
 #include "ci.h"

@@ -49,6 +50,7 @@ static int __devinit ci13xxx_imx_probe(struct platform_device *pdev)
 	struct device_node *phy_np;
 	struct resource *res;
 	struct regulator *reg_vbus;
+	struct pinctrl *pinctrl;
 	int ret;
 
 	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);

@@ -63,6 +65,11 @@ static int __devinit ci13xxx_imx_probe(struct platform_device *pdev)
 		return -ENOENT;
 	}
 
+	pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
+	if (IS_ERR(pinctrl))
+		dev_warn(&pdev->dev, "pinctrl get/select failed, err=%ld\n",
+			PTR_ERR(pinctrl));
+
 	data->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(data->clk)) {
 		dev_err(&pdev->dev,

Signature · 1 line

1.7.9.5

Richard Zhao

author

2012-07-12 07:01 UTC #2 lore raw

PATCH 2/12 USB: chipidea: delay 2ms before read ID status at probe time

In reply to Richard Zhao

The ID pin needs 1ms debounce time, event at probe time. We delay 2ms for safe.

Signed-off-by Richard Zhao <[email protected]>

---
 drivers/usb/chipidea/core.c |    2 ++
 1 file changed, 2 insertions(+)

drivers/usb/chipidea/core.c +2 -0

index 1083585..3c3ed77 100644
@@ -462,6 +462,8 @@ static int __devinit ci_hdrc_probe(struct platform_device *pdev)
 
 	if (ci->roles[CI_ROLE_HOST] && ci->roles[CI_ROLE_GADGET]) {
 		ci->is_otg = true;
+		/* ID pin needs 1ms debouce time, we delay 2ms for safe */
+		mdelay(2);
 		ci->role = ci_otg_role(ci);
 	} else {
 		ci->role = ci->roles[CI_ROLE_HOST]

Signature · 1 line

1.7.9.5

Richard Zhao

author

2012-07-12 07:01 UTC #3 lore raw

PATCH 3/12 USB: chipidea: move OTGSC_IDIS clearing from ci_role_work to irq handler

In reply to Richard Zhao

OTGSC_IDIS must be cleared in irq handler to avoid re-queue the work.

Signed-off-by Richard Zhao <[email protected]>

---
 drivers/usb/chipidea/core.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

drivers/usb/chipidea/core.c +1 -2

index 3c3ed77..19ef324 100644
@@ -273,8 +273,6 @@ static void ci_role_work(struct work_struct *work)
 	struct ci13xxx *ci = container_of(work, struct ci13xxx, work);
 	enum ci_role role = ci_otg_role(ci);
 
-	hw_write(ci, OP_OTGSC, OTGSC_IDIS, OTGSC_IDIS);
-
 	if (role != ci->role) {
 		dev_dbg(ci->dev, "switching from %s to %s\n",
 			ci_role(ci)->name, ci->roles[role]->name);

@@ -325,6 +323,7 @@ static irqreturn_t ci_irq(int irq, void *data)
 		u32 sts = hw_read(ci, OP_OTGSC, ~0);
 
 		if (sts & OTGSC_IDIS) {
+			hw_write(ci, OP_OTGSC, OTGSC_IDIS, OTGSC_IDIS);
 			queue_work(ci->wq, &ci->work);
 			ret = IRQ_HANDLED;
 		}

Signature · 1 line

1.7.9.5

Richard Zhao

author

2012-07-12 07:01 UTC #4 lore raw

PATCH 4/12 USB: chipidea: clear gadget struct at udc_start fail path

In reply to Richard Zhao

States in gadget are not needed any more, set it to zero.

Signed-off-by Richard Zhao <[email protected]>

---
 drivers/usb/chipidea/udc.c |    1 +
 1 file changed, 1 insertion(+)

drivers/usb/chipidea/udc.c +1 -0

index 9029985..fd27f4d 100644
@@ -1759,6 +1759,7 @@ free_pools:
 	dma_pool_destroy(ci->td_pool);
 free_qh_pool:
 	dma_pool_destroy(ci->qh_pool);
+	memset(&ci->gadget, 0, sizeof(ci->gadget));
 	return retval;
 }

Signature · 1 line

1.7.9.5

Richard Zhao

author

2012-07-12 07:01 UTC #5 lore raw

PATCH 5/12 USB: chipidea: don't let probe fail if otg controller start one role failed

In reply to Richard Zhao

One role failed, but the other role will possiblly still work. For example, when udc start failed, if plug in a host device, it works.

Signed-off-by Richard Zhao <[email protected]>

---
 drivers/usb/chipidea/core.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

drivers/usb/chipidea/core.c +5 -2

index 19ef324..8fd390a 100644
@@ -473,8 +473,11 @@ static int __devinit ci_hdrc_probe(struct platform_device *pdev)
 	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;
+		ci->role = CI_ROLE_END;
+		if (!ci->is_otg) {
+			ret = -ENODEV;
+			goto rm_wq;
+		}
 	}
 
 	platform_set_drvdata(pdev, ci);

Signature · 1 line

1.7.9.5

Richard Zhao

author

2012-07-12 07:01 UTC #6 lore raw

PATCH 6/12 USB: mxs-phy: add basic otg support

In reply to Richard Zhao

Signed-off-by Richard Zhao <[email protected]>

---
 drivers/usb/otg/mxs-phy.c |   21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

drivers/usb/otg/mxs-phy.c +21 -0

index c1a67cb..6a03e97 100644
@@ -97,12 +97,24 @@ static int mxs_phy_on_disconnect(struct usb_phy *phy, int port)
 	return 0;
 }
 
+static int mxs_phy_set_host(struct usb_otg *otg, struct usb_bus *host)
+{
+	return 0;
+}
+
+static int mxs_phy_set_peripheral(struct usb_otg *otg,
+					struct usb_gadget *gadget)
+{
+	return 0;
+}
+
 static int mxs_phy_probe(struct platform_device *pdev)
 {
 	struct resource *res;
 	void __iomem *base;
 	struct clk *clk;
 	struct mxs_phy *mxs_phy;
+	struct usb_otg *otg;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {

@@ -139,6 +151,15 @@ static int mxs_phy_probe(struct platform_device *pdev)
 
 	mxs_phy->clk = clk;
 
+	otg = devm_kzalloc(&pdev->dev, sizeof(struct usb_otg), GFP_KERNEL);
+	if (!otg)
+		return -ENOMEM;
+	otg->phy = &mxs_phy->phy;
+	otg->set_host = mxs_phy_set_host;
+	otg->set_peripheral = mxs_phy_set_peripheral;
+
+	mxs_phy->phy.otg = otg;
+
 	platform_set_drvdata(pdev, &mxs_phy->phy);
 
 	return 0;

Signature · 1 line

1.7.9.5

Richard Zhao

author

2012-07-12 07:01 UTC #7 lore raw

PATCH 7/12 USB: chipidea: add vbus detect for udc

In reply to Richard Zhao

Using vbus valid interrupt to detect vbus.

Signed-off-by Richard Zhao <[email protected]>

---
 drivers/usb/chipidea/ci.h  |    1 +
 drivers/usb/chipidea/udc.c |   39 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 39 insertions(+), 1 deletion(-)

drivers/usb/chipidea/ci.h +1 -0

index d738603..e25d126 100644
@@ -139,6 +139,7 @@ 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;

drivers/usb/chipidea/udc.c +38 -1

index fd27f4d..0b18191 100644
@@ -308,6 +308,18 @@ 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)

@@ -374,6 +386,16 @@ 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
  *****************************************************************************/

@@ -1376,6 +1398,7 @@ 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);

@@ -1528,8 +1551,10 @@ 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;

@@ -1635,6 +1660,13 @@ 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;

@@ -1710,6 +1742,7 @@ 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);

@@ -1773,6 +1806,9 @@ 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);

@@ -1813,6 +1849,7 @@ 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;
 }

Signature · 1 line

1.7.9.5

Richard Zhao

author

2012-07-12 07:01 UTC #8 lore raw

PATCH 8/12 USB: chipidea: convert to use devm_request_irq

In reply to Richard Zhao

Signed-off-by Richard Zhao <[email protected]>

---
 drivers/usb/chipidea/core.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

drivers/usb/chipidea/core.c +2 -3

index 8fd390a..7485c84 100644
@@ -481,8 +481,8 @@ static int __devinit ci_hdrc_probe(struct platform_device *pdev)
 	}
 
 	platform_set_drvdata(pdev, ci);
-	ret = request_irq(ci->irq, ci_irq, IRQF_SHARED, ci->platdata->name,
-			  ci);
+	ret = devm_request_irq(dev, ci->irq, ci_irq, IRQF_SHARED,
+				ci->platdata->name, ci);
 	if (ret)
 		goto stop;

@@ -513,7 +513,6 @@ static int __devexit ci_hdrc_remove(struct platform_device *pdev)
 	flush_workqueue(ci->wq);
 	destroy_workqueue(ci->wq);
 	device_remove_file(ci->dev, &dev_attr_role);
-	free_irq(ci->irq, ci);
 	ci_role_stop(ci);
 
 	return 0;

Signature · 1 line

1.7.9.5

Richard Zhao

author

2012-07-12 07:01 UTC #9 lore raw

PATCH 9/12 USB: chipidea: add -DDEBUG if CONFIG_USB_CHIPIDEA_DEBUG

In reply to Richard Zhao

Signed-off-by Richard Zhao <[email protected]>

---
 drivers/usb/chipidea/Makefile |    2 ++
 1 file changed, 2 insertions(+)

drivers/usb/chipidea/Makefile +2 -0

index 5c66d9c..3f56b76 100644
@@ -1,3 +1,5 @@
+ccflags-$(CONFIG_USB_CHIPIDEA_DEBUG) := -DDEBUG
+
 obj-$(CONFIG_USB_CHIPIDEA)		+= ci_hdrc.o
 
 ci_hdrc-y				:= core.o

Signature · 1 line

1.7.9.5

Richard Zhao

author

2012-07-12 07:01 UTC #10 lore raw

PATCH 10/12 USB: chipidea: add set_vbus_power support

In reply to Richard Zhao

set_vbus_power is used to enable or disable vbus power for usb host.

Signed-off-by Richard Zhao <[email protected]>

---
 drivers/usb/chipidea/ci13xxx_imx.c |   39 +++++++++++++++++++++++++-----------
 drivers/usb/chipidea/host.c        |    8 ++++++++
 include/linux/usb/chipidea.h       |    2 ++
 3 files changed, 37 insertions(+), 12 deletions(-)

drivers/usb/chipidea/ci13xxx_imx.c +27 ~~-12~~

index c94e30f..b3173d8 100644
@@ -26,6 +26,8 @@
 
 #define pdev_to_phy(pdev) \
 	((struct usb_phy *)platform_get_drvdata(pdev))
+#define ci_to_imx_data(ci) \
+	((struct ci13xxx_imx_data *)dev_get_drvdata(ci->dev->parent))
 
 struct ci13xxx_imx_data {
 	struct device_node *phy_np;

@@ -35,12 +37,32 @@ struct ci13xxx_imx_data {
 	struct regulator *reg_vbus;
 };
 
+static int ci13xxx_imx_vbus(struct ci13xxx *ci, int enable)
+{
+	struct ci13xxx_imx_data *data = ci_to_imx_data(ci);
+	int ret;
+
+	if (!data->reg_vbus)
+		return 0;
+
+	if (enable)
+		ret = regulator_enable(data->reg_vbus);
+	else
+		ret = regulator_disable(data->reg_vbus);
+	if (ret)
+		dev_err(ci->dev, "ci13xxx_imx_vbus failed, enable:%d err:%d\n",
+			enable, ret);
+
+	return ret;
+}
+
 static struct ci13xxx_platform_data ci13xxx_imx_platdata __devinitdata  = {
 	.name			= "ci13xxx_imx",
 	.flags			= CI13XXX_REQUIRE_TRANSCEIVER |
 				  CI13XXX_PULLUP_ON_VBUS |
 				  CI13XXX_DISABLE_STREAMING,
 	.capoffset		= DEF_CAPOFFSET,
+	.set_vbus_power		= ci13xxx_imx_vbus,
 };
 
 static int __devinit ci13xxx_imx_probe(struct platform_device *pdev)

@@ -101,18 +123,10 @@ static int __devinit 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;

@@ -127,6 +141,9 @@ static int __devinit ci13xxx_imx_probe(struct platform_device *pdev)
 		*pdev->dev.dma_mask = DMA_BIT_MASK(32);
 		dma_set_coherent_mask(&pdev->dev, *pdev->dev.dma_mask);
 	}
+
+	platform_set_drvdata(pdev, data);
+
 	plat_ci = ci13xxx_add_device(&pdev->dev,
 				pdev->resource, pdev->num_resources,
 				&ci13xxx_imx_platdata);

@@ -139,7 +156,6 @@ static int __devinit ci13xxx_imx_probe(struct platform_device *pdev)
 	}
 
 	data->ci_pdev = plat_ci;
-	platform_set_drvdata(pdev, data);
 
 	pm_runtime_no_callbacks(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);

@@ -149,7 +165,6 @@ static int __devinit ci13xxx_imx_probe(struct platform_device *pdev)
 err:
 	if (reg_vbus)
 		regulator_disable(reg_vbus);
-put_np:
 	if (phy_np)
 		of_node_put(phy_np);
 	clk_disable_unprepare(data->clk);

drivers/usb/chipidea/host.c +8 -0

index ebff9f4..e091147 100644
@@ -106,6 +106,12 @@ static int host_start(struct ci13xxx *ci)
 	if (usb_disabled())
 		return -ENODEV;
 
+	if (ci->platdata->set_vbus_power) {
+		ret = ci->platdata->set_vbus_power(ci, 1);
+		if (ret)
+			return ret;
+	}
+
 	hcd = usb_create_hcd(&ci_ehci_hc_driver, ci->dev, dev_name(ci->dev));
 	if (!hcd)
 		return -ENOMEM;

@@ -138,6 +144,8 @@ static void host_stop(struct ci13xxx *ci)
 
 	usb_remove_hcd(hcd);
 	usb_put_hcd(hcd);
+	if (ci->platdata->set_vbus_power)
+		ci->platdata->set_vbus_power(ci, 0);
 }
 
 int ci_hdrc_host_init(struct ci13xxx *ci)

include/linux/usb/chipidea.h +2 -0

index 544825d..080f479 100644
@@ -23,6 +23,8 @@ struct ci13xxx_platform_data {
 #define CI13XXX_CONTROLLER_RESET_EVENT		0
 #define CI13XXX_CONTROLLER_STOPPED_EVENT	1
 	void	(*notify_event) (struct ci13xxx *ci, unsigned event);
+	/* set vbus power, it must be called in non-atomic context */
+	int	(*set_vbus_power) (struct ci13xxx *ci, int enable);
 };
 
 /* Default offset of capability registers */

Signature · 1 line

1.7.9.5

Marc Kleine-Budde

2012-07-16 12:10 UTC #11 lore raw

PATCH 10/12 USB: chipidea: add set_vbus_power support

Quoted 56 lines · Richard Zhao

set_vbus_power is used to enable or disable vbus power for usb host.

Signed-off-by Richard Zhao <[email protected]>

---
 drivers/usb/chipidea/ci13xxx_imx.c |   39 +++++++++++++++++++++++++-----------
 drivers/usb/chipidea/host.c        |    8 ++++++++
 include/linux/usb/chipidea.h       |    2 ++
 3 files changed, 37 insertions(+), 12 deletions(-)

drivers/usb/chipidea/ci13xxx_imx.c +22 -0

index c94e30f..b3173d8 100644
@@ -26,6 +26,8 @@
 
 #define pdev_to_phy(pdev) \
 	((struct usb_phy *)platform_get_drvdata(pdev))
+#define ci_to_imx_data(ci) \
+	((struct ci13xxx_imx_data *)dev_get_drvdata(ci->dev->parent))
 
 struct ci13xxx_imx_data {
 	struct device_node *phy_np;

@@ -35,12 +37,32 @@ struct ci13xxx_imx_data {
 	struct regulator *reg_vbus;
 };
 
+static int ci13xxx_imx_vbus(struct ci13xxx *ci, int enable)
+{
+	struct ci13xxx_imx_data *data = ci_to_imx_data(ci);
+	int ret;
+
+	if (!data->reg_vbus)
+		return 0;
+
+	if (enable)
+		ret = regulator_enable(data->reg_vbus);
+	else
+		ret = regulator_disable(data->reg_vbus);
+	if (ret)
+		dev_err(ci->dev, "ci13xxx_imx_vbus failed, enable:%d err:%d\n",
+			enable, ret);
+
+	return ret;
+}
+
 static struct ci13xxx_platform_data ci13xxx_imx_platdata __devinitdata  = {
 	.name			= "ci13xxx_imx",
 	.flags			= CI13XXX_REQUIRE_TRANSCEIVER |
 				  CI13XXX_PULLUP_ON_VBUS |
 				  CI13XXX_DISABLE_STREAMING,
 	.capoffset		= DEF_CAPOFFSET,
+	.set_vbus_power		= ci13xxx_imx_vbus,
 };
 
 static int __devinit ci13xxx_imx_probe(struct platform_device *pdev)

drivers/usb/chipidea/ci13xxx_imx.c static int __devinit ci13xxx_imx_probe(struct platform_device *pdev) · line 124

Quoted 2 lines

drivers/usb/chipidea/ci13xxx_imx.c +0 -0

@@ -101,18 +123,10 @@ static int __devinit ci13xxx_imx_probe(struct platform_device *pdev)
 
 	/* we only support host now, so enable vbus here */

With this patch, the comment becomes wrong.

Quoted 6 lines

drivers/usb/chipidea/ci13xxx_imx.c +0 -5

 	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",

drivers/usb/chipidea/ci13xxx_imx.c static int __devinit ci13xxx_imx_probe(struct platform_device *pdev) · line 130

Quoted 10 lines

drivers/usb/chipidea/ci13xxx_imx.c +2 -5

-				ret);
-			goto put_np;
-		}
+	if (!IS_ERR(reg_vbus))
 		data->reg_vbus = reg_vbus;
-	} else {
+	else
 		reg_vbus = NULL;
-	}

Marc

Signature · 12 lines

Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 262 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120716/c2d25a56/attachment.sig>

Richard Zhao

author

2012-07-17 01:30 UTC #12 lore raw

PATCH 10/12 USB: chipidea: add set_vbus_power support

Quoted 55 lines · Marc Kleine-Budde

Quoted 54 lines · Richard Zhao

set_vbus_power is used to enable or disable vbus power for usb host.

Signed-off-by Richard Zhao <[email protected]>

---
 drivers/usb/chipidea/ci13xxx_imx.c |   39 +++++++++++++++++++++++++-----------
 drivers/usb/chipidea/host.c        |    8 ++++++++
 include/linux/usb/chipidea.h       |    2 ++
 3 files changed, 37 insertions(+), 12 deletions(-)

drivers/usb/chipidea/ci13xxx_imx.c +22 -0

index c94e30f..b3173d8 100644
@@ -26,6 +26,8 @@
 
 #define pdev_to_phy(pdev) \
 	((struct usb_phy *)platform_get_drvdata(pdev))
+#define ci_to_imx_data(ci) \
+	((struct ci13xxx_imx_data *)dev_get_drvdata(ci->dev->parent))
 
 struct ci13xxx_imx_data {
 	struct device_node *phy_np;

@@ -35,12 +37,32 @@ struct ci13xxx_imx_data {
 	struct regulator *reg_vbus;
 };
 
+static int ci13xxx_imx_vbus(struct ci13xxx *ci, int enable)
+{
+	struct ci13xxx_imx_data *data = ci_to_imx_data(ci);
+	int ret;
+
+	if (!data->reg_vbus)
+		return 0;
+
+	if (enable)
+		ret = regulator_enable(data->reg_vbus);
+	else
+		ret = regulator_disable(data->reg_vbus);
+	if (ret)
+		dev_err(ci->dev, "ci13xxx_imx_vbus failed, enable:%d err:%d\n",
+			enable, ret);
+
+	return ret;
+}
+
 static struct ci13xxx_platform_data ci13xxx_imx_platdata __devinitdata  = {
 	.name			= "ci13xxx_imx",
 	.flags			= CI13XXX_REQUIRE_TRANSCEIVER |
 				  CI13XXX_PULLUP_ON_VBUS |
 				  CI13XXX_DISABLE_STREAMING,
 	.capoffset		= DEF_CAPOFFSET,
+	.set_vbus_power		= ci13xxx_imx_vbus,
 };

Quoted 6 lines

drivers/usb/chipidea/ci13xxx_imx.c static int __devinit ci13xxx_imx_probe(struct platform_device *pdev) · line 124
Quoted 4 lines
drivers/usb/chipidea/ci13xxx_imx.c +0 -0
 static int __devinit ci13xxx_imx_probe(struct platform_device *pdev)
@@ -101,18 +123,10 @@ static int __devinit ci13xxx_imx_probe(struct platform_device *pdev)
 
 	/* we only support host now, so enable vbus here */
With this patch, the comment becomes wrong.

Good catch.

Thanks
Richard

Quoted 26 lines

Quoted 7 lines

drivers/usb/chipidea/ci13xxx_imx.c +0 -5

 	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",

drivers/usb/chipidea/ci13xxx_imx.c static int __devinit ci13xxx_imx_probe(struct platform_device *pdev) · line 130

Quoted 10 lines

drivers/usb/chipidea/ci13xxx_imx.c +2 -5

-				ret);
-			goto put_np;
-		}
+	if (!IS_ERR(reg_vbus))
 		data->reg_vbus = reg_vbus;
-	} else {
+	else
 		reg_vbus = NULL;
-	}

Marc

Signature · 4 lines

Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

Richard Zhao

author

2012-07-12 07:01 UTC #13 lore raw

PATCH 11/12 USB: chipidea: re-order irq handling to avoid unhandled irq

In reply to Richard Zhao

- let role driver handle irq before ID change check. It give the role driver a chance to handle disconnect.
- disable irq during switch role. No role driver to handle irq in the period.

Signed-off-by Richard Zhao <[email protected]>

---
 drivers/usb/chipidea/core.c |   21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

drivers/usb/chipidea/core.c +13 -8

index 7485c84..0942b9b 100644
@@ -279,6 +279,7 @@ static void ci_role_work(struct work_struct *work)
 
 		ci_role_stop(ci);
 		ci_role_start(ci, role);
+		enable_irq(ci->irq);
 	}
 }

@@ -318,18 +319,22 @@ static irqreturn_t ci_irq(int irq, void *data)
 {
 	struct ci13xxx *ci = data;
 	irqreturn_t ret = IRQ_NONE;
+	u32 otgsc = 0;
 
-	if (ci->is_otg) {
-		u32 sts = hw_read(ci, OP_OTGSC, ~0);
+	if (ci->is_otg)
+		otgsc = hw_read(ci, OP_OTGSC, ~0);
 
-		if (sts & OTGSC_IDIS) {
-			hw_write(ci, OP_OTGSC, OTGSC_IDIS, OTGSC_IDIS);
-			queue_work(ci->wq, &ci->work);
-			ret = IRQ_HANDLED;
-		}
+	if (ci->role != CI_ROLE_END)
+		ret = ci_role(ci)->irq(ci);
+
+	if (ci->is_otg && (otgsc & OTGSC_IDIS)) {
+		hw_write(ci, OP_OTGSC, OTGSC_IDIS, OTGSC_IDIS);
+		disable_irq_nosync(ci->irq);
+		queue_work(ci->wq, &ci->work);
+		ret = IRQ_HANDLED;
 	}
 
-	return ci->role == CI_ROLE_END ? ret : ci_role(ci)->irq(ci);
+	return ret;
 }
 
 static DEFINE_IDA(ci_ida);

Signature · 1 line

1.7.9.5

Richard Zhao

author

2012-07-12 07:01 UTC #14 lore raw

PATCH 12/12 USB: chipidea: add imx usbmisc support

In reply to Richard Zhao

i.MX usb controllers shares non-core registers, which may include SoC specific controls. We take it as a usbmisc device and usbmisc driver export functions needed by ci13xxx_imx driver.

For example, Sabrelite board has bad over-current design, we can usbmisc to disable over-current detect.

Signed-off-by Richard Zhao <[email protected]>

---
 .../devicetree/bindings/usb/imx-usbmisc.txt        |   15 ++
 drivers/usb/chipidea/Makefile                      |    2 +-
 drivers/usb/chipidea/ci13xxx_imx.c                 |    4 +
 drivers/usb/chipidea/imx_usbmisc.c                 |  144 ++++++++++++++++++++
 4 files changed, 164 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/usb/imx-usbmisc.txt
 create mode 100644 drivers/usb/chipidea/imx_usbmisc.c

Documentation/devicetree/bindings/usb/imx-usbmisc.txt new file +15 -0

new file mode 100644
index 0000000..09f01ca
@@ -0,0 +1,15 @@
+* Freescale i.MX non-core registers
+
+Required properties:
+- compatible: Should be "fsl,imx6q-usbmisc"
+- reg: Should contain registers location and length
+
+Optional properties:
+- fsl,disable-over-current: disable over current detect
+
+Examples:
+usbmisc at 02184800 {
+	compatible = "fsl,imx6q-usbmisc";
+	reg = <0x02184800 0x200>;
+	fsl,disable-over-current;
+};

drivers/usb/chipidea/Makefile +1 -1

index 3f56b76..46fc31c 100644
@@ -17,5 +17,5 @@ ifneq ($(CONFIG_PCI),)
 endif
 
 ifneq ($(CONFIG_OF_DEVICE),)
-	obj-$(CONFIG_USB_CHIPIDEA)	+= ci13xxx_imx.o
+	obj-$(CONFIG_USB_CHIPIDEA)	+= ci13xxx_imx.o imx_usbmisc.o
 endif

drivers/usb/chipidea/ci13xxx_imx.c +4 -0

index b3173d8..dd7f3a3 100644
@@ -24,6 +24,8 @@
 
 #include "ci.h"
 
+int imx_usbmisc_init(struct device *usbdev);
+
 #define pdev_to_phy(pdev) \
 	((struct usb_phy *)platform_get_drvdata(pdev))
 #define ci_to_imx_data(ci) \

@@ -142,6 +144,8 @@ static int __devinit ci13xxx_imx_probe(struct platform_device *pdev)
 		dma_set_coherent_mask(&pdev->dev, *pdev->dev.dma_mask);
 	}
 
+	imx_usbmisc_init(&pdev->dev);
+
 	platform_set_drvdata(pdev, data);
 
 	plat_ci = ci13xxx_add_device(&pdev->dev,

drivers/usb/chipidea/imx_usbmisc.c new file +144 -0

144 lines not on this page · raw message

Signature · 1 line

1.7.9.5

Richard Zhao

author

2012-07-12 07:10 UTC #15 lore raw

PATCH 12/12 USB: chipidea: add imx usbmisc support

[snip]

Quoted 9 lines

drivers/usb/chipidea/imx_usbmisc.c new file +6 -0

@@ -0,0 +1,144 @@
+/*
+ * Copyright 2012 Freescale Semiconductor, Inc.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:

drivers/usb/chipidea/imx_usbmisc.c line 16

Quoted 10 lines

drivers/usb/chipidea/imx_usbmisc.c new file +10 -0

+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <asm/io.h>

It should be linux/io.h

Thanks
Richard

Michael Grzeschik

2012-07-13 12:25 UTC #16 lore raw

PATCH 12/12 USB: chipidea: add imx usbmisc support

In reply to Richard Zhao

Quoted 3 lines · Richard Zhao

i.MX usb controllers shares non-core registers, which may include SoC specific controls. We take it as a usbmisc device and usbmisc driver export functions needed by ci13xxx_imx driver.

So this is SoC and not chipidea specific and should not be sorted into this subdir.

Quoted 2 lines

For example, Sabrelite board has bad over-current design, we can usbmisc to disable over-current detect.

This driver layout is also reflected in:

arch/arm/mach-imx/ehci-imx*.c

You should use these existing mx*_initialize_usb_hw functions to avoid code duplication and add your mx6_initialize_usb_hw routine for mx6 there.

This devicetree based glue code in this file can be reused to call the right mx*_initialize_usb_hw by the compatible.

We already have common flags available like i.e. MXC_EHCI_POWER_PINS_ENABLED which is currently used to disable overcurrent detection.

Quoted 94 lines

Signed-off-by Richard Zhao <[email protected]>

---
 .../devicetree/bindings/usb/imx-usbmisc.txt        |   15 ++
 drivers/usb/chipidea/Makefile                      |    2 +-
 drivers/usb/chipidea/ci13xxx_imx.c                 |    4 +
 drivers/usb/chipidea/imx_usbmisc.c                 |  144 ++++++++++++++++++++
 4 files changed, 164 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/usb/imx-usbmisc.txt
 create mode 100644 drivers/usb/chipidea/imx_usbmisc.c

Documentation/devicetree/bindings/usb/imx-usbmisc.txt new file +15 -0

new file mode 100644
index 0000000..09f01ca
@@ -0,0 +1,15 @@
+* Freescale i.MX non-core registers
+
+Required properties:
+- compatible: Should be "fsl,imx6q-usbmisc"
+- reg: Should contain registers location and length
+
+Optional properties:
+- fsl,disable-over-current: disable over current detect
+
+Examples:
+usbmisc at 02184800 {
+	compatible = "fsl,imx6q-usbmisc";
+	reg = <0x02184800 0x200>;
+	fsl,disable-over-current;
+};

drivers/usb/chipidea/Makefile +1 -1

index 3f56b76..46fc31c 100644
@@ -17,5 +17,5 @@ ifneq ($(CONFIG_PCI),)
 endif
 
 ifneq ($(CONFIG_OF_DEVICE),)
-	obj-$(CONFIG_USB_CHIPIDEA)	+= ci13xxx_imx.o
+	obj-$(CONFIG_USB_CHIPIDEA)	+= ci13xxx_imx.o imx_usbmisc.o
 endif

drivers/usb/chipidea/ci13xxx_imx.c +4 -0

index b3173d8..dd7f3a3 100644
@@ -24,6 +24,8 @@
 
 #include "ci.h"
 
+int imx_usbmisc_init(struct device *usbdev);
+
 #define pdev_to_phy(pdev) \
 	((struct usb_phy *)platform_get_drvdata(pdev))
 #define ci_to_imx_data(ci) \

@@ -142,6 +144,8 @@ static int __devinit ci13xxx_imx_probe(struct platform_device *pdev)
 		dma_set_coherent_mask(&pdev->dev, *pdev->dev.dma_mask);
 	}
 
+	imx_usbmisc_init(&pdev->dev);
+
 	platform_set_drvdata(pdev, data);
 
 	plat_ci = ci13xxx_add_device(&pdev->dev,

drivers/usb/chipidea/imx_usbmisc.c new file +24 -0

new file mode 100644
index 0000000..33a8ec0
@@ -0,0 +1,144 @@
+/*
+ * Copyright 2012 Freescale Semiconductor, Inc.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <asm/io.h>
+
+struct usbmisc;
+
+struct soc_data {
+	int (*init) (struct usbmisc *usbmisc, int id);
+	void (*exit) (struct usbmisc *usbmisc, int id);
+};
+

drivers/usb/chipidea/imx_usbmisc.c line 34

Quoted 10 lines

drivers/usb/chipidea/imx_usbmisc.c new file +10 -0

+struct usbmisc {
+	struct soc_data *soc_data;
+	void __iomem *base;
+	struct clk *clk;
+
+	int dis_oc:1;
+};
+
+/* Since we only have one usbmisc device at runtime, we global variables */
+static struct usbmisc *usbmisc;

Global variable?

drivers/usb/chipidea/imx_usbmisc.c line 36

Quoted 2 lines

drivers/usb/chipidea/imx_usbmisc.c new file +2 -0
+
+static int imx6q_usbmisc_init(struct usbmisc *usbmisc, int id)

What is initialized here? How about "preconfigure"? Even better is to reuse mx*_initialize_usb_hw here.

drivers/usb/chipidea/imx_usbmisc.c line 42

Quoted 6 lines

drivers/usb/chipidea/imx_usbmisc.c new file +6 -0

+{
+	u32 reg;
+
+	if (usbmisc->dis_oc) {
+		reg = readl_relaxed(usbmisc->base + id * 4);
+		writel_relaxed(reg | (1 << 7), usbmisc->base + id * 4);

#define IMX6_OTG_CTRL_USBMISC 1 << 7

drivers/usb/chipidea/imx_usbmisc.c line 54

Quoted 12 lines

drivers/usb/chipidea/imx_usbmisc.c new file +12 -0

+	}
+
+	return 0;
+}
+
+static struct soc_data imx6q_data = {
+	.init = imx6q_usbmisc_init,
+};
+
+
+static const struct of_device_id imx_usbmisc_dt_ids[] = {
+	{ .compatible = "fsl,imx6q-usbmisc", .data = &imx6q_data },

something like this:
{ .compatible = "fsl,imx6q-usbmisc", .data = &mx6_initialize_usb_hw },

Quoted 23 lines

drivers/usb/chipidea/imx_usbmisc.c new file +23 -0

+	{ /* sentinel */ }
+};
+
+static int __devinit imx_usbmisc_probe(struct platform_device *pdev)
+{
+	struct resource	*res;
+	struct usbmisc *data;
+	const struct of_device_id *of_id =
+			of_match_device(imx_usbmisc_dt_ids, &pdev->dev);
+
+	int ret;
+
+	if (usbmisc)
+		return -EBUSY;
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	data->base = devm_request_and_ioremap(&pdev->dev, res);
+	if (!data->base)
+		return -EADDRNOTAVAIL;

drivers/usb/chipidea/imx_usbmisc.c line 87

Quoted 10 lines

drivers/usb/chipidea/imx_usbmisc.c new file +10 -0

+
+	data->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(data->clk)) {
+		dev_err(&pdev->dev,
+			"failed to get clock, err=%ld\n", PTR_ERR(data->clk));
+		return PTR_ERR(data->clk);
+	}
+
+	ret = clk_prepare_enable(data->clk);
+	if (!ret) {

Better:

        if (ret)
                return ret;

drivers/usb/chipidea/imx_usbmisc.c line 94

Quoted 7 lines

drivers/usb/chipidea/imx_usbmisc.c new file +7 -0

+		if (of_find_property(pdev->dev.of_node,
+			"fsl,disable-over-current", NULL))
+			data->dis_oc = 1;
+		data->soc_data = of_id->data;
+		usbmisc = data;
+	}
+

        if (of_find_property(pdev->dev.of_node,
        ...

Quoted 26 lines

drivers/usb/chipidea/imx_usbmisc.c new file +26 -0

+	return ret;
+}
+
+static int __devexit imx_usbmisc_remove(struct platform_device *pdev)
+{
+	clk_disable_unprepare(usbmisc->clk);
+	return 0;
+}
+
+static struct platform_driver imx_usbmisc_driver = {
+	.probe = imx_usbmisc_probe,
+	.remove = __devexit_p(imx_usbmisc_remove),
+	.driver = {
+		.name = "imx_usbmisc",
+		.owner = THIS_MODULE,
+		.of_match_table = imx_usbmisc_dt_ids,
+	 },
+};
+
+int imx_usbmisc_init(struct device *usbdev)
+{
+	struct device_node *np = usbdev->of_node;
+	int ret;
+
+	if (!usbmisc)
+		return 0;

drivers/usb/chipidea/imx_usbmisc.c line 130

Quoted 10 lines

drivers/usb/chipidea/imx_usbmisc.c new file +10 -0

+
+	ret = of_alias_get_id(np, "usb");
+	if (ret < 0) {
+		dev_err(usbdev, "failed to get alias id, errno %d\n", ret);
+		return ret;
+	}
+
+	return usbmisc->soc_data->init(usbmisc, ret);
+}
+EXPORT_SYMBOL_GPL(imx_usbmisc_init);

EXPORT_SYMBOL is only needed because it is
compliled as an extra kernel module.

Quoted 16 lines

drivers/usb/chipidea/imx_usbmisc.c new file +14 -0

+
+static int __init imx_usbmisc_drv_init(void)
+{
+	return platform_driver_register(&imx_usbmisc_driver);
+}
+subsys_initcall(imx_usbmisc_drv_init);
+
+static void __exit imx_usbmisc_drv_exit(void)
+{
+	platform_driver_unregister(&imx_usbmisc_driver);
+}
+module_exit(imx_usbmisc_drv_exit);
+
+MODULE_LICENSE("GPL v2");

Signature · 1 line

1.7.9.5

Thanks,
Michael

Signature · 4 lines

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 |

Richard Zhao

2012-07-13 14:02 UTC #17 lore raw

PATCH 12/12 USB: chipidea: add imx usbmisc support

Quoted 7 lines · Michael Grzeschik

Quoted 3 lines · Richard Zhao
i.MX usb controllers shares non-core registers, which may include SoC specific controls. We take it as a usbmisc device and usbmisc driver export functions needed by ci13xxx_imx driver.
So this is SoC and not chipidea specific and should not be sorted into this subdir.

Yes, but it's usb specific and serve chipidea usb IP.

Quoted 7 lines

Quoted 3 lines
For example, Sabrelite board has bad over-current design, we can usbmisc to disable over-current detect.
This driver layout is also reflected in:
arch/arm/mach-imx/ehci-imx*.c

It sounds sensible, but I'm not quite sure. I saw drivers are moving out of there, event it's SoC specific, for example, clk, pinmux. And the non-core registers might not only be setup onetime on boot, but be called at runtime by usb driver. For now, it's set once.

Quoted 3 lines

You should use these existing mx*_initialize_usb_hw functions to avoid code duplication

I can not see what duplication it can avoid.

Quoted 1 line

and add your mx6_initialize_usb_hw routine for mx6 there.

Maybe.

Quoted 3 lines

This devicetree based glue code in this file can be reused to call the right mx*_initialize_usb_hw by the compatible.

The glue code makes it more like a normal driver. Doesn't it?

Quoted 3 lines

We already have common flags available like i.e. MXC_EHCI_POWER_PINS_ENABLED which is currently used to disable overcurrent detection.

The flags may be replace with properties in DT bindings.

Quoted 100 lines

Signed-off-by Richard Zhao <[email protected]>

---
 .../devicetree/bindings/usb/imx-usbmisc.txt        |   15 ++
 drivers/usb/chipidea/Makefile                      |    2 +-
 drivers/usb/chipidea/ci13xxx_imx.c                 |    4 +
 drivers/usb/chipidea/imx_usbmisc.c                 |  144 ++++++++++++++++++++
 4 files changed, 164 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/usb/imx-usbmisc.txt
 create mode 100644 drivers/usb/chipidea/imx_usbmisc.c

Documentation/devicetree/bindings/usb/imx-usbmisc.txt new file +15 -0

new file mode 100644
index 0000000..09f01ca
@@ -0,0 +1,15 @@
+* Freescale i.MX non-core registers
+
+Required properties:
+- compatible: Should be "fsl,imx6q-usbmisc"
+- reg: Should contain registers location and length
+
+Optional properties:
+- fsl,disable-over-current: disable over current detect
+
+Examples:
+usbmisc at 02184800 {
+	compatible = "fsl,imx6q-usbmisc";
+	reg = <0x02184800 0x200>;
+	fsl,disable-over-current;
+};

drivers/usb/chipidea/Makefile +1 -1

index 3f56b76..46fc31c 100644
@@ -17,5 +17,5 @@ ifneq ($(CONFIG_PCI),)
 endif
 
 ifneq ($(CONFIG_OF_DEVICE),)
-	obj-$(CONFIG_USB_CHIPIDEA)	+= ci13xxx_imx.o
+	obj-$(CONFIG_USB_CHIPIDEA)	+= ci13xxx_imx.o imx_usbmisc.o
 endif

drivers/usb/chipidea/ci13xxx_imx.c +4 -0

index b3173d8..dd7f3a3 100644
@@ -24,6 +24,8 @@
 
 #include "ci.h"
 
+int imx_usbmisc_init(struct device *usbdev);
+
 #define pdev_to_phy(pdev) \
 	((struct usb_phy *)platform_get_drvdata(pdev))
 #define ci_to_imx_data(ci) \

@@ -142,6 +144,8 @@ static int __devinit ci13xxx_imx_probe(struct platform_device *pdev)
 		dma_set_coherent_mask(&pdev->dev, *pdev->dev.dma_mask);
 	}
 
+	imx_usbmisc_init(&pdev->dev);
+
 	platform_set_drvdata(pdev, data);
 
 	plat_ci = ci13xxx_add_device(&pdev->dev,

drivers/usb/chipidea/imx_usbmisc.c new file +28 -0

new file mode 100644
index 0000000..33a8ec0
@@ -0,0 +1,144 @@
+/*
+ * Copyright 2012 Freescale Semiconductor, Inc.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <asm/io.h>
+
+struct usbmisc;
+
+struct soc_data {
+	int (*init) (struct usbmisc *usbmisc, int id);
+	void (*exit) (struct usbmisc *usbmisc, int id);
+};
+
+struct usbmisc {
+	struct soc_data *soc_data;
+	void __iomem *base;
+	struct clk *clk;

Quoted 8 lines

drivers/usb/chipidea/imx_usbmisc.c line 34
Quoted 6 lines
drivers/usb/chipidea/imx_usbmisc.c new file +6 -0
+
+	int dis_oc:1;
+};
+
+/* Since we only have one usbmisc device at runtime, we global variables */
+static struct usbmisc *usbmisc;
Global variable?

Yes. we only have one usbmisc.

Quoted 6 lines

drivers/usb/chipidea/imx_usbmisc.c line 36
Quoted 3 lines
drivers/usb/chipidea/imx_usbmisc.c new file +2 -0
+
+static int imx6q_usbmisc_init(struct usbmisc *usbmisc, int id)
What is initialized here? How about "preconfigure"? Even better is to reuse mx*_initialize_usb_hw here.

It's used to call by ci13xxx_imx driver.

Quoted 9 lines

drivers/usb/chipidea/imx_usbmisc.c line 42
Quoted 7 lines
drivers/usb/chipidea/imx_usbmisc.c new file +6 -0
+{
+	u32 reg;
+
+	if (usbmisc->dis_oc) {
+		reg = readl_relaxed(usbmisc->base + id * 4);
+		writel_relaxed(reg | (1 << 7), usbmisc->base + id * 4);
#define IMX6_OTG_CTRL_USBMISC 1 << 7

yes.

Quoted 16 lines

drivers/usb/chipidea/imx_usbmisc.c line 54
Quoted 13 lines
drivers/usb/chipidea/imx_usbmisc.c new file +12 -0
+	}
+
+	return 0;
+}
+
+static struct soc_data imx6q_data = {
+	.init = imx6q_usbmisc_init,
+};
+
+
+static const struct of_device_id imx_usbmisc_dt_ids[] = {
+	{ .compatible = "fsl,imx6q-usbmisc", .data = &imx6q_data },
something like this:
{ .compatible = "fsl,imx6q-usbmisc", .data = &mx6_initialize_usb_hw },

It's designed not only used when initialization.

Quoted 36 lines

Quoted 24 lines

drivers/usb/chipidea/imx_usbmisc.c new file +23 -0

+	{ /* sentinel */ }
+};
+
+static int __devinit imx_usbmisc_probe(struct platform_device *pdev)
+{
+	struct resource	*res;
+	struct usbmisc *data;
+	const struct of_device_id *of_id =
+			of_match_device(imx_usbmisc_dt_ids, &pdev->dev);
+
+	int ret;
+
+	if (usbmisc)
+		return -EBUSY;
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	data->base = devm_request_and_ioremap(&pdev->dev, res);
+	if (!data->base)
+		return -EADDRNOTAVAIL;

drivers/usb/chipidea/imx_usbmisc.c line 87

Quoted 10 lines

drivers/usb/chipidea/imx_usbmisc.c new file +10 -0

+
+	data->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(data->clk)) {
+		dev_err(&pdev->dev,
+			"failed to get clock, err=%ld\n", PTR_ERR(data->clk));
+		return PTR_ERR(data->clk);
+	}
+
+	ret = clk_prepare_enable(data->clk);
+	if (!ret) {

Better:

Quoted 2 lines

        if (ret)
                return ret;

To me, it does not make much difference.

Quoted 42 lines

drivers/usb/chipidea/imx_usbmisc.c line 94

Quoted 8 lines

drivers/usb/chipidea/imx_usbmisc.c new file +7 -0

+		if (of_find_property(pdev->dev.of_node,
+			"fsl,disable-over-current", NULL))
+			data->dis_oc = 1;
+		data->soc_data = of_id->data;
+		usbmisc = data;
+	}
+

        if (of_find_property(pdev->dev.of_node,
        ...

Quoted 31 lines

drivers/usb/chipidea/imx_usbmisc.c new file +31 -0

+	return ret;
+}
+
+static int __devexit imx_usbmisc_remove(struct platform_device *pdev)
+{
+	clk_disable_unprepare(usbmisc->clk);
+	return 0;
+}
+
+static struct platform_driver imx_usbmisc_driver = {
+	.probe = imx_usbmisc_probe,
+	.remove = __devexit_p(imx_usbmisc_remove),
+	.driver = {
+		.name = "imx_usbmisc",
+		.owner = THIS_MODULE,
+		.of_match_table = imx_usbmisc_dt_ids,
+	 },
+};
+
+int imx_usbmisc_init(struct device *usbdev)
+{
+	struct device_node *np = usbdev->of_node;
+	int ret;
+
+	if (!usbmisc)
+		return 0;
+
+	ret = of_alias_get_id(np, "usb");
+	if (ret < 0) {
+		dev_err(usbdev, "failed to get alias id, errno %d\n", ret);
+		return ret;

Quoted 8 lines

drivers/usb/chipidea/imx_usbmisc.c line 130
Quoted 5 lines
drivers/usb/chipidea/imx_usbmisc.c new file +5 -0
+	}
+
+	return usbmisc->soc_data->init(usbmisc, ret);
+}
+EXPORT_SYMBOL_GPL(imx_usbmisc_init);
EXPORT_SYMBOL is only needed because it is
compliled as an extra kernel module.

ci13xxx_imx may be module.

Thanks
Richard

Quoted 30 lines

Quoted 17 lines

drivers/usb/chipidea/imx_usbmisc.c new file +14 -0

+
+static int __init imx_usbmisc_drv_init(void)
+{
+	return platform_driver_register(&imx_usbmisc_driver);
+}
+subsys_initcall(imx_usbmisc_drv_init);
+
+static void __exit imx_usbmisc_drv_exit(void)
+{
+	platform_driver_unregister(&imx_usbmisc_driver);
+}
+module_exit(imx_usbmisc_drv_exit);
+
+MODULE_LICENSE("GPL v2");

Signature · 1 line

1.7.9.5

Thanks,
Michael

Signature · 8 lines

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 |
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Marc Kleine-Budde

2012-07-13 14:14 UTC #18 lore raw

PATCH 12/12 USB: chipidea: add imx usbmisc support

Quoted 9 lines · Richard Zhao

Quoted 7 lines · Michael Grzeschik
Quoted 3 lines · Richard Zhao
i.MX usb controllers shares non-core registers, which may include SoC specific controls. We take it as a usbmisc device and usbmisc driver export functions needed by ci13xxx_imx driver.
So this is SoC and not chipidea specific and should not be sorted into this subdir.
Yes, but it's usb specific and serve chipidea usb IP.

I don't care where this code is located.

Quoted 11 lines

Quoted 6 lines
Quoted 2 lines
For example, Sabrelite board has bad over-current design, we can usbmisc to disable over-current detect.
This driver layout is also reflected in:
arch/arm/mach-imx/ehci-imx*.c
It sounds sensible, but I'm not quite sure. I saw drivers are moving out of there, event it's SoC specific, for example, clk, pinmux. And the non-core registers might not only be setup onetime on boot, but be called at runtime by usb driver. For now, it's set once.
Quoted 1 line

Quoted 4 lines

Quoted 2 lines
You should use these existing mx*_initialize_usb_hw functions to avoid code duplication
I can not see what duplication it can avoid.

We want to use the chipidea driver for the other imx, too. We already have existing code for the non-DT case in arch/arm/mach-imx/ehci-imx*.c. So we need to glue these functions to the DT or duplicate the code, which is to be avoided.

Quoted 2 lines

Quoted 1 line
and add your mx6_initialize_usb_hw routine for mx6 there.
Maybe.

Or at least next to the other imx usb-misc code (wherever that will end up).

Quoted 7 lines

Quoted 2 lines
This devicetree based glue code in this file can be reused to call the right mx*_initialize_usb_hw by the compatible.
The glue code makes it more like a normal driver. Doesn't it?
Quoted 3 lines
We already have common flags available like i.e. MXC_EHCI_POWER_PINS_ENABLED which is currently used to disable overcurrent detection.
The flags may be replace with properties in DT bindings.

Yes, write some code that extracts these flags from the DT and then calls the existing code (for the existing imx platforms).

Quoted 179 lines

Quoted 98 lines

Signed-off-by Richard Zhao <[email protected]>

---
 .../devicetree/bindings/usb/imx-usbmisc.txt        |   15 ++
 drivers/usb/chipidea/Makefile                      |    2 +-
 drivers/usb/chipidea/ci13xxx_imx.c                 |    4 +
 drivers/usb/chipidea/imx_usbmisc.c                 |  144 ++++++++++++++++++++
 4 files changed, 164 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/usb/imx-usbmisc.txt
 create mode 100644 drivers/usb/chipidea/imx_usbmisc.c

Documentation/devicetree/bindings/usb/imx-usbmisc.txt new file +15 -0

new file mode 100644
index 0000000..09f01ca
@@ -0,0 +1,15 @@
+* Freescale i.MX non-core registers
+
+Required properties:
+- compatible: Should be "fsl,imx6q-usbmisc"
+- reg: Should contain registers location and length
+
+Optional properties:
+- fsl,disable-over-current: disable over current detect
+
+Examples:
+usbmisc at 02184800 {
+	compatible = "fsl,imx6q-usbmisc";
+	reg = <0x02184800 0x200>;
+	fsl,disable-over-current;
+};

drivers/usb/chipidea/Makefile +1 -1

index 3f56b76..46fc31c 100644
@@ -17,5 +17,5 @@ ifneq ($(CONFIG_PCI),)
 endif
 
 ifneq ($(CONFIG_OF_DEVICE),)
-	obj-$(CONFIG_USB_CHIPIDEA)	+= ci13xxx_imx.o
+	obj-$(CONFIG_USB_CHIPIDEA)	+= ci13xxx_imx.o imx_usbmisc.o
 endif

drivers/usb/chipidea/ci13xxx_imx.c +4 -0

index b3173d8..dd7f3a3 100644
@@ -24,6 +24,8 @@
 
 #include "ci.h"
 
+int imx_usbmisc_init(struct device *usbdev);
+
 #define pdev_to_phy(pdev) \
 	((struct usb_phy *)platform_get_drvdata(pdev))
 #define ci_to_imx_data(ci) \

@@ -142,6 +144,8 @@ static int __devinit ci13xxx_imx_probe(struct platform_device *pdev)
 		dma_set_coherent_mask(&pdev->dev, *pdev->dev.dma_mask);
 	}
 
+	imx_usbmisc_init(&pdev->dev);
+
 	platform_set_drvdata(pdev, data);
 
 	plat_ci = ci13xxx_add_device(&pdev->dev,

drivers/usb/chipidea/imx_usbmisc.c new file +28 -0

new file mode 100644
index 0000000..33a8ec0
@@ -0,0 +1,144 @@
+/*
+ * Copyright 2012 Freescale Semiconductor, Inc.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <asm/io.h>
+
+struct usbmisc;
+
+struct soc_data {
+	int (*init) (struct usbmisc *usbmisc, int id);
+	void (*exit) (struct usbmisc *usbmisc, int id);
+};
+
+struct usbmisc {
+	struct soc_data *soc_data;
+	void __iomem *base;
+	struct clk *clk;

Quoted 8 lines

drivers/usb/chipidea/imx_usbmisc.c line 34
Quoted 6 lines
drivers/usb/chipidea/imx_usbmisc.c new file +6 -0
+
+	int dis_oc:1;
+};
+
+/* Since we only have one usbmisc device at runtime, we global variables */
+static struct usbmisc *usbmisc;
Global variable?

Yes. we only have one usbmisc.

Quoted 6 lines

drivers/usb/chipidea/imx_usbmisc.c line 36
Quoted 3 lines
drivers/usb/chipidea/imx_usbmisc.c new file +2 -0
+
+static int imx6q_usbmisc_init(struct usbmisc *usbmisc, int id)
What is initialized here? How about "preconfigure"? Even better is to reuse mx*_initialize_usb_hw here.

It's used to call by ci13xxx_imx driver.

Quoted 9 lines

drivers/usb/chipidea/imx_usbmisc.c line 42
Quoted 7 lines
drivers/usb/chipidea/imx_usbmisc.c new file +6 -0
+{
+	u32 reg;
+
+	if (usbmisc->dis_oc) {
+		reg = readl_relaxed(usbmisc->base + id * 4);
+		writel_relaxed(reg | (1 << 7), usbmisc->base + id * 4);
#define IMX6_OTG_CTRL_USBMISC 1 << 7

yes.

Quoted 16 lines

drivers/usb/chipidea/imx_usbmisc.c line 54
Quoted 13 lines
drivers/usb/chipidea/imx_usbmisc.c new file +12 -0
+	}
+
+	return 0;
+}
+
+static struct soc_data imx6q_data = {
+	.init = imx6q_usbmisc_init,
+};
+
+
+static const struct of_device_id imx_usbmisc_dt_ids[] = {
+	{ .compatible = "fsl,imx6q-usbmisc", .data = &imx6q_data },
something like this:
{ .compatible = "fsl,imx6q-usbmisc", .data = &mx6_initialize_usb_hw },

It's designed not only used when initialization.

Quoted 36 lines

Quoted 24 lines

drivers/usb/chipidea/imx_usbmisc.c new file +23 -0

+	{ /* sentinel */ }
+};
+
+static int __devinit imx_usbmisc_probe(struct platform_device *pdev)
+{
+	struct resource	*res;
+	struct usbmisc *data;
+	const struct of_device_id *of_id =
+			of_match_device(imx_usbmisc_dt_ids, &pdev->dev);
+
+	int ret;
+
+	if (usbmisc)
+		return -EBUSY;
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	data->base = devm_request_and_ioremap(&pdev->dev, res);
+	if (!data->base)
+		return -EADDRNOTAVAIL;

drivers/usb/chipidea/imx_usbmisc.c line 87

Quoted 10 lines

drivers/usb/chipidea/imx_usbmisc.c new file +10 -0

+
+	data->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(data->clk)) {
+		dev_err(&pdev->dev,
+			"failed to get clock, err=%ld\n", PTR_ERR(data->clk));
+		return PTR_ERR(data->clk);
+	}
+
+	ret = clk_prepare_enable(data->clk);
+	if (!ret) {

Better:

Quoted 2 lines

        if (ret)
                return ret;

Quoted 1 line

To me, it does not make much difference.

Not the the compiler, but IMHO the common pattern in the kernel is:

ret = sensible_init();
if (ret)

	return ret;

do_work();

Quoted 50 lines

Quoted 42 lines

drivers/usb/chipidea/imx_usbmisc.c line 94

Quoted 8 lines

drivers/usb/chipidea/imx_usbmisc.c new file +7 -0

+		if (of_find_property(pdev->dev.of_node,
+			"fsl,disable-over-current", NULL))
+			data->dis_oc = 1;
+		data->soc_data = of_id->data;
+		usbmisc = data;
+	}
+

        if (of_find_property(pdev->dev.of_node,
        ...

Quoted 31 lines

drivers/usb/chipidea/imx_usbmisc.c new file +31 -0

+	return ret;
+}
+
+static int __devexit imx_usbmisc_remove(struct platform_device *pdev)
+{
+	clk_disable_unprepare(usbmisc->clk);
+	return 0;
+}
+
+static struct platform_driver imx_usbmisc_driver = {
+	.probe = imx_usbmisc_probe,
+	.remove = __devexit_p(imx_usbmisc_remove),
+	.driver = {
+		.name = "imx_usbmisc",
+		.owner = THIS_MODULE,
+		.of_match_table = imx_usbmisc_dt_ids,
+	 },
+};
+
+int imx_usbmisc_init(struct device *usbdev)
+{
+	struct device_node *np = usbdev->of_node;
+	int ret;
+
+	if (!usbmisc)
+		return 0;
+
+	ret = of_alias_get_id(np, "usb");
+	if (ret < 0) {
+		dev_err(usbdev, "failed to get alias id, errno %d\n", ret);
+		return ret;

Quoted 8 lines

drivers/usb/chipidea/imx_usbmisc.c line 130
Quoted 5 lines
drivers/usb/chipidea/imx_usbmisc.c new file +5 -0
+	}
+
+	return usbmisc->soc_data->init(usbmisc, ret);
+}
+EXPORT_SYMBOL_GPL(imx_usbmisc_init);
EXPORT_SYMBOL is only needed because it is
compliled as an extra kernel module.

Quoted 1 line

ci13xxx_imx may be module.

Why make the usb_misc an independent module, why not just link it into the ci13xxx_imx.ko

Marc

Signature · 12 lines

Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 262 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120713/63bb5eb7/attachment.sig>

Richard Zhao

author

2012-07-16 02:53 UTC #19 lore raw

PATCH 12/12 USB: chipidea: add imx usbmisc support

Sascha & Shawn, Felipe & Alex,

What's your opinion where I put this driver? The usbmisc drivers is SoC specific and provide callbacks to ci13xxx_imx driver at runtime.

- How to organize the driver
This patch is designed to put all per-soc driver in one file. But I plan to put it in discrete files.

- Where to put it
Option 1: arch/arm/mach-imx/ehci-imxXXX.c
Option 2: drivers/usb/chipidea/imx/ , yes, we may need a folder. Both is fine for me.

Quoted 7 lines · Marc Kleine-Budde

Quoted 6 lines · Richard Zhao
Quoted 5 lines · Michael Grzeschik
Quoted 3 lines · Richard Zhao
i.MX usb controllers shares non-core registers, which may include SoC specific controls. We take it as a usbmisc device and usbmisc driver export functions needed by ci13xxx_imx driver.

Quoted 5 lines

Quoted 3 lines
Quoted 2 lines
So this is SoC and not chipidea specific and should not be sorted into this subdir.
Yes, but it's usb specific and serve chipidea usb IP.
I don't care where this code is located.

I need maintainers' instruction here. :)

Quoted 15 lines

Quoted 14 lines
Quoted 7 lines
Quoted 3 lines
For example, Sabrelite board has bad over-current design, we can usbmisc to disable over-current detect.
This driver layout is also reflected in:
arch/arm/mach-imx/ehci-imx*.c
It sounds sensible, but I'm not quite sure. I saw drivers are moving out of there, event it's SoC specific, for example, clk, pinmux. And the non-core registers might not only be setup onetime on boot, but be called at runtime by usb driver. For now, it's set once.
Quoted 3 lines
You should use these existing mx*_initialize_usb_hw functions to avoid code duplication

Quoted 6 lines

Quoted 1 line
I can not see what duplication it can avoid.
We want to use the chipidea driver for the other imx, too. We already have existing code for the non-DT case in arch/arm/mach-imx/ehci-imx*.c. So we need to glue these functions to the DT or duplicate the code, which is to be avoided.

The existing code is too simple. ci13xxx_imx may call them at runtime.

Quoted 256 lines

Quoted 3 lines
Quoted 2 lines
and add your mx6_initialize_usb_hw routine for mx6 there.
Maybe.
Or at least next to the other imx usb-misc code (wherever that will end up).
Quoted 7 lines
Quoted 2 lines
This devicetree based glue code in this file can be reused to call the right mx*_initialize_usb_hw by the compatible.
The glue code makes it more like a normal driver. Doesn't it?
Quoted 3 lines
We already have common flags available like i.e. MXC_EHCI_POWER_PINS_ENABLED which is currently used to disable overcurrent detection.
The flags may be replace with properties in DT bindings.
Yes, write some code that extracts these flags from the DT and then calls the existing code (for the existing imx platforms).
Quoted 179 lines
Quoted 98 lines
Quoted 98 lines
Signed-off-by Richard Zhao <[email protected]>
---
 .../devicetree/bindings/usb/imx-usbmisc.txt        |   15 ++
 drivers/usb/chipidea/Makefile                      |    2 +-
 drivers/usb/chipidea/ci13xxx_imx.c                 |    4 +
 drivers/usb/chipidea/imx_usbmisc.c                 |  144 ++++++++++++++++++++
 4 files changed, 164 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/usb/imx-usbmisc.txt
 create mode 100644 drivers/usb/chipidea/imx_usbmisc.c
Documentation/devicetree/bindings/usb/imx-usbmisc.txt new file +15 -0
new file mode 100644
index 0000000..09f01ca
@@ -0,0 +1,15 @@
+* Freescale i.MX non-core registers
+
+Required properties:
+- compatible: Should be "fsl,imx6q-usbmisc"
+- reg: Should contain registers location and length
+
+Optional properties:
+- fsl,disable-over-current: disable over current detect
+
+Examples:
+usbmisc at 02184800 {
+	compatible = "fsl,imx6q-usbmisc";
+	reg = <0x02184800 0x200>;
+	fsl,disable-over-current;
+};
drivers/usb/chipidea/Makefile +1 -1
index 3f56b76..46fc31c 100644
@@ -17,5 +17,5 @@ ifneq ($(CONFIG_PCI),)
 endif
 
 ifneq ($(CONFIG_OF_DEVICE),)
-	obj-$(CONFIG_USB_CHIPIDEA)	+= ci13xxx_imx.o
+	obj-$(CONFIG_USB_CHIPIDEA)	+= ci13xxx_imx.o imx_usbmisc.o
 endif
drivers/usb/chipidea/ci13xxx_imx.c +4 -0
index b3173d8..dd7f3a3 100644
@@ -24,6 +24,8 @@
 
 #include "ci.h"
 
+int imx_usbmisc_init(struct device *usbdev);
+
 #define pdev_to_phy(pdev) \
 	((struct usb_phy *)platform_get_drvdata(pdev))
 #define ci_to_imx_data(ci) \
@@ -142,6 +144,8 @@ static int __devinit ci13xxx_imx_probe(struct platform_device *pdev)
 		dma_set_coherent_mask(&pdev->dev, *pdev->dev.dma_mask);
 	}
 
+	imx_usbmisc_init(&pdev->dev);
+
 	platform_set_drvdata(pdev, data);
 
 	plat_ci = ci13xxx_add_device(&pdev->dev,
drivers/usb/chipidea/imx_usbmisc.c new file +28 -0
new file mode 100644
index 0000000..33a8ec0
@@ -0,0 +1,144 @@
+/*
+ * Copyright 2012 Freescale Semiconductor, Inc.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <asm/io.h>
+
+struct usbmisc;
+
+struct soc_data {
+	int (*init) (struct usbmisc *usbmisc, int id);
+	void (*exit) (struct usbmisc *usbmisc, int id);
+};
+
+struct usbmisc {
+	struct soc_data *soc_data;
+	void __iomem *base;
+	struct clk *clk;
Quoted 8 lines
drivers/usb/chipidea/imx_usbmisc.c line 34
Quoted 6 lines
drivers/usb/chipidea/imx_usbmisc.c new file +6 -0
+
+	int dis_oc:1;
+};
+
+/* Since we only have one usbmisc device at runtime, we global variables */
+static struct usbmisc *usbmisc;
Global variable?
Yes. we only have one usbmisc.
Quoted 6 lines
drivers/usb/chipidea/imx_usbmisc.c line 36
Quoted 3 lines
drivers/usb/chipidea/imx_usbmisc.c new file +2 -0
+
+static int imx6q_usbmisc_init(struct usbmisc *usbmisc, int id)
What is initialized here? How about "preconfigure"? Even better is to reuse mx*_initialize_usb_hw here.
It's used to call by ci13xxx_imx driver.
Quoted 9 lines
drivers/usb/chipidea/imx_usbmisc.c line 42
Quoted 7 lines
drivers/usb/chipidea/imx_usbmisc.c new file +6 -0
+{
+	u32 reg;
+
+	if (usbmisc->dis_oc) {
+		reg = readl_relaxed(usbmisc->base + id * 4);
+		writel_relaxed(reg | (1 << 7), usbmisc->base + id * 4);
#define IMX6_OTG_CTRL_USBMISC 1 << 7
yes.
Quoted 16 lines
drivers/usb/chipidea/imx_usbmisc.c line 54
Quoted 13 lines
drivers/usb/chipidea/imx_usbmisc.c new file +12 -0
+	}
+
+	return 0;
+}
+
+static struct soc_data imx6q_data = {
+	.init = imx6q_usbmisc_init,
+};
+
+
+static const struct of_device_id imx_usbmisc_dt_ids[] = {
+	{ .compatible = "fsl,imx6q-usbmisc", .data = &imx6q_data },
something like this:
{ .compatible = "fsl,imx6q-usbmisc", .data = &mx6_initialize_usb_hw },
It's designed not only used when initialization.
Quoted 36 lines
Quoted 24 lines
drivers/usb/chipidea/imx_usbmisc.c new file +23 -0
+	{ /* sentinel */ }
+};
+
+static int __devinit imx_usbmisc_probe(struct platform_device *pdev)
+{
+	struct resource	*res;
+	struct usbmisc *data;
+	const struct of_device_id *of_id =
+			of_match_device(imx_usbmisc_dt_ids, &pdev->dev);
+
+	int ret;
+
+	if (usbmisc)
+		return -EBUSY;
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	data->base = devm_request_and_ioremap(&pdev->dev, res);
+	if (!data->base)
+		return -EADDRNOTAVAIL;
drivers/usb/chipidea/imx_usbmisc.c line 87
Quoted 10 lines
drivers/usb/chipidea/imx_usbmisc.c new file +10 -0
+
+	data->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(data->clk)) {
+		dev_err(&pdev->dev,
+			"failed to get clock, err=%ld\n", PTR_ERR(data->clk));
+		return PTR_ERR(data->clk);
+	}
+
+	ret = clk_prepare_enable(data->clk);
+	if (!ret) {
Better:
Quoted 2 lines
        if (ret)
                return ret;
Quoted 1 line
To me, it does not make much difference.
Not the the compiler, but IMHO the common pattern in the kernel is:
ret = sensible_init();
if (ret)
	return ret;
do_work();
Quoted 50 lines
Quoted 50 lines
drivers/usb/chipidea/imx_usbmisc.c line 94
Quoted 8 lines
drivers/usb/chipidea/imx_usbmisc.c new file +7 -0
+		if (of_find_property(pdev->dev.of_node,
+			"fsl,disable-over-current", NULL))
+			data->dis_oc = 1;
+		data->soc_data = of_id->data;
+		usbmisc = data;
+	}
+
        if (of_find_property(pdev->dev.of_node,
        ...
Quoted 26 lines
drivers/usb/chipidea/imx_usbmisc.c new file +26 -0
+	return ret;
+}
+
+static int __devexit imx_usbmisc_remove(struct platform_device *pdev)
+{
+	clk_disable_unprepare(usbmisc->clk);
+	return 0;
+}
+
+static struct platform_driver imx_usbmisc_driver = {
+	.probe = imx_usbmisc_probe,
+	.remove = __devexit_p(imx_usbmisc_remove),
+	.driver = {
+		.name = "imx_usbmisc",
+		.owner = THIS_MODULE,
+		.of_match_table = imx_usbmisc_dt_ids,
+	 },
+};
+
+int imx_usbmisc_init(struct device *usbdev)
+{
+	struct device_node *np = usbdev->of_node;
+	int ret;
+
+	if (!usbmisc)
+		return 0;
drivers/usb/chipidea/imx_usbmisc.c line 130
Quoted 10 lines
drivers/usb/chipidea/imx_usbmisc.c new file +10 -0
+
+	ret = of_alias_get_id(np, "usb");
+	if (ret < 0) {
+		dev_err(usbdev, "failed to get alias id, errno %d\n", ret);
+		return ret;
+	}
+
+	return usbmisc->soc_data->init(usbmisc, ret);
+}
+EXPORT_SYMBOL_GPL(imx_usbmisc_init);
EXPORT_SYMBOL is only needed because it is
compliled as an extra kernel module.

Quoted 4 lines

Quoted 1 line
ci13xxx_imx may be module.
Why make the usb_misc an independent module, why not just link it into the ci13xxx_imx.ko

Yes

Thanks
Richard

Quoted 8 lines

Marc

Signature · 4 lines

Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

Sascha Hauer

maintainer NAK

2012-07-16 08:25 UTC #20 lore raw

PATCH 12/12 USB: chipidea: add imx usbmisc support

In reply to Richard Zhao

Quoted 83 lines · Richard Zhao

i.MX usb controllers shares non-core registers, which may include SoC specific controls. We take it as a usbmisc device and usbmisc driver export functions needed by ci13xxx_imx driver.

For example, Sabrelite board has bad over-current design, we can usbmisc to disable over-current detect.

Signed-off-by Richard Zhao <[email protected]>

---
 .../devicetree/bindings/usb/imx-usbmisc.txt        |   15 ++
 drivers/usb/chipidea/Makefile                      |    2 +-
 drivers/usb/chipidea/ci13xxx_imx.c                 |    4 +
 drivers/usb/chipidea/imx_usbmisc.c                 |  144 ++++++++++++++++++++
 4 files changed, 164 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/usb/imx-usbmisc.txt
 create mode 100644 drivers/usb/chipidea/imx_usbmisc.c

Documentation/devicetree/bindings/usb/imx-usbmisc.txt new file +15 -0

new file mode 100644
index 0000000..09f01ca
@@ -0,0 +1,15 @@
+* Freescale i.MX non-core registers
+
+Required properties:
+- compatible: Should be "fsl,imx6q-usbmisc"
+- reg: Should contain registers location and length
+
+Optional properties:
+- fsl,disable-over-current: disable over current detect
+
+Examples:
+usbmisc at 02184800 {
+	compatible = "fsl,imx6q-usbmisc";
+	reg = <0x02184800 0x200>;
+	fsl,disable-over-current;
+};

drivers/usb/chipidea/Makefile +1 -1

index 3f56b76..46fc31c 100644
@@ -17,5 +17,5 @@ ifneq ($(CONFIG_PCI),)
 endif
 
 ifneq ($(CONFIG_OF_DEVICE),)
-	obj-$(CONFIG_USB_CHIPIDEA)	+= ci13xxx_imx.o
+	obj-$(CONFIG_USB_CHIPIDEA)	+= ci13xxx_imx.o imx_usbmisc.o
 endif

drivers/usb/chipidea/ci13xxx_imx.c +4 -0

index b3173d8..dd7f3a3 100644
@@ -24,6 +24,8 @@
 
 #include "ci.h"
 
+int imx_usbmisc_init(struct device *usbdev);
+
 #define pdev_to_phy(pdev) \
 	((struct usb_phy *)platform_get_drvdata(pdev))
 #define ci_to_imx_data(ci) \

@@ -142,6 +144,8 @@ static int __devinit ci13xxx_imx_probe(struct platform_device *pdev)
 		dma_set_coherent_mask(&pdev->dev, *pdev->dev.dma_mask);
 	}
 
+	imx_usbmisc_init(&pdev->dev);
+
 	platform_set_drvdata(pdev, data);
 
 	plat_ci = ci13xxx_add_device(&pdev->dev,

drivers/usb/chipidea/imx_usbmisc.c new file +6 -0

new file mode 100644
index 0000000..33a8ec0
@@ -0,0 +1,144 @@
+/*
+ * Copyright 2012 Freescale Semiconductor, Inc.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:

drivers/usb/chipidea/imx_usbmisc.c line 16

Quoted 10 lines

drivers/usb/chipidea/imx_usbmisc.c new file +10 -0

+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <asm/io.h>

linux/io.h

Quoted 8 lines

drivers/usb/chipidea/imx_usbmisc.c new file +8 -0

+
+struct usbmisc;
+
+struct soc_data {
+	int (*init) (struct usbmisc *usbmisc, int id);
+	void (*exit) (struct usbmisc *usbmisc, int id);
+};
+

drivers/usb/chipidea/imx_usbmisc.c line 34

Quoted 10 lines

drivers/usb/chipidea/imx_usbmisc.c new file +10 -0

+struct usbmisc {
+	struct soc_data *soc_data;
+	void __iomem *base;
+	struct clk *clk;
+
+	int dis_oc:1;
+};
+
+/* Since we only have one usbmisc device at runtime, we global variables */
+static struct usbmisc *usbmisc;

NACK

What you've done here exactly matches your current usecase but is not enough for any of the other usecases I can think of. Even the i.MX6 has three USB ports, each of them has a overcurrent disable bit. Also, there are more flags, like:

- use internal phy
- power pin polarity
- ttl enabled

I think the best you can do here is to add the flags to the fsl,imx6q-usb device nodes:

	usb at 02184000 { /* USB OTG */
		compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
		reg = <0x02184000 0x200>;
		interrupts = <0 43 0x04>;
		fsl,usbphy = <&usbphy1>;
		status = "disabled";
		fsl,disable-over-current;
		<other flags>;
	};

Then add a usbmisc device which does not contain any flags. During initialization of the fsl,imx6q-usb device you can then pass a pointer to its device node to imx_usbmisc_init.

Sascha

Signature · 4 lines

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 |

Richard Zhao

author

2012-07-16 08:38 UTC #21 lore raw

PATCH 12/12 USB: chipidea: add imx usbmisc support

Quoted 109 lines · Sascha Hauer

Quoted 83 lines · Richard Zhao

i.MX usb controllers shares non-core registers, which may include SoC specific controls. We take it as a usbmisc device and usbmisc driver export functions needed by ci13xxx_imx driver.

For example, Sabrelite board has bad over-current design, we can usbmisc to disable over-current detect.

Signed-off-by Richard Zhao <[email protected]>

---
 .../devicetree/bindings/usb/imx-usbmisc.txt        |   15 ++
 drivers/usb/chipidea/Makefile                      |    2 +-
 drivers/usb/chipidea/ci13xxx_imx.c                 |    4 +
 drivers/usb/chipidea/imx_usbmisc.c                 |  144 ++++++++++++++++++++
 4 files changed, 164 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/usb/imx-usbmisc.txt
 create mode 100644 drivers/usb/chipidea/imx_usbmisc.c

Documentation/devicetree/bindings/usb/imx-usbmisc.txt new file +15 -0

new file mode 100644
index 0000000..09f01ca
@@ -0,0 +1,15 @@
+* Freescale i.MX non-core registers
+
+Required properties:
+- compatible: Should be "fsl,imx6q-usbmisc"
+- reg: Should contain registers location and length
+
+Optional properties:
+- fsl,disable-over-current: disable over current detect
+
+Examples:
+usbmisc at 02184800 {
+	compatible = "fsl,imx6q-usbmisc";
+	reg = <0x02184800 0x200>;
+	fsl,disable-over-current;
+};

drivers/usb/chipidea/Makefile +1 -1

index 3f56b76..46fc31c 100644
@@ -17,5 +17,5 @@ ifneq ($(CONFIG_PCI),)
 endif
 
 ifneq ($(CONFIG_OF_DEVICE),)
-	obj-$(CONFIG_USB_CHIPIDEA)	+= ci13xxx_imx.o
+	obj-$(CONFIG_USB_CHIPIDEA)	+= ci13xxx_imx.o imx_usbmisc.o
 endif

drivers/usb/chipidea/ci13xxx_imx.c +4 -0

index b3173d8..dd7f3a3 100644
@@ -24,6 +24,8 @@
 
 #include "ci.h"
 
+int imx_usbmisc_init(struct device *usbdev);
+
 #define pdev_to_phy(pdev) \
 	((struct usb_phy *)platform_get_drvdata(pdev))
 #define ci_to_imx_data(ci) \

@@ -142,6 +144,8 @@ static int __devinit ci13xxx_imx_probe(struct platform_device *pdev)
 		dma_set_coherent_mask(&pdev->dev, *pdev->dev.dma_mask);
 	}
 
+	imx_usbmisc_init(&pdev->dev);
+
 	platform_set_drvdata(pdev, data);
 
 	plat_ci = ci13xxx_add_device(&pdev->dev,

drivers/usb/chipidea/imx_usbmisc.c new file +6 -0

new file mode 100644
index 0000000..33a8ec0
@@ -0,0 +1,144 @@
+/*
+ * Copyright 2012 Freescale Semiconductor, Inc.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:

drivers/usb/chipidea/imx_usbmisc.c line 16

Quoted 10 lines

drivers/usb/chipidea/imx_usbmisc.c new file +10 -0

+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <asm/io.h>

linux/io.h

Quoted 12 lines

drivers/usb/chipidea/imx_usbmisc.c new file +12 -0

+
+struct usbmisc;
+
+struct soc_data {
+	int (*init) (struct usbmisc *usbmisc, int id);
+	void (*exit) (struct usbmisc *usbmisc, int id);
+};
+
+struct usbmisc {
+	struct soc_data *soc_data;
+	void __iomem *base;
+	struct clk *clk;

Quoted 8 lines

drivers/usb/chipidea/imx_usbmisc.c line 34
Quoted 6 lines
drivers/usb/chipidea/imx_usbmisc.c new file +6 -0
+
+	int dis_oc:1;
+};
+
+/* Since we only have one usbmisc device at runtime, we global variables */
+static struct usbmisc *usbmisc;
NACK

Right, it's a bad design. I'm considering change it too. Maybe I will move it out this patch series. As I asked you in another mail in this thread, do you think it's good to put it here or in mach-imx/ ?

Quoted 5 lines

What you've done here exactly matches your current usecase but is not enough for any of the other usecases I can think of. Even the i.MX6 has three USB ports, each of them has a overcurrent disable bit. Also, there are more flags, like:

Yes, I'll change it to support different usb with different properties.

Quoted 4 lines

- use internal phy
- power pin polarity
- ttl enabled

I focus on imx6 now. So I think the property can be added when it's needed. Now I only use disable oc.

Quoted 14 lines

I think the best you can do here is to add the flags to the fsl,imx6q-usb device nodes:

	usb at 02184000 { /* USB OTG */
		compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
		reg = <0x02184000 0x200>;
		interrupts = <0 43 0x04>;
		fsl,usbphy = <&usbphy1>;
		status = "disabled";
		fsl,disable-over-current;
		<other flags>;
	};

Quoted 3 lines

Then add a usbmisc device which does not contain any flags. During initialization of the fsl,imx6q-usb device you can then pass a pointer to its device node to imx_usbmisc_init.

Good suggestion.

Thanks
Richard

Quoted 9 lines

Sascha

Signature · 4 lines

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 |

Sascha Hauer

maintainer

2012-07-16 08:50 UTC #22 lore raw

PATCH 12/12 USB: chipidea: add imx usbmisc support

Quoted 5 lines · Richard Zhao

Quoted 2 lines
NACK
Right, it's a bad design. I'm considering change it too. Maybe I will move it out this patch series. As I asked you in another mail in this thread, do you think it's good to put it here or in mach-imx/ ?

I prefer it to be in drivers/usb/chipidea.

Quoted 5 lines

Quoted 5 lines
What you've done here exactly matches your current usecase but is not enough for any of the other usecases I can think of. Even the i.MX6 has three USB ports, each of them has a overcurrent disable bit. Also, there are more flags, like:

Quoted 7 lines

Yes, I'll change it to support different usb with different properties.
Quoted 4 lines
- use internal phy
- power pin polarity
- ttl enabled
I focus on imx6 now. So I think the property can be added when it's needed. Now I only use disable oc.

That's fine, but you should give the impression that you thought about other usecases and other SoCs than your current one, it'll make it easier for us to believe that your patch does the right thing ;)

Sascha

Signature · 4 lines

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 |

Marek Vasut

2012-07-16 12:24 UTC #23 lore raw

PATCH 12/12 USB: chipidea: add imx usbmisc support

In reply to Richard Zhao

Dear Richard Zhao,

[...]

drivers/usb/chipidea/imx_usbmisc.c line 42

Quoted 7 lines

drivers/usb/chipidea/imx_usbmisc.c new file +7 -0

+static int imx6q_usbmisc_init(struct usbmisc *usbmisc, int id)
+{
+	u32 reg;
+
+	if (usbmisc->dis_oc) {
+		reg = readl_relaxed(usbmisc->base + id * 4);
+		writel_relaxed(reg | (1 << 7), usbmisc->base + id * 4);

1<< 7 ? This yearns for fixing with proper #defined value.

[...]

The rest of the patches looks OK, you can add my Reviewed-By: <[email protected]> to those. This one though will need further investigation.

Best regards,
Marek Vasut

Greg KH

maintainer

2012-07-17 00:40 UTC #24 lore raw

cover chipidea/imx: add otg support and some bug fix

In reply to Richard Zhao

Quoted 10 lines · Richard Zhao

The patch set is tested on imx6q_sabrelite board.
The patch can also be found at
https://github.com/riczhao/kernel-imx/commits/topics/usb-driver
For test which merged platform patches:
https://github.com/riczhao/kernel-imx/commits/topics/usb-test
It's better apply after patch I sent out:
usb: chipidea: cleanup dma_pool if udc_start() fails

I need acks from Alexander before I can accept any of these.

thanks,

greg k-h

Richard Zhao

author

2012-07-19 02:05 UTC #25 lore raw

cover chipidea/imx: add otg support and some bug fix

Quoted 6 lines · Greg KH

Quoted 5 lines · Richard Zhao
The patch set is tested on imx6q_sabrelite board.
The patch can also be found at
https://github.com/riczhao/kernel-imx/commits/topics/usb-driver

Quoted 7 lines

Quoted 5 lines
For test which merged platform patches:
https://github.com/riczhao/kernel-imx/commits/topics/usb-test
It's better apply after patch I sent out:
usb: chipidea: cleanup dma_pool if udc_start() fails
I need acks from Alexander before I can accept any of these.

ping Alexander ...

Quoted 5 lines

thanks,
greg k-h

Richard Zhao

author

2012-07-26 10:59 UTC #26 lore raw

cover chipidea/imx: add otg support and some bug fix

Quoted 7 lines · Richard Zhao

Quoted 6 lines · Greg KH
Quoted 5 lines · Richard Zhao
The patch set is tested on imx6q_sabrelite board.
The patch can also be found at
https://github.com/riczhao/kernel-imx/commits/topics/usb-driver

Quoted 8 lines

Quoted 7 lines
Quoted 5 lines
For test which merged platform patches:
https://github.com/riczhao/kernel-imx/commits/topics/usb-test
It's better apply after patch I sent out:
usb: chipidea: cleanup dma_pool if udc_start() fails
I need acks from Alexander before I can accept any of these.
ping Alexander ...

ping Alexander again ...

Richard Zhao

author

2012-07-30 09:17 UTC #27 lore raw

cover chipidea/imx: add otg support and some bug fix

Quoted 11 lines · Richard Zhao

Quoted 10 lines · Richard Zhao
Quoted 9 lines · Greg KH
Quoted 8 lines · Richard Zhao
The patch set is tested on imx6q_sabrelite board.
The patch can also be found at
https://github.com/riczhao/kernel-imx/commits/topics/usb-driver
For test which merged platform patches:
https://github.com/riczhao/kernel-imx/commits/topics/usb-test

Quoted 6 lines

Quoted 5 lines
Quoted 4 lines
Quoted 2 lines
It's better apply after patch I sent out:
usb: chipidea: cleanup dma_pool if udc_start() fails
I need acks from Alexander before I can accept any of these.
ping Alexander ...
ping Alexander again ...

Hi Greg,

Is there a better way but keeping waiting?

Hi Felipe,

Could you help review the patch series?

Thanks
Richard

Quoted 6 lines

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

Greg KH

maintainer

2012-07-30 16:00 UTC #28 lore raw

cover chipidea/imx: add otg support and some bug fix

Quoted 15 lines · Richard Zhao

Quoted 14 lines · Richard Zhao
Quoted 13 lines · Richard Zhao
Quoted 12 lines · Greg KH
Quoted 10 lines · Richard Zhao
The patch set is tested on imx6q_sabrelite board.
The patch can also be found at
https://github.com/riczhao/kernel-imx/commits/topics/usb-driver
For test which merged platform patches:
https://github.com/riczhao/kernel-imx/commits/topics/usb-test
It's better apply after patch I sent out:
usb: chipidea: cleanup dma_pool if udc_start() fails

Quoted 6 lines

Quoted 3 lines
Quoted 2 lines
Quoted 1 line
I need acks from Alexander before I can accept any of these.
ping Alexander ...
ping Alexander again ...
Hi Greg,
Is there a better way but keeping waiting?

It's the middle of the merge window, even if Alexander shows up right now, I can't do anything with these patches until 3.6-rc1 is out.

So be patient, this is vacation time for lots of people.

greg k-h