[U-Boot] [PATCH v2 4/5] arm: socfpga: imply/default common config options

[Simon Goldschmidt]

Am 05.03.2019 um 21:58 schrieb Marek Vasut:
> On 3/5/19 9:28 PM, Simon Goldschmidt wrote:
>> Am 04.03.2019 um 22:34 schrieb Marek Vasut:
>>> On 3/4/19 10:23 PM, Simon Goldschmidt wrote:
>>>>
>>>>
>>>> Marek Vasut <marex at denx.de <mailto:marex at denx.de>> schrieb am Mo., 4.
>>>> März 2019, 22:19:
>>>>
>>>>       On 3/4/19 9:53 PM, Simon Goldschmidt wrote:
>>>>       > This commit moves common config options used in all socfpga
>>>> boards
>>>>       > to select/imply in Kconfig. This both cleans up the defconfig
>>>> files
>>>>       > as well as makes future changes easier.
>>>>       >
>>>>       > Options implied/defaulted for all sub-arches:
>>>>       > - SPL, SPL_DM, USE_TINY_PRINTF, NR_DRAM_BANKS
>>>>       >
>>>>       > Options implied/defaulted for implied for A10 & gen5:
>>>>       > - FPGA_SOCFPGA, SYS_MALLOC_F_LEN, SYS_TEXT_BASE
>>>>       >
>>>>       > Options implied/defaulted for A10:
>>>>       > - SPL_SYS_MALLOC_F_LEN
>>>>       >
>>>>       > Options implied/defaulted for gen5:
>>>>       > - SPL_STACK_R, SPL_SYS_MALLOC_SIMPLE, SPL_STACK_R_ADDR
>>>>       >
>>>>       > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com
>>>>       <mailto:simon.k.r.goldschmidt at gmail.com>>
>>>>       > ---
>>>>       >
>>>>       > Changes in v2:
>>>>       > - added patch to imply/default common config options
>>>>       >
>>>>       >  arch/arm/Kconfig                       |  3 +++
>>>>       >  arch/arm/mach-socfpga/Kconfig          | 21
>>>> +++++++++++++++++++++
>>>>       >  configs/socfpga_arria10_defconfig      |  8 --------
>>>>       >  configs/socfpga_arria5_defconfig       | 10 ----------
>>>>       >  configs/socfpga_cyclone5_defconfig     | 10 ----------
>>>>       >  configs/socfpga_dbm_soc1_defconfig     | 10 ----------
>>>>       >  configs/socfpga_de0_nano_soc_defconfig | 10 ----------
>>>>       >  configs/socfpga_de10_nano_defconfig    | 10 ----------
>>>>       >  configs/socfpga_de1_soc_defconfig      | 10 ----------
>>>>       >  configs/socfpga_is1_defconfig          |  8 --------
>>>>       >  configs/socfpga_sockit_defconfig       | 10 ----------
>>>>       >  configs/socfpga_socrates_defconfig     | 10 ----------
>>>>       >  configs/socfpga_sr1500_defconfig       | 10 ----------
>>>>       >  configs/socfpga_stratix10_defconfig    |  4 ----
>>>>       >  configs/socfpga_vining_fpga_defconfig  | 10 ----------
>>>>       >  15 files changed, 24 insertions(+), 120 deletions(-)
>>>>       >
>>>>       > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>>>>       > index ded7c11a4c..71bb14acce 100644
>>>>       > --- a/arch/arm/Kconfig
>>>>       > +++ b/arch/arm/Kconfig
>>>>       > @@ -839,12 +839,15 @@ config ARCH_SOCFPGA
>>>>       >       imply DM_SPI
>>>>       >       imply DM_SPI_FLASH
>>>>       >       imply FAT_WRITE
>>>>       > +     imply SPL
>>>>       > +     imply SPL_DM
>>>>       >       imply SPL_LIBDISK_SUPPORT
>>>>       >       imply SPL_MMC_SUPPORT
>>>>       >       imply SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION
>>>>       >       imply SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION_TYPE
>>>>       >       imply SPL_SPI_FLASH_SUPPORT
>>>>       >       imply SPL_SPI_SUPPORT
>>>>       > +     imply USE_TINY_PRINTF
>>>>       >
>>>>       >  config ARCH_SUNXI
>>>>       >       bool "Support sunxi (Allwinner) SoCs"
>>>>       > diff --git a/arch/arm/mach-socfpga/Kconfig
>>>>       b/arch/arm/mach-socfpga/Kconfig
>>>>       > index 5e87371f8c..da801eb660 100644
>>>>       > --- a/arch/arm/mach-socfpga/Kconfig
>>>>       > +++ b/arch/arm/mach-socfpga/Kconfig
>>>>       > @@ -1,8 +1,25 @@
>>>>       >  if ARCH_SOCFPGA
>>>>       >
>>>>       > +config NR_DRAM_BANKS
>>>>       > +     default 1
>>>>       > +
>>>>       > +config SPL_STACK_R_ADDR
>>>>       > +     default 0x00800000 if TARGET_SOCFPGA_GEN5
>>>>       > +
>>>>       > +config SPL_SYS_MALLOC_F_LEN
>>>>       > +     default 0x10000 if TARGET_SOCFPGA_ARRIA10
>>>>       > +
>>>>
>>>>       This is already defined in /Kconfig, won't you end up with
>>>>       duplicate/redefined symbols this way ?
>>>>
>>>>
>>>> I did not get errors. I copied the handling
>>>> from SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION_TYPE in the same file. Plus it
>>>> seems like it's done like this in other Kconfig files, too.
>>>
>>> I know it is done. I am not convinced it's right.
>>
>> Would it be better acceptable if I moved the defaults to the Kconfig
>> files where these settings are defined? I always thought it to be
>> strange to clutter those with arch-specific settings, but I don't really
>> have a strong preference...
> 
> I agree with you on this. I'd really like some reply from Yamada-san.

So without a reply from Yamada-san for 10 days, how do we continue here?

In the next version (v3), Patch 2/5 should be fixed for a10.
I'd also extend that next version with further cleanup patches from an 
earlier series [1] since I have I2C EEPROM finally running with DM 
(tested on the socfpga_socrates board by using the "Phy1" daughter board 
and a matching FPGA configuration).

[1] https://patchwork.ozlabs.org/project/uboot/list/?series=85229

Regards,
Simon