[U-Boot] 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
diff --git a/arch/arm/dts/rk3xxx.dtsi b/arch/arm/dts/rk3xxx.dtsi index 6d9e36d235..21f2afc104 100644 --- a/arch/arm/dts/rk3xxx.dtsi +++ b/arch/arm/dts/rk3xxx.dtsi @@ -157,7 +157,7 @@ };
usb_host: usb@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
On Wed, 23 Aug 2017, Artturi Alm wrote:
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.
diff --git a/arch/arm/dts/rk3xxx.dtsi b/arch/arm/dts/rk3xxx.dtsi index 6d9e36d235..21f2afc104 100644 --- a/arch/arm/dts/rk3xxx.dtsi +++ b/arch/arm/dts/rk3xxx.dtsi @@ -157,7 +157,7 @@ };
usb_host: usb@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).
reg = <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:
- Heiko
On Wed, 23 Aug 2017, Artturi Alm wrote:
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.
diff --git a/arch/arm/dts/rk3xxx.dtsi b/arch/arm/dts/rk3xxx.dtsi index 6d9e36d235..21f2afc104 100644 --- a/arch/arm/dts/rk3xxx.dtsi +++ b/arch/arm/dts/rk3xxx.dtsi @@ -157,7 +157,7 @@ };
usb_host: usb@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).
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

On Sat, Aug 26, 2017 at 07:48:28PM +0200, Heiko Stuebner wrote:
Am Freitag, 25. August 2017, 13:20:47 CEST schrieb Philipp Tomsich:
- Heiko
On Wed, 23 Aug 2017, Artturi Alm wrote:
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.
diff --git a/arch/arm/dts/rk3xxx.dtsi b/arch/arm/dts/rk3xxx.dtsi index 6d9e36d235..21f2afc104 100644 --- a/arch/arm/dts/rk3xxx.dtsi +++ b/arch/arm/dts/rk3xxx.dtsi @@ -157,7 +157,7 @@ };
usb_host: usb@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).
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
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
participants (3)
-
Artturi Alm
-
Heiko Stuebner
-
Philipp Tomsich