From: will.deacon@arm.com (Will Deacon)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: fix mistaken dependency for CPU_HAS_PMU
Date: Wed, 18 May 2011 10:53:18 +0100 [thread overview]
Message-ID: <000201cc1541$6a5f26a0$3f1d73e0$@deacon@arm.com> (raw)
In-Reply-To: <BANLkTinkZZwPG6S2goLYhuNV8JcVB-xsFw@mail.gmail.com>
> >> From: Ming Lei <tom.leiming@gmail.com>
> >>
> >> Obviously it is mistake for CPU_HAS_PMU to depend on
> >> (!ARCH_OMAP3 || OMAP3_EMU).
> >
> > It's not that obvious!
>
> No, on ubuntu 11.04, default config options enables ARCH_OMAP3
> and ARCH_OMAP4, and disable OMAP3_EMU, so cause pmu.o
> can't be built in kernel.
>
> pmu.o is just a platform driver for pmu device, so we should always
> allow it to be built into kernel no matter there are pmu devices or
> not, so we can support multiple boards with one same image.
CPU_HAS_PMU is also used as a condition for enabling HW_PERF_EVENTS.
I don't think we should set that for configurations where the platform
doesn't provide working hardware counters.
On top of that, pmu.o provides a reservation mechanism for the PMU
which is used by tools such as perf and LTTng. If CPU_HAS_PMU is not
set then you will get back -ENODEV, which I think is better than providing
access to a PMU that's not working.
> In fact, we should keep arch/arm/Kconfig or arch/arm/kernel/pmu.c more generic,
> and avoid to make arm core code depend on machines options as far as possible.
>
> For the pmu issue on omap3, below fix should be better than what
> 8954bb0d did, shouldn't it?
>
> diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
> index 7b85585..c20a3a3 100644
> --- a/arch/arm/mach-omap2/devices.c
> +++ b/arch/arm/mach-omap2/devices.c
> @@ -410,9 +410,12 @@ static void omap_init_pmu(void)
> {
> if (cpu_is_omap24xx())
> omap_pmu_device.resource = &omap2_pmu_resource;
> - else if (cpu_is_omap34xx())
> + else if (cpu_is_omap34xx()) {
> omap_pmu_device.resource = &omap3_pmu_resource;
> - else
> +#ifndef CONFIG_OMAP3_EMU
> + return;
> +#endif
> + } else
> return;
>
> platform_device_register(&omap_pmu_device);
The problem with this is that it still allows users to build kernels
where perf is enabled but doesn't work. Since we know this is the case
at build time, we have chance to stop them doing it then (or at least
not let them select HW_PERF_EVENTS).
> > Is there a problem building the etm support for Omap4?
>
> Of course, there is the problem I described above.
What I mean is, why can't you just select OMAP3_EMU in your Omap3/4 combined
kernel config? Then everything should work.
Will
prev parent reply other threads:[~2011-05-18 9:53 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-05-18 2:34 [PATCH] ARM: fix mistaken dependency for CPU_HAS_PMU tom.leiming at gmail.com
2011-05-18 8:47 ` Will Deacon
[not found] ` <4785097973995347565@unknownmsgid>
2011-05-18 9:31 ` Ming Lei
2011-05-18 9:53 ` Will Deacon [this message]
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='000201cc1541$6a5f26a0$3f1d73e0$@deacon@arm.com' \
--to=will.deacon@arm.com \
--cc=linux-arm-kernel@lists.infradead.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).