All of lore.kernel.org
 help / color / mirror / Atom feed
From: James Hogan <james.hogan@imgtec.com>
To: Bjorn Helgaas <bhelgaas@google.com>, Ralf Baechle <ralf@linux-mips.org>
Cc: Andrew Bresticker <abrestic@chromium.org>,
	<linux-mips@linux-mips.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 6/9] MIPS: Remove "weak" from get_c0_compare_int() declaration
Date: Mon, 13 Jul 2015 10:36:24 +0100	[thread overview]
Message-ID: <55A38698.8080206@imgtec.com> (raw)
In-Reply-To: <20150712231137.11177.3681.stgit@bhelgaas-glaptop2.roam.corp.google.com>

[-- Attachment #1: Type: text/plain, Size: 2656 bytes --]

On 13/07/15 00:11, Bjorn Helgaas wrote:
> Weak header file declarations are error-prone because they make every
> definition weak, and the linker chooses one based on link order (see
> 10629d711ed7 ("PCI: Remove __weak annotation from pcibios_get_phb_of_node
> decl")).
> 
> get_c0_compare_int() is defined in several files.  Each definition is weak,
> so I assume Kconfig prevents two or more from being included.  The caller
> contains default code used when get_c0_compare_int() isn't defined at all.
> 
> Add a weak get_c0_compare_int() definition with the default code and remove
> the weak annotation from the declaration.
> 
> Then the platform implementations will be strong and will override the weak
> default.  If multiple platforms are ever configured in, we'll get a link
> error instead of calling a random platform's implementation.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

Reviewed-by: James Hogan <james.hogan@imgtec.com>

Cheers
James

> ---
>  arch/mips/include/asm/time.h |    2 +-
>  arch/mips/kernel/cevt-r4k.c  |   11 +++++++----
>  2 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/mips/include/asm/time.h b/arch/mips/include/asm/time.h
> index ce6a7d5..44a9c3a 100644
> --- a/arch/mips/include/asm/time.h
> +++ b/arch/mips/include/asm/time.h
> @@ -51,7 +51,7 @@ extern int get_c0_perfcount_int(void);
>  /*
>   * Initialize the calling CPU's compare interrupt as clockevent device
>   */
> -extern unsigned int __weak get_c0_compare_int(void);
> +extern unsigned int get_c0_compare_int(void);
>  extern int r4k_clockevent_init(void);
>  
>  static inline int mips_clockevent_init(void)
> diff --git a/arch/mips/kernel/cevt-r4k.c b/arch/mips/kernel/cevt-r4k.c
> index d70c4d8..cc7cc46 100644
> --- a/arch/mips/kernel/cevt-r4k.c
> +++ b/arch/mips/kernel/cevt-r4k.c
> @@ -174,6 +174,11 @@ int c0_compare_int_usable(void)
>  	return 1;
>  }
>  
> +unsigned int __weak get_c0_compare_int(void)
> +{
> +	return MIPS_CPU_IRQ_BASE + cp0_compare_irq;
> +}
> +
>  int r4k_clockevent_init(void)
>  {
>  	unsigned int cpu = smp_processor_id();
> @@ -189,11 +194,9 @@ int r4k_clockevent_init(void)
>  	/*
>  	 * With vectored interrupts things are getting platform specific.
>  	 * get_c0_compare_int is a hook to allow a platform to return the
> -	 * interrupt number of it's liking.
> +	 * interrupt number of its liking.
>  	 */
> -	irq = MIPS_CPU_IRQ_BASE + cp0_compare_irq;
> -	if (get_c0_compare_int)
> -		irq = get_c0_compare_int();
> +	irq = get_c0_compare_int();
>  
>  	cd = &per_cpu(mips_clockevent_device, cpu);
>  
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: James Hogan <james.hogan@imgtec.com>
To: Bjorn Helgaas <bhelgaas@google.com>, Ralf Baechle <ralf@linux-mips.org>
Cc: Andrew Bresticker <abrestic@chromium.org>,
	linux-mips@linux-mips.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 6/9] MIPS: Remove "weak" from get_c0_compare_int() declaration
Date: Mon, 13 Jul 2015 10:36:24 +0100	[thread overview]
Message-ID: <55A38698.8080206@imgtec.com> (raw)
Message-ID: <20150713093624.AUD49oZZey_tfupkMyIBKct2ug-p-7QU4VaW3JaCAF0@z> (raw)
In-Reply-To: <20150712231137.11177.3681.stgit@bhelgaas-glaptop2.roam.corp.google.com>

[-- Attachment #1: Type: text/plain, Size: 2656 bytes --]

On 13/07/15 00:11, Bjorn Helgaas wrote:
> Weak header file declarations are error-prone because they make every
> definition weak, and the linker chooses one based on link order (see
> 10629d711ed7 ("PCI: Remove __weak annotation from pcibios_get_phb_of_node
> decl")).
> 
> get_c0_compare_int() is defined in several files.  Each definition is weak,
> so I assume Kconfig prevents two or more from being included.  The caller
> contains default code used when get_c0_compare_int() isn't defined at all.
> 
> Add a weak get_c0_compare_int() definition with the default code and remove
> the weak annotation from the declaration.
> 
> Then the platform implementations will be strong and will override the weak
> default.  If multiple platforms are ever configured in, we'll get a link
> error instead of calling a random platform's implementation.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

Reviewed-by: James Hogan <james.hogan@imgtec.com>

Cheers
James

> ---
>  arch/mips/include/asm/time.h |    2 +-
>  arch/mips/kernel/cevt-r4k.c  |   11 +++++++----
>  2 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/mips/include/asm/time.h b/arch/mips/include/asm/time.h
> index ce6a7d5..44a9c3a 100644
> --- a/arch/mips/include/asm/time.h
> +++ b/arch/mips/include/asm/time.h
> @@ -51,7 +51,7 @@ extern int get_c0_perfcount_int(void);
>  /*
>   * Initialize the calling CPU's compare interrupt as clockevent device
>   */
> -extern unsigned int __weak get_c0_compare_int(void);
> +extern unsigned int get_c0_compare_int(void);
>  extern int r4k_clockevent_init(void);
>  
>  static inline int mips_clockevent_init(void)
> diff --git a/arch/mips/kernel/cevt-r4k.c b/arch/mips/kernel/cevt-r4k.c
> index d70c4d8..cc7cc46 100644
> --- a/arch/mips/kernel/cevt-r4k.c
> +++ b/arch/mips/kernel/cevt-r4k.c
> @@ -174,6 +174,11 @@ int c0_compare_int_usable(void)
>  	return 1;
>  }
>  
> +unsigned int __weak get_c0_compare_int(void)
> +{
> +	return MIPS_CPU_IRQ_BASE + cp0_compare_irq;
> +}
> +
>  int r4k_clockevent_init(void)
>  {
>  	unsigned int cpu = smp_processor_id();
> @@ -189,11 +194,9 @@ int r4k_clockevent_init(void)
>  	/*
>  	 * With vectored interrupts things are getting platform specific.
>  	 * get_c0_compare_int is a hook to allow a platform to return the
> -	 * interrupt number of it's liking.
> +	 * interrupt number of its liking.
>  	 */
> -	irq = MIPS_CPU_IRQ_BASE + cp0_compare_irq;
> -	if (get_c0_compare_int)
> -		irq = get_c0_compare_int();
> +	irq = get_c0_compare_int();
>  
>  	cd = &per_cpu(mips_clockevent_device, cpu);
>  
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

  reply	other threads:[~2015-07-13  9:36 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-12 23:10 [PATCH 0/9] MIPS: Remove "weak" usage Bjorn Helgaas
2015-07-12 23:10 ` [PATCH 1/9] MIPS: CPC: Remove "weak" from mips_cpc_phys_base() and make it static Bjorn Helgaas
2015-07-12 23:11 ` [PATCH 2/9] MIPS: Remove "weak" from platform_maar_init() declaration Bjorn Helgaas
2015-07-13  9:46   ` James Hogan
2015-07-13  9:46     ` James Hogan
2015-07-12 23:11 ` [PATCH 3/9] MIPS: VPE: Exit vpe_release() early if vpe_run() isn't defined Bjorn Helgaas
2015-07-12 23:11 ` [PATCH 4/9] MIPS: MT: Remove "weak" from vpe_run() declaration Bjorn Helgaas
2015-07-13 10:27   ` James Hogan
2015-07-13 10:27     ` James Hogan
2015-07-13 21:35     ` Bjorn Helgaas
2015-07-12 23:11 ` [PATCH 5/9] MIPS: Remove "weak" from get_c0_perfcount_int() declaration Bjorn Helgaas
2015-07-13  9:43   ` James Hogan
2015-07-13  9:43     ` James Hogan
2015-07-13 21:13     ` Bjorn Helgaas
2015-07-12 23:11 ` [PATCH 6/9] MIPS: Remove "weak" from get_c0_compare_int() declaration Bjorn Helgaas
2015-07-13  9:36   ` James Hogan [this message]
2015-07-13  9:36     ` James Hogan
2015-07-12 23:11 ` [PATCH 7/9] MIPS: Remove "weak" from get_c0_fdc_int() declaration Bjorn Helgaas
2015-07-13  9:31   ` James Hogan
2015-07-13  9:31     ` James Hogan
2015-07-12 23:11 ` [PATCH 8/9] MIPS: Remove "weak" from mips_cdmm_phys_base() declaration Bjorn Helgaas
2015-07-13  9:20   ` James Hogan
2015-07-13  9:20     ` James Hogan
2015-07-12 23:12 ` [PATCH 9/9] MIPS: Remove "__weak" definition from arch-specific linkage.h Bjorn Helgaas
2015-07-13 10:08   ` James Hogan
2015-07-13 10:08     ` James Hogan

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=55A38698.8080206@imgtec.com \
    --to=james.hogan@imgtec.com \
    --cc=abrestic@chromium.org \
    --cc=bhelgaas@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@linux-mips.org \
    --cc=ralf@linux-mips.org \
    /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.