| Message ID | 20230627110353.1879477-1-xu.yang_2@nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] usb: chipidea: add USB PHY event | expand |
On 23-06-27 19:03:51, Xu Yang 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> Acked-by: Peter Chen <peter.chen@kernel.org> Peter > --- > 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 >
Hello Xu, On Tue, 27 Jun 2023 19:03:51 +0800 Xu Yang <xu.yang_2@nxp.com> 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> I tested this patchset on top of v6.5-rc2 and USB detection is still broken on the Colibri iMX6ULL. With or without the patches the behavior is the same: USB devices are detected only during boot, and anything connected after boot is never detected. Is there anything I can test for you do understand what's going wrong here? Luca
> 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. - Paul
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
Hi Xu Yang, Thanks for reply. > 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? Yes, work well. - Paul > > Thanks, > Xu Yang > > > > > - Paul
Hi Greg and Sasha, On Tue, Jun 27, 2023 at 07:03:51PM +0800, Xu Yang 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> These 3 patches had already merged to usb tree. But I didn't add fix tag to patch #1 and #3, therefore, only patch #2 went to stable tree. Now the stable tree linux-5.15.y and linux-6.1.y have issue due to patch #2 depends on patch #1. So could you please add patch #1 and patch #3 to linux-5.15.y and linux-6.1.y? Or should I post a normal request to achieve this? Thanks in advance! Best Regards, Xu Yang > --- > 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 >
On Mon, Sep 02, 2024 at 03:59:12PM +0800, Xu Yang wrote: > Hi Greg and Sasha, > > On Tue, Jun 27, 2023 at 07:03:51PM +0800, Xu Yang 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> > > These 3 patches had already merged to usb tree. But I didn't add fix tag > to patch #1 and #3, therefore, only patch #2 went to stable tree. Now > the stable tree linux-5.15.y and linux-6.1.y have issue due to patch #2 > depends on patch #1. So could you please add patch #1 and patch #3 to > linux-5.15.y and linux-6.1.y? Or should I post a normal request to achieve > this? Please send a "normal" request as I have no idea what to do here, nor what the git commit ids are. thanks, greg k-h
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);
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(-)