Introduce SMP support for CI20 (based on JZ4780) v6.

Introduce SMP support for CI20 (based on JZ4780) v6.

[周琰杰 (Zhou Yanjie)]

Introduce SMP support for MIPS Creator CI20, which is
based on Ingenic JZ4780 SoC.

[PATCH v6 0/7] Introduce SMP support for CI20 (based on JZ4780).

[周琰杰 (Zhou Yanjie)]

Introduce SMP support for MIPS Creator CI20, which is
based on Ingenic JZ4780 SoC.

周琰杰 (Zhou Yanjie) (7):
  clk: JZ4780: Add function for enable the second core.
  MIPS: JZ4780: Introduce SMP support.
  MIPS: CI20: Modify DTS to support high resolution timer for SMP.
  clocksource: Ingenic: Add high resolution timer support for SMP.
  dt-bindings: MIPS: Document Ingenic SoCs binding.
  MIPS: Ingenic: Add 'cpus' node for Ingenic SoCs.
  MIPS: CI20: Update defconfig to support SMP.

 .../bindings/mips/ingenic/ingenic,cpu.yaml         |  61 +++++
 .../bindings/mips/ingenic/ingenic,soc.yaml         |  34 +++
 arch/mips/boot/dts/ingenic/ci20.dts                |  13 +-
 arch/mips/boot/dts/ingenic/jz4740.dtsi             |  14 ++
 arch/mips/boot/dts/ingenic/jz4770.dtsi             |  15 +-
 arch/mips/boot/dts/ingenic/jz4780.dtsi             |  23 ++
 arch/mips/boot/dts/ingenic/x1000.dtsi              |  14 ++
 arch/mips/configs/ci20_defconfig                   |   2 +
 arch/mips/include/asm/mach-jz4740/smp.h            |  81 +++++++
 arch/mips/jz4740/Kconfig                           |   3 +
 arch/mips/jz4740/Makefile                          |   5 +
 arch/mips/jz4740/prom.c                            |   4 +
 arch/mips/jz4740/smp-entry.S                       |  57 +++++
 arch/mips/jz4740/smp.c                             | 254 +++++++++++++++++++++
 arch/mips/kernel/idle.c                            |  36 ++-
 drivers/clk/ingenic/jz4780-cgu.c                   |  55 ++++-
 drivers/clocksource/ingenic-timer.c                | 113 +++++++--
 17 files changed, 752 insertions(+), 32 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml
 create mode 100644 Documentation/devicetree/bindings/mips/ingenic/ingenic,soc.yaml
 create mode 100644 arch/mips/include/asm/mach-jz4740/smp.h
 create mode 100644 arch/mips/jz4740/smp-entry.S
 create mode 100644 arch/mips/jz4740/smp.c

-- 
2.7.4

[PATCH v6 1/7] clk: JZ4780: Add function for enable the second core.

[周琰杰 (Zhou Yanjie)]

Add "jz4780_core1_enable()" for enable the second core of JZ4780,
prepare for later commits.

Tested-by: H. Nikolaus Schaller <hns@goldelico.com>
Tested-by: Paul Boddie <paul@boddie.org.uk>
Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
Reviewed-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---

Notes:
    v5:
    New patch, split from [1/6] in v4.
    
    v5->v6:
    simplify code with "readl_poll_timeout()".

 drivers/clk/ingenic/jz4780-cgu.c | 55 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 50 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/ingenic/jz4780-cgu.c b/drivers/clk/ingenic/jz4780-cgu.c
index d07fff1..92102d8 100644
--- a/drivers/clk/ingenic/jz4780-cgu.c
+++ b/drivers/clk/ingenic/jz4780-cgu.c
@@ -9,14 +9,16 @@
 #include <linux/clk-provider.h>
 #include <linux/delay.h>
 #include <linux/io.h>
+#include <linux/iopoll.h>
 #include <linux/of.h>
+
 #include <dt-bindings/clock/jz4780-cgu.h>
 #include "cgu.h"
 #include "pm.h"
 
 /* CGU register offsets */
 #define CGU_REG_CLOCKCONTROL	0x00
-#define CGU_REG_PLLCONTROL	0x0c
+#define CGU_REG_LCR			0x04
 #define CGU_REG_APLL		0x10
 #define CGU_REG_MPLL		0x14
 #define CGU_REG_EPLL		0x18
@@ -46,8 +48,8 @@
 #define CGU_REG_CLOCKSTATUS	0xd4
 
 /* bits within the OPCR register */
-#define OPCR_SPENDN0		(1 << 7)
-#define OPCR_SPENDN1		(1 << 6)
+#define OPCR_SPENDN0		BIT(7)
+#define OPCR_SPENDN1		BIT(6)
 
 /* bits within the USBPCR register */
 #define USBPCR_USB_MODE		BIT(31)
@@ -88,6 +90,13 @@
 #define USBVBFIL_IDDIGFIL_MASK	(0xffff << USBVBFIL_IDDIGFIL_SHIFT)
 #define USBVBFIL_USBVBFIL_MASK	(0xffff)
 
+/* bits within the LCR register */
+#define LCR_PD_SCPU			BIT(31)
+#define LCR_SCPUS			BIT(27)
+
+/* bits within the CLKGR1 register */
+#define CLKGR1_CORE1		BIT(15)
+
 static struct ingenic_cgu *cgu;
 
 static u8 jz4780_otg_phy_get_parent(struct clk_hw *hw)
@@ -205,6 +214,42 @@ static const struct clk_ops jz4780_otg_phy_ops = {
 	.set_rate = jz4780_otg_phy_set_rate,
 };
 
+static int jz4780_core1_enable(struct clk_hw *hw)
+{
+	struct ingenic_clk *ingenic_clk = to_ingenic_clk(hw);
+	struct ingenic_cgu *cgu = ingenic_clk->cgu;
+	const unsigned int timeout = 5000;
+	unsigned long flags;
+	int retval;
+	u32 lcr, clkgr1;
+
+	spin_lock_irqsave(&cgu->lock, flags);
+
+	lcr = readl(cgu->base + CGU_REG_LCR);
+	lcr &= ~LCR_PD_SCPU;
+	writel(lcr, cgu->base + CGU_REG_LCR);
+
+	clkgr1 = readl(cgu->base + CGU_REG_CLKGR1);
+	clkgr1 &= ~CLKGR1_CORE1;
+	writel(clkgr1, cgu->base + CGU_REG_CLKGR1);
+
+	spin_unlock_irqrestore(&cgu->lock, flags);
+
+	/* wait for the CPU to be powered up */
+	retval = readl_poll_timeout(cgu->base + CGU_REG_LCR, lcr,
+				 !(lcr & LCR_SCPUS), 10, timeout);
+	if (retval == -ETIMEDOUT) {
+		pr_err("%s: Wait for power up core1 timeout\n", __func__);
+		return retval;
+	}
+
+	return 0;
+}
+
+static const struct clk_ops jz4780_core1_ops = {
+	.enable = jz4780_core1_enable,
+};
+
 static const s8 pll_od_encoding[16] = {
 	0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7,
 	0x8, 0x9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf,
@@ -701,9 +746,9 @@ static const struct ingenic_cgu_clk_info jz4780_cgu_clocks[] = {
 	},
 
 	[JZ4780_CLK_CORE1] = {
-		"core1", CGU_CLK_GATE,
+		"core1", CGU_CLK_CUSTOM,
 		.parents = { JZ4780_CLK_CPU, -1, -1, -1 },
-		.gate = { CGU_REG_CLKGR1, 15 },
+		.custom = { &jz4780_core1_ops },
 	},
 
 };
-- 
2.7.4

Re: [PATCH v6 1/7] clk: JZ4780: Add function for enable the second core.

[Stephen Boyd]

Quoting Zhou Yanjie(2020-02-20 08:24:43)
> Add "jz4780_core1_enable()" for enable the second core of JZ4780,
> prepare for later commits.

Applied to clk-next

[PATCH v6 2/7] MIPS: JZ4780: Introduce SMP support.

[周琰杰 (Zhou Yanjie)]

Forward port smp support from kernel 3.18.3 of CI20_linux
to upstream kernel 5.6.

Tested-by: H. Nikolaus Schaller <hns@goldelico.com>
Tested-by: Paul Boddie <paul@boddie.org.uk>
Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
Reviewed-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---

Notes:
    v1->v2:
    1.Remove unnecessary "plat_irq_dispatch(void)" in irq-ingenic.c.
    2.Add a timeout check for "jz4780_boot_secondary()" to avoid a dead loop.
    3.Replace hard code in smp.c with macro.
    
    v2->v3:
    1.Remove unnecessary "extern void (*r4k_blast_dcache)(void)" in smp.c.
    2.Use "for_each_of_cpu_node" instead "for_each_compatible_node" in smp.c.
    3.Use "of_cpu_node_to_id" instead "of_property_read_u32_index" in smp.c.
    4.Move LCR related operations to jz4780-cgu.c.
    
    v3->v4:
    Rebase on top of kernel 5.6-rc1.
    
    v4->v5:
    1.Splitting changes involving "jz4780-cgu.c" into separate commit.
    2.Use "request_irq()" replace "setup_irq()".
    
    v5->v6:
    In order to have a kernel that works on multiple SoCs at the same
    time, use "IS_ENABLED()" replace "#ifdef".

 arch/mips/include/asm/mach-jz4740/smp.h |  81 ++++++++++
 arch/mips/jz4740/Kconfig                |   3 +
 arch/mips/jz4740/Makefile               |   5 +
 arch/mips/jz4740/prom.c                 |   4 +
 arch/mips/jz4740/smp-entry.S            |  57 +++++++
 arch/mips/jz4740/smp.c                  | 254 ++++++++++++++++++++++++++++++++
 arch/mips/kernel/idle.c                 |  36 ++++-
 7 files changed, 439 insertions(+), 1 deletion(-)
 create mode 100644 arch/mips/include/asm/mach-jz4740/smp.h
 create mode 100644 arch/mips/jz4740/smp-entry.S
 create mode 100644 arch/mips/jz4740/smp.c

diff --git a/arch/mips/include/asm/mach-jz4740/smp.h b/arch/mips/include/asm/mach-jz4740/smp.h
new file mode 100644
index 00000000..af3c21b
--- /dev/null
+++ b/arch/mips/include/asm/mach-jz4740/smp.h
@@ -0,0 +1,81 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ *  Copyright (C) 2013, Paul Burton <paul.burton@imgtec.com>
+ *  JZ4780 SMP definitions
+ */
+
+#ifndef __MIPS_ASM_MACH_JZ4740_JZ4780_SMP_H__
+#define __MIPS_ASM_MACH_JZ4740_JZ4780_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 smp_clr_pending(mask) do {		\
+		unsigned int stat;		\
+		stat = read_c0_corestatus();	\
+		stat &= ~((mask) & 0xff);	\
+		write_c0_corestatus(stat);	\
+	} while (0)
+
+/*
+ * Core Control register
+ */
+#define CORECTRL_SLEEP1M_SHIFT	17
+#define CORECTRL_SLEEP1M	(_ULCAST_(0x1) << CORECTRL_SLEEP1M_SHIFT)
+#define CORECTRL_SLEEP0M_SHIFT	16
+#define CORECTRL_SLEEP0M	(_ULCAST_(0x1) << CORECTRL_SLEEP0M_SHIFT)
+#define CORECTRL_RPC1_SHIFT	9
+#define CORECTRL_RPC1		(_ULCAST_(0x1) << CORECTRL_RPC1_SHIFT)
+#define CORECTRL_RPC0_SHIFT	8
+#define CORECTRL_RPC0		(_ULCAST_(0x1) << CORECTRL_RPC0_SHIFT)
+#define CORECTRL_SWRST1_SHIFT	1
+#define CORECTRL_SWRST1		(_ULCAST_(0x1) << CORECTRL_SWRST1_SHIFT)
+#define CORECTRL_SWRST0_SHIFT	0
+#define CORECTRL_SWRST0		(_ULCAST_(0x1) << CORECTRL_SWRST0_SHIFT)
+
+/*
+ * Core Status register
+ */
+#define CORESTATUS_SLEEP1_SHIFT	17
+#define CORESTATUS_SLEEP1	(_ULCAST_(0x1) << CORESTATUS_SLEEP1_SHIFT)
+#define CORESTATUS_SLEEP0_SHIFT	16
+#define CORESTATUS_SLEEP0	(_ULCAST_(0x1) << CORESTATUS_SLEEP0_SHIFT)
+#define CORESTATUS_IRQ1P_SHIFT	9
+#define CORESTATUS_IRQ1P	(_ULCAST_(0x1) << CORESTATUS_IRQ1P_SHIFT)
+#define CORESTATUS_IRQ0P_SHIFT	8
+#define CORESTATUS_IRQ0P	(_ULCAST_(0x1) << CORESTATUS_IRQ8P_SHIFT)
+#define CORESTATUS_MIRQ1P_SHIFT	1
+#define CORESTATUS_MIRQ1P	(_ULCAST_(0x1) << CORESTATUS_MIRQ1P_SHIFT)
+#define CORESTATUS_MIRQ0P_SHIFT	0
+#define CORESTATUS_MIRQ0P	(_ULCAST_(0x1) << CORESTATUS_MIRQ0P_SHIFT)
+
+/*
+ * Reset Entry & IRQ Mask register
+ */
+#define REIM_ENTRY_SHIFT	16
+#define REIM_ENTRY		(_ULCAST_(0xffff) << REIM_ENTRY_SHIFT)
+#define REIM_IRQ1M_SHIFT	9
+#define REIM_IRQ1M		(_ULCAST_(0x1) << REIM_IRQ1M_SHIFT)
+#define REIM_IRQ0M_SHIFT	8
+#define REIM_IRQ0M		(_ULCAST_(0x1) << REIM_IRQ0M_SHIFT)
+#define REIM_MBOXIRQ1M_SHIFT	1
+#define REIM_MBOXIRQ1M		(_ULCAST_(0x1) << REIM_MBOXIRQ1M_SHIFT)
+#define REIM_MBOXIRQ0M_SHIFT	0
+#define REIM_MBOXIRQ0M		(_ULCAST_(0x1) << REIM_MBOXIRQ0M_SHIFT)
+
+extern void jz4780_smp_init(void);
+extern void jz4780_secondary_cpu_entry(void);
+
+#endif /* __MIPS_ASM_MACH_JZ4740_JZ4780_SMP_H__ */
diff --git a/arch/mips/jz4740/Kconfig b/arch/mips/jz4740/Kconfig
index 412d2fa..0239597 100644
--- a/arch/mips/jz4740/Kconfig
+++ b/arch/mips/jz4740/Kconfig
@@ -34,9 +34,12 @@ config MACH_JZ4770
 
 config MACH_JZ4780
 	bool
+	select GENERIC_CLOCKEVENTS_BROADCAST if SMP
 	select MIPS_CPU_SCACHE
+	select NR_CPUS_DEFAULT_2
 	select SYS_HAS_CPU_MIPS32_R2
 	select SYS_SUPPORTS_HIGHMEM
+	select SYS_SUPPORTS_SMP
 
 config MACH_X1000
 	bool
diff --git a/arch/mips/jz4740/Makefile b/arch/mips/jz4740/Makefile
index 6de14c0..0a0f024 100644
--- a/arch/mips/jz4740/Makefile
+++ b/arch/mips/jz4740/Makefile
@@ -12,3 +12,8 @@ CFLAGS_setup.o = -I$(src)/../../../scripts/dtc/libfdt
 # PM support
 
 obj-$(CONFIG_PM) += pm.o
+
+# SMP support
+
+obj-$(CONFIG_SMP) += smp.o
+obj-$(CONFIG_SMP) += smp-entry.o
diff --git a/arch/mips/jz4740/prom.c b/arch/mips/jz4740/prom.c
index ff4555c..026259e 100644
--- a/arch/mips/jz4740/prom.c
+++ b/arch/mips/jz4740/prom.c
@@ -8,10 +8,14 @@
 
 #include <asm/bootinfo.h>
 #include <asm/fw/fw.h>
+#include <asm/mach-jz4740/smp.h>
 
 void __init prom_init(void)
 {
 	fw_init_cmdline();
+
+	if (IS_ENABLED(CONFIG_MACH_JZ4780) && IS_ENABLED(CONFIG_SMP))
+		jz4780_smp_init();
 }
 
 void __init prom_free_prom_memory(void)
