public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] OMAP3: Introduce CONFIG option for power code
@ 2009-05-06 15:21 Dirk Behme
  2009-05-07 20:46 ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 1 reply; 8+ messages in thread
From: Dirk Behme @ 2009-05-06 15:21 UTC (permalink / raw)
  To: u-boot

Some OMAP3 boards need control for external power companion
chips. Introduce a CONFIG option for this, to avoid Makefile
changes for each board.

Signed-off-by: Dirk Behme <dirk.behme@googlemail.com>
---

Compile tested with ./MAKEALL ARM_CORTEX_A8, boot tested on
OMAP3 based BeagleBoard.

Note: This patch trys to fix the first comment of

http://lists.denx.de/pipermail/u-boot/2009-April/051383.html

 board/omap3/common/Makefile     |    5 +----
 doc/README.omap3                |   11 +++++++++++
 include/configs/omap3_beagle.h  |    1 +
 include/configs/omap3_overo.h   |    1 +
 include/configs/omap3_pandora.h |    1 +
 include/configs/omap3_zoom1.h   |    1 +
 6 files changed, 16 insertions(+), 4 deletions(-)

Index: u-boot-main/doc/README.omap3
===================================================================
--- u-boot-main.orig/doc/README.omap3
+++ u-boot-main/doc/README.omap3
@@ -77,6 +77,17 @@ For all other commands see
 
 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.
+
+This enables compilation of board/omap3/common/power.c.
+
 Acknowledgements
 ================
 
Index: u-boot-main/board/omap3/common/Makefile
===================================================================
--- u-boot-main.orig/board/omap3/common/Makefile
+++ u-boot-main/board/omap3/common/Makefile
@@ -29,10 +29,7 @@ endif
 
 LIB	= $(obj)lib$(VENDOR).a
 
-COBJS-$(CONFIG_OMAP3_BEAGLE) += power.o
-COBJS-$(CONFIG_OMAP3_OVERO) += power.o
-COBJS-$(CONFIG_OMAP3_PANDORA) += power.o
-COBJS-$(CONFIG_OMAP3_ZOOM1) += power.o
+COBJS-$(CONFIG_OMAP3_POWER) += power.o
 
 COBJS	:= $(COBJS-y)
 SRCS	:= $(COBJS:.o=.c)
Index: u-boot-main/include/configs/omap3_beagle.h
===================================================================
--- u-boot-main.orig/include/configs/omap3_beagle.h
+++ u-boot-main/include/configs/omap3_beagle.h
@@ -37,6 +37,7 @@
 #define CONFIG_OMAP34XX		1	/* which is a 34XX */
 #define CONFIG_OMAP3430		1	/* which is in a 3430 */
 #define CONFIG_OMAP3_BEAGLE	1	/* working with BEAGLE */
+#define CONFIG_OMAP3_POWER	1	/* use power code */
 
 #include <asm/arch/cpu.h>		/* get chip and board defs */
 #include <asm/arch/omap3.h>
Index: u-boot-main/include/configs/omap3_overo.h
===================================================================
--- u-boot-main.orig/include/configs/omap3_overo.h
+++ u-boot-main/include/configs/omap3_overo.h
@@ -29,6 +29,7 @@
 #define CONFIG_OMAP34XX		1	/* which is a 34XX */
 #define CONFIG_OMAP3430		1	/* which is in a 3430 */
 #define CONFIG_OMAP3_OVERO		1	/* working with overo */
+#define CONFIG_OMAP3_POWER	1	/* use power code */
 
 #include <asm/arch/cpu.h>	/* get chip and board defs */
 #include <asm/arch/omap3.h>
Index: u-boot-main/include/configs/omap3_pandora.h
===================================================================
--- u-boot-main.orig/include/configs/omap3_pandora.h
+++ u-boot-main/include/configs/omap3_pandora.h
@@ -32,6 +32,7 @@
 #define CONFIG_OMAP34XX		1	/* which is a 34XX */
 #define CONFIG_OMAP3430		1	/* which is in a 3430 */
 #define CONFIG_OMAP3_PANDORA	1	/* working with pandora */
