linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: nm@ti.com (Nishanth Menon)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/2] OMAP: features: export symbol omap3_features
Date: Fri, 24 Sep 2010 11:51:58 -0500	[thread overview]
Message-ID: <4C9CD72E.40407@ti.com> (raw)
In-Reply-To: <alpine.DEB.2.00.1009240920370.10678@utopia.booyaka.com>

Paul Walmsley had written, on 09/24/2010 10:34 AM, the following:
[...]
>>  
>>  u32 omap3_features;
>> +EXPORT_SYMBOL(omap3_features);
>>  
>>  unsigned int omap_rev(void)
>>  {
>> -- 
>> 1.7.1
>>
> 
> This omap3_features variable should not be used directly by any device 
> drivers since it is an OMAP-ism.  This type of feature info should be 
> passed through struct platform_data.  Looks like this would be quite easy 
> to add by editing mach-omap2/devices.c and adding platform_data?
> 
> ...
> 
> In the medium-term, definitely all of those 
> 
>     #if defined(CONFIG_ARCH_OMAP*)
> 
> and 
> 
>     if (cpu_is_omap*()) {
> 
> in this driver need to be removed.  The integration code (currently in 
> mach-omap2/devices.c) is what should handle this, or better yet, struct 
> dev_attr from hwmod.
> 
> Also the entire mailboxes driver at some point should be moved into 
> drivers/* ...
just my 2 cents:

The intent of omap_has_featureX() is to ensure that the drivers dont do 
cpu_is_omap123(). Now if we had OMAP dma driver which has DMA chaining - 
what options do we have DMA driver?

a) we introduce it based on cpu_is_omap123() -> bad bad nightmare for 
maintenance
b) we introduce it based on module h/w block(TI internal terminology "IP 
block") revision -> unfortunately no luck in some of the h/w blocks.
c) we use if (omap_has_dma_chaining())

If the hwmod itself was expanded to contain feature, and erratas then it 
is excellent solution - one stop information source for all drivers and 
a standardized soln as well.. (but we dont have it on omap1).

by not exporting this, the drivers are restricted to being static, and 
exporting allows those omap specific ones to be loadable (if someone 
really wants so minimalist a system - i believe i2c is an example of 
loadable module)..

I believe omap_has_feature() is a readable and maintainable way of 
handling features and exporting helps those drivers to become modules 
(yes with the risk of some other driver misusing it as well) - this 
definitely is something we've been doing for some time in l-o I guess.. 
a better alternative might be hwmod - but I am no expert to comment on 
it's feasibility..

-- 
Regards,
Nishanth Menon

  reply	other threads:[~2010-09-24 16:51 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-24  3:41 [PATCH 1/2] OMAP: features: export symbol omap3_features Omar Ramirez Luna
2010-09-24 14:58 ` Kevin Hilman
2010-09-24 15:34 ` Paul Walmsley
2010-09-24 16:51   ` Nishanth Menon [this message]
2010-09-24 17:06     ` Paul Walmsley
2010-09-24 17:17       ` Paul Walmsley
2010-09-24 17:26         ` Nishanth Menon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4C9CD72E.40407@ti.com \
    --to=nm@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).