From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nishanth Menon Subject: Re: [PATCH 6/6] omap: move generic omap3 features to generic Date: Mon, 31 May 2010 20:09:27 +0300 Message-ID: <4C03ED47.6060308@gmail.com> References: <1274585799-16226-1-git-send-email-nm@ti.com> <1274585799-16226-2-git-send-email-nm@ti.com> <1274585799-16226-3-git-send-email-nm@ti.com> <1274585799-16226-4-git-send-email-nm@ti.com> <1274585799-16226-5-git-send-email-nm@ti.com> <1274585799-16226-6-git-send-email-nm@ti.com> <1274585799-16226-7-git-send-email-nm@ti.com> <4BFE9C14.9040703@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ew0-f216.google.com ([209.85.219.216]:39129 "EHLO mail-ew0-f216.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755815Ab0EaRJd (ORCPT ); Mon, 31 May 2010 13:09:33 -0400 Received: by ewy8 with SMTP id 8so1019376ewy.28 for ; Mon, 31 May 2010 10:09:32 -0700 (PDT) In-Reply-To: Sender: linux-omap-owner@vger.kernel.org List-Id: linux-omap@vger.kernel.org To: Venkatraman S Cc: Nishanth Menon , linux omap , Tony Lindgren , Angelo Arrifano , "Zebediah C. McClure" , Alistair Buxton , Paul Walmsley , "Premi, Sanjeev" , "Shilimkar, Santosh" , "Guruswamy, Senthilvadivu" , Kevin Hilman , Tomi Valkeinen , Aaro Koskinen , "Pandita, Vikram" , "S, Vishwanath" On 05/31/2010 07:06 PM, Venkatraman S wrote: > Nishanth Menon wrote: >> On 05/27/2010 01:24 PM, S, Venkatraman wrote: >>> >>> Nishanth Menon wrote: >> >> [..] >>>> >>>> diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c >>>> index 809e13a..01555b6 100644 >>>> --- a/arch/arm/mach-omap2/id.c >>>> +++ b/arch/arm/mach-omap2/id.c >>>> @@ -161,7 +161,7 @@ static void __init omap24xx_check_revision(void) >>>> #define OMAP3_CHECK_FEATURE(status,feat) \ >>>> if (((status& OMAP3_ ##feat## _MASK) \ >>>> >> OMAP3_ ##feat## _SHIFT) != FEAT_ ##feat## _NONE) { \ >>>> - omap3_features |= OMAP3_HAS_ ##feat; \ >>>> + omap_features |= OMAP_HAS_ ##feat; \ >>>> } >>> >>> "CHECK" sounds like a querying API, whereas the macro populates data. >>> Maybe UPDATE or SET ? >>> >> Depends on where you are looking at it from: overall it is checking the >> status bits from OMAP and deciding what features it has - this is >> specifically important for 35xx series of processors. it is indeed a check >> in that sense. if you look at it from features variable, yeah it is updating >> it, but the idea of usage of the Macro is: check in status if feature X is >> available.. which is what it does ;). btw, the intent of the current patch >> was not meant to rename CHECK_FEATURE as it was very OMAP3 specific ;) >> > > It's ok. I don't want to worry too much about the name. Thanks. > >>>> static void __init omap3_check_features(void) >>>> @@ -310,20 +310,20 @@ static void __init omap3_cpuinfo(void) >>>> /* >> >> [...] >> >>>> +OMAP_HAS_FEATURE(, l2cache, L2CACHE) >>>> +OMAP_HAS_FEATURE(, sgx, SGX) >>>> +OMAP_HAS_FEATURE(, iva, IVA) >>>> +OMAP_HAS_FEATURE(, neon, NEON) >>>> +OMAP_HAS_FEATURE(, isp, ISP) >>>> + >>>> +/* >>>> * Runtime detection of OMAP3 features >>>> */ >>>> extern u32 omap3_features; >>>> >>>> -#define OMAP3_HAS_L2CACHE BIT(0) >>>> -#define OMAP3_HAS_IVA BIT(1) >>>> -#define OMAP3_HAS_SGX BIT(2) >>>> -#define OMAP3_HAS_NEON BIT(3) >>>> -#define OMAP3_HAS_ISP BIT(4) >>>> #define OMAP3_HAS_192MHZ_CLK BIT(5) >>>> >>>> -OMAP_HAS_FEATURE(3, l2cache, L2CACHE) >>>> -OMAP_HAS_FEATURE(3, sgx, SGX) >>>> -OMAP_HAS_FEATURE(3, iva, IVA) >>>> -OMAP_HAS_FEATURE(3, neon, NEON) >>>> -OMAP_HAS_FEATURE(3, isp, ISP) >>>> OMAP_HAS_FEATURE(3, 192mhz_clk, 192MHZ_CLK) >>>> >>>> #endif >>>> -- >>> >>> What about feature detection for OMAP2 and OMAP4 (similar to >>> omap3_check_features) ? >>> At least a dummy implementation with warning messages would be good, >>> so that they are not used without initialization. >> >> there is no need for warning messages, they will return as feature not >> present. cpu.h is a common header and variable omap_features is in common.c, >> the check_feature and id.c has not set that bit, the variable will remain 0, >> hence omap_has_sgx() will return 0 unless someone enables that for lets say >> OMAP4 -> feel free to do it, as I mentioned in 0/6, this series was meant >> solely to reorganize and provide an infrastructure for further development. >> > > I don't agree. The patch / series is about making them 'generic', where generic > is, to the minimum, "making the features exposed for major silicon versions". > If they are not usable beyond OMAP3, it's not generic. > Which part is not generic? I am unable to comprehend your contention here. Are you contending that ISP, l2cache, neon, isp are *not* generic OMAP features? or are you contending that since I did not add the OMAP2,4 logic to detect the same, this patch is not valid? all silicon revisions CAN use the function omap_has_sgx() now, even though I have not added detection logic to actually populate the same. if you do have a patch for detecting actual OMAP4/2 features feel free to add to the patch list, I am more than happy to ack them, if they look good - I personally dont have a omap4 platform at the very moment to write and post a patch - however trivial it may be. Regards, Nishanth Menon