[PATCH] usb: musb: Add support for optional VBUS irq to dsps glue layer

[PATCH] usb: musb: Add support for optional VBUS irq to dsps glue layer

[Tony Lindgren]

We can now configure the PMIC interrupt to provide us VBUS
events. In that case we don't need to constantly poll the
status and can make it optional. This is only wired up
for the mini-B interface on beaglebone.

Note that eventually we should get also the connect status
for the host interface when the am335x internal PM coprocessor
provides us with an IRQ chip. For now, we still need to poll
for the host mode status.

Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
 drivers/usb/musb/musb_dsps.c | 114 ++++++++++++++++++++++++++++++++++---------
 1 file changed, 90 insertions(+), 24 deletions(-)

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -118,6 +118,7 @@ struct dsps_glue {
 	struct device *dev;
 	struct platform_device *musb;	/* child musb pdev */
 	const struct dsps_musb_wrapper *wrp; /* wrapper register offsets */
+	int vbus_irq;			/* optional vbus irq */
 	struct timer_list timer;	/* otg_workaround timer */
 	unsigned long last_timer;    /* last timer data for each instance */
 	bool sw_babble_enabled;
@@ -145,6 +146,29 @@ static const struct debugfs_reg32 dsps_musb_regs[] = {
 	{ "mode",		0xe8 },
 };
 
+static void dsps_mod_timer(struct dsps_glue *glue, int wait_ms)
+{
+	int wait;
+
+	if (wait_ms < 0)
+		wait = msecs_to_jiffies(glue->wrp->poll_timeout);
+	else
+		wait = msecs_to_jiffies(wait_ms);
+
+	mod_timer(&glue->timer, jiffies + wait);
+}
+
+/*
+ * If no vbus irq from the PMIC is configured, we need to poll VBUS status.
+ */
+static void dsps_mod_timer_optional(struct dsps_glue *glue)
+{
+	if (glue->vbus_irq)
+		return;
+
+	dsps_mod_timer(glue, -1);
+}
+
 /**
  * dsps_musb_enable - enable interrupts
  */
@@ -167,8 +191,7 @@ static void dsps_musb_enable(struct musb *musb)
 	/* start polling for ID change in dual-role idle mode */
 	if (musb->xceiv->otg->state == OTG_STATE_B_IDLE &&
 			musb->port_mode == MUSB_PORT_MODE_DUAL_ROLE)
-		mod_timer(&glue->timer, jiffies +
-				msecs_to_jiffies(wrp->poll_timeout));
+		dsps_mod_timer(glue, -1);
 }
 
 /**
@@ -199,6 +222,9 @@ static int dsps_check_status(struct musb *musb, void *unused)
 	u8 devctl;
 	int skip_session = 0;
 
+	if (glue->vbus_irq)
+		del_timer(&glue->timer);
+
 	/*
 	 * We poll because DSPS IP's won't expose several OTG-critical
 	 * status change events (from the transceiver) otherwise.
@@ -209,8 +235,7 @@ static int dsps_check_status(struct musb *musb, void *unused)
 
 	switch (musb->xceiv->otg->state) {
 	case OTG_STATE_A_WAIT_VRISE:
-		mod_timer(&glue->timer, jiffies +
-				msecs_to_jiffies(wrp->poll_timeout));
+		dsps_mod_timer_optional(glue);
 		break;
 	case OTG_STATE_A_WAIT_BCON:
 		musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
@@ -219,17 +244,18 @@ static int dsps_check_status(struct musb *musb, void *unused)
 
 	case OTG_STATE_A_IDLE:
 	case OTG_STATE_B_IDLE:
-		if (devctl & MUSB_DEVCTL_BDEVICE) {
-			musb->xceiv->otg->state = OTG_STATE_B_IDLE;
-			MUSB_DEV_MODE(musb);
-		} else {
-			musb->xceiv->otg->state = OTG_STATE_A_IDLE;
-			MUSB_HST_MODE(musb);
+		if (!glue->vbus_irq) {
+			if (devctl & MUSB_DEVCTL_BDEVICE) {
+				musb->xceiv->otg->state = OTG_STATE_B_IDLE;
+				MUSB_DEV_MODE(musb);
+			} else {
+				musb->xceiv->otg->state = OTG_STATE_A_IDLE;
+				MUSB_HST_MODE(musb);
+			}
+			if (!(devctl & MUSB_DEVCTL_SESSION) && !skip_session)
+				musb_writeb(mregs, MUSB_DEVCTL, MUSB_DEVCTL_SESSION);
 		}
-		if (!(devctl & MUSB_DEVCTL_SESSION) && !skip_session)
-			musb_writeb(mregs, MUSB_DEVCTL, MUSB_DEVCTL_SESSION);
-		mod_timer(&glue->timer, jiffies +
-				msecs_to_jiffies(wrp->poll_timeout));
+		dsps_mod_timer_optional(glue);
 		break;
 	case OTG_STATE_A_WAIT_VFALL:
 		musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
@@ -321,15 +347,13 @@ static irqreturn_t dsps_interrupt(int irq, void *hci)
 			 */
 			musb->int_usb &= ~MUSB_INTR_VBUSERROR;
 			musb->xceiv->otg->state = OTG_STATE_A_WAIT_VFALL;
