Re: [PATCH RESEND] devicetree: bindings: separate CPU enable method descriptions

From: Mark Rutland
Date: Tue Jun 03 2014 - 06:09:41 EST

Hi Alex,

On Fri, May 30, 2014 at 11:11:54PM +0100, Alex Elder wrote:
> The bindings for CPU enable methods are defined in ".../arm/cpus.txt". As
> additional 32-bit ARM CPUS are converted to use the "enable-method" CPU
> property to imply a particular set of SMP operations to use, the list of these
> methods is likely to become unwieldy. The current documentation already
> contains several property descriptions that are meaningful only for certain
> enable methods.
>
> This patch defines a new Documentation subdirectory whose purpose is to give
> each CPU enable method its own place to define how and when it's used, as
> well as what other properties (optional or required) are associated with
> the method. The existing enable method documentation is expanded and moved
> from ".../arm/cpus.txt" into new files accordingly.
>
> Signed-off-by: Alex Elder <elder@xxxxxxxxxx>
> ---
> v2: Rename "arm,psci.txt" to be "psci.txt" and fix its content
>
> .../bindings/arm/cpu-enable-method/README | 20 +++++
> .../bindings/arm/cpu-enable-method/psci.txt | 45 ++++++++++
> .../arm/cpu-enable-method/qcom,gcc-msm8660 | 30 +++++++
> .../arm/cpu-enable-method/qcom,kpss-acc-v1 | 56 +++++++++++++
> .../arm/cpu-enable-method/qcom,kpss-acc-v2 | 56 +++++++++++++
> .../bindings/arm/cpu-enable-method/spin-table.txt | 95 ++++++++++++++++++++++
> Documentation/devicetree/bindings/arm/cpus.txt | 29 +------
> 7 files changed, 305 insertions(+), 26 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/arm/cpu-enable-method/README
> create mode 100644 Documentation/devicetree/bindings/arm/cpu-enable-method/psci.txt
> create mode 100644 Documentation/devicetree/bindings/arm/cpu-enable-method/qcom,gcc-msm8660
> create mode 100644 Documentation/devicetree/bindings/arm/cpu-enable-method/qcom,kpss-acc-v1
> create mode 100644 Documentation/devicetree/bindings/arm/cpu-enable-method/qcom,kpss-acc-v2
> create mode 100644 Documentation/devicetree/bindings/arm/cpu-enable-method/spin-table.txt
>
> diff --git a/Documentation/devicetree/bindings/arm/cpu-enable-method/README b/Documentation/devicetree/bindings/arm/cpu-enable-method/README
> new file mode 100644
> index 0000000..cc9431e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/cpu-enable-method/README
> @@ -0,0 +1,20 @@
> +==========================
> +CPU enable-method bindings
> +==========================
> +
> +The device tree describes the layout of CPUs in a machine in a single "cpus"
> +node, which in turn contains a number of "cpu" sub-nodes defining properties
> +for each cpu.
> +
> +For multiprocessing configurations, CPU cores can be individually enabled
> +and disabled. The enabling capability is used for SMP startup as well as
> +CPU hotplug. A CPU enable method--normally specified in the device tree
> +using an "enable-method" property--defines how cores are enabled. If all
> +CPUs in a machine use the same enable method and related property values,
> +these properties should be defined in the "cpus" node, which associates the
> +property values with all CPUs. Alternatively, every "cpu" node can define
> +its "enable-method" separately.
> +
> +Documents in this directory define how each of the CPU enable methods are to
> +be used, as well the names and possible values of related properties that
> +are required by or affect each enable method.
> diff --git a/Documentation/devicetree/bindings/arm/cpu-enable-method/psci.txt b/Documentation/devicetree/bindings/arm/cpu-enable-method/psci.txt
> new file mode 100644
> index 0000000..68b26c2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/cpu-enable-method/psci.txt
> @@ -0,0 +1,45 @@
> +================================
> +CPU enable-method "psci" binding
> +================================
> +
> +This document describes the "psci" method for enabling secondary CPUs. A
> +"psci" enable method is supported only in individual "cpu" nodes (even if
> + all CPU cores use the "psci" enable method).
> +
> +Enable method name: "psci"
> +Compatible cpus: "arm,cortex-a57" (?)

