[U-Boot] [PATCH 05/30] Convert CONFIG_ENV_IS_IN_FLASH to Kconfig

[Tom Rini]

On Tue, Jul 18, 2017 at 05:01:39PM +0200, Daniel Schwierzeck wrote:
> 2017-07-18 16:00 GMT+02:00 Simon Glass <sjg at chromium.org>:
> > Hi Daniel,
> >
> > On 10 July 2017 at 15:09, Daniel Schwierzeck
> > <daniel.schwierzeck at gmail.com> wrote:
> >>
> >>
> >> Am 09.07.2017 um 22:52 schrieb Simon Glass:
> >>> This converts the following to Kconfig:
> >>>    CONFIG_ENV_IS_IN_FLASH
> >>>
> >>> Signed-off-by: Simon Glass <sjg at chromium.org>
> >>> ---
> >>>
> >>
> >> ...
> >>
> >>> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> >>> index 579371e8ee..3046d90b4d 100644
> >>> --- a/arch/mips/Kconfig
> >>> +++ b/arch/mips/Kconfig
> >>> @@ -21,6 +21,7 @@ config TARGET_QEMU_MIPS
> >>>       select SUPPORTS_CPU_MIPS64_R1
> >>>       select SUPPORTS_CPU_MIPS64_R2
> >>>       select ROM_EXCEPTION_VECTORS
> >>> +     imply ENV_IS_IN_FLASH
> >>>
> >>>  config TARGET_MALTA
> >>>       bool "Support malta"
> >>> @@ -42,6 +43,7 @@ config TARGET_MALTA
> >>>       select SWAP_IO_SPACE
> >>>       select MIPS_L1_CACHE_SHIFT_6
> >>>       select ROM_EXCEPTION_VECTORS
> >>> +     imply ENV_IS_IN_FLASH
> >>>
> >>>  config TARGET_VCT
> >>>       bool "Support vct"
> >>> @@ -108,6 +110,7 @@ config TARGET_BOSTON
> >>>       select SUPPORTS_CPU_MIPS64_R2
> >>>       select SUPPORTS_CPU_MIPS64_R6
> >>>       select ROM_EXCEPTION_VECTORS
> >>> +     imply ENV_IS_IN_FLASH
> >>>
> >>>  config TARGET_XILFPGA
> >>>       bool "Support Imagination Xilfpga"
> >>> @@ -197,6 +200,7 @@ config CPU_MIPS64_R2
> >>>       bool "MIPS64 Release 2"
> >>>       depends on SUPPORTS_CPU_MIPS64_R2
> >>>       select 64BIT
> >>> +     imply ENV_IS_IN_FLASH
> >>>       help
> >>>         Choose this option to build a kernel for release 2 through 5 of the
> >>>         MIPS64 architecture.
> >>> @@ -297,6 +301,7 @@ config CPU_MIPS32
> >>>  config CPU_MIPS64
> >>>       bool
> >>>       default y if CPU_MIPS64_R1 || CPU_MIPS64_R2 || CPU_MIPS64_R6
> >>> +     imply ENV_IS_IN_FLASH
> >>
> >> this is wrong as CONFIG_CPU_MIPS64 is a generic MIPS symbol and not a
> >> specific for a machine or board
> >
> > Well the intent is to reduce the number of changes to individual
> > boards / defconfig files. So in fact I want to use this more generic
> > option.
> 
> yes, but adding "imply ENV_IS_IN_FLASH" to the Kconfig symbols
> CPU_MIPS64 and MIPS_CM is wrong due to the logical intent of these
> symbols. If you want to add the imply for all MIPS boards, then the
> correct place would be the MIPS symbol in arch/Kconfig. But enabling
> ENV_IS_IN_FLASH for all MIPS boards would be wrong too. Thus the
> "imply ENV_IS_IN_FLASH" only makes sense with the TARGET_* symbols
> because these describe default board configs.
> 
> >
> >>
> >>>
> >>>  config MIPS_TUNE_4KC
> >>>       bool
> >>> @@ -401,6 +406,7 @@ config DYNAMIC_IO_PORT_BASE
> >>>
> >>>  config MIPS_CM
> >>>       bool
> >>> +     imply ENV_IS_IN_FLASH
> >>
> >> dito for CONFIG_MIPS_CM
> >>
> >>>       help
> >>>         Select this if your system contains a MIPS Coherence Manager and you
> >>>         wish U-Boot to configure it or make use of it to retrieve system
> >>
> >> ...
> >>
> >>> diff --git a/configs/vct_platinum_defconfig b/configs/vct_platinum_defconfig
> >>> index f8b9d7e61b..0e4fcbaa26 100644
> >>> --- a/configs/vct_platinum_defconfig
> >>> +++ b/configs/vct_platinum_defconfig
> >>> @@ -12,6 +12,7 @@ CONFIG_CMD_DHCP=y
> >>>  CONFIG_CMD_PING=y
> >>>  CONFIG_CMD_SNTP=y
> >>>  CONFIG_CMD_FAT=y
> >>> +CONFIG_ENV_IS_IN_FLASH=y
> >>
> >> for consistency with the other MIPS boards in this patch, this should be
> >> added as "imply CONFIG_ENV_IS_IN_FLASH" in arch/mips/Kconfig under
> >> "config TARGET_VCT"
> >
> > If there are only 1-2 boards affected then I think it makes more sense
> > to put the change in the defconfig file. Adjusting Kconfig for
> > individual boards just seems odd...?
> >
> 
> but you already touched all TARGET_* symbols in arch/mips/Kconfig
> except TARGET_VCT. Instead you patched all defconfig files for the 12
> VCT related boards which is contrary to the intention of your patch.
> 
> So please only update the TARGET_* options either in arch/mips/Kconfig
> or arch/mips/mach-*/Kconfig, thanks.

To echo this, the moveconfig.py imply logic is good, but we then need to
see if it follows the human logic side.  ie environment is a board
choice, not SoC/etc, so we need to shuffle it around to the right area.
Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170718/97290ff2/attachment.sig>