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@xxxxxxxxxx>
> > Fixes: e1b5d2bed67c ("usb: chipidea: core: handle usb role switch in a common way")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Xu Yang <xu.yang_2@xxxxxxx>
> > 
> > ---
> > 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