[PATCH v6 0/3] ARM: rockchip: fix the SMP

[PATCH v6 0/3] ARM: rockchip: fix the SMP

[Caesar Wang]

Verified on url =
    https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.14
Tested by 179200 cycles are pass with CPU up/dowm test scripts.

Changes in v6:
- As Russell suggestion, detect whether of_reset_control_get() failed
- add the comment for 1ms delay.
Series-changes: 5
- back to v2 cpu on/off flow, As Heiko point out in patch v3.
- delay more time in rockchip_boot_secondary().
  From CPU up/down tests, Needed more time to complete CPU process.
  In order to ensure a more, Here that be delayed 1ms.
Series-changes: 4
- Add reset_control_put(rstc) for the non-error case.
Series-changes: 3
- FIx the PATCH v2, it doesn't work on chromium 3.14.
Series-changes: 2
- As Heiko suggestion, re-adjust the cpu on/off flow.
  CPU off:
    reset_control_assert
    regmap_update_bits(pmu, PMU_PWRDN_CON, BIT(pd), BIT(pd))
    wait_for_power_domain_to_turn_off
  CPU on:
    regmap_update_bits(pmu, PMU_PWRDN_CON, BIT(pd), 0)
    wait_for_power_domain_to_turn_on
    reset_control_deassert
- Fix the delay 1ms describing.
Series-changes: 5
- Fix the patch decription.
- Add the changelog.
Series-changes: 2
- As Kever points out, Fix the subject typo WFI/WFE.
- fix the commnet Unified format.
Series-changes: 5
- Add the changelog.
Series-changes: 2
- Use the checkpatch.pl -f --subjective to check.

Caesar Wang (3):
  ARM: rockchip: fix the CPU soft reset
  ARM: rockchip: ensure CPU to enter WFI/WFE state
  ARM: rockchip: fix the SMP code style

 arch/arm/mach-rockchip/platsmp.c | 58 ++++++++++++++++++++++++----------------
 1 file changed, 35 insertions(+), 23 deletions(-)

-- 
1.9.1

[PATCH v6 1/3] ARM: rockchip: fix the CPU soft reset

[Caesar Wang]

We need different orderings when turning a core on and turning a core
off.  In one case we need to assert reset before turning power off.
In ther other case we need to turn power on and the deassert reset.

In general, the correct flow is:

CPU off:
    reset_control_assert
    regmap_update_bits(pmu, PMU_PWRDN_CON, BIT(pd), BIT(pd))
    wait_for_power_domain_to_turn_off
CPU on:
    regmap_update_bits(pmu, PMU_PWRDN_CON, BIT(pd), 0)
    wait_for_power_domain_to_turn_on
    reset_control_deassert

