Re: [PATCH v4 6/6] usb:cdns3 Fix for stuck packets in on-chip OUT buffer.

2019-03-07 Thread Roger Quadros
On 07/03/2019 09:06, Pawel Laszczak wrote:
> Hi,
> 
>> Hi,
>>
>> On 21/02/2019 09:14, Felipe Balbi wrote:
>>>
>>> Hi,
>>>
>>> (please break your emails at 80-columns)
>>>
>>> Pawel Laszczak  writes:
>> One more thing. Workaround has implemented algorithm that decide for 
>> which
>> endpoint it should be enabled.  e.g for composite device MSC+NCM+ACM it
>> should work only for ACM OUT endpoint.
>>
>
> If ACM driver didn't queue the request for ACM OUT endpoint, why does the
> controller accept the data at all?
>
> I didn't understand why we need a workaround for this. It should be 
> standard
> behaviour to NAK data if function driver didn't request for all endpoints.

 Yes, I agree with you. Controller shouldn’t accept such packet. As I know 
 this
 behavior will be fixed in RTL.

 But I assume that some older version of this controller are one the market,
 and driver should work correct with them.

 In the feature this workaround can be limited only to selected controllers.

 Even now I assume that it can be enabled/disabled by module parameter.
>>>
>>> no module parameters, please. Use revision detection in runtime.
>>>
>>
>> This is about whether to enable or disable the workaround.
>> By default we don't want this workaround to be enabled.
>>
>> I'm debating whether we should have this workaround at all or not.
>>
>> It has the following problems.
>>
>> 1) It ACKs packets even when gadget end is not ready to accept the transfers.
>> 2) It stores these packets in a temporary buffer and then pushes them to the
>> gadget driver whenever the gadget driver is ready to process the data.
>> 3) Since the gadget driver can become ready at an indefinite time in the
>> future, it poses 2 problems:
>> a) It is sending stale data to the sink. (problematic at next protocol 
>> level?)
>> b) If this temporary buffer runs out we still hit the lock up issue.
>>
>> I think the right solution is to make sure that the gadget driver is always
>> reading all the enabled OUT endpoints *or* (keep the OUT endpoints disabled
>> if gadget driver is not ready to process OUT transfers).
> 
> If driver disable endpoint then it not answer for packets from host. 
> It will result that host reset the device, so I can't disable such endpoints. 
> 
> Other good solution is to change ACM driver in a way that it sends requests 
> to controller driver after enabling endpoint. The class driver could decide 

The ACM driver is doing exactly that. However, there is a large delay (depending
on when user opens the ttyACM) between endpoint enable and request queue and
that's the issue for this controller.

The endpoint is enabled whenever the host sends a SET_INTERFACE
[acm_set_alt()->gserial_connect()]
but the first request is queued only when user opens the ttyACM
[gs_open()->gs_start_io()->gs_start_rx()].

I'm don't think this sequence can be changed.
What happens if you defer gserial_connect() to be done at gs_open()?

> what should do with such not expected packets. It could delete all or e.g 
> keep only few last packets. 
> 
> This issue will be fixed in RTL but maybe driver should be compatible with 
> previous 
> controller’s version.
> 
> By default, this workaround will be disabled or will depend on controller 
> version.  
>>
> 
> Cheers,
> Pawel
> 

-- 
cheers,
-roger
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


RE: [PATCH v4 6/6] usb:cdns3 Fix for stuck packets in on-chip OUT buffer.

2019-03-06 Thread Pawel Laszczak
Hi,

>Hi,
>
>On 21/02/2019 09:14, Felipe Balbi wrote:
>>
>> Hi,
>>
>> (please break your emails at 80-columns)
>>
>> Pawel Laszczak  writes:
> One more thing. Workaround has implemented algorithm that decide for which
> endpoint it should be enabled.  e.g for composite device MSC+NCM+ACM it
> should work only for ACM OUT endpoint.
>

 If ACM driver didn't queue the request for ACM OUT endpoint, why does the
 controller accept the data at all?

 I didn't understand why we need a workaround for this. It should be 
 standard
 behaviour to NAK data if function driver didn't request for all endpoints.
