[U-Boot] [PATCH 08/12] ARM: socfpga: use "select" instead of default value in defconfig

[Simon Glass]

On 7 April 2015 at 20:46, Simon Glass <sjg at chromium.org> wrote:
> Hi Masahiro,
>
> On 5 April 2015 at 21:22, Masahiro Yamada <yamada.masahiro at socionext.com> wrote:
>> Hi Pavel,
>>
>>
>> 2015-04-02 21:47 GMT+09:00 Pavel Machek <pavel at denx.de>:
>>> Hi!
>>>
>>>> >> Signed-off-by: Masahiro Yamada <yamada.masahiro at socionext.com>
>>>> >> ---
>>>> >>
>>>> >>  arch/arm/Kconfig                   | 6 ++++++
>>>> >>  configs/socfpga_arria5_defconfig   | 3 ---
>>>> >>  configs/socfpga_cyclone5_defconfig | 3 ---
>>>> >>  configs/socfpga_socrates_defconfig | 3 ---
>>>> >>  4 files changed, 6 insertions(+), 9 deletions(-)
>>>> >>
>>>> >> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>>>> >> index b25a4a9..6ae48c6 100644
>>>> >> --- a/arch/arm/Kconfig
>>>> >> +++ b/arch/arm/Kconfig
>>>> >> @@ -618,11 +618,17 @@ config TARGET_SOCFPGA_ARRIA5
>>>> >>       bool "Support socfpga_arria5"
>>>> >>       select CPU_V7
>>>> >>       select SUPPORT_SPL
>>>> >> +     select DM
>>>> >> +     select DM_SPI_FLASH
>>>> >> +     select DM_SPI
>>>
>>>> > Are you sure?
>>>>
>>>> Yes, I'm sure.
>>>>
>>>> > config DM_SPI_FLASH
>>>> >         bool "Enable Driver Model for SPI flash"
>>>> >         depends on DM && SPI
>>>> >                 help
>>>> >
>>>> > DM_SPI_FLASH depends on SPI, you can't just select it...
>>>> >
>>>>
>>>>
>>>> This dependency is wrong.
>>>> Before this patch, it must be fixed.
>>>>
>>>> Please see 01/12 of this series:
>>>> http://patchwork.ozlabs.org/patch/456458/
>>>
>>> But I'm not sure.
>>>
>>> With this, you made CONFIG_DM_SPI_FLASH mandatory... even for users
>>> that have no SPI flash. There must be better solution...?
>>
>> In my opinion,
>>
>> CONFIG_DM_* should be user-unconfigurable.
>>
>> CONFIG_DM_* should not enable/disable a feature,
>> but should define how it is implemented.
>>
>> For example,
>>
>>
>> !defined(CONFIG_SPI)   --> SPI is disabled
>>
>> defined(CONFIG_SPI) && defined(CONFIG_DM_SPI)  --> Driver Model SPI is enabled
>>
>> defined(CONFIG_SPI) && !defined(CONFIG_DM_SPI) --> ad-hoc SPI is enabled
>>
>>
>> Likewise, I think CONFIG_SPI_FLASH should decide if the system has SPI flash.
>
> This sounds right to me. In fact it is important, since we want to
> remove the CONFIG_DM options eventually. There are only there as a
> migration aid.

Acked-by: Simon Glass <sjg at chromium.org>