> diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
> index 8b9e0a9..35cabe5 100644
> --- a/Documentation/devicetree/bindings/arm/cpus.txt
> +++ b/Documentation/devicetree/bindings/arm/cpus.txt
> @@ -185,6 +185,8 @@ nodes to be present and contain the properties described below.
> be one of:
> "psci"
> "spin-table"
In the case of these two, there's documentation on what the OS, FW, and
HW are expected to do. There's a PSCI spec, and spin-table is documented
in booting.txt (which is admittedly not fantastic).
> + "qcom,arm-cortex-acc"
However, this has no semantics associated with it. Per the code below
it seems to encompass more than just poking the APSS ACC, so the name
isn't great either.
[...]
> + * Based on arch/arm64/kernel/smp_spin_table.c
This is not a phrase that will ever make me happy.
[...]
> +static DEFINE_RAW_SPINLOCK(boot_lock);
We got rid of this for spin-table. It's pointless.
> +DEFINE_PER_CPU(int, cold_boot_done);
This looks suspicious.
> +#if 0
> +static int cold_boot_flags[] = {
> + 0,
> + QCOM_SCM_FLAG_COLDBOOT_CPU1,
> + QCOM_SCM_FLAG_COLDBOOT_CPU2,
> + QCOM_SCM_FLAG_COLDBOOT_CPU3,
> +};
> +#endif
I take it this shouldn't be here?
[...]
> +static int power_on_l2_msm8916(struct device_node *l2ccc_node, u32 pon_mask,
> + int cpu)
> +{
> + u32 pon_status;
> + void __iomem *l2_base;
> +
> + l2_base = of_iomap(l2ccc_node, 0);
> + if (!l2_base)
> + return -ENOMEM;
I didn't see any mention of an L2 requiring power-up in the rest of the
series...
[...]
> +static void write_pen_release(u64 val)
> +{
> + void *start = (void *)&secondary_holding_pen_release;
> + unsigned long size = sizeof(secondary_holding_pen_release);
> +
> + secondary_holding_pen_release = val;
> + smp_wmb();
> + __flush_dcache_area(start, size);
> +}
> +
> +static int secondary_pen_release(unsigned int cpu)
> +{
> + unsigned long timeout;
> +
> + /*
> + * Set synchronisation state between this boot processor
> + * and the secondary one
> + */
> + raw_spin_lock(&boot_lock);
> + write_pen_release(cpu_logical_map(cpu));
> +
> + timeout = jiffies + (1 * HZ);
> + while (time_before(jiffies, timeout)) {
> + if (secondary_holding_pen_release == INVALID_HWID)
> + break;
> + udelay(10);
> + }
> + raw_spin_unlock(&boot_lock);
> +
> + return secondary_holding_pen_release != INVALID_HWID ? -ENOSYS : 0;
> +}
So you want to share the pen, but duplicate the code for managing it?
> +static int __init msm_cpu_init(struct device_node *dn, unsigned int cpu)
> +{
> + /* Mark CPU0 cold boot flag as done */
> + if (!cpu && !per_cpu(cold_boot_done, cpu))
> + per_cpu(cold_boot_done, cpu) = true;
> +
> + return 0;
> +}
[...]
> +static int msm_cpu_boot(unsigned int cpu)
> +{
> + int ret = 0;
> +
> + if (per_cpu(cold_boot_done, cpu) == false) {
> + ret = msm_unclamp_secondary_arm_cpu(cpu);
> + if (ret)
> + return ret;
> + per_cpu(cold_boot_done, cpu) = true;
> + }
> + return secondary_pen_release(cpu);
> +}
Ah, so cold_boot_done is for pseudo-hotplug. Absolute NAK to that.
The only thing this gives you over spin-table is one-time powering up of
the CPUs that can be performed prior to entry to Linux. If you do that,
you can trivially share the spin-table code by setting each CPU's
enable-method to "spin-table".
That won't give you cpuidle or actual hotplug. For those you'll need
PSCI.
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
