[PATCH 0/5] Chipidea driver support for the AR933x platform

[PATCH 0/5] Chipidea driver support for the AR933x platform

[Svetoslav Neykov]

Add support for the usb controller in AR933x platform.
The processor is big-endian so all multi-byte values of the usb 
descriptors must be converted explicitly. Another difference is that
the controller supports both host and device modes but not OTG.
The patches are tested on WR703n router running OpenWRT trunk.

Svetoslav Neykov (5):
  usb: chipidea: big-endian support
  usb: chipidea: flags to force usb mode (host/device)
  usb: chipidea: Don't access OTG related registers
  usb: chipidea: AR933x platform support for the chipidea driver
  usb: chipidea: Fix incorrect check of function return value

 arch/mips/ath79/dev-usb.c                      |   19 ++++++
 arch/mips/include/asm/mach-ath79/ar71xx_regs.h |    3 +
 drivers/usb/chipidea/Makefile                  |    1 +
 drivers/usb/chipidea/ci13xxx_ar933x.c          |   73 +++++++++++++++++++++
 drivers/usb/chipidea/core.c                    |   26 ++++++--
 drivers/usb/chipidea/udc.c                     |   83 ++++++++++++++----------
 include/linux/usb/chipidea.h                   |    2 +
 7 files changed, 164 insertions(+), 43 deletions(-)
 create mode 100644 drivers/usb/chipidea/ci13xxx_ar933x.c

-- 
1.7.9.5

[PATCH 1/5] usb: chipidea: big-endian support

[Svetoslav Neykov]

Convert between big-endian and little-endian format when accessing the usb controller structures which are little-endian by specification.
Fix cases where the little-endian memory layout is taken for granted.
The patch doesn't have any effect on the already supported little-endian architectures.

Signed-off-by: Svetoslav Neykov <svetoslav@neykov.name>
---
 drivers/usb/chipidea/core.c |    2 +-
 drivers/usb/chipidea/udc.c  |   59 +++++++++++++++++++++++--------------------
 2 files changed, 32 insertions(+), 29 deletions(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index aebf695..3cefb4c 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -181,7 +181,7 @@ static int hw_device_init(struct ci13xxx *ci, void __iomem *base)
 
 	ci->hw_bank.cap = ci->hw_bank.abs;
 	ci->hw_bank.cap += ci->platdata->capoffset;
-	ci->hw_bank.op = ci->hw_bank.cap + ioread8(ci->hw_bank.cap);
+	ci->hw_bank.op = ci->hw_bank.cap + (ioread32(ci->hw_bank.cap) & 0xFF);
 
 	hw_alloc_regmap(ci, false);
 	reg = hw_read(ci, CAP_HCCPARAMS, HCCPARAMS_LEN) >>
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 2f45bba..78ac5e5 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -432,10 +432,10 @@ static int _hardware_enqueue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq)
 			return -ENOMEM;
 
 		memset(mReq->zptr, 0, sizeof(*mReq->zptr));
-		mReq->zptr->next    = TD_TERMINATE;
-		mReq->zptr->token   = TD_STATUS_ACTIVE;
+		mReq->zptr->next    = cpu_to_le32(TD_TERMINATE);
+		mReq->zptr->token   = cpu_to_le32(TD_STATUS_ACTIVE);
 		if (!mReq->req.no_interrupt)
-			mReq->zptr->token   |= TD_IOC;
+			mReq->zptr->token   |= cpu_to_le32(TD_IOC);
 	}
 	ret = usb_gadget_map_request(&ci->gadget, &mReq->req, mEp->dir);
 	if (ret)
@@ -446,32 +446,35 @@ static int _hardware_enqueue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq)
 	 * TODO - handle requests which spawns into several TDs
 	 */
 	memset(mReq->ptr, 0, sizeof(*mReq->ptr));
-	mReq->ptr->token    = length << ffs_nr(TD_TOTAL_BYTES);
-	mReq->ptr->token   &= TD_TOTAL_BYTES;
-	mReq->ptr->token   |= TD_STATUS_ACTIVE;
+	mReq->ptr->token    = cpu_to_le32(length << ffs_nr(TD_TOTAL_BYTES));
+	mReq->ptr->token   &= cpu_to_le32(TD_TOTAL_BYTES);
+	mReq->ptr->token   |= cpu_to_le32(TD_STATUS_ACTIVE);
 	if (mReq->zptr) {
-		mReq->ptr->next    = mReq->zdma;
+		mReq->ptr->next    = cpu_to_le32(mReq->zdma);
 	} else {
-		mReq->ptr->next    = TD_TERMINATE;
+		mReq->ptr->next    = cpu_to_le32(TD_TERMINATE);
 		if (!mReq->req.no_interrupt)
-			mReq->ptr->token  |= TD_IOC;
+			mReq->ptr->token  |= cpu_to_le32(TD_IOC);
+	}
+	mReq->ptr->page[0]  = cpu_to_le32(mReq->req.dma);
+	for (i = 1; i < 5; i++) {
+		u32 page = mReq->req.dma + i * CI13XXX_PAGE_SIZE;
+		page &= ~TD_RESERVED_MASK;
+		mReq->ptr->page[i] = cpu_to_le32(page);
 	}
-	mReq->ptr->page[0]  = mReq->req.dma;
-	for (i = 1; i < 5; i++)
-		mReq->ptr->page[i] =
-			(mReq->req.dma + i * CI13XXX_PAGE_SIZE) & ~TD_RESERVED_MASK;
 
 	if (!list_empty(&mEp->qh.queue)) {
 		struct ci13xxx_req *mReqPrev;
 		int n = hw_ep_bit(mEp->num, mEp->dir);
 		int tmp_stat;
+		u32 next = mReq->dma & TD_ADDR_MASK;
 
 		mReqPrev = list_entry(mEp->qh.queue.prev,
 				struct ci13xxx_req, queue);
 		if (mReqPrev->zptr)
-			mReqPrev->zptr->next = mReq->dma & TD_ADDR_MASK;
+			mReqPrev->zptr->next = cpu_to_le32(next);
 		else
-			mReqPrev->ptr->next = mReq->dma & TD_ADDR_MASK;
+			mReqPrev->ptr->next = cpu_to_le32(next);
 		wmb();
 		if (hw_read(ci, OP_ENDPTPRIME, BIT(n)))
 			goto done;
@@ -485,9 +488,9 @@ static int _hardware_enqueue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq)
 	}
 
 	/*  QH configuration */
