[PATCH v2 1/3] usb: chipidea: core: refactor ci_usb_role_switch_set()

[PATCH v2 1/3] usb: chipidea: core: refactor ci_usb_role_switch_set()

[Xu Yang]

Current code is redundant, refactor the code, no function change.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v2:
 - new patch
---
 drivers/usb/chipidea/core.c | 29 +++++++----------------------
 1 file changed, 7 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index fac11f20cf0a..87be716dff3e 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -618,28 +618,13 @@ static int ci_usb_role_switch_set(struct usb_role_switch *sw,
 	struct ci_hdrc *ci = usb_role_switch_get_drvdata(sw);
 	struct ci_hdrc_cable *cable;
 
-	if (role == USB_ROLE_HOST) {
-		cable = &ci->platdata->id_extcon;
-		cable->changed = true;
-		cable->connected = true;
-		cable = &ci->platdata->vbus_extcon;
-		cable->changed = true;
-		cable->connected = false;
-	} else if (role == USB_ROLE_DEVICE) {
-		cable = &ci->platdata->id_extcon;
-		cable->changed = true;
-		cable->connected = false;
-		cable = &ci->platdata->vbus_extcon;
-		cable->changed = true;
-		cable->connected = true;
-	} else {
-		cable = &ci->platdata->id_extcon;
-		cable->changed = true;
-		cable->connected = false;
-		cable = &ci->platdata->vbus_extcon;
-		cable->changed = true;
-		cable->connected = false;
-	}
+	cable = &ci->platdata->id_extcon;
+	cable->changed = true;
+	cable->connected = (role == USB_ROLE_HOST);
+
+	cable = &ci->platdata->vbus_extcon;
+	cable->changed = true;
+	cable->connected = (role == USB_ROLE_DEVICE);
 
 	ci_irq(ci);
 	return 0;
-- 
2.34.1

[PATCH v2 2/3] usb: chipidea: core: allow ci_irq_handler() handle both ID and VBUS change

[Xu Yang]

For USB role switch-triggered IRQ, ID and VBUS change come together, for
example when switching from host to device mode. ID indicate a role switch
and VBUS is required to determine whether the device controller can start
operating. Currently, ci_irq_handler() handles only a single event per
invocation. This can cause an issue where switching to device mode results
in the device controller not working at all. Allowing ci_irq_handler() to
handle both ID and VBUS change in one call resolves this issue.

Meanwhile, this change also affects the VBUS event handling logic.
Previously, if an ID event indicated host mode the VBUS IRQ will be
ignored as the device disable BSE when stop() is called. With the new
behavior, if ID and VBUS IRQ occur together and the target mode is host,
the VBUS event is queued and ci_handle_vbus_change() will call
usb_gadget_vbus_connect(), after which USBMODE is switched to device mode,
causing host mode to stop working. To prevent this, an additional check is
added to skip handling VBUS event when current role is not device mode.

Suggested-by: Peter Chen <peter.chen@kernel.org>
Fixes: e1b5d2bed67c ("usb: chipidea: core: handle usb role switch in a common way")
Cc: stable@vger.kernel.org
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v2:
 - change ci_irq_handler() instead of assign id_event/b_sess_valid_event
   as true and queue otg work directly
---
 drivers/usb/chipidea/core.c | 45 +++++++++++++++++++------------------
 drivers/usb/chipidea/otg.c  |  3 +++
 2 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 87be716dff3e..7cfabb04a4fb 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -544,30 +544,31 @@ static irqreturn_t ci_irq_handler(int irq, void *data)
 			if (ret == IRQ_HANDLED)
 				return ret;
 		}
-	}
 
-	/*
-	 * Handle id change interrupt, it indicates device/host function
-	 * switch.
-	 */
-	if (ci->is_otg && (otgsc & OTGSC_IDIE) && (otgsc & OTGSC_IDIS)) {
-		ci->id_event = true;
-		/* Clear ID change irq status */
-		hw_write_otgsc(ci, OTGSC_IDIS, OTGSC_IDIS);
-		ci_otg_queue_work(ci);
-		return IRQ_HANDLED;
-	}
+		/*
+		 * Handle id change interrupt, it indicates device/host function
+		 * switch.
+		 */
+		if ((otgsc & OTGSC_IDIE) && (otgsc & OTGSC_IDIS)) {
+			ci->id_event = true;
+			/* Clear ID change irq status */
+			hw_write_otgsc(ci, OTGSC_IDIS, OTGSC_IDIS);
+		}
 