>>>
>>> Yes, I agree with you. Controller shouldn’t accept such packet. As I know 
>>> this
>>> behavior will be fixed in RTL.
>>>
>>> But I assume that some older version of this controller are one the market,
>>> and driver should work correct with them.
>>>
>>> In the feature this workaround can be limited only to selected controllers.
>>>
>>> Even now I assume that it can be enabled/disabled by module parameter.
>>
>> no module parameters, please. Use revision detection in runtime.
>>
>
>This is about whether to enable or disable the workaround.
>By default we don't want this workaround to be enabled.
>
>I'm debating whether we should have this workaround at all or not.
>
>It has the following problems.
>
>1) It ACKs packets even when gadget end is not ready to accept the transfers.
>2) It stores these packets in a temporary buffer and then pushes them to the
>gadget driver whenever the gadget driver is ready to process the data.
>3) Since the gadget driver can become ready at an indefinite time in the
>future, it poses 2 problems:
> a) It is sending stale data to the sink. (problematic at next protocol level?)
> b) If this temporary buffer runs out we still hit the lock up issue.
>
>I think the right solution is to make sure that the gadget driver is always
>reading all the enabled OUT endpoints *or* (keep the OUT endpoints disabled
>if gadget driver is not ready to process OUT transfers).

If driver disable endpoint then it not answer for packets from host. 
It will result that host reset the device, so I can't disable such endpoints. 

Other good solution is to change ACM driver in a way that it sends requests 
to controller driver after enabling endpoint. The class driver could decide 
what should do with such not expected packets. It could delete all or e.g 
keep only few last packets. 

This issue will be fixed in RTL but maybe driver should be compatible with 
previous 
controller’s version.

By default, this workaround will be disabled or will depend on controller 
version.  
>
>cheers,
>-roger
>--
>Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Cheers,
Pawel


Re: [PATCH v4 6/6] usb:cdns3 Fix for stuck packets in on-chip OUT buffer.

2019-03-04 Thread Roger Quadros
Hi,

On 21/02/2019 09:14, Felipe Balbi wrote:
> 
> Hi,
> 
> (please break your emails at 80-columns)
> 
> Pawel Laszczak  writes:
 One more thing. Workaround has implemented algorithm that decide for which
 endpoint it should be enabled.  e.g for composite device MSC+NCM+ACM it
 should work only for ACM OUT endpoint.

>>>
>>> If ACM driver didn't queue the request for ACM OUT endpoint, why does the
>>> controller accept the data at all?
>>>
>>> I didn't understand why we need a workaround for this. It should be standard
>>> behaviour to NAK data if function driver didn't request for all endpoints.
>>
>> Yes, I agree with you. Controller shouldn’t accept such packet. As I know 
>> this
>> behavior will be fixed in RTL.
>>
>> But I assume that some older version of this controller are one the market,
>> and driver should work correct with them.
>>
>> In the feature this workaround can be limited only to selected controllers.
>>
>> Even now I assume that it can be enabled/disabled by module parameter.
> 
> no module parameters, please. Use revision detection in runtime.
> 

This is about whether to enable or disable the workaround.
By default we don't want this workaround to be enabled.

I'm debating whether we should have this workaround at all or not.

It has the following problems.

1) It ACKs packets even when gadget end is not ready to accept the transfers.
2) It stores these packets in a temporary buffer and then pushes them to the
gadget driver whenever the gadget driver is ready to process the data.
3) Since the gadget driver can become ready at an indefinite time in the
future, it poses 2 problems:
 a) It is sending stale data to the sink. (problematic at next protocol level?)
 b) If this temporary buffer runs out we still hit the lock up issue.

I think the right solution is to make sure that the gadget driver is always
reading all the enabled OUT endpoints *or* (keep the OUT endpoints disabled
if gadget driver is not ready to process OUT transfers).

cheers,
-roger
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


RE: [PATCH v4 6/6] usb:cdns3 Fix for stuck packets in on-chip OUT buffer.

2019-02-20 Thread Felipe Balbi

Hi,

(please break your emails at 80-columns)

