[U-Boot] [PATCH] OMAP3: Introduce CONFIG option for power code

[Dirk Behme]

Dear Jean-Christophe,,

Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 17:27 Fri 08 May     , Dirk Behme wrote:
>> Dear Jean-Christophe,
>>
>> Jean-Christophe PLAGNIOL-VILLARD wrote:
>>> On 17:21 Wed 06 May     , Dirk Behme wrote:
>>>> Some OMAP3 boards need control for external power companion
>>>> chips. Introduce a CONFIG option for this, to avoid Makefile
>>>> changes for each board.
>>> please also move it to cpu/arm_cortexa8/omap3/
>>>
>>> omap3 and davinci is not a vendor
>>> so common code will need to go to drivers or cpu/<arch>/<soc>
>> No, this wouldn't be correct. We used board/omap3/common/ for this  
>> intentionally.
> no 

Yes ;) Yes, we use the directory board/omap3/common/ intentionally.

> the current omap3 layout is wrong

Sorry, but I can't follow your argumentation here any more. Above, you 
proposed to move the power code to cpu/arm_cortexa8/omap3/. So I 
understand the topic we talk about is "where to locate the file 
power.c". Now, you mention that you think the layout of the directory 
board/omap3 is wrong (?). This looks like two different discussion 
topics to me, how is this related?

> it's board/<soc>
> 
> but it's supposed to be board/<vendor>

Where do you get this supposing from? Looking at /board I find there e.g.

sbc2410x
sbc405
sbc8240
sbc8260
sbc8349
etc.
etc.

and I don't think these are <vendor>?

> as davinci which is starting to be clean

Sorry, but I can't find any board/ti which would be board/<vendor> you 
mention above. Even not for davinci. I looked into u-boot-arm-master 
and -next.

But what I can find in both trees are

board/davinci/common
board/davinci/dvevm
board/davinci/schmoogie
board/davinci/<all other Davinci boards>

This is 100% the same layout we use for OMAP3 boards. Looks fine to me.

>> We talk about *board* specific code here, it is totally unrelated to  
>> <arch> or the <soc> we use. This board specific code configures an OMAP3 
>> (SoC) external companion chip which is on the board (or not). Some boards 
>> which share the basic layout have this companion chip, some not. Please 
>> note that original config options (we remove with this patch) were the 
>> *board* config options (e.g. CONFIG_OMAP3_BEAGLE) to enable the 
>> compilation of power.c, too.
> as show now the power.c code is shared by a lot's of omap3 boards
> and as you said it's a power companion for the omap3
> 
> so 2 choices
> move the code to cpu/omap3 as it's omap3 specific

No and no. Above I mentioned why cpu/ is wrong (because it's board 
related code) and that it's not OMAP3 (SoC) specific. It's (OMAP3) 
*board* specific.

> or to drivers/

Driver makes no sense either, because it's not a driver. Power.c 
*uses* drivers e.g. I2C (like the board code) to access board 
components. If you would move power.c to drivers, it would be similar 
if you would move everything in board/* to drivers.

Please have a look to code in board/davinci/common. These are no 
drivers, too. It's board code, which normally is in 
board/davinci/<board>/<board>.c file, but is moved to /common to avoid 
copy & paste. Same is with OMAP3 power.c code. It was originally in 
board/omap3/<board>/<board>.c file, but you (correctly) asked to move 
it to a common place to avoid copy & paste. If you like, we can move 
it back into <board>.c files ;)

>>>>   help
>>>>  +Custom configs
>>>> +==============
>>>> +
>>>> +CONFIG_OMAP3_POWER
>>>> +
>>>> +Some OMAP3 boards use external power companion chips to be configured. Enable
>>>> +this CONFIG option in your board specific configuration file if your board
>>>> +uses such a companion chip.
>>> could you name it ine the config and the file name
>> Sorry, I don't understand what you want here. What should I name where? 
>> Which file name?
> the name of the companion chip as you can use different one

You mean I shall mention the name of the companion chip(s) in the 
above README.omap3 and the power.c, correct? (just to get this 
understanding right)

Yes, I can do this. Unfortunately, you can't imagine how clever TI is 
with selling mainly the same functionality (chips) with different chip 
names. So most probably there is more than on chip name, and I'm not 
sure if I will get them right. Most probably only TI marketing 
department will get this right ;)

Best regards

Dirk