Re: dwc3-qcom: tx-fifo-resize regression on Poco F1 (sdm845) with v5.15-rc1

From: Wesley Cheng
Date: Tue Sep 21 2021 - 03:32:45 EST


Hi Amit,

On 9/21/2021 12:04 AM, Amit Pundir wrote:
> Hi Wesley,
>
> On Tue, 21 Sept 2021 at 02:44, Wesley Cheng <wcheng@xxxxxxxxxxxxxx> wrote:
>>
>> Hi Amit,
>>
>> On 9/20/2021 1:45 PM, Amit Pundir wrote:
>>> Hi Wesley, All,
>>>
>>> I see a reboot loop on Xiaomi Pocophone F1 (sdm845) with TX FIFO
>>> resize patches which landed in v5.15-rc1. Upstream commit cefdd52fa045
>>> "usb: dwc3: dwc3-qcom: Enable tx-fifo-resize property by default" to
>>> be specific, which switched on this feature by default.
>>>
>>> At times the phone crashes into the fastboot mode after the reboot
>>> loop, but mostly end up booting to UI after a while. This is what it
>>> looks like https://people.linaro.org/~amit.pundir/beryllium-userdebug/PXL_20210920_162749483.mp4.
>>>
>>
>> I believe Android will attempt a number of bootup sequences and if it
>> fails, it falls back to fastboot mode. Are there any available logs you
>> might be able to collect to see where the issue is?
>
> It is a stock phone with no UART access, so I can't get early crash
> logs unless I'm booted up to adb shell. I can try getting some info
> using pstore-ramoops but warm reset support for sdm845 was not
> upstreamed when I tried using that the last time.
>

I see, can we maybe avoid the actual resizing by commenting out the
following writel() calls, but let the fifo resize logic calculate the EPs?

void dwc3_gadget_clear_tx_fifos(struct dwc3 *dwc)
{
...
/* Don't change TXFRAMNUM on usb31 version */
size = DWC3_IP_IS(DWC3) ? 0 :
dwc3_readl(dwc->regs, DWC3_GTXFIFOSIZ(num >> 1)) &
DWC31_GTXFIFOSIZ_TXFRAMNUM;
/* Comment the dwc3_writel() */
//dwc3_writel(dwc->regs, DWC3_GTXFIFOSIZ(num >> 1), size);

and

static int dwc3_gadget_resize_tx_fifos(struct dwc3_ep *dep)
{
...
/* Comment the dwc3_writel() */
//dwc3_writel(dwc->regs, DWC3_GTXFIFOSIZ(dep->number >> 1), fifo_size);
dwc->num_ep_resized++;

Those 2 writel() would be the one that actually programs the TXFIFO
register. I hope when commented out, no resize should actually happen
anymore.

With this, hopefully we can get some logs from the device at least :)

>>
>>> PocoF1 does support TX fifo resizing as I can see that in the
>>> downstream dts. So maybe it is the tx-fifo-max-num which need to be
>>> adjusted for the device? I couldn't find the tx-fifo-max-num
>>> equivalent in the downstream tree though
>>> https://github.com/MiCode/Xiaomi_Kernel_OpenSource/tree/dipper-q-oss/
>>>
>>
>> I assume that you've already confirmed reverting that change resolves
>> the constant reboots?
>
> Yes reverting that change resolves the reboot loop issue. Speaking of
> which, since no other platform seem to be running into this issue and
> "tx-fifo-max-num" property is apparently not at fault either, is it
> reasonable to skip adding "tx-fifo-resize" property for PocoF1 using
> of_machine_is_compatible("xiaomi,beryllium") as a workaround?
>

Since SDM845 does technically support txfifo resize downstream, let me
see if I can figure out what is different on this particular device
after getting the logs.

Thanks
Wesley Cheng

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project