Re: [PATCH 2/3] MIPS: Ingenic: Add JZ4780 SMP support to the generic platform

[Paul Cercueil]

Hi Florian,

Le jeudi 05 novembre 2026 à 12:32 -0800, Florian Fainelli a écrit :
> From: 周琰杰 (Zhou Yanjie) <zhouyanjie@xxxxxxxxxxxxxx>
> 
> Port the JZ4780 reset-entry and CP0 mailbox implementation from Zhou
> Yanjie's 2020 v8 series, based on Paul Burton's original CI20 SMP code.
> The old arch/mips/jz4740 platform has moved into the generic MIPS code,
> so register the Ingenic SMP operations after unflattening the device
> tree and before selecting the generic fallback implementations.
> 
> Reuse the upstream CPU clock gates, CPU device-tree nodes and per-CPU
> TCU timers. Enumerate only available CPU nodes within the two-core
> hardware limit. Prepare the CPU1 clock before boot, propagate clock
> failures, serialize mailbox accesses with a raw spinlock and publish
> bootstrap state before releasing the secondary core from reset. Align
> the secondary reset entry to the REIM register's 64 KiB granularity.
> Retain the UP fallback for other SoCs in a shared Ingenic kernel.
> 
> Write back the private data cache before WAIT, since XBurst gates its
> clock during idle and another core must not access dirty data in that
> sleeping cache. Return with interrupts disabled as required by the
> current idle API. Existing per-CPU TCU events make the old forced
> broadcast-timer workaround unnecessary.

This reads like a report from AI. Please use your own words, mention what's
worthwhile to mention (e.g. the Xburst-specific wait mechanism) and drop what's
implementation details.

> This adaptation incorporates the CPU-enumeration review feedback and
> removes the old unused CPU mask. Preserve the original source credits;
> historical Tested-by and Reviewed-by tags are not carried to this
> substantially changed port. Adaptation and validation used Codex
> assistance.
> 
> Link: https://lkml.iu.edu/hypermail/linux/kernel/2005.2/03488.html
> Link: https://lists.openwall.net/linux-kernel/2020/05/13/1482
> Link: https://lists.openwall.net/linux-kernel/2020/05/19/1099
> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@xxxxxxxxxxxxxx>
> [florian: forward ported to latest version]
> Signed-off-by: Florian Fainelli <f.fainelli@xxxxxxxxx>
> ---
>  MAINTAINERS                           |   1 +
>  arch/mips/generic/Makefile            |   1 +
>  arch/mips/generic/ingenic-smp-entry.S |  54 ++++++
>  arch/mips/generic/ingenic-smp.c       | 240 ++++++++++++++++++++++++++
>  arch/mips/generic/ingenic-smp.h       |  20 +++
>  arch/mips/generic/init.c              |   4 +
>  arch/mips/ingenic/Kconfig             |   6 +
>  arch/mips/kernel/idle.c               |  25 ++-
>  8 files changed, 350 insertions(+), 1 deletion(-)
>  create mode 100644 arch/mips/generic/ingenic-smp-entry.S
>  create mode 100644 arch/mips/generic/ingenic-smp.c
>  create mode 100644 arch/mips/generic/ingenic-smp.h

These files really don't belong in arch/mips/generic/. We have a
arch/mips/ingenic/, put them there. You'll have to add a small Makefile there
and list the folder in arch/mips/Kbuild.platforms.

> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 09713dc4c6a7..101027579ccd 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13117,6 +13117,7 @@ L:	linux-mips@xxxxxxxxxxxxxxx
>  S:	Maintained
>  F:	arch/mips/boot/dts/ingenic/
>  F:	arch/mips/generic/board-ingenic.c
> +F:	arch/mips/generic/ingenic-smp*

