Message ID | 20170822213911.GC95008@gmail.com |
---|---|
State | Rejected |
Delegated to: | Philipp Tomsich |
Headers | show |
+ 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
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>;