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

[PATCH v5 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 176000 cycles are pass with CPU up/dowm test scripts.


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 | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

-- 
1.9.1

[PATCH v5 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 reproducile 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
    [ 4072.587426] CPU3: Booted secondary processor
    <hang>

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

Changes in v5:
    - 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.

Changes in v4:
    - Add reset_control_put(rstc) for the non-error case.

Changes in v3:
    - FIx the PATCH v2, it doesn't work on chromium 3.14.

Changes in v2:
    - 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 | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-rockchip/platsmp.c b/arch/arm/mach-rockchip/platsmp.c
index 5b4ca3c..bd40852 100644
--- a/arch/arm/mach-rockchip/platsmp.c
+++ b/arch/arm/mach-rockchip/platsmp.c
@@ -72,6 +72,7 @@ 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;
 
 	/*
@@ -80,20 +81,15 @@ static int pmu_set_power_domain(int pd, bool on)
 	 * processor is powered down.
 	 */
 	if (read_cpuid_part() != ARM_CPU_PART_CORTEX_A9) {
-		struct reset_control *rstc = rockchip_get_core_reset(pd);
-
+		/* We only require the reset on the RK3288 at the moment */
 		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
+		if (!on)
 			reset_control_assert(rstc);
-
-		reset_control_put(rstc);
 	}
 
 	ret = regmap_update_bits(pmu, PMU_PWRDN_CON, BIT(pd), val);
@@ -112,6 +108,12 @@ static int pmu_set_power_domain(int pd, bool on)
 		}
 	}
 
+	if (read_cpuid_part() != ARM_CPU_PART_CORTEX_A9 && on)
+		reset_control_deassert(rstc);
+
+	if (!IS_ERR(rstc))
+		reset_control_put(rstc);
+
 	return 0;
 }
 
@@ -148,7 +150,7 @@ static int __cpuinit rockchip_boot_secondary(unsigned int cpu,
 		 * sram_base_addr + 4: 0xdeadbeaf
 		 * sram_base_addr + 8: start address for pc
 		 * */
-		udelay(10);
+		mdelay(1);
 		writel(virt_to_phys(rockchip_secondary_startup),
 			sram_base_addr + 8);
 		writel(0xDEADBEAF, sram_base_addr + 4);
-- 
1.9.1

[PATCH v5 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 v5:
    - Fix the patch decription.
    - Add the changelog.

Changes in v4: None

Changes in v3: None

Changes in v2: None

Changes in v2:
    - As Kever points out, Fix the subject typo WFI/WFE.

---

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

diff --git a/arch/arm/mach-rockchip/platsmp.c b/arch/arm/mach-rockchip/platsmp.c
index bd40852..5bc2a89 100644
--- a/arch/arm/mach-rockchip/platsmp.c
+++ b/arch/arm/mach-rockchip/platsmp.c
@@ -321,6 +321,9 @@ static void __init rockchip_smp_prepare_cpus(unsigned int max_cpus)
 #ifdef CONFIG_HOTPLUG_CPU
 static int rockchip_cpu_kill(unsigned int cpu)
 {
+	/* ensure CPU can enter the WFI/WFE state */
+	mdelay(1);
+
 	pmu_set_power_domain(0 + cpu, false);
 	return 1;
 }
-- 
1.9.1

[PATCH v5 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 v5:
    - Add the changelog.

Changes in v4: None

Changes in v3: None

Changes in v2:
    - Use the checkpatch.pl -f --subjective to check.

---

 arch/arm/mach-rockchip/platsmp.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-rockchip/platsmp.c b/arch/arm/mach-rockchip/platsmp.c
index 5bc2a89..e538b13 100644
--- a/arch/arm/mach-rockchip/platsmp.c
+++ b/arch/arm/mach-rockchip/platsmp.c
@@ -103,7 +103,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;
 		}
 	}
@@ -133,7 +133,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;
 	}
 
@@ -152,7 +152,7 @@ static int __cpuinit rockchip_boot_secondary(unsigned int cpu,
 		 * */
 		mdelay(1);
 		writel(virt_to_phys(rockchip_secondary_startup),
-			sram_base_addr + 8);
+		       sram_base_addr + 8);
 		writel(0xDEADBEAF, sram_base_addr + 4);
 		dsb_sev();
 	}
