All of lore.kernel.org
 help / color / mirror / Atom feed
From: chunyan.zhang@spreadtrum.com (Chunyan Zhang)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/5] arm64: dts: Add basic DT to support Spreadtrum's SP9860G
Date: Fri, 17 Feb 2017 15:28:39 +0800	[thread overview]
Message-ID: <20170217072839.GA8767@spreadtrum.com> (raw)
In-Reply-To: <CAPKp9ub35nL8gc_TKx02pSHHNwBhiMacWL=CuEmNGnecWikfGQ@mail.gmail.com>

Hi Sudeep,

On ?,  2? 14, 2017 at 04:44:53?? +0000, Sudeep Holla wrote:
> On Tue, Feb 14, 2017 at 9:19 AM, Chunyan Zhang
> <chunyan.zhang@spreadtrum.com> wrote:
> > From: Orson Zhai <orson.zhai@spreadtrum.com>
> >
> > SC9860G is a 8 cores of A53 SoC with 4G LTE support SoC from Spreadtrum.
> >
> > According to regular hierarchy of sprd dts, whale2.dtsi contains SoC
> > peripherals IP nodes, sc9860.dtsi contains stuff related to ARM core stuff
> > and sp9860g dts is for the board level.
> >
> > Signed-off-by: Orson Zhai <orson.zhai@spreadtrum.com>
> > Signed-off-by: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
> > ---
> >  arch/arm64/boot/dts/sprd/Makefile         |   3 +-
> >  arch/arm64/boot/dts/sprd/sc9860.dtsi      | 534 ++++++++++++++++++++++++++++++
> >  arch/arm64/boot/dts/sprd/sp9860g-1h10.dts |  58 ++++
> >  arch/arm64/boot/dts/sprd/whale2.dtsi      |  66 ++++
> >  4 files changed, 660 insertions(+), 1 deletion(-)
> >  create mode 100644 arch/arm64/boot/dts/sprd/sc9860.dtsi
> >  create mode 100644 arch/arm64/boot/dts/sprd/sp9860g-1h10.dts
> >  create mode 100644 arch/arm64/boot/dts/sprd/whale2.dtsi
> >
> > diff --git a/arch/arm64/boot/dts/sprd/Makefile b/arch/arm64/boot/dts/sprd/Makefile
> > index b658c5e..f0535e6 100644
> > --- a/arch/arm64/boot/dts/sprd/Makefile
> > +++ b/arch/arm64/boot/dts/sprd/Makefile
> > @@ -1,4 +1,5 @@
> > -dtb-$(CONFIG_ARCH_SPRD) += sc9836-openphone.dtb
> > +dtb-$(CONFIG_ARCH_SPRD) += sc9836-openphone.dtb \
> > +                       sp9860g-1h10.dtb
> >
> >  always         := $(dtb-y)
> >  subdir-y       := $(dts-dirs)
> > diff --git a/arch/arm64/boot/dts/sprd/sc9860.dtsi b/arch/arm64/boot/dts/sprd/sc9860.dtsi
> > new file mode 100644
> > index 0000000..604a8c9
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/sprd/sc9860.dtsi
> > @@ -0,0 +1,534 @@
> 
> [...]
> 
> > +       idle-states{
> > +               entry-method = "arm,psci";
> > +
> > +               CORE_PD: core_pd {
> > +                       compatible = "arm,idle-state";
> > +                       entry-latency-us = <1000>;
> > +                       exit-latency-us = <700>;
> > +                       min-residency-us = <2500>;
> > +                       local-timer-stop;
> > +                       arm,psci-suspend-param = <0x00010002>;
> > +               };
> > +
> > +               CLUSTER_PD: cluster_pd {
> > +                       compatible = "arm,idle-state";
> > +                       entry-latency-us = <1000>;
> > +                       exit-latency-us = <1000>;
> > +                       min-residency-us = <3000>;
> > +                       local-timer-stop;
> > +                       arm,psci-suspend-param = <0x01010003>;
> > +               };
> > +
> > +               DEEP_SLEEP: deep_sleep {
> > +                       compatible = "arm,idle-state";
> > +                       wakeup-latency-us = <0xffffffff>;
> 
> A value > 4294 seconds(i.e >1 hour) seems suspicious.
> Are you working around the firmware issue with high latency value so
> that it's never entered ? Why not remove advertising the state from DT.
>

Haved checked with related colleagues, this node 'deep_sleep' was not for working
around any firmware issue, but was a trick utilization of idle subsystem, and that
was definitely not elegant, the author indeed intendly didn't want CPU entered this
state, I will remove this node therefore.
 
> Can you get me the dump of:
> grep "" /sys/devices/system/cpu/cpu*/cpuidle/state*/{time,usage}
> 

FYI: https://www.irccloud.com/pastebin/XyEMLzfq/

Thanks,
Chunyan

> IIUC, you might have seen boot issue without this values and workaround
> the issue with such high values ? If so please drop this state.
> 
> > +                       entry-latency-us = <1500>;
> > +                       exit-latency-us = <1500>;
> > +                       min-residency-us = <0xffffffff>;
> > +                       local-timer-stop;
> > +                       arm,psci-suspend-param = <0x01010005>;
> > +               };
> > +       };
> >
> 
> --
> Regards,
> Sudeep