+#define CONFIG_OMAP3_POWER	1	/* use power code */
 
 #include <asm/arch/cpu.h>	/* get chip and board defs */
 #include <asm/arch/omap3.h>
Index: u-boot-main/include/configs/omap3_zoom1.h
===================================================================
--- u-boot-main.orig/include/configs/omap3_zoom1.h
+++ u-boot-main/include/configs/omap3_zoom1.h
@@ -38,6 +38,7 @@
 #define CONFIG_OMAP34XX		1	/* which is a 34XX */
 #define CONFIG_OMAP3430		1	/* which is in a 3430 */
 #define CONFIG_OMAP3_ZOOM1	1	/* working with Zoom MDK Rev1 */
+#define CONFIG_OMAP3_POWER	1	/* use power code */
 
 #include <asm/arch/cpu.h>		/* get chip and board defs */
 #include <asm/arch/omap3.h>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH] OMAP3: Introduce CONFIG option for power code
  2009-05-06 15:21 [U-Boot] [PATCH] OMAP3: Introduce CONFIG option for power code Dirk Behme
@ 2009-05-07 20:46 ` Jean-Christophe PLAGNIOL-VILLARD
  2009-05-08 15:27   ` Dirk Behme
  0 siblings, 1 reply; 8+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2009-05-07 20:46 UTC (permalink / raw)
  To: u-boot

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>
for the davinci it's already start to move to it

Best Regards,
J.
> 
> Signed-off-by: Dirk Behme <dirk.behme@googlemail.com>
> ---
> 
> Compile tested with ./MAKEALL ARM_CORTEX_A8, boot tested on
> OMAP3 based BeagleBoard.
> 
> Note: This patch trys to fix the first comment of
> 
> http://lists.denx.de/pipermail/u-boot/2009-April/051383.html
> 
>  board/omap3/common/Makefile     |    5 +----
>  doc/README.omap3                |   11 +++++++++++
>  include/configs/omap3_beagle.h  |    1 +
>  include/configs/omap3_overo.h   |    1 +
>  include/configs/omap3_pandora.h |    1 +
>  include/configs/omap3_zoom1.h   |    1 +
>  6 files changed, 16 insertions(+), 4 deletions(-)
> 
> Index: u-boot-main/doc/README.omap3
> ===================================================================
> --- u-boot-main.orig/doc/README.omap3
> +++ u-boot-main/doc/README.omap3
> @@ -77,6 +77,17 @@ For all other commands see
>  
>  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

Best Regards,
J.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH] OMAP3: Introduce CONFIG option for power code
  2009-05-07 20:46 ` Jean-Christophe PLAGNIOL-VILLARD
@ 2009-05-08 15:27   ` Dirk Behme
  2009-05-10 15:16     ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 1 reply; 8+ messages in thread
From: Dirk Behme @ 2009-05-08 15:27 UTC (permalink / raw)
  To: u-boot

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.

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.

> Best Regards,
> J.

I nearly missed your comment below because of this ;)

>> Signed-off-by: Dirk Behme <dirk.behme@googlemail.com>
>> ---
>>
>> Compile tested with ./MAKEALL ARM_CORTEX_A8, boot tested on
>> OMAP3 based BeagleBoard.
>>
>> Note: This patch trys to fix the first comment of
>>
>> http://lists.denx.de/pipermail/u-boot/2009-April/051383.html
>>
>>  board/omap3/common/Makefile     |    5 +----
>>  doc/README.omap3                |   11 +++++++++++
>>  include/configs/omap3_beagle.h  |    1 +
>>  include/configs/omap3_overo.h   |    1 +
>>  include/configs/omap3_pandora.h |    1 +
>>  include/configs/omap3_zoom1.h   |    1 +
>>  6 files changed, 16 insertions(+), 4 deletions(-)
>>
>> Index: u-boot-main/doc/README.omap3
>> ===================================================================
>> --- u-boot-main.orig/doc/README.omap3
>> +++ u-boot-main/doc/README.omap3
>> @@ -77,6 +77,17 @@ For all other commands see
>>  
>>  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?

Dirk

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH] OMAP3: Introduce CONFIG option for power code
  2009-05-08 15:27   ` Dirk Behme
@ 2009-05-10 15:16     ` Jean-Christophe PLAGNIOL-VILLARD
  2009-05-10 17:03       ` Dirk Behme
  0 siblings, 1 reply; 8+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2009-05-10 15:16 UTC (permalink / raw)
  To: u-boot

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 the current omap3 layout is wrong

it's board/<soc>

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

as davinci which is starting to be clean
>
> 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
or to drivers/


>>>   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

Best Regards,
J.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH] OMAP3: Introduce CONFIG option for power code
  2009-05-10 15:16     ` Jean-Christophe PLAGNIOL-VILLARD
@ 2009-05-10 17:03       ` Dirk Behme
  2009-05-12  1:13         ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 1 reply; 8+ messages in thread
From: Dirk Behme @ 2009-05-10 17:03 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH] OMAP3: Introduce CONFIG option for power code
  2009-05-10 17:03       ` Dirk Behme
@ 2009-05-12  1:13         ` Jean-Christophe PLAGNIOL-VILLARD
  2009-05-12 17:41           ` Dirk Behme
  0 siblings, 1 reply; 8+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2009-05-12  1:13 UTC (permalink / raw)
  To: u-boot

>
>> 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.
and the code is start to be moved
>
> 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.
not 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.
no I2C is the communication bus, but you write a i2c drivers to manage the
power chips so it's a drivers

>
> 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 ;)
why not start with the chip name or the familly name if they can be manage by
the same driver

Best Regards,
J.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH] OMAP3: Introduce CONFIG option for power code
  2009-05-12  1:13         ` Jean-Christophe PLAGNIOL-VILLARD
@ 2009-05-12 17:41           ` Dirk Behme
  2009-05-12 22:34             ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 1 reply; 8+ messages in thread
From: Dirk Behme @ 2009-05-12 17:41 UTC (permalink / raw)
  To: u-boot

Dear Jean-Christophe,

Jean-Christophe PLAGNIOL-VILLARD wrote:

[some unanswered questions removed here?]

>>> 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.
> and the code is start to be moved

Sorry, but this confuses me because I can't find this "start" 
anywhere. So for me this sounds like "I have some (undiscussed) 
changes in mind or in my local tree, nobody does know about, but every 
new patch has to care about this (publically unknown) changes" (i.e. 
this is 'supposed' to be used to use your wording). If this 
understanding is right, no, this way of dealing with patches and 
NACKing (?) them doesn't work. If this understanding is wrong, sorry, 
please correct.

>> 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.
> not to me

Then, as the easiest way of dealing with directory/file moves is doing 
it directly in your git, the best would be if you do this re-sorting 
directly.

NACKing a quite trivial patch (this one) because of an existing 
directory/file layout you don't like, while you requested yourself 
this change we talk about, and making it a show stopper for adding 
additional boards [1] like e.g. Matthias' one, sounds somehow strange 
to me, btw.

[1] http://lists.denx.de/pipermail/u-boot/2009-April/051383.html

>>>> 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.
> no I2C is the communication bus, but you write a i2c drivers to manage the
> power chips so it's a drivers

No. I don't write a driver. I just use a driver. If you want to name 
code that uses a driver a driver again, then you wouldn't need the 
board directory any more. Unfortunately, you removed my argumentation 
regarding this from the previous mail:

-- cut --
.... 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
-- cut --

Additionally, if you don't like the Davinci example, let's take an 
other one randomly chosen:

http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=blob;f=board/freescale/mpc8568mds/mpc8568mds.c;h=8f991e537df10d8ff48d7783e00bb4aeaa231c79;hb=HEAD

Following your argumentation, for pib_init() you would need an IO 
expander driver. For pci_init_board() you would need a PCI driver. 
Both functions are larger than what we talk about here, btw. If you go 
on with this, you have to create a LED driver for a one liner which 
enables/disables a LED using a gpio driver. And at the end you will 
have an empty board directory.

