On Wed, Jul 16, 2025 at 07:59:16PM +0000, Bernhard Beschow wrote:
>
>
> Am 13. April 2025 16:03:16 UTC schrieb Guenter Roeck <li...@roeck-us.net>:
> >On 4/11/25 00:40, Nicholas Piggin wrote:
> >> On Sun Apr 6, 2025 at 12:00 AM AEST, Guenter Roeck wrote:
> >>> According to the XHCI specification, ERSTBA should be written in Low-High
> >>> order. The Linux kernel writes the high word first. This results in an
> >>> initialization failure.
> >>
> >> This should probably be reworded, it's not so much that Linux does it,
> >> this kind of implies a Linux bug. It is that the hardware requires it
> >> and Linux works around such quirk.
> >>
> >> According to the XHCI specification, ERSTBA should be written in
> >> Low-High
> >> order, however some controllers have a quirk that requires the low
> >> word to be written last.
> >>
> >>>
> >>> The following information is found in the Linux kernel commit log.
> >>>
> >>> [Synopsys]- The host controller was design to support ERST setting
> >>> during the RUN state. But since there is a limitation in controller
> >>> in supporting separate ERSTBA_HI and ERSTBA_LO programming,
> >>> It is supported when the ERSTBA is programmed in 64bit,
> >>> or in 32 bit mode ERSTBA_HI before ERSTBA_LO
> >>>
> >>> [Synopsys]- The internal initialization of event ring fetches
> >>> the "Event Ring Segment Table Entry" based on the indication of
> >>> ERSTBA_LO written.
> >>
> >> Could you include a reference to the commit in the normal form?
> >>
> >> The following information is found in the changelog for Linux kernel
> >> commit sha ("blah").
> >>
> >>>
> >>> Add property to support writing the high word first.
> >>>
> >>> Signed-off-by: Guenter Roeck <li...@roeck-us.net>
> >>> ---
> >>> hw/usb/hcd-xhci.c | 8 +++++++-
> >>> hw/usb/hcd-xhci.h | 1 +
> >>> 2 files changed, 8 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> >>> index 64c3a23b9b..8c0ba569c8 100644
> >>> --- a/hw/usb/hcd-xhci.c
> >>> +++ b/hw/usb/hcd-xhci.c
> >>> @@ -3107,10 +3107,15 @@ static void xhci_runtime_write(void *ptr, hwaddr
> >>> reg,
> >>> } else {
> >>> intr->erstba_low = val & 0xffffffc0;
> >>> }
> >>> + if (xhci->erstba_hi_lo) {
> >>> + xhci_er_reset(xhci, v);
> >>> + }
> >>> break;
> >>> case 0x14: /* ERSTBA high */
> >>> intr->erstba_high = val;
> >>> - xhci_er_reset(xhci, v);
> >>> + if (!xhci->erstba_hi_lo) {
> >>> + xhci_er_reset(xhci, v);
> >>> + }
> >>> break;
> >>> case 0x18: /* ERDP low */
> >>> if (val & ERDP_EHB) {
> >>> @@ -3636,6 +3641,7 @@ static const Property xhci_properties[] = {
> >>> DEFINE_PROP_UINT32("p3", XHCIState, numports_3, 4),
> >>> DEFINE_PROP_LINK("host", XHCIState, hostOpaque, TYPE_DEVICE,
> >>> DeviceState *),
> >>> + DEFINE_PROP_BOOL("erstba-hi-lo", XHCIState, erstba_hi_lo, false),
> >>> };
> >>> static void xhci_class_init(ObjectClass *klass, void *data)
> >>> diff --git a/hw/usb/hcd-xhci.h b/hw/usb/hcd-xhci.h
> >>> index 9c3974f148..cf3f074261 100644
> >>> --- a/hw/usb/hcd-xhci.h
> >>> +++ b/hw/usb/hcd-xhci.h
> >>> @@ -189,6 +189,7 @@ typedef struct XHCIState {
> >>> uint32_t numports_3;
> >>> uint32_t numintrs;
> >>> uint32_t numslots;
> >>> + bool erstba_hi_lo;
> >>
> >> Could you use the "quirk" prefix for the device and property name?
> >>
> >
> >Done all, except then I noticed that you want me to prepend "quirk"
> >and I appended it out of habit. So I'll need to start over :-(.
>
> Ping
>
> Any news? Would be nice if a fix made it into 10.1.
>
Sorry, I ran out of time and won't be able to look into this again
anytime soon. I don't mind if someone else wants to pick it up.
Guenter