WARNING: multiple messages have this Message-ID (diff)
From: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
To: Sudeep Holla <sudeep.holla@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	devicetree@vger.kernel.org, orson.zhai@spreadtrum.com,
	Arnd Bergmann <arnd@arndb.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Will Deacon <will.deacon@arm.com>,
	open list <linux-kernel@vger.kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	linux-arm <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 1/5] arm64: dts: Add basic DT to support Spreadtrum's SP9860G
Date: Fri, 17 Feb 2017 15:28:39 +0800	[thread overview]
Message-ID: <20170217072839.GA8767@spreadtrum.com> (raw)
In-Reply-To: <CAPKp9ub35nL8gc_TKx02pSHHNwBhiMacWL=CuEmNGnecWikfGQ@mail.gmail.com>

Hi Sudeep,

On 二,  2月 14, 2017 at 04:44:53下午 +0000, Sudeep Holla wrote:
> On Tue, Feb 14, 2017 at 9:19 AM, Chunyan Zhang
> <chunyan.zhang@spreadtrum.com> wrote:
> > From: Orson Zhai <orson.zhai@spreadtrum.com>
> >
> > SC9860G is a 8 cores of A53 SoC with 4G LTE support SoC from Spreadtrum.
> >
> > According to regular hierarchy of sprd dts, whale2.dtsi contains SoC
> > peripherals IP nodes, sc9860.dtsi contains stuff related to ARM core stuff
> > and sp9860g dts is for the board level.
> >
> > Signed-off-by: Orson Zhai <orson.zhai@spreadtrum.com>
> > Signed-off-by: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
> > ---
> >  arch/arm64/boot/dts/sprd/Makefile         |   3 +-
> >  arch/arm64/boot/dts/sprd/sc9860.dtsi      | 534 ++++++++++++++++++++++++++++++
> >  arch/arm64/boot/dts/sprd/sp9860g-1h10.dts |  58 ++++
> >  arch/arm64/boot/dts/sprd/whale2.dtsi      |  66 ++++
> >  4 files changed, 660 insertions(+), 1 deletion(-)
> >  create mode 100644 arch/arm64/boot/dts/sprd/sc9860.dtsi
> >  create mode 100644 arch/arm64/boot/dts/sprd/sp9860g-1h10.dts
> >  create mode 100644 arch/arm64/boot/dts/sprd/whale2.dtsi
> >
> > diff --git a/arch/arm64/boot/dts/sprd/Makefile b/arch/arm64/boot/dts/sprd/Makefile
> > index b658c5e..f0535e6 100644
> > --- a/arch/arm64/boot/dts/sprd/Makefile
> > +++ b/arch/arm64/boot/dts/sprd/Makefile
> > @@ -1,4 +1,5 @@
> > -dtb-$(CONFIG_ARCH_SPRD) += sc9836-openphone.dtb
> > +dtb-$(CONFIG_ARCH_SPRD) += sc9836-openphone.dtb \
> > +                       sp9860g-1h10.dtb
> >
> >  always         := $(dtb-y)
> >  subdir-y       := $(dts-dirs)
> > diff --git a/arch/arm64/boot/dts/sprd/sc9860.dtsi b/arch/arm64/boot/dts/sprd/sc9860.dtsi
> > new file mode 100644
> > index 0000000..604a8c9
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/sprd/sc9860.dtsi
> > @@ -0,0 +1,534 @@
> 
> [...]
> 
> > +       idle-states{
> > +               entry-method = "arm,psci";
> > +
> > +               CORE_PD: core_pd {
> > +                       compatible = "arm,idle-state";
> > +                       entry-latency-us = <1000>;
> > +                       exit-latency-us = <700>;
> > +                       min-residency-us = <2500>;
> > +                       local-timer-stop;
> > +                       arm,psci-suspend-param = <0x00010002>;
> > +               };
> > +
> > +               CLUSTER_PD: cluster_pd {
> > +                       compatible = "arm,idle-state";
> > +                       entry-latency-us = <1000>;
> > +                       exit-latency-us = <1000>;
> > +                       min-residency-us = <3000>;
> > +                       local-timer-stop;
> > +                       arm,psci-suspend-param = <0x01010003>;
> > +               };
> > +
> > +               DEEP_SLEEP: deep_sleep {
> > +                       compatible = "arm,idle-state";
> > +                       wakeup-latency-us = <0xffffffff>;
> 
> A value > 4294 seconds(i.e >1 hour) seems suspicious.
> Are you working around the firmware issue with high latency value so
> that it's never entered ? Why not remove advertising the state from DT.
>

Haved checked with related colleagues, this node 'deep_sleep' was not for working
around any firmware issue, but was a trick utilization of idle subsystem, and that
was definitely not elegant, the author indeed intendly didn't want CPU entered this
state, I will remove this node therefore.
 
> Can you get me the dump of:
> grep "" /sys/devices/system/cpu/cpu*/cpuidle/state*/{time,usage}
> 

FYI: https://www.irccloud.com/pastebin/XyEMLzfq/

Thanks,
Chunyan

> IIUC, you might have seen boot issue without this values and workaround
> the issue with such high values ? If so please drop this state.
> 
> > +                       entry-latency-us = <1500>;
> > +                       exit-latency-us = <1500>;
> > +                       min-residency-us = <0xffffffff>;
> > +                       local-timer-stop;
> > +                       arm,psci-suspend-param = <0x01010005>;
> > +               };
> > +       };
> >
> 
> --
> Regards,
> Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
To: Sudeep Holla <sudeep.holla@arm.com>
Cc: Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will.deacon@arm.com>, Arnd Bergmann <arnd@arndb.de>,
	<devicetree@vger.kernel.org>, <orson.zhai@spreadtrum.com>,
	open list <linux-kernel@vger.kernel.org>,
	linux-arm <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 1/5] arm64: dts: Add basic DT to support Spreadtrum's SP9860G
