diff mbox

[U-Boot] ARM: tegra: remove custom define for Jetson TK1

Message ID 1406849378-32034-1-git-send-email-swarren@wwwdotorg.org
State Accepted
Delegated to: Tom Warren
Headers show

Commit Message

Stephen Warren July 31, 2014, 11:29 p.m. UTC
From: Stephen Warren <swarren@nvidia.com>

Now that Kconfig has a per-board option, we can use that directly rather
than inventing a custom define for the AS3722 code to determine which
board it's being built for.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 board/nvidia/venice2/as3722_init.h | 2 +-
 configs/jetson-tk1_defconfig       | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

Comments

Masahiro Yamada Aug. 1, 2014, 3:12 a.m. UTC | #1
Hi Stephen,



On Thu, 31 Jul 2014 17:29:38 -0600
Stephen Warren <swarren@wwwdotorg.org> wrote:

> From: Stephen Warren <swarren@nvidia.com>
> 
> Now that Kconfig has a per-board option, we can use that directly rather
> than inventing a custom define for the AS3722 code to determine which
> board it's being built for.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
>  board/nvidia/venice2/as3722_init.h | 2 +-
>  configs/jetson-tk1_defconfig       | 1 -
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/board/nvidia/venice2/as3722_init.h b/board/nvidia/venice2/as3722_init.h
> index a7b24039f6aa..06c366e0d0d8 100644
> --- a/board/nvidia/venice2/as3722_init.h
> +++ b/board/nvidia/venice2/as3722_init.h
> @@ -18,7 +18,7 @@
>  #define AS3722_LDO6VOLTAGE_REG	0x16	/* VDD_SDMMC */
>  #define AS3722_LDCONTROL_REG	0x4E
>  
> -#ifdef CONFIG_BOARD_JETSON_TK1
> +#ifdef CONFIG_TARGET_JETSON_TK1
>  #define AS3722_SD0VOLTAGE_DATA	(0x3C00 | AS3722_SD0VOLTAGE_REG)
>  #else
>  #define AS3722_SD0VOLTAGE_DATA	(0x2800 | AS3722_SD0VOLTAGE_REG)
> diff --git a/configs/jetson-tk1_defconfig b/configs/jetson-tk1_defconfig
> index 9ce97c9f61b2..6926257d894b 100644
> --- a/configs/jetson-tk1_defconfig
> +++ b/configs/jetson-tk1_defconfig
> @@ -1,4 +1,3 @@
>  CONFIG_SPL=y
> -CONFIG_SYS_EXTRA_OPTIONS="BOARD_JETSON_TK1="
>  +S:CONFIG_ARM=y
>  +S:CONFIG_TARGET_JETSON_TK1=y
> -- 
> 1.9.1



Or you may rename CONFIG_TARGET_JETSON_TK1  to CONFIG_BOARD_JETSON_TK1
or another name.

I don't want to force CONFIG_TARGET_  name convention.
You can change config names if you like. It is up to you.


FYI, this is the reason why I used CONFIG_TARGET_  when I
auto-generated the initial version of Kconfig and defconfig files:

The 'integrator' board is supported for various ARM cores such as
arm720t, ar920t, arm926ejs, arm1136.  They use the same board
but different config headers.
So CONFIG_BOARD_INTEGRATOR is not an identical macro pointing to
the config header.

When I see Tegra family, each board has its own config header.
CONFIG_BOARD_ as well as CONFIG_TARGET_ will work.



In future, perhaps only the SoC select
(CONFIG_TEGRA20, _TEGRA30, _TEGRA114, _TEGRA124)
will rename in the Kconfig and
the board select might be pushed into device trees.
I am not sure..






Best Regards
Masahiro Yamada
Stephen Warren Aug. 1, 2014, 6:36 p.m. UTC | #2
On 07/31/2014 09:12 PM, Masahiro Yamada wrote:
> Hi Stephen,
>
>
>
> On Thu, 31 Jul 2014 17:29:38 -0600
> Stephen Warren <swarren@wwwdotorg.org> wrote:
>
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> Now that Kconfig has a per-board option, we can use that directly rather
>> than inventing a custom define for the AS3722 code to determine which
>> board it's being built for.
>>
>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>> ---
>>   board/nvidia/venice2/as3722_init.h | 2 +-
>>   configs/jetson-tk1_defconfig       | 1 -
>>   2 files changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/board/nvidia/venice2/as3722_init.h b/board/nvidia/venice2/as3722_init.h
>> index a7b24039f6aa..06c366e0d0d8 100644
>> --- a/board/nvidia/venice2/as3722_init.h
>> +++ b/board/nvidia/venice2/as3722_init.h
>> @@ -18,7 +18,7 @@
>>   #define AS3722_LDO6VOLTAGE_REG	0x16	/* VDD_SDMMC */
>>   #define AS3722_LDCONTROL_REG	0x4E
>>
>> -#ifdef CONFIG_BOARD_JETSON_TK1
>> +#ifdef CONFIG_TARGET_JETSON_TK1
>>   #define AS3722_SD0VOLTAGE_DATA	(0x3C00 | AS3722_SD0VOLTAGE_REG)
>>   #else
>>   #define AS3722_SD0VOLTAGE_DATA	(0x2800 | AS3722_SD0VOLTAGE_REG)
>> diff --git a/configs/jetson-tk1_defconfig b/configs/jetson-tk1_defconfig
>> index 9ce97c9f61b2..6926257d894b 100644
>> --- a/configs/jetson-tk1_defconfig
>> +++ b/configs/jetson-tk1_defconfig
>> @@ -1,4 +1,3 @@
>>   CONFIG_SPL=y
>> -CONFIG_SYS_EXTRA_OPTIONS="BOARD_JETSON_TK1="
>>   +S:CONFIG_ARM=y
>>   +S:CONFIG_TARGET_JETSON_TK1=y
>> --
>> 1.9.1
>
>
>
> Or you may rename CONFIG_TARGET_JETSON_TK1  to CONFIG_BOARD_JETSON_TK1
> or another name.
>
> I don't want to force CONFIG_TARGET_  name convention.
> You can change config names if you like. It is up to you.

I assume you don't object to the current patch, but were just pointing 
out that I could rename the variable if I wanted?

All the configs/${board}_defconfig I looked at use 
CONFIG_TARGET_${board}, so I think it's best if I just convert the code 
to using that so everything is consistent.

...
> When I see Tegra family, each board has its own config header.
> CONFIG_BOARD_ as well as CONFIG_TARGET_ will work.

AFAIK, only Jetson TK1 used a custom CONFIG_BOARD_xxx variable. I only 
did that because it shares some code with Venice2 and needed to define 
some variable do distinguish the two boards, and there wasn't already a 
standard variable for this in the build process. I'm quite happy to 
convert to the standard (or even de-facto) standard variable we have now.
Masahiro Yamada Aug. 4, 2014, 10:36 a.m. UTC | #3
Hi Stephen,



On Fri, 01 Aug 2014 12:36:22 -0600
Stephen Warren <swarren@wwwdotorg.org> wrote:

> >
> >
> > Or you may rename CONFIG_TARGET_JETSON_TK1  to CONFIG_BOARD_JETSON_TK1
> > or another name.
> >
> > I don't want to force CONFIG_TARGET_  name convention.
> > You can change config names if you like. It is up to you.
> 
> I assume you don't object to the current patch, but were just pointing out that I could rename the variable if I wanted?

I do not object to this patch at all.
Yes, I just pointed out it just in case.



> All the configs/${board}_defconfig I looked at use CONFIG_TARGET_${board}, so I think it's best if I just convert the code to using that so everything is consistent.
> 
> ...
> > When I see Tegra family, each board has its own config header.
> > CONFIG_BOARD_ as well as CONFIG_TARGET_ will work.
> 
> AFAIK, only Jetson TK1 used a custom CONFIG_BOARD_xxx variable. I only did that because it shares some code with Venice2 and needed to define some variable do distinguish the two boards, and there wasn't already a standard variable for this in the build process. I'm quite happy to convert to the standard (or even de-facto) standard variable we have now.



This might be rahter an open question, but I was just wondering which prefix is nice for choosing a board.

- CONFIG_BOARD_*

   This seems clear prefix for board select. But we will have the name space conflict with
    exsting macros such as
      CONFIG_BOARD_LATE_INIT, CONFIG_BOARD_EARLY_INIT_F, etc.

- CONFIG_TARGET_*

   The current implementation is using this.


- CONFIG_MACH_*

   ARM Linux adopts this name rule and I can see various headers are using it.


Best Regards
Masahiro Yamada
Stephen Warren Aug. 4, 2014, 4:43 p.m. UTC | #4
On 08/04/2014 04:36 AM, Masahiro Yamada wrote:
> Hi Stephen,
>
>
>
> On Fri, 01 Aug 2014 12:36:22 -0600
> Stephen Warren <swarren@wwwdotorg.org> wrote:
>
>>>
>>>
>>> Or you may rename CONFIG_TARGET_JETSON_TK1  to CONFIG_BOARD_JETSON_TK1
>>> or another name.
>>>
>>> I don't want to force CONFIG_TARGET_  name convention.
>>> You can change config names if you like. It is up to you.
>>
>> I assume you don't object to the current patch, but were just pointing out that I could rename the variable if I wanted?
>
> I do not object to this patch at all.
> Yes, I just pointed out it just in case.

Great. For now, I think I'll stick with the patch I sent.

>> All the configs/${board}_defconfig I looked at use CONFIG_TARGET_${board}, so I think it's best if I just convert the code to using that so everything is consistent.
>>
>> ...
>>> When I see Tegra family, each board has its own config header.
>>> CONFIG_BOARD_ as well as CONFIG_TARGET_ will work.
>>
>> AFAIK, only Jetson TK1 used a custom CONFIG_BOARD_xxx variable. I only did that because it shares some code with Venice2 and needed to define some variable do distinguish the two boards, and there wasn't already a standard variable for this in the build process. I'm quite happy to convert to the standard (or even de-facto) standard variable we have now.
>
>
>
> This might be rahter an open question, but I was just wondering which prefix is nice for choosing a board.
>
> - CONFIG_BOARD_*
>
>     This seems clear prefix for board select. But we will have the name space conflict with
>      exsting macros such as
>        CONFIG_BOARD_LATE_INIT, CONFIG_BOARD_EARLY_INIT_F, etc.
>
> - CONFIG_TARGET_*
>
>     The current implementation is using this.

This option seems fine to me. It's pretty obvious what it means, and as 
you noted avoids the possibility of conflicting with various 
CONFIG_BOARD_* already in use in U-Boot.

>
>
> - CONFIG_MACH_*
>
>     ARM Linux adopts this name rule and I can see various headers are using it.
>
>
> Best Regards
> Masahiro Yamada
>
diff mbox

Patch

diff --git a/board/nvidia/venice2/as3722_init.h b/board/nvidia/venice2/as3722_init.h
index a7b24039f6aa..06c366e0d0d8 100644
--- a/board/nvidia/venice2/as3722_init.h
+++ b/board/nvidia/venice2/as3722_init.h
@@ -18,7 +18,7 @@ 
 #define AS3722_LDO6VOLTAGE_REG	0x16	/* VDD_SDMMC */
 #define AS3722_LDCONTROL_REG	0x4E
 
-#ifdef CONFIG_BOARD_JETSON_TK1
+#ifdef CONFIG_TARGET_JETSON_TK1
 #define AS3722_SD0VOLTAGE_DATA	(0x3C00 | AS3722_SD0VOLTAGE_REG)
 #else
 #define AS3722_SD0VOLTAGE_DATA	(0x2800 | AS3722_SD0VOLTAGE_REG)
diff --git a/configs/jetson-tk1_defconfig b/configs/jetson-tk1_defconfig
index 9ce97c9f61b2..6926257d894b 100644
--- a/configs/jetson-tk1_defconfig
+++ b/configs/jetson-tk1_defconfig
@@ -1,4 +1,3 @@ 
 CONFIG_SPL=y
-CONFIG_SYS_EXTRA_OPTIONS="BOARD_JETSON_TK1="
 +S:CONFIG_ARM=y
 +S:CONFIG_TARGET_JETSON_TK1=y