Subject: Re: [PATCH 05/10] MIPS: bmips: merge CPU options into one option

From: Florian Fainelli <[email-masked]>
Date: Tue, 16 Jul 2013 13:31:20 +0100

Hello Jonas,

2013/6/29 Jonas Gorski <[email-masked]>:
[snip]

> +config CPU_BMIPS
> +       bool "Broadcom BMIPS"
> +       depends on SYS_HAS_CPU_BMIPS
> +       select CPU_MIPS32
> +       select CPU_BMIPS3300 if SYS_HAS_CPU_BMIPS3300
> +       select CPU_BMIPS4350 if SYS_HAS_CPU_BMIPS4350
> +       select CPU_BMIPS4350 if SYS_HAS_CPU_BMIPS4380

As you already made me notice privately, this should be:

select CPU_BMIPS4380 if SYS_HAS_CPU_BMIPS4380
--
Florian

---

From: Jonas Gorski <[email-masked]>
Date: Tue, 16 Jul 2013 14:41:45 +0200

> select CPU_BMIPS4380 if SYS_HAS_CPU_BMIPS4380

I'm currently preparing a V2 that will fix this, the missed squash
commit message in 6/10, and adds bcm47xx bmips selection (so that
Ralf's cputype patch won't break bcm47xx and bcm63xx anymore). I
marked the patches in patchwork accordingly.

Jonas

---

From: Florian Fainelli <[email-masked]>
Date: Tue, 16 Jul 2013 14:06:36 +0100

Hello Jonas,

2013/6/29 Jonas Gorski <[email-masked]>:
> This patchset aims at unifying the different BMIPS support code to allow
> building a kernel that runs on multiple BCM63XX SoCs which might have
> different BMIPS flavours on them, regardless of SMP support enabled in
> the kernel.
>
> The first few patches clean up BMIPS itself and prepare it for multi-cpu
> support, while the latter add support to BCM63XX for running a SMP kernel
> with support for all SoCs, even those that do not have a SMP capable
> CPU.
>
> This patchset is runtime tested on BCM6348, BCM6328 and BCM6368, to
> verify that it actually does what it claims it does.
>
> Lacking hardware, it is only build tested for BMIPS4380 and BMIPS5000.
>
> Jonas Gorski (10):
>   MIPS: bmips: fix compilation for BMIPS5000
>   MIPS: allow asm/cpu.h to be included from assembly
>   MIPS: bmips: add macros for testing the current bmips CPU
>   MIPS: bmips: change compile time checks to runtime checks
>   MIPS: bmips: merge CPU options into one option
>   MIPS: BCM63XX: let the individual SoCs select the appropriate CPUs
>   MIPS: bmips: add a helper function for registering smp ops
>   MIPS: BCM63XX: always register bmips smp ops
>   MIPS: BCM63XX: change the guard to a BMIPS4350 check
>   MIPS: BCM63XX: disable SMP also on BCM3368

After fixing the typo on BMIPS4350 vs BMIPS4380 and fixing the
following (which I will submit just in a few minutes)

@@ -187,7 +187,7 @@ static void bmips_boot_secondary(int cpu, struct task_struct
        } else {
                if (cpu_is_bmips4350() || cpu_is_bmips4380()) {
                        /* Reset slave TP1 if booting from TP0 */
-                       if (cpu_logical_map(cpu) == 0)
+                       if (cpu_logical_map(cpu) == 1)
                                set_c0_brcm_cmt_ctrl(0x01);
                } else if (cpu_is_bmips5000()) {
                        if (cpu & 0x01)

it works just nicely on BMIPS4380. I plan on doing some testing on
BMIPS5000 later this week.

Thanks!
--
Florian

---

From: Jonas Gorski <[email-masked]>
Date: Tue, 16 Jul 2013 16:13:16 +0200

> BMIPS5000 later this week.

Great, thanks for testing. This change looks quite correct. I'll
rebase my patchset then onto your patch.

Regards
Jonas

---

From: Florian Fainelli <[email-masked]>
Date: Tue, 16 Jul 2013 15:26:38 +0100

>>                 } else if (cpu_is_bmips5000()) {

Another way would be to make the test be:

cpu_logical_map(0) == 0, which seems more accurate with respect to the
above comment.

--
Florian

---

From: Florian Fainelli <[email-masked]>
Date: Tue, 16 Jul 2013 22:37:30 +0100

> following (which I will submit just in a few minutes)

I just gave this patchset a try on a BMIPS5000 system, but unfortunately the 
kernel crashes early on due to a change in smp-bmips.c. It crashed in 
mem_init(), and adding a printk in bmips_ebase_setup() in the final else clause 
before the return did no longer make the kernel crash. This could be some 
nasty cache issue and simply changing the kernel size did help. Will 
investigate this more.
-- 
Florian