rk3xxx.dtsi /usb_host missing specific compatible
Hi,
no idea if this is the right place to mail about this, but i got suggested this node is out-of-norm, and the diff below fixes that for me on rk3188.
-Artturi
arch/arm/dts/rk3xxx.dtsi +1 -1
@@ -157,7 +157,7 @@ }; usb_host: usb at 101c0000 { - compatible = "snps,dwc2"; + compatible = "rockchip,rk3066-usb", "snps,dwc2"; reg = <0x101c0000 0x40000>; interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>; clocks = <&cru HCLK_OTG1>;
+ Heiko
Quoted 8 lines · Artturi Alm
Hi,
no idea if this is the right place to mail about this, but i got suggested this node is out-of-norm, and the diff below fixes that for me on rk3188.
-Artturi
When submitting changes, please send a patch w/ an appropriate commit message (e.g. using patman). If you tag it as "rockchip:", it will eventually get assigned to my queue.
arch/arm/dts/rk3xxx.dtsi line 160
Quoted 11 lines
arch/arm/dts/rk3xxx.dtsi +1
-1@@ -157,7 +157,7 @@ }; usb_host: usb at 101c0000 { - compatible = "snps,dwc2"; + compatible = "rockchip,rk3066-usb", "snps,dwc2";
This is the same on the Linux upstream, which is the leading repository for this DTS file. Also, the "rockchip,rk3066-usb" is used by none of the drivers (whereas "snsp,dwc2" is matche by drivers/usb/host/dwc2.c.
From my point of view, there's no point in changing this (unless Heiko would like to see this changed both here and in Linux).
Quoted 4 lines
arch/arm/dts/rk3xxx.dtsi +0
-0reg = <0x101c0000 0x40000>; interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>; clocks = <&cru HCLK_OTG1>;
Am Freitag, 25. August 2017, 13:20:47 CEST schrieb Philipp Tomsich:
Quoted 33 lines
+ Heiko
Quoted 8 lines · Artturi Alm
Hi,
no idea if this is the right place to mail about this, but i got suggested this node is out-of-norm, and the diff below fixes that for me on rk3188.
-Artturi
When submitting changes, please send a patch w/ an appropriate commit message (e.g. using patman). If you tag it as "rockchip:", it will eventually get assigned to my queue.
arch/arm/dts/rk3xxx.dtsi line 160
Quoted 11 lines
arch/arm/dts/rk3xxx.dtsi +1
-1@@ -157,7 +157,7 @@ }; usb_host: usb at 101c0000 { - compatible = "snps,dwc2"; + compatible = "rockchip,rk3066-usb", "snps,dwc2";This is the same on the Linux upstream, which is the leading repository for this DTS file. Also, the "rockchip,rk3066-usb" is used by none of the drivers (whereas "snsp,dwc2" is matche by
drivers/usb/host/dwc2.c.
Quoted 2 lines
From my point of view, there's no point in changing this (unless Heiko would like to see this changed both here and in Linux).
In general it is common practice to have a more specialized compatible as a reserve, to be able add "quirks" later on if necessary without needing devicetree updates as well. For example the otg node does already have the rk3066-usb compatible.
On the kernel-side, we even do have specialized init values for Rockchip dwc2 controllers, which is bound to the rk3066-usb compatible. I'm not sure why only the otg controller got it though and the addition of the dwc2 nodes in the mainline kernel was already in 2014 :-) .
So I don't have a set opinion one way or another, as it looks like things work reasonably well as they are now, but if someone sends in a _tested_ kernel patch setting the specific compatible, I'll look at it and possibly apply it :-) .
Heiko
Quoted 47 lines · Heiko Stuebner
Am Freitag, 25. August 2017, 13:20:47 CEST schrieb Philipp Tomsich:
Quoted 33 lines
+ Heiko
Quoted 8 lines · Artturi Alm
Hi,
no idea if this is the right place to mail about this, but i got suggested this node is out-of-norm, and the diff below fixes that for me on rk3188.
-Artturi
When submitting changes, please send a patch w/ an appropriate commit message (e.g. using patman). If you tag it as "rockchip:", it will eventually get assigned to my queue.
arch/arm/dts/rk3xxx.dtsi line 160
Quoted 11 lines
arch/arm/dts/rk3xxx.dtsi +1
-1@@ -157,7 +157,7 @@ }; usb_host: usb at 101c0000 { - compatible = "snps,dwc2"; + compatible = "rockchip,rk3066-usb", "snps,dwc2";This is the same on the Linux upstream, which is the leading repository for this DTS file. Also, the "rockchip,rk3066-usb" is used by none of the drivers (whereas "snsp,dwc2" is matche by
drivers/usb/host/dwc2.c.Quoted 2 lines
From my point of view, there's no point in changing this (unless Heiko would like to see this changed both here and in Linux).
In general it is common practice to have a more specialized compatible as a reserve, to be able add "quirks" later on if necessary without needing devicetree updates as well. For example the otg node does already have the rk3066-usb compatible.
On the kernel-side, we even do have specialized init values for Rockchip dwc2 controllers, which is bound to the rk3066-usb compatible. I'm not sure why only the otg controller got it though and the addition of the dwc2 nodes in the mainline kernel was already in 2014 :-) .
Quoted 7 lines
So I don't have a set opinion one way or another, as it looks like things work reasonably well as they are now, but if someone sends in a _tested_ kernel patch setting the specific compatible, I'll look at it and possibly apply it :-) .
Heiko
I was asking this for OpenBSD actually, as it has these FDT "attachment-drivers" via rather generic stubs to do the driver attachment with compatibles alone for matching the needed config.
I've already worked around, but think of it as ugly SoC-specific hack as-is.
Hmmph, i missed a detail before, i might actually need this fixed in Linux too. I haven't seen any patches to .dts files in the OpenBSD-ports package, for the .dtb files shipped for use, and iirc they're sourced from Linux.
I'll leave the tested linux patch for a very rainy day, because atm. u-boot is broken enough on rk3188 that i can't really test anything on them.
-Artturi