Date: Fri, 17 Feb 2017 15:28:39 +0800	[thread overview]
Message-ID: <20170217072839.GA8767@spreadtrum.com> (raw)
In-Reply-To: <CAPKp9ub35nL8gc_TKx02pSHHNwBhiMacWL=CuEmNGnecWikfGQ@mail.gmail.com>

Hi Sudeep,

On 二,  2月 14, 2017 at 04:44:53下午 +0000, Sudeep Holla wrote:
> On Tue, Feb 14, 2017 at 9:19 AM, Chunyan Zhang
> <chunyan.zhang@spreadtrum.com> wrote:
> > From: Orson Zhai <orson.zhai@spreadtrum.com>
> >
> > SC9860G is a 8 cores of A53 SoC with 4G LTE support SoC from Spreadtrum.
> >
> > According to regular hierarchy of sprd dts, whale2.dtsi contains SoC
> > peripherals IP nodes, sc9860.dtsi contains stuff related to ARM core stuff
> > and sp9860g dts is for the board level.
> >
> > Signed-off-by: Orson Zhai <orson.zhai@spreadtrum.com>
> > Signed-off-by: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
> > ---
> >  arch/arm64/boot/dts/sprd/Makefile         |   3 +-
> >  arch/arm64/boot/dts/sprd/sc9860.dtsi      | 534 ++++++++++++++++++++++++++++++
> >  arch/arm64/boot/dts/sprd/sp9860g-1h10.dts |  58 ++++
> >  arch/arm64/boot/dts/sprd/whale2.dtsi      |  66 ++++
> >  4 files changed, 660 insertions(+), 1 deletion(-)
> >  create mode 100644 arch/arm64/boot/dts/sprd/sc9860.dtsi
> >  create mode 100644 arch/arm64/boot/dts/sprd/sp9860g-1h10.dts
> >  create mode 100644 arch/arm64/boot/dts/sprd/whale2.dtsi
> >
> > diff --git a/arch/arm64/boot/dts/sprd/Makefile b/arch/arm64/boot/dts/sprd/Makefile
> > index b658c5e..f0535e6 100644
> > --- a/arch/arm64/boot/dts/sprd/Makefile
> > +++ b/arch/arm64/boot/dts/sprd/Makefile
> > @@ -1,4 +1,5 @@
> > -dtb-$(CONFIG_ARCH_SPRD) += sc9836-openphone.dtb
> > +dtb-$(CONFIG_ARCH_SPRD) += sc9836-openphone.dtb \
> > +                       sp9860g-1h10.dtb
> >
> >  always         := $(dtb-y)
> >  subdir-y       := $(dts-dirs)
> > diff --git a/arch/arm64/boot/dts/sprd/sc9860.dtsi b/arch/arm64/boot/dts/sprd/sc9860.dtsi
> > new file mode 100644
> > index 0000000..604a8c9
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/sprd/sc9860.dtsi
> > @@ -0,0 +1,534 @@
> 
> [...]
> 
> > +       idle-states{
> > +               entry-method = "arm,psci";
> > +
> > +               CORE_PD: core_pd {
> > +                       compatible = "arm,idle-state";
> > +                       entry-latency-us = <1000>;
> > +                       exit-latency-us = <700>;
> > +                       min-residency-us = <2500>;
> > +                       local-timer-stop;
> > +                       arm,psci-suspend-param = <0x00010002>;
> > +               };
> > +
> > +               CLUSTER_PD: cluster_pd {
> > +                       compatible = "arm,idle-state";
> > +                       entry-latency-us = <1000>;
> > +                       exit-latency-us = <1000>;
> > +                       min-residency-us = <3000>;
> > +                       local-timer-stop;
> > +                       arm,psci-suspend-param = <0x01010003>;
> > +               };
> > +
> > +               DEEP_SLEEP: deep_sleep {
> > +                       compatible = "arm,idle-state";
> > +                       wakeup-latency-us = <0xffffffff>;
> 
> A value > 4294 seconds(i.e >1 hour) seems suspicious.
> Are you working around the firmware issue with high latency value so
> that it's never entered ? Why not remove advertising the state from DT.
>

Haved checked with related colleagues, this node 'deep_sleep' was not for working
around any firmware issue, but was a trick utilization of idle subsystem, and that
was definitely not elegant, the author indeed intendly didn't want CPU entered this
state, I will remove this node therefore.
 
> Can you get me the dump of:
> grep "" /sys/devices/system/cpu/cpu*/cpuidle/state*/{time,usage}
> 

FYI: https://www.irccloud.com/pastebin/XyEMLzfq/

Thanks,
Chunyan

> IIUC, you might have seen boot issue without this values and workaround
> the issue with such high values ? If so please drop this state.
> 
> > +                       entry-latency-us = <1500>;
> > +                       exit-latency-us = <1500>;
> > +                       min-residency-us = <0xffffffff>;
> > +                       local-timer-stop;
> > +                       arm,psci-suspend-param = <0x01010005>;
> > +               };
> > +       };
> >
> 
> --
> Regards,
> Sudeep

  reply	other threads:[~2017-02-17  7:28 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-14  9:19 [PATCH 0/5] Add Spreadtrum SP9860G support Chunyan Zhang
2017-02-14  9:19 ` Chunyan Zhang
2017-02-14  9:19 ` Chunyan Zhang
2017-02-14  9:19 ` [PATCH 1/5] arm64: dts: Add basic DT to support Spreadtrum's SP9860G Chunyan Zhang
2017-02-14  9:19   ` Chunyan Zhang
2017-02-14  9:19   ` Chunyan Zhang
2017-02-14 15:57   ` Mark Rutland
2017-02-14 15:57     ` Mark Rutland
2017-02-14 15:57     ` Mark Rutland
2017-02-16  7:57     ` Chunyan Zhang
2017-02-16  7:57       ` Chunyan Zhang
2017-02-16  7:57       ` Chunyan Zhang
2017-02-14 16:44   ` Sudeep Holla
2017-02-14 16:44     ` Sudeep Holla
2017-02-14 16:44     ` Sudeep Holla
2017-02-17  7:28     ` Chunyan Zhang [this message]
2017-02-17  7:28       ` Chunyan Zhang
2017-02-17  7:28       ` Chunyan Zhang
2017-02-17 10:28       ` Sudeep Holla
2017-02-17 10:28         ` Sudeep Holla
2017-02-17 10:28         ` Sudeep Holla
2017-02-20  9:37         ` Chunyan Zhang
2017-02-20  9:37           ` Chunyan Zhang
2017-02-20  9:37           ` Chunyan Zhang
2017-02-20 10:47           ` Sudeep Holla
2017-02-20 10:47             ` Sudeep Holla
2017-02-20 10:47             ` Sudeep Holla
2017-02-20 12:56             ` Chunyan Zhang
2017-02-20 12:56               ` Chunyan Zhang
2017-02-20 12:56               ` Chunyan Zhang
2017-02-14  9:19 ` [PATCH 2/5] Documentation: sprd: Add bindings for SP9860G Chunyan Zhang
2017-02-14  9:19   ` Chunyan Zhang
2017-02-14  9:19   ` Chunyan Zhang
2017-02-14  9:19 ` [PATCH 3/5] devicetree: bindings: revise compatible string of sprd uart Chunyan Zhang
2017-02-14  9:19   ` Chunyan Zhang
2017-02-14  9:19   ` Chunyan Zhang
2017-02-14 17:02   ` Rob Herring
2017-02-14 17:02     ` Rob Herring
2017-02-14 17:02     ` Rob Herring
2017-02-14  9:19 ` [PATCH 4/5] sprd_serial: switch comptible string to sc-uart Chunyan Zhang
2017-02-14  9:19   ` Chunyan Zhang
2017-02-14  9:19   ` Chunyan Zhang
2017-02-16 13:31   ` Arnd Bergmann
2017-02-16 13:31     ` Arnd Bergmann
2017-02-16 13:31     ` Arnd Bergmann
2017-02-17  7:35     ` Chunyan Zhang
2017-02-17  7:35       ` Chunyan Zhang
2017-02-17  7:35       ` Chunyan Zhang
2017-02-14  9:19 ` [PATCH 5/5] serial: sprd: adjust TIMEOUT to a big value Chunyan Zhang
2017-02-14  9:19   ` Chunyan Zhang
2017-02-14  9:19   ` Chunyan Zhang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170217072839.GA8767@spreadtrum.com \
    --to=chunyan.zhang@spreadtrum.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.