All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Qais Yousef <qais.yousef@imgtec.com>
To: Tony Wu <tung7970@gmail.com>
Cc: <linux-mips@linux-mips.org>, <ralf@linux-mips.org>,
	James Hogan <james.hogan@imgtec.com>,
	Paul Burton <paul.burton@imgtec.com>
Subject: Re: [PATCH] MIPS: Add platform specific secondary core init
Date: Thu, 18 Jun 2015 17:52:18 +0100	[thread overview]
Message-ID: <5582F742.30905@imgtec.com> (raw)
In-Reply-To: <20150618220254-tung7970@googlemail.com>

On 06/18/2015 03:05 PM, Tony Wu wrote:
> Define plat_smp_init_secondary() API, and move platform specific
> secondary core initialization to each platform.

I really can't see that this code being platform specific. Why would a 
platform care about the c0_status register when a secondary boot is 
initialised?

That comment about Malta in smp-mt.c is misleading.

Qais

>
> Signed-off-by: Tony Wu <tung7970@gmail.com>
> Cc: James Hogan <james.hogan@imgtec.com>
> Cc: Paul Burton <paul.burton@imgtec.com>
> Cc: linux-mips@linux-mips.org
> ---
>   arch/mips/include/asm/smp-ops.h  |    1 +
>   arch/mips/kernel/smp-cmp.c       |    8 ++------
>   arch/mips/kernel/smp-cps.c       |    4 ++--
>   arch/mips/kernel/smp-mt.c        |   12 ++----------
>   arch/mips/mti-malta/malta-init.c |   13 +++++++++++++
>   5 files changed, 20 insertions(+), 18 deletions(-)
>
> diff --git a/arch/mips/include/asm/smp-ops.h b/arch/mips/include/asm/smp-ops.h
> index 6ba1fb8..53ce7e1 100644
> --- a/arch/mips/include/asm/smp-ops.h
> +++ b/arch/mips/include/asm/smp-ops.h
> @@ -36,6 +36,7 @@ struct plat_smp_ops {
>   };
>   
>   extern void register_smp_ops(struct plat_smp_ops *ops);
> +extern void plat_smp_init_secondary(void)
>   
>   static inline void plat_smp_setup(void)
>   {
> diff --git a/arch/mips/kernel/smp-cmp.c b/arch/mips/kernel/smp-cmp.c
> index d5e0f94..e8c0258 100644
> --- a/arch/mips/kernel/smp-cmp.c
> +++ b/arch/mips/kernel/smp-cmp.c
> @@ -43,17 +43,13 @@ static void cmp_init_secondary(void)
>   {
>   	struct cpuinfo_mips *c __maybe_unused = &current_cpu_data;
>   
> -	/* Assume GIC is present */
> -	change_c0_status(ST0_IM, STATUSF_IP2 | STATUSF_IP3 | STATUSF_IP4 |
> -				 STATUSF_IP5 | STATUSF_IP6 | STATUSF_IP7);
> -
> -	/* Enable per-cpu interrupts: platform specific */
> -
>   #ifdef CONFIG_MIPS_MT_SMP
>   	if (cpu_has_mipsmt)
>   		c->vpe_id = (read_c0_tcbind() >> TCBIND_CURVPE_SHIFT) &
>   			TCBIND_CURVPE;
>   #endif
> +	/* Call platform specific init secondary */
> +	plat_smp_init_secondary();
>   }
>   
>   static void cmp_smp_finish(void)
> diff --git a/arch/mips/kernel/smp-cps.c b/arch/mips/kernel/smp-cps.c
> index 4251d39..5570bc8 100644
> --- a/arch/mips/kernel/smp-cps.c
> +++ b/arch/mips/kernel/smp-cps.c
> @@ -279,8 +279,8 @@ static void cps_init_secondary(void)
>   	if (cpu_has_mipsmt)
>   		dmt();
>   
> -	change_c0_status(ST0_IM, STATUSF_IP2 | STATUSF_IP3 | STATUSF_IP4 |
> -				 STATUSF_IP5 | STATUSF_IP6 | STATUSF_IP7);
> +	/* Call platform specific init secondary */
> +	plat_smp_init_secondary();
>   }
>   
>   static void cps_smp_finish(void)
> diff --git a/arch/mips/kernel/smp-mt.c b/arch/mips/kernel/smp-mt.c
> index 86311a1..3d1e32a 100644
> --- a/arch/mips/kernel/smp-mt.c
> +++ b/arch/mips/kernel/smp-mt.c
> @@ -158,16 +158,8 @@ static void vsmp_send_ipi_mask(const struct cpumask *mask, unsigned int action)
>   
>   static void vsmp_init_secondary(void)
>   {
> -#ifdef CONFIG_MIPS_GIC
> -	/* This is Malta specific: IPI,performance and timer interrupts */
> -	if (gic_present)
> -		change_c0_status(ST0_IM, STATUSF_IP2 | STATUSF_IP3 |
> -					 STATUSF_IP4 | STATUSF_IP5 |
> -					 STATUSF_IP6 | STATUSF_IP7);
> -	else
> -#endif
> -		change_c0_status(ST0_IM, STATUSF_IP0 | STATUSF_IP1 |
> -					 STATUSF_IP6 | STATUSF_IP7);
> +	/* Call platform specific init secondary */
> +	plat_smp_init_secondary();
>   }
>   
>   static void vsmp_smp_finish(void)
> diff --git a/arch/mips/mti-malta/malta-init.c b/arch/mips/mti-malta/malta-init.c
> index cec3e18..3385ad9 100644
> --- a/arch/mips/mti-malta/malta-init.c
> +++ b/arch/mips/mti-malta/malta-init.c
> @@ -116,6 +116,19 @@ phys_addr_t mips_cpc_default_phys_base(void)
>   	return CPC_BASE_ADDR;
>   }
>   
> +void plat_smp_init_secondary(void)
> +{
> +#ifdef CONFIG_MIPS_GIC
> +	if (gic_present)
> +		change_c0_status(ST0_IM, STATUSF_IP2 | STATUSF_IP3 |
> +					 STATUSF_IP4 | STATUSF_IP5 |
> +					 STATUSF_IP6 | STATUSF_IP7);
> +	else
> +#endif
> +		change_c0_status(ST0_IM, STATUSF_IP0 | STATUSF_IP1 |
> +					 STATUSF_IP6 | STATUSF_IP7);
> +}
> +
>   void __init prom_init(void)
>   {
>   	mips_display_message("LINUX");

WARNING: multiple messages have this Message-ID (diff)
From: Qais Yousef <qais.yousef@imgtec.com>
To: Tony Wu <tung7970@gmail.com>
Cc: linux-mips@linux-mips.org, ralf@linux-mips.org,
	James Hogan <james.hogan@imgtec.com>,
	Paul Burton <paul.burton@imgtec.com>
Subject: Re: [PATCH] MIPS: Add platform specific secondary core init
Date: Thu, 18 Jun 2015 17:52:18 +0100	[thread overview]
Message-ID: <5582F742.30905@imgtec.com> (raw)
Message-ID: <20150618165218.4-my96o3U80nBOvud7NDabLnl7oizo0MOi1U5-zoRm8@z> (raw)
In-Reply-To: <20150618220254-tung7970@googlemail.com>

On 06/18/2015 03:05 PM, Tony Wu wrote:
> Define plat_smp_init_secondary() API, and move platform specific
> secondary core initialization to each platform.

I really can't see that this code being platform specific. Why would a 
platform care about the c0_status register when a secondary boot is 
initialised?

That comment about Malta in smp-mt.c is misleading.

Qais

>
> Signed-off-by: Tony Wu <tung7970@gmail.com>
> Cc: James Hogan <james.hogan@imgtec.com>
> Cc: Paul Burton <paul.burton@imgtec.com>
> Cc: linux-mips@linux-mips.org
> ---
>   arch/mips/include/asm/smp-ops.h  |    1 +
>   arch/mips/kernel/smp-cmp.c       |    8 ++------
>   arch/mips/kernel/smp-cps.c       |    4 ++--
>   arch/mips/kernel/smp-mt.c        |   12 ++----------
>   arch/mips/mti-malta/malta-init.c |   13 +++++++++++++
>   5 files changed, 20 insertions(+), 18 deletions(-)
>
> diff --git a/arch/mips/include/asm/smp-ops.h b/arch/mips/include/asm/smp-ops.h
> index 6ba1fb8..53ce7e1 100644
> --- a/arch/mips/include/asm/smp-ops.h
> +++ b/arch/mips/include/asm/smp-ops.h
> @@ -36,6 +36,7 @@ struct plat_smp_ops {
>   };
>   
>   extern void register_smp_ops(struct plat_smp_ops *ops);
> +extern void plat_smp_init_secondary(void)
>   
>   static inline void plat_smp_setup(void)
>   {
> diff --git a/arch/mips/kernel/smp-cmp.c b/arch/mips/kernel/smp-cmp.c
> index d5e0f94..e8c0258 100644
> --- a/arch/mips/kernel/smp-cmp.c
> +++ b/arch/mips/kernel/smp-cmp.c
> @@ -43,17 +43,13 @@ static void cmp_init_secondary(void)
>   {
>   	struct cpuinfo_mips *c __maybe_unused = &current_cpu_data;
>   
> -	/* Assume GIC is present */
> -	change_c0_status(ST0_IM, STATUSF_IP2 | STATUSF_IP3 | STATUSF_IP4 |
> -				 STATUSF_IP5 | STATUSF_IP6 | STATUSF_IP7);
> -
> -	/* Enable per-cpu interrupts: platform specific */
> -
>   #ifdef CONFIG_MIPS_MT_SMP
>   	if (cpu_has_mipsmt)
>   		c->vpe_id = (read_c0_tcbind() >> TCBIND_CURVPE_SHIFT) &
>   			TCBIND_CURVPE;
>   #endif
> +	/* Call platform specific init secondary */
> +	plat_smp_init_secondary();
>   }
>   
>   static void cmp_smp_finish(void)
> diff --git a/arch/mips/kernel/smp-cps.c b/arch/mips/kernel/smp-cps.c
> index 4251d39..5570bc8 100644
> --- a/arch/mips/kernel/smp-cps.c
> +++ b/arch/mips/kernel/smp-cps.c
> @@ -279,8 +279,8 @@ static void cps_init_secondary(void)
>   	if (cpu_has_mipsmt)
>   		dmt();
>   
> -	change_c0_status(ST0_IM, STATUSF_IP2 | STATUSF_IP3 | STATUSF_IP4 |
> -				 STATUSF_IP5 | STATUSF_IP6 | STATUSF_IP7);
> +	/* Call platform specific init secondary */
> +	plat_smp_init_secondary();
>   }
>   
>   static void cps_smp_finish(void)
> diff --git a/arch/mips/kernel/smp-mt.c b/arch/mips/kernel/smp-mt.c
> index 86311a1..3d1e32a 100644
> --- a/arch/mips/kernel/smp-mt.c
> +++ b/arch/mips/kernel/smp-mt.c
> @@ -158,16 +158,8 @@ static void vsmp_send_ipi_mask(const struct cpumask *mask, unsigned int action)
>   
>   static void vsmp_init_secondary(void)
>   {
> -#ifdef CONFIG_MIPS_GIC
> -	/* This is Malta specific: IPI,performance and timer interrupts */
> -	if (gic_present)
> -		change_c0_status(ST0_IM, STATUSF_IP2 | STATUSF_IP3 |
> -					 STATUSF_IP4 | STATUSF_IP5 |
> -					 STATUSF_IP6 | STATUSF_IP7);
> -	else
> -#endif
> -		change_c0_status(ST0_IM, STATUSF_IP0 | STATUSF_IP1 |
> -					 STATUSF_IP6 | STATUSF_IP7);
> +	/* Call platform specific init secondary */
> +	plat_smp_init_secondary();
>   }
>   
>   static void vsmp_smp_finish(void)
> diff --git a/arch/mips/mti-malta/malta-init.c b/arch/mips/mti-malta/malta-init.c
> index cec3e18..3385ad9 100644
> --- a/arch/mips/mti-malta/malta-init.c
> +++ b/arch/mips/mti-malta/malta-init.c
> @@ -116,6 +116,19 @@ phys_addr_t mips_cpc_default_phys_base(void)
>   	return CPC_BASE_ADDR;
>   }
>   
> +void plat_smp_init_secondary(void)
> +{
> +#ifdef CONFIG_MIPS_GIC
> +	if (gic_present)
> +		change_c0_status(ST0_IM, STATUSF_IP2 | STATUSF_IP3 |
> +					 STATUSF_IP4 | STATUSF_IP5 |
> +					 STATUSF_IP6 | STATUSF_IP7);
> +	else
> +#endif
> +		change_c0_status(ST0_IM, STATUSF_IP0 | STATUSF_IP1 |
> +					 STATUSF_IP6 | STATUSF_IP7);
> +}
> +
>   void __init prom_init(void)
>   {
>   	mips_display_message("LINUX");

  reply	other threads:[~2015-06-18 16:52 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-18 14:05 [PATCH] MIPS: Add platform specific secondary core init Tony Wu
2015-06-18 16:52 ` Qais Yousef [this message]
2015-06-18 16:52   ` Qais Yousef

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5582F742.30905@imgtec.com \
    --to=qais.yousef@imgtec.com \
    --cc=james.hogan@imgtec.com \
    --cc=linux-mips@linux-mips.org \
    --cc=paul.burton@imgtec.com \
    --cc=ralf@linux-mips.org \
    --cc=tung7970@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.