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_perfcount_int() is defined in several files. Every definition is
> weak, so I assume Kconfig prevents two or more from being included. The
> callers contain identical default code used when get_c0_perfcount_int()
> isn't defined at all.
>
> Add a weak get_c0_perfcount_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@xxxxxxxxxx>
> CC: Andrew Bresticker <abrestic@xxxxxxxxxxxx>
> ---
> arch/mips/include/asm/time.h | 2 +-
> arch/mips/kernel/perf_event_mipsxx.c | 7 +------
> arch/mips/kernel/time.c | 10 +++++++++-
> arch/mips/oprofile/op_model_mipsxx.c | 8 +-------
> 4 files changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/arch/mips/include/asm/time.h b/arch/mips/include/asm/time.h
> index 8ab2874..ce6a7d5 100644
> --- a/arch/mips/include/asm/time.h
> +++ b/arch/mips/include/asm/time.h
> @@ -46,7 +46,7 @@ extern unsigned int mips_hpt_frequency;
> * so it lives here.
> */
> extern int (*perf_irq)(void);
> -extern int __weak get_c0_perfcount_int(void);
> +extern int get_c0_perfcount_int(void);
>
> /*
> * Initialize the calling CPU's compare interrupt as clockevent device
> diff --git a/arch/mips/kernel/perf_event_mipsxx.c b/arch/mips/kernel/perf_event_mipsxx.c
> index cc1b6fa..c126b1c 100644
> --- a/arch/mips/kernel/perf_event_mipsxx.c
> +++ b/arch/mips/kernel/perf_event_mipsxx.c
> @@ -1682,12 +1682,7 @@ init_hw_perf_events(void)
> counters = counters_total_to_per_cpu(counters);
> #endif
>
> - if (get_c0_perfcount_int)
> - irq = get_c0_perfcount_int();
> - else if (cp0_perfcount_irq >= 0)
> - irq = MIPS_CPU_IRQ_BASE + cp0_perfcount_irq;
> - else
> - irq = -1;
> + irq = get_c0_perfcount_int();
>
> mipspmu.map_raw_event = mipsxx_pmu_map_raw_event;
>
> diff --git a/arch/mips/kernel/time.c b/arch/mips/kernel/time.c
> index 8d01709..ec7082d 100644
> --- a/arch/mips/kernel/time.c
> +++ b/arch/mips/kernel/time.c
> @@ -55,9 +55,17 @@ static int null_perf_irq(void)
> }
>
> int (*perf_irq)(void) = null_perf_irq;
> -
> EXPORT_SYMBOL(perf_irq);
>
> +#ifdef MIPS_CPU_IRQ_BASE
why the ifdef? This would be the only such ifdef in the kernel, and
arch/mips/include/asm/mach-generic/irq.h seems to ensure it is always
defined anyway.
Aside from that the patch looks good.
Cheers
James
> +int __weak get_c0_perfcount_int(void)
> +{
> + if (cp0_perfcount_irq >= 0)
> + return MIPS_CPU_IRQ_BASE + cp0_perfcount_irq;
> + return -1;
> +}
> +#endif
> +
> /*
> * time_init() - it does the following things.
> *
> diff --git a/arch/mips/oprofile/op_model_mipsxx.c b/arch/mips/oprofile/op_model_mipsxx.c
> index 6a6e2cc..c0cffa9 100644
> --- a/arch/mips/oprofile/op_model_mipsxx.c
> +++ b/arch/mips/oprofile/op_model_mipsxx.c
> @@ -438,13 +438,7 @@ static int __init mipsxx_init(void)
> save_perf_irq = perf_irq;
> perf_irq = mipsxx_perfcount_handler;
>
> - if (get_c0_perfcount_int)
> - perfcount_irq = get_c0_perfcount_int();
> - else if (cp0_perfcount_irq >= 0)
> - perfcount_irq = MIPS_CPU_IRQ_BASE + cp0_perfcount_irq;
> - else
> - perfcount_irq = -1;
> -
> + perfcount_irq = get_c0_perfcount_int();
> if (perfcount_irq >= 0)
> return request_irq(perfcount_irq, mipsxx_perfcount_int,
> IRQF_PERCPU | IRQF_NOBALANCING |
>
Attachment:
signature.asc
Description: OpenPGP digital signature
