[PATCH v4 0/6] Add support for SW babble Control

[PATCH v4 0/6] Add support for SW babble Control

[George Cherian]

Subject: [PATCH v3 0/5] Add support for SW babble Control

Series add support for SW babble control logic found in 
new silicon versions of AM335x. Runtime differentiation of
silicon version is done by checking the BABBLE_CTL register.
For newer silicon the register default value read is 0x4 and
for older versions its 0x0.

Patch 1 -> Handle Babble only if MUSB is in HOST mode
Patch 2 -> Convert recover work to delayed work.
Patch 3 -> usb_phy_vbus_(off/_on) are NOPs for am335x PHY
	   so use usb_phy(_shutdown/_init) in musb_platform_reset()
Patch 4 -> Add return value for musb_platform_reset() in prepration
	   to support SW babble_ctrl
Patch 5 -> Add the sw_babble_control()
Patch 6 -> Enable sw babble control for newer silicon

v3 -> v4 : Fixes an issue in gagdet mode - BUS RESET should not
	   be handled as a BABBLE. Added a check for the same.(Patch #1)
	   Enable sw babble control properly (Patch #6)
	   	
v2 -> v3 : Modify musb_platform_reset() to return zero on success.

George Cherian (6):
  usb: musb: core: Handle Babble condition only in HOST mode
  usb: musb: core: Convert babble recover work to delayed work
  usb: musb: dsps: Call usb_phy(_shutdown/_init) during
    musb_platform_reset()
  usb: musb: core: Convert the musb_platform_reset to have a return
    value.
  usb: musb: dsps: Add the sw_babble_control()
  usb: musb: dsps: Enable sw babble control for newer silicon

 drivers/usb/musb/musb_core.c | 51 ++++++++++++++------------
 drivers/usb/musb/musb_core.h | 12 ++++---
 drivers/usb/musb/musb_dsps.c | 86 ++++++++++++++++++++++++++++++++++++++++++--
 drivers/usb/musb/musb_regs.h |  7 ++++
 4 files changed, 125 insertions(+), 31 deletions(-)

-- 
1.8.3.1

[PATCH v4 1/6] usb: musb: core: Handle Babble condition only in HOST mode

[George Cherian]

BABBLE and RESET share the same interrupt. The interrupt
is considered to be RESET if MUSB is in peripheral mode and
as a BABBLE if MUSB is in HOST mode.

Handle babble condition iff MUSB is in HOST mode.

Signed-off-by: George Cherian <george.cherian@ti.com>
---
 drivers/usb/musb/musb_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 61da471..eff3c5c 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -849,7 +849,7 @@ b_host:
 	}
 
 	/* handle babble condition */
-	if (int_usb & MUSB_INTR_BABBLE)
+	if (int_usb & MUSB_INTR_BABBLE && is_host_active(musb))
 		schedule_work(&musb->recover_work);
 
 #if 0
-- 
1.8.3.1

[PATCH v4 2/6] usb: musb: core: Convert babble recover work to delayed work

[George Cherian]

During babble condition both first disconnect of devices are
initiated. Make sure MUSB controller is reset and re-initialized
after all disconnects.

To acheive this schedule a delayed work for babble rrecovery.

While at that convert udelay to usleep_range.
Refer Documentation/timers/timers-howto.txt

Signed-off-by: George Cherian <george.cherian@ti.com>
---
 drivers/usb/musb/musb_core.c | 15 ++++++++-------
 drivers/usb/musb/musb_core.h |  2 +-
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index eff3c5c..8920b80 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -850,7 +850,8 @@ b_host:
 
 	/* handle babble condition */
 	if (int_usb & MUSB_INTR_BABBLE && is_host_active(musb))
-		schedule_work(&musb->recover_work);
+		schedule_delayed_work(&musb->recover_work,
+				      msecs_to_jiffies(100));
 
 #if 0
 /* REVISIT ... this would be for multiplexing periodic endpoints, or
@@ -1753,16 +1754,16 @@ static void musb_irq_work(struct work_struct *data)
 /* Recover from babble interrupt conditions */
 static void musb_recover_work(struct work_struct *data)
 {
-	struct musb *musb = container_of(data, struct musb, recover_work);
+	struct musb *musb = container_of(data, struct musb, recover_work.work);
 	int status;
 
 	musb_platform_reset(musb);
 
 	usb_phy_vbus_off(musb->xceiv);
-	udelay(100);
+	usleep_range(100, 200);
 
 	usb_phy_vbus_on(musb->xceiv);
-	udelay(100);
+	usleep_range(100, 200);
 
 	/*
 	 * When a babble condition occurs, the musb controller removes the
@@ -1945,7 +1946,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
 
 	/* Init IRQ workqueue before request_irq */
 	INIT_WORK(&musb->irq_work, musb_irq_work);
-	INIT_WORK(&musb->recover_work, musb_recover_work);
+	INIT_DELAYED_WORK(&musb->recover_work, musb_recover_work);
 	INIT_DELAYED_WORK(&musb->deassert_reset_work, musb_deassert_reset);
 	INIT_DELAYED_WORK(&musb->finish_resume_work, musb_host_finish_resume);
 
@@ -2041,7 +2042,7 @@ fail4:
 
 fail3:
 	cancel_work_sync(&musb->irq_work);
-	cancel_work_sync(&musb->recover_work);
+	cancel_delayed_work_sync(&musb->recover_work);
 	cancel_delayed_work_sync(&musb->finish_resume_work);
 	cancel_delayed_work_sync(&musb->deassert_reset_work);
 	if (musb->dma_controller)
@@ -2107,7 +2108,7 @@ static int musb_remove(struct platform_device *pdev)
 		dma_controller_destroy(musb->dma_controller);
 
 	cancel_work_sync(&musb->irq_work);
-	cancel_work_sync(&musb->recover_work);
+	cancel_delayed_work_sync(&musb->recover_work);
 	cancel_delayed_work_sync(&musb->finish_resume_work);
 	cancel_delayed_work_sync(&musb->deassert_reset_work);
 	musb_free(musb);
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index 47e8874..423cd00 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -297,7 +297,7 @@ struct musb {
 
 	irqreturn_t		(*isr)(int, void *);
 	struct work_struct	irq_work;
-	struct work_struct	recover_work;
+	struct delayed_work	recover_work;
 	struct delayed_work	deassert_reset_work;
 	struct delayed_work	finish_resume_work;
 	u16			hwvers;
-- 
1.8.3.1

[PATCH v4 3/6] usb: musb: dsps: Call usb_phy(_shutdown/_init) during musb_platform_reset()

[George Cherian]

For DSPS platform usb_phy_vbus(_off/_on) are NOPs.
So during musb_platform_reset() call usb_phy(_shutdown/_init)

Signed-off-by: George Cherian <george.cherian@ti.com>
---
 drivers/usb/musb/musb_dsps.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 51beb13..74c4193 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -543,7 +543,11 @@ static void dsps_musb_reset(struct musb *musb)
 	const struct dsps_musb_wrapper *wrp = glue->wrp;
 
 	dsps_writel(musb->ctrl_base, wrp->control, (1 << wrp->reset));
-	udelay(100);
+	usleep_range(100, 200);
+	usb_phy_shutdown(musb->xceiv);
+	usleep_range(100, 200);
+	usb_phy_init(musb->xceiv);
+
 }
 
 static struct musb_platform_ops dsps_ops = {
-- 
1.8.3.1

[PATCH v4 4/6] usb: musb: core: Convert the musb_platform_reset to have a return value.

[George Cherian]

Currently musb_platform_reset() is only used by dsps.
In case of BABBLE interrupt for other platforms the  musb_platform_reset()
is a NOP. In such situations no need to re-initialize the endpoints.
Also in the latest silicon revision of AM335x, we do have a babble recovery
mechanism without resetting the IP block. In preperation to add that support
its better to have a rest_done return for  musb_platform_reset().

Signed-off-by: George Cherian <george.cherian@ti.com>
---
 drivers/usb/musb/musb_core.c | 38 +++++++++++++++++++++-----------------
 drivers/usb/musb/musb_core.h | 10 ++++++----
 drivers/usb/musb/musb_dsps.c |  3 ++-
 3 files changed, 29 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 8920b80..1b0b85d 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1755,28 +1755,32 @@ static void musb_irq_work(struct work_struct *data)
 static void musb_recover_work(struct work_struct *data)
 {
 	struct musb *musb = container_of(data, struct musb, recover_work.work);
-	int status;
+	int status, ret;
 
-	musb_platform_reset(musb);
+	ret  = musb_platform_reset(musb);
+	if (ret  < 0)
+		return;
 
-	usb_phy_vbus_off(musb->xceiv);
-	usleep_range(100, 200);
+	if (!ret) {
+		usb_phy_vbus_off(musb->xceiv);
+		usleep_range(100, 200);
 
-	usb_phy_vbus_on(musb->xceiv);
-	usleep_range(100, 200);
+		usb_phy_vbus_on(musb->xceiv);
+		usleep_range(100, 200);
 
-	/*
-	 * When a babble condition occurs, the musb controller removes the
-	 * session bit and the endpoint config is lost.
-	 */
-	if (musb->dyn_fifo)
-		status = ep_config_from_table(musb);
-	else
-		status = ep_config_from_hw(musb);
+		/*
+		 * When a babble condition occurs, the musb controller
+		 * removes the session bit and the endpoint config is lost.
+		 */
+		if (musb->dyn_fifo)
+			status = ep_config_from_table(musb);
+		else
+			status = ep_config_from_hw(musb);
 
-	/* start the session again */
-	if (status == 0)
-		musb_start(musb);
+		/* start the session again */
+		if (status == 0)
+			musb_start(musb);
+	}
 }
 
 /* --------------------------------------------------------------------------
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index 423cd00..3ccb428 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -192,7 +192,7 @@ struct musb_platform_ops {
 
 	int	(*set_mode)(struct musb *musb, u8 mode);
 	void	(*try_idle)(struct musb *musb, unsigned long timeout);
-	void	(*reset)(struct musb *musb);
+	int	(*reset)(struct musb *musb);
 
 	int	(*vbus_status)(struct musb *musb);
 	void	(*set_vbus)(struct musb *musb, int on);
@@ -554,10 +554,12 @@ static inline void musb_platform_try_idle(struct musb *musb,
 		musb->ops->try_idle(musb, timeout);
 }
 
-static inline void musb_platform_reset(struct musb *musb)
+static inline int  musb_platform_reset(struct musb *musb)
 {
-	if (musb->ops->reset)
-		musb->ops->reset(musb);
+	if (!musb->ops->reset)
+		return -EINVAL;
+
+	return musb->ops->reset(musb);
 }
 
 static inline int musb_platform_get_vbus_status(struct musb *musb)
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 74c4193..f6f3087 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -536,7 +536,7 @@ static int dsps_musb_set_mode(struct musb *musb, u8 mode)
 	return 0;
 }
 
-static void dsps_musb_reset(struct musb *musb)
+static int dsps_musb_reset(struct musb *musb)
 {
 	struct device *dev = musb->controller;
 	struct dsps_glue *glue = dev_get_drvdata(dev->parent);
@@ -548,6 +548,7 @@ static void dsps_musb_reset(struct musb *musb)
 	usleep_range(100, 200);
 	usb_phy_init(musb->xceiv);
 
+	return 0;
 }
 
 static struct musb_platform_ops dsps_ops = {
-- 
1.8.3.1

[PATCH v4 5/6] usb: musb: dsps: Add the sw_babble_control()

[George Cherian]

Add sw_babble_control() logic to differentiate between transient
babble and real babble condition. Also add the SW babble control
register definitions.

Babble control register logic is implemented in the latest
revision of AM335x.

Signed-off-by: George Cherian <george.cherian@ti.com>
---
 drivers/usb/musb/musb_dsps.c | 50 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/musb/musb_regs.h |  7 +++++++
 2 files changed, 57 insertions(+)

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index f6f3087..eb1985a 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -536,6 +536,56 @@ static int dsps_musb_set_mode(struct musb *musb, u8 mode)
 	return 0;
 }
 
+static int sw_babble_control(struct musb *musb)
+{
+	int timeout = 10;
+	u8 babble_ctl, session_restart = 0;
+
+	babble_ctl = dsps_readb(musb->mregs, MUSB_BABBLE_CTL);
+	dev_dbg(musb->controller, "babble: MUSB_BABBLE_CTL value %x\n",
+		babble_ctl);
+	/*
+	 * check line monitor flag to check whether babble is
+	 * due to noise
+	 */
+	dev_dbg(musb->controller, "STUCK_J is %s\n",
+		babble_ctl & MUSB_BABBLE_STUCK_J ? "set" : "reset");
+
+	if (babble_ctl & MUSB_BABBLE_STUCK_J) {
+		/*
+		 * babble is due to noise, then set transmit idle (d7 bit)
+		 * to resume normal operation
+		 */
+		babble_ctl = musb_readb(musb->mregs, MUSB_BABBLE_CTL);
+		babble_ctl |= MUSB_BABBLE_FORCE_TXIDLE;
+		dsps_writeb(musb->mregs, MUSB_BABBLE_CTL, babble_ctl);
+
+		/* wait till line monitor flag cleared */
+		dev_dbg(musb->controller, "Set TXIDLE, wait J to clear\n");
+		do {
+			babble_ctl = dsps_readb(musb->mregs, MUSB_BABBLE_CTL);
+			udelay(1);
+		} while ((babble_ctl & MUSB_BABBLE_STUCK_J) && timeout--);
+
+		/* check whether stuck_at_j bit cleared */
+		if (babble_ctl & MUSB_BABBLE_STUCK_J) {
+			/*
+			 * real babble condition is occured
+			 * restart the controller to start the
+			 * session again
+			 */
+			dev_dbg(musb->controller, "J not cleared, misc (%x)\n",
+				babble_ctl);
+			session_restart = 1;
+		}
+
+	} else {
+		session_restart = 1;
+	}
+
+	return session_restart;
+}
+
 static int dsps_musb_reset(struct musb *musb)
 {
 	struct device *dev = musb->controller;
diff --git a/drivers/usb/musb/musb_regs.h b/drivers/usb/musb/musb_regs.h
index 03f2655..b9bcda5 100644
--- a/drivers/usb/musb/musb_regs.h
+++ b/drivers/usb/musb/musb_regs.h
@@ -72,6 +72,12 @@
 #define MUSB_DEVCTL_HR		0x02
 #define MUSB_DEVCTL_SESSION	0x01
 
+/* BABBLE_CTL */
+#define MUSB_BABBLE_FORCE_TXIDLE	0x80
+#define MUSB_BABBLE_SW_SESSION_CTRL	0x40
+#define MUSB_BABBLE_STUCK_J		0x20
+#define MUSB_BABBLE_RCV_DISABLE		0x04
+
 /* MUSB ULPI VBUSCONTROL */
 #define MUSB_ULPI_USE_EXTVBUS	0x01
 #define MUSB_ULPI_USE_EXTVBUSIND 0x02
@@ -246,6 +252,7 @@
  */
 
 #define MUSB_DEVCTL		0x60	/* 8 bit */
+#define MUSB_BABBLE_CTL		0x61	/* 8 bit */
 
 /* These are always controlled through the INDEX register */
 #define MUSB_TXFIFOSZ		0x62	/* 8-bit (see masks) */
-- 
1.8.3.1

[PATCH v4 6/6] usb: musb: dsps: Enable sw babble control for newer silicon

[George Cherian]

Find whether we are running on newer silicon. The babble control
register reads 0x4 by default in newer silicon as opposed to 0
in old versions of AM335x. Based on this enable the sw babble
control logic.

Signed-off-by: George Cherian <george.cherian@ti.com>
---
 drivers/usb/musb/musb_dsps.c | 37 +++++++++++++++++++++++++++++++------
 1 file changed, 31 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index eb1985a..8daccb2 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -136,6 +136,7 @@ struct dsps_glue {
 	const struct dsps_musb_wrapper *wrp; /* wrapper register offsets */
 	struct timer_list timer;	/* otg_workaround timer */
 	unsigned long last_timer;    /* last timer data for each instance */
+	bool sw_babble_enabled;
 
 	struct dsps_context context;
 	struct debugfs_regset32 regset;
@@ -469,6 +470,19 @@ static int dsps_musb_init(struct musb *musb)
 	val &= ~(1 << wrp->otg_disable);
 	dsps_writel(musb->ctrl_base, wrp->phy_utmi, val);
 
+	/*
+	 *  Check whether the dsps version has babble control enabled.
+	 * In latest silicon revision the babble control logic is enabled.
+	 * If MUSB_BABBLE_CTL returns 0x4 then we have the babble control
+	 * logic enabled.
+	 */
+	val = dsps_readb(musb->mregs, MUSB_BABBLE_CTL);
+	if (val == MUSB_BABBLE_RCV_DISABLE) {
+		glue->sw_babble_enabled = true;
+		val |= MUSB_BABBLE_SW_SESSION_CTRL;
+		dsps_writeb(musb->mregs, MUSB_BABBLE_CTL, val);
+	}
+
 	ret = dsps_musb_dbg_init(musb, glue);
 	if (ret)
 		return ret;
@@ -591,14 +605,25 @@ static int dsps_musb_reset(struct musb *musb)
 	struct device *dev = musb->controller;
 	struct dsps_glue *glue = dev_get_drvdata(dev->parent);
 	const struct dsps_musb_wrapper *wrp = glue->wrp;
+	int session_restart = 0;
 
-	dsps_writel(musb->ctrl_base, wrp->control, (1 << wrp->reset));
-	usleep_range(100, 200);
-	usb_phy_shutdown(musb->xceiv);
-	usleep_range(100, 200);
-	usb_phy_init(musb->xceiv);
+	if (glue->sw_babble_enabled)
+		session_restart = sw_babble_control(musb);
+	/*
+	 * In case of new silicon version babble condition can be recovered
+	 * without resetting the MUSB. But for older silicon versions, MUSB
+	 * reset is needed
+	 */
+	if (session_restart || !glue->sw_babble_enabled) {
+		dsps_writel(musb->ctrl_base, wrp->control, (1 << wrp->reset));
+		usleep_range(100, 200);
+		usb_phy_shutdown(musb->xceiv);
+		usleep_range(100, 200);
+		usb_phy_init(musb->xceiv);
+		session_restart = 1;
+	}
 
-	return 0;
+	return !session_restart;
 }
 
 static struct musb_platform_ops dsps_ops = {
-- 
1.8.3.1

Re: [PATCH v4 1/6] usb: musb: core: Handle Babble condition only in HOST mode

[Bin Liu]

Hi,

On Mon, May 19, 2014 at 8:39 AM, George Cherian <george.cherian@ti.com> wrote:
> BABBLE and RESET share the same interrupt. The interrupt
> is considered to be RESET if MUSB is in peripheral mode and
> as a BABBLE if MUSB is in HOST mode.
>
> Handle babble condition iff MUSB is in HOST mode.
>
> Signed-off-by: George Cherian <george.cherian@ti.com>
> ---
>  drivers/usb/musb/musb_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> index 61da471..eff3c5c 100644
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> @@ -849,7 +849,7 @@ b_host:
>         }
>
>         /* handle babble condition */
> -       if (int_usb & MUSB_INTR_BABBLE)
> +       if (int_usb & MUSB_INTR_BABBLE && is_host_active(musb))
>                 schedule_work(&musb->recover_work);

I guess my following comments are for Daniel's patch as while which
initially added the babble work.

Should this if statement be merged into the previous 'if(int_usb &
MUSB_INTR_RESET)' one, which handles the same interrupt and already
handles host and device mode respectively.

Regards,
-Bin.

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

Re: [PATCH v4 4/6] usb: musb: core: Convert the musb_platform_reset to have a return value.

[Bin Liu]

Hi,

On Mon, May 19, 2014 at 8:39 AM, George Cherian <george.cherian@ti.com> wrote:
> Currently musb_platform_reset() is only used by dsps.
> In case of BABBLE interrupt for other platforms the  musb_platform_reset()
> is a NOP. In such situations no need to re-initialize the endpoints.
> Also in the latest silicon revision of AM335x, we do have a babble recovery
> mechanism without resetting the IP block. In preperation to add that support
> its better to have a rest_done return for  musb_platform_reset().
>
> Signed-off-by: George Cherian <george.cherian@ti.com>
> ---
>  drivers/usb/musb/musb_core.c | 38 +++++++++++++++++++++-----------------
>  drivers/usb/musb/musb_core.h | 10 ++++++----
>  drivers/usb/musb/musb_dsps.c |  3 ++-
>  3 files changed, 29 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> index 8920b80..1b0b85d 100644
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> @@ -1755,28 +1755,32 @@ static void musb_irq_work(struct work_struct *data)
>  static void musb_recover_work(struct work_struct *data)
>  {
>         struct musb *musb = container_of(data, struct musb, recover_work.work);
> -       int status;
> +       int status, ret;
>
> -       musb_platform_reset(musb);
> +       ret  = musb_platform_reset(musb);
> +       if (ret  < 0)

If change it to ' if (ret)' here, you don't have to indent the reset
of the code.

Regards,
-Bin.

> +               return;
>
> -       usb_phy_vbus_off(musb->xceiv);
> -       usleep_range(100, 200);
> +       if (!ret) {
> +               usb_phy_vbus_off(musb->xceiv);
> +               usleep_range(100, 200);
>
> -       usb_phy_vbus_on(musb->xceiv);
> -       usleep_range(100, 200);
> +               usb_phy_vbus_on(musb->xceiv);
> +               usleep_range(100, 200);
>
> -       /*
> -        * When a babble condition occurs, the musb controller removes the
> -        * session bit and the endpoint config is lost.
> -        */
> -       if (musb->dyn_fifo)
> -               status = ep_config_from_table(musb);
> -       else
> -               status = ep_config_from_hw(musb);
> +               /*
> +                * When a babble condition occurs, the musb controller
> +                * removes the session bit and the endpoint config is lost.
> +                */
> +               if (musb->dyn_fifo)
> +                       status = ep_config_from_table(musb);
> +               else
> +                       status = ep_config_from_hw(musb);
>
> -       /* start the session again */
> -       if (status == 0)
> -               musb_start(musb);
> +               /* start the session again */
> +               if (status == 0)
> +                       musb_start(musb);
> +       }
>  }
>
>  /* --------------------------------------------------------------------------
> diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
> index 423cd00..3ccb428 100644
> --- a/drivers/usb/musb/musb_core.h
> +++ b/drivers/usb/musb/musb_core.h
> @@ -192,7 +192,7 @@ struct musb_platform_ops {
>
>         int     (*set_mode)(struct musb *musb, u8 mode);
>         void    (*try_idle)(struct musb *musb, unsigned long timeout);
> -       void    (*reset)(struct musb *musb);
> +       int     (*reset)(struct musb *musb);
>
>         int     (*vbus_status)(struct musb *musb);
>         void    (*set_vbus)(struct musb *musb, int on);
> @@ -554,10 +554,12 @@ static inline void musb_platform_try_idle(struct musb *musb,
>                 musb->ops->try_idle(musb, timeout);
>  }
>
> -static inline void musb_platform_reset(struct musb *musb)
> +static inline int  musb_platform_reset(struct musb *musb)
>  {
> -       if (musb->ops->reset)
> -               musb->ops->reset(musb);
> +       if (!musb->ops->reset)
> +               return -EINVAL;
> +
> +       return musb->ops->reset(musb);
>  }
>
>  static inline int musb_platform_get_vbus_status(struct musb *musb)
> diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
> index 74c4193..f6f3087 100644
> --- a/drivers/usb/musb/musb_dsps.c
> +++ b/drivers/usb/musb/musb_dsps.c
> @@ -536,7 +536,7 @@ static int dsps_musb_set_mode(struct musb *musb, u8 mode)
>         return 0;
>  }
>
> -static void dsps_musb_reset(struct musb *musb)
> +static int dsps_musb_reset(struct musb *musb)
>  {
>         struct device *dev = musb->controller;
>         struct dsps_glue *glue = dev_get_drvdata(dev->parent);
> @@ -548,6 +548,7 @@ static void dsps_musb_reset(struct musb *musb)
>         usleep_range(100, 200);
>         usb_phy_init(musb->xceiv);
>
> +       return 0;
>  }
>
>  static struct musb_platform_ops dsps_ops = {
> --
> 1.8.3.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 1/6] usb: musb: core: Handle Babble condition only in HOST mode

[George Cherian]

Hi Bin,
On 5/19/2014 9:24 PM, Bin Liu wrote:
> Hi,
>
> On Mon, May 19, 2014 at 8:39 AM, George Cherian <george.cherian@ti.com> wrote:
>> BABBLE and RESET share the same interrupt. The interrupt
>> is considered to be RESET if MUSB is in peripheral mode and
>> as a BABBLE if MUSB is in HOST mode.
>>
>> Handle babble condition iff MUSB is in HOST mode.
>>
>> Signed-off-by: George Cherian <george.cherian@ti.com>
>> ---
>>   drivers/usb/musb/musb_core.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
>> index 61da471..eff3c5c 100644
>> --- a/drivers/usb/musb/musb_core.c
>> +++ b/drivers/usb/musb/musb_core.c
>> @@ -849,7 +849,7 @@ b_host:
>>          }
>>
>>          /* handle babble condition */
>> -       if (int_usb & MUSB_INTR_BABBLE)
>> +       if (int_usb & MUSB_INTR_BABBLE && is_host_active(musb))
>>                  schedule_work(&musb->recover_work);
> I guess my following comments are for Daniel's patch as while which
> initially added the babble work.
>
> Should this if statement be merged into the previous 'if(int_usb &
> MUSB_INTR_RESET)' one, which handles the same interrupt and already
> handles host and device mode respectively.

Initially I too had the babble handling as part of  'if(int_usb & 
MUSB_INTR_RESET)'
one. But during my tests I hit a corner case where in we hit a BABBLE 
condition
on disconnect. In such case the babble interrupt can be handled only if 
we have a seperate
check, else its considered as a BUS RESET.

