Messages in this thread | | | From | Arnd Bergmann <> | Date | Fri, 17 Sep 2021 11:57:04 +0200 | Subject | Re: [PATCH V3 20/22] LoongArch: Add multi-processor (SMP) support |
| |
On Fri, Sep 17, 2021 at 5:57 AM Huacai Chen <chenhuacai@loongson.cn> wrote:
> + > +struct task_struct; > + > +struct plat_smp_ops { > + void (*send_ipi_single)(int cpu, unsigned int action); > + void (*send_ipi_mask)(const struct cpumask *mask, unsigned int action); > + void (*smp_setup)(void); > + void (*prepare_cpus)(unsigned int max_cpus); > + int (*boot_secondary)(int cpu, struct task_struct *idle); > + void (*init_secondary)(void); > + void (*smp_finish)(void); > +#ifdef CONFIG_HOTPLUG_CPU > + int (*cpu_disable)(void); > + void (*cpu_die)(unsigned int cpu); > +#endif > +};
Do you foresee having more than one implementation of these in the near future? If not, I would suggest leaving out the extra indirection and just using direct function calls.
> +#ifdef CONFIG_SMP > + > +static inline void plat_smp_setup(void) > +{ > + extern const struct plat_smp_ops *mp_ops; /* private */ > + > + mp_ops->smp_setup(); > +} > + > +#else /* !CONFIG_SMP */ > + > +static inline void plat_smp_setup(void) { } > + > +#endif /* !CONFIG_SMP */
You could even go further and do what arch/arm64 has, making SMP support unconditional. This obviously depends on hardware roadmaps, but if all harfdware you support has multiple cores, then non-SMP mode just adds complexity.
> + > +#define MAX_CPUS 64
You CONFIG_NR_CPUS allows up to 256. I think you need to adjust one of the numbers to match the other, or remove this definition and just use CONFIG_NR_CPUS directly.
> + > +static volatile void *ipi_set_regs[MAX_CPUS]; > +static volatile void *ipi_clear_regs[MAX_CPUS]; > +static volatile void *ipi_status_regs[MAX_CPUS]; > +static volatile void *ipi_en_regs[MAX_CPUS]; > +static volatile void *ipi_mailbox_buf[MAX_CPUS];
Why are these 'volatile'? If they are MMIO registers, they should be __iomem, and accessed using readl()/writel() etc
Arnd
| |