> -----Original Message----- > From: Menon, Nishanth > Sent: Tuesday, July 13, 2010 9:08 PM > To: Premi, Sanjeev > Cc: Tony Lindgren; felipe.balbi@xxxxxxxxx; linux-omap; Angelo > Arrifano; Zebediah C. McClure; Alistair Buxton; Grazvydas > Ignotas; Paul Walmsley; Shilimkar, Santosh; Guruswamy, > Senthilvadivu; Kevin Hilman; DebBarma, Tarun Kanti; > ValkeinenTomi (Nokia-MS/Helsinki); Koskinen Aaro > (Nokia-MS/Helsinki); Pandita, Vikram; S, Vishwanath > Subject: Re: [PATCH 3/9 v3] omap: generic: introduce a single > check_revision > > Premi, Sanjeev had written, on 07/13/2010 10:06 AM, the following: > >> -----Original Message----- > >> From: Menon, Nishanth > >> Sent: Thursday, July 08, 2010 7:57 PM > >> To: Tony Lindgren > >> Cc: felipe.balbi@xxxxxxxxx; linux-omap; Angelo Arrifano; > >> Zebediah C. McClure; Alistair Buxton; Grazvydas Ignotas; Paul > >> Walmsley; Premi, Sanjeev; Shilimkar, Santosh; Guruswamy, > >> Senthilvadivu; Kevin Hilman; DebBarma, Tarun Kanti; > >> ValkeinenTomi (Nokia-MS/Helsinki); Koskinen Aaro > >> (Nokia-MS/Helsinki); Pandita, Vikram; S, Vishwanath > >> Subject: Re: [PATCH 3/9 v3] omap: generic: introduce a single > >> check_revision > >> > >> Tony Lindgren had written, on 07/08/2010 07:21 AM, the following: > >>> * Menon, Nishanth <nm@xxxxxx> [100708 14:49]: > >>>> ----- Original message ----- > >>>>> Hi, > >>>>> > >>>>> On Wed, Jul 07, 2010 at 07:24:16PM +0200, ext Nishanth > >> Menon wrote: > >>>>>> I am not sure.. if you would like drivers to be > >> modprobabe, there may > >>>>> be > >>>>>> quirks that you'd want to enable based on cpu_is_omapxxx > >> checks. so it > >>>>>> probably does not make sense to __initdata the revision/feature > >>>>> variables. > >>>>> > >>>>> can't you pass the quirks via pdata, then ? > >>>> If pdata is passed based on board: Imagine 3630 and uart > >> quirk. Why share errata xyz over pdata for every board using > >> 3630? Quirks are cpu specific and not really domain of board.. > >>> We should be able to handle the quirks by passing some > >>> flag or function pointer from platform data. > >>> > >>> The drivers should be arch independent, using cpu_is_omapxxxx > >>> tests anywhere under drivers/* is wrong and should be > >>> fixed. > >> there are two forms of quirks: > >> a) quirks which can be detected based on IP rev > >> b) quirks which are silicon integration related - only > >> cpu_is_xxxx can > >> be used to detect them. > >> for a) - I disagree that pdata should be used (this was my > original > >> contention) > >> for b) the question IMHO is: How is pdata provided to the > >> driver - that > >> is important. IMHO, pdata taken into drivers could have > >> quirks, but if > >> the quirk addition is done from board files, I disagree, then > >> should be > >> done in arch/arm/mach-omap[12]/somefile.c where somefile.c is > >> common for > >> all boards (e.g. device.c) and that allows the driver to be cpu > >> independent and allows board files not to have redundant > information. > >> > >> BUT, features *should* be kept distinct from quirks for > readability > >> purposes. > > > > Nishanth, > > > > Sorry missed most of discussion due to vacation. But just wanted to > > know the benefit of check_version() without the processor context. > > lets try not to confuse things here: > a) feature - this debate is part of this thread [sp] The OMAP_FEATURE appears in the patch below only because the corresponding lines didn't change in the patch I created. Not because I wanted to get another discussion on this subject. Subject of this mail is "omap: generic: introduce a single check_revision" and hence I responded to same. > b) errata - done per driver [sp] Didn't get the context here. > c) cpu_is_xyz() - used for cpu type detection > d) cpu_revision() - the patch that you post below - should be in a > seperate thread. > > I am not averse to the new functions you are introducing and IMHO, I > agree that it will improve readability, can you rebase and > post seperately? [sp] Will be doing it. The patch here was only for illustration. > > > > > I had earlier submitted related patches; but 36x being considered > > as 34x broke the logic. Before going on vacation I had created a > > patch (against 2.6.32). Putting here for review (there are two > > typos in this specific patch, pl ignore that): > > > > Usage would be: > > if (cpu_rev_eq(omap34xx, OMAP34XX_ES_1_0)) > maybe we could simplify this a bit(OMAP34XX prefix seems redundant): Yes. I know, but then I am, not sure if the bit definitions will remain same in future. An early version of this patch had no OMAP34XX prefix. > if (cpu_rev_eq(omap34xx, ES_1_0))? > we should probably take the discussion seperately. > > > > > > > commit 6e4a9439fbc67eb2a55f440923cfade74c4509d2 > > Author: Sanjeev Premi <premi@xxxxxx> > > Date: Thu Jun 3 21:28:39 2010 +0530 > > > > omap: Add macros to evaluate cpu revision > > > > This patch adds macros to evaluate the cpu revision. > > These macros increase readability by reducing the > > repetitive code when multiple silicon revisions have > > to be tested. > > > > diff --git a/arch/arm/plat-omap/include/plat/cpu.h > b/arch/arm/plat-omap/include/ > > index 7514174..7cc5611 100644 > > --- a/arch/arm/plat-omap/include/plat/cpu.h > > +++ b/arch/arm/plat-omap/include/plat/cpu.h > > @@ -70,6 +70,12 @@ unsigned int omap_rev(void); > > #define OMAP_REVBITS_20 0x20 > > #define OMAP_REVBITS_30 0x30 > > #define OMAP_REVBITS_40 0x40 > > +#define OMAP_REVBITS_50 0x50 > > + > > +/* > > + * Get the CPU Id for OMAP devices > > + */ > > +#define GET_OMAP_ID() ((omap_rev() >> 16) & 0xffff) > > > > /* > > * Get the CPU revision for OMAP devices > > @@ -385,6 +391,12 @@ IS_OMAP_TYPE(3517, 0x3517) > > #define OMAP3505_REV(v) (OMAP35XX_CLASS | > (0x3505 << 16) | (v << > > #define OMAP3517_REV(v) (OMAP35XX_CLASS | > (0x3517 << 16) | (v << > > > > +#define AM37XX_CLASS 0x37000034 > > +#define AM3703_REV(v) (AM37XX_CLASS | (0x3503 << > 16) | (v << 8)) > > +#define AM3715_REV(v) (AM37XX_CLASS | (0x3515 << > 16) | (v << 8)) > > +#define AM3725_REV(v) (AM37XX_CLASS | (0x3525 << > 16) | (v << 8)) > > +#define AM3730_REV(v) (AM37XX_CLASS | (0x3530 << > 16) | (v << 8)) > > + > > #define OMAP443X_CLASS 0x44300044 > > #define OMAP4430_REV_ES1_0 0x44300044 > > > > @@ -458,4 +470,36 @@ OMAP3_HAS_FEATURE(neon, NEON) > > OMAP3_HAS_FEATURE(isp, ISP) > > OMAP3_HAS_FEATURE(192mhz_clk, 192MHZ_CLK) > > > > +/* > > + * Map revision bits to silicon specific revisions > > + */ > > +#define OMAP34XX_ES_1_0 OMAP_REVBITS_00 > > +#define OMAP34XX_ES_2_0 OMAP_REVBITS_10 > > +#define OMAP34XX_ES_2_1 OMAP_REVBITS_20 > > +#define OMAP34XX_ES_3_0 OMAP_REVBITS_30 > > +#define OMAP34XX_ES_3_1 OMAP_REVBITS_40 > > +#define OMAP34XX_ES_3_1_2 OMAP_REVBITS_50 > > + > > +#define AM3517_ES_1_0 OMAP_REVBITS_00 > > + > > +#define OMAP36XX_ES_1_0 OMAP_REVBITS_00 > > + > > +/* > > + * Macros to evaluate CPU revision > > + */ > > +#define cpu_rev_lt(cpu,rev) \ > > + ((cpu_is_omap ##cpu() && (GET_OMAP_REVISION() < > (rev))) ? 1 : 0) > > + > > +#define cpu_rev_le(cpu,rev) \ > > + ((cpu_is_omap ##cpu() && (GET_OMAP_REVISION() <= > (rev))) ? 1 : 0) > > + > > +#define cpu_rev_eq(cpu,rev) \ > > + ((cpu_is_omap ##cpu() && (GET_OMAP_REVISION() == > (rev))) ? 1 : 0) > > + > > +#define cpu_rev_ge(cpu,rev) \ > > + ((cpu_is_omap ##cpu() && (GET_OMAP_REVISION() >= > (rev))) ? 1 : 0) > > + > > +#define cpu_rev_gt(cpu,rev) \ > > + ((cpu_is_omap ##cpu() && (GET_OMAP_REVISION() > > (rev))) ? 1 : 0) > > + > > #endif > > > > ~sanjeev > > > >> -- > >> Regards, > >> Nishanth Menon > >> > > > -- > Regards, > Nishanth Menon > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
