Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1390351 > unrolled thread

Re: [PATCH v4 2/4] ARM64: dts: rockchip: add core dtsi file for RK3399 SoCs

Started byBrian Norris <briannorris@chromium.org>
First post2016-04-28 18:10 +0200
Last post2016-04-29 00:30 +0200
Articles 4 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH v4 2/4] ARM64: dts: rockchip: add core dtsi file for  RK3399 SoCs Brian Norris <briannorris@chromium.org> - 2016-04-28 18:10 +0200
    Re: [PATCH v4 2/4] ARM64: dts: rockchip: add core dtsi file for  RK3399 SoCs Brian Norris <briannorris@chromium.org> - 2016-04-28 20:30 +0200
      Re: [PATCH v4 2/4] ARM64: dts: rockchip: add core dtsi file for RK3399 SoCs Heiko Stübner <heiko@sntech.de> - 2016-04-28 22:40 +0200
        Re: [PATCH v4 2/4] ARM64: dts: rockchip: add core dtsi file for  RK3399 SoCs Brian Norris <briannorris@chromium.org> - 2016-04-29 00:30 +0200

#1390351 — Re: [PATCH v4 2/4] ARM64: dts: rockchip: add core dtsi file for RK3399 SoCs

FromBrian Norris <briannorris@chromium.org>
Date2016-04-28 18:10 +0200
SubjectRe: [PATCH v4 2/4] ARM64: dts: rockchip: add core dtsi file for RK3399 SoCs
Message-ID<rsWFP-6n2-1@gated-at.bofh.it>
Hi Heiko, Jianqun,

On Wed, Apr 27, 2016 at 03:54:51PM +0800, Jianqun Xu wrote:
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> new file mode 100644
> index 0000000..5a8a915
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -0,0 +1,1022 @@
[...]
> +	sdhci: sdhci@fe330000 {
> +		compatible = "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1";

Not to rain on the parade too much, as this is already applied, but is
the "rockchip,rk3399-sdhci-5.1" string documented anywhere? I don't see
it.

> +		reg = <0x0 0xfe330000 0x0 0x10000>;
> +		interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&cru SCLK_EMMC>, <&cru ACLK_EMMC>;
> +		clock-names = "clk_xin", "clk_ahb";
> +		status = "disabled";
> +	};

[...]

Brian

[toc] | [next] | [standalone]


#1390441

FromBrian Norris <briannorris@chromium.org>
Date2016-04-28 20:30 +0200
Message-ID<rsYRk-83-5@gated-at.bofh.it>
In reply to#1390351
One more thing:

On Thu, Apr 28, 2016 at 09:03:53AM -0700, Brian Norris wrote:
> Hi Heiko, Jianqun,
> 
> On Wed, Apr 27, 2016 at 03:54:51PM +0800, Jianqun Xu wrote:
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > new file mode 100644
> > index 0000000..5a8a915
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > @@ -0,0 +1,1022 @@
> [...]
> > +	sdhci: sdhci@fe330000 {
> > +		compatible = "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1";
> 
> Not to rain on the parade too much, as this is already applied, but is
> the "rockchip,rk3399-sdhci-5.1" string documented anywhere? I don't see
> it.

According to the latest binding for "arasan,sdhci-5.1", the "phy" and
"phy-names" properties are required. Fortunately, this device stays
"disabled" for now in your EVB DTS. But just FYI.

> > +		reg = <0x0 0xfe330000 0x0 0x10000>;
> > +		interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
> > +		clocks = <&cru SCLK_EMMC>, <&cru ACLK_EMMC>;
> > +		clock-names = "clk_xin", "clk_ahb";
> > +		status = "disabled";
> > +	};

Brian

[toc] | [prev] | [next] | [standalone]


#1390498 — Re: [PATCH v4 2/4] ARM64: dts: rockchip: add core dtsi file for RK3399 SoCs