-	mEp->qh.ptr->td.next   = mReq->dma;    /* TERMINATE = 0 */
-	mEp->qh.ptr->td.token &= ~TD_STATUS;   /* clear status */
-	mEp->qh.ptr->cap |=  QH_ZLT;
+	mEp->qh.ptr->td.next   = cpu_to_le32(mReq->dma);    /* TERMINATE = 0 */
+	mEp->qh.ptr->td.token &= cpu_to_le32(~TD_STATUS);   /* clear status */
+	mEp->qh.ptr->cap |=  cpu_to_le32(QH_ZLT);
 
 	wmb();   /* synchronize before ep prime */
 
@@ -509,11 +512,11 @@ static int _hardware_dequeue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq)
 	if (mReq->req.status != -EALREADY)
 		return -EINVAL;
 
-	if ((TD_STATUS_ACTIVE & mReq->ptr->token) != 0)
+	if ((cpu_to_le32(TD_STATUS_ACTIVE) & mReq->ptr->token) != 0)
 		return -EBUSY;
 
 	if (mReq->zptr) {
-		if ((TD_STATUS_ACTIVE & mReq->zptr->token) != 0)
+		if ((cpu_to_le32(TD_STATUS_ACTIVE) & mReq->zptr->token) != 0)
 			return -EBUSY;
 		dma_pool_free(mEp->td_pool, mReq->zptr, mReq->zdma);
 		mReq->zptr = NULL;
@@ -523,7 +526,7 @@ static int _hardware_dequeue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq)
 
 	usb_gadget_unmap_request(&mEp->ci->gadget, &mReq->req, mEp->dir);
 
-	mReq->req.status = mReq->ptr->token & TD_STATUS;
+	mReq->req.status = le32_to_cpu(mReq->ptr->token) & TD_STATUS;
 	if ((TD_STATUS_HALTED & mReq->req.status) != 0)
 		mReq->req.status = -1;
 	else if ((TD_STATUS_DT_ERR & mReq->req.status) != 0)
@@ -531,7 +534,7 @@ static int _hardware_dequeue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq)
 	else if ((TD_STATUS_TR_ERR & mReq->req.status) != 0)
 		mReq->req.status = -1;
 
-	mReq->req.actual   = mReq->ptr->token & TD_TOTAL_BYTES;
+	mReq->req.actual   = le32_to_cpu(mReq->ptr->token) & TD_TOTAL_BYTES;
 	mReq->req.actual >>= ffs_nr(TD_TOTAL_BYTES);
 	mReq->req.actual   = mReq->req.length - mReq->req.actual;
 	mReq->req.actual   = mReq->req.status ? 0 : mReq->req.actual;
@@ -801,7 +804,7 @@ __acquires(mEp->lock)
 		if (retval < 0)
 			break;
 		list_del_init(&mReq->queue);
