From patchwork Wed May 18 09:31:30 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ming Lei X-Patchwork-Id: 793232 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter2.kernel.org (8.14.4/8.14.3) with ESMTP id p4I9VX7f013947 for ; Wed, 18 May 2011 09:31:33 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932168Ab1ERJbc (ORCPT ); Wed, 18 May 2011 05:31:32 -0400 Received: from mail-yx0-f174.google.com ([209.85.213.174]:50165 "EHLO mail-yx0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932138Ab1ERJbb (ORCPT ); Wed, 18 May 2011 05:31:31 -0400 Received: by yxs7 with SMTP id 7so466595yxs.19 for ; Wed, 18 May 2011 02:31:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=ajIPHGnV31my+fHBguqhqkpOjoROVerk0xei0x9M0PA=; b=GVXFTy5Dv6CVWqyrDYNjR3dDIHbAnIgXK+xSHQTcePXTyCxsYUmrDL1iE53TejrCBG jXNxvKHnPwS5SIcid5Oh2gbHX2KfI6AGXrASU9/bKW4/FwMznYTgiAaMtfbYMJTqbiKD Oeit8nYiZZxxSCYd6d+FW/4yMi57H0flAFdaM= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; b=JGdEWG2aqLrFvbnCjEK3IwVXIrGR9LYe8iAYNttnKATia/NJTftTpGcAGinRnYXnpq GzI3KhbXYIHctQGeep4JCF0FXL4bIsqkj4mkMvyoTSwF8ZPyncUnP8fOuCiGvNoNHUwS mWW707v4PPoUziF/marHJVhAVPwsqfJMRBKos= MIME-Version: 1.0 Received: by 10.236.143.8 with SMTP id k8mr1772873yhj.351.1305711090644; Wed, 18 May 2011 02:31:30 -0700 (PDT) Received: by 10.236.70.198 with HTTP; Wed, 18 May 2011 02:31:30 -0700 (PDT) In-Reply-To: <4785097973995347565@unknownmsgid> References: <1305686080-13182-1-git-send-email-tom.leiming@gmail.com> <4785097973995347565@unknownmsgid> Date: Wed, 18 May 2011 17:31:30 +0800 Message-ID: Subject: Re: [PATCH] ARM: fix mistaken dependency for CPU_HAS_PMU From: Ming Lei To: Will Deacon Cc: linux@arm.linux.org.uk, linux-arm-kernel@lists.infradead.org, linux-omap@vger.kernel.org Sender: linux-omap-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-omap@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter2.kernel.org [140.211.167.43]); Wed, 18 May 2011 09:31:33 +0000 (UTC) Hi, 2011/5/18 Will Deacon : > Hi, > >> From: Ming Lei >> >> 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. > >> No mattery it is omap3 or not, we can build pmu.o into >> kernel safely. In fact, arm-v7 of omap3 has pmu inside >> also. > > Yes, but the PMU on Omap3 doesn't work unless you enable the emu clk, which > is done in kernel/etm.c. Rather than duplicate that code (which only seems > to be needed for Omap anyway) I decided to add a dependency on OMAP3_EMU so > the clock gets enabled in etb_probe. > >> So fix it that we can build pmu into kernel to support perf >> for some valid config(such as enable both omap4 and omap3, >> but OMAP3_EMU). > > No, I don't like this. The reason I don't like it is because it will lead to > lots of bug reports that perf doesn't work on Omap3 because they haven't got > the emu clock ticking. It was this scenario that led to the dependency being > introduced in the first place: > > 8954bb0d ("OMAP3: pmu: make CPU_HAS_PMU dependent on OMAP3_EMU") I don't think it is good fix. 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? > Is there a problem building the etm support for Omap4? Of course, there is the problem I described above. thanks, 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);