Re: [PATCH v2] usb: xhci: add debugfs support for ep with stream

[Mathias Nyman <mathias.nyman@linux.intel.com>]

On 3.9.2020 10.46, Jun Li wrote:
> 
>> -----Original Message-----
>> From: Mathias Nyman <mathias.nyman@linux.intel.com>
>> Sent: Thursday, September 3, 2020 3:24 PM
>> To: Jun Li <jun.li@nxp.com>; mathias.nyman@intel.com
>> Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; dl-linux-imx
>> <linux-imx@nxp.com>
>> Subject: Re: [PATCH v2] usb: xhci: add debugfs support for ep with stream
>>
>>>> I think this debugfs code is just called too early. It shouldn't need
>>>> to check new_ring pointer at all.
>>>>
>>>> I wrote a fix that changes the order and makes sure endpoint is
>>>> enabled and ring pointer is set correctly before we call
>>>> xhci_debugfs_create_endpoint()
>>>>
>>>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit
>>>> .kernel.o
>>>> rg%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fmnyman%2Fxhci.git%2Fcommit%2F
>>>> %3Fh%3Dfo
>>>> r-usb-linus%26id%3Dcf99aef5624a592fd4f3374c7060bfa1a65f15df&amp;data=
>>>> 02%7C01%7
>>>> Cjun.li%40nxp.com%7C73e4663a6f6641fbb8b308d84f3d021a%7C686ea1d3bc2b4c
>>>> 6fa92cd99
>>>> c5c301635%7C0%7C0%7C637346470803922895&amp;sdata=i4DfCW8EVUSAWnzb8Ql4
>>>> jPjAOD5wp
>>>> tfbaMrO8vKvtDc%3D&amp;reserved=0
>>>
>>> This is a good place for non-zero Eps, but does not cover ep0.
>>>
>>
>> ep0 is special, it's not touched in these add/drop endpoint or check bandwidth
>> functions.
>>
>> ep0 ring is allocated earlier during slot creation in
>> xhci_alloc_virt_device()
>>   ...
>>   /* Allocate endpoint 0 ring */
>>   dev->eps[0].ring = xhci_ring_alloc(xhci, 2, 1, TYPE_CTRL, 0, flags);
>>
>> and for debugfs ep00 is added manually together with the slot
>> xhci_debugfs_create_slot()
>>   ...
>>   xhci_debugfs_create_ring_dir(xhci, &dev->eps[0].ring, "ep00", priv->root);
>>
>> So regarding ep0 the change should be ok.
> 
> Sorry, I forgot debugfs of ep0 is created via xhci_debugfs_create_slot().
> 
> Then I think your change is OK, also I gave a test with my stream/UAS device
> on top of your patch and it can work fine.
> 
> Do you need I post a new version of my patch(to remove touch of .new_ring)?

If you could yes, and also change to a double pointer:

 struct xhci_ep_priv {
	...
+	struct xhci_ring	**show_ring;
 };

And modify other places this change affects, such as: 
epriv->show_ring = &dev->eps[ep_index].ring;

-Mathias