Message ID | 1406849378-32034-1-git-send-email-swarren@wwwdotorg.org |
---|---|
State | Accepted |
Delegated to: | Tom Warren |
Headers | show |
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
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.
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
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 --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