All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nishanth Menon <nm@ti.com>
To: Kevin Hilman <khilman@deeprootsystems.com>
Cc: linux-omap <linux-omap@vger.kernel.org>, Tony <tony@atomide.com>,
	Thomas <thomas.petazzoni@free-electrons.com>
Subject: Re: [PATCH v4 2/3] omap4: opp: add OPP table data
Date: Tue, 23 Nov 2010 09:19:22 -0600	[thread overview]
Message-ID: <4CEBDB7A.8010807@ti.com> (raw)
In-Reply-To: <87tyj8kii7.fsf@deeprootsystems.com>

Kevin Hilman had written, on 11/22/2010 06:09 PM, the following:
> Nishanth Menon <nm@ti.com> writes:
> 
>> Kevin Hilman wrote, on 11/22/2010 05:19 PM:
>>> Nishanth Menon<nm@ti.com>  writes:
>>>
>>>> This patch adds OPP tables for OMAP4. New file has been added to keep
>>>> the OMAP4 opp tables and the registration of these tables with the
>>>> generic opp framework by OMAP SoC OPP interface.
>>> [...]
>>>
>>>> diff --git a/arch/arm/mach-omap2/opp.c b/arch/arm/mach-omap2/opp.c
>>>> index 66e12be..48a553f 100644
>>>> --- a/arch/arm/mach-omap2/opp.c
>>>> +++ b/arch/arm/mach-omap2/opp.c
>>>> @@ -131,4 +131,5 @@ static int __init omap_init_opp_table(struct omap_opp_def *opp_def,
>>>>
>>>>   /* omap3 opps */
>>>>   #include "opp3xxx_data.c"
>>>> -
>>>> +/* omap4 opps */
>>>> +#include "opp4xxx_data.c"
>>> I'm not sure I like the including of C files.  Any reason you prefer
>>> this to just adding them to the Makefile?  e.g. opp24xx_dta.c are
>>> compiled in via Makefile and these two are included.
>> I dont buy it. I am seeing us go around in circles for this:
>> http://marc.info/?l=linux-omap&m=128986880406272&w=2
>> a) we dont want others to use specifics implemented in opp.c in other
>> files (e.g. board files)
> 
> not sure how this is prevented.
The approach is as follows:
the #defines and struct definitions are in opp.c
opp{3,4}xxx_data.c use the same - by itself wont build because they need 
the defines in opp.c
that way, there is no possibility of clean hacks possible except to 
follow the current mechanism for a future omap silicon

The main objective was to restrict the potential of board developers 
from hacking the default OPP table - which would be possible by exposing 
the headers for board files - which as Thomas rightly pointed out in the 
  thread I pointed out,  opens up a possibility for board files to 
include them as well and re-instantiate the table again..

> 
>> b) we have many similar usage in linux kernel - so this usage is not
>> first time.
>> c) opp2xx usage is very different from opp3/4 usage
> 
> I'm not going to insist on one way or the other, just stating my
> preference for not including C files from C files without good
> justification.  You've stated your reasons, I guess Tony can decide.
okay by me - will post a v5 after seeing the final decision on that front :)

-- 
Regards,
Nishanth Menon

  reply	other threads:[~2010-11-23 15:19 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <[PATCH v3 1/3] omap: opp: add OMAP3 OPP table data and common init>
2010-11-16 18:54 ` [PATCH v4 0/3] OMAP: Add opp data Nishanth Menon
2010-11-16 18:54 ` [PATCH v4 1/3] omap: opp: add OMAP3 OPP table data and common init Nishanth Menon
2010-11-22 22:21   ` Kevin Hilman
2010-11-16 18:54 ` [PATCH v4 2/3] omap4: opp: add OPP table data Nishanth Menon
2010-11-22 22:45   ` Kevin Hilman
2010-11-22 22:53     ` Nishanth Menon
2010-11-22 23:12       ` Kevin Hilman
2010-11-22 23:19   ` Kevin Hilman
2010-11-22 23:30     ` Nishanth Menon
2010-11-23  0:09       ` Kevin Hilman
2010-11-23 15:19         ` Nishanth Menon [this message]
2010-11-23 20:38           ` Kevin Hilman
2010-11-23 22:33           ` Cousson, Benoit
2010-11-23 22:56             ` Nishanth Menon
2010-11-23 23:30               ` Cousson, Benoit
2010-11-24  0:16                 ` Kevin Hilman
2010-11-24  2:34                   ` Nishanth Menon
2010-11-16 18:54 ` [PATCH v3 3/3] OMAP3: remove OPP interfaces from OMAP PM layer 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=4CEBDB7A.8010807@ti.com \
    --to=nm@ti.com \
    --cc=khilman@deeprootsystems.com \
    --cc=linux-omap@vger.kernel.org \
    --cc=thomas.petazzoni@free-electrons.com \
    --cc=tony@atomide.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.