FromHeiko Stübner <heiko@sntech.de>
Date2016-04-28 22:40 +0200
SubjectRe: [PATCH v4 2/4] ARM64: dts: rockchip: add core dtsi file for RK3399 SoCs
Message-ID<rt0T8-1Qs-13@gated-at.bofh.it>
In reply to#1390441
Am Donnerstag, 28. April 2016, 11:29:38 schrieb Brian Norris:
> One more thing:
> 
> On Thu, Apr 28, 2016 at 09:03:53AM -0700, Brian Norris wrote:
> > Hi Heiko, Jianqun,
> > 
> > On Wed, Apr 27, 2016 at 03:54:51PM +0800, Jianqun Xu wrote:
> > > diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > > b/arch/arm64/boot/dts/rockchip/rk3399.dtsi new file mode 100644
> > > index 0000000..5a8a915
> > > --- /dev/null
> > > +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > > @@ -0,0 +1,1022 @@
> > 
> > [...]
> > 
> > > +	sdhci: sdhci@fe330000 {
> > > +		compatible = "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1";
> > 
> > Not to rain on the parade too much, as this is already applied, but is
> > the "rockchip,rk3399-sdhci-5.1" string documented anywhere? I don't see
> > it.

I don't think it is. I'm still not sure how those dangling (aka spare bindings 
for later use) should be handled.

Their use is suggested by dt maintainers, to be able to handle ip-block quirks 
later on without needing to touch the devicetree, but in this case spamming 
the arasan dt-binding document with numerous of those compatible values also 
feels wrong.

> According to the latest binding for "arasan,sdhci-5.1", the "phy" and
> "phy-names" properties are required. Fortunately, this device stays
> "disabled" for now in your EVB DTS. But just FYI.

Thanks for catching this. As the patch was still local to my repository, I've 
amended the commit and dropped the whole emmc block for now.

The emmc phy-binding just moved under the GRF (in 4.6-rc5 I think), so I guess 
we should handle that whole thing in the next version, as we're nearing (or 
are [nearly] over the armsoc cutoff already).

> > > +		reg = <0x0 0xfe330000 0x0 0x10000>;
> > > +		interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
> > > +		clocks = <&cru SCLK_EMMC>, <&cru ACLK_EMMC>;
> > > +		clock-names = "clk_xin", "clk_ahb";
> > > +		status = "disabled";
> > > +	};
> 
> Brian

[toc] | [prev] | [next] | [standalone]


#1390604

FromBrian Norris <briannorris@chromium.org>
Date2016-04-29 00:30 +0200
Message-ID<rt2BA-3kM-9@gated-at.bofh.it>
In reply to#1390498
On Thu, Apr 28, 2016 at 10:33:07PM +0200, Heiko Stuebner wrote:
> Am Donnerstag, 28. April 2016, 11:29:38 schrieb Brian Norris:
> > On Thu, Apr 28, 2016 at 09:03:53AM -0700, Brian Norris wrote:
> > > On Wed, Apr 27, 2016 at 03:54:51PM +0800, Jianqun Xu wrote:
> > > > diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > > > b/arch/arm64/boot/dts/rockchip/rk3399.dtsi new file mode 100644
> > > > index 0000000..5a8a915
> > > > --- /dev/null
> > > > +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > > > @@ -0,0 +1,1022 @@
> > > 
> > > [...]
> > > 
> > > > +	sdhci: sdhci@fe330000 {
> > > > +		compatible = "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1";
> > > 
> > > Not to rain on the parade too much, as this is already applied, but is
> > > the "rockchip,rk3399-sdhci-5.1" string documented anywhere? I don't see
> > > it.
> 
> I don't think it is. I'm still not sure how those dangling (aka spare bindings 
> for later use) should be handled.
> 
> Their use is suggested by dt maintainers, to be able to handle ip-block quirks 
> later on without needing to touch the devicetree, but in this case spamming 
> the arasan dt-binding document with numerous of those compatible values also 
> feels wrong.

Hmm, good point. I was mostly tipped off by checkpatch when applying
locally, and I also didn't have any context for whether there were any
known quirks for rk3399 vs. the "standard" core. Given your comments, I
suppose it's fair to use without documenting it (do I get burned at the
stake for saying that?).

> > According to the latest binding for "arasan,sdhci-5.1", the "phy" and
> > "phy-names" properties are required. Fortunately, this device stays
> > "disabled" for now in your EVB DTS. But just FYI.
> 
> Thanks for catching this. As the patch was still local to my repository, I've 
> amended the commit and dropped the whole emmc block for now.

No problem.

> The emmc phy-binding just moved under the GRF (in 4.6-rc5 I think), so I guess 
> we should handle that whole thing in the next version, as we're nearing (or 
> are [nearly] over the armsoc cutoff already).

Sounds good. I'm already tracking/testing that modification.

Brian

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web