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

Simon Glass
2015-04-08T02:46:55Z

Hi Masahiro,

On 5 April 2015 at 21:22, Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
> Hi Pavel,
>
>
> 2015-04-02 21:47 GMT+09:00 Pavel Machek <pavel@denx.de>:
>> Hi!
>>
>>> >> Signed-off-by: Masahiro Yamada <yamada.masahiro@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