> -----Original Message-----
> From: Olof Johansson [mailto:[email protected]]
> Sent: Monday, September 07, 2009 6:04 PM
> To: Premi, Sanjeev
> Cc: [email protected]
> Subject: Re: [RFC] Common mechanism to identify Si revision
>
> Hi,
>
> On Mon, Sep 07, 2009 at 11:33:32AM +0530, Premi, Sanjeev wrote:
>
> > > What's the purpose of most of these checks in the first
> place? I can
> > > see two immediate needs:
> > >
> > > 1) To check for various errata and do appropriate workarounds
> >
> > [sp] I believe the only need would be to make easy check if
> the version
> > has multiple errata fixes and/or enhancements. And, of
> course, a
> > verbose information to user who may not have (and may not need)
> > information of all the errata/enhancements.
> >
> > Today, there are multiple ways of doing the same
> thing... Each way
> > builds upon minor issues in the existing one; but adds its own.
>
> Yeah, ok. I was just fearing that this added ES-level
> checking would be
> used to clutter up the code with extra testing instead of
> hiding it away in
> header files. But it seems like most of it is covered already.
>
>
> > > 2) To check if the current chip has a certain feature
> > >
> > > Both of these could just as well be abstracted away such
> that you use
> > > tests on the form:
> > >
> > > if (OMAP_HAS_ERRATA_FOO) ...
> > >
> > > or:
> > > if (OMAP_FEATURE_FOO) ...
> > >
> > > And then move the actual checking of a feature into the
> header file
> > > where the errata/feature setups are defined.
> >
> > [sp I have submitted a patch that takes the first step toward this:
> > http://marc.info/?l=linux-omap&m=125050987112798&w=2
> > ...still waiting to hear from Tony on this.
>
> Looks good. I didn't subscribe to linux-omap until recently so I don't
> have the original emails (and thus can't comment on them), but my only
> feedback is that it really doesn't belong in /proc/cpuinfo whether a
> SoC has SGX on-chip or not.
>
> It'd be better to move that information somewhere else. Keep in mind
> that adding information to a /proc file means that you are changing an
> API that we would need to live with forever.
>
> > > There's two major benefits to this:
> > >
> > > 1) Readability. No need to sit and look up in a manual
> why there's a
> > > check for version X here.
> > > (and/or no need to add a specific comment about it).
> > >
> > > 2) Keeping changes centralized. If there's a new revision or chip,
> > > there's just one header file to update, not 20 different
> source files.
> > >
> > > For example, a bunch of the checks in pm34xx.c would be nicer
> > > to have as:
> > >
> > > if (OMAP_HAS_USBHOST())
> >
> > [sp] Can you look and comment on this discussion as well:
> > http://marc.info/?l=linux-omap&m=125017671720718&w=2
>
> I think it is a great step in the right direction. My only concern is
> that the usage conventions are confusing:
>
> OMAP3_HAS_FEATURE(neon, NEON)
[sp] This is only used to declare a function that would translate to:
unsigned int omap3_has_neon(void)
{
return (omap_features & OMAP_HAS_NEON);
}
EXPORT_SYMBOL(omap3_has_neon);
A user would always do something like:
if (omap_has_neon())
do_neon_specific_stuff();
Best regards,
Sanjeev
>
> Just seeing that, what is the difference between neon and NEON? Also,
> it always requires two consecutive calls to check for a feature (get
> the flags, then check the mask).
>
> I would personally prefer to keep a global that is initialized early
> in boot (called __omap_features or so -- the __-prefix would indicate
> that it is not for direct use in code), then have OMAP3_HAS_FEATURE()
> just look at that value instead of having it passed in. It's not like
> it changes during runtime anyway.
>
> It can be debated if the format should then be
> OMAP3_HAS_FEATURE_SGX or
> OMAP3_HAS_FEATURE(SGX), but either is fine with me.
>
>
> -Olof
>
> --
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
