* [PATCH] usb: cdnsp: Fix incorrect status for control request
@ 2021-12-07 9:18 Pawel Laszczak
2021-12-09 11:34 ` Peter Chen
0 siblings, 1 reply; 6+ messages in thread
From: Pawel Laszczak @ 2021-12-07 9:18 UTC (permalink / raw)
To: peter.chen; +Cc: gregkh, linux-usb, linux-kernel, pawell, jianhe, stable
From: Pawel Laszczak <pawell@cadence.com>
Patch fixes incorrect status for control request.
Without this fix all usb_request objects were returned to upper drivers
with usb_reqest->status field set to -EINPROGRESS.
Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP DRD Driver")
Reported-by: Ken (Jian) He <jianhe@ambarella.com>
cc: <stable@vger.kernel.org>
Signed-off-by: Pawel Laszczak <pawell@cadence.com>
---
drivers/usb/cdns3/cdnsp-ring.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/usb/cdns3/cdnsp-ring.c b/drivers/usb/cdns3/cdnsp-ring.c
index 1b1438457fb0..e8f5ecbb5c75 100644
--- a/drivers/usb/cdns3/cdnsp-ring.c
+++ b/drivers/usb/cdns3/cdnsp-ring.c
@@ -1029,6 +1029,8 @@ static void cdnsp_process_ctrl_td(struct cdnsp_device *pdev,
return;
}
+ *status = 0;
+
cdnsp_finish_td(pdev, td, event, pep, status);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] usb: cdnsp: Fix incorrect status for control request
2021-12-07 9:18 [PATCH] usb: cdnsp: Fix incorrect status for control request Pawel Laszczak
@ 2021-12-09 11:34 ` Peter Chen
2021-12-09 12:57 ` Pawel Laszczak
0 siblings, 1 reply; 6+ messages in thread
From: Peter Chen @ 2021-12-09 11:34 UTC (permalink / raw)
To: Pawel Laszczak; +Cc: gregkh, linux-usb, linux-kernel, jianhe, stable
On 21-12-07 10:18:38, Pawel Laszczak wrote:
> From: Pawel Laszczak <pawell@cadence.com>
>
> Patch fixes incorrect status for control request.
> Without this fix all usb_request objects were returned to upper drivers
> with usb_reqest->status field set to -EINPROGRESS.
>
> Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP DRD Driver")
> Reported-by: Ken (Jian) He <jianhe@ambarella.com>
> cc: <stable@vger.kernel.org>
> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> ---
> drivers/usb/cdns3/cdnsp-ring.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/usb/cdns3/cdnsp-ring.c b/drivers/usb/cdns3/cdnsp-ring.c
> index 1b1438457fb0..e8f5ecbb5c75 100644
> --- a/drivers/usb/cdns3/cdnsp-ring.c
> +++ b/drivers/usb/cdns3/cdnsp-ring.c
> @@ -1029,6 +1029,8 @@ static void cdnsp_process_ctrl_td(struct cdnsp_device *pdev,
> return;
> }
>
> + *status = 0;
> +
> cdnsp_finish_td(pdev, td, event, pep, status);
> }
>
> --
I think you may move *status = 0 at the beginning of
cdnsp_process_ctrl_td in case you would like to handle some error
conditions during this function.
--
Thanks,
Peter Chen
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [PATCH] usb: cdnsp: Fix incorrect status for control request
2021-12-09 11:34 ` Peter Chen
@ 2021-12-09 12:57 ` Pawel Laszczak
2021-12-13 13:02 ` Peter Chen
0 siblings, 1 reply; 6+ messages in thread
From: Pawel Laszczak @ 2021-12-09 12:57 UTC (permalink / raw)
To: Peter Chen
Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org,
linux-kernel@vger.kernel.org, jianhe@ambarella.com,
stable@vger.kernel.org
>> From: Pawel Laszczak <pawell@cadence.com>
>>
>> Patch fixes incorrect status for control request.
>> Without this fix all usb_request objects were returned to upper drivers
>> with usb_reqest->status field set to -EINPROGRESS.
>>
>> Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP DRD Driver")
>> Reported-by: Ken (Jian) He <jianhe@ambarella.com>
>> cc: <stable@vger.kernel.org>
>> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
>> ---
>> drivers/usb/cdns3/cdnsp-ring.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/usb/cdns3/cdnsp-ring.c b/drivers/usb/cdns3/cdnsp-ring.c
>> index 1b1438457fb0..e8f5ecbb5c75 100644
>> --- a/drivers/usb/cdns3/cdnsp-ring.c
>> +++ b/drivers/usb/cdns3/cdnsp-ring.c
>> @@ -1029,6 +1029,8 @@ static void cdnsp_process_ctrl_td(struct cdnsp_device *pdev,
>> return;
>> }
>>
>> + *status = 0;
>> +
>> cdnsp_finish_td(pdev, td, event, pep, status);
>> }
>>
>> --
>I think you may move *status = 0 at the beginning of
>cdnsp_process_ctrl_td in case you would like to handle some error
>conditions during this function.
I don't predict any other status code for control request in this place.
I wanted to set this status only once after completion status stage. It was the reason why I put this
statement at the end of function.
--
Regards
Pawel Laszczak
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] usb: cdnsp: Fix incorrect status for control request
2021-12-09 12:57 ` Pawel Laszczak
@ 2021-12-13 13:02 ` Peter Chen
2021-12-13 13:17 ` Pawel Laszczak
0 siblings, 1 reply; 6+ messages in thread
From: Peter Chen @ 2021-12-13 13:02 UTC (permalink / raw)
To: Pawel Laszczak
Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org,
linux-kernel@vger.kernel.org, jianhe@ambarella.com,
stable@vger.kernel.org
On 21-12-09 12:57:31, Pawel Laszczak wrote:
> >> From: Pawel Laszczak <pawell@cadence.com>
> >>
> >> Patch fixes incorrect status for control request.
> >> Without this fix all usb_request objects were returned to upper drivers
> >> with usb_reqest->status field set to -EINPROGRESS.
> >>
> >> Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP DRD Driver")
> >> Reported-by: Ken (Jian) He <jianhe@ambarella.com>
> >> cc: <stable@vger.kernel.org>
> >> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> >> ---
> >> drivers/usb/cdns3/cdnsp-ring.c | 2 ++
> >> 1 file changed, 2 insertions(+)
> >>
> >> diff --git a/drivers/usb/cdns3/cdnsp-ring.c b/drivers/usb/cdns3/cdnsp-ring.c
> >> index 1b1438457fb0..e8f5ecbb5c75 100644
> >> --- a/drivers/usb/cdns3/cdnsp-ring.c
> >> +++ b/drivers/usb/cdns3/cdnsp-ring.c
> >> @@ -1029,6 +1029,8 @@ static void cdnsp_process_ctrl_td(struct cdnsp_device *pdev,
> >> return;
> >> }
> >>
> >> + *status = 0;
> >> +
> >> cdnsp_finish_td(pdev, td, event, pep, status);
> >> }
> >>
> >> --
> >I think you may move *status = 0 at the beginning of
> >cdnsp_process_ctrl_td in case you would like to handle some error
> >conditions during this function.
>
> I don't predict any other status code for control request in this place.
> I wanted to set this status only once after completion status stage. It was the reason why I put this
> statement at the end of function.
So, you always consider there is no error for control request handling?
--
Thanks,
Peter Chen
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [PATCH] usb: cdnsp: Fix incorrect status for control request
2021-12-13 13:02 ` Peter Chen
@ 2021-12-13 13:17 ` Pawel Laszczak
2021-12-13 13:31 ` Peter Chen
0 siblings, 1 reply; 6+ messages in thread
From: Pawel Laszczak @ 2021-12-13 13:17 UTC (permalink / raw)
To: Peter Chen
Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org,
linux-kernel@vger.kernel.org, jianhe@ambarella.com,
stable@vger.kernel.org
>
>On 21-12-09 12:57:31, Pawel Laszczak wrote:
>> >> From: Pawel Laszczak <pawell@cadence.com>
>> >>
>> >> Patch fixes incorrect status for control request.
>> >> Without this fix all usb_request objects were returned to upper drivers
>> >> with usb_reqest->status field set to -EINPROGRESS.
>> >>
>> >> Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP DRD Driver")
>> >> Reported-by: Ken (Jian) He <jianhe@ambarella.com>
>> >> cc: <stable@vger.kernel.org>
>> >> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
>> >> ---
>> >> drivers/usb/cdns3/cdnsp-ring.c | 2 ++
>> >> 1 file changed, 2 insertions(+)
>> >>
>> >> diff --git a/drivers/usb/cdns3/cdnsp-ring.c b/drivers/usb/cdns3/cdnsp-ring.c
>> >> index 1b1438457fb0..e8f5ecbb5c75 100644
>> >> --- a/drivers/usb/cdns3/cdnsp-ring.c
>> >> +++ b/drivers/usb/cdns3/cdnsp-ring.c
>> >> @@ -1029,6 +1029,8 @@ static void cdnsp_process_ctrl_td(struct cdnsp_device *pdev,
>> >> return;
>> >> }
>> >>
>> >> + *status = 0;
>> >> +
>> >> cdnsp_finish_td(pdev, td, event, pep, status);
>> >> }
>> >>
>> >> --
>> >I think you may move *status = 0 at the beginning of
>> >cdnsp_process_ctrl_td in case you would like to handle some error
>> >conditions during this function.
>>
>> I don't predict any other status code for control request in this place.
>> I wanted to set this status only once after completion status stage. It was the reason why I put this
>> statement at the end of function.
>
>So, you always consider there is no error for control request handling?
Not exactly. In this place driver handles only case when status stage has been completed,
so control transfer has been finished successfully.
Driver still take into account for control transfer such errors as: STALL, EINVAL and ECONNRESET but
in other parts of source code.
--
Regards,
Pawel Laszczak
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] usb: cdnsp: Fix incorrect status for control request
2021-12-13 13:17 ` Pawel Laszczak
@ 2021-12-13 13:31 ` Peter Chen
0 siblings, 0 replies; 6+ messages in thread
From: Peter Chen @ 2021-12-13 13:31 UTC (permalink / raw)
To: Pawel Laszczak
Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org,
linux-kernel@vger.kernel.org, jianhe@ambarella.com,
stable@vger.kernel.org
On 21-12-13 13:17:53, Pawel Laszczak wrote:
> >
> >On 21-12-09 12:57:31, Pawel Laszczak wrote:
> >> >> From: Pawel Laszczak <pawell@cadence.com>
> >> >>
> >> >> Patch fixes incorrect status for control request.
> >> >> Without this fix all usb_request objects were returned to upper drivers
> >> >> with usb_reqest->status field set to -EINPROGRESS.
> >> >>
> >> >> Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP DRD Driver")
> >> >> Reported-by: Ken (Jian) He <jianhe@ambarella.com>
> >> >> cc: <stable@vger.kernel.org>
> >> >> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> >> >> ---
> >> >> drivers/usb/cdns3/cdnsp-ring.c | 2 ++
> >> >> 1 file changed, 2 insertions(+)
> >> >>
> >> >> diff --git a/drivers/usb/cdns3/cdnsp-ring.c b/drivers/usb/cdns3/cdnsp-ring.c
> >> >> index 1b1438457fb0..e8f5ecbb5c75 100644
> >> >> --- a/drivers/usb/cdns3/cdnsp-ring.c
> >> >> +++ b/drivers/usb/cdns3/cdnsp-ring.c
> >> >> @@ -1029,6 +1029,8 @@ static void cdnsp_process_ctrl_td(struct cdnsp_device *pdev,
> >> >> return;
> >> >> }
> >> >>
> >> >> + *status = 0;
> >> >> +
> >> >> cdnsp_finish_td(pdev, td, event, pep, status);
> >> >> }
> >> >>
> >> >> --
> >> >I think you may move *status = 0 at the beginning of
> >> >cdnsp_process_ctrl_td in case you would like to handle some error
> >> >conditions during this function.
> >>
> >> I don't predict any other status code for control request in this place.
> >> I wanted to set this status only once after completion status stage. It was the reason why I put this
> >> statement at the end of function.
> >
> >So, you always consider there is no error for control request handling?
>
> Not exactly. In this place driver handles only case when status stage has been completed,
> so control transfer has been finished successfully.
>
> Driver still take into account for control transfer such errors as: STALL, EINVAL and ECONNRESET but
> in other parts of source code.
>
Reviewed-by: Peter Chen <peter.chen@kernel.org>
--
Thanks,
Peter Chen
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-12-13 13:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-12-07 9:18 [PATCH] usb: cdnsp: Fix incorrect status for control request Pawel Laszczak
2021-12-09 11:34 ` Peter Chen
2021-12-09 12:57 ` Pawel Laszczak
2021-12-13 13:02 ` Peter Chen
2021-12-13 13:17 ` Pawel Laszczak
2021-12-13 13:31 ` Peter Chen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox