> -----Original Message-----
> From: Kevin Hilman [mailto:[email protected]]
> Sent: Thursday, August 13, 2009 9:43 PM
> To: Premi, Sanjeev
> Cc: [email protected]
> Subject: Re: [PATCH] Runtime detection of Si features
>
> Sanjeev Premi <[email protected]> writes:
>
> > The OMAP35x family has multiple variants differing
> > in the HW features. This patch detects these features
> > at runtime and prints information during the boot.
> >
> > Since most of the code seemed repetitive, macros
> > have been used for readability.
> >
> > Signed-off-by: Sanjeev Premi <[email protected]>
>
> I like the feature-based approach.
>
> A couple questions though. Is there a bit/register that reports the
> collapsed powerdomains of the devices with modified PRCM?
[sp] Unfortunately no. This is the reason, I used cpu_is_omap3505()
and cpu_is_omap3517() in my previous set of patches. This is what I
plan to do here as well; once I can identify the si based on the
features + the hawkeye number.
>
> Also, how will other code query the features? You're currently
> exporting the omap_has_*() functions, but there are no prototypes.
I think you missed the definition of macro OMAP3_HAS_FEATURE.
Since most of the code was repetitive, I used this macro for better
Readability.
>
> I think I'd rather see a static inline functions in <mach/cpu.h>
> for checking features. Comments to that end inlined below...
Yes. That can be done. With this patch I just wanted to get a view
on the overall approach.
>
> > ---
> > arch/arm/mach-omap2/id.c | 106
> ++++++++++++++++++++++++++++++++++++++++++++--
> > 1 files changed, 102 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
> > index a98201c..4476491 100644
> > --- a/arch/arm/mach-omap2/id.c
> > +++ b/arch/arm/mach-omap2/id.c
> > @@ -25,9 +25,49 @@
> > #include <mach/control.h>
> > #include <mach/cpu.h>
> >
> > +/*
> > + * OMAP3 features
> > + */
> > +#define OMAP3_CONTROL_OMAP_STATUS 0x044c
>
> This should probably go in <mach/control.h>
>
[sp] This was originally in control.h. Just shifted here for this
Patch. Will revert it back.
> > +#define OMAP3_SGX_SHIFT 13
> > +#define OMAP3_SGX_MASK (3 << OMAP3_SGX_SHIFT)
> > +#define FEAT_SGX_FULL 0
> > +#define FEAT_SGX_HALF 1
> > +#define FEAT_SGX_NONE 2
> >
> > +#define OMAP3_IVA_SHIFT 12
> > +#define OMAP3_IVA_MASK (1 << OMAP3_SGX_SHIFT)
> > +#define FEAT_IVA 0
> > +#define FEAT_IVA_NONE 1
> > +
> > +#define OMAP3_L2CACHE_SHIFT 10
> > +#define OMAP3_L2CACHE_MASK (3 << OMAP3_L2CACHE_SHIFT)
> > +#define FEAT_L2CACHE_NONE 0
> > +#define FEAT_L2CACHE_64KB 1
> > +#define FEAT_L2CACHE_128KB 2
> > +#define FEAT_L2CACHE_256KB 3
> > +
> > +#define OMAP3_ISP_SHIFT 5
> > +#define OMAP3_ISP_MASK (1<< OMAP3_ISP_SHIFT)
> > +#define FEAT_ISP 0
> > +#define FEAT_ISP_NONE 1
> > +
> > +#define OMAP3_NEON_SHIFT 4
> > +#define OMAP3_NEON_MASK (1<< OMAP3_NEON_SHIFT)
> > +#define FEAT_NEON 0
> > +#define FEAT_NEON_NONE 1
> > +
> > +
> > +#define OMAP_HAS_L2CACHE 1
> > +#define OMAP_HAS_IVA (1 << 1)
> > +#define OMAP_HAS_SGX (1 << 2)
> > +#define OMAP_HAS_NEON (1 << 3)
> > +#define OMAP_HAS_ISP (1 << 4)
> > +
>
> Use BIT()
> Rename to OMAP3_*
> move to <mach/cpu.h>
[sp] Okay
>
> > static struct omap_chip_id omap_chip;
> > static unsigned int omap_revision;
> > -
> > +static u32 omap_features ;
>
> Call this omap3_features and make it global (with extern in
> <mach/cpu.h>)
>
[sp] Okay.
> > unsigned int omap_rev(void)
> > {
> > @@ -35,6 +75,19 @@ unsigned int omap_rev(void)
> > }
> > EXPORT_SYMBOL(omap_rev);
> >
> > +#define OMAP3_HAS_FEATURE(feat,flag)
> \
> > + unsigned int omap3_has_ ##feat(void) \
>
> make this
> static inline unsigned int omap3_has_ ##feat(void) ...
>
>
> > + { \
> > + return (omap_features & OMAP_HAS_ ##flag); \
> > + } \
> > + EXPORT_SYMBOL(omap3_has_ ##feat);
> >
> > +OMAP3_HAS_FEATURE(l2cache, L2CACHE);
> > +OMAP3_HAS_FEATURE(sgx, SGX);
> > +OMAP3_HAS_FEATURE(iva, IVA);
> > +OMAP3_HAS_FEATURE(neon, NEON);
> > +OMAP3_HAS_FEATURE(isp, ISP);
> > +
>
> ... and move this set to <mach/cpu.h>
>
> and I'm ok with the rest of this patch.
>
> Kevin
>
> > /**
> > * omap_chip_is - test whether currently running OMAP
> matches a chip type
> > * @oc: omap_chip_t to test against
> > @@ -155,7 +208,33 @@ void __init omap24xx_check_revision(void)
> > pr_info("\n");
> > }
> >
> > -void __init omap34xx_check_revision(void)
> > +#define OMAP3_CHECK_FEATURE(status,feat)
> \
> > + if (((status & OMAP3_ ##feat## _MASK)
> \
> > + >> OMAP3_ ##feat## _SHIFT) != FEAT_ ##feat##
> _NONE) { \
> > + omap_features |= OMAP_HAS_ ##feat;
> \
> > + }
> > +
> > +void __init omap3_check_features(void)
> > +{
> > + u32 status, temp;
> > +
> > + omap_features = 0;
> > +
> > + status = omap_ctrl_readl(OMAP3_CONTROL_OMAP_STATUS);
> > +
> > + OMAP3_CHECK_FEATURE(status, L2CACHE);
> > + OMAP3_CHECK_FEATURE(status, IVA);
> > + OMAP3_CHECK_FEATURE(status, SGX);
> > + OMAP3_CHECK_FEATURE(status, NEON);
> > + OMAP3_CHECK_FEATURE(status, ISP);
> > +
> > + /*
> > + * TODO: Get additional info (where applicable)
> > + * e.g. Size of L2 cache.
> > + */
> > +}
> > +
> > +void __init omap3_check_revision(void)
> > {
> > u32 cpuid, idcode;
> > u16 hawkeye;
> > @@ -212,6 +291,22 @@ out:
> > pr_info("OMAP%04x %s\n", omap_rev() >> 16, rev_name);
> > }
> >
> > +#define OMAP3_SHOW_FEATURE(feat) \
> > + if (omap3_has_ ##feat()) { \
> > + pr_info (" - "#feat" : Y"); \
> > + } else { \
> > + pr_info (" - "#feat" : N"); \
> > + }
> > +
> > +void __init omap3_cpuinfo(void)
> > +{
> > + OMAP3_SHOW_FEATURE(l2cache);
> > + OMAP3_SHOW_FEATURE(iva);
> > + OMAP3_SHOW_FEATURE(sgx);
> > + OMAP3_SHOW_FEATURE(neon);
> > + OMAP3_SHOW_FEATURE(isp);
> > +}
> > +
> > /*
> > * Try to detect the exact revision of the omap we're running on
> > */
> > @@ -223,8 +318,11 @@ void __init omap2_check_revision(void)
> > */
> > if (cpu_is_omap24xx())
> > omap24xx_check_revision();
> > - else if (cpu_is_omap34xx())
> > - omap34xx_check_revision();
> > + else if (cpu_is_omap34xx()) {
> > + omap3_check_features();
> > + omap3_check_revision();
> > + omap3_cpuinfo();
> > + }
> > else if (cpu_is_omap44xx()) {
> > printk(KERN_INFO "FIXME: CPU revision = OMAP4430\n");
> > return;
> > --
> > 1.6.2.2
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe
> linux-omap" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