When all devices are disconnected  MUSB_DEVCTL_HM = 0 and the code 
always enter the
else path. In this path it treats the BABBLE as a BUS RESET.

> Regards,
> -Bin.
>
>>   #if 0
>> --
>> 1.8.3.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
-George

Re: [PATCH v4 1/6] usb: musb: core: Handle Babble condition only in HOST mode

[Bin Liu]

Hi George,

On Mon, May 19, 2014 at 11:32 PM, George Cherian <george.cherian@ti.com> wrote:
> Hi Bin,
>
> On 5/19/2014 9:24 PM, Bin Liu wrote:
>>
>> Hi,
>>
>> On Mon, May 19, 2014 at 8:39 AM, George Cherian <george.cherian@ti.com>
>> wrote:
>>>
>>> BABBLE and RESET share the same interrupt. The interrupt
>>> is considered to be RESET if MUSB is in peripheral mode and
>>> as a BABBLE if MUSB is in HOST mode.
>>>
>>> Handle babble condition iff MUSB is in HOST mode.
>>>
>>> Signed-off-by: George Cherian <george.cherian@ti.com>
>>> ---
>>>   drivers/usb/musb/musb_core.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
>>> index 61da471..eff3c5c 100644
>>> --- a/drivers/usb/musb/musb_core.c
>>> +++ b/drivers/usb/musb/musb_core.c
>>> @@ -849,7 +849,7 @@ b_host:
>>>          }
>>>
>>>          /* handle babble condition */
>>> -       if (int_usb & MUSB_INTR_BABBLE)
>>> +       if (int_usb & MUSB_INTR_BABBLE && is_host_active(musb))
>>>                  schedule_work(&musb->recover_work);
>>
>> I guess my following comments are for Daniel's patch as while which
>> initially added the babble work.
>>
>> Should this if statement be merged into the previous 'if(int_usb &
>> MUSB_INTR_RESET)' one, which handles the same interrupt and already
>> handles host and device mode respectively.
>
>
> Initially I too had the babble handling as part of  'if(int_usb &
> MUSB_INTR_RESET)'
> one. But during my tests I hit a corner case where in we hit a BABBLE
> condition
> on disconnect. In such case the babble interrupt can be handled only if we
> have a seperate
> check, else its considered as a BUS RESET.
>
> When all devices are disconnected  MUSB_DEVCTL_HM = 0 and the code always
> enter the
> else path. In this path it treats the BABBLE as a BUS RESET.

