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

Masahiro Yamada
2015-04-06T03:22:16Z

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.

--
Best Regards
Masahiro Yamada