[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

+ 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