-	/*
-	 * Handle vbus change interrupt, it indicates device connection
-	 * and disconnection events.
-	 */
-	if (ci->is_otg && (otgsc & OTGSC_BSVIE) && (otgsc & OTGSC_BSVIS)) {
-		ci->b_sess_valid_event = true;
-		/* Clear BSV irq */
-		hw_write_otgsc(ci, OTGSC_BSVIS, OTGSC_BSVIS);
-		ci_otg_queue_work(ci);
-		return IRQ_HANDLED;
+		/*
+		 * Handle vbus change interrupt, it indicates device connection
+		 * and disconnection events.
+		 */
+		if ((otgsc & OTGSC_BSVIE) && (otgsc & OTGSC_BSVIS)) {
+			ci->b_sess_valid_event = true;
+			/* Clear BSV irq */
+			hw_write_otgsc(ci, OTGSC_BSVIS, OTGSC_BSVIS);
+		}
+
+		if (ci->id_event || ci->b_sess_valid_event) {
+			ci_otg_queue_work(ci);
+			return IRQ_HANDLED;
+		}
 	}
 
 	/* Handle device/host interrupt */
diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
index 647e98f4e351..def933b73a90 100644
--- a/drivers/usb/chipidea/otg.c
+++ b/drivers/usb/chipidea/otg.c
@@ -130,6 +130,9 @@ enum ci_role ci_otg_role(struct ci_hdrc *ci)
 
 void ci_handle_vbus_change(struct ci_hdrc *ci)
 {
+	if (ci->role != CI_ROLE_GADGET)
+		return;
+
 	if (!ci->is_otg) {
 		if (ci->platdata->flags & CI_HDRC_FORCE_VBUS_ACTIVE_ALWAYS)
 			usb_gadget_vbus_connect(&ci->gadget);
-- 
2.34.1

Re: [PATCH v2 2/3] usb: chipidea: core: allow ci_irq_handler() handle both ID and VBUS change

[Peter Chen (CIX)]

On 26-04-02 15:14:56, Xu Yang wrote:
> For USB role switch-triggered IRQ, ID and VBUS change come together, for
> example when switching from host to device mode. ID indicate a role switch
> and VBUS is required to determine whether the device controller can start
> operating. Currently, ci_irq_handler() handles only a single event per
> invocation. This can cause an issue where switching to device mode results
> in the device controller not working at all. Allowing ci_irq_handler() to
> handle both ID and VBUS change in one call resolves this issue.
> 
> Meanwhile, this change also affects the VBUS event handling logic.
> Previously, if an ID event indicated host mode the VBUS IRQ will be
> ignored as the device disable BSE when stop() is called. With the new
> behavior, if ID and VBUS IRQ occur together and the target mode is host,
> the VBUS event is queued and ci_handle_vbus_change() will call
> usb_gadget_vbus_connect(), after which USBMODE is switched to device mode,
> causing host mode to stop working. To prevent this, an additional check is
> added to skip handling VBUS event when current role is not device mode.
> 
> Suggested-by: Peter Chen <peter.chen@kernel.org>
> Fixes: e1b5d2bed67c ("usb: chipidea: core: handle usb role switch in a common way")
> Cc: stable@vger.kernel.org
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 
> ---
> Changes in v2:
>  - change ci_irq_handler() instead of assign id_event/b_sess_valid_event
>    as true and queue otg work directly
> ---
>  drivers/usb/chipidea/core.c | 45 +++++++++++++++++++------------------
>  drivers/usb/chipidea/otg.c  |  3 +++
>  2 files changed, 26 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 87be716dff3e..7cfabb04a4fb 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -544,30 +544,31 @@ static irqreturn_t ci_irq_handler(int irq, void *data)
>  			if (ret == IRQ_HANDLED)
>  				return ret;
>  		}
> -	}
>  
> -	/*
> -	 * Handle id change interrupt, it indicates device/host function
> -	 * switch.
> -	 */
> -	if (ci->is_otg && (otgsc & OTGSC_IDIE) && (otgsc & OTGSC_IDIS)) {
> -		ci->id_event = true;
> -		/* Clear ID change irq status */
> -		hw_write_otgsc(ci, OTGSC_IDIS, OTGSC_IDIS);
> -		ci_otg_queue_work(ci);
> -		return IRQ_HANDLED;
> -	}
> +		/*
> +		 * Handle id change interrupt, it indicates device/host function
> +		 * switch.
> +		 */
> +		if ((otgsc & OTGSC_IDIE) && (otgsc & OTGSC_IDIS)) {
> +			ci->id_event = true;
> +			/* Clear ID change irq status */
> +			hw_write_otgsc(ci, OTGSC_IDIS, OTGSC_IDIS);
> +		}
>  
> -	/*
> -	 * Handle vbus change interrupt, it indicates device connection
> -	 * and disconnection events.
> -	 */
> -	if (ci->is_otg && (otgsc & OTGSC_BSVIE) && (otgsc & OTGSC_BSVIS)) {
> -		ci->b_sess_valid_event = true;
> -		/* Clear BSV irq */
> -		hw_write_otgsc(ci, OTGSC_BSVIS, OTGSC_BSVIS);
> -		ci_otg_queue_work(ci);
> -		return IRQ_HANDLED;
> +		/*
> +		 * Handle vbus change interrupt, it indicates device connection
> +		 * and disconnection events.
> +		 */
> +		if ((otgsc & OTGSC_BSVIE) && (otgsc & OTGSC_BSVIS)) {
> +			ci->b_sess_valid_event = true;
> +			/* Clear BSV irq */
> +			hw_write_otgsc(ci, OTGSC_BSVIS, OTGSC_BSVIS);
> +		}
> +
> +		if (ci->id_event || ci->b_sess_valid_event) {
> +			ci_otg_queue_work(ci);
> +			return IRQ_HANDLED;
> +		}
>  	}
>  
>  	/* Handle device/host interrupt */
> diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
> index 647e98f4e351..def933b73a90 100644
> --- a/drivers/usb/chipidea/otg.c
> +++ b/drivers/usb/chipidea/otg.c
> @@ -130,6 +130,9 @@ enum ci_role ci_otg_role(struct ci_hdrc *ci)
>  
>  void ci_handle_vbus_change(struct ci_hdrc *ci)
>  {
> +	if (ci->role != CI_ROLE_GADGET)
> +		return;
> +

Are there the situations that the ci->role is not CI_ROLE_GADGET, but
it needs to handle VBUS? Eg, ci->role is CI_ROLE_NONE, and VBUS event
occurs?

-- 

Best regards,
Peter

Re: [PATCH v2 2/3] usb: chipidea: core: allow ci_irq_handler() handle both ID and VBUS change

[Xu Yang]

On Wed, Apr 08, 2026 at 01:39:52PM +0800, Peter Chen (CIX) wrote:
> On 26-04-02 15:14:56, Xu Yang wrote:
> > For USB role switch-triggered IRQ, ID and VBUS change come together, for
> > example when switching from host to device mode. ID indicate a role switch
> > and VBUS is required to determine whether the device controller can start
> > operating. Currently, ci_irq_handler() handles only a single event per
> > invocation. This can cause an issue where switching to device mode results
> > in the device controller not working at all. Allowing ci_irq_handler() to
> > handle both ID and VBUS change in one call resolves this issue.
> > 
> > Meanwhile, this change also affects the VBUS event handling logic.
> > Previously, if an ID event indicated host mode the VBUS IRQ will be
> > ignored as the device disable BSE when stop() is called. With the new
> > behavior, if ID and VBUS IRQ occur together and the target mode is host,
> > the VBUS event is queued and ci_handle_vbus_change() will call
> > usb_gadget_vbus_connect(), after which USBMODE is switched to device mode,
> > causing host mode to stop working. To prevent this, an additional check is
> > added to skip handling VBUS event when current role is not device mode.
> > 
> > Suggested-by: Peter Chen <peter.chen@kernel.org>
> > Fixes: e1b5d2bed67c ("usb: chipidea: core: handle usb role switch in a common way")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > 
> > ---
> > Changes in v2:
> >  - change ci_irq_handler() instead of assign id_event/b_sess_valid_event
> >    as true and queue otg work directly
> > ---
> >  drivers/usb/chipidea/core.c | 45 +++++++++++++++++++------------------
> >  drivers/usb/chipidea/otg.c  |  3 +++
> >  2 files changed, 26 insertions(+), 22 deletions(-)
> > 
> > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> > index 87be716dff3e..7cfabb04a4fb 100644
> > --- a/drivers/usb/chipidea/core.c
> > +++ b/drivers/usb/chipidea/core.c
> > @@ -544,30 +544,31 @@ static irqreturn_t ci_irq_handler(int irq, void *data)
> >  			if (ret == IRQ_HANDLED)
> >  				return ret;
> >  		}
> > -	}
> >  
> > -	/*
> > -	 * Handle id change interrupt, it indicates device/host function
> > -	 * switch.
> > -	 */
> > -	if (ci->is_otg && (otgsc & OTGSC_IDIE) && (otgsc & OTGSC_IDIS)) {
> > -		ci->id_event = true;
> > -		/* Clear ID change irq status */
> > -		hw_write_otgsc(ci, OTGSC_IDIS, OTGSC_IDIS);
> > -		ci_otg_queue_work(ci);
> > -		return IRQ_HANDLED;
> > -	}
> > +		/*
> > +		 * Handle id change interrupt, it indicates device/host function
> > +		 * switch.
> > +		 */
> > +		if ((otgsc & OTGSC_IDIE) && (otgsc & OTGSC_IDIS)) {
> > +			ci->id_event = true;
> > +			/* Clear ID change irq status */
> > +			hw_write_otgsc(ci, OTGSC_IDIS, OTGSC_IDIS);
> > +		}
> >  
> > -	/*
> > -	 * Handle vbus change interrupt, it indicates device connection
> > -	 * and disconnection events.
> > -	 */
> > -	if (ci->is_otg && (otgsc & OTGSC_BSVIE) && (otgsc & OTGSC_BSVIS)) {
> > -		ci->b_sess_valid_event = true;
> > -		/* Clear BSV irq */
> > -		hw_write_otgsc(ci, OTGSC_BSVIS, OTGSC_BSVIS);
> > -		ci_otg_queue_work(ci);
> > -		return IRQ_HANDLED;
> > +		/*
> > +		 * Handle vbus change interrupt, it indicates device connection
> > +		 * and disconnection events.
> > +		 */
> > +		if ((otgsc & OTGSC_BSVIE) && (otgsc & OTGSC_BSVIS)) {
> > +			ci->b_sess_valid_event = true;
> > +			/* Clear BSV irq */
> > +			hw_write_otgsc(ci, OTGSC_BSVIS, OTGSC_BSVIS);
> > +		}
> > +
> > +		if (ci->id_event || ci->b_sess_valid_event) {
> > +			ci_otg_queue_work(ci);
> > +			return IRQ_HANDLED;
> > +		}
> >  	}
> >  
> >  	/* Handle device/host interrupt */
> > diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
> > index 647e98f4e351..def933b73a90 100644
> > --- a/drivers/usb/chipidea/otg.c
> > +++ b/drivers/usb/chipidea/otg.c
> > @@ -130,6 +130,9 @@ enum ci_role ci_otg_role(struct ci_hdrc *ci)
> >  
> >  void ci_handle_vbus_change(struct ci_hdrc *ci)
> >  {
> > +	if (ci->role != CI_ROLE_GADGET)
> > +		return;
> > +
> 
> Are there the situations that the ci->role is not CI_ROLE_GADGET, but
> it needs to handle VBUS? Eg, ci->role is CI_ROLE_NONE, and VBUS event
> occurs?

I suppose you refer CI_ROLE_END.

When the role is CI_ROLE_END, the VBUS IRQ is disabled.
 - If the controller keeps at CI_ROLE_END state and VBUS comes, the IRQ
   won't trigger just like it is ignored as it means nothing for CI_ROLE_END.
 - If VBUS comes at CI_ROLE_END state and the controller switches to
   CI_ROLE_GADGET later, the VBUS event was lost but the driver will lookup
   real VBUS state and do proper actions.

So the situations seem not exist.

Thanks,
Xu Yang

[PATCH v2 3/3] usb: chipidea: otg: not wait vbus drop if use role_switch

[Xu Yang]

The usb role switch will update ID and VBUS states at the same time, and
vbus will not drop when execute data role swap in Type-C usecase. So lets
not wait vbus drop in usb role switch case too.

Fixes: e1b5d2bed67c ("usb: chipidea: core: handle usb role switch in a common way")
Cc: stable@vger.kernel.org
Acked-by: Peter Chen <peter.chen@kernel.org>
Reviewed-by: Jun Li <jun.li@nxp.com>
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v2:
 - add Ack by tag
---
 drivers/usb/chipidea/otg.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
index def933b73a90..fecc7d7e2f0d 100644
--- a/drivers/usb/chipidea/otg.c
+++ b/drivers/usb/chipidea/otg.c
@@ -190,8 +190,8 @@ void ci_handle_id_switch(struct ci_hdrc *ci)
 
 		ci_role_stop(ci);
 
-		if (role == CI_ROLE_GADGET &&
-				IS_ERR(ci->platdata->vbus_extcon.edev))
+		if (role == CI_ROLE_GADGET && !ci->role_switch &&
+		    IS_ERR(ci->platdata->vbus_extcon.edev))
 			/*
 			 * Wait vbus lower than OTGSC_BSV before connecting
 			 * to host. If connecting status is from an external
-- 
2.34.1

Re: [PATCH v2 1/3] usb: chipidea: core: refactor ci_usb_role_switch_set()

[Peter Chen (CIX)]

On 26-04-02 15:14:55, Xu Yang wrote:
> Current code is redundant, refactor the code, no function change.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

Acked-by: Peter Chen <peter.chen@kernel.org>

Peter
> 
> ---
> Changes in v2:
>  - new patch
> ---
>  drivers/usb/chipidea/core.c | 29 +++++++----------------------
>  1 file changed, 7 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index fac11f20cf0a..87be716dff3e 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -618,28 +618,13 @@ static int ci_usb_role_switch_set(struct usb_role_switch *sw,
>  	struct ci_hdrc *ci = usb_role_switch_get_drvdata(sw);
>  	struct ci_hdrc_cable *cable;
>  
> -	if (role == USB_ROLE_HOST) {
> -		cable = &ci->platdata->id_extcon;
> -		cable->changed = true;
> -		cable->connected = true;
> -		cable = &ci->platdata->vbus_extcon;
> -		cable->changed = true;
> -		cable->connected = false;
> -	} else if (role == USB_ROLE_DEVICE) {
> -		cable = &ci->platdata->id_extcon;
> -		cable->changed = true;
> -		cable->connected = false;
> -		cable = &ci->platdata->vbus_extcon;
> -		cable->changed = true;
> -		cable->connected = true;
> -	} else {
> -		cable = &ci->platdata->id_extcon;
> -		cable->changed = true;
> -		cable->connected = false;
> -		cable = &ci->platdata->vbus_extcon;
> -		cable->changed = true;
> -		cable->connected = false;
> -	}
> +	cable = &ci->platdata->id_extcon;
> +	cable->changed = true;
> +	cable->connected = (role == USB_ROLE_HOST);
> +
> +	cable = &ci->platdata->vbus_extcon;
> +	cable->changed = true;
> +	cable->connected = (role == USB_ROLE_DEVICE);
>  
>  	ci_irq(ci);
>  	return 0;
> -- 
> 2.34.1
> 

-- 

Best regards,
Peter