Re: [PATCH 1/3] usb: chipidea: add USB PHY event

[Xu Yang <xu.yang_2@nxp.com>]

Hi Paul,

On Tue, Aug 27, 2024 at 06:10:09AM +0000, Pu, Hui wrote:
> > Add USB PHY event for below situation:
> > - usb role changed
> > - vbus connect
> > - vbus disconnect
> > - gadget driver is enumerated
> > 
> > USB PHY driver can get the last event after above situation occurs and deal with
> > different situations.
> > 
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > ---
> >  drivers/usb/chipidea/ci.h  | 18 ++++++++++++++++--
> > drivers/usb/chipidea/udc.c | 10 ++++++++++
> >  2 files changed, 26 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h index
> > f210b7489fd5..d262b9df7b3d 100644
> > --- a/drivers/usb/chipidea/ci.h
> > +++ b/drivers/usb/chipidea/ci.h
> > @@ -281,8 +281,19 @@ static inline int ci_role_start(struct ci_hdrc *ci, enum
> > ci_role role)
> >  		return -ENXIO;
> > 
> >  	ret = ci->roles[role]->start(ci);
> > -	if (!ret)
> > -		ci->role = role;
> > +	if (ret)
> > +		return ret;
> > +
> > +	ci->role = role;
> > +
> > +	if (ci->usb_phy) {
> > +		if (role == CI_ROLE_HOST)
> > +			usb_phy_set_event(ci->usb_phy, USB_EVENT_ID);
> > +		else
> > +			/* in device mode but vbus is invalid*/
> > +			usb_phy_set_event(ci->usb_phy, USB_EVENT_NONE);
> > +	}
> > +
> >  	return ret;
> >  }
> > 
> > @@ -296,6 +307,9 @@ static inline void ci_role_stop(struct ci_hdrc *ci)
> >  	ci->role = CI_ROLE_END;
> > 
> >  	ci->roles[role]->stop(ci);
> > +
> > +	if (ci->usb_phy)
> > +		usb_phy_set_event(ci->usb_phy, USB_EVENT_NONE);
> >  }
> > 
> >  static inline enum usb_role ci_role_to_usb_role(struct ci_hdrc *ci) diff --git
> > a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index
> > 54c09245ad05..d58355427eeb 100644
> > --- a/drivers/usb/chipidea/udc.c
> > +++ b/drivers/usb/chipidea/udc.c
> > @@ -1718,6 +1718,13 @@ static int ci_udc_vbus_session(struct usb_gadget
> > *_gadget, int is_active)
> >  		ret = ci->platdata->notify_event(ci,
> >  				CI_HDRC_CONTROLLER_VBUS_EVENT);
> > 
> > +	if (ci->usb_phy) {
> > +		if (is_active)
> > +			usb_phy_set_event(ci->usb_phy, USB_EVENT_VBUS);
> > +		else
> > +			usb_phy_set_event(ci->usb_phy, USB_EVENT_NONE);
> > +	}
> > +
> >  	if (ci->driver)
> >  		ci_hdrc_gadget_connect(_gadget, is_active);
> > 
> > @@ -2034,6 +2041,9 @@ static irqreturn_t udc_irq(struct ci_hdrc *ci)
> >  		if (USBi_PCI & intr) {
> >  			ci->gadget.speed = hw_port_is_high_speed(ci) ?
> >  				USB_SPEED_HIGH : USB_SPEED_FULL;
> > +			if (ci->usb_phy)
> > +				usb_phy_set_event(ci->usb_phy,
> > +					USB_EVENT_ENUMERATED);
> >  			if (ci->suspended) {
> >  				if (ci->driver->resume) {
> >  					spin_unlock(&ci->lock);
> > --
> > 2.34.1
> 
> 
> Hi guys,
> 
> I'm not sure if I'm replying correctly, please correct me if any mistake.
> (I didn't find the cover letter in this thread.)
> 
> This patchset has been merged on master branch, but only the 2/3 patch on linux-5.15.y and linux-6.1.y.
> So, on 5.15.y and 6.1.y, there's a degradation on the i.MX6 devices that the usb hub cannot work well.

Thanks for reporting this. 
Could the usb hub work well if you apply patch 1 and patch 3 to your kernel?

Thanks,
Xu Yang

> 
> - Paul