Cc: [email protected]
Signed-off-by: Lee Jones <[email protected]>
---
Changelog:
- Using OPP-v2
- Moved (and linked) a bunch of documentation to ../power/
.../devicetree/bindings/cpufreq/cpufreq-st.txt | 40 ++++++++++++++++++++++
1 file changed, 40 insertions(+)
create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-st.txt
diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-st.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-st.txt
new file mode 100644
index 0000000..79add9d
--- /dev/null
+++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-st.txt
@@ -0,0 +1,40 @@
+Binding for ST's CPUFreq driver
+===============================
+
+Frequency Scaling only
+----------------------
+
+Located in CPU's node:
+
+- operating-points : [See: ../power/opp.txt]
+
+Example [safe]
+--------------
+
+cpus {
+ cpu@0 {
+ /* kHz uV */
+ operating-points = <1500000 0
+ 1200000 0
+ 800000 0
+ 500000 0>;
+ };
+};
+
+Dynamic Voltage and Frequency Scaling (DVFS)
+--------------------------------------------
+
+Located in CPU's node:
+
+- operating-points-v2-sti : [See ../power/opp-st.txt]
+
+Example [unsafe]
+----------------
+
+cpus {
+ cpu@0 {
+ operating-points-v2 = <[OPP list phandle]>;
+ };
+};
+
+For an example of an OPP list, see ../power/opp-st.txt.
--
1.9.1
These OPPs are used in ST's CPUFreq implementation.
Signed-off-by: Lee Jones <[email protected]>
---
Changelog:
- None, new patch
Documentation/devicetree/bindings/power/opp-st.txt | 76 ++++++++++++++++++++++
1 file changed, 76 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/opp-st.txt
diff --git a/Documentation/devicetree/bindings/power/opp-st.txt b/Documentation/devicetree/bindings/power/opp-st.txt
new file mode 100644
index 0000000..6eb2a91
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/opp-st.txt
@@ -0,0 +1,76 @@
+STMicroelectronics OPP (Operating Performance Points) Bindings
+--------------------------------------------------------------
+
+Frequency Scaling only
+----------------------
+
+Located in CPU's node:
+
+- operating-points : [See: ./opp.txt]
+
+Example [safe]
+--------------
+
+cpus {
+ cpu@0 {
+ /* kHz uV */
+ operating-points = <1500000 0
+ 1200000 0
+ 800000 0
+ 500000 0>;
+ };
+};
+
+Dynamic Voltage and Frequency Scaling (DVFS)
+--------------------------------------------
+
+Located in 'cpu0-opp-list' node [to be provided ONLY by the bootloader]:
+
+- compatible : Should be "operating-points-v2-sti"
+- opp{1..N} : Each 'oppX' subnode will contain the following properties:
+ - opp-hz : CPU frequency [Hz] for this OPP [See: ./opp.txt]
+ - st,avs : List of available voltages [uV] indexed by process code
+ - st,cuts : Cut version this OPP is suitable for [0xFF means ALL]
+ - st,substrate : Substrate version this OPP is suitable for [0xFF means ALL]
+- st,syscfg : Phandle to Major number register
+ First cell: offset to major number
+- st,syscfg-eng : Phandle to Minor number and Pcode registers
+ First cell: offset to process code
+ Second cell: offset to minor number
+
+WARNING: The opp{1..N} nodes will be provided by the bootloader. Do not attempt to
+ artificially synthesise the opp{1..N} nodes or any of their descendants.
+ They are very platform specific and may damage the hardware if created
+ incorrectly.
+
+Example [unsafe]
+----------------
+
+cpus {
+ cpu@0 {
+ operating-points-v2 = <&cpu0_opp_list>;
+ };
+};
+
+/* ############################################################ */
+/* # WARNING: Do not attempt to copy/replicate this node, # */
+/* # it is only to be supplied by the bootloader !!! # */
+/* ############################################################ */
+cpu0-opp-list {
+ compatible = "operating-points-v2-sti";
+ st,syscfg = <&syscfg [major_offset]>;
+ st,syscfg-eng = <&syscfg_eng [pcode_offset] [minor_offset]>;
+
+ opp0 {
+ opp-hz = <1200000000>;
+ st,avs = <1110 1150 1100 1080 1040 1020 980 930>;
+ st,substrate = <0xff>;
+ st,cuts = <0xff>;
+ };
+ opp1 {
+ opp-hz = <1500000000>;
+ st,avs = <1200 1200 1200 1200 1170 1140 1100 1070>;
+ st,substrate = <0xff>;
+ st,cuts = <0x2>;
+ };
+};
--
1.9.1
On 27-07-15, 16:20, Lee Jones wrote:
> Cc: [email protected]
> Signed-off-by: Lee Jones <[email protected]>
> ---
>
> Changelog:
> - Using OPP-v2
> - Moved (and linked) a bunch of documentation to ../power/
>
> .../devicetree/bindings/cpufreq/cpufreq-st.txt | 40 ++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-st.txt
>
> diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-st.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-st.txt
> new file mode 100644
> index 0000000..79add9d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-st.txt
> @@ -0,0 +1,40 @@
> +Binding for ST's CPUFreq driver
> +===============================
> +
> +Frequency Scaling only
> +----------------------
> +
> +Located in CPU's node:
> +
> +- operating-points : [See: ../power/opp.txt]
> +
> +Example [safe]
> +--------------
> +
> +cpus {
> + cpu@0 {
> + /* kHz uV */
> + operating-points = <1500000 0
> + 1200000 0
> + 800000 0
> + 500000 0>;
> + };
> +};
> +
> +Dynamic Voltage and Frequency Scaling (DVFS)
> +--------------------------------------------
> +
> +Located in CPU's node:
> +
> +- operating-points-v2-sti : [See ../power/opp-st.txt]
> +
> +Example [unsafe]
> +----------------
> +
> +cpus {
> + cpu@0 {
> + operating-points-v2 = <[OPP list phandle]>;
> + };
> +};
> +
> +For an example of an OPP list, see ../power/opp-st.txt.
I don't think you need this file/patch at all.. It is almost blank and
doesn't define a binding. And so there is no need to specify this at
all. Just an OPP binding file is enough.
--
viresh
Cc'ing few people (whom I cc'd last time as well :)).
On 27-07-15, 16:20, Lee Jones wrote:
> These OPPs are used in ST's CPUFreq implementation.
>
> Signed-off-by: Lee Jones <[email protected]>
> ---
>
> Changelog:
> - None, new patch
>
> Documentation/devicetree/bindings/power/opp-st.txt | 76 ++++++++++++++++++++++
> 1 file changed, 76 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/power/opp-st.txt
>
> diff --git a/Documentation/devicetree/bindings/power/opp-st.txt b/Documentation/devicetree/bindings/power/opp-st.txt
> new file mode 100644
> index 0000000..6eb2a91
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/opp-st.txt
> @@ -0,0 +1,76 @@
> +STMicroelectronics OPP (Operating Performance Points) Bindings
> +--------------------------------------------------------------
> +
> +Frequency Scaling only
> +----------------------
> +
> +Located in CPU's node:
> +
> +- operating-points : [See: ./opp.txt]
> +
> +Example [safe]
> +--------------
> +
> +cpus {
> + cpu@0 {
> + /* kHz uV */
> + operating-points = <1500000 0
> + 1200000 0
> + 800000 0
> + 500000 0>;
> + };
> +};
> +
> +Dynamic Voltage and Frequency Scaling (DVFS)
> +--------------------------------------------
> +
> +Located in 'cpu0-opp-list' node [to be provided ONLY by the bootloader]:
> +
> +- compatible : Should be "operating-points-v2-sti"
> +- opp{1..N} : Each 'oppX' subnode will contain the following properties:
Or should we mention:
- opp{1..N} : Each 'oppX' subnode shall contain below properties,
over what ./opp.txt defines:
?
> + - opp-hz : CPU frequency [Hz] for this OPP [See: ./opp.txt]
> + - st,avs : List of available voltages [uV] indexed by process code
> + - st,cuts : Cut version this OPP is suitable for [0xFF means ALL]
> + - st,substrate : Substrate version this OPP is suitable for [0xFF means ALL]
> +- st,syscfg : Phandle to Major number register
> + First cell: offset to major number
> +- st,syscfg-eng : Phandle to Minor number and Pcode registers
> + First cell: offset to process code
> + Second cell: offset to minor number
> +
> +WARNING: The opp{1..N} nodes will be provided by the bootloader. Do not attempt to
> + artificially synthesise the opp{1..N} nodes or any of their descendants.
> + They are very platform specific and may damage the hardware if created
> + incorrectly.
> +
> +Example [unsafe]
> +----------------
> +
> +cpus {
> + cpu@0 {
> + operating-points-v2 = <&cpu0_opp_list>;
> + };
> +};
> +
> +/* ############################################################ */
> +/* # WARNING: Do not attempt to copy/replicate this node, # */
> +/* # it is only to be supplied by the bootloader !!! # */
> +/* ############################################################ */
> +cpu0-opp-list {
> + compatible = "operating-points-v2-sti";
> + st,syscfg = <&syscfg [major_offset]>;
> + st,syscfg-eng = <&syscfg_eng [pcode_offset] [minor_offset]>;
> +
> + opp0 {
> + opp-hz = <1200000000>;
> + st,avs = <1110 1150 1100 1080 1040 1020 980 930>;
> + st,substrate = <0xff>;
> + st,cuts = <0xff>;
> + };
> + opp1 {
> + opp-hz = <1500000000>;
> + st,avs = <1200 1200 1200 1200 1170 1140 1100 1070>;
> + st,substrate = <0xff>;
> + st,cuts = <0x2>;
> + };
> +};
I don't see more problems here, unless we can move some of this to the
generic bindings.
@Rob/Stephen: Please respond before it is late :)
--
viresh
On Tue, 28 Jul 2015, Viresh Kumar wrote:
> Cc'ing few people (whom I cc'd last time as well :)).
>
> On 27-07-15, 16:20, Lee Jones wrote:
> > These OPPs are used in ST's CPUFreq implementation.
> >
> > Signed-off-by: Lee Jones <[email protected]>
> > ---
> >
> > Changelog:
> > - None, new patch
> >
> > Documentation/devicetree/bindings/power/opp-st.txt | 76 ++++++++++++++++++++++
> > 1 file changed, 76 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/power/opp-st.txt
> >
> > diff --git a/Documentation/devicetree/bindings/power/opp-st.txt b/Documentation/devicetree/bindings/power/opp-st.txt
> > new file mode 100644
> > index 0000000..6eb2a91
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/opp-st.txt
> > @@ -0,0 +1,76 @@
> > +STMicroelectronics OPP (Operating Performance Points) Bindings
> > +--------------------------------------------------------------
> > +
> > +Frequency Scaling only
> > +----------------------
> > +
> > +Located in CPU's node:
> > +
> > +- operating-points : [See: ./opp.txt]
> > +
> > +Example [safe]
> > +--------------
> > +
> > +cpus {
> > + cpu@0 {
> > + /* kHz uV */
> > + operating-points = <1500000 0
> > + 1200000 0
> > + 800000 0
> > + 500000 0>;
> > + };
> > +};
> > +
> > +Dynamic Voltage and Frequency Scaling (DVFS)
> > +--------------------------------------------
> > +
> > +Located in 'cpu0-opp-list' node [to be provided ONLY by the bootloader]:
> > +
> > +- compatible : Should be "operating-points-v2-sti"
> > +- opp{1..N} : Each 'oppX' subnode will contain the following properties:
>
> Or should we mention:
> - opp{1..N} : Each 'oppX' subnode shall contain below properties,
> over what ./opp.txt defines:
>
> ?
I disagree. For one, only 'opp-hz' is defined in ./opp.tx. Secondly
it would be annoying to have to have to keep jumping between documents
to obtain the whole picture. Finally, generic bindings are repeated
in platform/device specific documentation all the time. Grep for
'clocks' or 'regulator-* or 'interrupts' or 'reg' or 'clock-frequency'
(which IMHO I think you should have used instead of 'opp-hz', but
that's by the by), or any number of other generic properties.
> > + - opp-hz : CPU frequency [Hz] for this OPP [See: ./opp.txt]
> > + - st,avs : List of available voltages [uV] indexed by process code
> > + - st,cuts : Cut version this OPP is suitable for [0xFF means ALL]
> > + - st,substrate : Substrate version this OPP is suitable for [0xFF means ALL]
> > +- st,syscfg : Phandle to Major number register
> > + First cell: offset to major number
> > +- st,syscfg-eng : Phandle to Minor number and Pcode registers
> > + First cell: offset to process code
> > + Second cell: offset to minor number
> > +
> > +WARNING: The opp{1..N} nodes will be provided by the bootloader. Do not attempt to
> > + artificially synthesise the opp{1..N} nodes or any of their descendants.
> > + They are very platform specific and may damage the hardware if created
> > + incorrectly.
> > +
> > +Example [unsafe]
> > +----------------
> > +
> > +cpus {
> > + cpu@0 {
> > + operating-points-v2 = <&cpu0_opp_list>;
> > + };
> > +};
> > +
> > +/* ############################################################ */
> > +/* # WARNING: Do not attempt to copy/replicate this node, # */
> > +/* # it is only to be supplied by the bootloader !!! # */
> > +/* ############################################################ */
> > +cpu0-opp-list {
> > + compatible = "operating-points-v2-sti";
> > + st,syscfg = <&syscfg [major_offset]>;
> > + st,syscfg-eng = <&syscfg_eng [pcode_offset] [minor_offset]>;
> > +
> > + opp0 {
> > + opp-hz = <1200000000>;
> > + st,avs = <1110 1150 1100 1080 1040 1020 980 930>;
> > + st,substrate = <0xff>;
> > + st,cuts = <0xff>;
> > + };
> > + opp1 {
> > + opp-hz = <1500000000>;
> > + st,avs = <1200 1200 1200 1200 1170 1140 1100 1070>;
> > + st,substrate = <0xff>;
> > + st,cuts = <0x2>;
> > + };
> > +};
>
> I don't see more problems here, unless we can move some of this to the
> generic bindings.
>
> @Rob/Stephen: Please respond before it is late :)
No one knows this stuff better than you. If you can't think of an
already existing binding that could suit to portray our 'cuts' and
'substrate' information (with a similar way to support our "all cuts"
and "all substrates" options, then there probably isn't one. ;)
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Tue, 28 Jul 2015, Viresh Kumar wrote:
> On 27-07-15, 16:20, Lee Jones wrote:
> > Cc: [email protected]
> > Signed-off-by: Lee Jones <[email protected]>
> > ---
> >
> > Changelog:
> > - Using OPP-v2
> > - Moved (and linked) a bunch of documentation to ../power/
> >
> > .../devicetree/bindings/cpufreq/cpufreq-st.txt | 40 ++++++++++++++++++++++
> > 1 file changed, 40 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-st.txt
> >
> > diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-st.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-st.txt
> > new file mode 100644
> > index 0000000..79add9d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-st.txt
> > @@ -0,0 +1,40 @@
> > +Binding for ST's CPUFreq driver
> > +===============================
> > +
> > +Frequency Scaling only
> > +----------------------
> > +
> > +Located in CPU's node:
> > +
> > +- operating-points : [See: ../power/opp.txt]
> > +
> > +Example [safe]
> > +--------------
> > +
> > +cpus {
> > + cpu@0 {
> > + /* kHz uV */
> > + operating-points = <1500000 0
> > + 1200000 0
> > + 800000 0
> > + 500000 0>;
> > + };
> > +};
> > +
> > +Dynamic Voltage and Frequency Scaling (DVFS)
> > +--------------------------------------------
> > +
> > +Located in CPU's node:
> > +
> > +- operating-points-v2-sti : [See ../power/opp-st.txt]
> > +
> > +Example [unsafe]
> > +----------------
> > +
> > +cpus {
> > + cpu@0 {
> > + operating-points-v2 = <[OPP list phandle]>;
> > + };
> > +};
> > +
> > +For an example of an OPP list, see ../power/opp-st.txt.
>
> I don't think you need this file/patch at all.. It is almost blank and
> doesn't define a binding. And so there is no need to specify this at
> all. Just an OPP binding file is enough.
I have two issues with that. Firstly, although the driver uses the
OPP API (it also uses the Regulator and Clock API too), it is
fundamentally a CPUFreq driver, so I think it should have a CPUFreq
DT entry. Secondly, if someone doesn't know the history of the
ST CPUFreq set, they will look here for an accompanying document. I
personally wouldn't think to look in power/*opp* for a CPUFreq
binding.
Perhaps, as all of the CPUFreq drivers use the OPP API, everything
should be moved to drivers/base/power or drivers/power?
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On 28-07-15, 08:34, Lee Jones wrote:
> I disagree. For one, only 'opp-hz' is defined in ./opp.tx. Secondly
There are other properties in op.txt like turbo, opp-suspend, latency,
etc.. which can be useful for your platform to. Its not used for now
is a different thing.
> it would be annoying to have to have to keep jumping between documents
> to obtain the whole picture. Finally, generic bindings are repeated
> in platform/device specific documentation all the time. Grep for
> 'clocks' or 'regulator-* or 'interrupts' or 'reg' or 'clock-frequency'
Yeah, I agree. I am not against that. What I meant to say was that its
an extension to opp-v2. So whatever is present in opp-v2 can be used
by ST's driver, without mentioning that here as well.
> (which IMHO I think you should have used instead of 'opp-hz', but
> that's by the by), or any number of other generic properties.
Hmm, probably yes. See I don't know everything :)
> > @Rob/Stephen: Please respond before it is late :)
>
> No one knows this stuff better than you. If you can't think of an
> already existing binding that could suit to portray our 'cuts' and
> 'substrate' information (with a similar way to support our "all cuts"
> and "all substrates" options, then there probably isn't one. ;)
Oh, I wasn't saying that there is an existing binding for supporting
your case. But if we want to move the cuts property to the generic
bindings so that others can use it. :)
--
viresh
Cc'ing Rob as well..
On 28-07-15, 08:41, Lee Jones wrote:
> I have two issues with that. Firstly, although the driver uses the
> OPP API (it also uses the Regulator and Clock API too), it is
> fundamentally a CPUFreq driver, so I think it should have a CPUFreq
> DT entry. Secondly, if someone doesn't know the history of the
> ST CPUFreq set, they will look here for an accompanying document. I
> personally wouldn't think to look in power/*opp* for a CPUFreq
> binding.
>
> Perhaps, as all of the CPUFreq drivers use the OPP API, everything
> should be moved to drivers/base/power or drivers/power?
Okay, looks fine :)
--
viresh
On Tue, 28 Jul 2015, Viresh Kumar wrote:
> On 28-07-15, 08:34, Lee Jones wrote:
> > I disagree. For one, only 'opp-hz' is defined in ./opp.tx. Secondly
>
> There are other properties in op.txt like turbo, opp-suspend, latency,
> etc.. which can be useful for your platform to. Its not used for now
> is a different thing.
>
> > it would be annoying to have to have to keep jumping between documents
> > to obtain the whole picture. Finally, generic bindings are repeated
> > in platform/device specific documentation all the time. Grep for
> > 'clocks' or 'regulator-* or 'interrupts' or 'reg' or 'clock-frequency'
>
> Yeah, I agree. I am not against that. What I meant to say was that its
> an extension to opp-v2. So whatever is present in opp-v2 can be used
> by ST's driver, without mentioning that here as well.
>
> > (which IMHO I think you should have used instead of 'opp-hz', but
> > that's by the by), or any number of other generic properties.
>
> Hmm, probably yes. See I don't know everything :)
>
> > > @Rob/Stephen: Please respond before it is late :)
> >
> > No one knows this stuff better than you. If you can't think of an
> > already existing binding that could suit to portray our 'cuts' and
> > 'substrate' information (with a similar way to support our "all cuts"
> > and "all substrates" options, then there probably isn't one. ;)
>
> Oh, I wasn't saying that there is an existing binding for supporting
> your case. But if we want to move the cuts property to the generic
> bindings so that others can use it. :)
Okay, so what are we waiting for before you'd be prepared to Ack the
binding?
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On 27-07-15, 16:20, Lee Jones wrote:
> Cc: [email protected]
> Signed-off-by: Lee Jones <[email protected]>
> ---
>
> Changelog:
> - Using OPP-v2
> - Moved (and linked) a bunch of documentation to ../power/
>
> .../devicetree/bindings/cpufreq/cpufreq-st.txt | 40 ++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-st.txt
For both patches:
Acked-by: Viresh Kumar <[email protected]>
But I would like Rob to have a look as well :)
--
viresh
On Tue, 28 Jul 2015, Viresh Kumar wrote:
> On 27-07-15, 16:20, Lee Jones wrote:
> > Cc: [email protected]
> > Signed-off-by: Lee Jones <[email protected]>
> > ---
> >
> > Changelog:
> > - Using OPP-v2
> > - Moved (and linked) a bunch of documentation to ../power/
> >
> > .../devicetree/bindings/cpufreq/cpufreq-st.txt | 40 ++++++++++++++++++++++
> > 1 file changed, 40 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-st.txt
>
> For both patches:
>
> Acked-by: Viresh Kumar <[email protected]>
W00t! I'll start fixing up the driver.
> But I would like Rob to have a look as well :)
No problem.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Mon, Jul 27, 2015 at 10:20 AM, Lee Jones <[email protected]> wrote:
> These OPPs are used in ST's CPUFreq implementation.
>
> Signed-off-by: Lee Jones <[email protected]>
> ---
>
> Changelog:
> - None, new patch
>
> Documentation/devicetree/bindings/power/opp-st.txt | 76 ++++++++++++++++++++++
> 1 file changed, 76 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/power/opp-st.txt
>
> diff --git a/Documentation/devicetree/bindings/power/opp-st.txt b/Documentation/devicetree/bindings/power/opp-st.txt
> new file mode 100644
> index 0000000..6eb2a91
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/opp-st.txt
> @@ -0,0 +1,76 @@
> +STMicroelectronics OPP (Operating Performance Points) Bindings
> +--------------------------------------------------------------
> +
> +Frequency Scaling only
> +----------------------
> +
> +Located in CPU's node:
> +
> +- operating-points : [See: ./opp.txt]
> +
> +Example [safe]
> +--------------
> +
> +cpus {
> + cpu@0 {
> + /* kHz uV */
> + operating-points = <1500000 0
> + 1200000 0
> + 800000 0
> + 500000 0>;
> + };
> +};
> +
> +Dynamic Voltage and Frequency Scaling (DVFS)
> +--------------------------------------------
> +
> +Located in 'cpu0-opp-list' node [to be provided ONLY by the bootloader]:
> +
> +- compatible : Should be "operating-points-v2-sti"
> +- opp{1..N} : Each 'oppX' subnode will contain the following properties:
> + - opp-hz : CPU frequency [Hz] for this OPP [See: ./opp.txt]
> + - st,avs : List of available voltages [uV] indexed by process code
Add a unit suffix (-microvolt).
> + - st,cuts : Cut version this OPP is suitable for [0xFF means ALL]
> + - st,substrate : Substrate version this OPP is suitable for [0xFF means ALL]
How about not present means all?
> +- st,syscfg : Phandle to Major number register
> + First cell: offset to major number
> +- st,syscfg-eng : Phandle to Minor number and Pcode registers
> + First cell: offset to process code
> + Second cell: offset to minor number
Would the proposed nvmem binding work for this?
Rob
On Tue, 28 Jul 2015, Rob Herring wrote:
> On Mon, Jul 27, 2015 at 10:20 AM, Lee Jones <[email protected]> wrote:
> > These OPPs are used in ST's CPUFreq implementation.
> >
> > Signed-off-by: Lee Jones <[email protected]>
> > ---
> >
> > Changelog:
> > - None, new patch
> >
> > Documentation/devicetree/bindings/power/opp-st.txt | 76 ++++++++++++++++++++++
> > 1 file changed, 76 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/power/opp-st.txt
> >
> > diff --git a/Documentation/devicetree/bindings/power/opp-st.txt b/Documentation/devicetree/bindings/power/opp-st.txt
> > new file mode 100644
> > index 0000000..6eb2a91
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/opp-st.txt
> > @@ -0,0 +1,76 @@
> > +STMicroelectronics OPP (Operating Performance Points) Bindings
> > +--------------------------------------------------------------
> > +
> > +Frequency Scaling only
> > +----------------------
> > +
> > +Located in CPU's node:
> > +
> > +- operating-points : [See: ./opp.txt]
> > +
> > +Example [safe]
> > +--------------
> > +
> > +cpus {
> > + cpu@0 {
> > + /* kHz uV */
> > + operating-points = <1500000 0
> > + 1200000 0
> > + 800000 0
> > + 500000 0>;
> > + };
> > +};
> > +
> > +Dynamic Voltage and Frequency Scaling (DVFS)
> > +--------------------------------------------
> > +
> > +Located in 'cpu0-opp-list' node [to be provided ONLY by the bootloader]:
> > +
> > +- compatible : Should be "operating-points-v2-sti"
> > +- opp{1..N} : Each 'oppX' subnode will contain the following properties:
> > + - opp-hz : CPU frequency [Hz] for this OPP [See: ./opp.txt]
> > + - st,avs : List of available voltages [uV] indexed by process code
>
> Add a unit suffix (-microvolt).
Sure.
> > + - st,cuts : Cut version this OPP is suitable for [0xFF means ALL]
> > + - st,substrate : Substrate version this OPP is suitable for [0xFF means ALL]
>
> How about not present means all?
I think that would be an unsafe assumption. If it's forgotten, then
we may try to run an invalid/dangerous voltage/frequency combination.
I'd really like 'all' to be defined an deliberate.
> > +- st,syscfg : Phandle to Major number register
> > + First cell: offset to major number
> > +- st,syscfg-eng : Phandle to Minor number and Pcode registers
> > + First cell: offset to process code
> > + Second cell: offset to minor number
>
> Would the proposed nvmem binding work for this?
We already use sysconf for all of this type of stuff, as this
information is contained in ST's Sysconf banks. Moving over to a new
API would be a huge move and would require lots of planning
discussions with ST.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Tue, Jul 28, 2015 at 9:39 AM, Lee Jones <[email protected]> wrote:
> On Tue, 28 Jul 2015, Rob Herring wrote:
>
>> On Mon, Jul 27, 2015 at 10:20 AM, Lee Jones <[email protected]> wrote:
>> > These OPPs are used in ST's CPUFreq implementation.
>> >
>> > Signed-off-by: Lee Jones <[email protected]>
>> > ---
>> >
>> > Changelog:
>> > - None, new patch
>> >
>> > Documentation/devicetree/bindings/power/opp-st.txt | 76 ++++++++++++++++++++++
>> > 1 file changed, 76 insertions(+)
>> > create mode 100644 Documentation/devicetree/bindings/power/opp-st.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/power/opp-st.txt b/Documentation/devicetree/bindings/power/opp-st.txt
>> > new file mode 100644
>> > index 0000000..6eb2a91
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/power/opp-st.txt
>> > @@ -0,0 +1,76 @@
>> > +STMicroelectronics OPP (Operating Performance Points) Bindings
>> > +--------------------------------------------------------------
>> > +
>> > +Frequency Scaling only
>> > +----------------------
>> > +
>> > +Located in CPU's node:
>> > +
>> > +- operating-points : [See: ./opp.txt]
>> > +
>> > +Example [safe]
>> > +--------------
>> > +
>> > +cpus {
>> > + cpu@0 {
>> > + /* kHz uV */
>> > + operating-points = <1500000 0
>> > + 1200000 0
>> > + 800000 0
>> > + 500000 0>;
>> > + };
>> > +};
>> > +
>> > +Dynamic Voltage and Frequency Scaling (DVFS)
>> > +--------------------------------------------
>> > +
>> > +Located in 'cpu0-opp-list' node [to be provided ONLY by the bootloader]:
>> > +
>> > +- compatible : Should be "operating-points-v2-sti"
>> > +- opp{1..N} : Each 'oppX' subnode will contain the following properties:
>> > + - opp-hz : CPU frequency [Hz] for this OPP [See: ./opp.txt]
>> > + - st,avs : List of available voltages [uV] indexed by process code
>>
>> Add a unit suffix (-microvolt).
>
> Sure.
>
>> > + - st,cuts : Cut version this OPP is suitable for [0xFF means ALL]
>> > + - st,substrate : Substrate version this OPP is suitable for [0xFF means ALL]
>>
>> How about not present means all?
>
> I think that would be an unsafe assumption. If it's forgotten, then
> we may try to run an invalid/dangerous voltage/frequency combination.
>
> I'd really like 'all' to be defined an deliberate.
Okay.
>> > +- st,syscfg : Phandle to Major number register
>> > + First cell: offset to major number
>> > +- st,syscfg-eng : Phandle to Minor number and Pcode registers
>> > + First cell: offset to process code
>> > + Second cell: offset to minor number
>>
>> Would the proposed nvmem binding work for this?
>
> We already use sysconf for all of this type of stuff, as this
> information is contained in ST's Sysconf banks. Moving over to a new
> API would be a huge move and would require lots of planning
> discussions with ST.
Okay.
Rob
On Tue, 28 Jul 2015, Rob Herring wrote:
> On Tue, Jul 28, 2015 at 9:39 AM, Lee Jones <[email protected]> wrote:
> > On Tue, 28 Jul 2015, Rob Herring wrote:
> >
> >> On Mon, Jul 27, 2015 at 10:20 AM, Lee Jones <[email protected]> wrote:
> >> > These OPPs are used in ST's CPUFreq implementation.
> >> >
> >> > Signed-off-by: Lee Jones <[email protected]>
> >> > ---
> >> >
> >> > Changelog:
> >> > - None, new patch
> >> >
> >> > Documentation/devicetree/bindings/power/opp-st.txt | 76 ++++++++++++++++++++++
> >> > 1 file changed, 76 insertions(+)
> >> > create mode 100644 Documentation/devicetree/bindings/power/opp-st.txt
> >> >
> >> > diff --git a/Documentation/devicetree/bindings/power/opp-st.txt b/Documentation/devicetree/bindings/power/opp-st.txt
> >> > new file mode 100644
> >> > index 0000000..6eb2a91
> >> > --- /dev/null
> >> > +++ b/Documentation/devicetree/bindings/power/opp-st.txt
> >> > @@ -0,0 +1,76 @@
> >> > +STMicroelectronics OPP (Operating Performance Points) Bindings
> >> > +--------------------------------------------------------------
> >> > +
> >> > +Frequency Scaling only
> >> > +----------------------
> >> > +
> >> > +Located in CPU's node:
> >> > +
> >> > +- operating-points : [See: ./opp.txt]
> >> > +
> >> > +Example [safe]
> >> > +--------------
> >> > +
> >> > +cpus {
> >> > + cpu@0 {
> >> > + /* kHz uV */
> >> > + operating-points = <1500000 0
> >> > + 1200000 0
> >> > + 800000 0
> >> > + 500000 0>;
> >> > + };
> >> > +};
> >> > +
> >> > +Dynamic Voltage and Frequency Scaling (DVFS)
> >> > +--------------------------------------------
> >> > +
> >> > +Located in 'cpu0-opp-list' node [to be provided ONLY by the bootloader]:
> >> > +
> >> > +- compatible : Should be "operating-points-v2-sti"
> >> > +- opp{1..N} : Each 'oppX' subnode will contain the following properties:
> >> > + - opp-hz : CPU frequency [Hz] for this OPP [See: ./opp.txt]
> >> > + - st,avs : List of available voltages [uV] indexed by process code
> >>
> >> Add a unit suffix (-microvolt).
> >
> > Sure.
> >
> >> > + - st,cuts : Cut version this OPP is suitable for [0xFF means ALL]
> >> > + - st,substrate : Substrate version this OPP is suitable for [0xFF means ALL]
> >>
> >> How about not present means all?
> >
> > I think that would be an unsafe assumption. If it's forgotten, then
> > we may try to run an invalid/dangerous voltage/frequency combination.
> >
> > I'd really like 'all' to be defined an deliberate.
>
> Okay.
>
> >> > +- st,syscfg : Phandle to Major number register
> >> > + First cell: offset to major number
> >> > +- st,syscfg-eng : Phandle to Minor number and Pcode registers
> >> > + First cell: offset to process code
> >> > + Second cell: offset to minor number
> >>
> >> Would the proposed nvmem binding work for this?
> >
> > We already use sysconf for all of this type of stuff, as this
> > information is contained in ST's Sysconf banks. Moving over to a new
> > API would be a huge move and would require lots of planning
> > discussions with ST.
>
> Okay.
Thanks.
I'm going to fix up your suggestions above and add your Ack to make
Viresh happy. :)
Thanks Rob.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On 07/28, Viresh Kumar wrote:
> Cc'ing few people (whom I cc'd last time as well :)).
>
> On 27-07-15, 16:20, Lee Jones wrote:
> > These OPPs are used in ST's CPUFreq implementation.
> >
> > Signed-off-by: Lee Jones <[email protected]>
> > ---
> >
> > Changelog:
> > - None, new patch
> >
> > Documentation/devicetree/bindings/power/opp-st.txt | 76 ++++++++++++++++++++++
> > 1 file changed, 76 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/power/opp-st.txt
> >
> > diff --git a/Documentation/devicetree/bindings/power/opp-st.txt b/Documentation/devicetree/bindings/power/opp-st.txt
> > new file mode 100644
> > index 0000000..6eb2a91
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/opp-st.txt
> > @@ -0,0 +1,76 @@
> > +STMicroelectronics OPP (Operating Performance Points) Bindings
> > +--------------------------------------------------------------
> > +
> > +Frequency Scaling only
> > +----------------------
> > +
> > +Located in CPU's node:
> > +
> > +- operating-points : [See: ./opp.txt]
> > +
> > +Example [safe]
> > +--------------
> > +
> > +cpus {
> > + cpu@0 {
> > + /* kHz uV */
> > + operating-points = <1500000 0
> > + 1200000 0
> > + 800000 0
> > + 500000 0>;
> > + };
> > +};
> > +
> > +Dynamic Voltage and Frequency Scaling (DVFS)
> > +--------------------------------------------
> > +
> > +Located in 'cpu0-opp-list' node [to be provided ONLY by the bootloader]:
> > +
> > +- compatible : Should be "operating-points-v2-sti"
> > +- opp{1..N} : Each 'oppX' subnode will contain the following properties:
>
> Or should we mention:
> - opp{1..N} : Each 'oppX' subnode shall contain below properties,
> over what ./opp.txt defines:
>
> ?
>
>
> > + - opp-hz : CPU frequency [Hz] for this OPP [See: ./opp.txt]
> > + - st,avs : List of available voltages [uV] indexed by process code
> > + - st,cuts : Cut version this OPP is suitable for [0xFF means ALL]
> > + - st,substrate : Substrate version this OPP is suitable for [0xFF means ALL]
> > +- st,syscfg : Phandle to Major number register
> > + First cell: offset to major number
> > +- st,syscfg-eng : Phandle to Minor number and Pcode registers
> > + First cell: offset to process code
> > + Second cell: offset to minor number
> > +
> > +WARNING: The opp{1..N} nodes will be provided by the bootloader. Do not attempt to
> > + artificially synthesise the opp{1..N} nodes or any of their descendants.
> > + They are very platform specific and may damage the hardware if created
> > + incorrectly.
> > +
> > +Example [unsafe]
> > +----------------
> > +
> > +cpus {
> > + cpu@0 {
> > + operating-points-v2 = <&cpu0_opp_list>;
> > + };
> > +};
> > +
> > +/* ############################################################ */
> > +/* # WARNING: Do not attempt to copy/replicate this node, # */
> > +/* # it is only to be supplied by the bootloader !!! # */
> > +/* ############################################################ */
> > +cpu0-opp-list {
> > + compatible = "operating-points-v2-sti";
> > + st,syscfg = <&syscfg [major_offset]>;
> > + st,syscfg-eng = <&syscfg_eng [pcode_offset] [minor_offset]>;
> > +
> > + opp0 {
> > + opp-hz = <1200000000>;
> > + st,avs = <1110 1150 1100 1080 1040 1020 980 930>;
> > + st,substrate = <0xff>;
> > + st,cuts = <0xff>;
> > + };
> > + opp1 {
> > + opp-hz = <1500000000>;
> > + st,avs = <1200 1200 1200 1200 1170 1140 1100 1070>;
> > + st,substrate = <0xff>;
> > + st,cuts = <0x2>;
> > + };
> > +};
>
> I don't see more problems here, unless we can move some of this to the
> generic bindings.
>
> @Rob/Stephen: Please respond before it is late :)
>
It's interesting to have vendor specific properties like avs,
cuts, and substrate. That could replace our planned usage of the
opp-names property where we encode similar information (speed
bin, revision, etc.) into a string that we look for.
So I wonder why the avs/cut/substrate information can't be
encoded into the opp name? That would make these properties
obsolete, given that all they're used for is to pick out the
correct OPP?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On Tue, 28 Jul 2015, Stephen Boyd wrote:
> On 07/28, Viresh Kumar wrote:
> > Cc'ing few people (whom I cc'd last time as well :)).
> >
> > On 27-07-15, 16:20, Lee Jones wrote:
> > > These OPPs are used in ST's CPUFreq implementation.
> > >
> > > Signed-off-by: Lee Jones <[email protected]>
> > > ---
> > >
> > > Changelog:
> > > - None, new patch
> > >
> > > Documentation/devicetree/bindings/power/opp-st.txt | 76 ++++++++++++++++++++++
> > > 1 file changed, 76 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/power/opp-st.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/power/opp-st.txt b/Documentation/devicetree/bindings/power/opp-st.txt
> > > new file mode 100644
> > > index 0000000..6eb2a91
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/power/opp-st.txt
> > > @@ -0,0 +1,76 @@
> > > +STMicroelectronics OPP (Operating Performance Points) Bindings
> > > +--------------------------------------------------------------
> > > +
> > > +Frequency Scaling only
> > > +----------------------
> > > +
> > > +Located in CPU's node:
> > > +
> > > +- operating-points : [See: ./opp.txt]
> > > +
> > > +Example [safe]
> > > +--------------
> > > +
> > > +cpus {
> > > + cpu@0 {
> > > + /* kHz uV */
> > > + operating-points = <1500000 0
> > > + 1200000 0
> > > + 800000 0
> > > + 500000 0>;
> > > + };
> > > +};
> > > +
> > > +Dynamic Voltage and Frequency Scaling (DVFS)
> > > +--------------------------------------------
> > > +
> > > +Located in 'cpu0-opp-list' node [to be provided ONLY by the bootloader]:
> > > +
> > > +- compatible : Should be "operating-points-v2-sti"
> > > +- opp{1..N} : Each 'oppX' subnode will contain the following properties:
> >
> > Or should we mention:
> > - opp{1..N} : Each 'oppX' subnode shall contain below properties,
> > over what ./opp.txt defines:
> >
> > ?
> >
> >
> > > + - opp-hz : CPU frequency [Hz] for this OPP [See: ./opp.txt]
> > > + - st,avs : List of available voltages [uV] indexed by process code
> > > + - st,cuts : Cut version this OPP is suitable for [0xFF means ALL]
> > > + - st,substrate : Substrate version this OPP is suitable for [0xFF means ALL]
> > > +- st,syscfg : Phandle to Major number register
> > > + First cell: offset to major number
> > > +- st,syscfg-eng : Phandle to Minor number and Pcode registers
> > > + First cell: offset to process code
> > > + Second cell: offset to minor number
> > > +
> > > +WARNING: The opp{1..N} nodes will be provided by the bootloader. Do not attempt to
> > > + artificially synthesise the opp{1..N} nodes or any of their descendants.
> > > + They are very platform specific and may damage the hardware if created
> > > + incorrectly.
> > > +
> > > +Example [unsafe]
> > > +----------------
> > > +
> > > +cpus {
> > > + cpu@0 {
> > > + operating-points-v2 = <&cpu0_opp_list>;
> > > + };
> > > +};
> > > +
> > > +/* ############################################################ */
> > > +/* # WARNING: Do not attempt to copy/replicate this node, # */
> > > +/* # it is only to be supplied by the bootloader !!! # */
> > > +/* ############################################################ */
> > > +cpu0-opp-list {
> > > + compatible = "operating-points-v2-sti";
> > > + st,syscfg = <&syscfg [major_offset]>;
> > > + st,syscfg-eng = <&syscfg_eng [pcode_offset] [minor_offset]>;
> > > +
> > > + opp0 {
> > > + opp-hz = <1200000000>;
> > > + st,avs = <1110 1150 1100 1080 1040 1020 980 930>;
> > > + st,substrate = <0xff>;
> > > + st,cuts = <0xff>;
> > > + };
> > > + opp1 {
> > > + opp-hz = <1500000000>;
> > > + st,avs = <1200 1200 1200 1200 1170 1140 1100 1070>;
> > > + st,substrate = <0xff>;
> > > + st,cuts = <0x2>;
> > > + };
> > > +};
> >
> > I don't see more problems here, unless we can move some of this to the
> > generic bindings.
> >
> > @Rob/Stephen: Please respond before it is late :)
>
> It's interesting to have vendor specific properties like avs,
> cuts, and substrate. That could replace our planned usage of the
> opp-names property where we encode similar information (speed
> bin, revision, etc.) into a string that we look for.
>
> So I wonder why the avs/cut/substrate information can't be
> encoded into the opp name? That would make these properties
> obsolete, given that all they're used for is to pick out the
> correct OPP?
You could hack the substrate and cut version into a string, but that's
exactly what it would be, a hack. I'm struggling how you would do the
same for 'st,avs', which is an array of u32s.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On 07/29, Lee Jones wrote:
> On Tue, 28 Jul 2015, Stephen Boyd wrote:
>
> > On 07/28, Viresh Kumar wrote:
> > > Cc'ing few people (whom I cc'd last time as well :)).
> > >
> > > On 27-07-15, 16:20, Lee Jones wrote:
> > > > + - opp-hz : CPU frequency [Hz] for this OPP [See: ./opp.txt]
> > > > + - st,avs : List of available voltages [uV] indexed by process code
> > > > + - st,cuts : Cut version this OPP is suitable for [0xFF means ALL]
> > > > + - st,substrate : Substrate version this OPP is suitable for [0xFF means ALL]
[...]
> > > > +cpu0-opp-list {
> > > > + compatible = "operating-points-v2-sti";
> > > > + st,syscfg = <&syscfg [major_offset]>;
> > > > + st,syscfg-eng = <&syscfg_eng [pcode_offset] [minor_offset]>;
> > > > +
> > > > + opp0 {
> > > > + opp-hz = <1200000000>;
> > > > + st,avs = <1110 1150 1100 1080 1040 1020 980 930>;
> > > > + st,substrate = <0xff>;
> > > > + st,cuts = <0xff>;
> > > > + };
> > > > + opp1 {
> > > > + opp-hz = <1500000000>;
> > > > + st,avs = <1200 1200 1200 1200 1170 1140 1100 1070>;
> > > > + st,substrate = <0xff>;
> > > > + st,cuts = <0x2>;
> > > > + };
> > > > +};
> > >
> > > I don't see more problems here, unless we can move some of this to the
> > > generic bindings.
> > >
> > > @Rob/Stephen: Please respond before it is late :)
> >
> > It's interesting to have vendor specific properties like avs,
> > cuts, and substrate. That could replace our planned usage of the
> > opp-names property where we encode similar information (speed
> > bin, revision, etc.) into a string that we look for.
> >
> > So I wonder why the avs/cut/substrate information can't be
> > encoded into the opp name? That would make these properties
> > obsolete, given that all they're used for is to pick out the
> > correct OPP?
>
> You could hack the substrate and cut version into a string, but that's
> exactly what it would be, a hack. I'm struggling how you would do the
> same for 'st,avs', which is an array of u32s.
>
(I don't understand the st,avs property to begin with, so feel
free to ignore the rest of this mail.)
For qcom platforms we have the pvs bin and speed bin, and
sometimes a revision number. Each one of these properties
corresponds to a different set of OPPs (opp table). So we might
have speed1-pvs2-v0 for speed bin 1, pvs bin 2 and version 0. We
fill out an opp table for this and then point the opps property
at the table and have a corresponding opp-name "speed1-pvs2-v0"
in the "consumer" node.
operating-points-v2 = <&speed1_pvs2_v0>;
operating-points-names = "speed1-pvs2-v0";
We have quite a few of these tables because the values are
always different. If the values were the same then we could use
the same table with different names I suppose, but we're not
doing that.
>From a quick read of the st properties (that I admit I don't
understand), it looks like we're trying to compress the OPP
tables by listing all the voltages that could be used for a
particular frequency depending on which avs is present on the
device? And then limiting the frequency voltage pairs depending
on which cut and substrate is present?
So we'd probably have to expand out the tables to be unique per
avs/cut/substrate parameter. Something like:
avs0-cut2 {
compatible = "operating-points-v2";
opp0 {
opp-hz = <1200000000>;
opp-microvolt = <1100>;
};
opp1 {
opp-hz = <1500000000>;
opp-microvolt = <1200>;
};
};
avs1-cut2 {
compatible = "operating-points-v2";
opp0 {
opp-hz = <1200000000>;
opp-microvolt = <1150>;
};
opp1 {
opp-hz = <1500000000>;
opp-microvolt = <1200>;
};
};
And then another copy of these for the devices without cuts != 2
where the top frequency is gone?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On Wed, 29 Jul 2015, Stephen Boyd wrote:
> On 07/29, Lee Jones wrote:
> > On Tue, 28 Jul 2015, Stephen Boyd wrote:
> >
> > > On 07/28, Viresh Kumar wrote:
> > > > Cc'ing few people (whom I cc'd last time as well :)).
> > > >
> > > > On 27-07-15, 16:20, Lee Jones wrote:
> > > > > + - opp-hz : CPU frequency [Hz] for this OPP [See: ./opp.txt]
> > > > > + - st,avs : List of available voltages [uV] indexed by process code
> > > > > + - st,cuts : Cut version this OPP is suitable for [0xFF means ALL]
> > > > > + - st,substrate : Substrate version this OPP is suitable for [0xFF means ALL]
> [...]
> > > > > +cpu0-opp-list {
> > > > > + compatible = "operating-points-v2-sti";
> > > > > + st,syscfg = <&syscfg [major_offset]>;
> > > > > + st,syscfg-eng = <&syscfg_eng [pcode_offset] [minor_offset]>;
> > > > > +
> > > > > + opp0 {
> > > > > + opp-hz = <1200000000>;
> > > > > + st,avs = <1110 1150 1100 1080 1040 1020 980 930>;
> > > > > + st,substrate = <0xff>;
> > > > > + st,cuts = <0xff>;
> > > > > + };
> > > > > + opp1 {
> > > > > + opp-hz = <1500000000>;
> > > > > + st,avs = <1200 1200 1200 1200 1170 1140 1100 1070>;
> > > > > + st,substrate = <0xff>;
> > > > > + st,cuts = <0x2>;
> > > > > + };
> > > > > +};
> > > >
> > > > I don't see more problems here, unless we can move some of this to the
> > > > generic bindings.
> > > >
> > > > @Rob/Stephen: Please respond before it is late :)
> > >
> > > It's interesting to have vendor specific properties like avs,
> > > cuts, and substrate. That could replace our planned usage of the
> > > opp-names property where we encode similar information (speed
> > > bin, revision, etc.) into a string that we look for.
> > >
> > > So I wonder why the avs/cut/substrate information can't be
> > > encoded into the opp name? That would make these properties
> > > obsolete, given that all they're used for is to pick out the
> > > correct OPP?
> >
> > You could hack the substrate and cut version into a string, but that's
> > exactly what it would be, a hack. I'm struggling how you would do the
> > same for 'st,avs', which is an array of u32s.
> >
>
>
> (I don't understand the st,avs property to begin with, so feel
> free to ignore the rest of this mail.)
>
> For qcom platforms we have the pvs bin and speed bin, and
> sometimes a revision number. Each one of these properties
> corresponds to a different set of OPPs (opp table). So we might
> have speed1-pvs2-v0 for speed bin 1, pvs bin 2 and version 0. We
> fill out an opp table for this and then point the opps property
> at the table and have a corresponding opp-name "speed1-pvs2-v0"
> in the "consumer" node.
>
> operating-points-v2 = <&speed1_pvs2_v0>;
> operating-points-names = "speed1-pvs2-v0";
>
> We have quite a few of these tables because the values are
> always different. If the values were the same then we could use
> the same table with different names I suppose, but we're not
> doing that.
>
> From a quick read of the st properties (that I admit I don't
> understand), it looks like we're trying to compress the OPP
> tables by listing all the voltages that could be used for a
> particular frequency depending on which avs is present on the
> device? And then limiting the frequency voltage pairs depending
> on which cut and substrate is present?
>
> So we'd probably have to expand out the tables to be unique per
> avs/cut/substrate parameter. Something like:
>
> avs0-cut2 {
> compatible = "operating-points-v2";
>
> opp0 {
> opp-hz = <1200000000>;
> opp-microvolt = <1100>;
> };
>
> opp1 {
> opp-hz = <1500000000>;
> opp-microvolt = <1200>;
> };
> };
>
> avs1-cut2 {
> compatible = "operating-points-v2";
>
> opp0 {
> opp-hz = <1200000000>;
> opp-microvolt = <1150>;
> };
>
> opp1 {
> opp-hz = <1500000000>;
> opp-microvolt = <1200>;
> };
> };
>
> And then another copy of these for the devices without cuts != 2
> where the top frequency is gone?
There is nothing stopping us from representing the data in this way.
On the plus side, it would mean that we wouldn't need any vendor
specific properties. However, far outweighing the positives are the
fact that, even in our very simple example provided, where we only
have 2 frequencies, differ between only 1 cut and support all
substrates, we would still need 16 OPP tables. When any one of those
variables increase (and they will), we would then have a large number
of permutations and subsequently and unruly amount of OPP tables.
(... and we haven't even provided the body-biasing information yet.)
The way we've chosen to represent our circumstance is the least
intrusive and the most succinct way we can think of. Which IMHO
outweighs the fact that we have to introduce a couple of vendor
properties by a country mile.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Thu, Jul 30, 2015 at 3:46 AM, Lee Jones <[email protected]> wrote:
> On Wed, 29 Jul 2015, Stephen Boyd wrote:
>
>> On 07/29, Lee Jones wrote:
>> > On Tue, 28 Jul 2015, Stephen Boyd wrote:
>> >
>> > > On 07/28, Viresh Kumar wrote:
>> > > > Cc'ing few people (whom I cc'd last time as well :)).
>> > > >
>> > > > On 27-07-15, 16:20, Lee Jones wrote:
>> > > > > + - opp-hz : CPU frequency [Hz] for this OPP [See: ./opp.txt]
>> > > > > + - st,avs : List of available voltages [uV] indexed by process code
>> > > > > + - st,cuts : Cut version this OPP is suitable for [0xFF means ALL]
>> > > > > + - st,substrate : Substrate version this OPP is suitable for [0xFF means ALL]
>> [...]
>> > > > > +cpu0-opp-list {
>> > > > > + compatible = "operating-points-v2-sti";
>> > > > > + st,syscfg = <&syscfg [major_offset]>;
>> > > > > + st,syscfg-eng = <&syscfg_eng [pcode_offset] [minor_offset]>;
>> > > > > +
>> > > > > + opp0 {
>> > > > > + opp-hz = <1200000000>;
>> > > > > + st,avs = <1110 1150 1100 1080 1040 1020 980 930>;
>> > > > > + st,substrate = <0xff>;
>> > > > > + st,cuts = <0xff>;
>> > > > > + };
>> > > > > + opp1 {
>> > > > > + opp-hz = <1500000000>;
>> > > > > + st,avs = <1200 1200 1200 1200 1170 1140 1100 1070>;
>> > > > > + st,substrate = <0xff>;
>> > > > > + st,cuts = <0x2>;
>> > > > > + };
>> > > > > +};
>> > > >
>> > > > I don't see more problems here, unless we can move some of this to the
>> > > > generic bindings.
>> > > >
>> > > > @Rob/Stephen: Please respond before it is late :)
>> > >
>> > > It's interesting to have vendor specific properties like avs,
>> > > cuts, and substrate. That could replace our planned usage of the
>> > > opp-names property where we encode similar information (speed
>> > > bin, revision, etc.) into a string that we look for.
>> > >
>> > > So I wonder why the avs/cut/substrate information can't be
>> > > encoded into the opp name? That would make these properties
>> > > obsolete, given that all they're used for is to pick out the
>> > > correct OPP?
>> >
>> > You could hack the substrate and cut version into a string, but that's
>> > exactly what it would be, a hack. I'm struggling how you would do the
>> > same for 'st,avs', which is an array of u32s.
>> >
>>
>>
>> (I don't understand the st,avs property to begin with, so feel
>> free to ignore the rest of this mail.)
>>
>> For qcom platforms we have the pvs bin and speed bin, and
>> sometimes a revision number. Each one of these properties
>> corresponds to a different set of OPPs (opp table). So we might
>> have speed1-pvs2-v0 for speed bin 1, pvs bin 2 and version 0. We
>> fill out an opp table for this and then point the opps property
>> at the table and have a corresponding opp-name "speed1-pvs2-v0"
>> in the "consumer" node.
>>
>> operating-points-v2 = <&speed1_pvs2_v0>;
>> operating-points-names = "speed1-pvs2-v0";
>>
>> We have quite a few of these tables because the values are
>> always different. If the values were the same then we could use
>> the same table with different names I suppose, but we're not
>> doing that.
>>
>> From a quick read of the st properties (that I admit I don't
>> understand), it looks like we're trying to compress the OPP
>> tables by listing all the voltages that could be used for a
>> particular frequency depending on which avs is present on the
>> device? And then limiting the frequency voltage pairs depending
>> on which cut and substrate is present?
>>
>> So we'd probably have to expand out the tables to be unique per
>> avs/cut/substrate parameter. Something like:
>>
>> avs0-cut2 {
>> compatible = "operating-points-v2";
>>
>> opp0 {
>> opp-hz = <1200000000>;
>> opp-microvolt = <1100>;
>> };
>>
>> opp1 {
>> opp-hz = <1500000000>;
>> opp-microvolt = <1200>;
>> };
>> };
>>
>> avs1-cut2 {
>> compatible = "operating-points-v2";
>>
>> opp0 {
>> opp-hz = <1200000000>;
>> opp-microvolt = <1150>;
>> };
>>
>> opp1 {
>> opp-hz = <1500000000>;
>> opp-microvolt = <1200>;
>> };
>> };
>>
>> And then another copy of these for the devices without cuts != 2
>> where the top frequency is gone?
>
> There is nothing stopping us from representing the data in this way.
> On the plus side, it would mean that we wouldn't need any vendor
> specific properties. However, far outweighing the positives are the
> fact that, even in our very simple example provided, where we only
> have 2 frequencies, differ between only 1 cut and support all
> substrates, we would still need 16 OPP tables. When any one of those
> variables increase (and they will), we would then have a large number
> of permutations and subsequently and unruly amount of OPP tables.
>
> (... and we haven't even provided the body-biasing information yet.)
>
> The way we've chosen to represent our circumstance is the least
> intrusive and the most succinct way we can think of. Which IMHO
> outweighs the fact that we have to introduce a couple of vendor
> properties by a country mile.
Regardless of which is better or worse, you are both doing essentially
the same thing. You are selecting OPPs based on some Si parameters. We
should not really be doing this 2 different ways. I'd be fine with 2
ways if it was 2 for all SOCs, but right now it is 2 for 2 SOCs.
Really, I'd like to see most if not all the selection or fixup of OPPs
be done in the bootloader and the kernel just deals with the correct
OPP table.
Where are you storing the data that gets selected to fill into these
properties? Is it just look-up tables or is there some kind of
algorithm to generate the values? If the former, then DT is as good a
data struct as any to store the tables. There is a lot of duplication
though with only a single property varying in each set. If you both
have that problem, then perhaps we can come up with a generic way to
list all possible values more concisely.
Rob
On 07/30, Rob Herring wrote:
> On Thu, Jul 30, 2015 at 3:46 AM, Lee Jones <[email protected]> wrote:
> >
> > There is nothing stopping us from representing the data in this way.
> > On the plus side, it would mean that we wouldn't need any vendor
> > specific properties. However, far outweighing the positives are the
> > fact that, even in our very simple example provided, where we only
> > have 2 frequencies, differ between only 1 cut and support all
> > substrates, we would still need 16 OPP tables. When any one of those
> > variables increase (and they will), we would then have a large number
> > of permutations and subsequently and unruly amount of OPP tables.
> >
> > (... and we haven't even provided the body-biasing information yet.)
> >
> > The way we've chosen to represent our circumstance is the least
> > intrusive and the most succinct way we can think of. Which IMHO
> > outweighs the fact that we have to introduce a couple of vendor
> > properties by a country mile.
>
> Regardless of which is better or worse, you are both doing essentially
> the same thing. You are selecting OPPs based on some Si parameters. We
> should not really be doing this 2 different ways. I'd be fine with 2
> ways if it was 2 for all SOCs, but right now it is 2 for 2 SOCs.
> Really, I'd like to see most if not all the selection or fixup of OPPs
> be done in the bootloader and the kernel just deals with the correct
> OPP table.
>
> Where are you storing the data that gets selected to fill into these
> properties? Is it just look-up tables or is there some kind of
> algorithm to generate the values? If the former, then DT is as good a
> data struct as any to store the tables. There is a lot of duplication
> though with only a single property varying in each set. If you both
> have that problem, then perhaps we can come up with a generic way to
> list all possible values more concisely.
For qcom platforms, the frequency is almost always constant.
There may be some tables where we have a couple higher
frequencies than others because the speed bin is different.
Otherwise the voltage/current is changing based on the silicon
characteristics. So the biggest duplication is the frequency
property.
As far as I know there isn't any algorithm to generate the
voltage values. It's all hand tuned tables based on the silicon
characterization, so we're left to store these tables in DT and
pick the right one at runtime. With regards to the table
explosion, on qcom platforms we haven't worried that we have ~40
tables, but I'm not opposed to expressing it in a smaller set of
nodes, tables, etc. if that's what's desired.
Do we need vendor specific properties for that though? Or do we
need some sort of extended frequency/voltage properties that are
arrays of values that we can index into based on some silicon
characteristics? I like the name based approach because it's
simple. Use this OPP table because it's called
x-y-z-characteristics and be done. Cramming the tables into less
lines may save us some typing and dtb space, but I'm not sure
what else it does.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On 31-07-15, 09:37, Stephen Boyd wrote:
> Do we need vendor specific properties for that though?
Sorry Lee :), but this is exactly why I wanted this thread to exist. We must and
should do this in a generic enough way.
--
viresh
On 31-07-15, 09:37, Stephen Boyd wrote:
> For qcom platforms, the frequency is almost always constant.
> There may be some tables where we have a couple higher
> frequencies than others because the speed bin is different.
> Otherwise the voltage/current is changing based on the silicon
> characteristics. So the biggest duplication is the frequency
> property.
>
> As far as I know there isn't any algorithm to generate the
> voltage values. It's all hand tuned tables based on the silicon
> characterization, so we're left to store these tables in DT and
> pick the right one at runtime. With regards to the table
> explosion, on qcom platforms we haven't worried that we have ~40
> tables, but I'm not opposed to expressing it in a smaller set of
> nodes, tables, etc. if that's what's desired.
>
> Do we need vendor specific properties for that though? Or do we
> need some sort of extended frequency/voltage properties that are
> arrays of values that we can index into based on some silicon
> characteristics? I like the name based approach because it's
> simple. Use this OPP table because it's called
> x-y-z-characteristics and be done. Cramming the tables into less
> lines may save us some typing and dtb space, but I'm not sure
> what else it does.
What about something like this:
diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
index 0cb44dc21f97..bad7a8299b9c 100644
--- a/Documentation/devicetree/bindings/opp/opp.txt
+++ b/Documentation/devicetree/bindings/opp/opp.txt
@@ -74,6 +74,8 @@ This describes the OPPs belonging to a device. This node can have following
reference an OPP.
Optional properties:
+- opp-cuts: One or more strings, describing the versions of hardware the OPPs
+ can support.
- opp-shared: Indicates that device nodes using this OPP Table Node's phandle
switch their DVFS state together, i.e. they share clock/voltage/current lines.
Missing property means devices have independent clock/voltage/current lines,
@@ -100,6 +102,9 @@ properties.
Entries for multiple regulators must be present in the same order as
regulators are specified in device's DT node.
+ If used with 'opp-cuts', then the number of entries present here must match
+ the number of strings present in 'opp-cuts'.
+
- opp-microamp: The maximum current drawn by the device in microamperes
considering system specific parameters (such as transients, process, aging,
maximum operating temperature range etc.) as necessary. This may be used to
--
viresh
On Mon, 03 Aug 2015, Viresh Kumar wrote:
> On 31-07-15, 09:37, Stephen Boyd wrote:
> > For qcom platforms, the frequency is almost always constant.
> > There may be some tables where we have a couple higher
> > frequencies than others because the speed bin is different.
> > Otherwise the voltage/current is changing based on the silicon
> > characteristics. So the biggest duplication is the frequency
> > property.
> >
> > As far as I know there isn't any algorithm to generate the
> > voltage values. It's all hand tuned tables based on the silicon
> > characterization, so we're left to store these tables in DT and
> > pick the right one at runtime. With regards to the table
> > explosion, on qcom platforms we haven't worried that we have ~40
> > tables, but I'm not opposed to expressing it in a smaller set of
> > nodes, tables, etc. if that's what's desired.
> >
> > Do we need vendor specific properties for that though? Or do we
> > need some sort of extended frequency/voltage properties that are
> > arrays of values that we can index into based on some silicon
> > characteristics? I like the name based approach because it's
> > simple. Use this OPP table because it's called
> > x-y-z-characteristics and be done. Cramming the tables into less
> > lines may save us some typing and dtb space, but I'm not sure
> > what else it does.
>
> What about something like this:
>
> diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> index 0cb44dc21f97..bad7a8299b9c 100644
> --- a/Documentation/devicetree/bindings/opp/opp.txt
> +++ b/Documentation/devicetree/bindings/opp/opp.txt
> @@ -74,6 +74,8 @@ This describes the OPPs belonging to a device. This node can have following
> reference an OPP.
>
> Optional properties:
> +- opp-cuts: One or more strings, describing the versions of hardware the OPPs
> + can support.
This isn't very generic.
I'm guessing some vendors my have quite a few ways to differentiate
between board versions/revisions/cuts etc.
How about another array where a vendor can choose to identify a piece
of hardware however they see fit.
Example 1 (simple version):
/* Version 1 */
opp-version = <1>;
Example 2 (using the kernel's versioning):
/* 2.6.32-rc1 */
opp-version = <2 6 32 1>;
Example 3 (using ST's versioning):
/* Major 2, Minor 0, Cut 2, All substrates */
opp-version = <2 0 2 0xff>;
Qcom (or anyone else wanting to use names to identify their revisions)
can continue to use their node name method, as it doesn't break any
convention.
> - opp-shared: Indicates that device nodes using this OPP Table Node's phandle
> switch their DVFS state together, i.e. they share clock/voltage/current lines.
> Missing property means devices have independent clock/voltage/current lines,
> @@ -100,6 +102,9 @@ properties.
> Entries for multiple regulators must be present in the same order as
> regulators are specified in device's DT node.
>
> + If used with 'opp-cuts', then the number of entries present here must match
> + the number of strings present in 'opp-cuts'.
> +
> - opp-microamp: The maximum current drawn by the device in microamperes
> considering system specific parameters (such as transients, process, aging,
> maximum operating temperature range etc.) as necessary. This may be used to
>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On 10-08-15, 14:22, Lee Jones wrote:
> > Optional properties:
> > +- opp-cuts: One or more strings, describing the versions of hardware the OPPs
> > + can support.
>
> This isn't very generic.
>
> I'm guessing some vendors my have quite a few ways to differentiate
> between board versions/revisions/cuts etc.
>
> How about another array where a vendor can choose to identify a piece
> of hardware however they see fit.
>
> Example 1 (simple version):
>
> /* Version 1 */
> opp-version = <1>;
>
> Example 2 (using the kernel's versioning):
>
> /* 2.6.32-rc1 */
> opp-version = <2 6 32 1>;
>
> Example 3 (using ST's versioning):
>
> /* Major 2, Minor 0, Cut 2, All substrates */
> opp-version = <2 0 2 0xff>;
But how will we parse this with generic code ?
--
viresh
On Tue, 11 Aug 2015, Viresh Kumar wrote:
> On 10-08-15, 14:22, Lee Jones wrote:
> > > Optional properties:
> > > +- opp-cuts: One or more strings, describing the versions of hardware the OPPs
> > > + can support.
> >
> > This isn't very generic.
> >
> > I'm guessing some vendors my have quite a few ways to differentiate
> > between board versions/revisions/cuts etc.
> >
> > How about another array where a vendor can choose to identify a piece
> > of hardware however they see fit.
> >
> > Example 1 (simple version):
> >
> > /* Version 1 */
> > opp-version = <1>;
> >
> > Example 2 (using the kernel's versioning):
> >
> > /* 2.6.32-rc1 */
> > opp-version = <2 6 32 1>;
> >
> > Example 3 (using ST's versioning):
> >
> > /* Major 2, Minor 0, Cut 2, All substrates */
> > opp-version = <2 0 2 0xff>;
>
> But how will we parse this with generic code ?
Why would you want to?
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On 11-08-15, 10:30, Lee Jones wrote:
> On Tue, 11 Aug 2015, Viresh Kumar wrote:
>
> > On 10-08-15, 14:22, Lee Jones wrote:
> > > > Optional properties:
> > > > +- opp-cuts: One or more strings, describing the versions of hardware the OPPs
> > > > + can support.
> > >
> > > This isn't very generic.
> > >
> > > I'm guessing some vendors my have quite a few ways to differentiate
> > > between board versions/revisions/cuts etc.
> > >
> > > How about another array where a vendor can choose to identify a piece
> > > of hardware however they see fit.
> > >
> > > Example 1 (simple version):
> > >
> > > /* Version 1 */
> > > opp-version = <1>;
> > >
> > > Example 2 (using the kernel's versioning):
> > >
> > > /* 2.6.32-rc1 */
> > > opp-version = <2 6 32 1>;
> > >
> > > Example 3 (using ST's versioning):
> > >
> > > /* Major 2, Minor 0, Cut 2, All substrates */
> > > opp-version = <2 0 2 0xff>;
> >
> > But how will we parse this with generic code ?
>
> Why would you want to?
So that individual platforms don't need to reinvent the wheel ?
--
viresh
On Tue, 11 Aug 2015, Viresh Kumar wrote:
> On 11-08-15, 10:30, Lee Jones wrote:
> > On Tue, 11 Aug 2015, Viresh Kumar wrote:
> >
> > > On 10-08-15, 14:22, Lee Jones wrote:
> > > > > Optional properties:
> > > > > +- opp-cuts: One or more strings, describing the versions of hardware the OPPs
> > > > > + can support.
> > > >
> > > > This isn't very generic.
> > > >
> > > > I'm guessing some vendors my have quite a few ways to differentiate
> > > > between board versions/revisions/cuts etc.
> > > >
> > > > How about another array where a vendor can choose to identify a piece
> > > > of hardware however they see fit.
> > > >
> > > > Example 1 (simple version):
> > > >
> > > > /* Version 1 */
> > > > opp-version = <1>;
> > > >
> > > > Example 2 (using the kernel's versioning):
> > > >
> > > > /* 2.6.32-rc1 */
> > > > opp-version = <2 6 32 1>;
> > > >
> > > > Example 3 (using ST's versioning):
> > > >
> > > > /* Major 2, Minor 0, Cut 2, All substrates */
> > > > opp-version = <2 0 2 0xff>;
> > >
> > > But how will we parse this with generic code ?
> >
> > Why would you want to?
>
> So that individual platforms don't need to reinvent the wheel ?
The framework does not need to parse this information. It is used
solely by the platform driver, whose job it is to decide which OPPs
are appropriate for the running platform.
Back to my question; how would you like arbitrary version information,
which means different things to different vendors to be used in
shared/generic/framework code?
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On 11-08-15, 12:54, Lee Jones wrote:
> The framework does not need to parse this information. It is used
> solely by the platform driver, whose job it is to decide which OPPs
> are appropriate for the running platform.
The OPP layer needs to parse OPP nodes in DT. But for doing that it
needs to know which OPPs to pick from the table as, in cases like
yours or qcom, not all OPPs might be available.
One of the ways to do that is:
- the platform reads its efuses (or whatever) and encodes the
information into a string.
- This string should match with the strings present (somewhere) in the
OPP table. That location can be like what I proposed few mails back.
- Then the *generic* OPP code can parse only those OPP nodes which
match with that string.
This way, we can avoid pushing the platform code to parse OPP tables.
> Back to my question; how would you like arbitrary version information,
> which means different things to different vendors to be used in
> shared/generic/framework code?
Exactly like I wrote above. The platform just needs to give a string
that should match with the OPPs ..
--
viresh
On Tue, 11 Aug 2015, Viresh Kumar wrote:
> On 11-08-15, 12:54, Lee Jones wrote:
> > The framework does not need to parse this information. It is used
> > solely by the platform driver, whose job it is to decide which OPPs
> > are appropriate for the running platform.
>
> The OPP layer needs to parse OPP nodes in DT. But for doing that it
> needs to know which OPPs to pick from the table as, in cases like
> yours or qcom, not all OPPs might be available.
>
> One of the ways to do that is:
> - the platform reads its efuses (or whatever) and encodes the
> information into a string.
> - This string should match with the strings present (somewhere) in the
> OPP table. That location can be like what I proposed few mails back.
> - Then the *generic* OPP code can parse only those OPP nodes which
> match with that string.
>
> This way, we can avoid pushing the platform code to parse OPP tables.
Okay, so what you're saying is that you've already made the decision
to create a separate node for every OPP permutation, despite the fact
that I've told you this could lead to more nodes than anyone would
care to successfully write or maintain?
Perhaps an example might help explain the issue.
Using the current driver, we need to place the following in DT and the
driver does the rest:
opp-list {
opp1 {
opp-hz = <1500000000>;
st,avs = <1200 1200 1200 1200 1170 1140 1100 1070>;
st,substrate = <0xff>;
st,cuts = <0xff>;
};
opp0 {
opp-hz = <1200000000>;
st,avs = <1110 1150 1100 1080 1040 1020 980 930>;
st,substrate = <0xff>;
st,cuts = <0x2>;
};
};
However, what you're suggesting, even for this very simple example
(imagine what this would look like with 5 or more frequencies where
two or more of them were only appropriate to run on particular
variants) requires this to be broken out to:
opp-list {
pcode0-cut2-allsubstrates {
opp0 {
opp-hz = <1200000000>;
opp-microvolt = <1110000>;
};
};
pcode0-allcuts-allsubstrates {
opp0 {
opp-hz = <1500000000>;
opp-microvolt = <1200000>;
};
};
pcode1-cut2-allsubstrates {
opp0 {
opp-hz = <1200000000>;
opp-microvolt = <1150000>;
};
};
pcode1-allcuts-allsubstrates {
opp0 {
opp-hz = <1500000000>;
opp-microvolt = <1200000>;
};
};
pcode2-cut2-allsubstrates {
opp0 {
opp-hz = <1200000000>;
opp-microvolt = <1100000>;
};
};
pcode2-allcuts-allsubstrates {
opp0 {
opp-hz = <1500000000>;
opp-microvolt = <1200000>;
};
};
pcode3-cut2-allsubstrates {
opp0 {
opp-hz = <1200000000>;
opp-microvolt = <1080000>;
};
};
pcode3-allcuts-allsubstrates {
opp0 {
opp-hz = <1500000000>;
opp-microvolt = <1200000>;
};
};
pcode4-cut2-allsubstrates {
opp0 {
opp-hz = <1200000000>;
opp-microvolt = <1040000>;
};
pcode4-allcuts-allsubstrates {
opp0 {
opp-hz = <1500000000>;
opp-microvolt = <1170000>;
};
};
pcode5-cut2-allsubstrates {
opp0 {
opp-hz = <1200000000>;
opp-microvolt = <1020000>;
};
};
pcode5-allcuts-allsubstrates {
opp0 {
opp-hz = <1500000000>;
opp-microvolt = <1140000>;
};
};
pcode6-cut2-allsubstrates {
opp0 {
opp-hz = <1200000000>;
opp-microvolt = <980000>;
};
};
pcode6-allcuts-allsubstrates {
opp0 {
opp-hz = <1500000000>;
opp-microvolt = <1100000>;
};
};
pcode7-cut2-allsubstrates {
opp0 {
opp-hz = <1200000000>;
opp-microvolt = <930000>;
};
};
pcode7-allcuts-allsubstrates {
opp0 {
opp-hz = <1500000000>;
opp-microvolt = <1070000>;
};
};
};
These will soon multiply once you start providing more complex
examples. And how do you plan on handling this in the framework? Can
the driver submit more than one variations of the string? In the
current example the driver would need to submit four strings to
provide all acceptable variations; "pcodeX-cutY-substrateZ",
"pcodeX-allcuts-substrateZ", "pcodeX-cutY-allsubstrates" and
"pcodeX-allcuts-allsubstrates"
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On 11-08-15, 14:27, Lee Jones wrote:
> Okay, so what you're saying is that you've already made the decision
> to create a separate node for every OPP permutation,
Absolutely not.
> despite the fact
> that I've told you this could lead to more nodes than anyone would
> care to successfully write or maintain?
I have enough fear of yours and then I have to see you in another
month as well. I wouldn't dare to disobey your command SIR :)
> Perhaps an example might help explain the issue.
>
> Using the current driver, we need to place the following in DT and the
> driver does the rest:
>
> opp-list {
> opp1 {
> opp-hz = <1500000000>;
> st,avs = <1200 1200 1200 1200 1170 1140 1100 1070>;
> st,substrate = <0xff>;
> st,cuts = <0xff>;
> };
> opp0 {
> opp-hz = <1200000000>;
> st,avs = <1110 1150 1100 1080 1040 1020 980 930>;
> st,substrate = <0xff>;
> st,cuts = <0x2>;
> };
> };
Nothing is fixed as of now but this is what I am thinking of:
cpu0_opp_table: opp_table0 {
compatible = "operating-points-v2";
opp-cuts = "10", "3c", "f0";
supply-names = "vcc0", "vcc1", "vcc2";
opp-shared;
opp00 {
opp-hz = /bits/ 64 <1000000000>;
clock-latency-ns = <300000>;
opp-microvolt-10 = <970000>;
opp-microvolt-3c = <950000>;
opp-microvolt-f0 = <930000>;
};
/* OR */
opp00 {
opp-hz = /bits/ 64 <1000000000>;
clock-latency-ns = <300000>;
opp-microvolt = <970000>, <950000>, <930000>;
};
};
And then the platform code needs to tell OPP layer:
"Use OPPs for cut f0 for device X", and OPP layer will store that
somewhere.
And then it will only initialize OPPs after matching this string with
the values.
Out of the earlier two options, I may prefer the first one. As we will
be soon adding support for multiple regulators, and a single regulator
can have min/max/target values.. So, a single list will become too
long.
But, something like this should be generic enough to capture most of
the cases.
@Stephen/Rob ??
--
viresh
On Tue, 11 Aug 2015, Viresh Kumar wrote:
> On 11-08-15, 14:27, Lee Jones wrote:
> > Okay, so what you're saying is that you've already made the decision
> > to create a separate node for every OPP permutation,
>
> Absolutely not.
>
> > despite the fact
> > that I've told you this could lead to more nodes than anyone would
> > care to successfully write or maintain?
>
> I have enough fear of yours and then I have to see you in another
> month as well. I wouldn't dare to disobey your command SIR :)
Funny guy! ;)
> > Perhaps an example might help explain the issue.
> >
> > Using the current driver, we need to place the following in DT and the
> > driver does the rest:
> >
> > opp-list {
> > opp1 {
> > opp-hz = <1500000000>;
> > st,avs = <1200 1200 1200 1200 1170 1140 1100 1070>;
> > st,substrate = <0xff>;
> > st,cuts = <0xff>;
> > };
> > opp0 {
> > opp-hz = <1200000000>;
> > st,avs = <1110 1150 1100 1080 1040 1020 980 930>;
> > st,substrate = <0xff>;
> > st,cuts = <0x2>;
> > };
> > };
>
> Nothing is fixed as of now but this is what I am thinking of:
>
> cpu0_opp_table: opp_table0 {
> compatible = "operating-points-v2";
> opp-cuts = "10", "3c", "f0";
> supply-names = "vcc0", "vcc1", "vcc2";
> opp-shared;
>
> opp00 {
> opp-hz = /bits/ 64 <1000000000>;
> clock-latency-ns = <300000>;
> opp-microvolt-10 = <970000>;
> opp-microvolt-3c = <950000>;
> opp-microvolt-f0 = <930000>;
> };
>
> /* OR */
>
> opp00 {
> opp-hz = /bits/ 64 <1000000000>;
> clock-latency-ns = <300000>;
> opp-microvolt = <970000>, <950000>, <930000>;
> };
> };
>
> And then the platform code needs to tell OPP layer:
> "Use OPPs for cut f0 for device X", and OPP layer will store that
> somewhere.
>
> And then it will only initialize OPPs after matching this string with
> the values.
>
> Out of the earlier two options, I may prefer the first one. As we will
> be soon adding support for multiple regulators, and a single regulator
> can have min/max/target values.. So, a single list will become too
> long.
This would work if we only had a single variable to contend with, but
what I showed you in my previous example is that we have 3 variables
to consider; cut (version), pcode and substrate.
Using the two (simple) examples I provided, how would your suggestion
look in our case?
> But, something like this should be generic enough to capture most of
> the cases.
>
> @Stephen/Rob ??
>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On 11-08-15, 16:17, Lee Jones wrote:
> This would work if we only had a single variable to contend with, but
> what I showed you in my previous example is that we have 3 variables
> to consider; cut (version), pcode and substrate.
>
> Using the two (simple) examples I provided, how would your suggestion
> look in our case?
So the solution I gave is for picking the microvolt based on pcode.
The other two (cut, substrate) aren't about picking microvolt, but if
the OPP is available or not. Right?
If these terms are generic enough, then we can add something similar
to what you have added..
@Stephen ?
--
viresh
On Wed, 12 Aug 2015, Viresh Kumar wrote:
> On 11-08-15, 16:17, Lee Jones wrote:
> > This would work if we only had a single variable to contend with, but
> > what I showed you in my previous example is that we have 3 variables
> > to consider; cut (version), pcode and substrate.
> >
> > Using the two (simple) examples I provided, how would your suggestion
> > look in our case?
>
> So the solution I gave is for picking the microvolt based on pcode.
> The other two (cut, substrate) aren't about picking microvolt, but if
> the OPP is available or not. Right?
'pcode', 'cut' and 'substrate' all determine whether a given set of
OPPs an be used on the running platform. I do not believe that you
can differentiate between them.
> If these terms are generic enough, then we can add something similar
> to what you have added..
If it makes it easier, you can treat them as version numbers 2.2.1
<pcode.cut.substrate>, but I don't see how this can help. Obviously
this becomes more difficult when you add wild cards to the OPPs, where
a particular OPP would be suitable for all cuts for example.
If you still think you can come up with a generic method to lay out
CPUFreq OPP nodes that will satisfy all vendors and not be a mass of
10's of separate nodes, then great. Again, I'm struggling to see how
that might be possible.
What I believe we shouldn't do, is have this blocked forever for the
sake of adding a couple of vendor properties however.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog