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

[Simon Glass]

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.

Regards,
Simon