diff --git a/arch/mips/jz4740/smp-entry.S b/arch/mips/jz4740/smp-entry.S
new file mode 100644
index 00000000..20049a3
--- /dev/null
+++ b/arch/mips/jz4740/smp-entry.S
@@ -0,0 +1,57 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ *  Copyright (C) 2013, Paul Burton <paul.burton@imgtec.com>
+ *  JZ4780 SMP entry point
+ */
+
+#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
+
+.extern jz4780_cpu_entry_sp
+.extern jz4780_cpu_entry_gp
+
+.section .text.smp-entry
+.balign 0x10000
+.set noreorder
+LEAF(jz4780_secondary_cpu_entry)
+	mtc0	zero, CP0_CAUSE
+
+	li	t0, ST0_CU0
+	mtc0	t0, CP0_STATUS
+
+	/* cache setup */
+	li	t0, KSEG0
+	ori	t1, t0, CACHE_SIZE
+	mtc0	zero, CP0_TAGLO, 0
+1:	cache	Index_Store_Tag_I, 0(t0)
+	cache	Index_Store_Tag_D, 0(t0)
+	bne	t0, t1, 1b
+	 addiu	t0, t0, CACHE_LINESIZE
+
+	/* kseg0 cache attribute */
+	mfc0	t0, CP0_CONFIG, 0
+	ori	t0, t0, CONF_CM_CACHABLE_NONCOHERENT
+	mtc0	t0, CP0_CONFIG, 0
+
+	/* pagemask */
+	mtc0	zero, CP0_PAGEMASK, 0
+
+	/* retrieve sp */
+	la	t0, jz4780_cpu_entry_sp
+	lw	sp, 0(t0)
+
+	/* retrieve gp */
+	la	t0, jz4780_cpu_entry_gp
+	lw	gp, 0(t0)
+
+	/* jump to the kernel in kseg0 */
+	la	t0, smp_bootstrap
+	jr	t0
+	 nop
+	END(jz4780_secondary_cpu_entry)
diff --git a/arch/mips/jz4740/smp.c b/arch/mips/jz4740/smp.c
new file mode 100644
index 00000000..5fe0cf3
--- /dev/null
+++ b/arch/mips/jz4740/smp.c
@@ -0,0 +1,254 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ *  Copyright (C) 2013, Paul Burton <paul.burton@imgtec.com>
+ *  JZ4780 SMP
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/of.h>
+#include <linux/sched.h>
+#include <linux/sched/task_stack.h>
+#include <linux/smp.h>
+#include <linux/tick.h>
+#include <asm/mach-jz4740/smp.h>
+#include <asm/smp-ops.h>
+
+static struct clk *cpu_clock_gates[CONFIG_NR_CPUS] = { NULL };
+
+u32 jz4780_cpu_entry_sp;
+u32 jz4780_cpu_entry_gp;
+
+static struct cpumask cpu_running;
+
+static DEFINE_SPINLOCK(smp_lock);
+
+static irqreturn_t mbox_handler(int irq, void *dev_id)
+{
+	int cpu = smp_processor_id();
+	u32 action, status;
+
+	spin_lock(&smp_lock);
+
+	switch (cpu) {
+	case 0:
+		action = read_c0_mailbox0();
+		write_c0_mailbox0(0);
+		break;
+	case 1:
+		action = read_c0_mailbox1();
+		write_c0_mailbox1(0);
+		break;
+	default:
+		panic("unhandled cpu %d!", cpu);
+	}
+
+	/* clear pending mailbox interrupt */
+	status = read_c0_corestatus();
+	status &= ~(CORESTATUS_MIRQ0P << cpu);
+	write_c0_corestatus(status);
+
+	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 jz4780_smp_setup(void)
+{
+	u32 addr, reim;
+	int cpu;
+
+	reim = read_c0_reim();
+
+	for (cpu = 0; cpu < NR_CPUS; cpu++) {
+		__cpu_number_map[cpu] = cpu;
+		__cpu_logical_map[cpu] = cpu;
+		set_cpu_possible(cpu, true);
+	}
+
+	/* mask mailbox interrupts for this core */
+	reim &= ~REIM_MBOXIRQ0M;
+	write_c0_reim(reim);
+
+	/* clear mailboxes & pending mailbox IRQs */
+	write_c0_mailbox0(0);
+	write_c0_mailbox1(0);
+	write_c0_corestatus(0);
+
+	/* set reset entry point */
+	addr = KSEG1ADDR((u32)&jz4780_secondary_cpu_entry);
+	WARN_ON(addr & ~REIM_ENTRY);
+	reim &= ~REIM_ENTRY;
+	reim |= addr & REIM_ENTRY;
+
+	/* unmask mailbox interrupts for this core */
+	reim |= REIM_MBOXIRQ0M;
+	write_c0_reim(reim);
+	set_c0_status(STATUSF_IP3);
+	irq_enable_hazard();
+
+	cpumask_set_cpu(cpu, &cpu_running);
+}
+
+static void jz4780_smp_prepare_cpus(unsigned int max_cpus)
+{
+	struct device_node *cpu_node;
+	unsigned cpu, ctrl;
+	int err;
+
+	/* setup the mailbox IRQ */
+	err = request_irq(MIPS_CPU_IRQ_BASE + 3, mbox_handler,
+			IRQF_PERCPU | IRQF_NO_THREAD, "core mailbox", NULL);
+	if (err)
+		pr_err("request_irq() on core mailbox failed\n");
+
+	init_cpu_present(cpu_possible_mask);
+
+	ctrl = read_c0_corectrl();
+
+	for (cpu = 0; cpu < max_cpus; cpu++) {
+		/* use reset entry point from REIM register */
+		ctrl |= CORECTRL_RPC0 << cpu;
+	}
+
+	for_each_of_cpu_node(cpu_node) {
+		cpu = of_cpu_node_to_id(cpu_node);
+		if (cpu < 0) {
+			pr_err("Failed to read index of %s\n",
+			       cpu_node->full_name);
+			continue;
+		}
+
+		cpu_clock_gates[cpu] = of_clk_get(cpu_node, 0);
+		if (IS_ERR(cpu_clock_gates[cpu])) {
+			cpu_clock_gates[cpu] = NULL;
+			continue;
+		}
+
+		err = clk_prepare(cpu_clock_gates[cpu]);
+		if (err)
+			pr_err("Failed to prepare CPU clock gate\n");
+	}
+
+	write_c0_corectrl(ctrl);
+}
+
+static int jz4780_boot_secondary(int cpu, struct task_struct *idle)
+{
+	unsigned long flags;
+	u32 ctrl;
+
+	spin_lock_irqsave(&smp_lock, flags);
+
+	/* ensure the core is in reset */
+	ctrl = read_c0_corectrl();
+	ctrl |= CORECTRL_SWRST0 << cpu;
+	write_c0_corectrl(ctrl);
+
+	/* ungate core clock */
+	if (cpu_clock_gates[cpu])
+		clk_enable(cpu_clock_gates[cpu]);
+
+	/* set entry sp/gp register values */
+	jz4780_cpu_entry_sp = __KSTK_TOS(idle);
+	jz4780_cpu_entry_gp = (u32)task_thread_info(idle);
+	smp_wmb();
+
+	/* take the core out of reset */
+	ctrl &= ~(CORECTRL_SWRST0 << cpu);
+	write_c0_corectrl(ctrl);
+
+	cpumask_set_cpu(cpu, &cpu_running);
+
+	spin_unlock_irqrestore(&smp_lock, flags);
+
+	return 0;
+}
+
+static void jz4780_init_secondary(void)
+{
+}
+
+static void jz4780_smp_finish(void)
+{
+	u32 reim;
+
+	spin_lock(&smp_lock);
+
+	/* unmask mailbox interrupts for this core */
+	reim = read_c0_reim();
+	reim |= REIM_MBOXIRQ0M << smp_processor_id();
+	write_c0_reim(reim);
+
+	spin_unlock(&smp_lock);
+
+	/* unmask interrupts for this core */
+	change_c0_status(ST0_IM, STATUSF_IP3 | STATUSF_IP2 |
+			 STATUSF_IP1 | STATUSF_IP0);
+	irq_enable_hazard();
+
+	/* force broadcast timer */
+	tick_broadcast_force();
+}
+
+static void jz4780_send_ipi_single_locked(int cpu, unsigned int action)
+{
+	u32 mbox;
+
+	switch (cpu) {
+	case 0:
+		mbox = read_c0_mailbox0();
+		write_c0_mailbox0(mbox | action);
+		break;
+	case 1:
+		mbox = read_c0_mailbox1();
+		write_c0_mailbox1(mbox | action);
+		break;
+	default:
+		panic("unhandled cpu %d!", cpu);
+	}
+}
+
+static void jz4780_send_ipi_single(int cpu, unsigned int action)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&smp_lock, flags);
+	jz4780_send_ipi_single_locked(cpu, action);
+	spin_unlock_irqrestore(&smp_lock, flags);
+}
+
+static void jz4780_send_ipi_mask(const struct cpumask *mask,
+				 unsigned int action)
+{
+	unsigned long flags;
+	int cpu;
+
+	spin_lock_irqsave(&smp_lock, flags);
+
+	for_each_cpu(cpu, mask)
+		jz4780_send_ipi_single_locked(cpu, action);
+
+	spin_unlock_irqrestore(&smp_lock, flags);
+}
+
+static 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_smp_prepare_cpus,
+};
+
+void jz4780_smp_init(void)
+{
+	register_smp_ops(&jz4780_smp_ops);
+}
diff --git a/arch/mips/kernel/idle.c b/arch/mips/kernel/idle.c
index 37f8e78..a3afd98 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,
@@ -88,6 +89,34 @@ static void __cpuidle rm7k_wait_irqoff(void)
 }
 
 /*
+ * The Ingenic jz4780 SMP variant has to write back dirty cache lines before
+ * executing wait. The CPU & cache clock will be gated until we return from
+ * the wait, and if another core attempts to access data from our data cache
+ * during this time then it will lock up.
+ */
+void jz4780_smp_wait_irqoff(void)
+{
+	unsigned long pending = read_c0_cause() & read_c0_status() & CAUSEF_IP;
+
+	/*
+	 * Going to idle has a significant overhead due to the cache flush so
+	 * try to avoid it if we'll immediately be woken again due to an IRQ.
+	 */
+	if (!need_resched() && !pending) {
+		r4k_blast_dcache();
+
+		__asm__(
+		"	.set push	\n"
+		"	.set mips3	\n"
+		"	sync		\n"
+		"	wait		\n"
+		"	.set pop	\n");
+	}
+
+	local_irq_enable();
+}
+
+/*
  * 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
  * interrupt can wake it, so they must be enabled before entering idle modes.
@@ -172,7 +201,6 @@ void __init check_wait(void)
 	case CPU_CAVIUM_OCTEON_PLUS:
 	case CPU_CAVIUM_OCTEON2:
 	case CPU_CAVIUM_OCTEON3:
-	case CPU_XBURST:
 	case CPU_LOONGSON32:
 	case CPU_XLR:
 	case CPU_XLP:
@@ -246,6 +274,12 @@ void __init check_wait(void)
 		   cpu_wait = r4k_wait;
 		 */
 		break;
+	case CPU_XBURST:
+		if (IS_ENABLED(CONFIG_MACH_JZ4780) &&
+				IS_ENABLED(CONFIG_SMP) && (NR_CPUS > 1))
+			cpu_wait = jz4780_smp_wait_irqoff;
+		else
+			cpu_wait = r4k_wait;
 	default:
 		break;
 	}
-- 
2.7.4

Re: [PATCH v6 2/7] MIPS: JZ4780: Introduce SMP support.

[Paul Cercueil]

Hi Zhou,


Le ven., févr. 21, 2020 at 00:24, 周琰杰 (Zhou Yanjie) 
<zhouyanjie@wanyeetech.com> a écrit :
> Forward port smp support from kernel 3.18.3 of CI20_linux
> to upstream kernel 5.6.
> 
> Tested-by: H. Nikolaus Schaller <hns@goldelico.com>
> Tested-by: Paul Boddie <paul@boddie.org.uk>
> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> Reviewed-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
> 
> Notes:
>     v1->v2:
>     1.Remove unnecessary "plat_irq_dispatch(void)" in irq-ingenic.c.
>     2.Add a timeout check for "jz4780_boot_secondary()" to avoid a 
> dead loop.
>     3.Replace hard code in smp.c with macro.
> 
>     v2->v3:
>     1.Remove unnecessary "extern void (*r4k_blast_dcache)(void)" in 
> smp.c.
>     2.Use "for_each_of_cpu_node" instead "for_each_compatible_node" 
> in smp.c.
>     3.Use "of_cpu_node_to_id" instead "of_property_read_u32_index" in 
> smp.c.
>     4.Move LCR related operations to jz4780-cgu.c.
> 
>     v3->v4:
>     Rebase on top of kernel 5.6-rc1.
> 
>     v4->v5:
>     1.Splitting changes involving "jz4780-cgu.c" into separate commit.
>     2.Use "request_irq()" replace "setup_irq()".
> 
>     v5->v6:
>     In order to have a kernel that works on multiple SoCs at the same
>     time, use "IS_ENABLED()" replace "#ifdef".
> 
>  arch/mips/include/asm/mach-jz4740/smp.h |  81 ++++++++++
>  arch/mips/jz4740/Kconfig                |   3 +
>  arch/mips/jz4740/Makefile               |   5 +
>  arch/mips/jz4740/prom.c                 |   4 +
>  arch/mips/jz4740/smp-entry.S            |  57 +++++++
>  arch/mips/jz4740/smp.c                  | 254 
> ++++++++++++++++++++++++++++++++
>  arch/mips/kernel/idle.c                 |  36 ++++-
>  7 files changed, 439 insertions(+), 1 deletion(-)
>  create mode 100644 arch/mips/include/asm/mach-jz4740/smp.h
>  create mode 100644 arch/mips/jz4740/smp-entry.S
>  create mode 100644 arch/mips/jz4740/smp.c
> 
> diff --git a/arch/mips/include/asm/mach-jz4740/smp.h 
> b/arch/mips/include/asm/mach-jz4740/smp.h
> new file mode 100644
> index 00000000..af3c21b
> --- /dev/null
> +++ b/arch/mips/include/asm/mach-jz4740/smp.h
> @@ -0,0 +1,81 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + *  Copyright (C) 2013, Paul Burton <paul.burton@imgtec.com>
> + *  JZ4780 SMP definitions
> + */
> +
> +#ifndef __MIPS_ASM_MACH_JZ4740_JZ4780_SMP_H__
> +#define __MIPS_ASM_MACH_JZ4740_JZ4780_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 smp_clr_pending(mask) do {		\
> +		unsigned int stat;		\
> +		stat = read_c0_corestatus();	\
> +		stat &= ~((mask) & 0xff);	\
> +		write_c0_corestatus(stat);	\
> +	} while (0)
> +
> +/*
> + * Core Control register
> + */
> +#define CORECTRL_SLEEP1M_SHIFT	17
> +#define CORECTRL_SLEEP1M	(_ULCAST_(0x1) << CORECTRL_SLEEP1M_SHIFT)
> +#define CORECTRL_SLEEP0M_SHIFT	16
> +#define CORECTRL_SLEEP0M	(_ULCAST_(0x1) << CORECTRL_SLEEP0M_SHIFT)
> +#define CORECTRL_RPC1_SHIFT	9
> +#define CORECTRL_RPC1		(_ULCAST_(0x1) << CORECTRL_RPC1_SHIFT)
> +#define CORECTRL_RPC0_SHIFT	8
> +#define CORECTRL_RPC0		(_ULCAST_(0x1) << CORECTRL_RPC0_SHIFT)
> +#define CORECTRL_SWRST1_SHIFT	1
> +#define CORECTRL_SWRST1		(_ULCAST_(0x1) << CORECTRL_SWRST1_SHIFT)
> +#define CORECTRL_SWRST0_SHIFT	0
> +#define CORECTRL_SWRST0		(_ULCAST_(0x1) << CORECTRL_SWRST0_SHIFT)
> +
> +/*
> + * Core Status register
> + */
> +#define CORESTATUS_SLEEP1_SHIFT	17
> +#define CORESTATUS_SLEEP1	(_ULCAST_(0x1) << CORESTATUS_SLEEP1_SHIFT)
> +#define CORESTATUS_SLEEP0_SHIFT	16
> +#define CORESTATUS_SLEEP0	(_ULCAST_(0x1) << CORESTATUS_SLEEP0_SHIFT)
> +#define CORESTATUS_IRQ1P_SHIFT	9
> +#define CORESTATUS_IRQ1P	(_ULCAST_(0x1) << CORESTATUS_IRQ1P_SHIFT)
> +#define CORESTATUS_IRQ0P_SHIFT	8
> +#define CORESTATUS_IRQ0P	(_ULCAST_(0x1) << CORESTATUS_IRQ8P_SHIFT)
> +#define CORESTATUS_MIRQ1P_SHIFT	1
> +#define CORESTATUS_MIRQ1P	(_ULCAST_(0x1) << CORESTATUS_MIRQ1P_SHIFT)
> +#define CORESTATUS_MIRQ0P_SHIFT	0
> +#define CORESTATUS_MIRQ0P	(_ULCAST_(0x1) << CORESTATUS_MIRQ0P_SHIFT)
> +
> +/*
> + * Reset Entry & IRQ Mask register
> + */
> +#define REIM_ENTRY_SHIFT	16
> +#define REIM_ENTRY		(_ULCAST_(0xffff) << REIM_ENTRY_SHIFT)
> +#define REIM_IRQ1M_SHIFT	9
> +#define REIM_IRQ1M		(_ULCAST_(0x1) << REIM_IRQ1M_SHIFT)
> +#define REIM_IRQ0M_SHIFT	8
> +#define REIM_IRQ0M		(_ULCAST_(0x1) << REIM_IRQ0M_SHIFT)
> +#define REIM_MBOXIRQ1M_SHIFT	1
> +#define REIM_MBOXIRQ1M		(_ULCAST_(0x1) << REIM_MBOXIRQ1M_SHIFT)
> +#define REIM_MBOXIRQ0M_SHIFT	0
> +#define REIM_MBOXIRQ0M		(_ULCAST_(0x1) << REIM_MBOXIRQ0M_SHIFT)
> +
> +extern void jz4780_smp_init(void);
> +extern void jz4780_secondary_cpu_entry(void);
> +
> +#endif /* __MIPS_ASM_MACH_JZ4740_JZ4780_SMP_H__ */
> diff --git a/arch/mips/jz4740/Kconfig b/arch/mips/jz4740/Kconfig
> index 412d2fa..0239597 100644
> --- a/arch/mips/jz4740/Kconfig
> +++ b/arch/mips/jz4740/Kconfig
> @@ -34,9 +34,12 @@ config MACH_JZ4770
> 
>  config MACH_JZ4780
>  	bool
> +	select GENERIC_CLOCKEVENTS_BROADCAST if SMP
>  	select MIPS_CPU_SCACHE
> +	select NR_CPUS_DEFAULT_2
>  	select SYS_HAS_CPU_MIPS32_R2
>  	select SYS_SUPPORTS_HIGHMEM
> +	select SYS_SUPPORTS_SMP
> 
>  config MACH_X1000
>  	bool
> diff --git a/arch/mips/jz4740/Makefile b/arch/mips/jz4740/Makefile
> index 6de14c0..0a0f024 100644
> --- a/arch/mips/jz4740/Makefile
> +++ b/arch/mips/jz4740/Makefile
> @@ -12,3 +12,8 @@ CFLAGS_setup.o = 
> -I$(src)/../../../scripts/dtc/libfdt
>  # PM support
> 
>  obj-$(CONFIG_PM) += pm.o
> +
> +# SMP support
> +
> +obj-$(CONFIG_SMP) += smp.o
> +obj-$(CONFIG_SMP) += smp-entry.o
> diff --git a/arch/mips/jz4740/prom.c b/arch/mips/jz4740/prom.c
> index ff4555c..026259e 100644
> --- a/arch/mips/jz4740/prom.c
> +++ b/arch/mips/jz4740/prom.c
> @@ -8,10 +8,14 @@
> 
>  #include <asm/bootinfo.h>
>  #include <asm/fw/fw.h>
> +#include <asm/mach-jz4740/smp.h>
> 
>  void __init prom_init(void)
>  {
>  	fw_init_cmdline();
> +
> +	if (IS_ENABLED(CONFIG_MACH_JZ4780) && IS_ENABLED(CONFIG_SMP))
> +		jz4780_smp_init();

I guess it's OK for a first version, but SMP should really be decoupled 
from the SoC version. A SMP-enabled kernel should be able to also 
support non-SMP SoCs.


>  }
> 
>  void __init prom_free_prom_memory(void)
> diff --git a/arch/mips/jz4740/smp-entry.S 
> b/arch/mips/jz4740/smp-entry.S
> new file mode 100644
> index 00000000..20049a3
> --- /dev/null
> +++ b/arch/mips/jz4740/smp-entry.S
> @@ -0,0 +1,57 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + *  Copyright (C) 2013, Paul Burton <paul.burton@imgtec.com>
> + *  JZ4780 SMP entry point
> + */
> +
> +#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
> +
> +.extern jz4780_cpu_entry_sp
> +.extern jz4780_cpu_entry_gp
> +
> +.section .text.smp-entry
> +.balign 0x10000
> +.set noreorder
> +LEAF(jz4780_secondary_cpu_entry)
> +	mtc0	zero, CP0_CAUSE
> +
> +	li	t0, ST0_CU0
> +	mtc0	t0, CP0_STATUS
> +
> +	/* cache setup */
> +	li	t0, KSEG0
> +	ori	t1, t0, CACHE_SIZE
> +	mtc0	zero, CP0_TAGLO, 0
> +1:	cache	Index_Store_Tag_I, 0(t0)
> +	cache	Index_Store_Tag_D, 0(t0)
> +	bne	t0, t1, 1b
> +	 addiu	t0, t0, CACHE_LINESIZE
> +
> +	/* kseg0 cache attribute */
> +	mfc0	t0, CP0_CONFIG, 0
> +	ori	t0, t0, CONF_CM_CACHABLE_NONCOHERENT
> +	mtc0	t0, CP0_CONFIG, 0
> +
> +	/* pagemask */
> +	mtc0	zero, CP0_PAGEMASK, 0
> +
> +	/* retrieve sp */
> +	la	t0, jz4780_cpu_entry_sp
> +	lw	sp, 0(t0)
> +
> +	/* retrieve gp */
> +	la	t0, jz4780_cpu_entry_gp
> +	lw	gp, 0(t0)
> +
> +	/* jump to the kernel in kseg0 */
> +	la	t0, smp_bootstrap
> +	jr	t0
> +	 nop
> +	END(jz4780_secondary_cpu_entry)
> diff --git a/arch/mips/jz4740/smp.c b/arch/mips/jz4740/smp.c
> new file mode 100644
> index 00000000..5fe0cf3
> --- /dev/null
> +++ b/arch/mips/jz4740/smp.c
> @@ -0,0 +1,254 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + *  Copyright (C) 2013, Paul Burton <paul.burton@imgtec.com>
> + *  JZ4780 SMP
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/of.h>
> +#include <linux/sched.h>
> +#include <linux/sched/task_stack.h>
> +#include <linux/smp.h>
> +#include <linux/tick.h>
> +#include <asm/mach-jz4740/smp.h>
> +#include <asm/smp-ops.h>
> +
> +static struct clk *cpu_clock_gates[CONFIG_NR_CPUS] = { NULL };
> +
> +u32 jz4780_cpu_entry_sp;
> +u32 jz4780_cpu_entry_gp;
> +
> +static struct cpumask cpu_running;

This is not used anywhere, is it?

> +
> +static DEFINE_SPINLOCK(smp_lock);
> +
> +static irqreturn_t mbox_handler(int irq, void *dev_id)
> +{
> +	int cpu = smp_processor_id();
> +	u32 action, status;
> +
> +	spin_lock(&smp_lock);
> +
> +	switch (cpu) {
> +	case 0:
> +		action = read_c0_mailbox0();
> +		write_c0_mailbox0(0);
> +		break;
> +	case 1:
> +		action = read_c0_mailbox1();
> +		write_c0_mailbox1(0);
> +		break;
> +	default:
> +		panic("unhandled cpu %d!", cpu);

XBurst has 4 mailboxes, maybe add them here?

> +	}
> +
> +	/* clear pending mailbox interrupt */
> +	status = read_c0_corestatus();
> +	status &= ~(CORESTATUS_MIRQ0P << cpu);
> +	write_c0_corestatus(status);
> +
> +	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 jz4780_smp_setup(void)
> +{
> +	u32 addr, reim;
> +	int cpu;
> +
> +	reim = read_c0_reim();
> +
> +	for (cpu = 0; cpu < NR_CPUS; cpu++) {
> +		__cpu_number_map[cpu] = cpu;
> +		__cpu_logical_map[cpu] = cpu;
> +		set_cpu_possible(cpu, true);
> +	}
> +
> +	/* mask mailbox interrupts for this core */
> +	reim &= ~REIM_MBOXIRQ0M;
> +	write_c0_reim(reim);
> +
> +	/* clear mailboxes & pending mailbox IRQs */
> +	write_c0_mailbox0(0);
> +	write_c0_mailbox1(0);
> +	write_c0_corestatus(0);
> +
> +	/* set reset entry point */
> +	addr = KSEG1ADDR((u32)&jz4780_secondary_cpu_entry);
> +	WARN_ON(addr & ~REIM_ENTRY);
> +	reim &= ~REIM_ENTRY;
> +	reim |= addr & REIM_ENTRY;
> +
> +	/* unmask mailbox interrupts for this core */
> +	reim |= REIM_MBOXIRQ0M;
> +	write_c0_reim(reim);
> +	set_c0_status(STATUSF_IP3);
> +	irq_enable_hazard();
> +
> +	cpumask_set_cpu(cpu, &cpu_running);
> +}
> +
> +static void jz4780_smp_prepare_cpus(unsigned int max_cpus)
> +{
> +	struct device_node *cpu_node;
> +	unsigned cpu, ctrl;
> +	int err;
> +
> +	/* setup the mailbox IRQ */
> +	err = request_irq(MIPS_CPU_IRQ_BASE + 3, mbox_handler,
> +			IRQF_PERCPU | IRQF_NO_THREAD, "core mailbox", NULL);
> +	if (err)
> +		pr_err("request_irq() on core mailbox failed\n");
> +
> +	init_cpu_present(cpu_possible_mask);

jz4780_smp_setup() marks CPUs 0..NR_CPUS as possible, NR_CPUS being a 
config option that can be anywhere between 2 and 256. This basically 
says that up to 256 CPUs are actually present.

> +
> +	ctrl = read_c0_corectrl();
> +
> +	for (cpu = 0; cpu < max_cpus; cpu++) {
> +		/* use reset entry point from REIM register */
> +		ctrl |= CORECTRL_RPC0 << cpu;

max_cpus is just NR_CPUS, so this does not work. You should move this 
in the "for_each_of_cpu_node" below.

> +	}
> +
> +	for_each_of_cpu_node(cpu_node) {
> +		cpu = of_cpu_node_to_id(cpu_node);
> +		if (cpu < 0) {
> +			pr_err("Failed to read index of %s\n",
> +			       cpu_node->full_name);
> +			continue;
> +		}
> +
> +		cpu_clock_gates[cpu] = of_clk_get(cpu_node, 0);
> +		if (IS_ERR(cpu_clock_gates[cpu])) {
> +			cpu_clock_gates[cpu] = NULL;
> +			continue;
> +		}
> +
> +		err = clk_prepare(cpu_clock_gates[cpu]);
> +		if (err)
> +			pr_err("Failed to prepare CPU clock gate\n");
> +	}
> +
> +	write_c0_corectrl(ctrl);
> +}
> +
> +static int jz4780_boot_secondary(int cpu, struct task_struct *idle)
> +{
> +	unsigned long flags;
> +	u32 ctrl;
> +
> +	spin_lock_irqsave(&smp_lock, flags);
> +
> +	/* ensure the core is in reset */
> +	ctrl = read_c0_corectrl();
> +	ctrl |= CORECTRL_SWRST0 << cpu;
> +	write_c0_corectrl(ctrl);
> +
> +	/* ungate core clock */
> +	if (cpu_clock_gates[cpu])
> +		clk_enable(cpu_clock_gates[cpu]);
> +
> +	/* set entry sp/gp register values */
> +	jz4780_cpu_entry_sp = __KSTK_TOS(idle);
> +	jz4780_cpu_entry_gp = (u32)task_thread_info(idle);
> +	smp_wmb();
> +
> +	/* take the core out of reset */
> +	ctrl &= ~(CORECTRL_SWRST0 << cpu);
> +	write_c0_corectrl(ctrl);
> +
> +	cpumask_set_cpu(cpu, &cpu_running);
> +
> +	spin_unlock_irqrestore(&smp_lock, flags);
> +
> +	return 0;
> +}
> +
> +static void jz4780_init_secondary(void)
> +{
> +}
> +
> +static void jz4780_smp_finish(void)
> +{
> +	u32 reim;
> +
> +	spin_lock(&smp_lock);
> +
> +	/* unmask mailbox interrupts for this core */
> +	reim = read_c0_reim();
> +	reim |= REIM_MBOXIRQ0M << smp_processor_id();
> +	write_c0_reim(reim);
> +
> +	spin_unlock(&smp_lock);
> +
> +	/* unmask interrupts for this core */
> +	change_c0_status(ST0_IM, STATUSF_IP3 | STATUSF_IP2 |
> +			 STATUSF_IP1 | STATUSF_IP0);
> +	irq_enable_hazard();
> +
> +	/* force broadcast timer */
> +	tick_broadcast_force();
> +}
> +
> +static void jz4780_send_ipi_single_locked(int cpu, unsigned int 
> action)
> +{
> +	u32 mbox;
> +
> +	switch (cpu) {
> +	case 0:
> +		mbox = read_c0_mailbox0();
> +		write_c0_mailbox0(mbox | action);
> +		break;
> +	case 1:
> +		mbox = read_c0_mailbox1();
> +		write_c0_mailbox1(mbox | action);
> +		break;

Same as above - there are 4 mailboxes.

> +	default:
> +		panic("unhandled cpu %d!", cpu);
> +	}
> +}
> +
> +static void jz4780_send_ipi_single(int cpu, unsigned int action)
> +{
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&smp_lock, flags);
> +	jz4780_send_ipi_single_locked(cpu, action);
> +	spin_unlock_irqrestore(&smp_lock, flags);
> +}
> +
> +static void jz4780_send_ipi_mask(const struct cpumask *mask,
> +				 unsigned int action)
> +{
> +	unsigned long flags;
> +	int cpu;
> +
> +	spin_lock_irqsave(&smp_lock, flags);
> +
> +	for_each_cpu(cpu, mask)
> +		jz4780_send_ipi_single_locked(cpu, action);
> +
> +	spin_unlock_irqrestore(&smp_lock, flags);
> +}
> +
> +static 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_smp_prepare_cpus,
> +};
> +
> +void jz4780_smp_init(void)

This one can be marked __init.

> +{
> +	register_smp_ops(&jz4780_smp_ops);
> +}
> diff --git a/arch/mips/kernel/idle.c b/arch/mips/kernel/idle.c
> index 37f8e78..a3afd98 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,
> @@ -88,6 +89,34 @@ static void __cpuidle rm7k_wait_irqoff(void)
>  }
> 
>  /*
> + * The Ingenic jz4780 SMP variant has to write back dirty cache 
> lines before
> + * executing wait. The CPU & cache clock will be gated until we 
> return from
> + * the wait, and if another core attempts to access data from our 
> data cache
> + * during this time then it will lock up.
> + */
> +void jz4780_smp_wait_irqoff(void)
> +{
> +	unsigned long pending = read_c0_cause() & read_c0_status() & 
> CAUSEF_IP;
> +
> +	/*
> +	 * Going to idle has a significant overhead due to the cache flush 
> so
> +	 * try to avoid it if we'll immediately be woken again due to an 
> IRQ.
> +	 */
> +	if (!need_resched() && !pending) {
> +		r4k_blast_dcache();
> +
> +		__asm__(
> +		"	.set push	\n"
> +		"	.set mips3	\n"
> +		"	sync		\n"
> +		"	wait		\n"
> +		"	.set pop	\n");
> +	}
> +
> +	local_irq_enable();
> +}
> +
> +/*
>   * 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
>   * interrupt can wake it, so they must be enabled before entering 
> idle modes.
> @@ -172,7 +201,6 @@ void __init check_wait(void)
>  	case CPU_CAVIUM_OCTEON_PLUS:
>  	case CPU_CAVIUM_OCTEON2:
>  	case CPU_CAVIUM_OCTEON3:
> -	case CPU_XBURST:
>  	case CPU_LOONGSON32:
>  	case CPU_XLR:
>  	case CPU_XLP:
> @@ -246,6 +274,12 @@ void __init check_wait(void)
>  		   cpu_wait = r4k_wait;
>  		 */
>  		break;
> +	case CPU_XBURST:
> +		if (IS_ENABLED(CONFIG_MACH_JZ4780) &&
> +				IS_ENABLED(CONFIG_SMP) && (NR_CPUS > 1))
> +			cpu_wait = jz4780_smp_wait_irqoff;
> +		else
> +			cpu_wait = r4k_wait;
>  	default:
>  		break;
>  	}
> --
> 2.7.4
> 

Re: [PATCH v6 2/7] MIPS: JZ4780: Introduce SMP support.

[Zhou Yanjie]

Hi Paul,

On 2020年02月21日 02:27, Paul Cercueil wrote:
> Hi Zhou,
>
>
> Le ven., févr. 21, 2020 at 00:24, 周琰杰 (Zhou Yanjie) 
> <zhouyanjie@wanyeetech.com> a écrit :
>> Forward port smp support from kernel 3.18.3 of CI20_linux
>> to upstream kernel 5.6.
>>
>> Tested-by: H. Nikolaus Schaller <hns@goldelico.com>
>> Tested-by: Paul Boddie <paul@boddie.org.uk>
>> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>> Reviewed-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> ---
>>
>> Notes:
>>     v1->v2:
>>     1.Remove unnecessary "plat_irq_dispatch(void)" in irq-ingenic.c.
>>     2.Add a timeout check for "jz4780_boot_secondary()" to avoid a 
>> dead loop.
>>     3.Replace hard code in smp.c with macro.
>>
>>     v2->v3:
>>     1.Remove unnecessary "extern void (*r4k_blast_dcache)(void)" in 
>> smp.c.
>>     2.Use "for_each_of_cpu_node" instead "for_each_compatible_node" 
>> in smp.c.
>>     3.Use "of_cpu_node_to_id" instead "of_property_read_u32_index" in 
>> smp.c.
>>     4.Move LCR related operations to jz4780-cgu.c.
>>
>>     v3->v4:
>>     Rebase on top of kernel 5.6-rc1.
>>
>>     v4->v5:
>>     1.Splitting changes involving "jz4780-cgu.c" into separate commit.
>>     2.Use "request_irq()" replace "setup_irq()".
>>
>>     v5->v6:
>>     In order to have a kernel that works on multiple SoCs at the same
>>     time, use "IS_ENABLED()" replace "#ifdef".
>>
>>  arch/mips/include/asm/mach-jz4740/smp.h |  81 ++++++++++
>>  arch/mips/jz4740/Kconfig                |   3 +
>>  arch/mips/jz4740/Makefile               |   5 +
>>  arch/mips/jz4740/prom.c                 |   4 +
>>  arch/mips/jz4740/smp-entry.S            |  57 +++++++
>>  arch/mips/jz4740/smp.c                  | 254 
>> ++++++++++++++++++++++++++++++++
>>  arch/mips/kernel/idle.c                 |  36 ++++-
>>  7 files changed, 439 insertions(+), 1 deletion(-)
>>  create mode 100644 arch/mips/include/asm/mach-jz4740/smp.h
>>  create mode 100644 arch/mips/jz4740/smp-entry.S
>>  create mode 100644 arch/mips/jz4740/smp.c
>>
>> diff --git a/arch/mips/include/asm/mach-jz4740/smp.h 
>> b/arch/mips/include/asm/mach-jz4740/smp.h
>> new file mode 100644
>> index 00000000..af3c21b
>> --- /dev/null
>> +++ b/arch/mips/include/asm/mach-jz4740/smp.h
>> @@ -0,0 +1,81 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + *  Copyright (C) 2013, Paul Burton <paul.burton@imgtec.com>
>> + *  JZ4780 SMP definitions
>> + */
>> +
>> +#ifndef __MIPS_ASM_MACH_JZ4740_JZ4780_SMP_H__
>> +#define __MIPS_ASM_MACH_JZ4740_JZ4780_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 smp_clr_pending(mask) do {        \
>> +        unsigned int stat;        \
>> +        stat = read_c0_corestatus();    \
>> +        stat &= ~((mask) & 0xff);    \
>> +        write_c0_corestatus(stat);    \
>> +    } while (0)
>> +
>> +/*
>> + * Core Control register
>> + */
>> +#define CORECTRL_SLEEP1M_SHIFT    17
>> +#define CORECTRL_SLEEP1M    (_ULCAST_(0x1) << CORECTRL_SLEEP1M_SHIFT)
>> +#define CORECTRL_SLEEP0M_SHIFT    16
>> +#define CORECTRL_SLEEP0M    (_ULCAST_(0x1) << CORECTRL_SLEEP0M_SHIFT)
>> +#define CORECTRL_RPC1_SHIFT    9
>> +#define CORECTRL_RPC1        (_ULCAST_(0x1) << CORECTRL_RPC1_SHIFT)
>> +#define CORECTRL_RPC0_SHIFT    8
>> +#define CORECTRL_RPC0        (_ULCAST_(0x1) << CORECTRL_RPC0_SHIFT)
>> +#define CORECTRL_SWRST1_SHIFT    1
>> +#define CORECTRL_SWRST1        (_ULCAST_(0x1) << CORECTRL_SWRST1_SHIFT)
>> +#define CORECTRL_SWRST0_SHIFT    0
>> +#define CORECTRL_SWRST0        (_ULCAST_(0x1) << CORECTRL_SWRST0_SHIFT)
>> +
>> +/*
>> + * Core Status register
>> + */
>> +#define CORESTATUS_SLEEP1_SHIFT    17
>> +#define CORESTATUS_SLEEP1    (_ULCAST_(0x1) << CORESTATUS_SLEEP1_SHIFT)
>> +#define CORESTATUS_SLEEP0_SHIFT    16
>> +#define CORESTATUS_SLEEP0    (_ULCAST_(0x1) << CORESTATUS_SLEEP0_SHIFT)
>> +#define CORESTATUS_IRQ1P_SHIFT    9
>> +#define CORESTATUS_IRQ1P    (_ULCAST_(0x1) << CORESTATUS_IRQ1P_SHIFT)
>> +#define CORESTATUS_IRQ0P_SHIFT    8
>> +#define CORESTATUS_IRQ0P    (_ULCAST_(0x1) << CORESTATUS_IRQ8P_SHIFT)
>> +#define CORESTATUS_MIRQ1P_SHIFT    1
>> +#define CORESTATUS_MIRQ1P    (_ULCAST_(0x1) << CORESTATUS_MIRQ1P_SHIFT)
>> +#define CORESTATUS_MIRQ0P_SHIFT    0
>> +#define CORESTATUS_MIRQ0P    (_ULCAST_(0x1) << CORESTATUS_MIRQ0P_SHIFT)
>> +
>> +/*
>> + * Reset Entry & IRQ Mask register
>> + */
>> +#define REIM_ENTRY_SHIFT    16
>> +#define REIM_ENTRY        (_ULCAST_(0xffff) << REIM_ENTRY_SHIFT)
>> +#define REIM_IRQ1M_SHIFT    9
>> +#define REIM_IRQ1M        (_ULCAST_(0x1) << REIM_IRQ1M_SHIFT)
>> +#define REIM_IRQ0M_SHIFT    8
>> +#define REIM_IRQ0M        (_ULCAST_(0x1) << REIM_IRQ0M_SHIFT)
>> +#define REIM_MBOXIRQ1M_SHIFT    1
>> +#define REIM_MBOXIRQ1M        (_ULCAST_(0x1) << REIM_MBOXIRQ1M_SHIFT)
>> +#define REIM_MBOXIRQ0M_SHIFT    0
>> +#define REIM_MBOXIRQ0M        (_ULCAST_(0x1) << REIM_MBOXIRQ0M_SHIFT)
>> +
>> +extern void jz4780_smp_init(void);
>> +extern void jz4780_secondary_cpu_entry(void);
>> +
>> +#endif /* __MIPS_ASM_MACH_JZ4740_JZ4780_SMP_H__ */
>> diff --git a/arch/mips/jz4740/Kconfig b/arch/mips/jz4740/Kconfig
>> index 412d2fa..0239597 100644
>> --- a/arch/mips/jz4740/Kconfig
>> +++ b/arch/mips/jz4740/Kconfig
>> @@ -34,9 +34,12 @@ config MACH_JZ4770
>>
>>  config MACH_JZ4780
>>      bool
>> +    select GENERIC_CLOCKEVENTS_BROADCAST if SMP
>>      select MIPS_CPU_SCACHE
>> +    select NR_CPUS_DEFAULT_2
>>      select SYS_HAS_CPU_MIPS32_R2
>>      select SYS_SUPPORTS_HIGHMEM
>> +    select SYS_SUPPORTS_SMP
>>
>>  config MACH_X1000
>>      bool
>> diff --git a/arch/mips/jz4740/Makefile b/arch/mips/jz4740/Makefile
>> index 6de14c0..0a0f024 100644
>> --- a/arch/mips/jz4740/Makefile
>> +++ b/arch/mips/jz4740/Makefile
>> @@ -12,3 +12,8 @@ CFLAGS_setup.o = -I$(src)/../../../scripts/dtc/libfdt
>>  # PM support
>>
>>  obj-$(CONFIG_PM) += pm.o
>> +
>> +# SMP support
>> +
>> +obj-$(CONFIG_SMP) += smp.o
>> +obj-$(CONFIG_SMP) += smp-entry.o
>> diff --git a/arch/mips/jz4740/prom.c b/arch/mips/jz4740/prom.c
>> index ff4555c..026259e 100644
>> --- a/arch/mips/jz4740/prom.c
>> +++ b/arch/mips/jz4740/prom.c
>> @@ -8,10 +8,14 @@
>>
>>  #include <asm/bootinfo.h>
>>  #include <asm/fw/fw.h>
>> +#include <asm/mach-jz4740/smp.h>
>>
>>  void __init prom_init(void)
>>  {
>>      fw_init_cmdline();
>> +
>> +    if (IS_ENABLED(CONFIG_MACH_JZ4780) && IS_ENABLED(CONFIG_SMP))
>> +        jz4780_smp_init();
>
> I guess it's OK for a first version, but SMP should really be 
> decoupled from the SoC version. A SMP-enabled kernel should be able to 
> also support non-SMP SoCs.
>

OK, will change in next version.

>
>>  }
>>
>>  void __init prom_free_prom_memory(void)
>> diff --git a/arch/mips/jz4740/smp-entry.S b/arch/mips/jz4740/smp-entry.S
>> new file mode 100644
>> index 00000000..20049a3
>> --- /dev/null
>> +++ b/arch/mips/jz4740/smp-entry.S
>> @@ -0,0 +1,57 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + *  Copyright (C) 2013, Paul Burton <paul.burton@imgtec.com>
>> + *  JZ4780 SMP entry point
>> + */
>> +
>> +#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
>> +
>> +.extern jz4780_cpu_entry_sp
>> +.extern jz4780_cpu_entry_gp
>> +
>> +.section .text.smp-entry
>> +.balign 0x10000
>> +.set noreorder
>> +LEAF(jz4780_secondary_cpu_entry)
>> +    mtc0    zero, CP0_CAUSE
>> +
>> +    li    t0, ST0_CU0
>> +    mtc0    t0, CP0_STATUS
>> +
>> +    /* cache setup */
>> +    li    t0, KSEG0
>> +    ori    t1, t0, CACHE_SIZE
>> +    mtc0    zero, CP0_TAGLO, 0
>> +1:    cache    Index_Store_Tag_I, 0(t0)
>> +    cache    Index_Store_Tag_D, 0(t0)
>> +    bne    t0, t1, 1b
>> +     addiu    t0, t0, CACHE_LINESIZE
>> +
>> +    /* kseg0 cache attribute */
>> +    mfc0    t0, CP0_CONFIG, 0
>> +    ori    t0, t0, CONF_CM_CACHABLE_NONCOHERENT
>> +    mtc0    t0, CP0_CONFIG, 0
>> +
>> +    /* pagemask */
>> +    mtc0    zero, CP0_PAGEMASK, 0
>> +
>> +    /* retrieve sp */
>> +    la    t0, jz4780_cpu_entry_sp
>> +    lw    sp, 0(t0)
>> +
>> +    /* retrieve gp */
>> +    la    t0, jz4780_cpu_entry_gp
>> +    lw    gp, 0(t0)
>> +
>> +    /* jump to the kernel in kseg0 */
>> +    la    t0, smp_bootstrap
>> +    jr    t0
>> +     nop
>> +    END(jz4780_secondary_cpu_entry)
>> diff --git a/arch/mips/jz4740/smp.c b/arch/mips/jz4740/smp.c
>> new file mode 100644
>> index 00000000..5fe0cf3
>> --- /dev/null
>> +++ b/arch/mips/jz4740/smp.c
>> @@ -0,0 +1,254 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + *  Copyright (C) 2013, Paul Burton <paul.burton@imgtec.com>
>> + *  JZ4780 SMP
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/of.h>
>> +#include <linux/sched.h>
>> +#include <linux/sched/task_stack.h>
>> +#include <linux/smp.h>
>> +#include <linux/tick.h>
>> +#include <asm/mach-jz4740/smp.h>
>> +#include <asm/smp-ops.h>
>> +
>> +static struct clk *cpu_clock_gates[CONFIG_NR_CPUS] = { NULL };
>> +
>> +u32 jz4780_cpu_entry_sp;
>> +u32 jz4780_cpu_entry_gp;
>> +
>> +static struct cpumask cpu_running;
>
> This is not used anywhere, is it?
>

It is used in "cpumask_set_cpu".

>> +
>> +static DEFINE_SPINLOCK(smp_lock);
>> +
>> +static irqreturn_t mbox_handler(int irq, void *dev_id)
>> +{
>> +    int cpu = smp_processor_id();
>> +    u32 action, status;
>> +
>> +    spin_lock(&smp_lock);
>> +
>> +    switch (cpu) {
>> +    case 0:
>> +        action = read_c0_mailbox0();
>> +        write_c0_mailbox0(0);
>> +        break;
>> +    case 1:
>> +        action = read_c0_mailbox1();
>> +        write_c0_mailbox1(0);
>> +        break;
>> +    default:
>> +        panic("unhandled cpu %d!", cpu);
>
> XBurst has 4 mailboxes, maybe add them here?
>

Sure.

>> +    }
>> +
>> +    /* clear pending mailbox interrupt */
>> +    status = read_c0_corestatus();
>> +    status &= ~(CORESTATUS_MIRQ0P << cpu);
>> +    write_c0_corestatus(status);
>> +
>> +    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 jz4780_smp_setup(void)
>> +{
>> +    u32 addr, reim;
>> +    int cpu;
>> +
>> +    reim = read_c0_reim();
>> +
>> +    for (cpu = 0; cpu < NR_CPUS; cpu++) {
>> +        __cpu_number_map[cpu] = cpu;
>> +        __cpu_logical_map[cpu] = cpu;
>> +        set_cpu_possible(cpu, true);
>> +    }
>> +
>> +    /* mask mailbox interrupts for this core */
>> +    reim &= ~REIM_MBOXIRQ0M;
>> +    write_c0_reim(reim);
>> +
>> +    /* clear mailboxes & pending mailbox IRQs */
>> +    write_c0_mailbox0(0);
>> +    write_c0_mailbox1(0);
>> +    write_c0_corestatus(0);
>> +
>> +    /* set reset entry point */
>> +    addr = KSEG1ADDR((u32)&jz4780_secondary_cpu_entry);
>> +    WARN_ON(addr & ~REIM_ENTRY);
>> +    reim &= ~REIM_ENTRY;
>> +    reim |= addr & REIM_ENTRY;
>> +
>> +    /* unmask mailbox interrupts for this core */
>> +    reim |= REIM_MBOXIRQ0M;
>> +    write_c0_reim(reim);
>> +    set_c0_status(STATUSF_IP3);
>> +    irq_enable_hazard();
>> +
>> +    cpumask_set_cpu(cpu, &cpu_running);
>> +}
>> +
>> +static void jz4780_smp_prepare_cpus(unsigned int max_cpus)
>> +{
>> +    struct device_node *cpu_node;
>> +    unsigned cpu, ctrl;
>> +    int err;
>> +
>> +    /* setup the mailbox IRQ */
>> +    err = request_irq(MIPS_CPU_IRQ_BASE + 3, mbox_handler,
>> +            IRQF_PERCPU | IRQF_NO_THREAD, "core mailbox", NULL);
>> +    if (err)
>> +        pr_err("request_irq() on core mailbox failed\n");
>> +
>> +    init_cpu_present(cpu_possible_mask);
>
> jz4780_smp_setup() marks CPUs 0..NR_CPUS as possible, NR_CPUS being a 
> config option that can be anywhere between 2 and 256. This basically 
> says that up to 256 CPUs are actually present.
>

I will change it in next version.

>> +
>> +    ctrl = read_c0_corectrl();
>> +
>> +    for (cpu = 0; cpu < max_cpus; cpu++) {
>> +        /* use reset entry point from REIM register */
>> +        ctrl |= CORECTRL_RPC0 << cpu;
>
> max_cpus is just NR_CPUS, so this does not work. You should move this 
> in the "for_each_of_cpu_node" below.
>

Sure.

>> +    }
>> +
>> +    for_each_of_cpu_node(cpu_node) {
>> +        cpu = of_cpu_node_to_id(cpu_node);
>> +        if (cpu < 0) {
>> +            pr_err("Failed to read index of %s\n",
>> +                   cpu_node->full_name);
>> +            continue;
>> +        }
>> +
>> +        cpu_clock_gates[cpu] = of_clk_get(cpu_node, 0);
>> +        if (IS_ERR(cpu_clock_gates[cpu])) {
>> +            cpu_clock_gates[cpu] = NULL;
>> +            continue;
>> +        }
>> +
>> +        err = clk_prepare(cpu_clock_gates[cpu]);
>> +        if (err)
>> +            pr_err("Failed to prepare CPU clock gate\n");
>> +    }
>> +
>> +    write_c0_corectrl(ctrl);
>> +}
>> +
>> +static int jz4780_boot_secondary(int cpu, struct task_struct *idle)
>> +{
>> +    unsigned long flags;
>> +    u32 ctrl;
>> +
>> +    spin_lock_irqsave(&smp_lock, flags);
>> +
>> +    /* ensure the core is in reset */
>> +    ctrl = read_c0_corectrl();
>> +    ctrl |= CORECTRL_SWRST0 << cpu;
>> +    write_c0_corectrl(ctrl);
>> +
>> +    /* ungate core clock */
>> +    if (cpu_clock_gates[cpu])
>> +        clk_enable(cpu_clock_gates[cpu]);
>> +
>> +    /* set entry sp/gp register values */
>> +    jz4780_cpu_entry_sp = __KSTK_TOS(idle);
>> +    jz4780_cpu_entry_gp = (u32)task_thread_info(idle);
>> +    smp_wmb();
>> +
>> +    /* take the core out of reset */
>> +    ctrl &= ~(CORECTRL_SWRST0 << cpu);
>> +    write_c0_corectrl(ctrl);
>> +
>> +    cpumask_set_cpu(cpu, &cpu_running);
>> +
>> +    spin_unlock_irqrestore(&smp_lock, flags);
>> +
>> +    return 0;
>> +}
>> +
>> +static void jz4780_init_secondary(void)
>> +{
>> +}
>> +
>> +static void jz4780_smp_finish(void)
>> +{
>> +    u32 reim;
>> +
>> +    spin_lock(&smp_lock);
>> +
>> +    /* unmask mailbox interrupts for this core */
>> +    reim = read_c0_reim();
>> +    reim |= REIM_MBOXIRQ0M << smp_processor_id();
>> +    write_c0_reim(reim);
>> +
>> +    spin_unlock(&smp_lock);
>> +
>> +    /* unmask interrupts for this core */
>> +    change_c0_status(ST0_IM, STATUSF_IP3 | STATUSF_IP2 |
>> +             STATUSF_IP1 | STATUSF_IP0);
>> +    irq_enable_hazard();
>> +
>> +    /* force broadcast timer */
>> +    tick_broadcast_force();
>> +}
>> +
>> +static void jz4780_send_ipi_single_locked(int cpu, unsigned int action)
>> +{
>> +    u32 mbox;
>> +
>> +    switch (cpu) {
>> +    case 0:
>> +        mbox = read_c0_mailbox0();
>> +        write_c0_mailbox0(mbox | action);
>> +        break;
>> +    case 1:
>> +        mbox = read_c0_mailbox1();
>> +        write_c0_mailbox1(mbox | action);
>> +        break;
>
> Same as above - there are 4 mailboxes.
>

Sure.

>> +    default:
>> +        panic("unhandled cpu %d!", cpu);
>> +    }
>> +}
>> +
>> +static void jz4780_send_ipi_single(int cpu, unsigned int action)
>> +{
>> +    unsigned long flags;
>> +
>> +    spin_lock_irqsave(&smp_lock, flags);
>> +    jz4780_send_ipi_single_locked(cpu, action);
>> +    spin_unlock_irqrestore(&smp_lock, flags);
>> +}
>> +
>> +static void jz4780_send_ipi_mask(const struct cpumask *mask,
>> +                 unsigned int action)
>> +{
>> +    unsigned long flags;
>> +    int cpu;
>> +
>> +    spin_lock_irqsave(&smp_lock, flags);
>> +
>> +    for_each_cpu(cpu, mask)
>> +        jz4780_send_ipi_single_locked(cpu, action);
>> +
>> +    spin_unlock_irqrestore(&smp_lock, flags);
>> +}
>> +
>> +static 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_smp_prepare_cpus,
>> +};
>> +
>> +void jz4780_smp_init(void)
>
> This one can be marked __init.
>

Sure.

>> +{
>> +    register_smp_ops(&jz4780_smp_ops);
>> +}
>> diff --git a/arch/mips/kernel/idle.c b/arch/mips/kernel/idle.c
>> index 37f8e78..a3afd98 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,
>> @@ -88,6 +89,34 @@ static void __cpuidle rm7k_wait_irqoff(void)
>>  }
>>
>>  /*
>> + * The Ingenic jz4780 SMP variant has to write back dirty cache 
>> lines before
>> + * executing wait. The CPU & cache clock will be gated until we 
>> return from
>> + * the wait, and if another core attempts to access data from our 
>> data cache
>> + * during this time then it will lock up.
>> + */
>> +void jz4780_smp_wait_irqoff(void)
>> +{
>> +    unsigned long pending = read_c0_cause() & read_c0_status() & 
>> CAUSEF_IP;
>> +
>> +    /*
>> +     * Going to idle has a significant overhead due to the cache 
>> flush so
>> +     * try to avoid it if we'll immediately be woken again due to an 
>> IRQ.
>> +     */
>> +    if (!need_resched() && !pending) {
>> +        r4k_blast_dcache();
>> +
>> +        __asm__(
>> +        "    .set push    \n"
>> +        "    .set mips3    \n"
>> +        "    sync        \n"
>> +        "    wait        \n"
>> +        "    .set pop    \n");
>> +    }
>> +
>> +    local_irq_enable();
>> +}
>> +
>> +/*
>>   * 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
>>   * interrupt can wake it, so they must be enabled before entering 
>> idle modes.
>> @@ -172,7 +201,6 @@ void __init check_wait(void)
>>      case CPU_CAVIUM_OCTEON_PLUS:
>>      case CPU_CAVIUM_OCTEON2:
>>      case CPU_CAVIUM_OCTEON3:
>> -    case CPU_XBURST:
>>      case CPU_LOONGSON32:
>>      case CPU_XLR:
>>      case CPU_XLP:
>> @@ -246,6 +274,12 @@ void __init check_wait(void)
>>             cpu_wait = r4k_wait;
>>           */
>>          break;
>> +    case CPU_XBURST:
>> +        if (IS_ENABLED(CONFIG_MACH_JZ4780) &&
>> +                IS_ENABLED(CONFIG_SMP) && (NR_CPUS > 1))
>> +            cpu_wait = jz4780_smp_wait_irqoff;
>> +        else
>> +            cpu_wait = r4k_wait;
>>      default:
>>          break;
>>      }
>> -- 
>> 2.7.4
>>
>

[PATCH v6 3/7] MIPS: CI20: Modify DTS to support high resolution timer for SMP.

[周琰杰 (Zhou Yanjie)]

Modify DTS, change tcu channel from 2 to 3, channel #0 and #1 for
per core local timer, #2 for clocksource.

Tested-by: H. Nikolaus Schaller <hns@goldelico.com>
Tested-by: Paul Boddie <paul@boddie.org.uk>
Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
---

Notes:
    v1->v2:
    No change.
    
    v2->v3:
    No change.
    
    v3->v4:
    Rebase on top of kernel 5.6-rc1.
    
    v4->v5:
    Move [5/6] in v4 to this patch, to ensure that we can
    git-bisect without ending up with a broken kernel.
    
    v5->v6:
    No change.

 arch/mips/boot/dts/ingenic/ci20.dts | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/arch/mips/boot/dts/ingenic/ci20.dts b/arch/mips/boot/dts/ingenic/ci20.dts
index 37b9316..29437aa 100644
--- a/arch/mips/boot/dts/ingenic/ci20.dts
+++ b/arch/mips/boot/dts/ingenic/ci20.dts
@@ -455,7 +455,14 @@
 };
 
 &tcu {
-	/* 3 MHz for the system timer and clocksource */
-	assigned-clocks = <&tcu TCU_CLK_TIMER0>, <&tcu TCU_CLK_TIMER1>;
-	assigned-clock-rates = <3000000>, <3000000>;
+	/* 3 MHz for the system timers, 750kHz for the clocksource */
+	assigned-clocks = <&tcu TCU_CLK_TIMER0>, <&tcu TCU_CLK_TIMER1>,
+					  <&tcu TCU_CLK_TIMER2>;
+	assigned-clock-rates = <3000000>, <3000000>, <750000>;
+
+	/*
+	 * Use channel #0 and #1 for the per core system timer,
+	 * and use channel #2 for the clocksource.
+	 */
+	ingenic,pwm-channels-mask = <0xF8>;
 };
-- 
2.7.4

[PATCH v6 4/7] clocksource: Ingenic: Add high resolution timer support for SMP.

[周琰杰 (Zhou Yanjie)]

Enable clock event handling on per CPU core basis.
Make sure that interrupts raised on the first core execute
event handlers on the correct CPU core.

Tested-by: H. Nikolaus Schaller <hns@goldelico.com>
Tested-by: Paul Boddie <paul@boddie.org.uk>
Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
---

Notes:
    v1->v2:
    1.Adjust function naming to make it more reasonable.
    2.Replace function smp_call_function_single() with
      smp_call_function_single_async() in order to resolve
      the warning below:
    
    [    0.350942] smp: Brought up 1 node, 2 CPUs
    [    0.365497] ------------[ cut here ]------------
    [    0.365522] WARNING: CPU: 0 PID: 1 at kernel/smp.c:300 smp_call_function_single+0x110/0x200
    [    0.365533] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.5.0-rc1+ #5
    [    0.365537] Stack : 00000000 59c73bcd 00000037 80074e80 80000000 80670000 805a0000 80620590
    [    0.365557]         8065ce38 8fc0dc8c 806d0000 00000000 80670000 00000001 8fc0dc20 59c73bcd
    [    0.365574]         00000000 00000000 806f0000 80670000 00000000 806dab00 00000000 2d302e35
    [    0.365591]         203a6d6d 806e0000 806e0000 70617773 80670000 00000000 00000000 00000009
    [    0.365610]         00000000 8fc94e20 8fc0de30 80690000 00000018 803592dc 00000000 806d0000
    [    0.365627]         ...
    [    0.365634] Call Trace:
    [    0.365647] [<8001b9a0>] show_stack+0x6c/0x12c
    [    0.365663] [<804aed20>] dump_stack+0x98/0xc8
    [    0.365673] [<8003044c>] __warn+0xc4/0xe8
    [    0.365682] [<800304f4>] warn_slowpath_fmt+0x84/0xb8
    [    0.365690] [<800a886c>] smp_call_function_single+0x110/0x200
    [    0.365703] ---[ end trace 5785856ca39c79d5 ]---
    [    0.365557]         8065ce38 8fc0dc8c 806d0000 00000000 80670000 00000001 8fc0dc20 59c73bcd
    [    0.365574]         00000000 00000000 806f0000 80670000 00000000 806dab00 00000000 2d302e35
    [    0.365591]         203a6d6d 806e0000 806e0000 70617773 80670000 00000000 00000000 00000009
    [    0.365610]         00000000 8fc94e20 8fc0de30 80690000 00000018 803592dc 00000000 806d0000
    [    0.365627]         ...
    [    0.365634] Call Trace:
    [    0.365647] [<8001b9a0>] show_stack+0x6c/0x12c
    [    0.365663] [<804aed20>] dump_stack+0x98/0xc8
    [    0.365673] [<8003044c>] __warn+0xc4/0xe8
    [    0.365682] [<800304f4>] warn_slowpath_fmt+0x84/0xb8
    [    0.365690] [<800a886c>] smp_call_function_single+0x110/0x200
    [    0.365703] ---[ end trace 5785856ca39c79d5 ]---
    
    v2->v3:
    No Change.
    
    v3->v4:
    Rebase on top of kernel 5.6-rc1.
    
    v4->v5:
    Move the check for (evt->event_handler) from "ingenic_per_cpu_event_handler"
    to "ingenic_tcu_cevt_cb".
    
    v5->v6:
    No change.

 drivers/clocksource/ingenic-timer.c | 113 +++++++++++++++++++++++++++++-------
 1 file changed, 91 insertions(+), 22 deletions(-)

diff --git a/drivers/clocksource/ingenic-timer.c b/drivers/clocksource/ingenic-timer.c
index 4bbdb3d..e396326 100644
--- a/drivers/clocksource/ingenic-timer.c
+++ b/drivers/clocksource/ingenic-timer.c
@@ -1,7 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * JZ47xx SoCs TCU IRQ driver
+ * XBurst SoCs TCU IRQ driver
  * Copyright (C) 2019 Paul Cercueil <paul@crapouillou.net>
+ * Copyright (C) 2020 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
  */
 
 #include <linux/bitops.h>
@@ -21,18 +22,23 @@
 
 #include <dt-bindings/clock/ingenic,tcu.h>
 
+static DEFINE_PER_CPU(call_single_data_t, ingenic_cevt_csd);
+
 struct ingenic_soc_info {
 	unsigned int num_channels;
 };
 
 struct ingenic_tcu {
 	struct regmap *map;
+	struct device_node *np;
 	struct clk *timer_clk, *cs_clk;
+	unsigned int timer_local[NR_CPUS];
 	unsigned int timer_channel, cs_channel;
 	struct clock_event_device cevt;
 	struct clocksource cs;
-	char name[4];
+	char name[8];
 	unsigned long pwm_channels_mask;
+	int cpu;
 };
 
 static struct ingenic_tcu *ingenic_tcu;
@@ -81,6 +87,24 @@ static int ingenic_tcu_cevt_set_next(unsigned long next,
 	return 0;
 }
 
+static void ingenic_per_cpu_event_handler(void *info)
+{
+	struct clock_event_device *cevt = (struct clock_event_device *) info;
+
+	cevt->event_handler(cevt);
+}
+
+static void ingenic_tcu_per_cpu_cb(struct clock_event_device *evt)
+{
+	struct ingenic_tcu *tcu = to_ingenic_tcu(evt);
+	call_single_data_t *csd;
+
+	csd = &per_cpu(ingenic_cevt_csd, tcu->cpu);
+	csd->info = (void *) evt;
+	csd->func = ingenic_per_cpu_event_handler;
+	smp_call_function_single_async(tcu->cpu, csd);
+}
+
 static irqreturn_t ingenic_tcu_cevt_cb(int irq, void *dev_id)
 {
 	struct clock_event_device *evt = dev_id;
@@ -89,7 +113,7 @@ static irqreturn_t ingenic_tcu_cevt_cb(int irq, void *dev_id)
 	regmap_write(tcu->map, TCU_REG_TECR, BIT(tcu->timer_channel));
 
 	if (evt->event_handler)
-		evt->event_handler(evt);
+		ingenic_tcu_per_cpu_cb(evt);
 
 	return IRQ_HANDLED;
 }
@@ -105,14 +129,21 @@ static struct clk * __init ingenic_tcu_get_clock(struct device_node *np, int id)
 	return of_clk_get_from_provider(&args);
 }
 
-static int __init ingenic_tcu_timer_init(struct device_node *np,
-					 struct ingenic_tcu *tcu)
+static int ingenic_tcu_setup_per_cpu_cevt(struct device_node *np,
+				     unsigned int channel)
 {
-	unsigned int timer_virq, channel = tcu->timer_channel;
+	unsigned int timer_virq;
 	struct irq_domain *domain;
+	struct ingenic_tcu *tcu;
 	unsigned long rate;
 	int err;
 
+	tcu = kzalloc(sizeof(*tcu), GFP_KERNEL);
+	if (!tcu)
+		return -ENOMEM;
+
+	tcu->map = ingenic_tcu->map;
+
 	tcu->timer_clk = ingenic_tcu_get_clock(np, channel);
 	if (IS_ERR(tcu->timer_clk))
 		return PTR_ERR(tcu->timer_clk);
@@ -139,13 +170,15 @@ static int __init ingenic_tcu_timer_init(struct device_node *np,
 		goto err_clk_disable;
 	}
 
-	snprintf(tcu->name, sizeof(tcu->name), "TCU");
+	snprintf(tcu->name, sizeof(tcu->name), "TCU%u", channel);
 
 	err = request_irq(timer_virq, ingenic_tcu_cevt_cb, IRQF_TIMER,
 			  tcu->name, &tcu->cevt);
 	if (err)
 		goto err_irq_dispose_mapping;
 
+	tcu->cpu = smp_processor_id();
+	tcu->timer_channel = channel;
 	tcu->cevt.cpumask = cpumask_of(smp_processor_id());
 	tcu->cevt.features = CLOCK_EVT_FEAT_ONESHOT;
 	tcu->cevt.name = tcu->name;
@@ -166,6 +199,25 @@ static int __init ingenic_tcu_timer_init(struct device_node *np,
 	return err;
 }
 
+static int ingenic_tcu_setup_cevt(unsigned int cpu)
+{
+	int ret;
+
+	ret = ingenic_tcu_setup_per_cpu_cevt(ingenic_tcu->np,
+						ingenic_tcu->timer_local[cpu]);
+	if (ret)
+		goto err_tcu_clocksource_cleanup;
+
+	return 0;
+
+err_tcu_clocksource_cleanup:
+	clocksource_unregister(&ingenic_tcu->cs);
+	clk_disable_unprepare(ingenic_tcu->cs_clk);
+	clk_put(ingenic_tcu->cs_clk);
+	kfree(ingenic_tcu);
+	return ret;
+}
+
 static int __init ingenic_tcu_clocksource_init(struct device_node *np,
 					       struct ingenic_tcu *tcu)
 {
@@ -239,6 +291,7 @@ static int __init ingenic_tcu_init(struct device_node *np)
 	const struct ingenic_soc_info *soc_info = id->data;
 	struct ingenic_tcu *tcu;
 	struct regmap *map;
+	unsigned cpu = 0;
 	long rate;
 	int ret;
 
@@ -252,13 +305,18 @@ static int __init ingenic_tcu_init(struct device_node *np)
 	if (!tcu)
 		return -ENOMEM;
 
-	/* Enable all TCU channels for PWM use by default except channels 0/1 */
-	tcu->pwm_channels_mask = GENMASK(soc_info->num_channels - 1, 2);
+	/*
+	 * Enable all TCU channels for PWM use by default except channels 0/1,
+	 * and channel 2 if target CPU is JZ4780 and SMP is selected.
+	 */
+	tcu->pwm_channels_mask = GENMASK(soc_info->num_channels - 1,
+								NR_CPUS + 1);
 	of_property_read_u32(np, "ingenic,pwm-channels-mask",
 			     (u32 *)&tcu->pwm_channels_mask);
 
-	/* Verify that we have at least two free channels */
-	if (hweight8(tcu->pwm_channels_mask) > soc_info->num_channels - 2) {
+	/* Verify that we have at least NR_CPUS + 1 free channels */
+	if (hweight8(tcu->pwm_channels_mask) >
+			soc_info->num_channels - NR_CPUS + 1) {
 		pr_crit("%s: Invalid PWM channel mask: 0x%02lx\n", __func__,
 			tcu->pwm_channels_mask);
 		ret = -EINVAL;
@@ -266,13 +324,27 @@ static int __init ingenic_tcu_init(struct device_node *np)
 	}
 
 	tcu->map = map;
+	tcu->np = np;
 	ingenic_tcu = tcu;
 
-	tcu->timer_channel = find_first_zero_bit(&tcu->pwm_channels_mask,
+	tcu->timer_local[cpu] = find_first_zero_bit(&tcu->pwm_channels_mask,
 						 soc_info->num_channels);
-	tcu->cs_channel = find_next_zero_bit(&tcu->pwm_channels_mask,
-					     soc_info->num_channels,
-					     tcu->timer_channel + 1);
+
+	if (NR_CPUS > 1) {
+		for (cpu = 1; cpu < NR_CPUS; cpu++)
+			tcu->timer_local[cpu] = find_next_zero_bit(
+						&tcu->pwm_channels_mask,
+						soc_info->num_channels,
+						tcu->timer_local[cpu - 1] + 1);
+
+		tcu->cs_channel = find_next_zero_bit(&tcu->pwm_channels_mask,
+					soc_info->num_channels,
+					tcu->timer_local[cpu-1] + 1);
+	} else {
+		tcu->cs_channel = find_next_zero_bit(&tcu->pwm_channels_mask,
+					soc_info->num_channels,
+					tcu->timer_local[cpu] + 1);
+	}
 
 	ret = ingenic_tcu_clocksource_init(np, tcu);
 	if (ret) {
@@ -280,9 +352,10 @@ static int __init ingenic_tcu_init(struct device_node *np)
 		goto err_free_ingenic_tcu;
 	}
 
-	ret = ingenic_tcu_timer_init(np, tcu);
-	if (ret)
-		goto err_tcu_clocksource_cleanup;
+	/* Setup clock events on each CPU core */
+	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "Ingenic XBurst: online",
+				ingenic_tcu_setup_cevt, NULL);
+	WARN_ON(ret < 0);
 
 	/* Register the sched_clock at the end as there's no way to undo it */
 	rate = clk_get_rate(tcu->cs_clk);
@@ -290,10 +363,6 @@ static int __init ingenic_tcu_init(struct device_node *np)
 
 	return 0;
 
-err_tcu_clocksource_cleanup:
-	clocksource_unregister(&tcu->cs);
-	clk_disable_unprepare(tcu->cs_clk);
-	clk_put(tcu->cs_clk);
 err_free_ingenic_tcu:
 	kfree(tcu);
 	return ret;
-- 
2.7.4

Re: [PATCH v6 4/7] clocksource: Ingenic: Add high resolution timer support for SMP.

[Paul Cercueil]

Hi Zhou,


Le ven., févr. 21, 2020 at 00:24, 周琰杰 (Zhou Yanjie) 
<zhouyanjie@wanyeetech.com> a écrit :
> Enable clock event handling on per CPU core basis.
> Make sure that interrupts raised on the first core execute
> event handlers on the correct CPU core.
> 
> Tested-by: H. Nikolaus Schaller <hns@goldelico.com>
> Tested-by: Paul Boddie <paul@boddie.org.uk>
> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> ---
> 
> Notes:
>     v1->v2:
>     1.Adjust function naming to make it more reasonable.
>     2.Replace function smp_call_function_single() with
>       smp_call_function_single_async() in order to resolve
>       the warning below:
> 
>     [    0.350942] smp: Brought up 1 node, 2 CPUs
>     [    0.365497] ------------[ cut here ]------------
>     [    0.365522] WARNING: CPU: 0 PID: 1 at kernel/smp.c:300 
> smp_call_function_single+0x110/0x200
>     [    0.365533] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
> 5.5.0-rc1+ #5
>     [    0.365537] Stack : 00000000 59c73bcd 00000037 80074e80 
> 80000000 80670000 805a0000 80620590
>     [    0.365557]         8065ce38 8fc0dc8c 806d0000 00000000 
> 80670000 00000001 8fc0dc20 59c73bcd
>     [    0.365574]         00000000 00000000 806f0000 80670000 
> 00000000 806dab00 00000000 2d302e35
>     [    0.365591]         203a6d6d 806e0000 806e0000 70617773 
> 80670000 00000000 00000000 00000009
>     [    0.365610]         00000000 8fc94e20 8fc0de30 80690000 
> 00000018 803592dc 00000000 806d0000
>     [    0.365627]         ...
>     [    0.365634] Call Trace:
>     [    0.365647] [<8001b9a0>] show_stack+0x6c/0x12c
>     [    0.365663] [<804aed20>] dump_stack+0x98/0xc8
>     [    0.365673] [<8003044c>] __warn+0xc4/0xe8
>     [    0.365682] [<800304f4>] warn_slowpath_fmt+0x84/0xb8
>     [    0.365690] [<800a886c>] smp_call_function_single+0x110/0x200
>     [    0.365703] ---[ end trace 5785856ca39c79d5 ]---
>     [    0.365557]         8065ce38 8fc0dc8c 806d0000 00000000 
> 80670000 00000001 8fc0dc20 59c73bcd
>     [    0.365574]         00000000 00000000 806f0000 80670000 
> 00000000 806dab00 00000000 2d302e35
>     [    0.365591]         203a6d6d 806e0000 806e0000 70617773 
> 80670000 00000000 00000000 00000009
>     [    0.365610]         00000000 8fc94e20 8fc0de30 80690000 
> 00000018 803592dc 00000000 806d0000
>     [    0.365627]         ...
>     [    0.365634] Call Trace:
>     [    0.365647] [<8001b9a0>] show_stack+0x6c/0x12c
>     [    0.365663] [<804aed20>] dump_stack+0x98/0xc8
>     [    0.365673] [<8003044c>] __warn+0xc4/0xe8
>     [    0.365682] [<800304f4>] warn_slowpath_fmt+0x84/0xb8
>     [    0.365690] [<800a886c>] smp_call_function_single+0x110/0x200
>     [    0.365703] ---[ end trace 5785856ca39c79d5 ]---
> 
>     v2->v3:
>     No Change.
> 
>     v3->v4:
>     Rebase on top of kernel 5.6-rc1.
> 
>     v4->v5:
>     Move the check for (evt->event_handler) from 
> "ingenic_per_cpu_event_handler"
>     to "ingenic_tcu_cevt_cb".
> 
>     v5->v6:
>     No change.
> 
>  drivers/clocksource/ingenic-timer.c | 113 
> +++++++++++++++++++++++++++++-------
>  1 file changed, 91 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/clocksource/ingenic-timer.c 
> b/drivers/clocksource/ingenic-timer.c
> index 4bbdb3d..e396326 100644
> --- a/drivers/clocksource/ingenic-timer.c
> +++ b/drivers/clocksource/ingenic-timer.c
> @@ -1,7 +1,8 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /*
> - * JZ47xx SoCs TCU IRQ driver
> + * XBurst SoCs TCU IRQ driver
>   * Copyright (C) 2019 Paul Cercueil <paul@crapouillou.net>
> + * Copyright (C) 2020 周琰杰 (Zhou Yanjie) 
> <zhouyanjie@wanyeetech.com>
>   */
> 
>  #include <linux/bitops.h>
> @@ -21,18 +22,23 @@
> 
>  #include <dt-bindings/clock/ingenic,tcu.h>
> 
> +static DEFINE_PER_CPU(call_single_data_t, ingenic_cevt_csd);
> +
>  struct ingenic_soc_info {
>  	unsigned int num_channels;
>  };
> 
>  struct ingenic_tcu {
>  	struct regmap *map;
> +	struct device_node *np;
>  	struct clk *timer_clk, *cs_clk;
> +	unsigned int timer_local[NR_CPUS];
>  	unsigned int timer_channel, cs_channel;
>  	struct clock_event_device cevt;
>  	struct clocksource cs;
> -	char name[4];
> +	char name[8];
>  	unsigned long pwm_channels_mask;
> +	int cpu;
>  };
> 
>  static struct ingenic_tcu *ingenic_tcu;
> @@ -81,6 +87,24 @@ static int ingenic_tcu_cevt_set_next(unsigned long 
> next,
>  	return 0;
>  }
> 
> +static void ingenic_per_cpu_event_handler(void *info)
> +{
> +	struct clock_event_device *cevt = (struct clock_event_device *) 
> info;
> +
> +	cevt->event_handler(cevt);
> +}
> +
> +static void ingenic_tcu_per_cpu_cb(struct clock_event_device *evt)
> +{
> +	struct ingenic_tcu *tcu = to_ingenic_tcu(evt);
> +	call_single_data_t *csd;
> +
> +	csd = &per_cpu(ingenic_cevt_csd, tcu->cpu);
> +	csd->info = (void *) evt;
> +	csd->func = ingenic_per_cpu_event_handler;
> +	smp_call_function_single_async(tcu->cpu, csd);
> +}
> +
>  static irqreturn_t ingenic_tcu_cevt_cb(int irq, void *dev_id)
>  {
>  	struct clock_event_device *evt = dev_id;
> @@ -89,7 +113,7 @@ static irqreturn_t ingenic_tcu_cevt_cb(int irq, 
> void *dev_id)
>  	regmap_write(tcu->map, TCU_REG_TECR, BIT(tcu->timer_channel));
> 
>  	if (evt->event_handler)
> -		evt->event_handler(evt);
> +		ingenic_tcu_per_cpu_cb(evt);
> 
>  	return IRQ_HANDLED;
>  }
> @@ -105,14 +129,21 @@ static struct clk * __init 
> ingenic_tcu_get_clock(struct device_node *np, int id)
>  	return of_clk_get_from_provider(&args);
>  }
> 
> -static int __init ingenic_tcu_timer_init(struct device_node *np,
> -					 struct ingenic_tcu *tcu)
> +static int ingenic_tcu_setup_per_cpu_cevt(struct device_node *np,
> +				     unsigned int channel)
>  {
> -	unsigned int timer_virq, channel = tcu->timer_channel;
> +	unsigned int timer_virq;
>  	struct irq_domain *domain;
> +	struct ingenic_tcu *tcu;
>  	unsigned long rate;
>  	int err;
> 
> +	tcu = kzalloc(sizeof(*tcu), GFP_KERNEL);
> +	if (!tcu)
> +		return -ENOMEM;
> +
> +	tcu->map = ingenic_tcu->map;
> +
>  	tcu->timer_clk = ingenic_tcu_get_clock(np, channel);
>  	if (IS_ERR(tcu->timer_clk))
>  		return PTR_ERR(tcu->timer_clk);
> @@ -139,13 +170,15 @@ static int __init ingenic_tcu_timer_init(struct 
> device_node *np,
>  		goto err_clk_disable;
>  	}
> 
> -	snprintf(tcu->name, sizeof(tcu->name), "TCU");
> +	snprintf(tcu->name, sizeof(tcu->name), "TCU%u", channel);
> 
>  	err = request_irq(timer_virq, ingenic_tcu_cevt_cb, IRQF_TIMER,
>  			  tcu->name, &tcu->cevt);
>  	if (err)
>  		goto err_irq_dispose_mapping;
> 
> +	tcu->cpu = smp_processor_id();
> +	tcu->timer_channel = channel;
>  	tcu->cevt.cpumask = cpumask_of(smp_processor_id());
>  	tcu->cevt.features = CLOCK_EVT_FEAT_ONESHOT;
>  	tcu->cevt.name = tcu->name;
> @@ -166,6 +199,25 @@ static int __init ingenic_tcu_timer_init(struct 
> device_node *np,
>  	return err;
>  }
> 
> +static int ingenic_tcu_setup_cevt(unsigned int cpu)
> +{
> +	int ret;
> +
> +	ret = ingenic_tcu_setup_per_cpu_cevt(ingenic_tcu->np,
> +						ingenic_tcu->timer_local[cpu]);
> +	if (ret)
> +		goto err_tcu_clocksource_cleanup;
> +
> +	return 0;
> +
> +err_tcu_clocksource_cleanup:
> +	clocksource_unregister(&ingenic_tcu->cs);
> +	clk_disable_unprepare(ingenic_tcu->cs_clk);
> +	clk_put(ingenic_tcu->cs_clk);
> +	kfree(ingenic_tcu);
> +	return ret;
> +}
> +
>  static int __init ingenic_tcu_clocksource_init(struct device_node 
> *np,
>  					       struct ingenic_tcu *tcu)
>  {
> @@ -239,6 +291,7 @@ static int __init ingenic_tcu_init(struct 
> device_node *np)
>  	const struct ingenic_soc_info *soc_info = id->data;
>  	struct ingenic_tcu *tcu;
>  	struct regmap *map;
> +	unsigned cpu = 0;
>  	long rate;
>  	int ret;
> 
> @@ -252,13 +305,18 @@ static int __init ingenic_tcu_init(struct 
> device_node *np)
>  	if (!tcu)
>  		return -ENOMEM;
> 
> -	/* Enable all TCU channels for PWM use by default except channels 
> 0/1 */
> -	tcu->pwm_channels_mask = GENMASK(soc_info->num_channels - 1, 2);
> +	/*
> +	 * Enable all TCU channels for PWM use by default except channels 
> 0/1,
> +	 * and channel 2 if target CPU is JZ4780 and SMP is selected.
> +	 */
> +	tcu->pwm_channels_mask = GENMASK(soc_info->num_channels - 1,
> +								NR_CPUS + 1);
>  	of_property_read_u32(np, "ingenic,pwm-channels-mask",
>  			     (u32 *)&tcu->pwm_channels_mask);
> 
> -	/* Verify that we have at least two free channels */
> -	if (hweight8(tcu->pwm_channels_mask) > soc_info->num_channels - 2) {
> +	/* Verify that we have at least NR_CPUS + 1 free channels */
> +	if (hweight8(tcu->pwm_channels_mask) >
> +			soc_info->num_channels - NR_CPUS + 1) {

NR_CPUS can be up to 256. You want to use num_online_cpus() here, I 
believe.

>  		pr_crit("%s: Invalid PWM channel mask: 0x%02lx\n", __func__,
>  			tcu->pwm_channels_mask);
>  		ret = -EINVAL;
> @@ -266,13 +324,27 @@ static int __init ingenic_tcu_init(struct 
> device_node *np)
>  	}
> 
>  	tcu->map = map;
> +	tcu->np = np;
>  	ingenic_tcu = tcu;
> 
> -	tcu->timer_channel = find_first_zero_bit(&tcu->pwm_channels_mask,
> +	tcu->timer_local[cpu] = find_first_zero_bit(&tcu->pwm_channels_mask,
>  						 soc_info->num_channels);
> -	tcu->cs_channel = find_next_zero_bit(&tcu->pwm_channels_mask,
> -					     soc_info->num_channels,
> -					     tcu->timer_channel + 1);
> +
> +	if (NR_CPUS > 1) {
> +		for (cpu = 1; cpu < NR_CPUS; cpu++)
> +			tcu->timer_local[cpu] = find_next_zero_bit(
> +						&tcu->pwm_channels_mask,
> +						soc_info->num_channels,
> +						tcu->timer_local[cpu - 1] + 1);
> +
> +		tcu->cs_channel = find_next_zero_bit(&tcu->pwm_channels_mask,
> +					soc_info->num_channels,
> +					tcu->timer_local[cpu-1] + 1);
> +	} else {
> +		tcu->cs_channel = find_next_zero_bit(&tcu->pwm_channels_mask,
> +					soc_info->num_channels,
> +					tcu->timer_local[cpu] + 1);
> +	}

I believe you can factorize the code a bit here - don't check for 
NR_CPUS > 1, and have a for (cpu = 0; cpu < num_online_cpus(); cpu++) 
loop. You could use a temporary variable to store the 'next bit' value.

-Paul

> 
>  	ret = ingenic_tcu_clocksource_init(np, tcu);
>  	if (ret) {
> @@ -280,9 +352,10 @@ static int __init ingenic_tcu_init(struct 
> device_node *np)
>  		goto err_free_ingenic_tcu;
>  	}
> 
> -	ret = ingenic_tcu_timer_init(np, tcu);
> -	if (ret)
> -		goto err_tcu_clocksource_cleanup;
> +	/* Setup clock events on each CPU core */
> +	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "Ingenic XBurst: 
> online",
> +				ingenic_tcu_setup_cevt, NULL);
> +	WARN_ON(ret < 0);
> 
>  	/* Register the sched_clock at the end as there's no way to undo it 
> */
>  	rate = clk_get_rate(tcu->cs_clk);
> @@ -290,10 +363,6 @@ static int __init ingenic_tcu_init(struct 
> device_node *np)
> 
>  	return 0;
> 
> -err_tcu_clocksource_cleanup:
> -	clocksource_unregister(&tcu->cs);
> -	clk_disable_unprepare(tcu->cs_clk);
> -	clk_put(tcu->cs_clk);
>  err_free_ingenic_tcu:
>  	kfree(tcu);
>  	return ret;
> --
> 2.7.4
> 

Re: [PATCH v6 4/7] clocksource: Ingenic: Add high resolution timer support for SMP.

[Zhou Yanjie]

Hi Paul,

On 2020年02月21日 02:33, Paul Cercueil wrote:
> Hi Zhou,
>
>
> Le ven., févr. 21, 2020 at 00:24, 周琰杰 (Zhou Yanjie) 
> <zhouyanjie@wanyeetech.com> a écrit :
>> Enable clock event handling on per CPU core basis.
>> Make sure that interrupts raised on the first core execute
>> event handlers on the correct CPU core.
>>
>> Tested-by: H. Nikolaus Schaller <hns@goldelico.com>
>> Tested-by: Paul Boddie <paul@boddie.org.uk>
>> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>> ---
>>
>> Notes:
>>     v1->v2:
>>     1.Adjust function naming to make it more reasonable.
>>     2.Replace function smp_call_function_single() with
>>       smp_call_function_single_async() in order to resolve
>>       the warning below:
>>
>>     [    0.350942] smp: Brought up 1 node, 2 CPUs
>>     [    0.365497] ------------[ cut here ]------------
>>     [    0.365522] WARNING: CPU: 0 PID: 1 at kernel/smp.c:300 
>> smp_call_function_single+0x110/0x200
>>     [    0.365533] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
>> 5.5.0-rc1+ #5
>>     [    0.365537] Stack : 00000000 59c73bcd 00000037 80074e80 
>> 80000000 80670000 805a0000 80620590
>>     [    0.365557]         8065ce38 8fc0dc8c 806d0000 00000000 
>> 80670000 00000001 8fc0dc20 59c73bcd
>>     [    0.365574]         00000000 00000000 806f0000 80670000 
>> 00000000 806dab00 00000000 2d302e35
>>     [    0.365591]         203a6d6d 806e0000 806e0000 70617773 
>> 80670000 00000000 00000000 00000009
>>     [    0.365610]         00000000 8fc94e20 8fc0de30 80690000 
>> 00000018 803592dc 00000000 806d0000
>>     [    0.365627]         ...
>>     [    0.365634] Call Trace:
>>     [    0.365647] [<8001b9a0>] show_stack+0x6c/0x12c
>>     [    0.365663] [<804aed20>] dump_stack+0x98/0xc8
>>     [    0.365673] [<8003044c>] __warn+0xc4/0xe8
>>     [    0.365682] [<800304f4>] warn_slowpath_fmt+0x84/0xb8
>>     [    0.365690] [<800a886c>] smp_call_function_single+0x110/0x200
>>     [    0.365703] ---[ end trace 5785856ca39c79d5 ]---
>>     [    0.365557]         8065ce38 8fc0dc8c 806d0000 00000000 
>> 80670000 00000001 8fc0dc20 59c73bcd
>>     [    0.365574]         00000000 00000000 806f0000 80670000 
>> 00000000 806dab00 00000000 2d302e35
>>     [    0.365591]         203a6d6d 806e0000 806e0000 70617773 
>> 80670000 00000000 00000000 00000009
>>     [    0.365610]         00000000 8fc94e20 8fc0de30 80690000 
>> 00000018 803592dc 00000000 806d0000
>>     [    0.365627]         ...
>>     [    0.365634] Call Trace:
>>     [    0.365647] [<8001b9a0>] show_stack+0x6c/0x12c
>>     [    0.365663] [<804aed20>] dump_stack+0x98/0xc8
>>     [    0.365673] [<8003044c>] __warn+0xc4/0xe8
>>     [    0.365682] [<800304f4>] warn_slowpath_fmt+0x84/0xb8
>>     [    0.365690] [<800a886c>] smp_call_function_single+0x110/0x200
>>     [    0.365703] ---[ end trace 5785856ca39c79d5 ]---
>>
>>     v2->v3:
>>     No Change.
>>
>>     v3->v4:
>>     Rebase on top of kernel 5.6-rc1.
>>
>>     v4->v5:
>>     Move the check for (evt->event_handler) from 
>> "ingenic_per_cpu_event_handler"
>>     to "ingenic_tcu_cevt_cb".
>>
>>     v5->v6:
>>     No change.
>>
>>  drivers/clocksource/ingenic-timer.c | 113 
>> +++++++++++++++++++++++++++++-------
>>  1 file changed, 91 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/clocksource/ingenic-timer.c 
>> b/drivers/clocksource/ingenic-timer.c
>> index 4bbdb3d..e396326 100644
>> --- a/drivers/clocksource/ingenic-timer.c
>> +++ b/drivers/clocksource/ingenic-timer.c
>> @@ -1,7 +1,8 @@
>>  // SPDX-License-Identifier: GPL-2.0
>>  /*
>> - * JZ47xx SoCs TCU IRQ driver
>> + * XBurst SoCs TCU IRQ driver
>>   * Copyright (C) 2019 Paul Cercueil <paul@crapouillou.net>
>> + * Copyright (C) 2020 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>>   */
>>
>>  #include <linux/bitops.h>
>> @@ -21,18 +22,23 @@
>>
>>  #include <dt-bindings/clock/ingenic,tcu.h>
>>
>> +static DEFINE_PER_CPU(call_single_data_t, ingenic_cevt_csd);
>> +
>>  struct ingenic_soc_info {
>>      unsigned int num_channels;
>>  };
>>
>>  struct ingenic_tcu {
>>      struct regmap *map;
>> +    struct device_node *np;
>>      struct clk *timer_clk, *cs_clk;
>> +    unsigned int timer_local[NR_CPUS];
>>      unsigned int timer_channel, cs_channel;
>>      struct clock_event_device cevt;
>>      struct clocksource cs;
>> -    char name[4];
>> +    char name[8];
>>      unsigned long pwm_channels_mask;
>> +    int cpu;
>>  };
>>
>>  static struct ingenic_tcu *ingenic_tcu;
>> @@ -81,6 +87,24 @@ static int ingenic_tcu_cevt_set_next(unsigned long 
>> next,
>>      return 0;
>>  }
>>
>> +static void ingenic_per_cpu_event_handler(void *info)
>> +{
>> +    struct clock_event_device *cevt = (struct clock_event_device *) 
>> info;
>> +
>> +    cevt->event_handler(cevt);
>> +}
>> +
>> +static void ingenic_tcu_per_cpu_cb(struct clock_event_device *evt)
>> +{
>> +    struct ingenic_tcu *tcu = to_ingenic_tcu(evt);
>> +    call_single_data_t *csd;
>> +
>> +    csd = &per_cpu(ingenic_cevt_csd, tcu->cpu);
>> +    csd->info = (void *) evt;
>> +    csd->func = ingenic_per_cpu_event_handler;
>> +    smp_call_function_single_async(tcu->cpu, csd);
>> +}
>> +
>>  static irqreturn_t ingenic_tcu_cevt_cb(int irq, void *dev_id)
>>  {
>>      struct clock_event_device *evt = dev_id;
>> @@ -89,7 +113,7 @@ static irqreturn_t ingenic_tcu_cevt_cb(int irq, 
>> void *dev_id)
>>      regmap_write(tcu->map, TCU_REG_TECR, BIT(tcu->timer_channel));
>>
>>      if (evt->event_handler)
>> -        evt->event_handler(evt);
>> +        ingenic_tcu_per_cpu_cb(evt);
>>
>>      return IRQ_HANDLED;
>>  }
>> @@ -105,14 +129,21 @@ static struct clk * __init 
>> ingenic_tcu_get_clock(struct device_node *np, int id)
>>      return of_clk_get_from_provider(&args);
>>  }
>>
>> -static int __init ingenic_tcu_timer_init(struct device_node *np,
>> -                     struct ingenic_tcu *tcu)
>> +static int ingenic_tcu_setup_per_cpu_cevt(struct device_node *np,
>> +                     unsigned int channel)
>>  {
>> -    unsigned int timer_virq, channel = tcu->timer_channel;
>> +    unsigned int timer_virq;
>>      struct irq_domain *domain;
>> +    struct ingenic_tcu *tcu;
>>      unsigned long rate;
>>      int err;
>>
>> +    tcu = kzalloc(sizeof(*tcu), GFP_KERNEL);
>> +    if (!tcu)
>> +        return -ENOMEM;
>> +
>> +    tcu->map = ingenic_tcu->map;
>> +
>>      tcu->timer_clk = ingenic_tcu_get_clock(np, channel);
>>      if (IS_ERR(tcu->timer_clk))
>>          return PTR_ERR(tcu->timer_clk);
>> @@ -139,13 +170,15 @@ static int __init ingenic_tcu_timer_init(struct 
>> device_node *np,
>>          goto err_clk_disable;
>>      }
>>
>> -    snprintf(tcu->name, sizeof(tcu->name), "TCU");
>> +    snprintf(tcu->name, sizeof(tcu->name), "TCU%u", channel);
>>
>>      err = request_irq(timer_virq, ingenic_tcu_cevt_cb, IRQF_TIMER,
>>                tcu->name, &tcu->cevt);
>>      if (err)
>>          goto err_irq_dispose_mapping;
>>
>> +    tcu->cpu = smp_processor_id();
>> +    tcu->timer_channel = channel;
>>      tcu->cevt.cpumask = cpumask_of(smp_processor_id());
>>      tcu->cevt.features = CLOCK_EVT_FEAT_ONESHOT;
>>      tcu->cevt.name = tcu->name;
>> @@ -166,6 +199,25 @@ static int __init ingenic_tcu_timer_init(struct 
>> device_node *np,
>>      return err;
>>  }
>>
>> +static int ingenic_tcu_setup_cevt(unsigned int cpu)
>> +{
>> +    int ret;
>> +
>> +    ret = ingenic_tcu_setup_per_cpu_cevt(ingenic_tcu->np,
>> +                        ingenic_tcu->timer_local[cpu]);
>> +    if (ret)
>> +        goto err_tcu_clocksource_cleanup;
>> +
>> +    return 0;
>> +
>> +err_tcu_clocksource_cleanup:
>> +    clocksource_unregister(&ingenic_tcu->cs);
>> +    clk_disable_unprepare(ingenic_tcu->cs_clk);
>> +    clk_put(ingenic_tcu->cs_clk);
>> +    kfree(ingenic_tcu);
>> +    return ret;
>> +}
>> +
>>  static int __init ingenic_tcu_clocksource_init(struct device_node *np,
>>                             struct ingenic_tcu *tcu)
>>  {
>> @@ -239,6 +291,7 @@ static int __init ingenic_tcu_init(struct 
>> device_node *np)
>>      const struct ingenic_soc_info *soc_info = id->data;
>>      struct ingenic_tcu *tcu;
>>      struct regmap *map;
>> +    unsigned cpu = 0;
>>      long rate;
>>      int ret;
>>
>> @@ -252,13 +305,18 @@ static int __init ingenic_tcu_init(struct 
>> device_node *np)
>>      if (!tcu)
>>          return -ENOMEM;
>>
>> -    /* Enable all TCU channels for PWM use by default except 
>> channels 0/1 */
>> -    tcu->pwm_channels_mask = GENMASK(soc_info->num_channels - 1, 2);
>> +    /*
>> +     * Enable all TCU channels for PWM use by default except 
>> channels 0/1,
>> +     * and channel 2 if target CPU is JZ4780 and SMP is selected.
>> +     */
>> +    tcu->pwm_channels_mask = GENMASK(soc_info->num_channels - 1,
>> +                                NR_CPUS + 1);
>>      of_property_read_u32(np, "ingenic,pwm-channels-mask",
>>                   (u32 *)&tcu->pwm_channels_mask);
>>
>> -    /* Verify that we have at least two free channels */
>> -    if (hweight8(tcu->pwm_channels_mask) > soc_info->num_channels - 
>> 2) {
>> +    /* Verify that we have at least NR_CPUS + 1 free channels */
>> +    if (hweight8(tcu->pwm_channels_mask) >
>> +            soc_info->num_channels - NR_CPUS + 1) {
>
> NR_CPUS can be up to 256. You want to use num_online_cpus() here, I 
> believe.

Sure, will change in next version.

>
>>          pr_crit("%s: Invalid PWM channel mask: 0x%02lx\n", __func__,
>>              tcu->pwm_channels_mask);
>>          ret = -EINVAL;
>> @@ -266,13 +324,27 @@ static int __init ingenic_tcu_init(struct 
>> device_node *np)
>>      }
>>
>>      tcu->map = map;
>> +    tcu->np = np;
>>      ingenic_tcu = tcu;
>>
>> -    tcu->timer_channel = find_first_zero_bit(&tcu->pwm_channels_mask,
>> +    tcu->timer_local[cpu] = 
>> find_first_zero_bit(&tcu->pwm_channels_mask,
>>                           soc_info->num_channels);
>> -    tcu->cs_channel = find_next_zero_bit(&tcu->pwm_channels_mask,
>> -                         soc_info->num_channels,
>> -                         tcu->timer_channel + 1);
>> +
>> +    if (NR_CPUS > 1) {
>> +        for (cpu = 1; cpu < NR_CPUS; cpu++)
>> +            tcu->timer_local[cpu] = find_next_zero_bit(
>> +                        &tcu->pwm_channels_mask,
>> +                        soc_info->num_channels,
>> +                        tcu->timer_local[cpu - 1] + 1);
>> +
>> +        tcu->cs_channel = find_next_zero_bit(&tcu->pwm_channels_mask,
>> +                    soc_info->num_channels,
>> +                    tcu->timer_local[cpu-1] + 1);
>> +    } else {
>> +        tcu->cs_channel = find_next_zero_bit(&tcu->pwm_channels_mask,
>> +                    soc_info->num_channels,
>> +                    tcu->timer_local[cpu] + 1);
>> +    }
>
> I believe you can factorize the code a bit here - don't check for 
> NR_CPUS > 1, and have a for (cpu = 0; cpu < num_online_cpus(); cpu++) 
> loop. You could use a temporary variable to store the 'next bit' value.

Sure.

>
> -Paul
>
>>
>>      ret = ingenic_tcu_clocksource_init(np, tcu);
>>      if (ret) {
>> @@ -280,9 +352,10 @@ static int __init ingenic_tcu_init(struct 
>> device_node *np)
>>          goto err_free_ingenic_tcu;
>>      }
>>
>> -    ret = ingenic_tcu_timer_init(np, tcu);
>> -    if (ret)
>> -        goto err_tcu_clocksource_cleanup;
>> +    /* Setup clock events on each CPU core */
>> +    ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "Ingenic XBurst: 
>> online",
>> +                ingenic_tcu_setup_cevt, NULL);
>> +    WARN_ON(ret < 0);
>>
>>      /* Register the sched_clock at the end as there's no way to undo 
>> it */
>>      rate = clk_get_rate(tcu->cs_clk);
>> @@ -290,10 +363,6 @@ static int __init ingenic_tcu_init(struct 
>> device_node *np)
>>
>>      return 0;
>>
>> -err_tcu_clocksource_cleanup:
>> -    clocksource_unregister(&tcu->cs);
>> -    clk_disable_unprepare(tcu->cs_clk);
>> -    clk_put(tcu->cs_clk);
>>  err_free_ingenic_tcu:
>>      kfree(tcu);
>>      return ret;
>> -- 
>> 2.7.4
>>
>

[PATCH v6 5/7] dt-bindings: MIPS: Document Ingenic SoCs binding.

[周琰杰 (Zhou Yanjie)]

Document the available properties for the SoC root node and the
CPU nodes of the devicetree for the Ingenic XBurst SoCs.

Tested-by: H. Nikolaus Schaller <hns@goldelico.com>
Tested-by: Paul Boddie <paul@boddie.org.uk>
Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
---

Notes:
    v1->v2:
    Change the two Document from txt to yaml.
    
    v2->v3:
    Fix formatting errors.
    
    v3->v4:
    Fix bugs in the two yaml files.
    
    v4->v5:
    No change.
    
    v5->v6:
    Rewrite the two yaml files.

 .../bindings/mips/ingenic/ingenic,cpu.yaml         | 61 ++++++++++++++++++++++
 .../bindings/mips/ingenic/ingenic,soc.yaml         | 34 ++++++++++++
 2 files changed, 95 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml
 create mode 100644 Documentation/devicetree/bindings/mips/ingenic/ingenic,soc.yaml

diff --git a/Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml b/Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml
new file mode 100644
index 00000000..ad1fd86
--- /dev/null
+++ b/Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml
@@ -0,0 +1,61 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mips/ingenic/ingenic,cpu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Bindings for Ingenic XBurst family CPUs
+
+maintainers:
+  - 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
+description: |
+  Ingenic XBurst family CPUs shall have the following properties.
+
+properties:
+  compatible:
+    oneOf:
+
+      - description: Ingenic XBurst®1 CPU Core
+        items:
+          - const: ingenic,xburst
+
+      - description: Ingenic XBurst®2 CPU Core
+        items:
+          - const: ingenic,xburst2
+
+  reg:
+    description: |
+      The number of the CPU.
+
+required:
+  - device_type
+  - compatible
+  - reg
+
+examples:
+  - |
+    #include <dt-bindings/clock/jz4780-cgu.h>
+
+    cpus {
+    	#address-cells = <1>;
+    	#size-cells = <0>;
+
+    	cpu0: cpu@0 {
+    		device_type = "cpu";
+    		compatible = "ingenic,xburst";
+    		reg = <0>;
+
+    		clocks = <&cgu JZ4780_CLK_CPU>;
+    		clock-names = "cpu";
+    	};
+
+    	cpu1: cpu@1 {
+    		device_type = "cpu";
+    		compatible = "ingenic,xburst";
+    		reg = <1>;
+
+    		clocks = <&cgu JZ4780_CLK_CORE1>;
+    		clock-names = "cpu";
+    	};
+    };
+...
diff --git a/Documentation/devicetree/bindings/mips/ingenic/ingenic,soc.yaml b/Documentation/devicetree/bindings/mips/ingenic/ingenic,soc.yaml
new file mode 100644
index 00000000..8943e73
--- /dev/null
+++ b/Documentation/devicetree/bindings/mips/ingenic/ingenic,soc.yaml
@@ -0,0 +1,34 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mips/ingenic/ingenic,soc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Bindings for Ingenic SoCs with XBurst CPU inside.
+
+maintainers:
+  - 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
+description: |
+  Ingenic SoCs with XBurst CPU inside shall have the following properties.
+
+properties:
+  $nodename:
+    const: '/'
+  compatible:
+    oneOf:
+
+      - description: Ingenic JZ47 Series Mobile Application Processor
+        items:
+          - const: ingenic,jz4740
+          - const: ingenic,jz4725b
+          - const: ingenic,jz4760
+          - const: ingenic,jz4760b
+          - const: ingenic,jz4770
+          - const: ingenic,jz4780
+
+      - description: Ingenic X Series IoT Application Processor
+        items:
+          - const: ingenic,x1000
+          - const: ingenic,x1000e
+          - const: ingenic,x1500
+...
-- 
2.7.4

Re: [PATCH v6 5/7] dt-bindings: MIPS: Document Ingenic SoCs binding.

[Rob Herring]

On Fri, Feb 21, 2020 at 12:24:47AM +0800, 周琰杰 (Zhou Yanjie) wrote:
> Document the available properties for the SoC root node and the
> CPU nodes of the devicetree for the Ingenic XBurst SoCs.
> 
> Tested-by: H. Nikolaus Schaller <hns@goldelico.com>
> Tested-by: Paul Boddie <paul@boddie.org.uk>
> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> ---
> 
> Notes:
>     v1->v2:
>     Change the two Document from txt to yaml.
>     
>     v2->v3:
>     Fix formatting errors.
>     
>     v3->v4:
>     Fix bugs in the two yaml files.
>     
>     v4->v5:
>     No change.
>     
>     v5->v6:
>     Rewrite the two yaml files.
> 
>  .../bindings/mips/ingenic/ingenic,cpu.yaml         | 61 ++++++++++++++++++++++
>  .../bindings/mips/ingenic/ingenic,soc.yaml         | 34 ++++++++++++
>  2 files changed, 95 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml
>  create mode 100644 Documentation/devicetree/bindings/mips/ingenic/ingenic,soc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml b/Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml
> new file mode 100644
> index 00000000..ad1fd86
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml
> @@ -0,0 +1,61 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mips/ingenic/ingenic,cpu.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Bindings for Ingenic XBurst family CPUs
> +
> +maintainers:
> +  - 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>

Blank line here.

> +description: |

Drop the '|'.

> +  Ingenic XBurst family CPUs shall have the following properties.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +
> +      - description: Ingenic XBurst®1 CPU Core
> +        items:
> +          - const: ingenic,xburst
> +
> +      - description: Ingenic XBurst®2 CPU Core
> +        items:
> +          - const: ingenic,xburst2

enum:
  - ingenic,xburst  # Ingenic XBurst®1 CPU Core
  - ingenic,xburst2 # Ingenic XBurst®2 CPU Core

Though I don't find the description really adds much.

> +
> +  reg:
> +    description: |
> +      The number of the CPU.

Drop this.

Add:

maxItems: 1

> +
> +required:
> +  - device_type
> +  - compatible
> +  - reg
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/jz4780-cgu.h>
> +
> +    cpus {
> +    	#address-cells = <1>;
> +    	#size-cells = <0>;
> +
> +    	cpu0: cpu@0 {
> +    		device_type = "cpu";
> +    		compatible = "ingenic,xburst";
> +    		reg = <0>;
> +

> +    		clocks = <&cgu JZ4780_CLK_CPU>;
> +    		clock-names = "cpu";

Not documented.

> +    	};
> +
> +    	cpu1: cpu@1 {
> +    		device_type = "cpu";
> +    		compatible = "ingenic,xburst";
> +    		reg = <1>;
> +
> +    		clocks = <&cgu JZ4780_CLK_CORE1>;
> +    		clock-names = "cpu";
> +    	};
> +    };
> +...
> diff --git a/Documentation/devicetree/bindings/mips/ingenic/ingenic,soc.yaml b/Documentation/devicetree/bindings/mips/ingenic/ingenic,soc.yaml
> new file mode 100644
> index 00000000..8943e73
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mips/ingenic/ingenic,soc.yaml
> @@ -0,0 +1,34 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mips/ingenic/ingenic,soc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Bindings for Ingenic SoCs with XBurst CPU inside.
> +
> +maintainers:
> +  - 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>

Blank line.

> +description: |
> +  Ingenic SoCs with XBurst CPU inside shall have the following properties.
> +
> +properties:
> +  $nodename:
> +    const: '/'
> +  compatible:
> +    oneOf:
> +
> +      - description: Ingenic JZ47 Series Mobile Application Processor
> +        items:
> +          - const: ingenic,jz4740
> +          - const: ingenic,jz4725b
> +          - const: ingenic,jz4760
> +          - const: ingenic,jz4760b
> +          - const: ingenic,jz4770
> +          - const: ingenic,jz4780

This is defining the root compatible is 6 strings. You want a enum here 
I think.

> +
> +      - description: Ingenic X Series IoT Application Processor
> +        items:
> +          - const: ingenic,x1000
> +          - const: ingenic,x1000e
> +          - const: ingenic,x1500

Same here.

Did you validate your dts file with this schema using 'make dtbs_check'?

Rob

Re: [PATCH v6 5/7] dt-bindings: MIPS: Document Ingenic SoCs binding.

[Paul Cercueil]

Hi,

Le mer., févr. 26, 2020 at 10:29, Rob Herring <robh@kernel.org> a 
écrit :
> On Fri, Feb 21, 2020 at 12:24:47AM +0800, 周琰杰 (Zhou Yanjie) 
> wrote:
>>  Document the available properties for the SoC root node and the
>>  CPU nodes of the devicetree for the Ingenic XBurst SoCs.
>> 
>>  Tested-by: H. Nikolaus Schaller <hns@goldelico.com>
>>  Tested-by: Paul Boddie <paul@boddie.org.uk>
>>  Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>>  ---
>> 
>>  Notes:
>>      v1->v2:
>>      Change the two Document from txt to yaml.
>> 
>>      v2->v3:
>>      Fix formatting errors.
>> 
>>      v3->v4:
>>      Fix bugs in the two yaml files.
>> 
>>      v4->v5:
>>      No change.
>> 
>>      v5->v6:
>>      Rewrite the two yaml files.
>> 
>>   .../bindings/mips/ingenic/ingenic,cpu.yaml         | 61 
>> ++++++++++++++++++++++
>>   .../bindings/mips/ingenic/ingenic,soc.yaml         | 34 
>> ++++++++++++
>>   2 files changed, 95 insertions(+)
>>   create mode 100644 
>> Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml
>>   create mode 100644 
>> Documentation/devicetree/bindings/mips/ingenic/ingenic,soc.yaml
>> 
>>  diff --git 
>> a/Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml 
>> b/Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml
>>  new file mode 100644
>>  index 00000000..ad1fd86
>>  --- /dev/null
>>  +++ 
>> b/Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml
>>  @@ -0,0 +1,61 @@
>>  +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>  +%YAML 1.2
>>  +---
>>  +$id: http://devicetree.org/schemas/mips/ingenic/ingenic,cpu.yaml#
>>  +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>  +
>>  +title: Bindings for Ingenic XBurst family CPUs
>>  +
>>  +maintainers:
>>  +  - 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> 
> Blank line here.
> 
>>  +description: |
> 
> Drop the '|'.
> 
>>  +  Ingenic XBurst family CPUs shall have the following properties.
>>  +
>>  +properties:
>>  +  compatible:
>>  +    oneOf:
>>  +
>>  +      - description: Ingenic XBurst®1 CPU Core
>>  +        items:
>>  +          - const: ingenic,xburst
>>  +
>>  +      - description: Ingenic XBurst®2 CPU Core
>>  +        items:
>>  +          - const: ingenic,xburst2
> 
> enum:
>   - ingenic,xburst  # Ingenic XBurst®1 CPU Core
>   - ingenic,xburst2 # Ingenic XBurst®2 CPU Core
> 
> Though I don't find the description really adds much.

About the enum values: shouldn't they be a bit more descriptive? There 
has been various versions of the Xburst1 chip, with slightly different 
instruction sets and hardware (FPU).

-Paul

>>  +
>>  +  reg:
>>  +    description: |
>>  +      The number of the CPU.
> 
> Drop this.
> 
> Add:
> 
> maxItems: 1
> 
>>  +
>>  +required:
>>  +  - device_type
>>  +  - compatible
>>  +  - reg
>>  +
>>  +examples:
>>  +  - |
>>  +    #include <dt-bindings/clock/jz4780-cgu.h>
>>  +
>>  +    cpus {
>>  +    	#address-cells = <1>;
>>  +    	#size-cells = <0>;
>>  +
>>  +    	cpu0: cpu@0 {
>>  +    		device_type = "cpu";
>>  +    		compatible = "ingenic,xburst";
>>  +    		reg = <0>;
>>  +
> 
>>  +    		clocks = <&cgu JZ4780_CLK_CPU>;
>>  +    		clock-names = "cpu";
> 
> Not documented.
> 
>>  +    	};
>>  +
>>  +    	cpu1: cpu@1 {
>>  +    		device_type = "cpu";
>>  +    		compatible = "ingenic,xburst";
>>  +    		reg = <1>;
>>  +
>>  +    		clocks = <&cgu JZ4780_CLK_CORE1>;
>>  +    		clock-names = "cpu";
>>  +    	};
>>  +    };
>>  +...
>>  diff --git 
>> a/Documentation/devicetree/bindings/mips/ingenic/ingenic,soc.yaml 
>> b/Documentation/devicetree/bindings/mips/ingenic/ingenic,soc.yaml
>>  new file mode 100644
>>  index 00000000..8943e73
>>  --- /dev/null
>>  +++ 
>> b/Documentation/devicetree/bindings/mips/ingenic/ingenic,soc.yaml
>>  @@ -0,0 +1,34 @@
>>  +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>  +%YAML 1.2
>>  +---
>>  +$id: http://devicetree.org/schemas/mips/ingenic/ingenic,soc.yaml#
>>  +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>  +
>>  +title: Bindings for Ingenic SoCs with XBurst CPU inside.
>>  +
>>  +maintainers:
>>  +  - 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> 
> Blank line.
> 
>>  +description: |
>>  +  Ingenic SoCs with XBurst CPU inside shall have the following 
>> properties.
>>  +
>>  +properties:
>>  +  $nodename:
>>  +    const: '/'
>>  +  compatible:
>>  +    oneOf:
>>  +
>>  +      - description: Ingenic JZ47 Series Mobile Application 
>> Processor
>>  +        items:
>>  +          - const: ingenic,jz4740
>>  +          - const: ingenic,jz4725b
>>  +          - const: ingenic,jz4760
>>  +          - const: ingenic,jz4760b
>>  +          - const: ingenic,jz4770
>>  +          - const: ingenic,jz4780
> 
> This is defining the root compatible is 6 strings. You want a enum 
> here
> I think.
> 
>>  +
>>  +      - description: Ingenic X Series IoT Application Processor
>>  +        items:
>>  +          - const: ingenic,x1000
>>  +          - const: ingenic,x1000e
>>  +          - const: ingenic,x1500
> 
> Same here.
> 
> Did you validate your dts file with this schema using 'make 
> dtbs_check'?
> 
> Rob

Re: [PATCH v6 5/7] dt-bindings: MIPS: Document Ingenic SoCs binding.

[Zhou Yanjie]

Hi Paul,

On 2020年02月27日 21:48, Paul Cercueil wrote:
> Hi,
>
> Le mer., févr. 26, 2020 at 10:29, Rob Herring <robh@kernel.org> a écrit :
>> On Fri, Feb 21, 2020 at 12:24:47AM +0800, 周琰杰 (Zhou Yanjie) wrote:
>>>  Document the available properties for the SoC root node and the
>>>  CPU nodes of the devicetree for the Ingenic XBurst SoCs.
>>>
>>>  Tested-by: H. Nikolaus Schaller <hns@goldelico.com>
>>>  Tested-by: Paul Boddie <paul@boddie.org.uk>
>>>  Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>>>  ---
>>>
>>>  Notes:
>>>      v1->v2:
>>>      Change the two Document from txt to yaml.
>>>
>>>      v2->v3:
>>>      Fix formatting errors.
>>>
>>>      v3->v4:
>>>      Fix bugs in the two yaml files.
>>>
>>>      v4->v5:
>>>      No change.
>>>
>>>      v5->v6:
>>>      Rewrite the two yaml files.
>>>
>>>   .../bindings/mips/ingenic/ingenic,cpu.yaml         | 61 
>>> ++++++++++++++++++++++
>>>   .../bindings/mips/ingenic/ingenic,soc.yaml         | 34 ++++++++++++
>>>   2 files changed, 95 insertions(+)
>>>   create mode 100644 
>>> Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml
>>>   create mode 100644 
>>> Documentation/devicetree/bindings/mips/ingenic/ingenic,soc.yaml
>>>
>>>  diff --git 
>>> a/Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml 
>>> b/Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml
>>>  new file mode 100644
>>>  index 00000000..ad1fd86
>>>  --- /dev/null
>>>  +++ b/Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml
>>>  @@ -0,0 +1,61 @@
>>>  +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>  +%YAML 1.2
>>>  +---
>>>  +$id: http://devicetree.org/schemas/mips/ingenic/ingenic,cpu.yaml#
>>>  +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>  +
>>>  +title: Bindings for Ingenic XBurst family CPUs
>>>  +
>>>  +maintainers:
>>>  +  - 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>>
>> Blank line here.
>>
>>>  +description: |
>>
>> Drop the '|'.
>>
>>>  +  Ingenic XBurst family CPUs shall have the following properties.
>>>  +
>>>  +properties:
>>>  +  compatible:
>>>  +    oneOf:
>>>  +
>>>  +      - description: Ingenic XBurst®1 CPU Core
>>>  +        items:
>>>  +          - const: ingenic,xburst
>>>  +
>>>  +      - description: Ingenic XBurst®2 CPU Core
>>>  +        items:
>>>  +          - const: ingenic,xburst2
>>
>> enum:
>>   - ingenic,xburst  # Ingenic XBurst®1 CPU Core
>>   - ingenic,xburst2 # Ingenic XBurst®2 CPU Core
>>
>> Though I don't find the description really adds much.
>
> About the enum values: shouldn't they be a bit more descriptive? There 
> has been various versions of the Xburst1 chip, with slightly different 
> instruction sets and hardware (FPU).

Sure, will change in next version.

>
> -Paul
>
>>>  +
>>>  +  reg:
>>>  +    description: |
>>>  +      The number of the CPU.
>>
>> Drop this.
>>
>> Add:
>>
>> maxItems: 1
>>
>>>  +
>>>  +required:
>>>  +  - device_type
>>>  +  - compatible
>>>  +  - reg
>>>  +
>>>  +examples:
>>>  +  - |
>>>  +    #include <dt-bindings/clock/jz4780-cgu.h>
>>>  +
>>>  +    cpus {
>>>  +        #address-cells = <1>;
>>>  +        #size-cells = <0>;
>>>  +
>>>  +        cpu0: cpu@0 {
>>>  +            device_type = "cpu";
>>>  +            compatible = "ingenic,xburst";
>>>  +            reg = <0>;
>>>  +
>>
>>>  +            clocks = <&cgu JZ4780_CLK_CPU>;
>>>  +            clock-names = "cpu";
>>
>> Not documented.
>>
>>>  +        };
>>>  +
>>>  +        cpu1: cpu@1 {
>>>  +            device_type = "cpu";
>>>  +            compatible = "ingenic,xburst";
>>>  +            reg = <1>;
>>>  +
>>>  +            clocks = <&cgu JZ4780_CLK_CORE1>;
>>>  +            clock-names = "cpu";
>>>  +        };
>>>  +    };
>>>  +...
>>>  diff --git 
>>> a/Documentation/devicetree/bindings/mips/ingenic/ingenic,soc.yaml 
>>> b/Documentation/devicetree/bindings/mips/ingenic/ingenic,soc.yaml
>>>  new file mode 100644
>>>  index 00000000..8943e73
>>>  --- /dev/null
>>>  +++ b/Documentation/devicetree/bindings/mips/ingenic/ingenic,soc.yaml
>>>  @@ -0,0 +1,34 @@
>>>  +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>  +%YAML 1.2
>>>  +---
>>>  +$id: http://devicetree.org/schemas/mips/ingenic/ingenic,soc.yaml#
>>>  +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>  +
>>>  +title: Bindings for Ingenic SoCs with XBurst CPU inside.
>>>  +
>>>  +maintainers:
>>>  +  - 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>>
>> Blank line.
>>
>>>  +description: |
>>>  +  Ingenic SoCs with XBurst CPU inside shall have the following 
>>> properties.
>>>  +
>>>  +properties:
>>>  +  $nodename:
>>>  +    const: '/'
>>>  +  compatible:
>>>  +    oneOf:
>>>  +
>>>  +      - description: Ingenic JZ47 Series Mobile Application Processor
>>>  +        items:
>>>  +          - const: ingenic,jz4740
>>>  +          - const: ingenic,jz4725b
>>>  +          - const: ingenic,jz4760
>>>  +          - const: ingenic,jz4760b
>>>  +          - const: ingenic,jz4770
>>>  +          - const: ingenic,jz4780
>>
>> This is defining the root compatible is 6 strings. You want a enum here
>> I think.
>>
>>>  +
>>>  +      - description: Ingenic X Series IoT Application Processor
>>>  +        items:
>>>  +          - const: ingenic,x1000
>>>  +          - const: ingenic,x1000e
>>>  +          - const: ingenic,x1500
>>
>> Same here.
>>
>> Did you validate your dts file with this schema using 'make dtbs_check'?
>>
>> Rob
>

Re: [PATCH v6 5/7] dt-bindings: MIPS: Document Ingenic SoCs binding.

[Zhou Yanjie]

Hi Rob,

On 2020年02月27日 00:29, Rob Herring wrote:
> On Fri, Feb 21, 2020 at 12:24:47AM +0800, 周琰杰 (Zhou Yanjie) wrote:
>> Document the available properties for the SoC root node and the
>> CPU nodes of the devicetree for the Ingenic XBurst SoCs.
>>
>> Tested-by: H. Nikolaus Schaller <hns@goldelico.com>
>> Tested-by: Paul Boddie <paul@boddie.org.uk>
>> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>> ---
>>
>> Notes:
>>      v1->v2:
>>      Change the two Document from txt to yaml.
>>      
>>      v2->v3:
>>      Fix formatting errors.
>>      
>>      v3->v4:
>>      Fix bugs in the two yaml files.
>>      
>>      v4->v5:
>>      No change.
>>      
>>      v5->v6:
>>      Rewrite the two yaml files.
>>
>>   .../bindings/mips/ingenic/ingenic,cpu.yaml         | 61 ++++++++++++++++++++++
>>   .../bindings/mips/ingenic/ingenic,soc.yaml         | 34 ++++++++++++
>>   2 files changed, 95 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml
>>   create mode 100644 Documentation/devicetree/bindings/mips/ingenic/ingenic,soc.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml b/Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml
>> new file mode 100644
>> index 00000000..ad1fd86
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml
>> @@ -0,0 +1,61 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/mips/ingenic/ingenic,cpu.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Bindings for Ingenic XBurst family CPUs
>> +
>> +maintainers:
>> +  - 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> Blank line here.
>
>> +description: |
> Drop the '|'.
>
>> +  Ingenic XBurst family CPUs shall have the following properties.
>> +
>> +properties:
>> +  compatible:
>> +    oneOf:
>> +
>> +      - description: Ingenic XBurst®1 CPU Core
>> +        items:
>> +          - const: ingenic,xburst
>> +
>> +      - description: Ingenic XBurst®2 CPU Core
>> +        items:
>> +          - const: ingenic,xburst2
> enum:
>    - ingenic,xburst  # Ingenic XBurst®1 CPU Core
>    - ingenic,xburst2 # Ingenic XBurst®2 CPU Core
>
> Though I don't find the description really adds much.
>
>> +
>> +  reg:
>> +    description: |
>> +      The number of the CPU.
> Drop this.
>
> Add:
>
> maxItems: 1
>
>> +
>> +required:
>> +  - device_type
>> +  - compatible
>> +  - reg
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/jz4780-cgu.h>
>> +
>> +    cpus {
>> +    	#address-cells = <1>;
>> +    	#size-cells = <0>;
>> +
>> +    	cpu0: cpu@0 {
>> +    		device_type = "cpu";
>> +    		compatible = "ingenic,xburst";
>> +    		reg = <0>;
>> +
>> +    		clocks = <&cgu JZ4780_CLK_CPU>;
>> +    		clock-names = "cpu";
> Not documented.
>
>> +    	};
>> +
>> +    	cpu1: cpu@1 {
>> +    		device_type = "cpu";
>> +    		compatible = "ingenic,xburst";
>> +    		reg = <1>;
>> +
>> +    		clocks = <&cgu JZ4780_CLK_CORE1>;
>> +    		clock-names = "cpu";
>> +    	};
>> +    };
>> +...
>> diff --git a/Documentation/devicetree/bindings/mips/ingenic/ingenic,soc.yaml b/Documentation/devicetree/bindings/mips/ingenic/ingenic,soc.yaml
>> new file mode 100644
>> index 00000000..8943e73
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mips/ingenic/ingenic,soc.yaml
>> @@ -0,0 +1,34 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/mips/ingenic/ingenic,soc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Bindings for Ingenic SoCs with XBurst CPU inside.
>> +
>> +maintainers:
>> +  - 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> Blank line.
>
>> +description: |
>> +  Ingenic SoCs with XBurst CPU inside shall have the following properties.
>> +
>> +properties:
>> +  $nodename:
>> +    const: '/'
>> +  compatible:
>> +    oneOf:
>> +
>> +      - description: Ingenic JZ47 Series Mobile Application Processor
>> +        items:
>> +          - const: ingenic,jz4740
>> +          - const: ingenic,jz4725b
>> +          - const: ingenic,jz4760
>> +          - const: ingenic,jz4760b
>> +          - const: ingenic,jz4770
>> +          - const: ingenic,jz4780
> This is defining the root compatible is 6 strings. You want a enum here
> I think.
>
>> +
>> +      - description: Ingenic X Series IoT Application Processor
>> +        items:
>> +          - const: ingenic,x1000
>> +          - const: ingenic,x1000e
>> +          - const: ingenic,x1500
> Same here.
>
> Did you validate your dts file with this schema using 'make dtbs_check'?

I am very sorry, to be honest, because I was not familiar with yaml before,
the previous versions have not been verified, but when v6, I have set up
the environment and checked it with "make dt_binding_check", but I
don't know why the problems you pointed out above were not prompted
during the check. I will fix these problems in the next version.

Thanks and best regards!

>
> Rob

[PATCH v6 6/7] MIPS: Ingenic: Add 'cpus' node for Ingenic SoCs.

[周琰杰 (Zhou Yanjie)]

Add 'cpus' node to the jz4740.dtsi, jz4770.dtsi, jz4780.dtsi
and x1000.dtsi files.

Tested-by: H. Nikolaus Schaller <hns@goldelico.com>
Tested-by: Paul Boddie <paul@boddie.org.uk>
Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
---

Notes:
    v1->v2:
    No change.
    
    v2->v3:
    No change.
    
    v3->v4:
    Rebase on top of kernel 5.6-rc1.
    
    v4->v5:
    No change.
    
    v5->v6:
    No change.

 arch/mips/boot/dts/ingenic/jz4740.dtsi | 14 ++++++++++++++
 arch/mips/boot/dts/ingenic/jz4770.dtsi | 15 ++++++++++++++-
 arch/mips/boot/dts/ingenic/jz4780.dtsi | 23 +++++++++++++++++++++++
 arch/mips/boot/dts/ingenic/x1000.dtsi  | 14 ++++++++++++++
 4 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/arch/mips/boot/dts/ingenic/jz4740.dtsi b/arch/mips/boot/dts/ingenic/jz4740.dtsi
index 5accda2..9627d95 100644
--- a/arch/mips/boot/dts/ingenic/jz4740.dtsi
+++ b/arch/mips/boot/dts/ingenic/jz4740.dtsi
@@ -6,6 +6,20 @@
 	#size-cells = <1>;
 	compatible = "ingenic,jz4740";
 
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		cpu0: cpu@0 {
+			device_type = "cpu";
+			compatible = "ingenic,xburst";
+			reg = <0>;
+
+			clocks = <&cgu JZ4740_CLK_CCLK>;
+			clock-names = "cpu";
+		};
+	};
+
 	cpuintc: interrupt-controller {
 		#address-cells = <0>;
 		#interrupt-cells = <1>;
diff --git a/arch/mips/boot/dts/ingenic/jz4770.dtsi b/arch/mips/boot/dts/ingenic/jz4770.dtsi
index 0bfb9ed..1b8114d 100644
--- a/arch/mips/boot/dts/ingenic/jz4770.dtsi
+++ b/arch/mips/boot/dts/ingenic/jz4770.dtsi
@@ -1,5 +1,4 @@
 // SPDX-License-Identifier: GPL-2.0
-
 #include <dt-bindings/clock/jz4770-cgu.h>
 
 / {
@@ -7,6 +6,20 @@
 	#size-cells = <1>;
 	compatible = "ingenic,jz4770";
 
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		cpu0: cpu@0 {
+			device_type = "cpu";
+			compatible = "ingenic,xburst";
+			reg = <0>;
+
+			clocks = <&cgu JZ4770_CLK_CCLK>;
+			clock-names = "cpu";
+		};
+	};
+
 	cpuintc: interrupt-controller {
 		#address-cells = <0>;
 		#interrupt-cells = <1>;
diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi b/arch/mips/boot/dts/ingenic/jz4780.dtsi
index f928329..93a0604 100644
--- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
+++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
@@ -7,6 +7,29 @@
 	#size-cells = <1>;
 	compatible = "ingenic,jz4780";
 
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		cpu0: cpu@0 {
+			device_type = "cpu";
+			compatible = "ingenic,xburst";
+			reg = <0>;
+
+			clocks = <&cgu JZ4780_CLK_CPU>;
+			clock-names = "cpu";
+		};
+
+		cpu1: cpu@1 {
+			device_type = "cpu";
+			compatible = "ingenic,xburst";
+			reg = <1>;
+
+			clocks = <&cgu JZ4780_CLK_CORE1>;
+			clock-names = "cpu";
+		};
+	};
+
 	cpuintc: interrupt-controller {
 		#address-cells = <0>;
 		#interrupt-cells = <1>;
diff --git a/arch/mips/boot/dts/ingenic/x1000.dtsi b/arch/mips/boot/dts/ingenic/x1000.dtsi
index 4994c69..376df1b 100644
--- a/arch/mips/boot/dts/ingenic/x1000.dtsi
+++ b/arch/mips/boot/dts/ingenic/x1000.dtsi
@@ -7,6 +7,20 @@
 	#size-cells = <1>;
 	compatible = "ingenic,x1000", "ingenic,x1000e";
 
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		cpu0: cpu@0 {
+			device_type = "cpu";
+			compatible = "ingenic,xburst";
+			reg = <0>;
+
+			clocks = <&cgu X1000_CLK_CPU>;
+			clock-names = "cpu";
+		};
+	};
+
 	cpuintc: interrupt-controller {
 		#address-cells = <0>;
 		#interrupt-cells = <1>;
-- 
2.7.4

[PATCH v6 7/7] MIPS: CI20: Update defconfig to support SMP.

[周琰杰 (Zhou Yanjie)]

Add "CONFIG_SMP=y" and "CONFIG_NR_CPUS=2" to support SMP.

Tested-by: H. Nikolaus Schaller <hns@goldelico.com>
Tested-by: Paul Boddie <paul@boddie.org.uk>
Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
---

Notes:
    v1->v2:
    No change.
    
    v2->v3:
    No change.
    
    v3->v4:
    Rebase on top of kernel 5.6-rc1.
    
    v4->v5:
    No change.
    
    v5->v6:
    No change.

 arch/mips/configs/ci20_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/mips/configs/ci20_defconfig b/arch/mips/configs/ci20_defconfig
index be41df2..3aadb2e 100644
--- a/arch/mips/configs/ci20_defconfig
+++ b/arch/mips/configs/ci20_defconfig
@@ -1,3 +1,5 @@
+CONFIG_SMP=y
+CONFIG_NR_CPUS=2
 # CONFIG_LOCALVERSION_AUTO is not set
 CONFIG_KERNEL_XZ=y
 CONFIG_SYSVIPC=y
-- 
2.7.4