-			mod_timer(&glue->timer, jiffies +
-					msecs_to_jiffies(wrp->poll_timeout));
+			dsps_mod_timer_optional(glue);
 			WARNING("VBUS error workaround (delay coming)\n");
 		} else if (drvvbus) {
 			MUSB_HST_MODE(musb);
 			musb->xceiv->otg->default_a = 1;
 			musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
-			mod_timer(&glue->timer, jiffies +
-				  msecs_to_jiffies(wrp->poll_timeout));
+			dsps_mod_timer_optional(glue);
 		} else {
 			musb->is_active = 0;
 			MUSB_DEV_MODE(musb);
@@ -353,8 +377,7 @@ static irqreturn_t dsps_interrupt(int irq, void *hci)
 	switch (musb->xceiv->otg->state) {
 	case OTG_STATE_B_IDLE:
 	case OTG_STATE_A_WAIT_BCON:
-		mod_timer(&glue->timer, jiffies +
-				msecs_to_jiffies(wrp->poll_timeout));
+		dsps_mod_timer_optional(glue);
 		break;
 	default:
 		break;
@@ -458,8 +481,7 @@ static int dsps_musb_init(struct musb *musb)
 		musb_writeb(musb->mregs, MUSB_BABBLE_CTL, val);
 	}
 
-	mod_timer(&glue->timer, jiffies +
-		  msecs_to_jiffies(glue->wrp->poll_timeout));
+	dsps_mod_timer(glue, -1);
 
 	return dsps_musb_dbg_init(musb, glue);
 }
@@ -753,6 +775,47 @@ static int dsps_create_musb_pdev(struct dsps_glue *glue,
 	return ret;
 }
 
+static irqreturn_t dsps_vbus_threaded_irq(int irq, void *priv)
+{
+	struct dsps_glue *glue = priv;
+	struct musb *musb = platform_get_drvdata(glue->musb);
+
+	if (!musb)
+		return IRQ_NONE;
+
+	dev_dbg(glue->dev, "VBUS interrupt\n");
+	dsps_mod_timer(glue, 0);
+
+	return IRQ_HANDLED;
+}
+
+static int dsps_setup_optional_vbus_irq(struct platform_device *pdev,
+					struct dsps_glue *glue)
+{
+	int error;
+
+	glue->vbus_irq = platform_get_irq_byname(pdev, "vbus");
+	if (glue->vbus_irq == -EPROBE_DEFER)
+		return glue->vbus_irq;
+
+	if (glue->vbus_irq <= 0) {
+		glue->vbus_irq = 0;
+		return 0;
+	}
+
+	error = devm_request_threaded_irq(glue->dev, glue->vbus_irq,
+					  NULL, dsps_vbus_threaded_irq,
+					  IRQF_ONESHOT,
+					  "vbus", glue);
+	if (error) {
+		glue->vbus_irq = 0;
+		return error;
+	}
+	dev_dbg(glue->dev, "VBUS irq %i configured\n", glue->vbus_irq);
+
+	return 0;
+}
+
 static int dsps_probe(struct platform_device *pdev)
 {
 	const struct of_device_id *match;
@@ -781,6 +844,10 @@ static int dsps_probe(struct platform_device *pdev)
 	glue->dev = &pdev->dev;
 	glue->wrp = wrp;
 
+	ret = dsps_setup_optional_vbus_irq(pdev, glue);
+	if (ret)
+		return ret;
+
 	platform_set_drvdata(pdev, glue);
 	pm_runtime_enable(&pdev->dev);
 	ret = dsps_create_musb_pdev(glue, pdev);
@@ -891,8 +958,7 @@ static int dsps_resume(struct device *dev)
 	musb_writel(mbase, wrp->rx_mode, glue->context.rx_mode);
 	if (musb->xceiv->otg->state == OTG_STATE_B_IDLE &&
 	    musb->port_mode == MUSB_PORT_MODE_DUAL_ROLE)
-		mod_timer(&glue->timer, jiffies +
-				msecs_to_jiffies(wrp->poll_timeout));
+		dsps_mod_timer(glue, -1);
 
 	return 0;
 }
