* [PATCH] usb: cdns3: Fixed incorrect gadget state
@ 2021-06-23 7:02 Pawel Laszczak
2021-06-26 8:56 ` Peter Chen
0 siblings, 1 reply; 5+ messages in thread
From: Pawel Laszczak @ 2021-06-23 7:02 UTC (permalink / raw)
To: peter.chen
Cc: rogerq, a-govindraju, gregkh, felipe.balbi, linux-usb,
linux-kernel, kishon, kurahul, sparmar, Pawel Laszczak, stable
From: Pawel Laszczak <pawell@cadence.com>
For delayed status phase, the usb_gadget->state was set
to USB_STATE_ADDRESS and it has never been updated to
USB_STATE_CONFIGURED.
Patch updates the gadget state to correct USB_STATE_CONFIGURED.
As a result of this bug the controller was not able to enter to
Test Mode while using MSC function.
Cc: <stable@vger.kernel.org>
Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver")
Signed-off-by: Pawel Laszczak <pawell@cadence.com>
---
drivers/usb/cdns3/cdns3-ep0.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/cdns3/cdns3-ep0.c b/drivers/usb/cdns3/cdns3-ep0.c
index 9a17802275d5..ec5bfd8944c3 100644
--- a/drivers/usb/cdns3/cdns3-ep0.c
+++ b/drivers/usb/cdns3/cdns3-ep0.c
@@ -731,6 +731,7 @@ static int cdns3_gadget_ep0_queue(struct usb_ep *ep,
request->actual = 0;
priv_dev->status_completion_no_call = true;
priv_dev->pending_status_request = request;
+ usb_gadget_set_state(&priv_dev->gadget, USB_STATE_CONFIGURED);
spin_unlock_irqrestore(&priv_dev->lock, flags);
/*
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] usb: cdns3: Fixed incorrect gadget state 2021-06-23 7:02 [PATCH] usb: cdns3: Fixed incorrect gadget state Pawel Laszczak @ 2021-06-26 8:56 ` Peter Chen 2021-06-28 3:46 ` Pawel Laszczak 0 siblings, 1 reply; 5+ messages in thread From: Peter Chen @ 2021-06-26 8:56 UTC (permalink / raw) To: Pawel Laszczak Cc: rogerq, a-govindraju, gregkh, felipe.balbi, linux-usb, linux-kernel, kishon, kurahul, sparmar, stable On 21-06-23 09:02:47, Pawel Laszczak wrote: > From: Pawel Laszczak <pawell@cadence.com> > > For delayed status phase, the usb_gadget->state was set > to USB_STATE_ADDRESS and it has never been updated to > USB_STATE_CONFIGURED. > Patch updates the gadget state to correct USB_STATE_CONFIGURED. > As a result of this bug the controller was not able to enter to > Test Mode while using MSC function. Pawel, would you please describe more about this issue? I remember the cdns3 controller at i.mx series SoC could enter test mode by using current code. Peter > > Cc: <stable@vger.kernel.org> > Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver") > Signed-off-by: Pawel Laszczak <pawell@cadence.com> > --- > drivers/usb/cdns3/cdns3-ep0.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/usb/cdns3/cdns3-ep0.c b/drivers/usb/cdns3/cdns3-ep0.c > index 9a17802275d5..ec5bfd8944c3 100644 > --- a/drivers/usb/cdns3/cdns3-ep0.c > +++ b/drivers/usb/cdns3/cdns3-ep0.c > @@ -731,6 +731,7 @@ static int cdns3_gadget_ep0_queue(struct usb_ep *ep, > request->actual = 0; > priv_dev->status_completion_no_call = true; > priv_dev->pending_status_request = request; > + usb_gadget_set_state(&priv_dev->gadget, USB_STATE_CONFIGURED); > spin_unlock_irqrestore(&priv_dev->lock, flags); > > /* > -- > 2.25.1 > -- Thanks, Peter Chen ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] usb: cdns3: Fixed incorrect gadget state 2021-06-26 8:56 ` Peter Chen @ 2021-06-28 3:46 ` Pawel Laszczak 2021-06-29 1:14 ` Peter Chen 0 siblings, 1 reply; 5+ messages in thread From: Pawel Laszczak @ 2021-06-28 3:46 UTC (permalink / raw) To: Peter Chen Cc: rogerq, a-govindraju, gregkh, felipe.balbi, linux-usb, linux-kernel, kishon, Rahul Kumar, Sanket Parmar, stable > >On 21-06-23 09:02:47, Pawel Laszczak wrote: >> From: Pawel Laszczak <pawell@cadence.com> >> >> For delayed status phase, the usb_gadget->state was set >> to USB_STATE_ADDRESS and it has never been updated to >> USB_STATE_CONFIGURED. >> Patch updates the gadget state to correct USB_STATE_CONFIGURED. >> As a result of this bug the controller was not able to enter to >> Test Mode while using MSC function. > >Pawel, would you please describe more about this issue? I remember the cdns3 >controller at i.mx series SoC could enter test mode by using current >code. The issue occurs only for MSC class. MSC class has delayed status stage, so after returning from cdns3_req_ep0_set_configuration function called for Set Configuration driver remains in USB_STATE_ADDRESS. In order to enter to test mode driver needs meet the condition included in cdns3_ep0_feature_handle_device function: if (state != USB_STATE_CONFIGURED || speed > USB_SPEED_HIGH) return -EINVAL; But it is still in USB_STATE_ADDRESS, because there was delayed status stage. To fix issue driver state must be updated to USB_STATE_CONFIGURED before or after finishing status stage. > >> >> Cc: <stable@vger.kernel.org> >> Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver") >> Signed-off-by: Pawel Laszczak <pawell@cadence.com> >> --- >> drivers/usb/cdns3/cdns3-ep0.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/usb/cdns3/cdns3-ep0.c b/drivers/usb/cdns3/cdns3-ep0.c >> index 9a17802275d5..ec5bfd8944c3 100644 >> --- a/drivers/usb/cdns3/cdns3-ep0.c >> +++ b/drivers/usb/cdns3/cdns3-ep0.c >> @@ -731,6 +731,7 @@ static int cdns3_gadget_ep0_queue(struct usb_ep *ep, >> request->actual = 0; >> priv_dev->status_completion_no_call = true; >> priv_dev->pending_status_request = request; >> + usb_gadget_set_state(&priv_dev->gadget, USB_STATE_CONFIGURED); >> spin_unlock_irqrestore(&priv_dev->lock, flags); >> >> /* >> -- >> 2.25.1 >> > -- Thanks, Pawel Laszczak ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] usb: cdns3: Fixed incorrect gadget state 2021-06-28 3:46 ` Pawel Laszczak @ 2021-06-29 1:14 ` Peter Chen 2021-06-29 4:14 ` Pawel Laszczak 0 siblings, 1 reply; 5+ messages in thread From: Peter Chen @ 2021-06-29 1:14 UTC (permalink / raw) To: Pawel Laszczak Cc: rogerq, a-govindraju, gregkh, felipe.balbi, linux-usb, linux-kernel, kishon, Rahul Kumar, Sanket Parmar, stable On 21-06-28 03:46:38, Pawel Laszczak wrote: > > > >On 21-06-23 09:02:47, Pawel Laszczak wrote: > >> From: Pawel Laszczak <pawell@cadence.com> > >> > >> For delayed status phase, the usb_gadget->state was set > >> to USB_STATE_ADDRESS and it has never been updated to > >> USB_STATE_CONFIGURED. > >> Patch updates the gadget state to correct USB_STATE_CONFIGURED. > >> As a result of this bug the controller was not able to enter to > >> Test Mode while using MSC function. > > > >Pawel, would you please describe more about this issue? I remember the cdns3 > >controller at i.mx series SoC could enter test mode by using current > >code. > > The issue occurs only for MSC class. MSC class has delayed status stage, so > after returning from cdns3_req_ep0_set_configuration function called > for Set Configuration driver remains in USB_STATE_ADDRESS. > > In order to enter to test mode driver needs meet the condition included in > cdns3_ep0_feature_handle_device function: > if (state != USB_STATE_CONFIGURED || speed > USB_SPEED_HIGH) > return -EINVAL; > > But it is still in USB_STATE_ADDRESS, because there was delayed status stage. > To fix issue driver state must be updated to USB_STATE_CONFIGURED before > or after finishing status stage. > I am wondering if the cdns3 driver set gadget state as USB_STATE_ADDRESS is correct for delayed status stage, since the composite core has already set it as USB_STATE_CONFIGURED at function set_config. Peter > > > >> > >> Cc: <stable@vger.kernel.org> > >> Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver") > >> Signed-off-by: Pawel Laszczak <pawell@cadence.com> > >> --- > >> drivers/usb/cdns3/cdns3-ep0.c | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/drivers/usb/cdns3/cdns3-ep0.c b/drivers/usb/cdns3/cdns3-ep0.c > >> index 9a17802275d5..ec5bfd8944c3 100644 > >> --- a/drivers/usb/cdns3/cdns3-ep0.c > >> +++ b/drivers/usb/cdns3/cdns3-ep0.c > >> @@ -731,6 +731,7 @@ static int cdns3_gadget_ep0_queue(struct usb_ep *ep, > >> request->actual = 0; > >> priv_dev->status_completion_no_call = true; > >> priv_dev->pending_status_request = request; > >> + usb_gadget_set_state(&priv_dev->gadget, USB_STATE_CONFIGURED); > >> spin_unlock_irqrestore(&priv_dev->lock, flags); > >> > >> /* > >> -- > >> 2.25.1 > >> > > > -- > > Thanks, > Pawel Laszczak > -- Thanks, Peter Chen ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] usb: cdns3: Fixed incorrect gadget state 2021-06-29 1:14 ` Peter Chen @ 2021-06-29 4:14 ` Pawel Laszczak 0 siblings, 0 replies; 5+ messages in thread From: Pawel Laszczak @ 2021-06-29 4:14 UTC (permalink / raw) To: Peter Chen Cc: rogerq, a-govindraju, gregkh, felipe.balbi, linux-usb, linux-kernel, balbi, Rahul Kumar, Sanket Parmar, stable, Brown, Zachary > >On 21-06-28 03:46:38, Pawel Laszczak wrote: >> > >> >On 21-06-23 09:02:47, Pawel Laszczak wrote: >> >> From: Pawel Laszczak <pawell@cadence.com> >> >> >> >> For delayed status phase, the usb_gadget->state was set >> >> to USB_STATE_ADDRESS and it has never been updated to >> >> USB_STATE_CONFIGURED. >> >> Patch updates the gadget state to correct USB_STATE_CONFIGURED. >> >> As a result of this bug the controller was not able to enter to >> >> Test Mode while using MSC function. >> > >> >Pawel, would you please describe more about this issue? I remember the cdns3 >> >controller at i.mx series SoC could enter test mode by using current >> >code. >> >> The issue occurs only for MSC class. MSC class has delayed status stage, so >> after returning from cdns3_req_ep0_set_configuration function called >> for Set Configuration driver remains in USB_STATE_ADDRESS. >> >> In order to enter to test mode driver needs meet the condition included in >> cdns3_ep0_feature_handle_device function: >> if (state != USB_STATE_CONFIGURED || speed > USB_SPEED_HIGH) >> return -EINVAL; >> >> But it is still in USB_STATE_ADDRESS, because there was delayed status stage. >> To fix issue driver state must be updated to USB_STATE_CONFIGURED before >> or after finishing status stage. >> > >I am wondering if the cdns3 driver set gadget state as USB_STATE_ADDRESS >is correct for delayed status stage, since the composite core has already >set it as USB_STATE_CONFIGURED at function set_config. Composite core set the driver to USB_STATE_CONFIGURED, then cdns3 for delayed status stage restore state to USB_STATE_ADDRESS, because the configuration has not been completed yet. However, after the configuration is successfully completed driver should set the state again to USB_STATE_CONFIGURED. The best place to do this is the completion event for status stage, but we haven't such event in this controller, so I've put it just after preparing controller for sending status stage. The same solution I found in e.g https://elixir.bootlin.com/linux/latest/source/drivers/usb/dwc3/ep0.c#L134 The other solution is to update gadget state in composite core before or after sending delayed status stage, but such solution will have impact for other UDC drivers . >> > >> >> >> >> Cc: <stable@vger.kernel.org> >> >> Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver") >> >> Signed-off-by: Pawel Laszczak <pawell@cadence.com> >> >> --- >> >> drivers/usb/cdns3/cdns3-ep0.c | 1 + >> >> 1 file changed, 1 insertion(+) >> >> >> >> diff --git a/drivers/usb/cdns3/cdns3-ep0.c b/drivers/usb/cdns3/cdns3-ep0.c >> >> index 9a17802275d5..ec5bfd8944c3 100644 >> >> --- a/drivers/usb/cdns3/cdns3-ep0.c >> >> +++ b/drivers/usb/cdns3/cdns3-ep0.c >> >> @@ -731,6 +731,7 @@ static int cdns3_gadget_ep0_queue(struct usb_ep *ep, >> >> request->actual = 0; >> >> priv_dev->status_completion_no_call = true; >> >> priv_dev->pending_status_request = request; >> >> + usb_gadget_set_state(&priv_dev->gadget, USB_STATE_CONFIGURED); >> >> spin_unlock_irqrestore(&priv_dev->lock, flags); >> >> >> >> /* >> >> -- >> >> 2.25.1 >> >> >> > >> -- >> >> Thanks, >> Pawel Laszczak >> > >-- > >Thanks, >Peter Chen -- Regards, Pawel Laszczak ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-06-29 4:14 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-23 7:02 [PATCH] usb: cdns3: Fixed incorrect gadget state Pawel Laszczak 2021-06-26 8:56 ` Peter Chen 2021-06-28 3:46 ` Pawel Laszczak 2021-06-29 1:14 ` Peter Chen 2021-06-29 4:14 ` Pawel Laszczak
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).