[PATCH 0/3] xhci: Some improvement for Etron xHCI host

Kuangyi Chiang posted 3 patches 2 months, 2 weeks ago
Only 0 patches received!
There is a newer version of this series
drivers/usb/host/xhci-pci.c  |  6 ++++++
drivers/usb/host/xhci-ring.c | 13 +++++++++++++
drivers/usb/host/xhci.c      | 19 +++++++++++++++++++
drivers/usb/host/xhci.h      |  2 ++
4 files changed, 40 insertions(+)
[PATCH 0/3] xhci: Some improvement for Etron xHCI host
Posted by Kuangyi Chiang 2 months, 2 weeks ago
Add two new quirks XHCI_NO_RESET_DEVICE/XHCI_NO_BREAK_CTRL_TD to
invoke the workaround:
 xhci: Don't issue Reset Device command to Etron xHCI host
 xhci: Fix control transfer error on Etron xHCI host

Apply quirk XHCI_NO_SOFT_RETRY to disable Soft Retry:
 xhci: Don't perform Soft Retry for Etron xHCI host

Kuangyi Chiang (3):
  xhci: Don't issue Reset Device command to Etron xHCI host
  xhci: Fix control transfer error on Etron xHCI host
  xhci: Don't perform Soft Retry for Etron xHCI host

 drivers/usb/host/xhci-pci.c  |  6 ++++++
 drivers/usb/host/xhci-ring.c | 13 +++++++++++++
 drivers/usb/host/xhci.c      | 19 +++++++++++++++++++
 drivers/usb/host/xhci.h      |  2 ++
 4 files changed, 40 insertions(+)

-- 
2.25.1
Re: [PATCH 0/3] xhci: Some improvement for Etron xHCI host
Posted by Michał Pecio 2 months, 2 weeks ago
Hi,

I have some Etron controller (forgot which one) but I'm not using it
because it crashes ("dies") all the time under my workloads.

I suppose I could try your patches if I find a moment for it.

I'm aware of one more bug which affects my Etron: if an error occurs
on an isochronous TD, two events are generated: first the error, then
"success", even if the error is on the final TRB (the common case).
Then the "success" causes "TRB DMA not part of current TD" warning.
I suspect that all Etron chips are the same. This should be easily
reproducible by unpligging an audio/video device while streaming.

Considering how utterly broken this hardware is, I think it could be
more efficient to have a single "Etron host" quirk. These bugs are
so stupid that it seems unlikely that any of Etron quirks would ever
be reused on other hardware. Of course it should still use "general"
quirks when applicable, such as "broken streams", which it does IIRC.

Regards,
Michal
Re: [PATCH 0/3] xhci: Some improvement for Etron xHCI host
Posted by Kuangyi Chiang 2 months, 2 weeks ago
Hi,

Thank you for the review.

Michał Pecio <michal.pecio@gmail.com> 於 2024年9月11日 週三 下午3:38寫道:
>
> Hi,
>
> I have some Etron controller (forgot which one) but I'm not using it
> because it crashes ("dies") all the time under my workloads.
>
> I suppose I could try your patches if I find a moment for it.
>
> I'm aware of one more bug which affects my Etron: if an error occurs
> on an isochronous TD, two events are generated: first the error, then
> "success", even if the error is on the final TRB (the common case).
> Then the "success" causes "TRB DMA not part of current TD" warning.
> I suspect that all Etron chips are the same. This should be easily
> reproducible by unpligging an audio/video device while streaming.

Hmm, I don't encounter this problem.

>
> Considering how utterly broken this hardware is, I think it could be
> more efficient to have a single "Etron host" quirk. These bugs are
> so stupid that it seems unlikely that any of Etron quirks would ever
> be reused on other hardware. Of course it should still use "general"
> quirks when applicable, such as "broken streams", which it does IIRC.
>

Ok, I will use one quirk XHCI_ETRON_HOST for these workarounds in the
next patch revision.

> Regards,
> Michal

Thanks,
Kuangyi Chiang
Re: [PATCH 0/3] xhci: Some improvement for Etron xHCI host
Posted by Michał Pecio 2 months, 2 weeks ago
Hi,

> > I'm aware of one more bug which affects my Etron: if an error occurs
> > on an isochronous TD, two events are generated: first the error,
> > then "success", even if the error is on the final TRB (the common
> > case). Then the "success" causes "TRB DMA not part of current TD"
> > warning. I suspect that all Etron chips are the same. This should
> > be easily reproducible by unpligging an audio/video device while
> > streaming.  
> 
> Hmm, I don't encounter this problem.

OK, I know what happened. This bug only affects SuperSpeed isochronous
endpoints. If you don't have this kind of device, you will not see it.
I checked that High-speed isochronous errors are reported correctly.

My motivation to develop a workaround for this bug has just decreased
another notch.


On the other hand, I was unable to reproduce the control transfer bug.
The exact chip I have is labeled "EtronTech EJ168A", for the record.

You are right, not all transfers have the data stage and transactions
get out of sync with segment boundaries. I modified the patch to only
print a warning instead of queuing a No-Op and then did various things
which use control transactions: setting baud rate on serial, changing
the volume on audio, starting video recording on a webcam, running
ethtool on a NIC.

The warning was printed a few times, but nothing interesting happened.
Dynamic debug was enabled on handle_tx_event() - no errors reported.

Maybe a different silicon/firmware revision, or maybe it's another
SuperSpeed-only bug, or other special conditions for it to happen?

> Ok, I will use one quirk XHCI_ETRON_HOST for these workarounds in the
> next patch revision.
That was just a suggestion, you should ask Mathias Nyman, I suppose.

But, again, my impression of this hardware is that it's pretty bad
and full of bugs, and they are bizarre enough to likely be unique.

Regards,
Michal
Re: [PATCH 0/3] xhci: Some improvement for Etron xHCI host
Posted by Kuangyi Chiang 2 months, 2 weeks ago
Hi,

Thank you for testing the patch.

Michał Pecio <michal.pecio@gmail.com> 於 2024年9月12日 週四 下午3:12寫道:
>
> Hi,
>
> > > I'm aware of one more bug which affects my Etron: if an error occurs
> > > on an isochronous TD, two events are generated: first the error,
> > > then "success", even if the error is on the final TRB (the common
> > > case). Then the "success" causes "TRB DMA not part of current TD"
> > > warning. I suspect that all Etron chips are the same. This should
> > > be easily reproducible by unpligging an audio/video device while
> > > streaming.
> >
> > Hmm, I don't encounter this problem.
>
> OK, I know what happened. This bug only affects SuperSpeed isochronous
> endpoints. If you don't have this kind of device, you will not see it.
> I checked that High-speed isochronous errors are reported correctly.
>
> My motivation to develop a workaround for this bug has just decreased
> another notch.
>
>
> On the other hand, I was unable to reproduce the control transfer bug.
> The exact chip I have is labeled "EtronTech EJ168A", for the record.
>
> You are right, not all transfers have the data stage and transactions
> get out of sync with segment boundaries. I modified the patch to only
> print a warning instead of queuing a No-Op and then did various things
> which use control transactions: setting baud rate on serial, changing
> the volume on audio, starting video recording on a webcam, running
> ethtool on a NIC.
>
> The warning was printed a few times, but nothing interesting happened.
> Dynamic debug was enabled on handle_tx_event() - no errors reported.
>
> Maybe a different silicon/firmware revision, or maybe it's another
> SuperSpeed-only bug, or other special conditions for it to happen?

Do you see any "Transfer error for slot..." error message?
What is the speed of your device? high speed?
I try to downgrade my ethernet adapter to high speed and do some tests,
no errors are reported in dmesg if dynamic debug is enabled.
I think it is a super speed issue, however, it doesn't happen on the high
speed device, I am not sure. So the patch will not check the speed of the
device.

>
> > Ok, I will use one quirk XHCI_ETRON_HOST for these workarounds in the
> > next patch revision.
> That was just a suggestion, you should ask Mathias Nyman, I suppose.

OK, thanks.

>
> But, again, my impression of this hardware is that it's pretty bad
> and full of bugs, and they are bizarre enough to likely be unique.
>
> Regards,
> Michal

Thanks,
Kuangyi Chiang