The code flow is a bit confusing, two if() handle the same interrupt.
The second one implied using 'handled = IRQ_HANDLED;' from the first
one.
Also does the switch() in else{} in the first if() cause any side effect?

Since this babble handing is AM335x specific, how about handle it in
dsps_interrupt() in musb_dsps.c, which already has an entry for babble
interrupt? TI 3.2 kernel does this way.

Regards,
-Bin.

>
>
>> Regards,
>> -Bin.
>>
>>>   #if 0
>>> --
>>> 1.8.3.1
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> -George
>

Re: [PATCH v4 1/6] usb: musb: core: Handle Babble condition only in HOST mode

[George Cherian]

On 5/23/2014 2:12 AM, Bin Liu wrote:
> Hi George,
>
> On Mon, May 19, 2014 at 11:32 PM, George Cherian <george.cherian@ti.com> wrote:
>> Hi Bin,
>>
>> On 5/19/2014 9:24 PM, Bin Liu wrote:
>>> Hi,
>>>
>>> On Mon, May 19, 2014 at 8:39 AM, George Cherian <george.cherian@ti.com>
>>> wrote:
>>>> BABBLE and RESET share the same interrupt. The interrupt
>>>> is considered to be RESET if MUSB is in peripheral mode and
>>>> as a BABBLE if MUSB is in HOST mode.
>>>>
>>>> Handle babble condition iff MUSB is in HOST mode.
>>>>
>>>> Signed-off-by: George Cherian <george.cherian@ti.com>
>>>> ---
>>>>    drivers/usb/musb/musb_core.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
>>>> index 61da471..eff3c5c 100644
>>>> --- a/drivers/usb/musb/musb_core.c
>>>> +++ b/drivers/usb/musb/musb_core.c
>>>> @@ -849,7 +849,7 @@ b_host:
>>>>           }
>>>>
>>>>           /* handle babble condition */
>>>> -       if (int_usb & MUSB_INTR_BABBLE)
>>>> +       if (int_usb & MUSB_INTR_BABBLE && is_host_active(musb))
>>>>                   schedule_work(&musb->recover_work);
>>> I guess my following comments are for Daniel's patch as while which
>>> initially added the babble work.
>>>
>>> Should this if statement be merged into the previous 'if(int_usb &
>>> MUSB_INTR_RESET)' one, which handles the same interrupt and already
>>> handles host and device mode respectively.
>>
>> Initially I too had the babble handling as part of  'if(int_usb &
>> MUSB_INTR_RESET)'
>> one. But during my tests I hit a corner case where in we hit a BABBLE
>> condition
>> on disconnect. In such case the babble interrupt can be handled only if we
>> have a seperate
>> check, else its considered as a BUS RESET.
>>
>> When all devices are disconnected  MUSB_DEVCTL_HM = 0 and the code always
>> enter the
>> else path. In this path it treats the BABBLE as a BUS RESET.
> The code flow is a bit confusing, two if() handle the same interrupt.
> The second one implied using 'handled = IRQ_HANDLED;' from the first
> one.
> Also does the switch() in else{} in the first if() cause any side effect?
No it doesn't.
> Since this babble handing is AM335x specific, how about handle it in
> dsps_interrupt() in musb_dsps.c, which already has an entry for babble
> interrupt? TI 3.2 kernel does this way.
That the reason we have platform specific callbacks added  from the main 
interrupt handler.
> Regards,
> -Bin.
>
>>
>>> Regards,
>>> -Bin.
>>>
>>>>    #if 0
>>>> --
>>>> 1.8.3.1
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
>> --
>> -George
>>


-- 
-George