Re: [RESEND PATCH] ARM: EXYNOS: Fix the sequence of secondary CPU boot for Exynos3250

2014-06-11 Thread Krzysztof Kozlowski
On śro, 2014-06-11 at 01:57 +0200, Tomasz Figa wrote:
> On 11.06.2014 01:44, Chanwoo Choi wrote:
> > On 06/11/2014 08:35 AM, Tomasz Figa wrote:
> >> Hi Chanwoo,
> >>
> >> On 11.06.2014 01:27, Chanwoo Choi wrote:
> >>> This patch set AUTOWAKEUP_EN bit to ARM_CORE_CONFIGURATION register
> >>> because Exynos3250 removes WFE in secure mode so that turn on 
> >>> automatically
> >>> after setting CORE_LOCAL_PWR_EN. Also, This patch use dbs_sev() macro
> >>> to guarantee the data synchronization of command instead of IPI_WAKEUP
> >>> because Exynos3250 don't have WFE mode in secue mode.
> >>>
> >>> Signed-off-by: Chanwoo Choi 
> >>> Acked-by: Kyungmin Park 
> >>> ---
> >>>  arch/arm/mach-exynos/platsmp.c  | 9 -
> >>>  arch/arm/mach-exynos/pm.c   | 8 ++--
> >>>  arch/arm/mach-exynos/regs-pmu.h | 4 
> >>>  3 files changed, 18 insertions(+), 3 deletions(-)
> >>>
> >>
> >> This patch seems to be unneeded with Krzysztof's patch send a while ago
> >> [1]. As reported by Krzysztof, that patch apparently fixes SMP support
> >> on Exynos3250 and is much smaller and less invasive.
> >>
> >> [1] - http://thread.gmane.org/gmane.linux.kernel.samsung-soc/32809
> > 
> > OK,
> > But Krzysztof's patch didn't include set S5P_CORE_AUTOWAKEUP_EN in 
> > EXYNOS_ARM_CORE_CONFIGURATION(cpu).
> > and then use arch_send_wakeup_ipi_mask(cpumask_of(cpu)) command instead of 
> > dsb_sev(). Exynos3250 don't need
> > send IPI message.
> 
> I don't know technical details about CPU boot-up on Exynos3250 as I
> haven't worked too much with this platform. According to my conversation
> with Krzysztof, he found S5P_CORE_AUTOWAKEUP_EN and dsb_sev() to be not
> needed. Instead S5P_CORE_WAKEUP_FROM_LOCAL_CFG can be set in
> EXYNOS_ARM_CORE1_STATUS and then normal arch_send_wakeup_ipi_mask()
> used. He might be able to provide more details.

I tried to avoid setting S5P_CORE_AUTOWAKEUP_EN because of simple
reason: in my SoC documentation this field is not documented. The
ARM_CORE_CONFIGURATION register has only "LOCAL_PWR_CFG" field and
nothing more.

As for the IPI wakeup - I think it makes code cleaner than adding 'if'
statement for specific chipset.

Best regards,
Krzysztof



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RESEND PATCH] ARM: EXYNOS: Fix the sequence of secondary CPU boot for Exynos3250

2014-06-11 Thread Krzysztof Kozlowski
On śro, 2014-06-11 at 01:57 +0200, Tomasz Figa wrote:
 On 11.06.2014 01:44, Chanwoo Choi wrote:
  On 06/11/2014 08:35 AM, Tomasz Figa wrote:
  Hi Chanwoo,
 
  On 11.06.2014 01:27, Chanwoo Choi wrote:
  This patch set AUTOWAKEUP_EN bit to ARM_CORE_CONFIGURATION register
  because Exynos3250 removes WFE in secure mode so that turn on 
  automatically
  after setting CORE_LOCAL_PWR_EN. Also, This patch use dbs_sev() macro
  to guarantee the data synchronization of command instead of IPI_WAKEUP
  because Exynos3250 don't have WFE mode in secue mode.
 
  Signed-off-by: Chanwoo Choi cw00.c...@samsung.com
  Acked-by: Kyungmin Park kyungmin.p...@samsung.com
  ---
   arch/arm/mach-exynos/platsmp.c  | 9 -
   arch/arm/mach-exynos/pm.c   | 8 ++--
   arch/arm/mach-exynos/regs-pmu.h | 4 
   3 files changed, 18 insertions(+), 3 deletions(-)
 
 
  This patch seems to be unneeded with Krzysztof's patch send a while ago
  [1]. As reported by Krzysztof, that patch apparently fixes SMP support
  on Exynos3250 and is much smaller and less invasive.
 
  [1] - http://thread.gmane.org/gmane.linux.kernel.samsung-soc/32809
  
  OK,
  But Krzysztof's patch didn't include set S5P_CORE_AUTOWAKEUP_EN in 
  EXYNOS_ARM_CORE_CONFIGURATION(cpu).
  and then use arch_send_wakeup_ipi_mask(cpumask_of(cpu)) command instead of 
  dsb_sev(). Exynos3250 don't need
  send IPI message.
 
 I don't know technical details about CPU boot-up on Exynos3250 as I
 haven't worked too much with this platform. According to my conversation
 with Krzysztof, he found S5P_CORE_AUTOWAKEUP_EN and dsb_sev() to be not
 needed. Instead S5P_CORE_WAKEUP_FROM_LOCAL_CFG can be set in
 EXYNOS_ARM_CORE1_STATUS and then normal arch_send_wakeup_ipi_mask()
 used. He might be able to provide more details.

I tried to avoid setting S5P_CORE_AUTOWAKEUP_EN because of simple
reason: in my SoC documentation this field is not documented. The
ARM_CORE_CONFIGURATION register has only LOCAL_PWR_CFG field and
nothing more.

As for the IPI wakeup - I think it makes code cleaner than adding 'if'
statement for specific chipset.

Best regards,
Krzysztof



--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RESEND PATCH] ARM: EXYNOS: Fix the sequence of secondary CPU boot for Exynos3250

2014-06-10 Thread Tomasz Figa
On 11.06.2014 01:44, Chanwoo Choi wrote:
> On 06/11/2014 08:35 AM, Tomasz Figa wrote:
>> Hi Chanwoo,
>>
>> On 11.06.2014 01:27, Chanwoo Choi wrote:
>>> This patch set AUTOWAKEUP_EN bit to ARM_CORE_CONFIGURATION register
>>> because Exynos3250 removes WFE in secure mode so that turn on automatically
>>> after setting CORE_LOCAL_PWR_EN. Also, This patch use dbs_sev() macro
>>> to guarantee the data synchronization of command instead of IPI_WAKEUP
>>> because Exynos3250 don't have WFE mode in secue mode.
>>>
>>> Signed-off-by: Chanwoo Choi 
>>> Acked-by: Kyungmin Park 
>>> ---
>>>  arch/arm/mach-exynos/platsmp.c  | 9 -
>>>  arch/arm/mach-exynos/pm.c   | 8 ++--
>>>  arch/arm/mach-exynos/regs-pmu.h | 4 
>>>  3 files changed, 18 insertions(+), 3 deletions(-)
>>>
>>
>> This patch seems to be unneeded with Krzysztof's patch send a while ago
>> [1]. As reported by Krzysztof, that patch apparently fixes SMP support
>> on Exynos3250 and is much smaller and less invasive.
>>
>> [1] - http://thread.gmane.org/gmane.linux.kernel.samsung-soc/32809
> 
> OK,
> But Krzysztof's patch didn't include set S5P_CORE_AUTOWAKEUP_EN in 
> EXYNOS_ARM_CORE_CONFIGURATION(cpu).
> and then use arch_send_wakeup_ipi_mask(cpumask_of(cpu)) command instead of 
> dsb_sev(). Exynos3250 don't need
> send IPI message.

I don't know technical details about CPU boot-up on Exynos3250 as I
haven't worked too much with this platform. According to my conversation
with Krzysztof, he found S5P_CORE_AUTOWAKEUP_EN and dsb_sev() to be not
needed. Instead S5P_CORE_WAKEUP_FROM_LOCAL_CFG can be set in
EXYNOS_ARM_CORE1_STATUS and then normal arch_send_wakeup_ipi_mask()
used. He might be able to provide more details.

> 
> I'll send next patch which include only S5P_CORE_AUTOWAKEUP_EN and without 
> sending IPI message.

I believe Krzysztof was first with this kind of patch and I'd prefer his
approach as it introduces less changes. (First version posted on 15.05
[1], with my comments addressed in v2.)

[1] http://thread.gmane.org/gmane.linux.kernel.samsung-soc/31614

> 
> Krzysztof's patch used of_machine_is_compatible("samsung,exynos3250") instead 
> of soc_is_exynos3250().
> Did you agree? If you agree to use of_machine_is_compatible(), I'll use it on 
> next patch(v2).

IMHO, there is no significant difference between those two. Both are
bad, because they bind quirks to specific SoCs and it's hard to reuse
them for new ones - every time a quirk is reused by a new SoC, a new (||
soc_is_xxx() or || of_machine_is_compatible("xxx")) must be added.

Best regards,
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RESEND PATCH] ARM: EXYNOS: Fix the sequence of secondary CPU boot for Exynos3250

2014-06-10 Thread Chanwoo Choi
On 06/11/2014 08:35 AM, Tomasz Figa wrote:
> Hi Chanwoo,
> 
> On 11.06.2014 01:27, Chanwoo Choi wrote:
>> This patch set AUTOWAKEUP_EN bit to ARM_CORE_CONFIGURATION register
>> because Exynos3250 removes WFE in secure mode so that turn on automatically
>> after setting CORE_LOCAL_PWR_EN. Also, This patch use dbs_sev() macro
>> to guarantee the data synchronization of command instead of IPI_WAKEUP
>> because Exynos3250 don't have WFE mode in secue mode.
>>
>> Signed-off-by: Chanwoo Choi 
>> Acked-by: Kyungmin Park 
>> ---
>>  arch/arm/mach-exynos/platsmp.c  | 9 -
>>  arch/arm/mach-exynos/pm.c   | 8 ++--
>>  arch/arm/mach-exynos/regs-pmu.h | 4 
>>  3 files changed, 18 insertions(+), 3 deletions(-)
>>
> 
> This patch seems to be unneeded with Krzysztof's patch send a while ago
> [1]. As reported by Krzysztof, that patch apparently fixes SMP support
> on Exynos3250 and is much smaller and less invasive.
> 
> [1] - http://thread.gmane.org/gmane.linux.kernel.samsung-soc/32809

OK,
But Krzysztof's patch didn't include set S5P_CORE_AUTOWAKEUP_EN in 
EXYNOS_ARM_CORE_CONFIGURATION(cpu).
and then use arch_send_wakeup_ipi_mask(cpumask_of(cpu)) command instead of 
dsb_sev(). Exynos3250 don't need
send IPI message.

I'll send next patch which include only S5P_CORE_AUTOWAKEUP_EN and without 
sending IPI message.

Krzysztof's patch used of_machine_is_compatible("samsung,exynos3250") instead 
of soc_is_exynos3250().
Did you agree? If you agree to use of_machine_is_compatible(), I'll use it on 
next patch(v2).

Best Regards,
Chanwoo Choi

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RESEND PATCH] ARM: EXYNOS: Fix the sequence of secondary CPU boot for Exynos3250

2014-06-10 Thread Tomasz Figa
Hi Chanwoo,

On 11.06.2014 01:27, Chanwoo Choi wrote:
> This patch set AUTOWAKEUP_EN bit to ARM_CORE_CONFIGURATION register
> because Exynos3250 removes WFE in secure mode so that turn on automatically
> after setting CORE_LOCAL_PWR_EN. Also, This patch use dbs_sev() macro
> to guarantee the data synchronization of command instead of IPI_WAKEUP
> because Exynos3250 don't have WFE mode in secue mode.
> 
> Signed-off-by: Chanwoo Choi 
> Acked-by: Kyungmin Park 
> ---
>  arch/arm/mach-exynos/platsmp.c  | 9 -
>  arch/arm/mach-exynos/pm.c   | 8 ++--
>  arch/arm/mach-exynos/regs-pmu.h | 4 
>  3 files changed, 18 insertions(+), 3 deletions(-)
> 

This patch seems to be unneeded with Krzysztof's patch send a while ago
[1]. As reported by Krzysztof, that patch apparently fixes SMP support
on Exynos3250 and is much smaller and less invasive.

[1] - http://thread.gmane.org/gmane.linux.kernel.samsung-soc/32809

Best regards,
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RESEND PATCH] ARM: EXYNOS: Fix the sequence of secondary CPU boot for Exynos3250

2014-06-10 Thread Tomasz Figa
Hi Chanwoo,

On 11.06.2014 01:27, Chanwoo Choi wrote:
 This patch set AUTOWAKEUP_EN bit to ARM_CORE_CONFIGURATION register
 because Exynos3250 removes WFE in secure mode so that turn on automatically
 after setting CORE_LOCAL_PWR_EN. Also, This patch use dbs_sev() macro
 to guarantee the data synchronization of command instead of IPI_WAKEUP
 because Exynos3250 don't have WFE mode in secue mode.
 
 Signed-off-by: Chanwoo Choi cw00.c...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 ---
  arch/arm/mach-exynos/platsmp.c  | 9 -
  arch/arm/mach-exynos/pm.c   | 8 ++--
  arch/arm/mach-exynos/regs-pmu.h | 4 
  3 files changed, 18 insertions(+), 3 deletions(-)
 

This patch seems to be unneeded with Krzysztof's patch send a while ago
[1]. As reported by Krzysztof, that patch apparently fixes SMP support
on Exynos3250 and is much smaller and less invasive.

[1] - http://thread.gmane.org/gmane.linux.kernel.samsung-soc/32809

Best regards,
Tomasz
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RESEND PATCH] ARM: EXYNOS: Fix the sequence of secondary CPU boot for Exynos3250

2014-06-10 Thread Chanwoo Choi
On 06/11/2014 08:35 AM, Tomasz Figa wrote:
 Hi Chanwoo,
 
 On 11.06.2014 01:27, Chanwoo Choi wrote:
 This patch set AUTOWAKEUP_EN bit to ARM_CORE_CONFIGURATION register
 because Exynos3250 removes WFE in secure mode so that turn on automatically
 after setting CORE_LOCAL_PWR_EN. Also, This patch use dbs_sev() macro
 to guarantee the data synchronization of command instead of IPI_WAKEUP
 because Exynos3250 don't have WFE mode in secue mode.

 Signed-off-by: Chanwoo Choi cw00.c...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 ---
  arch/arm/mach-exynos/platsmp.c  | 9 -
  arch/arm/mach-exynos/pm.c   | 8 ++--
  arch/arm/mach-exynos/regs-pmu.h | 4 
  3 files changed, 18 insertions(+), 3 deletions(-)

 
 This patch seems to be unneeded with Krzysztof's patch send a while ago
 [1]. As reported by Krzysztof, that patch apparently fixes SMP support
 on Exynos3250 and is much smaller and less invasive.
 
 [1] - http://thread.gmane.org/gmane.linux.kernel.samsung-soc/32809