@@ -331,7 +331,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
@@ -344,4 +344,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

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

[Caesar Wang]

在 2015年06月08日 15:11, 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 reproducile 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
>      [ 4072.587426] CPU3: Booted secondary processor
>      <hang>
>
> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
> Reviewed-by: Doug Anderson <dianders@chromium.org>
>
> Changes in v5:
>      - 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.
>
> Changes in v4:
>      - Add reset_control_put(rstc) for the non-error case.
>
> Changes in v3:
>      - FIx the PATCH v2, it doesn't work on chromium 3.14.
>
> Changes in v2:
>      - 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

Sorry for making a mistake in here.
'Series-changes' isn't  that 'Changes in' with patman.

That's why the change log can't be 'below the cut'.

> ---
>
>   arch/arm/mach-rockchip/platsmp.c | 18 ++++++++++--------
>   1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm/mach-rockchip/platsmp.c b/arch/arm/mach-rockchip/platsmp.c
> index 5b4ca3c..bd40852 100644
> --- a/arch/arm/mach-rockchip/platsmp.c
> +++ b/arch/arm/mach-rockchip/platsmp.c
> @@ -72,6 +72,7 @@ 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;
>   
>   	/*
> @@ -80,20 +81,15 @@ static int pmu_set_power_domain(int pd, bool on)
>   	 * processor is powered down.
>   	 */
>   	if (read_cpuid_part() != ARM_CPU_PART_CORTEX_A9) {
> -		struct reset_control *rstc = rockchip_get_core_reset(pd);
> -
> +		/* We only require the reset on the RK3288 at the moment */
>   		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
> +		if (!on)
>   			reset_control_assert(rstc);
> -
> -		reset_control_put(rstc);
>   	}
>   
>   	ret = regmap_update_bits(pmu, PMU_PWRDN_CON, BIT(pd), val);
> @@ -112,6 +108,12 @@ static int pmu_set_power_domain(int pd, bool on)
>   		}
>   	}
>   
> +	if (read_cpuid_part() != ARM_CPU_PART_CORTEX_A9 && on)
> +		reset_control_deassert(rstc);
> +
> +	if (!IS_ERR(rstc))
> +		reset_control_put(rstc);
> +
>   	return 0;
>   }
>   
> @@ -148,7 +150,7 @@ static int __cpuinit rockchip_boot_secondary(unsigned int cpu,
>   		 * sram_base_addr + 4: 0xdeadbeaf
>   		 * sram_base_addr + 8: start address for pc
>   		 * */
> -		udelay(10);
> +		mdelay(1);
>   		writel(virt_to_phys(rockchip_secondary_startup),
>   			sram_base_addr + 8);
>   		writel(0xDEADBEAF, sram_base_addr + 4);

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

[Russell King - ARM Linux]

On Mon, Jun 08, 2015 at 03:11:34PM +0800, 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 reproducile log:

"reproducable"

>     [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
>     [ 4072.587426] CPU3: Booted secondary processor
>     <hang>
> 
> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
> Reviewed-by: Doug Anderson <dianders@chromium.org>
> 
> Changes in v5:
>     - 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.
> 
> Changes in v4:
>     - Add reset_control_put(rstc) for the non-error case.
> 
> Changes in v3:
>     - FIx the PATCH v2, it doesn't work on chromium 3.14.
> 
> Changes in v2:
>     - 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 | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/mach-rockchip/platsmp.c b/arch/arm/mach-rockchip/platsmp.c
> index 5b4ca3c..bd40852 100644
> --- a/arch/arm/mach-rockchip/platsmp.c
> +++ b/arch/arm/mach-rockchip/platsmp.c
> @@ -72,6 +72,7 @@ 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;
>  
>  	/*
> @@ -80,20 +81,15 @@ static int pmu_set_power_domain(int pd, bool on)
>  	 * processor is powered down.
>  	 */
>  	if (read_cpuid_part() != ARM_CPU_PART_CORTEX_A9) {
> -		struct reset_control *rstc = rockchip_get_core_reset(pd);
> -
> +		/* We only require the reset on the RK3288 at the moment */
>  		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
> +		if (!on)
>  			reset_control_assert(rstc);
> -
> -		reset_control_put(rstc);
>  	}
>  
>  	ret = regmap_update_bits(pmu, PMU_PWRDN_CON, BIT(pd), val);
> @@ -112,6 +108,12 @@ static int pmu_set_power_domain(int pd, bool on)
>  		}
>  	}
>  
> +	if (read_cpuid_part() != ARM_CPU_PART_CORTEX_A9 && on)
> +		reset_control_deassert(rstc);
> +
> +	if (!IS_ERR(rstc))
> +		reset_control_put(rstc);
> +
>  	return 0;
>  }
>  
> @@ -148,7 +150,7 @@ static int __cpuinit rockchip_boot_secondary(unsigned int cpu,
>  		 * sram_base_addr + 4: 0xdeadbeaf
>  		 * sram_base_addr + 8: start address for pc
>  		 * */
> -		udelay(10);
> +		mdelay(1);

The reason for this delay needs a comment, as it's not obvious why you
would need to delay before writing to the SRAM.  Also documenting in
a comment why the delay is necessary would be good.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

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

[Russell King - ARM Linux]

On Mon, Jun 08, 2015 at 03:11:35PM +0800, Caesar Wang wrote:
> diff --git a/arch/arm/mach-rockchip/platsmp.c b/arch/arm/mach-rockchip/platsmp.c
> index bd40852..5bc2a89 100644
> --- a/arch/arm/mach-rockchip/platsmp.c
> +++ b/arch/arm/mach-rockchip/platsmp.c
> @@ -321,6 +321,9 @@ static void __init rockchip_smp_prepare_cpus(unsigned int max_cpus)
>  #ifdef CONFIG_HOTPLUG_CPU
>  static int rockchip_cpu_kill(unsigned int cpu)
>  {
> +	/* ensure CPU can enter the WFI/WFE state */

I would like to see a better comment here, describing what the problem
is.  Maybe something like this:

	/*
	 * 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.
	 */

Thanks.

> +	mdelay(1);
> +
>  	pmu_set_power_domain(0 + cpu, false);
>  	return 1;
>  }
> -- 
> 1.9.1
> 

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

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

[Russell King - ARM Linux]

On Mon, Jun 08, 2015 at 03:11:34PM +0800, Caesar Wang wrote:
> diff --git a/arch/arm/mach-rockchip/platsmp.c b/arch/arm/mach-rockchip/platsmp.c
> index 5b4ca3c..bd40852 100644
> --- a/arch/arm/mach-rockchip/platsmp.c
> +++ b/arch/arm/mach-rockchip/platsmp.c
> @@ -72,6 +72,7 @@ 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;
>  
>  	/*
> @@ -80,20 +81,15 @@ static int pmu_set_power_domain(int pd, bool on)
>  	 * processor is powered down.
>  	 */
>  	if (read_cpuid_part() != ARM_CPU_PART_CORTEX_A9) {
> -		struct reset_control *rstc = rockchip_get_core_reset(pd);
> -
> +		/* We only require the reset on the RK3288 at the moment */
>  		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
> +		if (!on)
>  			reset_control_assert(rstc);
> -
> -		reset_control_put(rstc);
>  	}

Do we need the CPU part number check for the assertion/deassertion of
the reset control?  Surely the DT provides this where appropriate and
omits it where it's inappropriate?

If so, I'd separate out the decision whether to error out from the
decision whether to assert the reset control, like this:

	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);
	}

	if (!IS_ERR(rstc) && !on)
		reset_control_assert(rstc);

or maybe:

	if (IS_ERR(rstc) && PTR_ERR(rstc) != -ENOENT) {
		pr_err("%s: could not get reset control for core %d: %d\n",
		       __func__, pd, PTR_ERR(rstc));
		return PTR_ERR(rstc);
	}

	if (!IS_ERR(rstc) && !on)
		reset_control_assert(rstc);

which lets you detect whether of_reset_control_get() failed because the
reset control was not present (and thus not required) vs failed for some
other reason - and this means that the issue of whether the reset
control is used is entirely up to the supplied DT file.

>  
>  	ret = regmap_update_bits(pmu, PMU_PWRDN_CON, BIT(pd), val);
> @@ -112,6 +108,12 @@ static int pmu_set_power_domain(int pd, bool on)
>  		}
>  	}
>  
> +	if (read_cpuid_part() != ARM_CPU_PART_CORTEX_A9 && on)
> +		reset_control_deassert(rstc);
> +
> +	if (!IS_ERR(rstc))
> +		reset_control_put(rstc);
> +

and here:
	if (!IS_ERR(rstc)) {
		if (on)
			reset_control_deassert(rstc);
		reset_control_put(rstc);
	}

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

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

[Caesar Wang]

在 2015年06月08日 17:24, Russell King - ARM Linux 写道:
> On Mon, Jun 08, 2015 at 03:11:34PM +0800, 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 reproducile log:
> "reproducable"
>
>>      [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
>>      [ 4072.587426] CPU3: Booted secondary processor
>>      <hang>
>>
>> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
>> Reviewed-by: Doug Anderson <dianders@chromium.org>
>>
>> Changes in v5:
>>      - 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.
>>
>> Changes in v4:
>>      - Add reset_control_put(rstc) for the non-error case.
>>
>> Changes in v3:
>>      - FIx the PATCH v2, it doesn't work on chromium 3.14.
>>
>> Changes in v2:
>>      - 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 | 18 ++++++++++--------
>>   1 file changed, 10 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/arm/mach-rockchip/platsmp.c b/arch/arm/mach-rockchip/platsmp.c
>> index 5b4ca3c..bd40852 100644
>> --- a/arch/arm/mach-rockchip/platsmp.c
>> +++ b/arch/arm/mach-rockchip/platsmp.c
>> @@ -72,6 +72,7 @@ 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;
>>   
>>   	/*
>> @@ -80,20 +81,15 @@ static int pmu_set_power_domain(int pd, bool on)
>>   	 * processor is powered down.
>>   	 */
>>   	if (read_cpuid_part() != ARM_CPU_PART_CORTEX_A9) {
>> -		struct reset_control *rstc = rockchip_get_core_reset(pd);
>> -
>> +		/* We only require the reset on the RK3288 at the moment */
>>   		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
>> +		if (!on)
>>   			reset_control_assert(rstc);
>> -
>> -		reset_control_put(rstc);
>>   	}
>>   
>>   	ret = regmap_update_bits(pmu, PMU_PWRDN_CON, BIT(pd), val);
>> @@ -112,6 +108,12 @@ static int pmu_set_power_domain(int pd, bool on)
>>   		}
>>   	}
>>   
>> +	if (read_cpuid_part() != ARM_CPU_PART_CORTEX_A9 && on)
>> +		reset_control_deassert(rstc);
>> +
>> +	if (!IS_ERR(rstc))
>> +		reset_control_put(rstc);
>> +
>>   	return 0;
>>   }
>>   
>> @@ -148,7 +150,7 @@ static int __cpuinit rockchip_boot_secondary(unsigned int cpu,
>>   		 * sram_base_addr + 4: 0xdeadbeaf
>>   		 * sram_base_addr + 8: start address for pc
>>   		 * */
>> -		udelay(10);
>> +		mdelay(1);
> The reason for this delay needs a comment, as it's not obvious why you
> would need to delay before writing to the SRAM.  Also documenting in
> a comment why the delay is necessary would be good.
>

Sorry for delay, I wait a better solution for this.
We don't need any 10us delay or 1m delay, I think.

-		udelay(10);

+        while (readl(sram_base_addr + 4 ) != 1); //lock =1


We need do that if i'm correct from the bootrom code.
Tested are pass over 120000 cycles on today, I will wait more testing 
cycles to confirm that's ok.



-- 
**************************************************************************************
王晓腾    Caesar Wang
Product R&D Dept.III
Fuzhou Rockchip Electronics Co.Ltd
Addr：  NO.18 Building, A District, Fuzhou Software Park,Gulou District,Fuzhou, Fujian,China(Fuzhou Headquarters)
               21F,Malata Building,Kejizhongyi Avenue,Nanshan District,Shenzhen  (Shenzhen Office)
Tel：+86-591-83991906/07 - 8221
Mobile:+86 15059456742
E-mail : wxt@rock-chips.com
***************************************************************************************
***************************************************************************************
IMPORTANT NOTICE: This email is from Fuzhou Rockchip Electronics Co., Ltd .The contents of this email and any attachments may
contain information that is privileged, confidential and/or exempt from disclosure under applicable law and relevant NDA.
If you are not the intended recipient, you are hereby notified that any disclosure, copying, distribution, or use of the
information is STRICTLY PROHIBITED. Please immediately contact the sender as soon as possible and destroy the material
in its entirety in any format. Thank you.
***************************************************************************************

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

[Caesar Wang]

在 2015年06月08日 17:28, Russell King - ARM Linux 写道:
> On Mon, Jun 08, 2015 at 03:11:35PM +0800, Caesar Wang wrote:
>> diff --git a/arch/arm/mach-rockchip/platsmp.c b/arch/arm/mach-rockchip/platsmp.c
>> index bd40852..5bc2a89 100644
>> --- a/arch/arm/mach-rockchip/platsmp.c
>> +++ b/arch/arm/mach-rockchip/platsmp.c
>> @@ -321,6 +321,9 @@ static void __init rockchip_smp_prepare_cpus(unsigned int max_cpus)
>>   #ifdef CONFIG_HOTPLUG_CPU
>>   static int rockchip_cpu_kill(unsigned int cpu)
>>   {
>> +	/* ensure CPU can enter the WFI/WFE state */
> I would like to see a better comment here, describing what the problem
> is.  Maybe something like this:
>
> 	/*
> 	 * 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.
> 	 */
>
> Thanks.

OK,
Thanks!
>> +	mdelay(1);
>> +
>>   	pmu_set_power_domain(0 + cpu, false);
>>   	return 1;
>>   }
>> -- 
>> 1.9.1
>>

-- 
Thanks,
- Caesar

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

[Caesar Wang]

在 2015年06月08日 17:43, Russell King - ARM Linux 写道:
> On Mon, Jun 08, 2015 at 03:11:34PM +0800, Caesar Wang wrote:
>> diff --git a/arch/arm/mach-rockchip/platsmp.c b/arch/arm/mach-rockchip/platsmp.c
>> index 5b4ca3c..bd40852 100644
>> --- a/arch/arm/mach-rockchip/platsmp.c
>> +++ b/arch/arm/mach-rockchip/platsmp.c
>> @@ -72,6 +72,7 @@ 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;
>>   
>>   	/*
>> @@ -80,20 +81,15 @@ static int pmu_set_power_domain(int pd, bool on)
>>   	 * processor is powered down.
>>   	 */
>>   	if (read_cpuid_part() != ARM_CPU_PART_CORTEX_A9) {
>> -		struct reset_control *rstc = rockchip_get_core_reset(pd);
>> -
>> +		/* We only require the reset on the RK3288 at the moment */
>>   		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
>> +		if (!on)
>>   			reset_control_assert(rstc);
>> -
>> -		reset_control_put(rstc);
>>   	}
> Do we need the CPU part number check for the assertion/deassertion of
> the reset control?  Surely the DT provides this where appropriate and
> omits it where it's inappropriate?
>
> If so, I'd separate out the decision whether to error out from the
> decision whether to assert the reset control, like this:
>
> 	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);
> 	}
>
> 	if (!IS_ERR(rstc) && !on)
> 		reset_control_assert(rstc);
>
> or maybe:
>
> 	if (IS_ERR(rstc) && PTR_ERR(rstc) != -ENOENT) {
> 		pr_err("%s: could not get reset control for core %d: %d\n",
> 		       __func__, pd, PTR_ERR(rstc));
> 		return PTR_ERR(rstc);
> 	}
>
> 	if (!IS_ERR(rstc) && !on)
> 		reset_control_assert(rstc);
>
> which lets you detect whether of_reset_control_get() failed because the
> reset control was not present (and thus not required) vs failed for some
> other reason - and this means that the issue of whether the reset
> control is used is entirely up to the supplied DT file.
Sound resonable for me.