-		dbg_done(_usb_addr(mEp), mReq->ptr->token, retval);
+		dbg_done(_usb_addr(mEp), le32_to_cpu(mReq->ptr->token), retval);
 		if (mReq->req.complete != NULL) {
 			spin_unlock(mEp->lock);
 			if ((mEp->type == USB_ENDPOINT_XFER_CONTROL) &&
@@ -1045,15 +1048,15 @@ static int ep_enable(struct usb_ep *ep,
 	mEp->qh.ptr->cap = 0;
 
 	if (mEp->type == USB_ENDPOINT_XFER_CONTROL)
-		mEp->qh.ptr->cap |=  QH_IOS;
+		mEp->qh.ptr->cap |=  cpu_to_le32(QH_IOS);
 	else if (mEp->type == USB_ENDPOINT_XFER_ISOC)
-		mEp->qh.ptr->cap &= ~QH_MULT;
+		mEp->qh.ptr->cap &= cpu_to_le32(~QH_MULT);
 	else
-		mEp->qh.ptr->cap &= ~QH_ZLT;
+		mEp->qh.ptr->cap &= cpu_to_le32(~QH_ZLT);
 
 	mEp->qh.ptr->cap |=
-		(mEp->ep.maxpacket << ffs_nr(QH_MAX_PKT)) & QH_MAX_PKT;
-	mEp->qh.ptr->td.next |= TD_TERMINATE;   /* needed? */
+		cpu_to_le32((mEp->ep.maxpacket << ffs_nr(QH_MAX_PKT)) & QH_MAX_PKT);
+	mEp->qh.ptr->td.next |= cpu_to_le32(TD_TERMINATE);   /* needed? */
 
 	/*
 	 * Enable endpoints in the HW other than ep0 as ep0
-- 
1.7.9.5

[PATCH 2/5] usb: chipidea: flags to force usb mode (host/device)

[Svetoslav Neykov]

The chipidea controller in the AR933x SOC supports both host and device modes but not OTG.
Which USB mode is used depends on a pin state (GIPO13) during boot - HIGH for host, LOW for device mode.
Currently if both host and device modes are available, the code assumes OTG support. Add flags to allow
the platform code for force a specific mode based on the pin state.

Signed-off-by: Svetoslav Neykov <svetoslav@neykov.name>
---
 drivers/usb/chipidea/core.c  |   22 +++++++++++++++++-----
 include/linux/usb/chipidea.h |    2 ++
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 3cefb4c..85c72e5 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -398,6 +398,8 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 	struct resource	*res;
 	void __iomem	*base;
 	int		ret;
+	bool force_host_mode;
+	bool force_device_mode;
 
 	if (!dev->platform_data) {
 		dev_err(dev, "platform data missing\n");
@@ -459,21 +461,31 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 	if (ret)
 		dev_info(dev, "doesn't support gadget\n");
 
-	if (!ci->roles[CI_ROLE_HOST] && !ci->roles[CI_ROLE_GADGET]) {
+	force_host_mode = ci->platdata->flags & CI13XXX_FORCE_HOST_MODE;
+	force_device_mode = ci->platdata->flags & CI13XXX_FORCE_DEVICE_MODE;
+	if ((!ci->roles[CI_ROLE_HOST] && !ci->roles[CI_ROLE_GADGET]) ||
+			(force_host_mode && !ci->roles[CI_ROLE_HOST]) ||
+			(force_device_mode && !ci->roles[CI_ROLE_GADGET])) {
 		dev_err(dev, "no supported roles\n");
 		ret = -ENODEV;
 		goto rm_wq;
 	}
 
-	if (ci->roles[CI_ROLE_HOST] && ci->roles[CI_ROLE_GADGET]) {
+	if (!force_host_mode && !force_device_mode &&
+			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]
-			? CI_ROLE_HOST
-			: CI_ROLE_GADGET;
+		if (force_host_mode)
+			ci->role = CI_ROLE_HOST;
+		else if (force_device_mode)
+			ci->role = CI_ROLE_GADGET;
+		else
+			ci->role = ci->roles[CI_ROLE_HOST]
+				? CI_ROLE_HOST
+				: CI_ROLE_GADGET;
 	}
 
 	ret = ci_role_start(ci, ci->role);
diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
index 544825d..e6f44d2 100644
--- a/include/linux/usb/chipidea.h
+++ b/include/linux/usb/chipidea.h
@@ -19,6 +19,8 @@ struct ci13xxx_platform_data {
 #define CI13XXX_REQUIRE_TRANSCEIVER	BIT(1)
 #define CI13XXX_PULLUP_ON_VBUS		BIT(2)
 #define CI13XXX_DISABLE_STREAMING	BIT(3)
+#define CI13XXX_FORCE_HOST_MODE		BIT(5)
+#define CI13XXX_FORCE_DEVICE_MODE	BIT(6)
 
 #define CI13XXX_CONTROLLER_RESET_EVENT		0
 #define CI13XXX_CONTROLLER_STOPPED_EVENT	1
-- 
1.7.9.5

Re: [PATCH 2/5] usb: chipidea: flags to force usb mode (host/device)

[Michael Grzeschik]

Hi Svetoslav,

On Wed, Feb 13, 2013 at 11:38:55PM +0200, Svetoslav Neykov wrote:
> The chipidea controller in the AR933x SOC supports both host and device modes but not OTG.
> Which USB mode is used depends on a pin state (GIPO13) during boot - HIGH for host, LOW for device mode.
> Currently if both host and device modes are available, the code assumes OTG support. Add flags to allow
> the platform code for force a specific mode based on the pin state.
> 
> Signed-off-by: Svetoslav Neykov <svetoslav@neykov.name>
> ---
>  drivers/usb/chipidea/core.c  |   22 +++++++++++++++++-----
>  include/linux/usb/chipidea.h |    2 ++
>  2 files changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 3cefb4c..85c72e5 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -398,6 +398,8 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>  	struct resource	*res;
>  	void __iomem	*base;
>  	int		ret;
> +	bool force_host_mode;
> +	bool force_device_mode;
>  
>  	if (!dev->platform_data) {
>  		dev_err(dev, "platform data missing\n");
> @@ -459,21 +461,31 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>  	if (ret)
>  		dev_info(dev, "doesn't support gadget\n");
>  
> -	if (!ci->roles[CI_ROLE_HOST] && !ci->roles[CI_ROLE_GADGET]) {
> +	force_host_mode = ci->platdata->flags & CI13XXX_FORCE_HOST_MODE;
> +	force_device_mode = ci->platdata->flags & CI13XXX_FORCE_DEVICE_MODE;
> +	if ((!ci->roles[CI_ROLE_HOST] && !ci->roles[CI_ROLE_GADGET]) ||
> +			(force_host_mode && !ci->roles[CI_ROLE_HOST]) ||
> +			(force_device_mode && !ci->roles[CI_ROLE_GADGET])) {
>  		dev_err(dev, "no supported roles\n");
>  		ret = -ENODEV;
>  		goto rm_wq;
>  	}
>  
> -	if (ci->roles[CI_ROLE_HOST] && ci->roles[CI_ROLE_GADGET]) {
> +	if (!force_host_mode && !force_device_mode &&
> +			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]
> -			? CI_ROLE_HOST
> -			: CI_ROLE_GADGET;
> +		if (force_host_mode)
> +			ci->role = CI_ROLE_HOST;
> +		else if (force_device_mode)
> +			ci->role = CI_ROLE_GADGET;
> +		else
> +			ci->role = ci->roles[CI_ROLE_HOST]
> +				? CI_ROLE_HOST
> +				: CI_ROLE_GADGET;
>  	}
>  
>  	ret = ci_role_start(ci, ci->role);
> diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
> index 544825d..e6f44d2 100644
> --- a/include/linux/usb/chipidea.h
> +++ b/include/linux/usb/chipidea.h
> @@ -19,6 +19,8 @@ struct ci13xxx_platform_data {
>  #define CI13XXX_REQUIRE_TRANSCEIVER	BIT(1)
>  #define CI13XXX_PULLUP_ON_VBUS		BIT(2)
>  #define CI13XXX_DISABLE_STREAMING	BIT(3)
> +#define CI13XXX_FORCE_HOST_MODE		BIT(5)
> +#define CI13XXX_FORCE_DEVICE_MODE	BIT(6)
>  
>  #define CI13XXX_CONTROLLER_RESET_EVENT		0
>  #define CI13XXX_CONTROLLER_STOPPED_EVENT	1

We already discuss such functionality:

https://patchwork-mail1.kernel.org/patch/2092051/

Regards,
Michael

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

[PATCH 3/5] usb: chipidea: Don't access OTG related registers

[Svetoslav Neykov]

According to the datasheet the chipidea controller in AR933x doesn't expose OTG and TEST registers.
If no OTG support is detected don't call functions which access those registers.

Signed-off-by: Svetoslav Neykov <svetoslav@neykov.name>
---
 drivers/usb/chipidea/udc.c |   24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 78ac5e5..9fda4d8 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1395,7 +1395,10 @@ 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);
+
+			if (ci->is_otg)
+				hw_enable_vbus_intr(ci);
+
 			hw_device_state(ci, ci->ep0out->qh.dma);
 		} else {
 			hw_device_state(ci, 0);
@@ -1572,7 +1575,8 @@ static int ci13xxx_start(struct usb_gadget *gadget,
 		if (ci->vbus_active) {
 			if (ci->platdata->flags & CI13XXX_REGS_SHARED) {
 				hw_device_reset(ci, USBMODE_CM_DC);
-				hw_enable_vbus_intr(ci);
+				if (ci->is_otg)
+					hw_enable_vbus_intr(ci);
 			}
 		} else {
 			pm_runtime_put_sync(&ci->gadget.dev);
@@ -1680,11 +1684,13 @@ static irqreturn_t udc_irq(struct ci13xxx *ci)
 		retval = IRQ_NONE;
 	}
 
-	intr = hw_read(ci, OP_OTGSC, ~0);
-	hw_write(ci, OP_OTGSC, ~0, intr);
+	if (ci->is_otg) {
+		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);
+		if (intr & (OTGSC_AVVIE & OTGSC_AVVIS))
+			queue_work(ci->wq, &ci->vbus_work);
+	}
 
 	spin_unlock(&ci->lock);
 
@@ -1761,7 +1767,8 @@ 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);
+		if (ci->is_otg)
+			hw_enable_vbus_intr(ci);
 	}
 
 	retval = device_register(&ci->gadget.dev);
@@ -1824,7 +1831,8 @@ static void udc_stop(struct ci13xxx *ci)
 	if (ci == NULL)
 		return;
 
-	hw_disable_vbus_intr(ci);
+	if (ci->is_otg)
+		hw_disable_vbus_intr(ci);
 	cancel_work_sync(&ci->vbus_work);
 
 	usb_del_gadget_udc(&ci->gadget);
-- 
1.7.9.5

Re: [PATCH 3/5] usb: chipidea: Don't access OTG related registers

[Alexander Shishkin]

Svetoslav Neykov <svetoslav@neykov.name> writes:

> According to the datasheet the chipidea controller in AR933x doesn't expose OTG and TEST registers.
> If no OTG support is detected don't call functions which access those registers.
>
> Signed-off-by: Svetoslav Neykov <svetoslav@neykov.name>
> ---
>  drivers/usb/chipidea/udc.c |   24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index 78ac5e5..9fda4d8 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -1395,7 +1395,10 @@ 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);
> +
> +			if (ci->is_otg)
> +				hw_enable_vbus_intr(ci);

It might be easier on the eyes if you move the "if (ci->is_otg)" check
inside hw_enable_vbus_intr(). But otherwise looks sensible.

Regards,
--
Alex

Re: [PATCH 3/5] usb: chipidea: Don't access OTG related registers

[Alexander Shishkin]

Svetoslav Neykov <svetoslav@neykov.name> writes:

> According to the datasheet the chipidea controller in AR933x doesn't expose OTG and TEST registers.
> If no OTG support is detected don't call functions which access those registers.
>
> Signed-off-by: Svetoslav Neykov <svetoslav@neykov.name>
> ---
>  drivers/usb/chipidea/udc.c |   24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index 78ac5e5..9fda4d8 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -1395,7 +1395,10 @@ 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);
> +
> +			if (ci->is_otg)
> +				hw_enable_vbus_intr(ci);

It might be easier on the eyes if you move the "if (ci->is_otg)" check
inside hw_enable_vbus_intr(). But otherwise looks sensible.

Regards,
--
Alex

[PATCH 4/5] usb: chipidea: AR933x platform support for the chipidea driver

[Svetoslav Neykov]

Support host and device usb modes for the chipidea controller in AR933x.
The controller doesn't support OTG functionality so the platform code
forces one of the modes based on the state of GPIO13 pin at startup.

Signed-off-by: Svetoslav Neykov <svetoslav@neykov.name>
---
 arch/mips/ath79/dev-usb.c                      |   19 ++++++
 arch/mips/include/asm/mach-ath79/ar71xx_regs.h |    3 +
 drivers/usb/chipidea/Makefile                  |    1 +
 drivers/usb/chipidea/ci13xxx_ar933x.c          |   73 ++++++++++++++++++++++++
 4 files changed, 96 insertions(+)
 create mode 100644 drivers/usb/chipidea/ci13xxx_ar933x.c

diff --git a/arch/mips/ath79/dev-usb.c b/arch/mips/ath79/dev-usb.c
index bd2bc10..52966b3 100644
--- a/arch/mips/ath79/dev-usb.c
+++ b/arch/mips/ath79/dev-usb.c
@@ -174,8 +174,27 @@ static void __init ar913x_usb_setup(void)
 	platform_device_register(&ath79_ehci_device);
 }
 
+static void __init ar933x_usb_setup_ctrl_config(void)
+{
+	void __iomem *usb_ctrl_base, *usb_config_reg;
+	u32 usb_config;
+
+	usb_ctrl_base = ioremap(AR71XX_USB_CTRL_BASE, AR71XX_USB_CTRL_SIZE);
+	usb_config_reg = usb_ctrl_base + AR71XX_USB_CTRL_REG_CONFIG;
+	usb_config = __raw_readl(usb_config_reg);
+	usb_config &= ~AR933X_USB_CONFIG_HOST_ONLY;
+	__raw_writel(usb_config, usb_config_reg);
+	iounmap(usb_ctrl_base);
+}
+
 static void __init ar933x_usb_setup(void)
 {
+	u32 bootstrap;
+
+	bootstrap = ath79_reset_rr(AR933X_RESET_REG_BOOTSTRAP);
+	if (!(bootstrap & AR933X_BOOTSTRAP_USB_MODE_HOST))
+		ar933x_usb_setup_ctrl_config();
+
 	ath79_device_reset_set(AR933X_RESET_USBSUS_OVERRIDE);
 	mdelay(10);
 
diff --git a/arch/mips/include/asm/mach-ath79/ar71xx_regs.h b/arch/mips/include/asm/mach-ath79/ar71xx_regs.h
index a5e0f17..13eb2d9 100644
--- a/arch/mips/include/asm/mach-ath79/ar71xx_regs.h
+++ b/arch/mips/include/asm/mach-ath79/ar71xx_regs.h
@@ -297,6 +297,7 @@
 #define AR934X_RESET_USB_PHY		BIT(4)
 #define AR934X_RESET_USBSUS_OVERRIDE	BIT(3)
 
+#define AR933X_BOOTSTRAP_USB_MODE_HOST	BIT(3)
 #define AR933X_BOOTSTRAP_REF_CLK_40	BIT(0)
 
 #define AR934X_BOOTSTRAP_SW_OPTION8	BIT(23)
@@ -315,6 +316,8 @@
 #define AR934X_BOOTSTRAP_SDRAM_DISABLED	BIT(1)
 #define AR934X_BOOTSTRAP_DDR1		BIT(0)
 
+#define AR933X_USB_CONFIG_HOST_ONLY	BIT(8)
+
 #define AR934X_PCIE_WMAC_INT_WMAC_MISC		BIT(0)
 #define AR934X_PCIE_WMAC_INT_WMAC_TX		BIT(1)
 #define AR934X_PCIE_WMAC_INT_WMAC_RXLP		BIT(2)
diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile
index d92ca32..196b7b4 100644
--- a/drivers/usb/chipidea/Makefile
+++ b/drivers/usb/chipidea/Makefile
@@ -10,6 +10,7 @@ ci_hdrc-$(CONFIG_USB_CHIPIDEA_DEBUG)	+= debug.o
 # Glue/Bridge layers go here
 
 obj-$(CONFIG_USB_CHIPIDEA)	+= ci13xxx_msm.o
+obj-$(CONFIG_USB_CHIPIDEA)	+= ci13xxx_ar933x.o
 
 # PCI doesn't provide stubs, need to check
 ifneq ($(CONFIG_PCI),)
diff --git a/drivers/usb/chipidea/ci13xxx_ar933x.c b/drivers/usb/chipidea/ci13xxx_ar933x.c
new file mode 100644
index 0000000..046a4b6
--- /dev/null
+++ b/drivers/usb/chipidea/ci13xxx_ar933x.c
@@ -0,0 +1,73 @@
+/* Copyright (c) 2010, Code Aurora Forum. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/usb/ulpi.h>
+#include <linux/usb/gadget.h>
+#include <linux/usb/chipidea.h>
+#include <asm/mach-ath79/ath79.h>
+#include <asm/mach-ath79/ar71xx_regs.h>
+
+#include "ci.h"
+
+static struct ci13xxx_platform_data ci13xxx_ar933x_platdata = {
+	.name			= "ci13xxx_ar933x",
+	.flags			= 0,
+	.capoffset		= DEF_CAPOFFSET
+};
+
+static int __devinit ci13xxx_ar933x_probe(struct platform_device *pdev)
+{
+	u32 bootstrap;
+	struct platform_device *plat_ci;
+
+	dev_dbg(&pdev->dev, "ci13xxx_ar933x_probe\n");
+
+	bootstrap = ath79_reset_rr(AR933X_RESET_REG_BOOTSTRAP);
+	if (bootstrap & AR933X_BOOTSTRAP_USB_MODE_HOST)
+		ci13xxx_ar933x_platdata.flags = CI13XXX_FORCE_HOST_MODE;
+	else
+		ci13xxx_ar933x_platdata.flags = CI13XXX_FORCE_DEVICE_MODE;
+
+	plat_ci = ci13xxx_add_device(&pdev->dev,
+				pdev->resource, pdev->num_resources,
+				&ci13xxx_ar933x_platdata);
+	if (IS_ERR(plat_ci)) {
+		dev_err(&pdev->dev, "ci13xxx_add_device failed!\n");
+		return PTR_ERR(plat_ci);
+	}
+
+	platform_set_drvdata(pdev, plat_ci);
+
+	pm_runtime_no_callbacks(&pdev->dev);
+	pm_runtime_enable(&pdev->dev);
+
+	return 0;
+}
+
+static int __devexit ci13xxx_ar933x_remove(struct platform_device *pdev)
+{
+	struct platform_device *plat_ci = platform_get_drvdata(pdev);
+
+	pm_runtime_disable(&pdev->dev);
+	ci13xxx_remove_device(plat_ci);
+
+	return 0;
+}
+
+static struct platform_driver ci13xxx_ar933x_driver = {
+	.probe = ci13xxx_ar933x_probe,
+	.remove = __devexit_p(ci13xxx_ar933x_remove),
+	.driver = { .name = "ehci-platform", },
+};
+
+module_platform_driver(ci13xxx_ar933x_driver);
+
+MODULE_ALIAS("platform:ar933x_hsusb");
+MODULE_LICENSE("GPL v2");
-- 
1.7.9.5

Re: [PATCH 4/5] usb: chipidea: AR933x platform support for the chipidea driver

[Alexander Shishkin]

Svetoslav Neykov <svetoslav@neykov.name> writes:

> Support host and device usb modes for the chipidea controller in AR933x.
> The controller doesn't support OTG functionality so the platform code
> forces one of the modes based on the state of GPIO13 pin at startup.

Some comments below.

>
> Signed-off-by: Svetoslav Neykov <svetoslav@neykov.name>
> ---
>  arch/mips/ath79/dev-usb.c                      |   19 ++++++
>  arch/mips/include/asm/mach-ath79/ar71xx_regs.h |    3 +
>  drivers/usb/chipidea/Makefile                  |    1 +
>  drivers/usb/chipidea/ci13xxx_ar933x.c          |   73 ++++++++++++++++++++++++
>  4 files changed, 96 insertions(+)
>  create mode 100644 drivers/usb/chipidea/ci13xxx_ar933x.c
>
> diff --git a/arch/mips/ath79/dev-usb.c b/arch/mips/ath79/dev-usb.c
> index bd2bc10..52966b3 100644
> --- a/arch/mips/ath79/dev-usb.c
> +++ b/arch/mips/ath79/dev-usb.c
> @@ -174,8 +174,27 @@ static void __init ar913x_usb_setup(void)
>  	platform_device_register(&ath79_ehci_device);
>  }
>  
> +static void __init ar933x_usb_setup_ctrl_config(void)
> +{
> +	void __iomem *usb_ctrl_base, *usb_config_reg;
> +	u32 usb_config;
> +
> +	usb_ctrl_base = ioremap(AR71XX_USB_CTRL_BASE, AR71XX_USB_CTRL_SIZE);
> +	usb_config_reg = usb_ctrl_base + AR71XX_USB_CTRL_REG_CONFIG;
> +	usb_config = __raw_readl(usb_config_reg);
> +	usb_config &= ~AR933X_USB_CONFIG_HOST_ONLY;
> +	__raw_writel(usb_config, usb_config_reg);
> +	iounmap(usb_ctrl_base);
> +}
> +
>  static void __init ar933x_usb_setup(void)
>  {
> +	u32 bootstrap;
> +
> +	bootstrap = ath79_reset_rr(AR933X_RESET_REG_BOOTSTRAP);
> +	if (!(bootstrap & AR933X_BOOTSTRAP_USB_MODE_HOST))
> +		ar933x_usb_setup_ctrl_config();
> +
>  	ath79_device_reset_set(AR933X_RESET_USBSUS_OVERRIDE);
>  	mdelay(10);
>  
> diff --git a/arch/mips/include/asm/mach-ath79/ar71xx_regs.h b/arch/mips/include/asm/mach-ath79/ar71xx_regs.h
> index a5e0f17..13eb2d9 100644
> --- a/arch/mips/include/asm/mach-ath79/ar71xx_regs.h
> +++ b/arch/mips/include/asm/mach-ath79/ar71xx_regs.h
> @@ -297,6 +297,7 @@
>  #define AR934X_RESET_USB_PHY		BIT(4)
>  #define AR934X_RESET_USBSUS_OVERRIDE	BIT(3)
>  
> +#define AR933X_BOOTSTRAP_USB_MODE_HOST	BIT(3)
>  #define AR933X_BOOTSTRAP_REF_CLK_40	BIT(0)
>  
>  #define AR934X_BOOTSTRAP_SW_OPTION8	BIT(23)
> @@ -315,6 +316,8 @@
>  #define AR934X_BOOTSTRAP_SDRAM_DISABLED	BIT(1)
>  #define AR934X_BOOTSTRAP_DDR1		BIT(0)
>  
> +#define AR933X_USB_CONFIG_HOST_ONLY	BIT(8)
> +
>  #define AR934X_PCIE_WMAC_INT_WMAC_MISC		BIT(0)
>  #define AR934X_PCIE_WMAC_INT_WMAC_TX		BIT(1)
>  #define AR934X_PCIE_WMAC_INT_WMAC_RXLP		BIT(2)
> diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile
> index d92ca32..196b7b4 100644
> --- a/drivers/usb/chipidea/Makefile
> +++ b/drivers/usb/chipidea/Makefile
> @@ -10,6 +10,7 @@ ci_hdrc-$(CONFIG_USB_CHIPIDEA_DEBUG)	+= debug.o
>  # Glue/Bridge layers go here
>  
>  obj-$(CONFIG_USB_CHIPIDEA)	+= ci13xxx_msm.o
> +obj-$(CONFIG_USB_CHIPIDEA)	+= ci13xxx_ar933x.o

The problem is that this will only compile for mips target, so you
should probably change the Makefile to only compile it for mips.

>  
>  # PCI doesn't provide stubs, need to check
>  ifneq ($(CONFIG_PCI),)
> diff --git a/drivers/usb/chipidea/ci13xxx_ar933x.c b/drivers/usb/chipidea/ci13xxx_ar933x.c
> new file mode 100644
> index 0000000..046a4b6
> --- /dev/null
> +++ b/drivers/usb/chipidea/ci13xxx_ar933x.c
> @@ -0,0 +1,73 @@
> +/* Copyright (c) 2010, Code Aurora Forum. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/usb/ulpi.h>
> +#include <linux/usb/gadget.h>
> +#include <linux/usb/chipidea.h>
> +#include <asm/mach-ath79/ath79.h>
> +#include <asm/mach-ath79/ar71xx_regs.h>
> +
> +#include "ci.h"
> +
> +static struct ci13xxx_platform_data ci13xxx_ar933x_platdata = {
> +	.name			= "ci13xxx_ar933x",
> +	.flags			= 0,
> +	.capoffset		= DEF_CAPOFFSET
> +};
> +
> +static int __devinit ci13xxx_ar933x_probe(struct platform_device *pdev)

__devinit/__devexit/__devexit_p() are gone from the kernel.

Regards,
--
Alex

Re: [PATCH 4/5] usb: chipidea: AR933x platform support for the chipidea driver

[Alexander Shishkin]

Svetoslav Neykov <svetoslav@neykov.name> writes:

> Support host and device usb modes for the chipidea controller in AR933x.
> The controller doesn't support OTG functionality so the platform code
> forces one of the modes based on the state of GPIO13 pin at startup.

Some comments below.

>
> Signed-off-by: Svetoslav Neykov <svetoslav@neykov.name>
> ---
>  arch/mips/ath79/dev-usb.c                      |   19 ++++++
>  arch/mips/include/asm/mach-ath79/ar71xx_regs.h |    3 +
>  drivers/usb/chipidea/Makefile                  |    1 +
>  drivers/usb/chipidea/ci13xxx_ar933x.c          |   73 ++++++++++++++++++++++++
>  4 files changed, 96 insertions(+)
>  create mode 100644 drivers/usb/chipidea/ci13xxx_ar933x.c
>
> diff --git a/arch/mips/ath79/dev-usb.c b/arch/mips/ath79/dev-usb.c
> index bd2bc10..52966b3 100644
> --- a/arch/mips/ath79/dev-usb.c
> +++ b/arch/mips/ath79/dev-usb.c
> @@ -174,8 +174,27 @@ static void __init ar913x_usb_setup(void)
>  	platform_device_register(&ath79_ehci_device);
>  }
>  
> +static void __init ar933x_usb_setup_ctrl_config(void)
> +{
> +	void __iomem *usb_ctrl_base, *usb_config_reg;
> +	u32 usb_config;
> +
> +	usb_ctrl_base = ioremap(AR71XX_USB_CTRL_BASE, AR71XX_USB_CTRL_SIZE);
> +	usb_config_reg = usb_ctrl_base + AR71XX_USB_CTRL_REG_CONFIG;
> +	usb_config = __raw_readl(usb_config_reg);
> +	usb_config &= ~AR933X_USB_CONFIG_HOST_ONLY;
> +	__raw_writel(usb_config, usb_config_reg);
> +	iounmap(usb_ctrl_base);
> +}
> +
>  static void __init ar933x_usb_setup(void)
>  {
> +	u32 bootstrap;
> +
> +	bootstrap = ath79_reset_rr(AR933X_RESET_REG_BOOTSTRAP);
> +	if (!(bootstrap & AR933X_BOOTSTRAP_USB_MODE_HOST))
> +		ar933x_usb_setup_ctrl_config();
> +
>  	ath79_device_reset_set(AR933X_RESET_USBSUS_OVERRIDE);
>  	mdelay(10);
>  
> diff --git a/arch/mips/include/asm/mach-ath79/ar71xx_regs.h b/arch/mips/include/asm/mach-ath79/ar71xx_regs.h
> index a5e0f17..13eb2d9 100644
> --- a/arch/mips/include/asm/mach-ath79/ar71xx_regs.h
> +++ b/arch/mips/include/asm/mach-ath79/ar71xx_regs.h
> @@ -297,6 +297,7 @@
>  #define AR934X_RESET_USB_PHY		BIT(4)
>  #define AR934X_RESET_USBSUS_OVERRIDE	BIT(3)
>  
> +#define AR933X_BOOTSTRAP_USB_MODE_HOST	BIT(3)
>  #define AR933X_BOOTSTRAP_REF_CLK_40	BIT(0)
>  
>  #define AR934X_BOOTSTRAP_SW_OPTION8	BIT(23)
> @@ -315,6 +316,8 @@
>  #define AR934X_BOOTSTRAP_SDRAM_DISABLED	BIT(1)
>  #define AR934X_BOOTSTRAP_DDR1		BIT(0)
>  
> +#define AR933X_USB_CONFIG_HOST_ONLY	BIT(8)
> +
>  #define AR934X_PCIE_WMAC_INT_WMAC_MISC		BIT(0)
>  #define AR934X_PCIE_WMAC_INT_WMAC_TX		BIT(1)
>  #define AR934X_PCIE_WMAC_INT_WMAC_RXLP		BIT(2)
> diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile
> index d92ca32..196b7b4 100644
> --- a/drivers/usb/chipidea/Makefile
> +++ b/drivers/usb/chipidea/Makefile
> @@ -10,6 +10,7 @@ ci_hdrc-$(CONFIG_USB_CHIPIDEA_DEBUG)	+= debug.o
>  # Glue/Bridge layers go here
>  
>  obj-$(CONFIG_USB_CHIPIDEA)	+= ci13xxx_msm.o
> +obj-$(CONFIG_USB_CHIPIDEA)	+= ci13xxx_ar933x.o

The problem is that this will only compile for mips target, so you
should probably change the Makefile to only compile it for mips.

>  
>  # PCI doesn't provide stubs, need to check
>  ifneq ($(CONFIG_PCI),)
> diff --git a/drivers/usb/chipidea/ci13xxx_ar933x.c b/drivers/usb/chipidea/ci13xxx_ar933x.c
> new file mode 100644
> index 0000000..046a4b6
> --- /dev/null
> +++ b/drivers/usb/chipidea/ci13xxx_ar933x.c
> @@ -0,0 +1,73 @@
> +/* Copyright (c) 2010, Code Aurora Forum. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/usb/ulpi.h>
> +#include <linux/usb/gadget.h>
> +#include <linux/usb/chipidea.h>
> +#include <asm/mach-ath79/ath79.h>
> +#include <asm/mach-ath79/ar71xx_regs.h>
> +
> +#include "ci.h"
> +
> +static struct ci13xxx_platform_data ci13xxx_ar933x_platdata = {
> +	.name			= "ci13xxx_ar933x",
> +	.flags			= 0,
> +	.capoffset		= DEF_CAPOFFSET
> +};
> +
> +static int __devinit ci13xxx_ar933x_probe(struct platform_device *pdev)

__devinit/__devexit/__devexit_p() are gone from the kernel.

Regards,
--
Alex

Re: [PATCH 4/5] usb: chipidea: AR933x platform support for the chipidea driver

[Gabor Juhos]

Hi Svetoslav,

> Support host and device usb modes for the chipidea controller in AR933x.
> The controller doesn't support OTG functionality so the platform code
> forces one of the modes based on the state of GPIO13 pin at startup.
> 
> Signed-off-by: Svetoslav Neykov <svetoslav@neykov.name>
> ---

<...>

> diff --git a/drivers/usb/chipidea/ci13xxx_ar933x.c b/drivers/usb/chipidea/ci13xxx_ar933x.c
> new file mode 100644
> index 0000000..046a4b6
> --- /dev/null
> +++ b/drivers/usb/chipidea/ci13xxx_ar933x.c
> @@ -0,0 +1,73 @@
> +/* Copyright (c) 2010, Code Aurora Forum. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/usb/ulpi.h>
> +#include <linux/usb/gadget.h>
> +#include <linux/usb/chipidea.h>
> +#include <asm/mach-ath79/ath79.h>
> +#include <asm/mach-ath79/ar71xx_regs.h>
> +
> +#include "ci.h"
> +
> +static struct ci13xxx_platform_data ci13xxx_ar933x_platdata = {
> +	.name			= "ci13xxx_ar933x",
> +	.flags			= 0,
> +	.capoffset		= DEF_CAPOFFSET
> +};

Static data only works if there are only one USB IP in the SoCs. It is true for
the AR9330 SoC, but newer SoCs may have more. Please use a dynamically allocated
structure and fill that in the probe routine.

> +
> +static int __devinit ci13xxx_ar933x_probe(struct platform_device *pdev)
> +{
> +	u32 bootstrap;
> +	struct platform_device *plat_ci;
> +
> +	dev_dbg(&pdev->dev, "ci13xxx_ar933x_probe\n");
> +
> +	bootstrap = ath79_reset_rr(AR933X_RESET_REG_BOOTSTRAP);
> +	if (bootstrap & AR933X_BOOTSTRAP_USB_MODE_HOST)
> +		ci13xxx_ar933x_platdata.flags = CI13XXX_FORCE_HOST_MODE;
> +	else
> +		ci13xxx_ar933x_platdata.flags = CI13XXX_FORCE_DEVICE_MODE;

This bootstrap setting is only valid for the AR933x SoCs. It would be better to
move this code into the SoC specific USB device registration code, and use
platform data to pass that information to this driver.

> +
> +	plat_ci = ci13xxx_add_device(&pdev->dev,
> +				pdev->resource, pdev->num_resources,
> +				&ci13xxx_ar933x_platdata);
> +	if (IS_ERR(plat_ci)) {
> +		dev_err(&pdev->dev, "ci13xxx_add_device failed!\n");
> +		return PTR_ERR(plat_ci);
> +	}
> +
> +	platform_set_drvdata(pdev, plat_ci);
> +
> +	pm_runtime_no_callbacks(&pdev->dev);
> +	pm_runtime_enable(&pdev->dev);
> +
> +	return 0;
> +}
> +
> +static int __devexit ci13xxx_ar933x_remove(struct platform_device *pdev)
> +{
> +	struct platform_device *plat_ci = platform_get_drvdata(pdev);
> +
> +	pm_runtime_disable(&pdev->dev);
> +	ci13xxx_remove_device(plat_ci);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver ci13xxx_ar933x_driver = {
> +	.probe = ci13xxx_ar933x_probe,
> +	.remove = __devexit_p(ci13xxx_ar933x_remove),
> +	.driver = { .name = "ehci-platform", },

This name is used by the ehci-platform driver. You should pick a different one
for this driver. Additionally, the device registration code in
'arch/mips/ath79/dev-usb.c' must be adjusted as well.

> +};
> +
> +module_platform_driver(ci13xxx_ar933x_driver);
> +
> +MODULE_ALIAS("platform:ar933x_hsusb");

The driver part of the MODULE_ALIAS string should match with the driver name.
You should use this instead ci13xxx_ar933x_driver

> +MODULE_LICENSE("GPL v2");

Regards,
Gabor

[PATCH 5/5] usb: chipidea: Fix incorrect check of function return value

[Svetoslav Neykov]

Use the correct variable to check for the return value of the last function.

Signed-off-by: Svetoslav Neykov <svetoslav@neykov.name>
---
 drivers/usb/chipidea/core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 85c72e5..8442305 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -413,7 +413,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 	}
 
 	base = devm_request_and_ioremap(dev, res);
-	if (!res) {
+	if (!base) {
 		dev_err(dev, "can't request and ioremap resource\n");
 		return -ENOMEM;
 	}
-- 
1.7.9.5

Re: [PATCH 5/5] usb: chipidea: Fix incorrect check of function return value

[Alexander Shishkin]

Svetoslav Neykov <svetoslav@neykov.name> writes:

> Use the correct variable to check for the return value of the last function.

This one is fixed already by Julia Lawall: 5c6e9bf0

Regards,
--
Alex

Re: [PATCH 5/5] usb: chipidea: Fix incorrect check of function return value

[Alexander Shishkin]

Svetoslav Neykov <svetoslav@neykov.name> writes:

> Use the correct variable to check for the return value of the last function.

This one is fixed already by Julia Lawall: 5c6e9bf0

Regards,
--
Alex

Re: [PATCH 0/5] Chipidea driver support for the AR933x platform

[Alexander Shishkin]

Svetoslav Neykov <svetoslav@neykov.name> writes:

> Add support for the usb controller in AR933x platform.
> The processor is big-endian so all multi-byte values of the usb 
> descriptors must be converted explicitly. Another difference is that
> the controller supports both host and device modes but not OTG.
> The patches are tested on WR703n router running OpenWRT trunk.

Svetoslav, are you working on these or do you have time to address the
issues raised in review?

Thanks,
--
Alex

Re: [PATCH 0/5] Chipidea driver support for the AR933x platform

[Alexander Shishkin]

Svetoslav Neykov <svetoslav@neykov.name> writes:

> Add support for the usb controller in AR933x platform.
> The processor is big-endian so all multi-byte values of the usb 
> descriptors must be converted explicitly. Another difference is that
> the controller supports both host and device modes but not OTG.
> The patches are tested on WR703n router running OpenWRT trunk.

Svetoslav, are you working on these or do you have time to address the
issues raised in review?

Thanks,
--
Alex

RE: [PATCH 0/5] Chipidea driver support for the AR933x platform

[Svetoslav Neykov]

Hi Alex,
I am working on the incorporating all received comments - thanks to all for
taking their time to comment.
Apologies for not replying to the received mails, didn't want to spam with
OK replies to each separately.

Regards,
Svetoslav.


-----Original Message-----
From: Alexander Shishkin [mailto:alexander.shishkin@linux.intel.com] 
Sent: Tuesday, February 26, 2013 3:36 PM
To: Svetoslav Neykov; Ralf Baechle; Greg Kroah-Hartman; Gabor Juhos; John
Crispin; Alan Stern; Luis R. Rodriguez
Cc: linux-mips@linux-mips.org; linux-usb@vger.kernel.org; Svetoslav Neykov
Subject: Re: [PATCH 0/5] Chipidea driver support for the AR933x platform

Svetoslav Neykov <svetoslav@neykov.name> writes:

> Add support for the usb controller in AR933x platform.
> The processor is big-endian so all multi-byte values of the usb 
> descriptors must be converted explicitly. Another difference is that
> the controller supports both host and device modes but not OTG.
> The patches are tested on WR703n router running OpenWRT trunk.

Svetoslav, are you working on these or do you have time to address the
issues raised in review?

Thanks,
--
Alex

RE: [PATCH 0/5] Chipidea driver support for the AR933x platform

[Svetoslav Neykov]

Hi Alex,
I am working on the incorporating all received comments - thanks to all for
taking their time to comment.
Apologies for not replying to the received mails, didn't want to spam with
OK replies to each separately.

Regards,
Svetoslav.


-----Original Message-----
From: Alexander Shishkin [mailto:alexander.shishkin@linux.intel.com] 
Sent: Tuesday, February 26, 2013 3:36 PM
To: Svetoslav Neykov; Ralf Baechle; Greg Kroah-Hartman; Gabor Juhos; John
Crispin; Alan Stern; Luis R. Rodriguez
Cc: linux-mips@linux-mips.org; linux-usb@vger.kernel.org; Svetoslav Neykov
Subject: Re: [PATCH 0/5] Chipidea driver support for the AR933x platform

Svetoslav Neykov <svetoslav@neykov.name> writes:

> Add support for the usb controller in AR933x platform.
> The processor is big-endian so all multi-byte values of the usb 
> descriptors must be converted explicitly. Another difference is that
> the controller supports both host and device modes but not OTG.
> The patches are tested on WR703n router running OpenWRT trunk.

Svetoslav, are you working on these or do you have time to address the
issues raised in review?

Thanks,
--
Alex

RE: [PATCH 0/5] Chipidea driver support for the AR933x platform

[Alexander Shishkin]

Svetoslav Neykov <svetoslav@neykov.name> writes:

> Hi Alex,
> I am working on the incorporating all received comments - thanks to all for
> taking their time to comment.
> Apologies for not replying to the received mails, didn't want to spam with
> OK replies to each separately.

Great, thanks!
Looks like this patchset will need some synchronization with Sacha's
dr_mode/phy_mode patchset, but I presume you're aware of that.

Regards,
--
Alex

RE: [PATCH 0/5] Chipidea driver support for the AR933x platform

[Svetoslav Neykov]

>Great, thanks!
>Looks like this patchset will need some synchronization with Sacha's
>dr_mode/phy_mode patchset, but I presume you're aware of that.

Yes, I will base the next patch on the existing changes, thanks for bringing
that up.

Regards,
Svetoslav.

RE: [PATCH 0/5] Chipidea driver support for the AR933x platform

[Svetoslav Neykov]

>Great, thanks!
>Looks like this patchset will need some synchronization with Sacha's
>dr_mode/phy_mode patchset, but I presume you're aware of that.

Yes, I will base the next patch on the existing changes, thanks for bringing
that up.

Regards,
Svetoslav.

Re: [PATCH 0/5] Chipidea driver support for the AR933x platform

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 934 bytes --]

On 02/26/2013 03:47 PM, Alexander Shishkin wrote:
> Svetoslav Neykov <svetoslav@neykov.name> writes:
> 
>> Hi Alex,
>> I am working on the incorporating all received comments - thanks to all for
>> taking their time to comment.
>> Apologies for not replying to the received mails, didn't want to spam with
>> OK replies to each separately.
> 
> Great, thanks!
> Looks like this patchset will need some synchronization with Sacha's
> dr_mode/phy_mode patchset, but I presume you're aware of that.

The current version of Sascha's patches is here:

http://git.pengutronix.de/?p=mgr/linux.git;a=shortlog;h=refs/heads/chipidea-for-v3.10

Marc

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]