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


Groups > linux.kernel > #1479214 > unrolled thread

Re: [PATCH 3/7] [v2] arm64: dts: add QorIQ LS1046A SoC support

Started byMark Rutland <mark.rutland@arm.com>
First post2016-09-08 15:20 +0200
Last post2016-09-09 11: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 3/7] [v2] arm64: dts: add QorIQ LS1046A SoC support Mark Rutland <mark.rutland@arm.com> - 2016-09-08 15:20 +0200
    Re: [PATCH 3/7] [v2] arm64: dts: add QorIQ LS1046A SoC support Mark Rutland <mark.rutland@arm.com> - 2016-09-09 11:20 +0200
      RE: [PATCH 3/7] [v2] arm64: dts: add QorIQ LS1046A SoC support "S.H. Xie" <shaohui.xie@nxp.com> - 2016-09-09 14:00 +0200
    RE: [PATCH 3/7] [v2] arm64: dts: add QorIQ LS1046A SoC support "S.H. Xie" <shaohui.xie@nxp.com> - 2016-09-09 11:30 +0200

#1479214 — Re: [PATCH 3/7] [v2] arm64: dts: add QorIQ LS1046A SoC support

FromMark Rutland <mark.rutland@arm.com>
Date2016-09-08 15:20 +0200
SubjectRe: [PATCH 3/7] [v2] arm64: dts: add QorIQ LS1046A SoC support
Message-ID<sf7pg-6h3-29@gated-at.bofh.it>
On Thu, Sep 08, 2016 at 02:13:26PM +0100, Mark Rutland wrote:
> On Mon, Sep 05, 2016 at 06:01:31PM +0800, shh.xie@gmail.com wrote:
> > +	cpus {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		cpu0: cpu@0 {
> > +			device_type = "cpu";
> > +			compatible = "arm,cortex-a72";
> > +			reg = <0x0>;
> > +			clocks = <&clockgen 1 0>;
> > +			next-level-cache = <&l2>;
> > +			cpu-idle-states = <&CPU_PH20>;
> > +		};
> 
> [...]
> 
> > +	};
> > +
> > +	idle-states {
> > +		entry-method = "arm,psci";
> > +
> > +		CPU_PH20: cpu-ph20 {
> > +			compatible = "arm,idle-state";
> > +			idle-state-name = "PH20";
> > +			arm,psci-suspend-param = <0x00010000>;
> > +			entry-latency-us = <1000>;
> > +			exit-latency-us = <1000>;
> > +			min-residency-us = <3000>;
> > +		};
> > +	};
> 
> There's no PSCI node in this file, and none from am included file, so
> this doesn't look right.

Looking again, none of the cpu nodes has an enable-method property, and
subsequent patches don't seem to add that to any cpu node.

Has this DT actually been tested?

Thanks,
Mark.

[toc] | [next] | [standalone]


#1479774

FromMark Rutland <mark.rutland@arm.com>
Date2016-09-09 11:20 +0200
Message-ID<sfq8x-Za-5@gated-at.bofh.it>
In reply to#1479214
On Fri, Sep 09, 2016 at 06:55:30AM +0000, S.H. Xie wrote:
> > On Thu, Sep 08, 2016 at 02:13:26PM +0100, Mark Rutland wrote:
> > > On Mon, Sep 05, 2016 at 06:01:31PM +0800, shh.xie@gmail.com wrote:
> > > > +	cpus {
> > > > +		#address-cells = <1>;
> > > > +		#size-cells = <0>;
> > > > +
> > > > +		cpu0: cpu@0 {
> > > > +			device_type = "cpu";
> > > > +			compatible = "arm,cortex-a72";
> > > > +			reg = <0x0>;
> > > > +			clocks = <&clockgen 1 0>;
> > > > +			next-level-cache = <&l2>;
> > > > +			cpu-idle-states = <&CPU_PH20>;
> > > > +		};
> > >
> > > [...]
> > >
> > > > +	};
> > > > +
> > > > +	idle-states {
> > > > +		entry-method = "arm,psci";
> > > > +
> > > > +		CPU_PH20: cpu-ph20 {
> > > > +			compatible = "arm,idle-state";
> > > > +			idle-state-name = "PH20";
> > > > +			arm,psci-suspend-param = <0x00010000>;
> > > > +			entry-latency-us = <1000>;
> > > > +			exit-latency-us = <1000>;
> > > > +			min-residency-us = <3000>;
> > > > +		};
> > > > +	};
> > >
> > > There's no PSCI node in this file, and none from am included file, so
> > > this doesn't look right.
> > 
> > Looking again, none of the cpu nodes has an enable-method property, and
> > subsequent patches don't seem to add that to any cpu node.
> > 
> > Has this DT actually been tested?
> [S.H] The PSCI node and the enable-method property are added by U-boot. 
> U-boot can determine if using PSCI. If U-boot enables PSCI, it will add these 
> missed parts in the dts. If not, it will not add these missed parts, 
> so kernel will not use PSCI.
> 
> In other words, the dts does not enable PSCI by default. 
> It's U-boot which adds the missed part if it determines to use PSCI.