Pawel Laszczak  writes:
>>> One more thing. Workaround has implemented algorithm that decide for which
>>> endpoint it should be enabled.  e.g for composite device MSC+NCM+ACM it
>>> should work only for ACM OUT endpoint.
>>>
>>
>>If ACM driver didn't queue the request for ACM OUT endpoint, why does the
>>controller accept the data at all?
>>
>>I didn't understand why we need a workaround for this. It should be standard
>>behaviour to NAK data if function driver didn't request for all endpoints.
>
> Yes, I agree with you. Controller shouldn’t accept such packet. As I know this
> behavior will be fixed in RTL.
>
> But I assume that some older version of this controller are one the market,
> and driver should work correct with them.
>
> In the feature this workaround can be limited only to selected controllers.
>
> Even now I assume that it can be enabled/disabled by module parameter.

no module parameters, please. Use revision detection in runtime.

-- 
balbi


signature.asc
Description: PGP signature


RE: [PATCH v4 6/6] usb:cdns3 Fix for stuck packets in on-chip OUT buffer.

2019-02-20 Thread Pawel Laszczak
>
>On 20/02/2019 13:18, Pawel Laszczak wrote:
>> Hi Roger.
>>>
>>> On 14/02/2019 21:45, Pawel Laszczak wrote:
 Controller for OUT endpoints has shared on-chip buffers for all incoming
 packets, including ep0out. It's FIFO buffer, so packets must be handle
 by DMA in correct order. If the first packet in the buffer will not be
 handled, then the following packets directed for other endpoints and
 functions will be blocked.

 Additionally the packets directed to one endpoint can block entire on-chip
 buffers. In this case transfer to other endpoints also will blocked.

 To resolve this issue after raising the descriptor missing interrupt
 driver prepares internal usb_request object and use it to arm DMA
 transfer.

 The problematic situation was observed in case when endpoint has
 been enabled but no usb_request were queued. Driver try detects
 such endpoints and will use this workaround only for these endpoint.

 Driver use limited number of buffer. This number can be set by macro
 CDNS_WA2_NUM_BUFFERS.

 Such blocking situation was observed on ACM gadget. For this function
 host send OUT data packet but ACM function is not prepared for
 this packet. It's cause that buffer placed in on chip memory block
 transfer to other endpoints.

 It's limitation of controller but maybe this issues should be fixed in
 function driver.

 This work around can be disabled/enabled by means of quirk_internal_buffer
 module parameter. By default feature is enabled. It can has impact to
 transfer performance and in most case this feature can be disabled.
>>>
>>> How much is the performance impact?
>>
>> I didn't check this, but performance will be decreased because driver has to
>> copy data from internally allocated buffer to usb_request->buff.
>>
>>> You mentioned that the issue was observed only in the ACM case and
>>> could be fixed in the function driver?
>>> It seems pointless to enable an endpoint and not have any requests queued 
>>> for it.
>>
>> Yes, it’s true. The request in ACM class is send to Controller Driver when 
>> user read file associated
>> with ACM gadget. Problem exist because host send data to controller but USB 
>> controller
>> has not prepared buffer for this data by ACM class.
>>
>>> Isn't fixing the ACM driver (if there is a real issue) a better approach 
>>> than having
>>> a workaround that affects performance of all other use cases?
>>
>> Yes it should be better. But what ACM driver should do with unexpected data. 
>> I'm not sure if we
>> can simply delete them.
>>
>> The best solution would be to make modification in controller. In such case 
>> Controller shouldn't accept
>> packet but should send NAK.
>
>Yes, that should be standard behaviour. No?
>
>>
>> One more thing. Workaround has implemented algorithm that decide for which 
>> endpoint it should be enabled.
>> e.g for composite device MSC+NCM+ACM it should work only for ACM OUT 
>> endpoint.
>>
>
>If ACM driver didn't queue the request for ACM OUT endpoint, why does the 
>controller accept the data at all?
>I didn't understand why we need a workaround for this. It should be standard 
>behaviour to NAK data if
>function driver didn't request for all endpoints.

Yes, I agree with you. Controller shouldn’t accept such packet. As I know this 
behavior will be fixed in RTL. 
But I assume that some older version of this controller are one the market, and 
driver should work correct with them.
In the feature this workaround can be limited only to selected controllers. 
Even now I assume that it can be enabled/disabled by module parameter. 

>
>Also I didn't understand why performance should be impacted to just NAK data.

Data waiting in on-chip buffers can be very fast copied to system memory when 
DMA will be armed.
In same case this feature can little increase performance, but it makes many 
problems under OS.

Cheers,
Pawel


>cheers,
>-roger
>
>> Cheers,
>> Pawel
>>>

 Signed-off-by: Pawel Laszczak 
 ---
  drivers/usb/cdns3/gadget.c | 273 -
  drivers/usb/cdns3/gadget.h |   7 +
  2 files changed, 278 insertions(+), 2 deletions(-)

 diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
 index 7f7f24ee3c4b..5dfbe6e1421c 100644
 --- a/drivers/usb/cdns3/gadget.c
 +++ b/drivers/usb/cdns3/gadget.c
 @@ -27,6 +27,37 @@
   * If (((Dequeue Ptr (i.e. EP_TRADDR) == Enqueue Ptr-1) or
   *(Dequeue Ptr (i.e. EP_TRADDR) == Enqueue Ptr))
   *and (DRBL==1 and (not EP0)))
 + *
 + * Work around 2:
 + * Controller for OUT endpoints has shared on-chip buffers for all 
 incoming
 + * packets, including ep0out. It's FIFO buffer, so packets must be handle 
 by DMA
 + * in correct order. If the first packet in the buffer will not be 
 handled,
 + * then the following packets 

Re: [PATCH v4 6/6] usb:cdns3 Fix for stuck packets in on-chip OUT buffer.

2019-02-20 Thread Roger Quadros
Pawel,

On 20/02/2019 13:18, Pawel Laszczak wrote:
> Hi Roger.
>>
>> On 14/02/2019 21:45, Pawel Laszczak wrote:
>>> Controller for OUT endpoints has shared on-chip buffers for all incoming
>>> packets, including ep0out. It's FIFO buffer, so packets must be handle
>>> by DMA in correct order. If the first packet in the buffer will not be
>>> handled, then the following packets directed for other endpoints and
>>> functions will be blocked.
>>>
>>> Additionally the packets directed to one endpoint can block entire on-chip
>>> buffers. In this case transfer to other endpoints also will blocked.
>>>
>>> To resolve this issue after raising the descriptor missing interrupt
>>> driver prepares internal usb_request object and use it to arm DMA
>>> transfer.
>>>
>>> The problematic situation was observed in case when endpoint has
>>> been enabled but no usb_request were queued. Driver try detects
>>> such endpoints and will use this workaround only for these endpoint.
>>>
>>> Driver use limited number of buffer. This number can be set by macro
>>> CDNS_WA2_NUM_BUFFERS.
>>>
>>> Such blocking situation was observed on ACM gadget. For this function
>>> host send OUT data packet but ACM function is not prepared for
>>> this packet. It's cause that buffer placed in on chip memory block
>>> transfer to other endpoints.
>>>
>>> It's limitation of controller but maybe this issues should be fixed in
>>> function driver.
>>>
>>> This work around can be disabled/enabled by means of quirk_internal_buffer
>>> module parameter. By default feature is enabled. It can has impact to
>>> transfer performance and in most case this feature can be disabled.
>>
>> How much is the performance impact?
> 
> I didn't check this, but performance will be decreased because driver has to 
> copy data from internally allocated buffer to usb_request->buff.
> 
>> You mentioned that the issue was observed only in the ACM case and
>> could be fixed in the function driver?
>> It seems pointless to enable an endpoint and not have any requests queued 
>> for it.
> 
> Yes, it’s true. The request in ACM class is send to Controller Driver when 
> user read file associated 
> with ACM gadget. Problem exist because host send data to controller but USB 
> controller 
> has not prepared buffer for this data by ACM class.
> 
>> Isn't fixing the ACM driver (if there is a real issue) a better approach 
>> than having
>> a workaround that affects performance of all other use cases?
> 
> Yes it should be better. But what ACM driver should do with unexpected data. 
> I'm not sure if we 
> can simply delete them. 
> 
> The best solution would be to make modification in controller. In such case 
> Controller shouldn't accept 
> packet but should send NAK. 

Yes, that should be standard behaviour. No?

>  
> One more thing. Workaround has implemented algorithm that decide for which 
> endpoint it should be enabled.
> e.g for composite device MSC+NCM+ACM it should work only for ACM OUT endpoint.
> 

If ACM driver didn't queue the request for ACM OUT endpoint, why does the 
controller accept the data at all?
I didn't understand why we need a workaround for this. It should be standard 
behaviour to NAK data if
function driver didn't request for all endpoints.

Also I didn't understand why performance should be impacted to just NAK data.

cheers,
-roger

> Cheers,
> Pawel
>>
>>>
>>> Signed-off-by: Pawel Laszczak 
>>> ---
>>>  drivers/usb/cdns3/gadget.c | 273 -
>>>  drivers/usb/cdns3/gadget.h |   7 +
>>>  2 files changed, 278 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
>>> index 7f7f24ee3c4b..5dfbe6e1421c 100644
>>> --- a/drivers/usb/cdns3/gadget.c
>>> +++ b/drivers/usb/cdns3/gadget.c
>>> @@ -27,6 +27,37 @@
>>>   * If (((Dequeue Ptr (i.e. EP_TRADDR) == Enqueue Ptr-1) or
>>>   * (Dequeue Ptr (i.e. EP_TRADDR) == Enqueue Ptr))
>>>   * and (DRBL==1 and (not EP0)))
>>> + *
>>> + * Work around 2:
>>> + * Controller for OUT endpoints has shared on-chip buffers for all incoming
>>> + * packets, including ep0out. It's FIFO buffer, so packets must be handle 
>>> by DMA
>>> + * in correct order. If the first packet in the buffer will not be handled,
>>> + * then the following packets directed for other endpoints and  functions
>>> + * will be blocked.
>>> + * Additionally the packets directed to one endpoint can block entire 
>>> on-chip
>>> + * buffers. In this case transfer to other endpoints also will blocked.
>>> + *
>>> + * To resolve this issue after raising the descriptor missing interrupt
>>> + * driver prepares internal usb_request object and use it to arm DMA 
>>> transfer.
>>> + *
>>> + * The problematic situation was observed in case when endpoint has been 
>>> enabled
>>> + * but no usb_request were queued. Driver try detects such endpoints and 
>>> will
>>> + * use this workaround only for these endpoint.
>>> + *
>>> + * Driver use limited number of buffer. This num

RE: [PATCH v4 6/6] usb:cdns3 Fix for stuck packets in on-chip OUT buffer.

2019-02-20 Thread Pawel Laszczak
Hi Roger.
>
>On 14/02/2019 21:45, Pawel Laszczak wrote:
>> Controller for OUT endpoints has shared on-chip buffers for all incoming
>> packets, including ep0out. It's FIFO buffer, so packets must be handle
>> by DMA in correct order. If the first packet in the buffer will not be
>> handled, then the following packets directed for other endpoints and
>> functions will be blocked.
>>
>> Additionally the packets directed to one endpoint can block entire on-chip
>> buffers. In this case transfer to other endpoints also will blocked.
>>
>> To resolve this issue after raising the descriptor missing interrupt
>> driver prepares internal usb_request object and use it to arm DMA
>> transfer.
>>
>> The problematic situation was observed in case when endpoint has
>> been enabled but no usb_request were queued. Driver try detects
>> such endpoints and will use this workaround only for these endpoint.
>>
>> Driver use limited number of buffer. This number can be set by macro
>> CDNS_WA2_NUM_BUFFERS.
>>
>> Such blocking situation was observed on ACM gadget. For this function
>> host send OUT data packet but ACM function is not prepared for
>> this packet. It's cause that buffer placed in on chip memory block
>> transfer to other endpoints.
>>
>> It's limitation of controller but maybe this issues should be fixed in
>> function driver.
>>
>> This work around can be disabled/enabled by means of quirk_internal_buffer
>> module parameter. By default feature is enabled. It can has impact to
>> transfer performance and in most case this feature can be disabled.
>
>How much is the performance impact?

I didn't check this, but performance will be decreased because driver has to 
copy data from internally allocated buffer to usb_request->buff.

>You mentioned that the issue was observed only in the ACM case and
>could be fixed in the function driver?
>It seems pointless to enable an endpoint and not have any requests queued for 
>it.

Yes, it’s true. The request in ACM class is send to Controller Driver when user 
read file associated 
with ACM gadget. Problem exist because host send data to controller but USB 
controller 
has not prepared buffer for this data by ACM class.

>Isn't fixing the ACM driver (if there is a real issue) a better approach than 
>having
>a workaround that affects performance of all other use cases?

Yes it should be better. But what ACM driver should do with unexpected data. 
I'm not sure if we 
can simply delete them. 

The best solution would be to make modification in controller. In such case 
Controller shouldn't accept 
packet but should send NAK. 
 
One more thing. Workaround has implemented algorithm that decide for which 
endpoint it should be enabled.
e.g for composite device MSC+NCM+ACM it should work only for ACM OUT endpoint.

Cheers,
Pawel
>
>>
>> Signed-off-by: Pawel Laszczak 
>> ---
>>  drivers/usb/cdns3/gadget.c | 273 -
>>  drivers/usb/cdns3/gadget.h |   7 +
>>  2 files changed, 278 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
>> index 7f7f24ee3c4b..5dfbe6e1421c 100644
>> --- a/drivers/usb/cdns3/gadget.c
>> +++ b/drivers/usb/cdns3/gadget.c
>> @@ -27,6 +27,37 @@
>>   * If (((Dequeue Ptr (i.e. EP_TRADDR) == Enqueue Ptr-1) or
>>   *  (Dequeue Ptr (i.e. EP_TRADDR) == Enqueue Ptr))
>>   *  and (DRBL==1 and (not EP0)))
>> + *
>> + * Work around 2:
>> + * Controller for OUT endpoints has shared on-chip buffers for all incoming
>> + * packets, including ep0out. It's FIFO buffer, so packets must be handle 
>> by DMA
>> + * in correct order. If the first packet in the buffer will not be handled,
>> + * then the following packets directed for other endpoints and  functions
>> + * will be blocked.
>> + * Additionally the packets directed to one endpoint can block entire 
>> on-chip
>> + * buffers. In this case transfer to other endpoints also will blocked.
>> + *
>> + * To resolve this issue after raising the descriptor missing interrupt
>> + * driver prepares internal usb_request object and use it to arm DMA 
>> transfer.
>> + *
>> + * The problematic situation was observed in case when endpoint has been 
>> enabled
>> + * but no usb_request were queued. Driver try detects such endpoints and 
>> will
>> + * use this workaround only for these endpoint.
>> + *
>> + * Driver use limited number of buffer. This number can be set by macro
>> + * CDNS_WA2_NUM_BUFFERS.
>> + *
>> + * Such blocking situation was observed on ACM gadget. For this function
>> + * host send OUT data packet but ACM function is not prepared for this 
>> packet.
>> + * It's cause that buffer placed in on chip memory block transfer to other
>> + * endpoints.
>> + *
>> + * It's limitation of controller but maybe this issues should be fixed in
>> + * function driver.
>> + *
>> + * This work around can be disabled/enabled by means of 
>> quirk_internal_buffer
>> + * module parameter. By default feature is enabled. It can has impact to

Re: [PATCH v4 6/6] usb:cdns3 Fix for stuck packets in on-chip OUT buffer.

2019-02-20 Thread Roger Quadros
Pawel,

On 14/02/2019 21:45, Pawel Laszczak wrote:
> Controller for OUT endpoints has shared on-chip buffers for all incoming
> packets, including ep0out. It's FIFO buffer, so packets must be handle
> by DMA in correct order. If the first packet in the buffer will not be
> handled, then the following packets directed for other endpoints and
> functions will be blocked.
> 
> Additionally the packets directed to one endpoint can block entire on-chip
> buffers. In this case transfer to other endpoints also will blocked.
> 
> To resolve this issue after raising the descriptor missing interrupt
> driver prepares internal usb_request object and use it to arm DMA
> transfer.
> 
> The problematic situation was observed in case when endpoint has
> been enabled but no usb_request were queued. Driver try detects
> such endpoints and will use this workaround only for these endpoint.
> 
> Driver use limited number of buffer. This number can be set by macro
> CDNS_WA2_NUM_BUFFERS.
> 
> Such blocking situation was observed on ACM gadget. For this function
> host send OUT data packet but ACM function is not prepared for
> this packet. It's cause that buffer placed in on chip memory block
> transfer to other endpoints.
> 
> It's limitation of controller but maybe this issues should be fixed in
> function driver.
> 
> This work around can be disabled/enabled by means of quirk_internal_buffer
> module parameter. By default feature is enabled. It can has impact to
> transfer performance and in most case this feature can be disabled.

