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

[Daniel Schwierzeck]

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.

-- 
- Daniel