From: Florian Fainelli <f.fainelli@gmail.com> To: Michel Pollet <michel.pollet@bp.renesas.com>, linux-renesas-soc@vger.kernel.org, Simon Horman <horms@verge.net.au> Cc: phil.edworthy@renesas.com, buserror+upstream@gmail.com, "Rob Herring" <robh+dt@kernel.org>, "Mark Rutland" <mark.rutland@arm.com>, "Magnus Damm" <magnus.damm@gmail.com>, "Russell King" <linux@armlinux.org.uk>, "Chen-Yu Tsai" <wens@csie.org>, "Martin Blumenstingl" <martin.blumenstingl@googlemail.com>, "Andreas Färber" <afaerber@suse.de>, "Andy Gross" <andy.gross@linaro.org>, "Carlo Caione" <carlo@endlessm.com>, "Rajendra Nayak" <rnayak@codeaurora.org>, "Frank Rowand" <frank.rowand@sony.com>, "Juri Lelli" <juri.lelli@arm.com>, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [RFC 3/3] arm: shmobile: Add the RZ/N1D SMP enabler driver. Date: Mon, 16 Apr 2018 14:46:16 -0700 [thread overview] Message-ID: <9f33a1d2-3c55-2485-a1f2-909def69e5b0@gmail.com> (raw) In-Reply-To: <1523871304-48517-5-git-send-email-michel.pollet@bp.renesas.com> 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
WARNING: multiple messages have this Message-ID (diff)
From: f.fainelli@gmail.com (Florian Fainelli) To: linux-arm-kernel@lists.infradead.org Subject: [RFC 3/3] arm: shmobile: Add the RZ/N1D SMP enabler driver. Date: Mon, 16 Apr 2018 14:46:16 -0700 [thread overview] Message-ID: <9f33a1d2-3c55-2485-a1f2-909def69e5b0@gmail.com> (raw) In-Reply-To: <1523871304-48517-5-git-send-email-michel.pollet@bp.renesas.com> 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
next prev parent reply other threads:[~2018-04-16 21:46 UTC|newest] Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-04-16 9:34 [RFC 0/3] Renesas RZ/N1D SMP enabler Michel Pollet 2018-04-16 9:34 ` Michel Pollet 2018-04-16 9:34 ` Michel Pollet 2018-04-16 9:34 ` [PATCH 1/1] arm: rzn1: Add support for the second CPU Michel Pollet 2018-04-16 9:34 ` Michel Pollet 2018-04-16 9:34 ` [RFC 1/3] dt-bindings: cpu: Add Renesas RZ/N1D SMP enable method Michel Pollet 2018-04-16 9:34 ` Michel Pollet 2018-04-16 21:37 ` Rob Herring 2018-04-16 21:37 ` Rob Herring 2018-04-16 9:34 ` [RFC 2/3] ARM: dts: " Michel Pollet 2018-04-16 9:34 ` Michel Pollet 2018-04-16 9:34 ` [RFC 3/3] arm: shmobile: Add the RZ/N1D SMP enabler driver Michel Pollet 2018-04-16 9:34 ` Michel Pollet 2018-04-16 21:46 ` Florian Fainelli [this message] 2018-04-16 21:46 ` Florian Fainelli 2018-04-17 9:32 ` Michel Pollet 2018-04-17 9:32 ` Michel Pollet 2018-04-17 9:32 ` Michel Pollet
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=9f33a1d2-3c55-2485-a1f2-909def69e5b0@gmail.com \ --to=f.fainelli@gmail.com \ --cc=afaerber@suse.de \ --cc=andy.gross@linaro.org \ --cc=buserror+upstream@gmail.com \ --cc=carlo@endlessm.com \ --cc=devicetree@vger.kernel.org \ --cc=frank.rowand@sony.com \ --cc=horms@verge.net.au \ --cc=juri.lelli@arm.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-renesas-soc@vger.kernel.org \ --cc=linux@armlinux.org.uk \ --cc=magnus.damm@gmail.com \ --cc=mark.rutland@arm.com \ --cc=martin.blumenstingl@googlemail.com \ --cc=michel.pollet@bp.renesas.com \ --cc=phil.edworthy@renesas.com \ --cc=rnayak@codeaurora.org \ --cc=robh+dt@kernel.org \ --cc=wens@csie.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.