From: Pawel Laszczak <[email protected]>
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 <[email protected]>
cc: <[email protected]>
Signed-off-by: Pawel Laszczak <[email protected]>
---
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
On 21-12-07 10:18:38, Pawel Laszczak wrote:
> From: Pawel Laszczak <[email protected]>
>
> 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 <[email protected]>
> cc: <[email protected]>
> Signed-off-by: Pawel Laszczak <[email protected]>
> ---
> 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
>> From: Pawel Laszczak <[email protected]>
>>
>> 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 <[email protected]>
>> cc: <[email protected]>
>> Signed-off-by: Pawel Laszczak <[email protected]>
>> ---
>> 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
On 21-12-09 12:57:31, Pawel Laszczak wrote:
> >> From: Pawel Laszczak <[email protected]>
> >>
> >> 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 <[email protected]>
> >> cc: <[email protected]>
> >> Signed-off-by: Pawel Laszczak <[email protected]>
> >> ---
> >> 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
>
>On 21-12-09 12:57:31, Pawel Laszczak wrote:
>> >> From: Pawel Laszczak <[email protected]>
>> >>
>> >> 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 <[email protected]>
>> >> cc: <[email protected]>
>> >> Signed-off-by: Pawel Laszczak <[email protected]>
>> >> ---
>> >> 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
On 21-12-13 13:17:53, Pawel Laszczak wrote:
> >
> >On 21-12-09 12:57:31, Pawel Laszczak wrote:
> >> >> From: Pawel Laszczak <[email protected]>
> >> >>
> >> >> 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 <[email protected]>
> >> >> cc: <[email protected]>
> >> >> Signed-off-by: Pawel Laszczak <[email protected]>
> >> >> ---
> >> >> 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 <[email protected]>
--
Thanks,
Peter Chen