>> 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 ;)
> why not start with the chip name 

I think I mentioned above that there are "different chip names". But
anyway ...

> or the familly name if they can be manage by
> the same driver

(no 'driver' btw)

... as mentioned above I will have a look if I can find some matching
naming and send an updated patch with this.

Best regards

Dirk

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH] OMAP3: Introduce CONFIG option for power code
  2009-05-12 17:41           ` Dirk Behme
@ 2009-05-12 22:34             ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 0 replies; 8+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2009-05-12 22:34 UTC (permalink / raw)
  To: u-boot

On 19:41 Tue 12 May     , Dirk Behme wrote:
> Dear Jean-Christophe,
>
> Jean-Christophe PLAGNIOL-VILLARD wrote:
>
> [some unanswered questions removed here?]
>
>>>> 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.
>> and the code is start to be moved
>
> Sorry, but this confuses me because I can't find this "start" anywhere. 
davinci: display correct clock info
http://git.denx.de/?p=u-boot/u-boot-arm.git;a=commit;h=5516e87d057c13fce05b6fc6e56cf3f4ddfc36b4
davinci: move psc support board-->cpu
http://git.denx.de/?p=u-boot/u-boot-arm.git;a=commit;h=20bb2384e2b81e77880993134c8f268d59fdfe07
>
>>> 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.
>> not to me
>
> Then, as the easiest way of dealing with directory/file moves is doing  
> it directly in your git, the best would be if you do this re-sorting  
> directly.
>
> NACKing a quite trivial patch (this one) because of an existing  
> directory/file layout you don't like, while you requested yourself this 
> change we talk about, and making it a show stopper for adding additional 
> boards [1] like e.g. Matthias' one, sounds somehow strange to me, btw.
>
> [1] http://lists.denx.de/pipermail/u-boot/2009-April/051383.html
>
>>>>> 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.
>> no I2C is the communication bus, but you write a i2c drivers to manage the
>> power chips so it's a drivers
>
> No. I don't write a driver. I just use a driver. If you want to name  
> code that uses a driver a driver again, then you wouldn't need the board 
> directory any more. Unfortunately, you removed my argumentation  
> regarding this from the previous mail:
>
> -- cut --
> .... 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.
in correct as recently the soc code as device init was move to
cpu/arm926ejs/davinci

this only remaining code is the trivial dram_init
and the EEProm Mac address management

> If you go on with 
> this, you have to create a LED driver for a one liner which  
> enables/disables a LED using a gpio driver. And at the end you will have 
> an empty board directory.
it's done for the at91 led drivers to avoid copy&paste
and it's in cpu/arm926ejs/at91
>
>>> 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 ;)
>> why not start with the chip name 
>
> I think I mentioned above that there are "different chip names". But
> anyway ...
I agree but to simplify it why not use at the beggening the current supported
or use chip
>
>> or the familly name if they can be manage by
>> the same driver
>
> (no 'driver' btw)
we disagree on the term driver

is this chip is useable for other board than omap3?
if yes (which I suspect correct me if I'm wrong) then it's use is not omap3
specific at all

>
> ... as mentioned above I will have a look if I can find some matching
> naming and send an updated patch with this.
ok

Best Regards,
J.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2009-05-12 22:34 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-05-06 15:21 [U-Boot] [PATCH] OMAP3: Introduce CONFIG option for power code Dirk Behme
2009-05-07 20:46 ` Jean-Christophe PLAGNIOL-VILLARD
2009-05-08 15:27   ` Dirk Behme
2009-05-10 15:16     ` Jean-Christophe PLAGNIOL-VILLARD
2009-05-10 17:03       ` Dirk Behme
2009-05-12  1:13         ` Jean-Christophe PLAGNIOL-VILLARD
2009-05-12 17:41           ` Dirk Behme
2009-05-12 22:34             ` Jean-Christophe PLAGNIOL-VILLARD

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox