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.
>
>Guenter
>
