| Message ID | 1387264612-17834-3-git-send-email-dennis@ausil.us |
|---|---|
| State | RFC |
| Delegated to: | Tom Rini |
| Headers | show |
On 12/17/2013 12:16 AM, Dennis Gilmore wrote: > Signed-off-by: Dennis Gilmore <dennis@ausil.us> Nit: A patch description might be useful; e.g. to describe that distros need to know that the bootloader enables a common set of options they can rely on, and this file is the definition of that set. > diff --git a/include/common.h b/include/common.h > +/* use generic distro config */ > +#ifdef DISTRO_DEFAULTS > +#include <config_distro_default.h> > +#endif Can we wrap that in the following also: +#ifdef DISTRO_DEFAULTS +#ifndef CONFIG_SPL_BUILD +#include <config_distro_default.h> +#endif +#endif That way, this header won't bloat up the size of Tegra's SPL, which is limited to ~16K. Or, would you expect that extra ifdef to be placed around the #define DISTRO_DEFAULTS? > diff --git a/include/config_distro_default.h b/include/config_distro_default.h Bike-shed: At least for Tegra, the headers which define common config options are in include/configs/tegra_*.h, just like the top-level board config files. Would this be better as include/configs/distro_defaults.h?
On 12/17/2013 12:16 AM, Dennis Gilmore wrote: > diff --git a/include/config_distro_default.h b/include/config_distro_default.h > +#define CONFIG_CMD_EXT2 > +#define CONFIG_CMD_EXT4 > +#define CONFIG_CMD_FAT For a generic config, I would be tempted to drop the fs-specific command sets enabled by the options above, and rely /only/ on the generic commands enabled by: > +#define CONFIG_CMD_FS_GENERIC
On 12/17/2013 12:16 AM, Dennis Gilmore wrote: > diff --git a/include/common.h b/include/common.h > #include <flash.h> > #include <image.h> > > +/* use generic distro config */ > +#ifdef DISTRO_DEFAULTS > +#include <config_distro_default.h> > +#endif There is another issue with including this header at this location: This include is pretty late in <common.h>. In particular, <part.h> is included before this point, and only prototypes some functions such as test_part_dos() if the relevant config option is enabled. I want to simplify all the Tegra config headers and remove any options that are also set by <config_distro_default.h>. However, if I do this, then e.g. CONFIG_DOS_PARTITION isn't set when <part.h> is included, and the build breaks. One could probably move the include of <config_distro_default.h> very near the top of <common.h>, i.e. just after the include of <config.h>. However, this still doesn't solve all problems, since <config.h> itself (which is an auto-generated file) includes <config_fallbacks.h>, which is where e.g. CONFIG_FS_FAT gets auto-enabled if CONFIG_CMD_FAT is enabled. This can cause the build to break since the makefiles don't know to link in the FAT fs code. I'd like to propose that instead of board config files doing: #define DISTRO_DEFAULTS They simply do: #include <distro_defaults.h> That way, there's nothing unusual about the handling of this header file; it's just some common location that sets up some common values, and the control flow of all the includes doesn't look any different to the tools that process the board config header file. Would you like me to post revised versions of your patches to show what I mean?
diff --git a/include/common.h b/include/common.h index d49c514..00969a5 100644 --- a/include/common.h +++ b/include/common.h @@ -99,6 +99,11 @@ typedef volatile unsigned char vu_char; #include <flash.h> #include <image.h> +/* use generic distro config */ +#ifdef DISTRO_DEFAULTS +#include <config_distro_default.h> +#endif + #ifdef DEBUG #define _DEBUG 1 #else diff --git a/include/config_distro_default.h b/include/config_distro_default.h new file mode 100644 index 0000000..7b13586 --- /dev/null +++ b/include/config_distro_default.h @@ -0,0 +1,55 @@ +/* + * Copyright 2013 Red Hat, Inc. + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#ifndef _CONFIG_CMD_DISTRO_DEFAULT_H +#define _CONFIG_CMD_DISTRO_DEFAULT_H + +/* + * List of all commands and options that when defined enables support for features + * required by distros to support boards in a standardised and consitant manner. + */ + +#define CONFIG_BOOTP_BOOTPATH +#define CONFIG_BOOTP_DNS +#define CONFIG_BOOTP_GATEWAY +#define CONFIG_BOOTP_HOSTNAME +#define CONFIG_BOOTP_PXE +#define CONFIG_BOOTP_SUBNETMASK + +#if defined(__arm__) +#define CONFIG_BOOTP_PXE_CLIENTARCH 0x100 +#if defined(__ARM_ARCH_7__) || defined(__ARM_ARCH_7A__) +#define CONFIG_BOOTP_VCI_STRING "U-boot.armv7" +#else +#define CONFIG_BOOTP_VCI_STRING "U-boot.arm" +#endif +#endif + +#define CONFIG_OF_LIBFDT + +#define CONFIG_CMD_BOOTZ +#define CONFIG_CMD_DHCP +#define CONFIG_CMD_ELF +#define CONFIG_CMD_EXT2 +#define CONFIG_CMD_EXT4 +#define CONFIG_CMD_FAT +#define CONFIG_CMD_FS_GENERIC +#define CONFIG_CMD_MII +#define CONFIG_CMD_NET +#define CONFIG_CMD_PING +#define CONFIG_CMD_PXE + +#define CONFIG_CMDLINE_EDITING +#define CONFIG_AUTO_COMPLETE +#define CONFIG_BOOTDELAY 2 +#define CONFIG_SYS_LONGHELP +#define CONFIG_MENU +#define CONFIG_DOS_PARTITION +#define CONFIG_EFI_PARTITION +#define CONFIG_SUPPORT_RAW_INITRD +#define CONFIG_SYS_HUSH_PARSER + +#endif /* _CONFIG_CMD_DISTRO_DEFAULT_H */
Signed-off-by: Dennis Gilmore <dennis@ausil.us> --- include/common.h | 5 ++++ include/config_distro_default.h | 55 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+) create mode 100644 include/config_distro_default.h