This is needed for stressing CPU up/down, as per:
    cd /sys/devices/system/cpu/
    for i in $(seq 10000); do
        echo "================= $i ============"
        for j in $(seq 100); do
            while [[ "$(cat cpu1/online)$(cat cpu2/online)$(cat cpu3/online)" != "000"" ]]
                echo 0 > cpu1/online
                echo 0 > cpu2/online
                echo 0 > cpu3/online
            done
            while [[ "$(cat cpu1/online)$(cat cpu2/online)$(cat cpu3/online)" != "111" ]]; do
                echo 1 > cpu1/online
                echo 1 > cpu2/online
                echo 1 > cpu3/online
            done
        done
    done

The following is reproducable log:
    [34466.186812] PM: noirq suspend of devices complete after 0.669 msecs
    [34466.186824] Disabling non-boot CPUs ...
    [34466.187509] CPU1: shutdown
    [34466.188672] CPU2: shutdown
    [34473.736627] Kernel panic - not syncing:Watchdog detected hard LOCKUP on cpu 0
    .......
or others similar log:
    .......
    [ 4072.454453] CPU1: shutdown
    [ 4072.504436] CPU2: shutdown
    [ 4072.554426] CPU3: shutdown
    [ 4072.577827] CPU1: Booted secondary processor
    [ 4072.582611] CPU2: Booted secondary processor
    <hang>

    Tested by cpu up/down scripts, the results told us need delay more time
before write the sram. The wait time is affected by many aspects
(e.g: cpu frequency, bootrom frequency, sram frequency, bus speed, ...).

    Although the cpus other than cpu0 will write the sram, the speedy is
no the same as cpu0, if the cpu0 early wake up, perhaps the other cpus
can't startup. As we know, the cpu0 can wake up when the cpu1/2/3 write
the 'sram+4/8' and send the sev.
    Anyway.....
    At the moment, 1ms delay will be happy work for cpu up/down scripts test.

Signed-off-by: Caesar Wang <wxt@rock-chips.com>
Reviewed-by: Doug Anderson <dianders@chromium.org>

---

Changes in v6:
- As Russell suggestion, detect whether of_reset_control_get() failed
- add the comment for 1ms delay.
Series-changes: 5
- back to v2 cpu on/off flow, As Heiko point out in patch v3.
- delay more time in rockchip_boot_secondary().
  From CPU up/down tests, Needed more time to complete CPU process.
  In order to ensure a more, Here that be delayed 1ms.
Series-changes: 4
- Add reset_control_put(rstc) for the non-error case.
Series-changes: 3
- FIx the PATCH v2, it doesn't work on chromium 3.14.
Series-changes: 2
- As Heiko suggestion, re-adjust the cpu on/off flow.
  CPU off:
    reset_control_assert
    regmap_update_bits(pmu, PMU_PWRDN_CON, BIT(pd), BIT(pd))
    wait_for_power_domain_to_turn_off
  CPU on:
    regmap_update_bits(pmu, PMU_PWRDN_CON, BIT(pd), 0)
    wait_for_power_domain_to_turn_on
    reset_control_deassert

 arch/arm/mach-rockchip/platsmp.c | 37 ++++++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 17 deletions(-)

diff --git a/arch/arm/mach-rockchip/platsmp.c b/arch/arm/mach-rockchip/platsmp.c
index 5b4ca3c..b379cc8 100644
--- a/arch/arm/mach-rockchip/platsmp.c
+++ b/arch/arm/mach-rockchip/platsmp.c
@@ -72,29 +72,22 @@ static struct reset_control *rockchip_get_core_reset(int cpu)
 static int pmu_set_power_domain(int pd, bool on)
 {
 	u32 val = (on) ? 0 : BIT(pd);
+	struct reset_control *rstc = rockchip_get_core_reset(pd);
 	int ret;
 
+	if (IS_ERR(rstc) && read_cpuid_part() != ARM_CPU_PART_CORTEX_A9) {
+		pr_err("%s: could not get reset control for core %d\n",
+		       __func__, pd);
+		return PTR_ERR(rstc);
+	}
+
 	/*
 	 * We need to soft reset the cpu when we turn off the cpu power domain,
 	 * or else the active processors might be stalled when the individual
 	 * processor is powered down.
 	 */
-	if (read_cpuid_part() != ARM_CPU_PART_CORTEX_A9) {
-		struct reset_control *rstc = rockchip_get_core_reset(pd);
-
-		if (IS_ERR(rstc)) {
-			pr_err("%s: could not get reset control for core %d\n",
-			       __func__, pd);
-			return PTR_ERR(rstc);
-		}
-
-		if (on)
-			reset_control_deassert(rstc);
-		else
-			reset_control_assert(rstc);
-
-		reset_control_put(rstc);
-	}
+	if (!IS_ERR(rstc) && !on)
+		reset_control_assert(rstc);
 
 	ret = regmap_update_bits(pmu, PMU_PWRDN_CON, BIT(pd), val);
 	if (ret < 0) {
@@ -112,6 +105,12 @@ static int pmu_set_power_domain(int pd, bool on)
 		}
 	}
 
+	if (!IS_ERR(rstc)) {
+		if (on)
+			reset_control_deassert(rstc);
+		reset_control_put(rstc);
+	}
+
 	return 0;
 }
 
@@ -147,8 +146,12 @@ static int __cpuinit rockchip_boot_secondary(unsigned int cpu,
 		 * the mailbox:
 		 * sram_base_addr + 4: 0xdeadbeaf
 		 * sram_base_addr + 8: start address for pc
+		 * The cpu0 need to wait the other cpus other than cpu0 entering
+		 * the wfe state.The wait time is affected by many aspects.
+		 * (e.g: cpu frequency, bootrom frequency, sram frequency, ...)
 		 * */
-		udelay(10);
+		mdelay(1); /* ensure the cpus other than cpu0 to startup */
+
 		writel(virt_to_phys(rockchip_secondary_startup),
 			sram_base_addr + 8);
 		writel(0xDEADBEAF, sram_base_addr + 4);
-- 
1.9.1

[PATCH v6 1/3] ARM: rockchip: fix the CPU soft reset

[Doug Anderson]

Caesar,

On Tue, Jun 9, 2015 at 2:49 AM, Caesar Wang <wxt@rock-chips.com> wrote:
> We need different orderings when turning a core on and turning a core
> off.  In one case we need to assert reset before turning power off.
> In ther other case we need to turn power on and the deassert reset.
>
> In general, the correct flow is:
>
> CPU off:
>     reset_control_assert
>     regmap_update_bits(pmu, PMU_PWRDN_CON, BIT(pd), BIT(pd))
>     wait_for_power_domain_to_turn_off
> CPU on:
>     regmap_update_bits(pmu, PMU_PWRDN_CON, BIT(pd), 0)
>     wait_for_power_domain_to_turn_on
>     reset_control_deassert
>
> This is needed for stressing CPU up/down, as per:
>     cd /sys/devices/system/cpu/
>     for i in $(seq 10000); do
>         echo "================= $i ============"
>         for j in $(seq 100); do
>             while [[ "$(cat cpu1/online)$(cat cpu2/online)$(cat cpu3/online)" != "000"" ]]
>                 echo 0 > cpu1/online
>                 echo 0 > cpu2/online
>                 echo 0 > cpu3/online
>             done
>             while [[ "$(cat cpu1/online)$(cat cpu2/online)$(cat cpu3/online)" != "111" ]]; do
>                 echo 1 > cpu1/online
>                 echo 1 > cpu2/online
>                 echo 1 > cpu3/online
>             done
>         done
>     done
>
> The following is reproducable log:
>     [34466.186812] PM: noirq suspend of devices complete after 0.669 msecs
>     [34466.186824] Disabling non-boot CPUs ...
>     [34466.187509] CPU1: shutdown
>     [34466.188672] CPU2: shutdown
>     [34473.736627] Kernel panic - not syncing:Watchdog detected hard LOCKUP on cpu 0
>     .......
> or others similar log:
>     .......
>     [ 4072.454453] CPU1: shutdown
>     [ 4072.504436] CPU2: shutdown
>     [ 4072.554426] CPU3: shutdown
>     [ 4072.577827] CPU1: Booted secondary processor
>     [ 4072.582611] CPU2: Booted secondary processor
>     <hang>
>
>     Tested by cpu up/down scripts, the results told us need delay more time
> before write the sram. The wait time is affected by many aspects
> (e.g: cpu frequency, bootrom frequency, sram frequency, bus speed, ...).
>
>     Although the cpus other than cpu0 will write the sram, the speedy is
> no the same as cpu0, if the cpu0 early wake up, perhaps the other cpus
> can't startup. As we know, the cpu0 can wake up when the cpu1/2/3 write
> the 'sram+4/8' and send the sev.
>     Anyway.....
>     At the moment, 1ms delay will be happy work for cpu up/down scripts test.
>
> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
> Reviewed-by: Doug Anderson <dianders@chromium.org>

Usually it's good to remove someone's "Reviewed-by" when you've made
as many changes as you have.  ...but in this case I am still happy
with this patch, so I'll re-assert:

Reviewed-by: Douglas Anderson <dianders@chromium.org>

[PATCH v6 1/3] ARM: rockchip: fix the CPU soft reset

[Kever Yang]

Hi Caesar,

On 06/09/2015 05:49 PM, Caesar Wang wrote:
> We need different orderings when turning a core on and turning a core
> off.  In one case we need to assert reset before turning power off.
> In ther other case we need to turn power on and the deassert reset.
>
> In general, the correct flow is:
>
> CPU off:
>      reset_control_assert
>      regmap_update_bits(pmu, PMU_PWRDN_CON, BIT(pd), BIT(pd))
>      wait_for_power_domain_to_turn_off
> CPU on:
>      regmap_update_bits(pmu, PMU_PWRDN_CON, BIT(pd), 0)
>      wait_for_power_domain_to_turn_on
>      reset_control_deassert
>
> This is needed for stressing CPU up/down, as per:
>      cd /sys/devices/system/cpu/
>      for i in $(seq 10000); do
>          echo "================= $i ============"
>          for j in $(seq 100); do
>              while [[ "$(cat cpu1/online)$(cat cpu2/online)$(cat cpu3/online)" != "000"" ]]
>                  echo 0 > cpu1/online
>                  echo 0 > cpu2/online
>                  echo 0 > cpu3/online
>              done
>              while [[ "$(cat cpu1/online)$(cat cpu2/online)$(cat cpu3/online)" != "111" ]]; do
>                  echo 1 > cpu1/online
>                  echo 1 > cpu2/online
>                  echo 1 > cpu3/online
>              done
>          done
>      done
>
> The following is reproducable log:
>      [34466.186812] PM: noirq suspend of devices complete after 0.669 msecs
>      [34466.186824] Disabling non-boot CPUs ...
>      [34466.187509] CPU1: shutdown
>      [34466.188672] CPU2: shutdown
>      [34473.736627] Kernel panic - not syncing:Watchdog detected hard LOCKUP on cpu 0
>      .......
> or others similar log:
>      .......
>      [ 4072.454453] CPU1: shutdown
>      [ 4072.504436] CPU2: shutdown
>      [ 4072.554426] CPU3: shutdown
>      [ 4072.577827] CPU1: Booted secondary processor
>      [ 4072.582611] CPU2: Booted secondary processor
>      <hang>
>
>      Tested by cpu up/down scripts, the results told us need delay more time
> before write the sram. The wait time is affected by many aspects
> (e.g: cpu frequency, bootrom frequency, sram frequency, bus speed, ...).
>
>      Although the cpus other than cpu0 will write the sram, the speedy is
> no the same as cpu0, if the cpu0 early wake up, perhaps the other cpus
> can't startup. As we know, the cpu0 can wake up when the cpu1/2/3 write
> the 'sram+4/8' and send the sev.
>      Anyway.....
>      At the moment, 1ms delay will be happy work for cpu up/down scripts test.
>
> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
> Reviewed-by: Doug Anderson <dianders@chromium.org>
>
> ---
>
> Changes in v6:
> - As Russell suggestion, detect whether of_reset_control_get() failed
> - add the comment for 1ms delay.
> Series-changes: 5
> - back to v2 cpu on/off flow, As Heiko point out in patch v3.
> - delay more time in rockchip_boot_secondary().
>    From CPU up/down tests, Needed more time to complete CPU process.
>    In order to ensure a more, Here that be delayed 1ms.
> Series-changes: 4
> - Add reset_control_put(rstc) for the non-error case.
> Series-changes: 3
> - FIx the PATCH v2, it doesn't work on chromium 3.14.
> Series-changes: 2
> - As Heiko suggestion, re-adjust the cpu on/off flow.
>    CPU off:
>      reset_control_assert
>      regmap_update_bits(pmu, PMU_PWRDN_CON, BIT(pd), BIT(pd))
>      wait_for_power_domain_to_turn_off
>    CPU on:
>      regmap_update_bits(pmu, PMU_PWRDN_CON, BIT(pd), 0)
>      wait_for_power_domain_to_turn_on
>      reset_control_deassert
>
>   arch/arm/mach-rockchip/platsmp.c | 37 ++++++++++++++++++++-----------------
>   1 file changed, 20 insertions(+), 17 deletions(-)
>
> diff --git a/arch/arm/mach-rockchip/platsmp.c b/arch/arm/mach-rockchip/platsmp.c
> index 5b4ca3c..b379cc8 100644
> --- a/arch/arm/mach-rockchip/platsmp.c
> +++ b/arch/arm/mach-rockchip/platsmp.c
> @@ -72,29 +72,22 @@ static struct reset_control *rockchip_get_core_reset(int cpu)
>   static int pmu_set_power_domain(int pd, bool on)
>   {
>   	u32 val = (on) ? 0 : BIT(pd);
> +	struct reset_control *rstc = rockchip_get_core_reset(pd);
>   	int ret;
>   
> +	if (IS_ERR(rstc) && read_cpuid_part() != ARM_CPU_PART_CORTEX_A9) {
> +		pr_err("%s: could not get reset control for core %d\n",
> +		       __func__, pd);
> +		return PTR_ERR(rstc);
> +	}
> +
>   	/*
>   	 * We need to soft reset the cpu when we turn off the cpu power domain,
>   	 * or else the active processors might be stalled when the individual
>   	 * processor is powered down.
>   	 */
> -	if (read_cpuid_part() != ARM_CPU_PART_CORTEX_A9) {
> -		struct reset_control *rstc = rockchip_get_core_reset(pd);
> -
> -		if (IS_ERR(rstc)) {
> -			pr_err("%s: could not get reset control for core %d\n",
> -			       __func__, pd);
> -			return PTR_ERR(rstc);
> -		}
> -
> -		if (on)
> -			reset_control_deassert(rstc);
> -		else
> -			reset_control_assert(rstc);
> -
> -		reset_control_put(rstc);
> -	}
> +	if (!IS_ERR(rstc) && !on)
> +		reset_control_assert(rstc);
>   
>   	ret = regmap_update_bits(pmu, PMU_PWRDN_CON, BIT(pd), val);
>   	if (ret < 0) {
> @@ -112,6 +105,12 @@ static int pmu_set_power_domain(int pd, bool on)
>   		}
>   	}
>   
> +	if (!IS_ERR(rstc)) {
> +		if (on)
> +			reset_control_deassert(rstc);
> +		reset_control_put(rstc);
> +	}
> +
>   	return 0;
>   }
>   
> @@ -147,8 +146,12 @@ static int __cpuinit rockchip_boot_secondary(unsigned int cpu,
>   		 * the mailbox:
>   		 * sram_base_addr + 4: 0xdeadbeaf
>   		 * sram_base_addr + 8: start address for pc
> +		 * The cpu0 need to wait the other cpus other than cpu0 entering
> +		 * the wfe state.The wait time is affected by many aspects.
> +		 * (e.g: cpu frequency, bootrom frequency, sram frequency, ...)
>   		 * */
> -		udelay(10);
> +		mdelay(1); /* ensure the cpus other than cpu0 to startup */
> +
>   		writel(virt_to_phys(rockchip_secondary_startup),
>   			sram_base_addr + 8);
>   		writel(0xDEADBEAF, sram_base_addr + 4);
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

[PATCH v6 2/3] ARM: rockchip: ensure CPU to enter WFI/WFE state

[Caesar Wang]

The patch can ensure that v7_exit_coherency_flush() in rockchip_cpu_die()
executed in time.
The mdelay(1) has enough time to fix the problem of CPU offlining.
That's a workaround way in rockchip hotplug code,
At least, we haven't a better way to solve it. Who know,
that maybe fixed by chip (hardware) in the future.

Signed-off-by: Caesar Wang <wxt@rock-chips.com>

---

Changes in v6:
- Fix the delay 1ms describing.
Series-changes: 5
- Fix the patch decription.
- Add the changelog.
Series-changes: 2
- As Kever points out, Fix the subject typo WFI/WFE.

 arch/arm/mach-rockchip/platsmp.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/mach-rockchip/platsmp.c b/arch/arm/mach-rockchip/platsmp.c
index b379cc8..d629206 100644
--- a/arch/arm/mach-rockchip/platsmp.c
+++ b/arch/arm/mach-rockchip/platsmp.c
@@ -322,6 +322,13 @@ static void __init rockchip_smp_prepare_cpus(unsigned int max_cpus)
 #ifdef CONFIG_HOTPLUG_CPU
 static int rockchip_cpu_kill(unsigned int cpu)
 {
+	/*
+	 * We need a delay here to ensure that the dying CPU can finish
+	 * executing v7_coherency_exit() and reach the WFI/WFE state
+	 * prior to having the power domain disabled.
+	 */
+	mdelay(1);
+
 	pmu_set_power_domain(0 + cpu, false);
 	return 1;
 }
-- 
1.9.1

[PATCH v6 2/3] ARM: rockchip: ensure CPU to enter WFI/WFE state

[Doug Anderson]

Caesar,

On Tue, Jun 9, 2015 at 2:49 AM, Caesar Wang <wxt@rock-chips.com> wrote:
> The patch can ensure that v7_exit_coherency_flush() in rockchip_cpu_die()
> executed in time.
> The mdelay(1) has enough time to fix the problem of CPU offlining.
> That's a workaround way in rockchip hotplug code,
> At least, we haven't a better way to solve it. Who know,
> that maybe fixed by chip (hardware) in the future.
>
> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
>
> ---
>
> Changes in v6:
> - Fix the delay 1ms describing.
> Series-changes: 5
> - Fix the patch decription.
> - Add the changelog.
> Series-changes: 2
> - As Kever points out, Fix the subject typo WFI/WFE.
>
>  arch/arm/mach-rockchip/platsmp.c | 7 +++++++
>  1 file changed, 7 insertions(+)

Reviewed-by: Douglas Anderson <dianders@chromium.org>

[PATCH v6 2/3] ARM: rockchip: ensure CPU to enter WFI/WFE state

[Kever Yang]

Hi Caesar,

On 06/09/2015 05:49 PM, Caesar Wang wrote:
> The patch can ensure that v7_exit_coherency_flush() in rockchip_cpu_die()
> executed in time.
> The mdelay(1) has enough time to fix the problem of CPU offlining.
> That's a workaround way in rockchip hotplug code,
> At least, we haven't a better way to solve it. Who know,
> that maybe fixed by chip (hardware) in the future.
>
> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
>
> ---
>
> Changes in v6:
> - Fix the delay 1ms describing.
> Series-changes: 5
> - Fix the patch decription.
> - Add the changelog.
> Series-changes: 2
> - As Kever points out, Fix the subject typo WFI/WFE.
>
>   arch/arm/mach-rockchip/platsmp.c | 7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/arch/arm/mach-rockchip/platsmp.c b/arch/arm/mach-rockchip/platsmp.c
> index b379cc8..d629206 100644
> --- a/arch/arm/mach-rockchip/platsmp.c
> +++ b/arch/arm/mach-rockchip/platsmp.c
> @@ -322,6 +322,13 @@ static void __init rockchip_smp_prepare_cpus(unsigned int max_cpus)
>   #ifdef CONFIG_HOTPLUG_CPU
>   static int rockchip_cpu_kill(unsigned int cpu)
>   {
> +	/*
> +	 * We need a delay here to ensure that the dying CPU can finish
> +	 * executing v7_coherency_exit() and reach the WFI/WFE state
> +	 * prior to having the power domain disabled.
> +	 */
> +	mdelay(1);
> +
>   	pmu_set_power_domain(0 + cpu, false);
>   	return 1;
>   }
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

[PATCH v6 3/3] ARM: rockchip: fix the SMP code style

[Caesar Wang]

Use the below scripts to check:
scripts/checkpatch.pl -f --subject arch/arm/mach-rockchip/platsmp.c

Signed-off-by: Caesar Wang <wxt@rock-chips.com>

---

Changes in v6:
- fix the commnet Unified format.
Series-changes: 5
- Add the changelog.
Series-changes: 2
- Use the checkpatch.pl -f --subjective to check.

 arch/arm/mach-rockchip/platsmp.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-rockchip/platsmp.c b/arch/arm/mach-rockchip/platsmp.c
index d629206..30ccb82 100644
--- a/arch/arm/mach-rockchip/platsmp.c
+++ b/arch/arm/mach-rockchip/platsmp.c
@@ -100,7 +100,7 @@ static int pmu_set_power_domain(int pd, bool on)
 		ret = pmu_power_domain_is_on(pd);
 		if (ret < 0) {
 			pr_err("%s: could not read power domain state\n",
-				 __func__);
+			       __func__);
 			return ret;
 		}
 	}
@@ -130,7 +130,7 @@ static int __cpuinit rockchip_boot_secondary(unsigned int cpu,
 
 	if (cpu >= ncores) {
 		pr_err("%s: cpu %d outside maximum number of cpus %d\n",
-							__func__, cpu, ncores);
+		       __func__, cpu, ncores);
 		return -ENXIO;
 	}
 
@@ -140,7 +140,8 @@ static int __cpuinit rockchip_boot_secondary(unsigned int cpu,
 		return ret;
 
 	if (read_cpuid_part() != ARM_CPU_PART_CORTEX_A9) {
-		/* We communicate with the bootrom to active the cpus other
+		/*
+		 * We communicate with the bootrom to active the cpus other
 		 * than cpu0, after a blob of initialize code, they will
 		 * stay at wfe state, once they are actived, they will check
 		 * the mailbox:
@@ -149,11 +150,11 @@ static int __cpuinit rockchip_boot_secondary(unsigned int cpu,
 		 * The cpu0 need to wait the other cpus other than cpu0 entering
 		 * the wfe state.The wait time is affected by many aspects.
 		 * (e.g: cpu frequency, bootrom frequency, sram frequency, ...)
-		 * */
+		 */
 		mdelay(1); /* ensure the cpus other than cpu0 to startup */
 
 		writel(virt_to_phys(rockchip_secondary_startup),
-			sram_base_addr + 8);
+		       sram_base_addr + 8);
 		writel(0xDEADBEAF, sram_base_addr + 4);
 		dsb_sev();
 	}
@@ -336,7 +337,7 @@ static int rockchip_cpu_kill(unsigned int cpu)
 static void rockchip_cpu_die(unsigned int cpu)
 {
 	v7_exit_coherency_flush(louis);
-	while(1)
+	while (1)
 		cpu_do_idle();
 }
 #endif
@@ -349,4 +350,5 @@ static struct smp_operations rockchip_smp_ops __initdata = {
 	.cpu_die		= rockchip_cpu_die,
 #endif
 };
+
 CPU_METHOD_OF_DECLARE(rk3066_smp, "rockchip,rk3066-smp", &rockchip_smp_ops);
-- 
1.9.1

[PATCH v6 3/3] ARM: rockchip: fix the SMP code style

[Doug Anderson]

Caesar,

On Tue, Jun 9, 2015 at 2:49 AM, Caesar Wang <wxt@rock-chips.com> wrote:
> Use the below scripts to check:
> scripts/checkpatch.pl -f --subject arch/arm/mach-rockchip/platsmp.c
>
> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
>
> ---
>
> Changes in v6:
> - fix the commnet Unified format.
> Series-changes: 5
> - Add the changelog.
> Series-changes: 2
> - Use the checkpatch.pl -f --subjective to check.
>
>  arch/arm/mach-rockchip/platsmp.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)

This patch hasn't changed, so you could have kept my Reviewed-by that
I gave previously.

Reviewed-by: Douglas Anderson <dianders@chromium.org>

[PATCH v6 3/3] ARM: rockchip: fix the SMP code style

[Kever Yang]

Hi Caesar,

On 06/09/2015 05:49 PM, Caesar Wang wrote:
> Use the below scripts to check:
> scripts/checkpatch.pl -f --subject arch/arm/mach-rockchip/platsmp.c
>
> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
>
> ---
>
> Changes in v6:
> - fix the commnet Unified format.
> Series-changes: 5
> - Add the changelog.
> Series-changes: 2
> - Use the checkpatch.pl -f --subjective to check.
>
>   arch/arm/mach-rockchip/platsmp.c | 14 ++++++++------
>   1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/mach-rockchip/platsmp.c b/arch/arm/mach-rockchip/platsmp.c
> index d629206..30ccb82 100644
> --- a/arch/arm/mach-rockchip/platsmp.c
> +++ b/arch/arm/mach-rockchip/platsmp.c
> @@ -100,7 +100,7 @@ static int pmu_set_power_domain(int pd, bool on)
>   		ret = pmu_power_domain_is_on(pd);
>   		if (ret < 0) {
>   			pr_err("%s: could not read power domain state\n",
> -				 __func__);
> +			       __func__);
>   			return ret;
>   		}
>   	}
> @@ -130,7 +130,7 @@ static int __cpuinit rockchip_boot_secondary(unsigned int cpu,
>   
>   	if (cpu >= ncores) {
>   		pr_err("%s: cpu %d outside maximum number of cpus %d\n",
> -							__func__, cpu, ncores);
> +		       __func__, cpu, ncores);
>   		return -ENXIO;
>   	}
>   
> @@ -140,7 +140,8 @@ static int __cpuinit rockchip_boot_secondary(unsigned int cpu,
>   		return ret;
>   
>   	if (read_cpuid_part() != ARM_CPU_PART_CORTEX_A9) {
> -		/* We communicate with the bootrom to active the cpus other
> +		/*
> +		 * We communicate with the bootrom to active the cpus other
>   		 * than cpu0, after a blob of initialize code, they will
>   		 * stay at wfe state, once they are actived, they will check
>   		 * the mailbox:
> @@ -149,11 +150,11 @@ static int __cpuinit rockchip_boot_secondary(unsigned int cpu,
>   		 * The cpu0 need to wait the other cpus other than cpu0 entering
>   		 * the wfe state.The wait time is affected by many aspects.
>   		 * (e.g: cpu frequency, bootrom frequency, sram frequency, ...)
> -		 * */
> +		 */
>   		mdelay(1); /* ensure the cpus other than cpu0 to startup */
>   
>   		writel(virt_to_phys(rockchip_secondary_startup),
> -			sram_base_addr + 8);
> +		       sram_base_addr + 8);
>   		writel(0xDEADBEAF, sram_base_addr + 4);
>   		dsb_sev();
>   	}
> @@ -336,7 +337,7 @@ static int rockchip_cpu_kill(unsigned int cpu)
>   static void rockchip_cpu_die(unsigned int cpu)
>   {
>   	v7_exit_coherency_flush(louis);
> -	while(1)
> +	while (1)
>   		cpu_do_idle();
>   }
>   #endif
> @@ -349,4 +350,5 @@ static struct smp_operations rockchip_smp_ops __initdata = {
>   	.cpu_die		= rockchip_cpu_die,
>   #endif
>   };
> +
>   CPU_METHOD_OF_DECLARE(rk3066_smp, "rockchip,rk3066-smp", &rockchip_smp_ops);
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

[PATCH v6 0/3] ARM: rockchip: fix the SMP

[Heiko Stübner]

Hi Caesar,


Am Dienstag, 9. Juni 2015, 17:49:56 schrieb Caesar Wang:
> Verified on url =
>    
> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-> 3.14 Tested by 179200 cycles are pass with CPU up/dowm test scripts.
> 
> Changes in v6:
> - As Russell suggestion, detect whether of_reset_control_get() failed
> - add the comment for 1ms delay.
> Series-changes: 5
> - back to v2 cpu on/off flow, As Heiko point out in patch v3.
> - delay more time in rockchip_boot_secondary().
>   From CPU up/down tests, Needed more time to complete CPU process.
>   In order to ensure a more, Here that be delayed 1ms.
> Series-changes: 4
> - Add reset_control_put(rstc) for the non-error case.
> Series-changes: 3
> - FIx the PATCH v2, it doesn't work on chromium 3.14.
> Series-changes: 2
> - As Heiko suggestion, re-adjust the cpu on/off flow.
>   CPU off:
>     reset_control_assert
>     regmap_update_bits(pmu, PMU_PWRDN_CON, BIT(pd), BIT(pd))
>     wait_for_power_domain_to_turn_off
>   CPU on:
>     regmap_update_bits(pmu, PMU_PWRDN_CON, BIT(pd), 0)
>     wait_for_power_domain_to_turn_on
>     reset_control_deassert
> - Fix the delay 1ms describing.
> Series-changes: 5
> - Fix the patch decription.
> - Add the changelog.
> Series-changes: 2
> - As Kever points out, Fix the subject typo WFI/WFE.
> - fix the commnet Unified format.
> Series-changes: 5
> - Add the changelog.
> Series-changes: 2
> - Use the checkpatch.pl -f --subjective to check.
> 
> Caesar Wang (3):
>   ARM: rockchip: fix the CPU soft reset
>   ARM: rockchip: ensure CPU to enter WFI/WFE state
>   ARM: rockchip: fix the SMP code style

the series looks good to me now. I'll give it another day or so before 
applying, just in case Russell wants to provide additional comments.


Heiko

[PATCH v6 0/3] ARM: rockchip: fix the SMP

[Heiko Stübner]

Hi Caesar,

Am Dienstag, 9. Juni 2015, 17:49:56 schrieb Caesar Wang:
> Verified on url =
>    
> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-> 3.14 Tested by 179200 cycles are pass with CPU up/dowm test scripts.

I've applied this series to my soc branch for 4.3 ... it's a bit late for 
trying to cram this into 4.2. I've also added a Cc stable@ tag for patch1


Heiko