| Message ID | 1717135657-120818-4-git-send-email-dh10.jung@samsung.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: Add quirk for writing high-low order | expand |
On 31/05/2024 08:07, Daehwan Jung wrote: > xHCI specification 5.1 "Register Conventions" states that 64 bit > registers should be written in low-high order. All writing operations > in xhci is done low-high order following the spec. What is high-low / low-high order? Are you talking about endianness? > > Add a new quirk to support workaround for high-low order. Why? If they should be written low-high, then why breaking the spec? Why this cannot be deduced from compatible? Which *upstream* hardware is affected? Best regards, Krzysztof
On Fri, May 31, 2024 at 10:12:03AM +0200, Krzysztof Kozlowski wrote: > On 31/05/2024 08:07, Daehwan Jung wrote: > > xHCI specification 5.1 "Register Conventions" states that 64 bit > > registers should be written in low-high order. All writing operations > > in xhci is done low-high order following the spec. > > What is high-low / low-high order? Are you talking about endianness? > It's not about endianness. It means 64 bit is written by 2 of 32 bit. It's when low-high / high-low order is needed. > > > > Add a new quirk to support workaround for high-low order. > > Why? If they should be written low-high, then why breaking the spec? Why > this cannot be deduced from compatible? > This quirk is for the controller that has a limitation in supporting separate ERSTBA_HI and ERSTBA_LO programming. I've copied below from other reply to tell why this is needed. I've found out the limitation of Synopsys dwc3 controller. This can work on Host mode using xHCI. A Register related to ERST should be written high-low order not low-high order. Registers are always written low-high order following xHCI spec.(64-bit written is done in each 2 of 32-bit) That's why new quirk is needed for workaround. This quirk is used not in dwc3 controller itself, but passed to xhci quirk eventually. That's because this issue occurs in Host mode using xHCI. > Which *upstream* hardware is affected? > dwc3 and xhci are affected. Best Regards, Jung Daehwan > > > Best regards, > Krzysztof > > >
On 03/06/2024 05:34, Jung Daehwan wrote: > On Fri, May 31, 2024 at 10:12:03AM +0200, Krzysztof Kozlowski wrote: >> On 31/05/2024 08:07, Daehwan Jung wrote: >>> xHCI specification 5.1 "Register Conventions" states that 64 bit >>> registers should be written in low-high order. All writing operations >>> in xhci is done low-high order following the spec. >> >> What is high-low / low-high order? Are you talking about endianness? >> > > It's not about endianness. It means 64 bit is written by 2 of 32 bit. > It's when low-high / high-low order is needed. > >>> >>> Add a new quirk to support workaround for high-low order. >> >> Why? If they should be written low-high, then why breaking the spec? Why >> this cannot be deduced from compatible? >> > > This quirk is for the controller that has a limitation in supporting > separate ERSTBA_HI and ERSTBA_LO programming. > > I've copied below from other reply to tell why this is needed. > > I've found out the limitation of Synopsys dwc3 controller. This can work > on Host mode using xHCI. A Register related to ERST should be written > high-low order not low-high order. Registers are always written low-high order > following xHCI spec.(64-bit written is done in each 2 of 32-bit) > That's why new quirk is needed for workaround. This quirk is used not in > dwc3 controller itself, but passed to xhci quirk eventually. That's because > this issue occurs in Host mode using xHCI. > >> Which *upstream* hardware is affected? >> > > dwc3 and xhci are affected. Which upstream controllers or SoCs are affected. You did not post any user of this. Best regards, Krzysztof
On Mon, Jun 03, 2024 at 08:58:10AM +0200, Krzysztof Kozlowski wrote: > On 03/06/2024 05:34, Jung Daehwan wrote: > > On Fri, May 31, 2024 at 10:12:03AM +0200, Krzysztof Kozlowski wrote: > >> On 31/05/2024 08:07, Daehwan Jung wrote: > >>> xHCI specification 5.1 "Register Conventions" states that 64 bit > >>> registers should be written in low-high order. All writing operations > >>> in xhci is done low-high order following the spec. > >> > >> What is high-low / low-high order? Are you talking about endianness? > >> > > > > It's not about endianness. It means 64 bit is written by 2 of 32 bit. > > It's when low-high / high-low order is needed. > > > >>> > >>> Add a new quirk to support workaround for high-low order. > >> > >> Why? If they should be written low-high, then why breaking the spec? Why > >> this cannot be deduced from compatible? > >> > > > > This quirk is for the controller that has a limitation in supporting > > separate ERSTBA_HI and ERSTBA_LO programming. > > > > I've copied below from other reply to tell why this is needed. > > > > I've found out the limitation of Synopsys dwc3 controller. This can work > > on Host mode using xHCI. A Register related to ERST should be written > > high-low order not low-high order. Registers are always written low-high order > > following xHCI spec.(64-bit written is done in each 2 of 32-bit) > > That's why new quirk is needed for workaround. This quirk is used not in > > dwc3 controller itself, but passed to xhci quirk eventually. That's because > > this issue occurs in Host mode using xHCI. > > > >> Which *upstream* hardware is affected? > >> > > > > dwc3 and xhci are affected. > > Which upstream controllers or SoCs are affected. You did not post any > user of this. > This issue is not specific on SoC side but dwc3 controller. I think it's better to add it to dwc3 directly but I can't find dts for dwc3. Dts for SoC, which uses dwc3 would be needed in this case, right? Best Regards, Jung Daehwan > Best regards, > Krzysztof > >
On Mon, Jun 03, 2024, Jung Daehwan wrote: > > This issue is not specific on SoC side but dwc3 controller. I think it's > better to add it to dwc3 directly but I can't find dts for dwc3. Dts for > SoC, which uses dwc3 would be needed in this case, right? > This quirk applies across different DWC_usb3x versions. IMO, you can just pass the xhci quirk flag along the dwc3_xhci_plat_quirk->quirks without needing to introduce a new xhci DTS binding. However, to do this, you should extract the xhci quirk flags to a separate header so that dwc3 can include and use them. BR, Thinh
On Tue, Jun 04, 2024 at 12:30:32AM +0000, Thinh Nguyen wrote: > On Mon, Jun 03, 2024, Jung Daehwan wrote: > > > > This issue is not specific on SoC side but dwc3 controller. I think it's > > better to add it to dwc3 directly but I can't find dts for dwc3. Dts for > > SoC, which uses dwc3 would be needed in this case, right? > > > > This quirk applies across different DWC_usb3x versions. IMO, you can > just pass the xhci quirk flag along the dwc3_xhci_plat_quirk->quirks > without needing to introduce a new xhci DTS binding. However, to do > this, you should extract the xhci quirk flags to a separate header so > that dwc3 can include and use them. > > BR, > Thinh I haven't got using dwc3_xhci_plat_quirk. Let me try to use it. Thanks for the comment. Best Regards, Jung Daehwan
diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.yaml b/Documentation/devicetree/bindings/usb/usb-xhci.yaml index 4238ae8..447d331 100644 --- a/Documentation/devicetree/bindings/usb/usb-xhci.yaml +++ b/Documentation/devicetree/bindings/usb/usb-xhci.yaml @@ -25,6 +25,10 @@ properties: description: Set if the controller has broken port disable mechanism type: boolean + write-64-hi-lo-quirk: + description: Set if the controller has a limitation of high-low order + type: boolean + imod-interval-ns: description: Interrupt moderation interval default: 5000
xHCI specification 5.1 "Register Conventions" states that 64 bit registers should be written in low-high order. All writing operations in xhci is done low-high order following the spec. Add a new quirk to support workaround for high-low order. Signed-off-by: Daehwan Jung <dh10.jung@samsung.com> --- Documentation/devicetree/bindings/usb/usb-xhci.yaml | 4 ++++ 1 file changed, 4 insertions(+)