Re: [PATCH v4 2/3] MIPS: Ingenic: Add XBurst SMP support

[Florian Fainelli <f.fainelli@gmail.com>]

On 11/13/26 04:54, Paul Cercueil wrote:
> Hi Florian,
> 
> Le jeudi 12 novembre 2026 à 10:44 -0800, Florian Fainelli a écrit :
>> From: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>>
>> Add support for the second XBurst core on the JZ4780, based on Zhou
>> Yanjie's 2020 series and Paul Burton's original CI20 SMP implementation.
>> Use the CP0 mailboxes for inter-processor interrupts.
>>
>> XBurst stops its private cache clock when executing WAIT. Write back
>> the data cache before entering idle so that the other core can access
>> data belonging to the sleeping core.
>>
>> 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
>> Reviewed-by: Paul Cercueil <paul@crapouillou.net>
>> Tested-by: Paul Cercueil <paul@crapouillou.net>
> 
> You updated the patch after my Reviewed-by, so that voids it...

Indeed, sorry about that.

> 
>> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>> [florian: forward ported to latest version]
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
> 
> Please add a changelog per patch too.

I would rather do this only in the cover letter, but fine, I will oblige 
here.

[snip]

>> +static const struct plat_smp_ops xburst_smp_ops = {
>> +	.send_ipi_single = xburst_send_ipi_single,
>> +	.send_ipi_mask = xburst_send_ipi_mask,
>> +	.init_secondary = xburst_init_secondary,
>> +	.smp_finish = xburst_smp_finish,
>> +	.boot_secondary = xburst_boot_secondary,
>> +	.smp_setup = xburst_smp_setup,
>> +	.prepare_cpus = xburst_prepare_cpus,
>> +#ifdef CONFIG_HOTPLUG_CPU
>> +	.cpu_disable = xburst_cpu_disable,
>> +#endif
> 
> Drop the macro guards:
> .cpu_disable = IF_PTR(IS_ENABLED(CONFIG_HOTPLUG_CPU), xburst_cpu_disable),

OK.

> 
>> +};
>> +
>> +int __init register_ingenic_smp_ops(void)
>> +{
>> +	if (!of_machine_is_compatible("ingenic,jz4780"))
>> +		return -ENODEV;
>> +
>> +	register_smp_ops(&xburst_smp_ops);
>> +	return 0;
>> +}
>> diff --git a/arch/mips/kernel/idle.c b/arch/mips/kernel/idle.c
>> index 80e8a04a642e..0b042a328ad7 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,27 @@ 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 xburst_wait_irqoff(void)
>> +{
>> +	unsigned long pending;
>> +
>> +	if (need_resched())
>> +		return;
>> +
>> +	r4k_blast_dcache();
>> +	pending = read_c0_cause() & read_c0_status() & CAUSEF_IP;
>> +	if (!need_resched() && !pending) {
> 
> You now call need_resched() twice, that's on purpose?

The two need_resched() checks are intentional: the first avoids an 
unnecessary cache flush; the second checks for work arriving during it.

> 
>> +		__asm__ __volatile__(
>> +			".set push\n"
>> +			".set arch=r4000\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 +178,16 @@ 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_MACH_JZ4780) &&
>> +		    IS_ENABLED(CONFIG_SMP) && num_possible_cpus() > 1)
>> +			cpu_wait = xburst_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) ||
>> diff --git a/arch/mips/kernel/smp.c b/arch/mips/kernel/smp.c
>> index 0f28b4a62e72..46c70d866723 100644
>> --- a/arch/mips/kernel/smp.c
>> +++ b/arch/mips/kernel/smp.c
>> @@ -338,7 +338,10 @@ int mips_smp_ipi_free(const struct cpumask *mask)
>>   
>>   static int __init mips_smp_ipi_init(void)
>>   {
>> -	if (num_possible_cpus() == 1)
>> +	/* Platforms with their own IPI callbacks do not need an IRQ domain.
>> */
>> +	if ((mp_ops->send_ipi_single != mips_smp_send_ipi_single &&
>> +	     mp_ops->send_ipi_mask != mips_smp_send_ipi_mask) ||
>> +	    num_possible_cpus() == 1)
> 
> Needs braces now because of the multi-line if().
> 
> This change wasn't in your v3, and I also feel like this would belong in a
> separate patch.

Yes, that's fair enough.
-- 
Florian