| Message ID | 20170501174118.15817-1-afaerber@suse.de |
|---|---|
| State | Changes Requested |
| Delegated to: | Simon Glass |
| Headers | show |
Am 01.05.2017 um 19:41 schrieb Andreas Färber: > diff --git a/configs/geekbox_defconfig b/configs/geekbox_defconfig > index f5783100c0..5e4d5f03a4 100644 > --- a/configs/geekbox_defconfig > +++ b/configs/geekbox_defconfig > @@ -4,6 +4,7 @@ CONFIG_ROCKCHIP_RK3368=y > CONFIG_TARGET_GEEKBOX=y > CONFIG_DEFAULT_DEVICE_TREE="rk3368-geekbox" > CONFIG_HUSH_PARSER=y > +CONFIG_DEFAULT_FDT_FILE="rockchip/rk3368-geekbox.dtb" The PX5 and Sheep boards would need the equivalent config setting or an empty #define FDTFILE, otherwise they'd end up with fdtfile=\0. Regards, Andreas > # CONFIG_DISPLAY_CPUINFO is not set > # CONFIG_CMD_IMLS is not set > CONFIG_REGMAP=y
Hi Andreas, On 1 May 2017 at 11:48, Andreas Färber <afaerber@suse.de> wrote: > Am 01.05.2017 um 19:41 schrieb Andreas Färber: >> diff --git a/configs/geekbox_defconfig b/configs/geekbox_defconfig >> index f5783100c0..5e4d5f03a4 100644 >> --- a/configs/geekbox_defconfig >> +++ b/configs/geekbox_defconfig >> @@ -4,6 +4,7 @@ CONFIG_ROCKCHIP_RK3368=y >> CONFIG_TARGET_GEEKBOX=y >> CONFIG_DEFAULT_DEVICE_TREE="rk3368-geekbox" >> CONFIG_HUSH_PARSER=y >> +CONFIG_DEFAULT_FDT_FILE="rockchip/rk3368-geekbox.dtb" > > The PX5 and Sheep boards would need the equivalent config setting or an > empty #define FDTFILE, otherwise they'd end up with fdtfile=\0. Can you fix that by making it empty automatically in that case? E.g.: #ifndef FDTFILE #define FDTFILE "fdtfile=" CONFIG_DEFAULT_FDT_FILE "\0" #else #define FDTFILE #endif > > Regards, > Andreas > >> # CONFIG_DISPLAY_CPUINFO is not set >> # CONFIG_CMD_IMLS is not set >> CONFIG_REGMAP=y Regards, Simon
Hi Simon, Am 08.05.2017 um 18:38 schrieb Simon Glass: > On 1 May 2017 at 11:48, Andreas Färber <afaerber@suse.de> wrote: >> Am 01.05.2017 um 19:41 schrieb Andreas Färber: >>> diff --git a/configs/geekbox_defconfig b/configs/geekbox_defconfig >>> index f5783100c0..5e4d5f03a4 100644 >>> --- a/configs/geekbox_defconfig >>> +++ b/configs/geekbox_defconfig >>> @@ -4,6 +4,7 @@ CONFIG_ROCKCHIP_RK3368=y >>> CONFIG_TARGET_GEEKBOX=y >>> CONFIG_DEFAULT_DEVICE_TREE="rk3368-geekbox" >>> CONFIG_HUSH_PARSER=y >>> +CONFIG_DEFAULT_FDT_FILE="rockchip/rk3368-geekbox.dtb" >> >> The PX5 and Sheep boards would need the equivalent config setting or an >> empty #define FDTFILE, otherwise they'd end up with fdtfile=\0. > > Can you fix that by making it empty automatically in that case? E.g.: > > #ifndef FDTFILE > #define FDTFILE "fdtfile=" CONFIG_DEFAULT_FDT_FILE "\0" > #else > #define FDTFILE > #endif That does not work. For one, it will cause a redefinition warning in the #else clause, for another it no longer allows to override FDTFILE. Unfortunately #ifdef CONFIG_DEFAULT_FDT_FILE does not work for an empty string. Do you see any reason not to set fdtfile for some boards? Regards, Andreas
Hi Andreas, On 9 May 2017 at 02:05, Andreas Färber <afaerber@suse.de> wrote: > Hi Simon, > > Am 08.05.2017 um 18:38 schrieb Simon Glass: >> On 1 May 2017 at 11:48, Andreas Färber <afaerber@suse.de> wrote: >>> Am 01.05.2017 um 19:41 schrieb Andreas Färber: >>>> diff --git a/configs/geekbox_defconfig b/configs/geekbox_defconfig >>>> index f5783100c0..5e4d5f03a4 100644 >>>> --- a/configs/geekbox_defconfig >>>> +++ b/configs/geekbox_defconfig >>>> @@ -4,6 +4,7 @@ CONFIG_ROCKCHIP_RK3368=y >>>> CONFIG_TARGET_GEEKBOX=y >>>> CONFIG_DEFAULT_DEVICE_TREE="rk3368-geekbox" >>>> CONFIG_HUSH_PARSER=y >>>> +CONFIG_DEFAULT_FDT_FILE="rockchip/rk3368-geekbox.dtb" >>> >>> The PX5 and Sheep boards would need the equivalent config setting or an >>> empty #define FDTFILE, otherwise they'd end up with fdtfile=\0. >> >> Can you fix that by making it empty automatically in that case? E.g.: >> >> #ifndef FDTFILE >> #define FDTFILE "fdtfile=" CONFIG_DEFAULT_FDT_FILE "\0" >> #else >> #define FDTFILE >> #endif > > That does not work. For one, it will cause a redefinition warning in the > #else clause, for another it no longer allows to override FDTFILE. > > Unfortunately #ifdef CONFIG_DEFAULT_FDT_FILE does not work for an empty > string. Yes I meant #ifdef CONFIG_DEFAULT_FDT_FILE, sorry. What does it do? I did a little test and it seemed to work for me. > > Do you see any reason not to set fdtfile for some boards? I don't think it is used on any Rockchip boards at present. The Kconfig help for the option is so brief I cannot tell what it is for. Could you please add a purpose to your commit message? It seems that we could calculate the filename rather than specifying it manually for each board? Regards, Simon
diff --git a/configs/geekbox_defconfig b/configs/geekbox_defconfig index f5783100c0..5e4d5f03a4 100644 --- a/configs/geekbox_defconfig +++ b/configs/geekbox_defconfig @@ -4,6 +4,7 @@ CONFIG_ROCKCHIP_RK3368=y CONFIG_TARGET_GEEKBOX=y CONFIG_DEFAULT_DEVICE_TREE="rk3368-geekbox" CONFIG_HUSH_PARSER=y +CONFIG_DEFAULT_FDT_FILE="rockchip/rk3368-geekbox.dtb" # CONFIG_DISPLAY_CPUINFO is not set # CONFIG_CMD_IMLS is not set CONFIG_REGMAP=y diff --git a/include/configs/rk3368_common.h b/include/configs/rk3368_common.h index d4fd54492c..0208e6f9f2 100644 --- a/include/configs/rk3368_common.h +++ b/include/configs/rk3368_common.h @@ -35,8 +35,13 @@ #include <config_distro_bootcmd.h> +#ifndef FDTFILE +#define FDTFILE "fdtfile=" CONFIG_DEFAULT_FDT_FILE "\0" +#endif + #define CONFIG_EXTRA_ENV_SETTINGS \ - BOOTENV + BOOTENV \ + FDTFILE #endif
Populate the fdtfile environment variable based on CONFIG_DEFAULT_FDT_FILE. Allow to override this default behavior via FDTFILE. Set CONFIG_DEFAULT_FDT_FILE for the GeekBox. Signed-off-by: Andreas Färber <afaerber@suse.de> --- configs/geekbox_defconfig | 1 + include/configs/rk3368_common.h | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-)