| Message ID | 20170105033641.6212-3-chris.packham@alliedtelesis.co.nz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Le 01/04/17 à 19:36, Chris Packham a écrit : > Compared to the armada-xp the 98DX3336 uses different registers to set > the boot address for the secondary CPU so a new enable-method is needed. > This will only work if the machine definition doesn't define an overall > smp_ops because there is not currently a way of overriding this from the > device tree if it is set in the machine definition. Not sure I follow you here, in theory, each individual CPU could have a different enable-method property, and considering how you leverage existing DTS include files, you should be able to override the DTS with the appropriate enable-method, or do you have a different problem? mv98dx3236_resume_set_cpu_boot_addr(int hw_cpu, void *boot_addr) > +{ > + WARN_ON(hw_cpu != 1); > + > + writel(0, mv98dx3236_resume_base + MV98DX3236_CPU_RESUME_CTRL_OFFSET); > + writel(virt_to_phys(boot_addr), mv98dx3236_resume_base + > + MV98DX3236_CPU_RESUME_ADDR_OFFSET); I just submitted a patch series that switches such users to __pa_symbol() instead of virt_to_phys() because this is a kernel image symbol. For now this will work as-is, but depending on which patch series makes it first, it may be a good idea to keep this on someone's TODO list (yours or mine). I do expect having to make a second pass of conversions anyway. [1]: https://lkml.org/lkml/2017/1/4/1101 > +} > + > +static int __init mv98dx3236_resume_init(void) > +{ > + struct device_node *np; > + struct resource res; > + int ret = 0; > + > + np = of_find_matching_node(NULL, of_mv98dx3236_resume_table); > + if (!np) > + return 0; > + > + pr_info("Initializing 98DX3236 Resume\n"); This can be probably be dropped, you should know fairly quickly if this succeeded or not. Can't this function be implemented as a smp_ops::smp_init_cpus instead of having this initialization done at arch_initcall time? > + > + if (of_address_to_resource(np, 0, &res)) { > + pr_err("unable to get resource\n"); > + ret = -ENOENT; > + goto out; > + } > + > + if (!request_mem_region(res.start, resource_size(&res), > + np->full_name)) { > + pr_err("unable to request region\n"); > + ret = -EBUSY; > + goto out; > + } You should be able to use of_io_request_and_map() and here.
On 05/01/17 17:04, Florian Fainelli wrote: > Le 01/04/17 à 19:36, Chris Packham a écrit : >> Compared to the armada-xp the 98DX3336 uses different registers to set >> the boot address for the secondary CPU so a new enable-method is needed. >> This will only work if the machine definition doesn't define an overall >> smp_ops because there is not currently a way of overriding this from the >> device tree if it is set in the machine definition. > > Not sure I follow you here, in theory, each individual CPU could have a > different enable-method property, and considering how you leverage > existing DTS include files, you should be able to override the DTS with > the appropriate enable-method, or do you have a different problem? > The problem is I can't override the enable method if it's set in the machine definition. It's because the devicetree is looked up first then the machine definition overrides it. Heres an old thread that basically outlines the problem. http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/300371.html I posted a few attempts to work around it but there never really was any consensus reached. > mv98dx3236_resume_set_cpu_boot_addr(int hw_cpu, void *boot_addr) >> +{ >> + WARN_ON(hw_cpu != 1); >> + >> + writel(0, mv98dx3236_resume_base + MV98DX3236_CPU_RESUME_CTRL_OFFSET); >> + writel(virt_to_phys(boot_addr), mv98dx3236_resume_base + >> + MV98DX3236_CPU_RESUME_ADDR_OFFSET); > > I just submitted a patch series that switches such users to > __pa_symbol() instead of virt_to_phys() because this is a kernel image > symbol. For now this will work as-is, but depending on which patch > series makes it first, it may be a good idea to keep this on someone's > TODO list (yours or mine). I do expect having to make a second pass of > conversions anyway. > > [1]: https://lkml.org/lkml/2017/1/4/1101 OK I'll keep that in mind. > >> +} >> + >> +static int __init mv98dx3236_resume_init(void) >> +{ >> + struct device_node *np; >> + struct resource res; >> + int ret = 0; >> + >> + np = of_find_matching_node(NULL, of_mv98dx3236_resume_table); >> + if (!np) >> + return 0; >> + >> + pr_info("Initializing 98DX3236 Resume\n"); > > This can be probably be dropped, you should know fairly quickly if this > succeeded or not. Will do. > > Can't this function be implemented as a smp_ops::smp_init_cpus instead > of having this initialization done at arch_initcall time? > I'll look into it. My initial reaction is no because I still need armada_xp_smp_init_cpus(). >> + >> + if (of_address_to_resource(np, 0, &res)) { >> + pr_err("unable to get resource\n"); >> + ret = -ENOENT; >> + goto out; >> + } >> + >> + if (!request_mem_region(res.start, resource_size(&res), >> + np->full_name)) { >> + pr_err("unable to request region\n"); >> + ret = -EBUSY; >> + goto out; >> + } > > You should be able to use of_io_request_and_map() and here. > Will do.
On 05/01/17 17:46, Chris Packham wrote: > On 05/01/17 17:04, Florian Fainelli wrote: >> Le 01/04/17 à 19:36, Chris Packham a écrit : >>> +} >>> + >>> +static int __init mv98dx3236_resume_init(void) >>> +{ >>> + struct device_node *np; >>> + struct resource res; >>> + int ret = 0; >>> + >>> + np = of_find_matching_node(NULL, of_mv98dx3236_resume_table); >>> + if (!np) >>> + return 0; >> >> Can't this function be implemented as a smp_ops::smp_init_cpus instead >> of having this initialization done at arch_initcall time? >> > > I'll look into it. My initial reaction is no because I still need > armada_xp_smp_init_cpus(). > I looked at this. I could write a mv98dx3236_smp_init_cpus() that calls armada_xp_smp_init_cpus() and inits the resume controller address. My original reason for this approach was to parallel mvebu_v7_pmsu_init. I won't do anything just yet but is there any downside to the current approach?
diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt index a1bcfeed5f24..3c2fd72d0bf9 100644 --- a/Documentation/devicetree/bindings/arm/cpus.txt +++ b/Documentation/devicetree/bindings/arm/cpus.txt @@ -202,6 +202,7 @@ nodes to be present and contain the properties described below. "marvell,armada-380-smp" "marvell,armada-390-smp" "marvell,armada-xp-smp" + "marvell,98dx3236-smp" "mediatek,mt6589-smp" "mediatek,mt81xx-tz-smp" "qcom,gcc-msm8660" diff --git a/Documentation/devicetree/bindings/arm/marvell/98dx3236-resume-ctrl.txt b/Documentation/devicetree/bindings/arm/marvell/98dx3236-resume-ctrl.txt new file mode 100644 index 000000000000..8082ba872edd --- /dev/null +++ b/Documentation/devicetree/bindings/arm/marvell/98dx3236-resume-ctrl.txt @@ -0,0 +1,18 @@ +Resume Control +-------------- +Available on Marvell SOCs: 98DX3336 and 98DX4251 + +Required properties: + +- compatible: must be "marvell,98dx3336-resume-ctrl" + +- reg: Should contain resume control registers location and length + +Example: + +resume@20980 { + compatible = "marvell,98dx3336-resume-ctrl"; + reg = <0x20980 0x10>; +}; + + diff --git a/arch/arm/mach-mvebu/Makefile b/arch/arm/mach-mvebu/Makefile index 6c6497e80a7b..2a2dd8324fb8 100644 --- a/arch/arm/mach-mvebu/Makefile +++ b/arch/arm/mach-mvebu/Makefile @@ -7,6 +7,7 @@ obj-$(CONFIG_MACH_MVEBU_ANY) += system-controller.o mvebu-soc-id.o ifeq ($(CONFIG_MACH_MVEBU_V7),y) obj-y += cpu-reset.o board-v7.o coherency.o coherency_ll.o pmsu.o pmsu_ll.o +obj-y += pmsu-98dx3236.o obj-$(CONFIG_PM) += pm.o pm-board.o obj-$(CONFIG_SMP) += platsmp.o headsmp.o platsmp-a9.o headsmp-a9.o diff --git a/arch/arm/mach-mvebu/common.h b/arch/arm/mach-mvebu/common.h index 6b775492cfad..099dabf23461 100644 --- a/arch/arm/mach-mvebu/common.h +++ b/arch/arm/mach-mvebu/common.h @@ -27,4 +27,5 @@ void __iomem *mvebu_get_scu_base(void); int mvebu_pm_suspend_init(void (*board_pm_enter)(void __iomem *sdram_reg, u32 srcmd)); +void mv98dx3236_resume_set_cpu_boot_addr(int hw_cpu, void *boot_addr); #endif diff --git a/arch/arm/mach-mvebu/platsmp.c b/arch/arm/mach-mvebu/platsmp.c index 46c742d3bd41..3c9ab9a008ad 100644 --- a/arch/arm/mach-mvebu/platsmp.c +++ b/arch/arm/mach-mvebu/platsmp.c @@ -182,5 +182,48 @@ const struct smp_operations armada_xp_smp_ops __initconst = { #endif }; +static int mv98dx3236_boot_secondary(unsigned int cpu, struct task_struct *idle) +{ + int ret, hw_cpu; + + pr_info("Booting CPU %d\n", cpu); + + hw_cpu = cpu_logical_map(cpu); + set_secondary_cpu_clock(hw_cpu); + mv98dx3236_resume_set_cpu_boot_addr(hw_cpu, + armada_xp_secondary_startup); + + /* + * This is needed to wake up CPUs in the offline state after + * using CPU hotplug. + */ + arch_send_wakeup_ipi_mask(cpumask_of(cpu)); + + /* + * This is needed to take secondary CPUs out of reset on the + * initial boot. + */ + ret = mvebu_cpu_reset_deassert(hw_cpu); + if (ret) { + pr_warn("unable to boot CPU: %d\n", ret); + return ret; + } + + return 0; +} + +struct smp_operations mv98dx3236_smp_ops __initdata = { + .smp_init_cpus = armada_xp_smp_init_cpus, + .smp_prepare_cpus = armada_xp_smp_prepare_cpus, + .smp_boot_secondary = mv98dx3236_boot_secondary, + .smp_secondary_init = armada_xp_secondary_init, +#ifdef CONFIG_HOTPLUG_CPU + .cpu_die = armada_xp_cpu_die, + .cpu_kill = armada_xp_cpu_kill, +#endif +}; + CPU_METHOD_OF_DECLARE(armada_xp_smp, "marvell,armada-xp-smp", &armada_xp_smp_ops); +CPU_METHOD_OF_DECLARE(mv98dx3236_smp, "marvell,98dx3236-smp", + &mv98dx3236_smp_ops); diff --git a/arch/arm/mach-mvebu/pmsu-98dx3236.c b/arch/arm/mach-mvebu/pmsu-98dx3236.c new file mode 100644 index 000000000000..87ca42ef40c7 --- /dev/null +++ b/arch/arm/mach-mvebu/pmsu-98dx3236.c @@ -0,0 +1,69 @@ +/** + * CPU resume support for 98DX3236 internal CPU (a.k.a. MSYS). + */ + +#define pr_fmt(fmt) "mv98dx3236-resume: " fmt + +#include <linux/kernel.h> +#include <linux/init.h> +#include <linux/of_address.h> +#include <linux/io.h> +#include "common.h" + +static void __iomem *mv98dx3236_resume_base; +#define MV98DX3236_CPU_RESUME_CTRL_OFFSET 0x08 +#define MV98DX3236_CPU_RESUME_ADDR_OFFSET 0x04 + +static const struct of_device_id of_mv98dx3236_resume_table[] = { + {.compatible = "marvell,98dx3336-resume-ctrl",}, + { /* end of list */ }, +}; + +void mv98dx3236_resume_set_cpu_boot_addr(int hw_cpu, void *boot_addr) +{ + WARN_ON(hw_cpu != 1); + + writel(0, mv98dx3236_resume_base + MV98DX3236_CPU_RESUME_CTRL_OFFSET); + writel(virt_to_phys(boot_addr), mv98dx3236_resume_base + + MV98DX3236_CPU_RESUME_ADDR_OFFSET); +} + +static int __init mv98dx3236_resume_init(void) +{ + struct device_node *np; + struct resource res; + int ret = 0; + + np = of_find_matching_node(NULL, of_mv98dx3236_resume_table); + if (!np) + return 0; + + pr_info("Initializing 98DX3236 Resume\n"); + + if (of_address_to_resource(np, 0, &res)) { + pr_err("unable to get resource\n"); + ret = -ENOENT; + goto out; + } + + if (!request_mem_region(res.start, resource_size(&res), + np->full_name)) { + pr_err("unable to request region\n"); + ret = -EBUSY; + goto out; + } + + mv98dx3236_resume_base = ioremap(res.start, resource_size(&res)); + if (!mv98dx3236_resume_base) { + pr_err("unable to map registers\n"); + release_mem_region(res.start, resource_size(&res)); + ret = -ENOMEM; + goto out; + } + +out: + of_node_put(np); + return ret; +} + +early_initcall(mv98dx3236_resume_init);
Compared to the armada-xp the 98DX3336 uses different registers to set the boot address for the secondary CPU so a new enable-method is needed. This will only work if the machine definition doesn't define an overall smp_ops because there is not currently a way of overriding this from the device tree if it is set in the machine definition. Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> --- Changes in v2: - Document new enable-method value - Correct some references from 98DX4521 to 98DX3236 Documentation/devicetree/bindings/arm/cpus.txt | 1 + .../bindings/arm/marvell/98dx3236-resume-ctrl.txt | 18 ++++++ arch/arm/mach-mvebu/Makefile | 1 + arch/arm/mach-mvebu/common.h | 1 + arch/arm/mach-mvebu/platsmp.c | 43 ++++++++++++++ arch/arm/mach-mvebu/pmsu-98dx3236.c | 69 ++++++++++++++++++++++ 6 files changed, 133 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/marvell/98dx3236-resume-ctrl.txt create mode 100644 arch/arm/mach-mvebu/pmsu-98dx3236.c