-- 
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: musb: Add support for optional VBUS irq to dsps glue layer

[Tony Lindgren]

* Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> [170105 11:14]:
> We can now configure the PMIC interrupt to provide us VBUS
> events. In that case we don't need to constantly poll the
> status and can make it optional. This is only wired up
> for the mini-B interface on beaglebone.
> 
> Note that eventually we should get also the connect status
> for the host interface when the am335x internal PM coprocessor
> provides us with an IRQ chip. For now, we still need to poll
> for the host mode status.

And here's a related patch I can queue separately to configure it.

Regards,

Tony

8< ---------------------------------
From: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
Date: Tue, 27 Dec 2016 08:03:41 -0800
Subject: [PATCH] ARM: dts: Configure BeagleBone peripheral USB VBUS irq

This prevents having to poll peripheral USB port cable status.

Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
 arch/arm/boot/dts/am335x-bone-common.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/am335x-bone-common.dtsi b/arch/arm/boot/dts/am335x-bone-common.dtsi
--- a/arch/arm/boot/dts/am335x-bone-common.dtsi
+++ b/arch/arm/boot/dts/am335x-bone-common.dtsi
@@ -207,6 +207,8 @@
 &usb0 {
 	status = "okay";
 	dr_mode = "peripheral";
+	interrupts-extended = <&intc 18 &tps 0>;
+	interrupt-names = "mc", "vbus";
 };
 
 &usb1 {
-- 
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: musb: Add support for optional VBUS irq to dsps glue layer

[Bin Liu]

On Thu, Jan 05, 2017 at 11:12:59AM -0800, Tony Lindgren wrote:
> We can now configure the PMIC interrupt to provide us VBUS
> events. In that case we don't need to constantly poll the
> status and can make it optional. This is only wired up
> for the mini-B interface on beaglebone.

Is it possible someone designed a board which hooks up the vbus of a
dual-role/otg port to PMIC? The port wouldn't be able to detect the
attach since no polling anymore.

> 
> Note that eventually we should get also the connect status
> for the host interface when the am335x internal PM coprocessor
> provides us with an IRQ chip. For now, we still need to poll
> for the host mode status.
> 
> Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/usb/musb/musb_dsps.c | 114 ++++++++++++++++++++++++++++++++++---------
>  1 file changed, 90 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
> --- a/drivers/usb/musb/musb_dsps.c
> +++ b/drivers/usb/musb/musb_dsps.c
> @@ -118,6 +118,7 @@ struct dsps_glue {
>  	struct device *dev;
>  	struct platform_device *musb;	/* child musb pdev */
>  	const struct dsps_musb_wrapper *wrp; /* wrapper register offsets */
> +	int vbus_irq;			/* optional vbus irq */
>  	struct timer_list timer;	/* otg_workaround timer */
>  	unsigned long last_timer;    /* last timer data for each instance */
>  	bool sw_babble_enabled;
> @@ -145,6 +146,29 @@ static const struct debugfs_reg32 dsps_musb_regs[] = {
>  	{ "mode",		0xe8 },
>  };
>  
> +static void dsps_mod_timer(struct dsps_glue *glue, int wait_ms)
> +{
> +	int wait;
> +
> +	if (wait_ms < 0)
> +		wait = msecs_to_jiffies(glue->wrp->poll_timeout);
> +	else
> +		wait = msecs_to_jiffies(wait_ms);
> +
> +	mod_timer(&glue->timer, jiffies + wait);
> +}
> +
> +/*
> + * If no vbus irq from the PMIC is configured, we need to poll VBUS status.
> + */
> +static void dsps_mod_timer_optional(struct dsps_glue *glue)
> +{
> +	if (glue->vbus_irq)
> +		return;
> +
> +	dsps_mod_timer(glue, -1);
> +}
> +
>  /**
>   * dsps_musb_enable - enable interrupts
>   */
> @@ -167,8 +191,7 @@ static void dsps_musb_enable(struct musb *musb)
>  	/* start polling for ID change in dual-role idle mode */
>  	if (musb->xceiv->otg->state == OTG_STATE_B_IDLE &&
>  			musb->port_mode == MUSB_PORT_MODE_DUAL_ROLE)
> -		mod_timer(&glue->timer, jiffies +
> -				msecs_to_jiffies(wrp->poll_timeout));
> +		dsps_mod_timer(glue, -1);
>  }
>  
>  /**
> @@ -199,6 +222,9 @@ static int dsps_check_status(struct musb *musb, void *unused)
>  	u8 devctl;
>  	int skip_session = 0;
>  
> +	if (glue->vbus_irq)
> +		del_timer(&glue->timer);
> +
>  	/*
>  	 * We poll because DSPS IP's won't expose several OTG-critical
>  	 * status change events (from the transceiver) otherwise.
> @@ -209,8 +235,7 @@ static int dsps_check_status(struct musb *musb, void *unused)
>  
>  	switch (musb->xceiv->otg->state) {
>  	case OTG_STATE_A_WAIT_VRISE:
> -		mod_timer(&glue->timer, jiffies +
> -				msecs_to_jiffies(wrp->poll_timeout));
> +		dsps_mod_timer_optional(glue);
>  		break;
>  	case OTG_STATE_A_WAIT_BCON:
>  		musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
> @@ -219,17 +244,18 @@ static int dsps_check_status(struct musb *musb, void *unused)
>  
>  	case OTG_STATE_A_IDLE:
>  	case OTG_STATE_B_IDLE:
> -		if (devctl & MUSB_DEVCTL_BDEVICE) {
> -			musb->xceiv->otg->state = OTG_STATE_B_IDLE;
> -			MUSB_DEV_MODE(musb);
> -		} else {
> -			musb->xceiv->otg->state = OTG_STATE_A_IDLE;
> -			MUSB_HST_MODE(musb);
> +		if (!glue->vbus_irq) {
> +			if (devctl & MUSB_DEVCTL_BDEVICE) {
> +				musb->xceiv->otg->state = OTG_STATE_B_IDLE;
> +				MUSB_DEV_MODE(musb);
> +			} else {
> +				musb->xceiv->otg->state = OTG_STATE_A_IDLE;
> +				MUSB_HST_MODE(musb);
> +			}
> +			if (!(devctl & MUSB_DEVCTL_SESSION) && !skip_session)
> +				musb_writeb(mregs, MUSB_DEVCTL, MUSB_DEVCTL_SESSION);

Line is more than 80 chars long.

>  		}
> -		if (!(devctl & MUSB_DEVCTL_SESSION) && !skip_session)
> -			musb_writeb(mregs, MUSB_DEVCTL, MUSB_DEVCTL_SESSION);
> -		mod_timer(&glue->timer, jiffies +
> -				msecs_to_jiffies(wrp->poll_timeout));
> +		dsps_mod_timer_optional(glue);
>  		break;
>  	case OTG_STATE_A_WAIT_VFALL:
>  		musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
> @@ -321,15 +347,13 @@ static irqreturn_t dsps_interrupt(int irq, void *hci)
>  			 */
>  			musb->int_usb &= ~MUSB_INTR_VBUSERROR;
>  			musb->xceiv->otg->state = OTG_STATE_A_WAIT_VFALL;
> -			mod_timer(&glue->timer, jiffies +
> -					msecs_to_jiffies(wrp->poll_timeout));
> +			dsps_mod_timer_optional(glue);
>  			WARNING("VBUS error workaround (delay coming)\n");
>  		} else if (drvvbus) {
>  			MUSB_HST_MODE(musb);
>  			musb->xceiv->otg->default_a = 1;
>  			musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
> -			mod_timer(&glue->timer, jiffies +
> -				  msecs_to_jiffies(wrp->poll_timeout));
> +			dsps_mod_timer_optional(glue);
>  		} else {
>  			musb->is_active = 0;
>  			MUSB_DEV_MODE(musb);
> @@ -353,8 +377,7 @@ static irqreturn_t dsps_interrupt(int irq, void *hci)
>  	switch (musb->xceiv->otg->state) {
>  	case OTG_STATE_B_IDLE:
>  	case OTG_STATE_A_WAIT_BCON:
> -		mod_timer(&glue->timer, jiffies +
> -				msecs_to_jiffies(wrp->poll_timeout));
> +		dsps_mod_timer_optional(glue);
>  		break;
>  	default:
>  		break;
> @@ -458,8 +481,7 @@ static int dsps_musb_init(struct musb *musb)
>  		musb_writeb(musb->mregs, MUSB_BABBLE_CTL, val);
>  	}
>  
> -	mod_timer(&glue->timer, jiffies +
> -		  msecs_to_jiffies(glue->wrp->poll_timeout));
> +	dsps_mod_timer(glue, -1);
>  
>  	return dsps_musb_dbg_init(musb, glue);
>  }
> @@ -753,6 +775,47 @@ static int dsps_create_musb_pdev(struct dsps_glue *glue,
>  	return ret;
>  }
>  
> +static irqreturn_t dsps_vbus_threaded_irq(int irq, void *priv)
> +{
> +	struct dsps_glue *glue = priv;
> +	struct musb *musb = platform_get_drvdata(glue->musb);
> +
> +	if (!musb)
> +		return IRQ_NONE;
> +
> +	dev_dbg(glue->dev, "VBUS interrupt\n");
> +	dsps_mod_timer(glue, 0);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int dsps_setup_optional_vbus_irq(struct platform_device *pdev,
> +					struct dsps_glue *glue)
> +{
> +	int error;
> +
> +	glue->vbus_irq = platform_get_irq_byname(pdev, "vbus");
> +	if (glue->vbus_irq == -EPROBE_DEFER)
> +		return glue->vbus_irq;

It would be more obvious if return -EPROBE_DEFER directly.

Regards,
-Bin.

> +
> +	if (glue->vbus_irq <= 0) {
> +		glue->vbus_irq = 0;
> +		return 0;
> +	}
> +
> +	error = devm_request_threaded_irq(glue->dev, glue->vbus_irq,
> +					  NULL, dsps_vbus_threaded_irq,
> +					  IRQF_ONESHOT,
> +					  "vbus", glue);
> +	if (error) {
> +		glue->vbus_irq = 0;
> +		return error;
> +	}
> +	dev_dbg(glue->dev, "VBUS irq %i configured\n", glue->vbus_irq);
> +
> +	return 0;
> +}
> +
>  static int dsps_probe(struct platform_device *pdev)
>  {
>  	const struct of_device_id *match;
> @@ -781,6 +844,10 @@ static int dsps_probe(struct platform_device *pdev)
>  	glue->dev = &pdev->dev;
>  	glue->wrp = wrp;
>  
> +	ret = dsps_setup_optional_vbus_irq(pdev, glue);
> +	if (ret)
> +		return ret;
> +
>  	platform_set_drvdata(pdev, glue);
>  	pm_runtime_enable(&pdev->dev);
>  	ret = dsps_create_musb_pdev(glue, pdev);
> @@ -891,8 +958,7 @@ static int dsps_resume(struct device *dev)
>  	musb_writel(mbase, wrp->rx_mode, glue->context.rx_mode);
>  	if (musb->xceiv->otg->state == OTG_STATE_B_IDLE &&
>  	    musb->port_mode == MUSB_PORT_MODE_DUAL_ROLE)
> -		mod_timer(&glue->timer, jiffies +
> -				msecs_to_jiffies(wrp->poll_timeout));
> +		dsps_mod_timer(glue, -1);
>  
>  	return 0;
>  }
> -- 
> 2.11.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: musb: Add support for optional VBUS irq to dsps glue layer

[Bin Liu]

On Tue, Jan 10, 2017 at 01:53:53PM -0600, Bin Liu wrote:
> On Thu, Jan 05, 2017 at 11:12:59AM -0800, Tony Lindgren wrote:
> > We can now configure the PMIC interrupt to provide us VBUS
> > events. In that case we don't need to constantly poll the
> > status and can make it optional. This is only wired up
> > for the mini-B interface on beaglebone.
> 
> Is it possible someone designed a board which hooks up the vbus of a
> dual-role/otg port to PMIC? The port wouldn't be able to detect the
> attach since no polling anymore.

I haven't seen any am335xx design other than the Beaglebone, that routes
VBUS to PMIC, and I guess the reason that Beaglebone did this way is
only for supporting bus-powered.

> 
> > 
> > Note that eventually we should get also the connect status
> > for the host interface when the am335x internal PM coprocessor
> > provides us with an IRQ chip. For now, we still need to poll
> > for the host mode status.
> > 
> > Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> > ---
> >  drivers/usb/musb/musb_dsps.c | 114 ++++++++++++++++++++++++++++++++++---------
> >  1 file changed, 90 insertions(+), 24 deletions(-)
> > 
> > diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
> > --- a/drivers/usb/musb/musb_dsps.c
> > +++ b/drivers/usb/musb/musb_dsps.c
> > @@ -118,6 +118,7 @@ struct dsps_glue {
> >  	struct device *dev;
> >  	struct platform_device *musb;	/* child musb pdev */
> >  	const struct dsps_musb_wrapper *wrp; /* wrapper register offsets */
> > +	int vbus_irq;			/* optional vbus irq */
> >  	struct timer_list timer;	/* otg_workaround timer */
> >  	unsigned long last_timer;    /* last timer data for each instance */
> >  	bool sw_babble_enabled;
> > @@ -145,6 +146,29 @@ static const struct debugfs_reg32 dsps_musb_regs[] = {
> >  	{ "mode",		0xe8 },
> >  };
> >  
> > +static void dsps_mod_timer(struct dsps_glue *glue, int wait_ms)
> > +{
> > +	int wait;
> > +
> > +	if (wait_ms < 0)
> > +		wait = msecs_to_jiffies(glue->wrp->poll_timeout);
> > +	else
> > +		wait = msecs_to_jiffies(wait_ms);
> > +
> > +	mod_timer(&glue->timer, jiffies + wait);
> > +}
> > +
> > +/*
> > + * If no vbus irq from the PMIC is configured, we need to poll VBUS status.
> > + */
> > +static void dsps_mod_timer_optional(struct dsps_glue *glue)
> > +{
> > +	if (glue->vbus_irq)
> > +		return;
> > +
> > +	dsps_mod_timer(glue, -1);
> > +}
> > +
> >  /**
> >   * dsps_musb_enable - enable interrupts
> >   */
> > @@ -167,8 +191,7 @@ static void dsps_musb_enable(struct musb *musb)
> >  	/* start polling for ID change in dual-role idle mode */
> >  	if (musb->xceiv->otg->state == OTG_STATE_B_IDLE &&
> >  			musb->port_mode == MUSB_PORT_MODE_DUAL_ROLE)
> > -		mod_timer(&glue->timer, jiffies +
> > -				msecs_to_jiffies(wrp->poll_timeout));
> > +		dsps_mod_timer(glue, -1);
> >  }
> >  
> >  /**
> > @@ -199,6 +222,9 @@ static int dsps_check_status(struct musb *musb, void *unused)
> >  	u8 devctl;
> >  	int skip_session = 0;
> >  
> > +	if (glue->vbus_irq)
> > +		del_timer(&glue->timer);
> > +
> >  	/*
> >  	 * We poll because DSPS IP's won't expose several OTG-critical
> >  	 * status change events (from the transceiver) otherwise.
> > @@ -209,8 +235,7 @@ static int dsps_check_status(struct musb *musb, void *unused)
> >  
> >  	switch (musb->xceiv->otg->state) {
> >  	case OTG_STATE_A_WAIT_VRISE:
> > -		mod_timer(&glue->timer, jiffies +
> > -				msecs_to_jiffies(wrp->poll_timeout));
> > +		dsps_mod_timer_optional(glue);
> >  		break;
> >  	case OTG_STATE_A_WAIT_BCON:
> >  		musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
> > @@ -219,17 +244,18 @@ static int dsps_check_status(struct musb *musb, void *unused)
> >  
> >  	case OTG_STATE_A_IDLE:
> >  	case OTG_STATE_B_IDLE:
> > -		if (devctl & MUSB_DEVCTL_BDEVICE) {
> > -			musb->xceiv->otg->state = OTG_STATE_B_IDLE;
> > -			MUSB_DEV_MODE(musb);
> > -		} else {
> > -			musb->xceiv->otg->state = OTG_STATE_A_IDLE;
> > -			MUSB_HST_MODE(musb);
> > +		if (!glue->vbus_irq) {
> > +			if (devctl & MUSB_DEVCTL_BDEVICE) {
> > +				musb->xceiv->otg->state = OTG_STATE_B_IDLE;
> > +				MUSB_DEV_MODE(musb);
> > +			} else {
> > +				musb->xceiv->otg->state = OTG_STATE_A_IDLE;
> > +				MUSB_HST_MODE(musb);
> > +			}
> > +			if (!(devctl & MUSB_DEVCTL_SESSION) && !skip_session)
> > +				musb_writeb(mregs, MUSB_DEVCTL, MUSB_DEVCTL_SESSION);
> 
> Line is more than 80 chars long.
> 
> >  		}
> > -		if (!(devctl & MUSB_DEVCTL_SESSION) && !skip_session)
> > -			musb_writeb(mregs, MUSB_DEVCTL, MUSB_DEVCTL_SESSION);
> > -		mod_timer(&glue->timer, jiffies +
> > -				msecs_to_jiffies(wrp->poll_timeout));
> > +		dsps_mod_timer_optional(glue);
> >  		break;
> >  	case OTG_STATE_A_WAIT_VFALL:
> >  		musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
> > @@ -321,15 +347,13 @@ static irqreturn_t dsps_interrupt(int irq, void *hci)
> >  			 */
> >  			musb->int_usb &= ~MUSB_INTR_VBUSERROR;
> >  			musb->xceiv->otg->state = OTG_STATE_A_WAIT_VFALL;
> > -			mod_timer(&glue->timer, jiffies +
> > -					msecs_to_jiffies(wrp->poll_timeout));
> > +			dsps_mod_timer_optional(glue);
> >  			WARNING("VBUS error workaround (delay coming)\n");
> >  		} else if (drvvbus) {
> >  			MUSB_HST_MODE(musb);
> >  			musb->xceiv->otg->default_a = 1;
> >  			musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
> > -			mod_timer(&glue->timer, jiffies +
> > -				  msecs_to_jiffies(wrp->poll_timeout));
> > +			dsps_mod_timer_optional(glue);
> >  		} else {
> >  			musb->is_active = 0;
> >  			MUSB_DEV_MODE(musb);
> > @@ -353,8 +377,7 @@ static irqreturn_t dsps_interrupt(int irq, void *hci)
> >  	switch (musb->xceiv->otg->state) {
> >  	case OTG_STATE_B_IDLE:
> >  	case OTG_STATE_A_WAIT_BCON:
> > -		mod_timer(&glue->timer, jiffies +
> > -				msecs_to_jiffies(wrp->poll_timeout));
> > +		dsps_mod_timer_optional(glue);
> >  		break;
> >  	default:
> >  		break;
> > @@ -458,8 +481,7 @@ static int dsps_musb_init(struct musb *musb)
> >  		musb_writeb(musb->mregs, MUSB_BABBLE_CTL, val);
> >  	}
> >  
> > -	mod_timer(&glue->timer, jiffies +
> > -		  msecs_to_jiffies(glue->wrp->poll_timeout));
> > +	dsps_mod_timer(glue, -1);
> >  
> >  	return dsps_musb_dbg_init(musb, glue);
> >  }
> > @@ -753,6 +775,47 @@ static int dsps_create_musb_pdev(struct dsps_glue *glue,
> >  	return ret;
> >  }
> >  
> > +static irqreturn_t dsps_vbus_threaded_irq(int irq, void *priv)
> > +{
> > +	struct dsps_glue *glue = priv;
> > +	struct musb *musb = platform_get_drvdata(glue->musb);
> > +
> > +	if (!musb)
> > +		return IRQ_NONE;
> > +
> > +	dev_dbg(glue->dev, "VBUS interrupt\n");
> > +	dsps_mod_timer(glue, 0);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static int dsps_setup_optional_vbus_irq(struct platform_device *pdev,
> > +					struct dsps_glue *glue)
> > +{
> > +	int error;
> > +

How about check dr_mode at here, and simply return if dr_mode is not
"peripheral". So this ensures this VBUS irq is only used for device
mode, it clear my concern above.

Regards,
-Bin.

> > +	glue->vbus_irq = platform_get_irq_byname(pdev, "vbus");
> > +	if (glue->vbus_irq == -EPROBE_DEFER)
> > +		return glue->vbus_irq;
> 
> It would be more obvious if return -EPROBE_DEFER directly.
> 
> Regards,
> -Bin.
> 
> > +
> > +	if (glue->vbus_irq <= 0) {
> > +		glue->vbus_irq = 0;
> > +		return 0;
> > +	}
> > +
> > +	error = devm_request_threaded_irq(glue->dev, glue->vbus_irq,
> > +					  NULL, dsps_vbus_threaded_irq,
> > +					  IRQF_ONESHOT,
> > +					  "vbus", glue);
> > +	if (error) {
> > +		glue->vbus_irq = 0;
> > +		return error;
> > +	}
> > +	dev_dbg(glue->dev, "VBUS irq %i configured\n", glue->vbus_irq);
> > +
> > +	return 0;
> > +}
> > +
> >  static int dsps_probe(struct platform_device *pdev)
> >  {
> >  	const struct of_device_id *match;
> > @@ -781,6 +844,10 @@ static int dsps_probe(struct platform_device *pdev)
> >  	glue->dev = &pdev->dev;
> >  	glue->wrp = wrp;
> >  
> > +	ret = dsps_setup_optional_vbus_irq(pdev, glue);
> > +	if (ret)
> > +		return ret;
> > +
> >  	platform_set_drvdata(pdev, glue);
> >  	pm_runtime_enable(&pdev->dev);
> >  	ret = dsps_create_musb_pdev(glue, pdev);
> > @@ -891,8 +958,7 @@ static int dsps_resume(struct device *dev)
> >  	musb_writel(mbase, wrp->rx_mode, glue->context.rx_mode);
> >  	if (musb->xceiv->otg->state == OTG_STATE_B_IDLE &&
> >  	    musb->port_mode == MUSB_PORT_MODE_DUAL_ROLE)
> > -		mod_timer(&glue->timer, jiffies +
> > -				msecs_to_jiffies(wrp->poll_timeout));
> > +		dsps_mod_timer(glue, -1);
> >  
> >  	return 0;
> >  }
> > -- 
> > 2.11.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: musb: Add support for optional VBUS irq to dsps glue layer

[Tony Lindgren]

* Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [170111 05:42]:
> On Tue, Jan 10, 2017 at 01:53:53PM -0600, Bin Liu wrote:
> > On Thu, Jan 05, 2017 at 11:12:59AM -0800, Tony Lindgren wrote:
> > > We can now configure the PMIC interrupt to provide us VBUS
> > > events. In that case we don't need to constantly poll the
> > > status and can make it optional. This is only wired up
> > > for the mini-B interface on beaglebone.
> > 
> > Is it possible someone designed a board which hooks up the vbus of a
> > dual-role/otg port to PMIC? The port wouldn't be able to detect the
> > attach since no polling anymore.
> 
> I haven't seen any am335xx design other than the Beaglebone, that routes
> VBUS to PMIC, and I guess the reason that Beaglebone did this way is
> only for supporting bus-powered.

Yeah I guess that remains to be seen. I guess it could also be configured
to use any 5V GPIO in the dts and ideally and work the same way.

> > > +static int dsps_setup_optional_vbus_irq(struct platform_device *pdev,
> > > +					struct dsps_glue *glue)
> > > +{
> > > +	int error;
> > > +
> 
> How about check dr_mode at here, and simply return if dr_mode is not
> "peripheral". So this ensures this VBUS irq is only used for device
> mode, it clear my concern above.

Yeah OK I like your idea of limiting it to peripheral only for now.
Will repost with your other comments addressed also.

Regards,

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