OK,
But Krzysztof's patch didn't include set S5P_CORE_AUTOWAKEUP_EN in 
EXYNOS_ARM_CORE_CONFIGURATION(cpu).
and then use arch_send_wakeup_ipi_mask(cpumask_of(cpu)) command instead of 
dsb_sev(). Exynos3250 don't need
send IPI message.

I'll send next patch which include only S5P_CORE_AUTOWAKEUP_EN and without 
sending IPI message.

Krzysztof's patch used of_machine_is_compatible(samsung,exynos3250) instead 
of soc_is_exynos3250().
Did you agree? If you agree to use of_machine_is_compatible(), I'll use it on 
next patch(v2).

Best Regards,
Chanwoo Choi

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RESEND PATCH] ARM: EXYNOS: Fix the sequence of secondary CPU boot for Exynos3250

2014-06-10 Thread Tomasz Figa
On 11.06.2014 01:44, Chanwoo Choi wrote:
 On 06/11/2014 08:35 AM, Tomasz Figa wrote:
 Hi Chanwoo,

 On 11.06.2014 01:27, Chanwoo Choi wrote:
 This patch set AUTOWAKEUP_EN bit to ARM_CORE_CONFIGURATION register
 because Exynos3250 removes WFE in secure mode so that turn on automatically
 after setting CORE_LOCAL_PWR_EN. Also, This patch use dbs_sev() macro
 to guarantee the data synchronization of command instead of IPI_WAKEUP
 because Exynos3250 don't have WFE mode in secue mode.

 Signed-off-by: Chanwoo Choi cw00.c...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 ---
  arch/arm/mach-exynos/platsmp.c  | 9 -
  arch/arm/mach-exynos/pm.c   | 8 ++--
  arch/arm/mach-exynos/regs-pmu.h | 4 
  3 files changed, 18 insertions(+), 3 deletions(-)


 This patch seems to be unneeded with Krzysztof's patch send a while ago
 [1]. As reported by Krzysztof, that patch apparently fixes SMP support
 on Exynos3250 and is much smaller and less invasive.

 [1] - http://thread.gmane.org/gmane.linux.kernel.samsung-soc/32809
 
 OK,
 But Krzysztof's patch didn't include set S5P_CORE_AUTOWAKEUP_EN in 
 EXYNOS_ARM_CORE_CONFIGURATION(cpu).
 and then use arch_send_wakeup_ipi_mask(cpumask_of(cpu)) command instead of 
 dsb_sev(). Exynos3250 don't need
 send IPI message.

I don't know technical details about CPU boot-up on Exynos3250 as I
haven't worked too much with this platform. According to my conversation
with Krzysztof, he found S5P_CORE_AUTOWAKEUP_EN and dsb_sev() to be not
needed. Instead S5P_CORE_WAKEUP_FROM_LOCAL_CFG can be set in
EXYNOS_ARM_CORE1_STATUS and then normal arch_send_wakeup_ipi_mask()
used. He might be able to provide more details.

 
 I'll send next patch which include only S5P_CORE_AUTOWAKEUP_EN and without 
 sending IPI message.

I believe Krzysztof was first with this kind of patch and I'd prefer his
approach as it introduces less changes. (First version posted on 15.05
[1], with my comments addressed in v2.)

[1] http://thread.gmane.org/gmane.linux.kernel.samsung-soc/31614

 
 Krzysztof's patch used of_machine_is_compatible(samsung,exynos3250) instead 
 of soc_is_exynos3250().
 Did you agree? If you agree to use of_machine_is_compatible(), I'll use it on 
 next patch(v2).

IMHO, there is no significant difference between those two. Both are
bad, because they bind quirks to specific SoCs and it's hard to reuse
them for new ones - every time a quirk is reused by a new SoC, a new (||
soc_is_xxx() or || of_machine_is_compatible(xxx)) must be added.

Best regards,
Tomasz
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/