Any CPU with the security extensions or hypervisor extensions may
support PSCI. So far, implementations exist for at Cortex-A{7,15,53,57},
in addition to AEMs.

> +Related properties: (none)
> +
> +Note:
> +This enable method is only available if a valid PSCI node[1] (compatible
> +with "arm,psci") is present in the device tree, and it defines a "cpu_on"
> +property.
> +
> +Example (contrived 2-core ARM Cortex-A57 64-bit system):
> +
> + psci {
> + compatible = "arm,psci";
> + method = "smc";
> + cpu_on = 0x1;

Missing '<' and '>'.

> + };
> + cpus {
> + #size-cells = <0>;
> + #address-cells = <2>;
> +
> + cpu@0 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a57";
> + reg = <0x0 0x0>;
> + enable-method = "psci";
> + };
> +
> + cpu@1 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a57";
> + reg = <0x0 0x1>;
> + enable-method = "psci";
> + };
> + };
> +
> +--
> +[1] arm/psci.txt

It may be worth folding the existing PSCI binding document in with the
enable-method portion.

> diff --git a/Documentation/devicetree/bindings/arm/cpu-enable-method/qcom,gcc-msm8660 b/Documentation/devicetree/bindings/arm/cpu-enable-method/qcom,gcc-msm8660
> new file mode 100644
> index 0000000..b19f51c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/cpu-enable-method/qcom,gcc-msm8660
> @@ -0,0 +1,30 @@
> +======================================================
> +Secondary CPU enable-method "qcom,gcc-msm8660" binding
> +======================================================
> +
> +This document describes the "qcom,gcc-msm8660" method for enabling secondary
> +CPUs. A "qcom,gcc-msm8660" enable method should only be used in the "cpus"
> +node, to apply to all CPUs.
> +
> +Enable method name: "qcom,gcc-msm8660"
> +Compatible cpu: "qcom,scorpion"
> +Related properties: (none)

>From a look at the code, we need a node compatible with
"qcom,gcc-msm8660" somewhere in the tree (see scss_release_secondary).
It would be good to mention that.

[...]

> diff --git a/Documentation/devicetree/bindings/arm/cpu-enable-method/spin-table.txt b/Documentation/devicetree/bindings/arm/cpu-enable-method/spin-table.txt
> new file mode 100644
> index 0000000..aee3617
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/cpu-enable-method/spin-table.txt
> @@ -0,0 +1,95 @@
> +================================================
> +Secondary CPU enable-method "spin-table" binding
> +================================================
> +
> +This document describes the "spin-table" method for enabling secondary CPUs.
> +A "spin-table" enable method can be used in either the "cpus" node or in
> +individual "cpu" nodes.
> +
> +Enable method name: "spin-table"
> +Compatible cpus: "arm,cortex-a57" (?)

Any CPU with AArch64 may support this.

> +Related properties:
> + - cpu-release-addr
> + Usage: required
> + Value type: <prop-encoded-array>

Just say it's a 64-bit value, "prop-encoded-array" isn't all that
helpful.

> + Definition:
> + A two cell value identifying a 64-bit memory location
> + used by the boot CPU to inform a secondary CPU it
> + should begin its kernel bootstrap. Memory at this
> + location must initially be zeroed.
> +
> +Examples (contrived 4-core ARM Cortex-A57 64-bit systems):
> +
> +The first example uses the same enable method for all cores.
> +
> + cpus {
> + #size-cells = <0>;
> + #address-cells = <2>;
> + enable-method = "spin-table";
> + cpu-release-addr = <0 0x20000000>;

Linux currently expects this to be described per-cpu, and does not
support either of these properties this being defined in /cpus for
arm64.

I would also very much like to discourage using a single release address
-- it means we can only throw all CPUs into the kernel pen, where some
may have to sit forever (breaking things like kexec). Ideally if a
system has to use spin-table the addresses should be unique.

So could we just have the example with unique addresses please?

[...]

Otherwise, this looks like a good first step. It would be nice to see
some qcom folk review the qcom documentation changes.

Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/