Ok. Could you please place a comment in the dts to that effect?

Thanks,
Mark.

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


#1479920

From"S.H. Xie" <shaohui.xie@nxp.com>
Date2016-09-09 14:00 +0200
Message-ID<sfsDn-2kg-3@gated-at.bofh.it>
In reply to#1479774
> On Fri, Sep 09, 2016 at 06:55:30AM +0000, S.H. Xie wrote:
> > > On Thu, Sep 08, 2016 at 02:13:26PM +0100, Mark Rutland wrote:
> > > > On Mon, Sep 05, 2016 at 06:01:31PM +0800, shh.xie@gmail.com wrote:
> > > > > +	cpus {
> > > > > +		#address-cells = <1>;
> > > > > +		#size-cells = <0>;
> > > > > +
> > > > > +		cpu0: cpu@0 {
> > > > > +			device_type = "cpu";
> > > > > +			compatible = "arm,cortex-a72";
> > > > > +			reg = <0x0>;
> > > > > +			clocks = <&clockgen 1 0>;
> > > > > +			next-level-cache = <&l2>;
> > > > > +			cpu-idle-states = <&CPU_PH20>;
> > > > > +		};
> > > >
> > > > [...]
> > > >
> > > > > +	};
> > > > > +
> > > > > +	idle-states {
> > > > > +		entry-method = "arm,psci";
> > > > > +
> > > > > +		CPU_PH20: cpu-ph20 {
> > > > > +			compatible = "arm,idle-state";
> > > > > +			idle-state-name = "PH20";
> > > > > +			arm,psci-suspend-param = <0x00010000>;
> > > > > +			entry-latency-us = <1000>;
> > > > > +			exit-latency-us = <1000>;
> > > > > +			min-residency-us = <3000>;
> > > > > +		};
> > > > > +	};
> > > >
> > > > There's no PSCI node in this file, and none from am included file,
> > > > so this doesn't look right.
> > >
> > > Looking again, none of the cpu nodes has an enable-method property,
> > > and subsequent patches don't seem to add that to any cpu node.
> > >
> > > Has this DT actually been tested?
> > [S.H] The PSCI node and the enable-method property are added by U-boot.
> > U-boot can determine if using PSCI. If U-boot enables PSCI, it will
> > add these missed parts in the dts. If not, it will not add these
> > missed parts, so kernel will not use PSCI.
> >
> > In other words, the dts does not enable PSCI by default.
> > It's U-boot which adds the missed part if it determines to use PSCI.
> 
> Ok. Could you please place a comment in the dts to that effect?
[S.H] Yes. Will add it in next version.

Thanks,
Shaohui

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


#1479786

From"S.H. Xie" <shaohui.xie@nxp.com>
Date2016-09-09 11:30 +0200
Message-ID<sfq8x-Za-7@gated-at.bofh.it>
In reply to#1479214
> On Thu, Sep 08, 2016 at 02:13:26PM +0100, Mark Rutland wrote:
> > On Mon, Sep 05, 2016 at 06:01:31PM +0800, shh.xie@gmail.com wrote:
> > > +	cpus {
> > > +		#address-cells = <1>;
> > > +		#size-cells = <0>;
> > > +
> > > +		cpu0: cpu@0 {
> > > +			device_type = "cpu";
> > > +			compatible = "arm,cortex-a72";
> > > +			reg = <0x0>;
> > > +			clocks = <&clockgen 1 0>;
> > > +			next-level-cache = <&l2>;
> > > +			cpu-idle-states = <&CPU_PH20>;
> > > +		};
> >
> > [...]
> >
> > > +	};
> > > +
> > > +	idle-states {
> > > +		entry-method = "arm,psci";
> > > +
> > > +		CPU_PH20: cpu-ph20 {
> > > +			compatible = "arm,idle-state";
> > > +			idle-state-name = "PH20";
> > > +			arm,psci-suspend-param = <0x00010000>;
> > > +			entry-latency-us = <1000>;
> > > +			exit-latency-us = <1000>;
> > > +			min-residency-us = <3000>;
> > > +		};
> > > +	};
> >
> > There's no PSCI node in this file, and none from am included file, so
> > this doesn't look right.
> 
> Looking again, none of the cpu nodes has an enable-method property, and
> subsequent patches don't seem to add that to any cpu node.
> 
> Has this DT actually been tested?
[S.H] The PSCI node and the enable-method property are added by U-boot. 
U-boot can determine if using PSCI. If U-boot enables PSCI, it will add these 
missed parts in the dts. If not, it will not add these missed parts, 
so kernel will not use PSCI.

In other words, the dts does not enable PSCI by default. 
It's U-boot which adds the missed part if it determines to use PSCI.

Thanks,
Shaohui

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web