That's sneaky. Giving me the maintainership like that.
(I'm OK with it though)

>  F:	arch/mips/include/asm/mach-ingenic/
>  F:	arch/mips/ingenic/Kconfig
>  F:	drivers/clk/ingenic/
> diff --git a/arch/mips/generic/Makefile b/arch/mips/generic/Makefile
> index ea0e4ad5e600..df92650c0f15 100644
> --- a/arch/mips/generic/Makefile
> +++ b/arch/mips/generic/Makefile
> @@ -12,5 +12,6 @@ obj-$(CONFIG_YAMON_DT_SHIM)		+= yamon-dt.o
>  obj-$(CONFIG_LEGACY_BOARD_SEAD3)	+= board-sead3.o
>  obj-$(CONFIG_LEGACY_BOARD_OCELOT)	+= board-ocelot.o
>  obj-$(CONFIG_MACH_INGENIC)			+= board-ingenic.o
> +obj-$(CONFIG_MIPS_INGENIC_SMP)	+= ingenic-smp.o ingenic-smp-entry.o
>  obj-$(CONFIG_VIRT_BOARD_RANCHU)		+= board-ranchu.o
>  obj-$(CONFIG_MACH_REALTEK_RTL)		+= board-realtek.o
> diff --git a/arch/mips/generic/ingenic-smp-entry.S
> b/arch/mips/generic/ingenic-smp-entry.S
> new file mode 100644
> index 000000000000..828df4687e84
> --- /dev/null
> +++ b/arch/mips/generic/ingenic-smp-entry.S
> @@ -0,0 +1,54 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2013 Paul Burton <paul.burton@xxxxxxxxxx>

Use his kernel.org address.

> + * JZ4780 secondary CPU reset entry
> + */
> +
> +#include <asm/addrspace.h>
> +#include <asm/asm.h>
> +#include <asm/asmmacro.h>
> +#include <asm/cacheops.h>
> +#include <asm/mipsregs.h>
> +
> +#define CACHE_SIZE	(32 * 1024)
> +#define CACHE_LINESIZE	32
> +
> +	/* The CP0 REIM register only stores the upper 16 address bits. */
> +	.section .text.smp-entry, "ax"
> +	.balign 0x10000
> +	.set noreorder
> +LEAF(jz4780_secondary_cpu_entry)
> +	mtc0	zero, CP0_CAUSE
> +	li	t0, ST0_CU0
> +	mtc0	t0, CP0_STATUS
> +	ehb
> +
> +	/* Invalidate both private caches before entering cached code. */
> +	li	t0, KSEG0
> +	ori	t1, t0, CACHE_SIZE
> +	mtc0	zero, CP0_TAGLO
> +	ehb
> +1:	cache	Index_Store_Tag_I, 0(t0)
> +	cache	Index_Store_Tag_D, 0(t0)
> +	addiu	t0, t0, CACHE_LINESIZE
> +	bne	t0, t1, 1b
> +	 nop
> +
> +	mfc0	t0, CP0_CONFIG
> +	li	t1, ~CONF_CM_CMASK
> +	and	t0, t0, t1
> +	ori	t0, t0, CONF_CM_CACHABLE_NONCOHERENT
> +	mtc0	t0, CP0_CONFIG
> +	mtc0	zero, CP0_PAGEMASK
> +	ehb
> +
> +	/* CPU0 publishes these in RAM before releasing CPU1 from reset. */
> +	la	t0, jz4780_cpu_entry_sp
> +	lw	sp, 0(t0)
> +	la	t0, jz4780_cpu_entry_gp
> +	lw	gp, 0(t0)
> +
> +	la	t0, smp_bootstrap
> +	jr	t0
> +	 nop
> +END(jz4780_secondary_cpu_entry)
> diff --git a/arch/mips/generic/ingenic-smp.c b/arch/mips/generic/ingenic-smp.c
> new file mode 100644
> index 000000000000..98c1fd7483db
> --- /dev/null
> +++ b/arch/mips/generic/ingenic-smp.c
> @@ -0,0 +1,240 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * JZ4780 SMP support, based on Paul Burton's CI20 implementation and
> + * Zhou Yanjie's 2020 JZ4780 SMP series.
> + * Copyright (C) 2013 Paul Burton <paul.burton@xxxxxxxxxx>

Use his kernel.org address.

> + */
> +
> +#include <linux/clk.h>
> +#include <linux/cpu.h>
> +#include <linux/interrupt.h>
> +#include <linux/of.h>
> +#include <linux/of_clk.h>
> +#include <linux/sched/task_stack.h>
> +#include <linux/smp.h>
> +
> +#include <asm/addrspace.h>
> +#include <asm/irq_cpu.h>
> +#include <asm/mipsregs.h>
> +#include <asm/r4kcache.h>
> +#include <asm/smp-ops.h>
> +
> +#include "ingenic-smp.h"
> +
> +#define read_c0_corectrl()	__read_32bit_c0_register($12, 2)
> +#define write_c0_corectrl(val)	__write_32bit_c0_register($12, 2, val)
> +#define read_c0_corestatus()	__read_32bit_c0_register($12, 3)
> +#define write_c0_corestatus(val)	__write_32bit_c0_register($12, 3,
> val)
> +#define read_c0_reim()		__read_32bit_c0_register($12, 4)
> +#define write_c0_reim(val)	__write_32bit_c0_register($12, 4, val)
> +#define read_c0_mailbox0()	__read_32bit_c0_register($20, 0)
> +#define write_c0_mailbox0(val)	__write_32bit_c0_register($20, 0, val)
> +#define read_c0_mailbox1()	__read_32bit_c0_register($20, 1)
> +#define write_c0_mailbox1(val)	__write_32bit_c0_register($20, 1, val)
> +
> +#define JZ4780_NR_CPUS		2
> +#define JZ4780_MBOX_IRQ		(MIPS_CPU_IRQ_BASE + 3)
> +#define CORECTRL_SWRST(cpu)	BIT(cpu)
> +#define CORECTRL_RPC(cpu)	BIT(8 + (cpu))
> +#define CORESTATUS_MIRQ(cpu)	BIT(cpu)
> +#define REIM_ENTRY		GENMASK(31, 16)
> +#define REIM_MBOXIRQ(cpu)	BIT(cpu)
> +
> +static struct clk *cpu1_clock;
> +static DEFINE_RAW_SPINLOCK(smp_lock);
> +
> +/* Referenced by the uncached secondary CPU reset entry. */
> +unsigned long jz4780_cpu_entry_sp;
> +unsigned long jz4780_cpu_entry_gp;
> +
> +static irqreturn_t jz4780_mbox_interrupt(int irq, void *dev_id)
> +{
> +	unsigned int cpu = cpu_logical_map(smp_processor_id());
> +	u32 action, status;
> +
> +	raw_spin_lock(&smp_lock);
> +	if (cpu == 0) {
> +		action = read_c0_mailbox0();
> +		write_c0_mailbox0(0);
> +	} else {
> +		action = read_c0_mailbox1();
> +		write_c0_mailbox1(0);
> +	}

Zhou's v8 supported the 4 mailboxes, this feels like a regression.

I understand that we can't really test a quad-core system (unless Nikolaus can?)
so I won't object, but that might need to change in the future.

> +
> +	status = read_c0_corestatus();
> +	write_c0_corestatus(status & ~CORESTATUS_MIRQ(cpu));
> +	irq_disable_hazard();
> +	raw_spin_unlock(&smp_lock);
> +
> +	if (action & SMP_RESCHEDULE_YOURSELF)
> +		scheduler_ipi();
> +	if (action & SMP_CALL_FUNCTION)
> +		generic_smp_call_function_interrupt();
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static void __init jz4780_smp_setup(void)
> +{
> +	struct device_node *np;
> +	u32 entry, reim;
> +	u64 hwid;
> +
> +	for_each_of_cpu_node(np) {
> +		hwid = of_get_cpu_hwid(np, 0);
> +		if (hwid >= nr_cpu_ids || hwid >= JZ4780_NR_CPUS ||

I think these checks on hwid are pointless;
- nr_cpu_ids will never be below hwid unless you read the CPU nodes in reverse
(in which case that's a DT issue),
- hwid >= JZ4780_NR_CPUS will only be true if the DT is broken, too.

> +		    !of_device_is_available(np))
> +			continue;
> +		__cpu_number_map[hwid] = hwid;
> +		__cpu_logical_map[hwid] = hwid;

That looks wrong. The map index should be incremented for each entry you add,
here you'll have holes if one CPU is offline.

> +		set_cpu_possible(hwid, true);
> +	}
> +
> +	/* Program the shared reset vector with mailbox delivery masked. */
> +	reim = read_c0_reim();
> +	reim &= ~(REIM_MBOXIRQ(0) | REIM_MBOXIRQ(1));
> +	write_c0_reim(reim);
> +	write_c0_mailbox0(0);
> +	write_c0_mailbox1(0);
> +	write_c0_corestatus(0);
> +
> +	entry = KSEG1ADDR((unsigned long)jz4780_secondary_cpu_entry);
> +	write_c0_reim((reim & ~REIM_ENTRY) | (entry & REIM_ENTRY));
> +}
> +
> +static void __init jz4780_prepare_cpus(unsigned int max_cpus)
> +{
> +	struct device_node *np;
> +	u32 reim;
> +	int ret;
> +
> +	ret = request_irq(JZ4780_MBOX_IRQ, jz4780_mbox_interrupt,
> +			  IRQF_PERCPU | IRQF_NO_THREAD, "core mailbox",
> NULL);
> +	if (ret)
> +		panic("JZ4780: cannot request mailbox IRQ: %d", ret);

I think this panic is superfluous - if you return here instead I believe that
the system will still be working, although in single-CPU mode.

> +
> +	reim = read_c0_reim();
> +	write_c0_reim(reim | REIM_MBOXIRQ(0));
> +	set_c0_status(STATUSF_IP3);
> +	irq_enable_hazard();
> +
> +	if (!cpu_possible(1))
> +		return;
> +
> +	np = of_get_cpu_node(1, NULL);
> +	cpu1_clock = of_clk_get(np, 0);
> +	of_node_put(np);
> +	if (IS_ERR(cpu1_clock)) {
> +		pr_err("JZ4780: cannot get CPU1 clock: %ld\n",
> +		       PTR_ERR(cpu1_clock));
> +		return;
> +	}
> +
> +	ret = clk_prepare(cpu1_clock);
> +	if (ret) {
> +		clk_put(cpu1_clock);
> +		cpu1_clock = ERR_PTR(ret);
> +		pr_err("JZ4780: cannot prepare CPU1 clock: %d\n", ret);
> +	}
> +}
> +
> +static int jz4780_boot_secondary(int cpu, struct task_struct *idle)
> +{
> +	unsigned long flags;
> +	u32 ctrl;
> +	int ret;
> +
> +	if (cpu_logical_map(cpu) != 1 || IS_ERR_OR_NULL(cpu1_clock))
> +		return -ENODEV;
> +
> +	raw_spin_lock_irqsave(&smp_lock, flags);
> +	ctrl = read_c0_corectrl() | CORECTRL_SWRST(1) | CORECTRL_RPC(1);
> +	write_c0_corectrl(ctrl);
> +	back_to_back_c0_hazard();
> +
> +	ret = clk_enable(cpu1_clock);
> +	if (ret)
> +		goto unlock;
> +
> +	jz4780_cpu_entry_sp = __KSTK_TOS(idle);
> +	jz4780_cpu_entry_gp = (unsigned long)task_thread_info(idle);
> +	/* Publish bootstrap state before CPU1 starts using its private
> cache. */
> +	r4k_blast_dcache();
> +	/* Complete cache writeback before releasing reset. */
> +	mb();
> +	write_c0_corectrl(ctrl & ~CORECTRL_SWRST(1));
> +	back_to_back_c0_hazard();
> +unlock:
> +	raw_spin_unlock_irqrestore(&smp_lock, flags);
> +	return ret;
> +}
> +
> +static void jz4780_init_secondary(void)
> +{
> +	clear_c0_status(ST0_IM);
> +	irq_disable_hazard();
> +}
> +
> +static void jz4780_smp_finish(void)
> +{
> +	unsigned int cpu = cpu_logical_map(smp_processor_id());
> +	u32 reim;
> +
> +	raw_spin_lock(&smp_lock);
> +	reim = read_c0_reim();
> +	write_c0_reim(reim | REIM_MBOXIRQ(cpu));
> +	raw_spin_unlock(&smp_lock);
> +
> +	change_c0_status(ST0_IM, STATUSF_IP3 | STATUSF_IP2 |
> +			 STATUSF_IP1 | STATUSF_IP0);
> +	irq_enable_hazard();
> +	local_irq_enable();
> +}
> +
> +static void jz4780_send_ipi_locked(int cpu, unsigned int action)
> +{
> +	if (cpu_logical_map(cpu) == 0)
> +		write_c0_mailbox0(read_c0_mailbox0() | action);
> +	else
> +		write_c0_mailbox1(read_c0_mailbox1() | action);
> +	back_to_back_c0_hazard();
> +}
> +
> +static void jz4780_send_ipi_single(int cpu, unsigned int action)
> +{
> +	unsigned long flags;
> +
> +	raw_spin_lock_irqsave(&smp_lock, flags);
> +	jz4780_send_ipi_locked(cpu, action);
> +	raw_spin_unlock_irqrestore(&smp_lock, flags);
> +}
> +
> +static void jz4780_send_ipi_mask(const struct cpumask *mask, unsigned int
> action)
> +{
> +	unsigned long flags;
> +	int cpu;
> +
> +	raw_spin_lock_irqsave(&smp_lock, flags);
> +	for_each_cpu(cpu, mask)
> +		jz4780_send_ipi_locked(cpu, action);
> +	raw_spin_unlock_irqrestore(&smp_lock, flags);
> +}
> +
> +static const struct plat_smp_ops jz4780_smp_ops = {
> +	.send_ipi_single = jz4780_send_ipi_single,
> +	.send_ipi_mask = jz4780_send_ipi_mask,
> +	.init_secondary = jz4780_init_secondary,
> +	.smp_finish = jz4780_smp_finish,
> +	.boot_secondary = jz4780_boot_secondary,
> +	.smp_setup = jz4780_smp_setup,
> +	.prepare_cpus = jz4780_prepare_cpus,
> +};
> +
> +int __init register_ingenic_smp_ops(void)
> +{
> +	if (!of_machine_is_compatible("ingenic,jz4780"))
> +		return -ENODEV;
> +
> +	register_smp_ops(&jz4780_smp_ops);
> +	return 0;
> +}

Overall, the code itself is pretty much a Xburst (CPU) driver that gives the
idea (through function names, etc) that it is SoC-specific (JZ4780), while the
source file has a very generic "ingenic-smp.c" name.

So I'd suggest to replace all 'jz4780' by 'xburst'.

> diff --git a/arch/mips/generic/ingenic-smp.h b/arch/mips/generic/ingenic-smp.h
> new file mode 100644
> index 000000000000..54191be8731a
> --- /dev/null
> +++ b/arch/mips/generic/ingenic-smp.h
> @@ -0,0 +1,20 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/* Copyright (C) 2013 Paul Burton <paul.burton@xxxxxxxxxx> */

Use his kernel.org address.

> +#ifndef __MIPS_GENERIC_INGENIC_SMP_H__
> +#define __MIPS_GENERIC_INGENIC_SMP_H__
> +
> +#include <linux/errno.h>
> +#include <linux/init.h>
> +#include <linux/kconfig.h>
> +
> +#ifdef CONFIG_MIPS_INGENIC_SMP
> +int __init register_ingenic_smp_ops(void);
> +void jz4780_secondary_cpu_entry(void);
> +#else
> +static inline int register_ingenic_smp_ops(void)
> +{
> +	return -ENODEV;
> +}
> +#endif
> +
> +#endif /* __MIPS_GENERIC_INGENIC_SMP_H__ */
> diff --git a/arch/mips/generic/init.c b/arch/mips/generic/init.c
> index 1d712eac1617..00149bd1e9a9 100644
> --- a/arch/mips/generic/init.c
> +++ b/arch/mips/generic/init.c
> @@ -20,6 +20,8 @@
>  #include <asm/smp-ops.h>
>  #include <asm/time.h>
>  
> +#include "ingenic-smp.h"
> +
>  static __initconst const void *fdt;
>  static __initconst const struct mips_machine *mach;
>  static __initconst const void *mach_match_data;
> @@ -113,6 +115,8 @@ void __init device_tree_init(void)
>  	unflatten_and_copy_device_tree();
>  	mips_cpc_probe();
>  
> +	if (!register_ingenic_smp_ops())
> +		return;
>  	if (!register_cps_smp_ops())
>  		return;
>  	if (!register_vsmp_smp_ops())
> diff --git a/arch/mips/ingenic/Kconfig b/arch/mips/ingenic/Kconfig
> index f595b339a4b8..1dfd0bfd0193 100644
> --- a/arch/mips/ingenic/Kconfig
> +++ b/arch/mips/ingenic/Kconfig
> @@ -64,6 +64,12 @@ config MACH_JZ4780
>  	select MIPS_CPU_SCACHE
>  	select SYS_HAS_CPU_MIPS32_R2
>  	select SYS_SUPPORTS_HIGHMEM
> +	select SYS_SUPPORTS_SMP
> +
> +config MIPS_INGENIC_SMP
> +	def_bool y
> +	depends on MACH_JZ4780 && SMP
> +	select SMP_UP

I don't think you need an extra symbol here, you could just add 'select SMP_UP
if SMP' to the MACH_JZ4780 symbol.

>  
>  config MACH_X1000
>  	bool
> diff --git a/arch/mips/kernel/idle.c b/arch/mips/kernel/idle.c
> index 80e8a04a642e..6cabdd4825a7 100644
> --- a/arch/mips/kernel/idle.c
> +++ b/arch/mips/kernel/idle.c
> @@ -18,6 +18,7 @@
>  #include <asm/cpu-type.h>
>  #include <asm/idle.h>
>  #include <asm/mipsregs.h>
> +#include <asm/r4kcache.h>
>  
>  /*
>   * Not all of the MIPS CPUs have the "wait" instruction available. Moreover,
> @@ -71,6 +72,23 @@ static void __cpuidle rm7k_wait_irqoff(void)
>  		"	.set	pop					\n");
>  }
>  
> +/* XBurst gates its private cache clock in WAIT; write back dirty data first.
> */
> +static void __cpuidle ingenic_smp_wait_irqoff(void)
> +{
> +	unsigned long pending = read_c0_cause() & read_c0_status() &
> CAUSEF_IP;
> +
> +	if (!need_resched() && !pending) {
> +		r4k_blast_dcache();
> +		__asm__ __volatile__(
> +			".set push\n"
> +			".set mips3\n"
> +			"sync\n"
> +			"wait\n"
> +			".set pop\n"
> +			: : : "memory");
> +	}
> +}
> +
>  /*
>   * Au1 'wait' is only useful when the 32kHz counter is used as timer,
>   * since coreclock (and the cp0 counter) stops upon executing it. Only an
> @@ -156,10 +174,15 @@ void __init check_wait(void)
>  	case CPU_CAVIUM_OCTEON_PLUS:
>  	case CPU_CAVIUM_OCTEON2:
>  	case CPU_CAVIUM_OCTEON3:
> -	case CPU_XBURST:
>  	case CPU_LOONGSON32:
>  		cpu_wait = r4k_wait;
>  		break;
> +	case CPU_XBURST:
> +		if (IS_ENABLED(CONFIG_MIPS_INGENIC_SMP))
> +			cpu_wait = ingenic_smp_wait_irqoff;
> +		else
> +			cpu_wait = r4k_wait;
> +		break;
>  	case CPU_LOONGSON64:
>  		if ((c->processor_id & (PRID_IMP_MASK | PRID_REV_MASK)) >=
>  				(PRID_IMP_LOONGSON_64C |
> PRID_REV_LOONGSON3A_R2_0) ||

Cheers,
-Paul