Re: [PATCH v5 0/3] Add logic to consolidate TRBs for Synopsys xHC

[Tejas Joglekar <Tejas.Joglekar@synopsys.com>]

Hello Mathias,
On 11/12/2020 8:01 AM, Tejas Joglekar wrote:
> Hi,
> On 11/6/2020 9:36 PM, Tejas Joglekar wrote:
>> Hi Felipe,
>> On 10/28/2020 8:50 PM, Tejas Joglekar wrote:
>>> Hi,
>>> On 10/28/2020 8:12 PM, Mathias Nyman wrote:
>>>> On 27.10.2020 20.28, Tejas Joglekar wrote:
>>>>> Hi Mathias,
>>>>> On 10/16/2020 6:34 PM, Tejas Joglekar wrote:
>>>>>> The Synopsys xHC has an internal TRB cache of size TRB_CACHE_SIZE for
>>>>>> each endpoint. The default value for TRB_CACHE_SIZE is 16 for SS and 8
>>>>>> for HS. The controller loads and updates the TRB cache from the
>>>>>> transfer ring in system memory whenever the driver issues a start
>>>>>> transfer or update transfer command.
>>>>>>
>>>>>> For chained TRBs, the Synopsys xHC requires that the total amount of
>>>>>> bytes for all TRBs loaded in the TRB cache be greater than or equal to
>>>>>> 1 MPS. Or the chain ends within the TRB cache (with a last TRB).
>>>>>>
>>>>>> If this requirement is not met, the controller will not be able to
>>>>>> send or receive a packet and it will hang causing a driver timeout and
>>>>>> error.
>>>>>>
>>>>>> This patch set adds logic to the XHCI driver to detect and prevent this
>>>>>> from happening along with the quirk to enable this logic for Synopsys
>>>>>> HAPS platform.
>>>>>>
>>>>>> Based on Mathias's feedback on previous implementation where consolidation
>>>>>> was done in TRB cache, with this patch series the implementation is done
>>>>>> during mapping of the URB by consolidating the SG list into a temporary
>>>>>> buffer if the SG list buffer sizes within TRB_CACHE_SIZE is less than MPS.
>>>>>>
>>>>>> Changes since v4:
>>>>>>  - Updated [Patch 3/3] platform data structure initialization
>>>>>>  - Trivial changes in commit wordings
>>>>>>
>>>>>> Changes since v3:
>>>>>>  - Removed the dt-binding patch
>>>>>>  - Added new patch to pass the quirk as platform data
>>>>>>  - Modified the patch to set the quirk
>>>>>>
>>>>>> Changes since v2:
>>>>>>  - Modified the xhci_unmap_temp_buffer function to unmap dma and c
>>>>>>    the dma flag
>>>>>>  - Removed RFC tag
>>>>>>
>>>>>> Changes since v1:
>>>>>>  - Comments from Greg are addressed on [PATCH 4/4] and [PATCH 1/4]
>>>>>>  - Renamed the property and quirk as in other patches based on [PATCH 1/4]
>>>>>>
>>>>>>
>>>>>>
>>>>>> Tejas Joglekar (3):
>>>>>>   usb: xhci: Set quirk for XHCI_SG_TRB_CACHE_SIZE_QUIRK
>>>>>>   usb: xhci: Use temporary buffer to consolidate SG
>>>>>>   usb: dwc3: Pass quirk as platform data
>>>>>>
>>>>>>  drivers/usb/dwc3/host.c      |  10 +++
>>>>>>  drivers/usb/host/xhci-plat.c |   3 +
>>>>>>  drivers/usb/host/xhci-ring.c |   2 +-
>>>>>>  drivers/usb/host/xhci.c      | 137 ++++++++++++++++++++++++++++++++++-
>>>>>>  drivers/usb/host/xhci.h      |   5 ++
>>>>>>  5 files changed, 155 insertions(+), 2 deletions(-)
>>>>>>
>>>>> I have removed the dependency on setting quirk through device tree binding
>>>>> and added the quirk using platform data. Can you please review and if 
>>>>> everything looks OK, can you please add this patch series to your tree?
>>>> Sure, there aren't any functional changes to xhci since v3 of this series right?
>>>> And if I remember correctly Felipe didn't have any objections to the dwc3 part either.
>>>>
>>> You are right, there are no functional changes to xhci since v3.
>>>
>>>> Felipe, do you want to take the 3/3 dwc3 patch separately after 1/3 and 2/3 are in, or should
>>>> I take them all, or ask if Greg would like to pick up this series?
>>>>
@Mathias: Can you please take up 2 patches and may be Felipe can take up the dwc3 ?

It would help if this patch series goes in upcoming rc.

>> How would you like Mathias to go about merging the patch series?
> @Felipe: Any input on this ?
>
>
Thanks & Regards,

Tejas Joglekar