I hope get the Heiko and Kever option.
After all the code is uploaded by Heiko and Kever.

>>   
>>   	ret = regmap_update_bits(pmu, PMU_PWRDN_CON, BIT(pd), val);
>> @@ -112,6 +108,12 @@ static int pmu_set_power_domain(int pd, bool on)
>>   		}
>>   	}
>>   
>> +	if (read_cpuid_part() != ARM_CPU_PART_CORTEX_A9 && on)
>> +		reset_control_deassert(rstc);
>> +
>> +	if (!IS_ERR(rstc))
>> +		reset_control_put(rstc);
>> +
> and here:
> 	if (!IS_ERR(rstc)) {
> 		if (on)
> 			reset_control_deassert(rstc);
> 		reset_control_put(rstc);
> 	}
Ditto.


-- 
Thanks,
- Caesar

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

[Heiko Stübner]

Am Dienstag, 9. Juni 2015, 08:43:56 schrieb Caesar Wang:
> 在 2015年06月08日 17:43, Russell King - ARM Linux 写道:
> > On Mon, Jun 08, 2015 at 03:11:34PM +0800, Caesar Wang wrote:
> >> diff --git a/arch/arm/mach-rockchip/platsmp.c
> >> b/arch/arm/mach-rockchip/platsmp.c index 5b4ca3c..bd40852 100644
> >> --- a/arch/arm/mach-rockchip/platsmp.c
> >> +++ b/arch/arm/mach-rockchip/platsmp.c
> >> @@ -72,6 +72,7 @@ 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;
> >>   	
> >>   	/*
> >> 
> >> @@ -80,20 +81,15 @@ static int pmu_set_power_domain(int pd, bool on)
> >> 
> >>   	 * processor is powered down.
> >>   	 */
> >>   	
> >>   	if (read_cpuid_part() != ARM_CPU_PART_CORTEX_A9) {
> >> 
> >> -		struct reset_control *rstc = rockchip_get_core_reset(pd);
> >> -
> >> +		/* We only require the reset on the RK3288 at the moment */
> >> 
> >>   		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
> >> +		if (!on)
> >> 
> >>   			reset_control_assert(rstc);
> >> 
> >> -
> >> -		reset_control_put(rstc);
> >> 
> >>   	}
> > 
> > Do we need the CPU part number check for the assertion/deassertion of
> > the reset control?  Surely the DT provides this where appropriate and
> > omits it where it's inappropriate?
> > 
> > If so, I'd separate out the decision whether to error out from the
> > 
> > decision whether to assert the reset control, like this:
> > 	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);
> > 	
> > 	}
> > 	
> > 	if (!IS_ERR(rstc) && !on)
> > 	
> > 		reset_control_assert(rstc);
> > 
> > or maybe:
> > 	if (IS_ERR(rstc) && PTR_ERR(rstc) != -ENOENT) {
> > 	
> > 		pr_err("%s: could not get reset control for core %d: %d\n",
> > 		
> > 		       __func__, pd, PTR_ERR(rstc));
> > 		
> > 		return PTR_ERR(rstc);
> > 	
> > 	}
> > 	
> > 	if (!IS_ERR(rstc) && !on)
> > 	
> > 		reset_control_assert(rstc);
> > 
> > which lets you detect whether of_reset_control_get() failed because the
> > reset control was not present (and thus not required) vs failed for some
> > other reason - and this means that the issue of whether the reset
> > control is used is entirely up to the supplied DT file.
> 
> Sound resonable for me.
> 
> I hope get the Heiko and Kever option.
> After all the code is uploaded by Heiko and Kever.