How much is the performance impact?

You mentioned that the issue was observed only in the ACM case and
could be fixed in the function driver?
It seems pointless to enable an endpoint and not have any requests queued for 
it.

Isn't fixing the ACM driver (if there is a real issue) a better approach than 
having
a workaround that affects performance of all other use cases?

cheers,
-roger

> 
> Signed-off-by: Pawel Laszczak 
> ---
>  drivers/usb/cdns3/gadget.c | 273 -
>  drivers/usb/cdns3/gadget.h |   7 +
>  2 files changed, 278 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
> index 7f7f24ee3c4b..5dfbe6e1421c 100644
> --- a/drivers/usb/cdns3/gadget.c
> +++ b/drivers/usb/cdns3/gadget.c
> @@ -27,6 +27,37 @@
>   * If (((Dequeue Ptr (i.e. EP_TRADDR) == Enqueue Ptr-1) or
>   *   (Dequeue Ptr (i.e. EP_TRADDR) == Enqueue Ptr))
>   *   and (DRBL==1 and (not EP0)))
> + *
> + * Work around 2:
> + * Controller for OUT endpoints has shared on-chip buffers for all incoming
> + * packets, including ep0out. It's FIFO buffer, so packets must be handle by 
> DMA
> + * in correct order. If the first packet in the buffer will not be handled,
> + * then the following packets directed for other endpoints and  functions
> + * will be blocked.
> + * Additionally the packets directed to one endpoint can block entire on-chip
> + * buffers. In this case transfer to other endpoints also will blocked.
> + *
> + * To resolve this issue after raising the descriptor missing interrupt
> + * driver prepares internal usb_request object and use it to arm DMA 
> transfer.
> + *
> + * The problematic situation was observed in case when endpoint has been 
> enabled
> + * but no usb_request were queued. Driver try detects such endpoints and will
> + * use this workaround only for these endpoint.
> + *
> + * Driver use limited number of buffer. This number can be set by macro
> + * CDNS_WA2_NUM_BUFFERS.
> + *
> + * Such blocking situation was observed on ACM gadget. For this function
> + * host send OUT data packet but ACM function is not prepared for this 
> packet.
> + * It's cause that buffer placed in on chip memory block transfer to other
> + * endpoints.
> + *
> + * It's limitation of controller but maybe this issues should be fixed in
> + * function driver.
> + *
> + * This work around can be disabled/enabled by means of quirk_internal_buffer
> + * module parameter. By default feature is enabled. It can has impact to
> + * transfer performance and in most case this feature can be disabled.
>   */
>  
>  #include 
> @@ -42,6 +73,14 @@ static int __cdns3_gadget_ep_queue(struct usb_ep *ep,
>  struct usb_request *request,
>  gfp_t gfp_flags);
>  
> +/*
> + * Parameter allows to disable/enable handling of work around 2 feature.
> + * By default this value is enabled.
> + */
> +static bool quirk_internal_buffer = 1;
> +module_param(quirk_internal_buffer, bool, 0644);
> +MODULE_PARM_DESC(quirk_internal_buffer, "Disable/enable WA2 algorithm");
> +
>  /**
>   * cdns3_handshake - spin reading  until handshake completes or fails
>   * @ptr: address of device controller register to be read
> @@ -105,6 +144,17 @@ struct usb_request *cdns3_next_request(struct list_head 
> *list)
>   return list_first_entry_or_null(list, struct usb_request, list);
>  }
>  
> +/**
> + * cdns3_nex