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 7/9] MIPS: Remove "weak" from get_c0_fdc_int() declaration
Date: Mon, 13 Jul 2015 10:31:57 +0100 [thread overview]
Message-ID: <55A3858D.6020405@imgtec.com> (raw)
In-Reply-To: <20150712231146.11177.23561.stgit@bhelgaas-glaptop2.roam.corp.google.com>
[-- Attachment #1: Type: text/plain, Size: 2616 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_fdc_int() is defined only in arch/mips/mti-malta/malta-time.c so
well, until 6b5e741e9a83 ("MIPS: Pistachio: Support CDMM & Fast Debug
Channel") in v4.2-rc2 ;-)
> there's no problem with multiple definitions. But it works better to have
> a weak default implementation and allow a strong function to override it.
> Then we don't have to test whether a definition is present, and if there
> are ever multiple strong definitions, we get a link error instead of
> calling a random definition.
>
> Add a weak get_c0_fdc_int() definition with the default code and remove the
> weak annotation from the declaration.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> CC: James Hogan <james.hogan@imgtec.com>
Reviewed-by: James Hogan <james.hogan@imgtec.com>
Thanks
James
> ---
> arch/mips/include/asm/irq.h | 2 +-
> drivers/tty/mips_ejtag_fdc.c | 9 ++++++---
> 2 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/arch/mips/include/asm/irq.h b/arch/mips/include/asm/irq.h
> index f0db99f8..15e0fec 100644
> --- a/arch/mips/include/asm/irq.h
> +++ b/arch/mips/include/asm/irq.h
> @@ -49,7 +49,7 @@ extern int cp0_compare_irq_shift;
> extern int cp0_perfcount_irq;
> extern int cp0_fdc_irq;
>
> -extern int __weak get_c0_fdc_int(void);
> +extern int get_c0_fdc_int(void);
>
> void arch_trigger_all_cpu_backtrace(bool);
> #define arch_trigger_all_cpu_backtrace arch_trigger_all_cpu_backtrace
> diff --git a/drivers/tty/mips_ejtag_fdc.c b/drivers/tty/mips_ejtag_fdc.c
> index 358323c..a8c8cfd 100644
> --- a/drivers/tty/mips_ejtag_fdc.c
> +++ b/drivers/tty/mips_ejtag_fdc.c
> @@ -879,6 +879,11 @@ static const struct tty_operations mips_ejtag_fdc_tty_ops = {
> .chars_in_buffer = mips_ejtag_fdc_tty_chars_in_buffer,
> };
>
> +int __weak get_c0_fdc_int(void)
> +{
> + return -1;
> +}
> +
> static int mips_ejtag_fdc_tty_probe(struct mips_cdmm_device *dev)
> {
> int ret, nport;
> @@ -967,9 +972,7 @@ static int mips_ejtag_fdc_tty_probe(struct mips_cdmm_device *dev)
> wake_up_process(priv->thread);
>
> /* Look for an FDC IRQ */
> - priv->irq = -1;
> - if (get_c0_fdc_int)
> - priv->irq = get_c0_fdc_int();
> + priv->irq = get_c0_fdc_int();
>
> /* Try requesting the IRQ */
> if (priv->irq >= 0) {
>
[-- 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 7/9] MIPS: Remove "weak" from get_c0_fdc_int() declaration
Date: Mon, 13 Jul 2015 10:31:57 +0100 [thread overview]
Message-ID: <55A3858D.6020405@imgtec.com> (raw)
Message-ID: <20150713093157.PAsZe2-chG2tKUYvTHx8I9iDHL6VtgwPkyuZaU-P2qM@z> (raw)
In-Reply-To: <20150712231146.11177.23561.stgit@bhelgaas-glaptop2.roam.corp.google.com>
[-- Attachment #1: Type: text/plain, Size: 2616 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_fdc_int() is defined only in arch/mips/mti-malta/malta-time.c so
well, until 6b5e741e9a83 ("MIPS: Pistachio: Support CDMM & Fast Debug
Channel") in v4.2-rc2 ;-)
> there's no problem with multiple definitions. But it works better to have
> a weak default implementation and allow a strong function to override it.
> Then we don't have to test whether a definition is present, and if there
> are ever multiple strong definitions, we get a link error instead of
> calling a random definition.
>
> Add a weak get_c0_fdc_int() definition with the default code and remove the
> weak annotation from the declaration.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> CC: James Hogan <james.hogan@imgtec.com>
Reviewed-by: James Hogan <james.hogan@imgtec.com>
Thanks
James
> ---
> arch/mips/include/asm/irq.h | 2 +-
> drivers/tty/mips_ejtag_fdc.c | 9 ++++++---
> 2 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/arch/mips/include/asm/irq.h b/arch/mips/include/asm/irq.h
> index f0db99f8..15e0fec 100644
> --- a/arch/mips/include/asm/irq.h
> +++ b/arch/mips/include/asm/irq.h
> @@ -49,7 +49,7 @@ extern int cp0_compare_irq_shift;
> extern int cp0_perfcount_irq;
> extern int cp0_fdc_irq;
>
> -extern int __weak get_c0_fdc_int(void);
> +extern int get_c0_fdc_int(void);
>
> void arch_trigger_all_cpu_backtrace(bool);
> #define arch_trigger_all_cpu_backtrace arch_trigger_all_cpu_backtrace
> diff --git a/drivers/tty/mips_ejtag_fdc.c b/drivers/tty/mips_ejtag_fdc.c
> index 358323c..a8c8cfd 100644
> --- a/drivers/tty/mips_ejtag_fdc.c
> +++ b/drivers/tty/mips_ejtag_fdc.c
> @@ -879,6 +879,11 @@ static const struct tty_operations mips_ejtag_fdc_tty_ops = {
> .chars_in_buffer = mips_ejtag_fdc_tty_chars_in_buffer,
> };
>
> +int __weak get_c0_fdc_int(void)
> +{
> + return -1;
> +}
> +
> static int mips_ejtag_fdc_tty_probe(struct mips_cdmm_device *dev)
> {
> int ret, nport;
> @@ -967,9 +972,7 @@ static int mips_ejtag_fdc_tty_probe(struct mips_cdmm_device *dev)
> wake_up_process(priv->thread);
>
> /* Look for an FDC IRQ */
> - priv->irq = -1;
> - if (get_c0_fdc_int)
> - priv->irq = get_c0_fdc_int();
> + priv->irq = get_c0_fdc_int();
>
> /* Try requesting the IRQ */
> if (priv->irq >= 0) {
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
next prev parent reply other threads:[~2015-07-13 9:32 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
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 [this message]
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=55A3858D.6020405@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.