Russell's suggestion is very nice indeed.

We have reset controls for the cpu cores on all Rockchip SoCs but currently 
only absolutely require them on the rk3288, so what Russell suggested handles 
this nicely if we require this later for other SoCs as well (like the A7 ones) 
.


Heiko

> 
> >>   	ret = regmap_update_bits(pmu, PMU_PWRDN_CON, BIT(pd), val);
> >> 
> >> @@ -112,6 +108,12 @@ static int pmu_set_power_domain(int pd, bool on)
> >> 
> >>   		}
> >>   	
> >>   	}
> >> 
> >> +	if (read_cpuid_part() != ARM_CPU_PART_CORTEX_A9 && on)
> >> +		reset_control_deassert(rstc);
> >> +
> >> +	if (!IS_ERR(rstc))
> >> +		reset_control_put(rstc);
> >> +
> > 
> > and here:
> > 	if (!IS_ERR(rstc)) {
> > 	
> > 		if (on)
> > 		
> > 			reset_control_deassert(rstc);
> > 		
> > 		reset_control_put(rstc);
> > 	
> > 	}
> 
> Ditto.

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

[Caesar Wang]

在 2015年06月09日 05:54, Caesar Wang 写道:
>
>
> 在 2015年06月08日 17:24, Russell King - ARM Linux 写道:
>> On Mon, Jun 08, 2015 at 03:11:34PM +0800, 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 reproducile log:
>> "reproducable"
>>
>>>      [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
>>>      [ 4072.587426] CPU3: Booted secondary processor
>>>      <hang>
>>>
>>> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
>>> Reviewed-by: Doug Anderson <dianders@chromium.org>
>>>
>>> Changes in v5:
>>>      - 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.
>>>
>>> Changes in v4:
>>>      - Add reset_control_put(rstc) for the non-error case.
>>>
>>> Changes in v3:
>>>      - FIx the PATCH v2, it doesn't work on chromium 3.14.
>>>
>>> Changes in v2:
>>>      - 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 | 18 ++++++++++--------
>>>   1 file changed, 10 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-rockchip/platsmp.c 
>>> b/arch/arm/mach-rockchip/platsmp.c
>>> index 5b4ca3c..bd40852 100644
>>> --- a/arch/arm/mach-rockchip/platsmp.c
>>> +++ b/arch/arm/mach-rockchip/platsmp.c
>>> @@ -72,6 +72,7 @@ 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;
>>>         /*
>>> @@ -80,20 +81,15 @@ static int pmu_set_power_domain(int pd, bool on)
>>>        * processor is powered down.
>>>        */
>>>       if (read_cpuid_part() != ARM_CPU_PART_CORTEX_A9) {
>>> -        struct reset_control *rstc = rockchip_get_core_reset(pd);
>>> -
>>> +        /* We only require the reset on the RK3288 at the moment */
>>>           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
>>> +        if (!on)
>>>               reset_control_assert(rstc);
>>> -
>>> -        reset_control_put(rstc);
>>>       }
>>>         ret = regmap_update_bits(pmu, PMU_PWRDN_CON, BIT(pd), val);
>>> @@ -112,6 +108,12 @@ static int pmu_set_power_domain(int pd, bool on)
>>>           }
>>>       }
>>>   +    if (read_cpuid_part() != ARM_CPU_PART_CORTEX_A9 && on)
>>> +        reset_control_deassert(rstc);
>>> +
>>> +    if (!IS_ERR(rstc))
>>> +        reset_control_put(rstc);
>>> +
>>>       return 0;
>>>   }
>>>   @@ -148,7 +150,7 @@ static int __cpuinit 
>>> rockchip_boot_secondary(unsigned int cpu,
>>>            * sram_base_addr + 4: 0xdeadbeaf
>>>            * sram_base_addr + 8: start address for pc
>>>            * */
>>> -        udelay(10);
>>> +        mdelay(1);
>> The reason for this delay needs a comment, as it's not obvious why you
>> would need to delay before writing to the SRAM.  Also documenting in
>> a comment why the delay is necessary would be good.
>>
>
> Sorry for delay, I wait a better solution for this.
> We don't need any 10us delay or 1m delay, I think.
>
> -        udelay(10);
>
> +        while (readl(sram_base_addr + 4 ) != 1); //lock =1
>
>
> We need do that if i'm correct from the bootrom code.
> Tested are pass over 120000 cycles on today, I will wait more testing 
> cycles to confirm that's ok.
>

Please forget it!

It's helpful that be caused by adding a log.:-(

>
>

-- 
Thanks,
- Caesar