All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nishanth Menon <nm@ti.com>
To: "Dasgupta, Romit" <romit@ti.com>
Cc: Nishanth Menon <menon.nishanth@gmail.com>,
	"khilman@deeprootsystems.com" <khilman@deeprootsystems.com>,
	"linux-omap@vger.kernel.org" <linux-omap@vger.kernel.org>
Subject: Re: [PM-WIP-OPP][PATCH] OPP: Introduces enum for addressing different OPP types
Date: Fri, 15 Jan 2010 04:56:56 -0600	[thread overview]
Message-ID: <4B5049F8.2090708@ti.com> (raw)
In-Reply-To: <4B504678.8030407@ti.com>

Dasgupta, Romit had written, on 01/15/2010 04:42 AM, the following:
[..]
> I like this message. I will include it.
:) I am getting this wrong feel that I should start writing novels ;).

[...]
>> now, I was recommending doing a, till a thought a little more on the 
>> implementation(array based) and how long that implementation might 
>> last(we might potentially move opp.c to a list implementation). the 
>> effort would be to complicate the opp_init,add functions for a very 
>> short lifetime. This effort maynot be worth it.
> 
> I understand your concern. I have made some changes in the code. Please look at
> the reposted patch (in few mins from now I shall post them).
Thanks..

>>>>>> Enum type and variable have the same name :( mebbe a rename of variable is
>>>>>> appropriate
>>>>>>     
>>>>> Not sure why you say this. Did you see the compiler throwing up any warning?
>>>>>   
>>>> The usage later in the code is opp_t -> this is a readability issue not 
>>>> a compiler warning.
>>> What is the readability issue? Why cant we declare something like enum opp_t opp_t?
>> Let me try to explain this clearly. assume we have a struct opp_t (not 
>> enum) for the time being.
>> void some_func(struct opp_t *opp_t)
>> {
>>    struct opp_t *opp;
>>
>> ..
>> 200 line of code (>one page full)
>> ....
>> /* point 1 */
>>     BUG_ON(opp_t.xyz)
>> ...
>>   200 lines of more code
>> ..
>> /* point 2 */
>>     BUG_ON(opp.xyz)
>> ...
>>
>> }
>>
>> lets say this is compiled by some non follower of this mail chain,
>> compiler throws an error for point 1: filex:liney
>> so the guy/gal fires up vim and opens the filex, goes to line y
>> he/she cannot see the start of the function, knows that there is a 
>> struct opp_t
> 
> If a function is that big then the fault lies there to start with! What do you say?
> Nevertheless, your suggestion is cosmetic but I think we should not assume that
> developers are so ignorant. For now I will do away with your suggestion. Please
> feel free to change the code if you think what you say is the right thing.
Having spent a few years on properitory s/w and been one of those 
confused blokes(I being a certified nitwit), I will try to see if I can 
provide a patch on top and Kevin/community can choose to add their 
comments/club the patches and make the few folks like me happy.

-- 
Regards,
Nishanth Menon

  reply	other threads:[~2010-01-15 10:56 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-01-12 12:39 [PM-WIP-OPP][PATCH] OPP: Introduces enum for addressing different OPP types Romit Dasgupta
2010-01-12 17:19 ` Nishanth Menon
2010-01-12 17:19 ` Kevin Hilman
2010-01-12 17:36   ` Cousson, Benoit
2010-01-12 19:26     ` Kevin Hilman
2010-01-13 10:31   ` Romit Dasgupta
2010-01-12 17:57 ` Menon, Nishanth
2010-01-13 10:41   ` Romit Dasgupta
2010-01-13 12:54     ` Nishanth Menon
2010-01-13 13:22       ` Romit Dasgupta
2010-01-15 10:35         ` Nishanth Menon
2010-01-15 10:42           ` Romit Dasgupta
2010-01-15 10:56             ` Nishanth Menon [this message]
2010-01-13 14:43     ` Kevin Hilman

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=4B5049F8.2090708@ti.com \
    --to=nm@ti.com \
    --cc=khilman@deeprootsystems.com \
    --cc=linux-omap@vger.kernel.org \
    --cc=menon.nishanth@gmail.com \
    --cc=romit@ti.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.