lkml.org 
[lkml]   [2018]   [Apr]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFC 3/3] arm: shmobile: Add the RZ/N1D SMP enabler driver.
From
Date
Hi Michel,

On 04/16/2018 02:34 AM, Michel Pollet wrote:
> The Renesas RZ/N1D second CA7 is parked in a ROM pen at boot time, it
> requires a special enable method to get it started at boot time.
>
> Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>

Some few comments below. This patch should probably be re-ordered in
your patch series, I would expect you to have this become patch 2 and
have patch 2 be patch 3 (first you add infrastructure for using
something, then you make use of it).

> +static int rzn1_smp_boot_secondary(unsigned int cpu, struct task_struct *idle)
> +{
> + if (!cpu_bootaddr)
> + return -ENODEV;
> +
> + spin_lock(&cpu_lock);
> +
> + writel(virt_to_phys(secondary_startup), cpu_bootaddr);

Consider using __pa_symbol() instead of virt_to_phys() since
secondary_startup is part of the kernel's linear memory map.

> + arch_send_wakeup_ipi_mask(cpumask_of(cpu));
> +
> + spin_unlock(&cpu_lock);
> +
> + return 0;
> +}
> +
> +static void __init rzn1_smp_prepare_cpus(unsigned int max_cpus)
> +{
> + struct device_node *dn;
> + int ret;
> + u32 bootaddr;
> +
> + dn = of_get_cpu_node(1, NULL);
> + if (!dn) {
> + pr_err("CPU#1: missing device tree node\n");
> + return;
> + }
> + /*
> + * Determine the address from which the CPU is polling.
> + */
> + ret = of_property_read_u32(dn, "cpu-release-addr", &bootaddr);
> + if (ret)
> + pr_err("CPU#1: invalid cpu-release-addr property\n");
> +
> + of_node_put(dn);
> + /* The bootloader *does* change this property */

This comment should probably be moved above the function that fetches
"cpu-release-addr"

> + pr_info("CPU#1: cpu-release-addr %08x\n", (u32)bootaddr);
> +
> + if (!bootaddr)
> + return;

Would not you want to show a message here to help catch such conditions

> +
> + cpu_bootaddr = ioremap(bootaddr, sizeof(bootaddr));

Relying on ioremap() to reject values that might be outside of the
allowed range may be a little fragile, but I can't suggest any better
alternative.

> + if (!cpu_bootaddr)
> + pr_err("CPU#1: cpu-release-addr map failed\n");
> +}
> +
> +static const struct smp_operations rzn1_smp_ops __initconst = {
> + .smp_prepare_cpus = rzn1_smp_prepare_cpus,
> + .smp_boot_secondary = rzn1_smp_boot_secondary,
> +};
> +CPU_METHOD_OF_DECLARE(rzn1_smp, "renesas,r9a06g032-smp", &rzn1_smp_ops);
>


--
Florian

\
 
 \ /
  Last update: 2018-04-16 23:47    [W:0.050 / U:15.564 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site