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

Masahiro Yamada yamada.masahiro at socionext.com
Thu Mar 21 04:28:20 UTC 2019


Hi Simon,

Sorry, I completely missed this thread.


On Wed, Mar 20, 2019 at 7:40 PM Simon Goldschmidt
<simon.k.r.goldschmidt at gmail.com> wrote:
>
> Hi Mrek,
>
> Am 15.03.2019 um 21:34 schrieb 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.


This is correct in terms of Kconfig grammar.
You can define the same symbol multiple times.


For example,


if ARCH_SOCFPGA

config FOO
        default 100

endif

config FOO
        int "foo"
        default 10



... is equivalent to


config FOO
        default 100 if ARCH_SOCFPGA
        int "foo"
        default 10



You can exploit this trick.

However, I see one problem
when we sync defconfig files globally
with tools/moveconfig.py


savedefconfig sorts CONFIG options in the order
they are parsed.

Every time people add/remove entry to/from
their arch/arm/mach-*/Kconfig or board/*/Kconfig,
the location where the symbol first appears changes.


That's is why symbols go up and down for every resync.
For example, see commit fca0128da4803




> >>>>
> >>>> 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,

I agree.  We are screwed up with:

config SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR
        hex "Address on the MMC to load U-Boot from"
        depends on SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR
        default 0x50 if ARCH_SUNXI
        default 0x75 if ARCH_DAVINCI
        default 0x8a if ARCH_MX6 || ARCH_MX7
        default 0x100 if ARCH_UNIPHIER
        default 0x140 if ARCH_MVEBU
        default 0x200 if ARCH_SOCFPGA || ARCH_AT91
        default 0x300 if ARCH_ZYNQ || ARCH_KEYSTONE || OMAP34XX || OMAP44XX || \
                         OMAP54XX || AM33XX || AM43XX || ARCH_K3
        default 0x4000 if ARCH_ROCKCHIP





When I introduced Kconfig to U-Boot,
I did not mean to move CONFIG options verbatim
from include/config/<board>.h to Kconfig files.

I expected old clutter things would be replaced
with something cool. But, it did not happen.
We are just moving CONFIG options to Kconfig as they are.

In old days, people added CONFIG_ prefix to anything,
most of which are not configuration at all.

For example, CONFIG_BOOTARGS in configs/axm_defconfig
is an extremely crazy case.



Probably, you want to see a solution instead of
my complaint.

  Let's use barebox   (Bummer!)


But, hold on.

Barebox had already solved problems U-Boot is suffering from.

If you are annoyed by duplicated defconfigs,
Barebox supports multiple boards mechanism.
So just a single 'socfpga_defconfig' generates multiple board images.

The image is still small by exploiting dead code elimination.

$ make ARCH=arm socfpga_defconfig
$ make ARCH=arm CROSS_COMPILE=arm-linux-gnueabi-

will produce

  SHIPPED images/barebox-socfpga-de0_nano_soc.img
  SHIPPED images/barebox-socfpga-sockit.img
  SHIPPED images/barebox-socfpga-socrates.img
images built:
barebox-socfpga-de0_nano_soc.img
barebox-socfpga-sockit.img
barebox-socfpga-socrates.img


I do not think we should change U-Boot in the same way, though.
Probably, it is too late for U-Boot, and people who like cleaner
code base can switch to barebox.



Anyway.
Let's go back to U-Boot topic.

For the duplicated defconfig issue,
recently we discussed here:

http://patchwork.ozlabs.org/patch/1043986/


I do not know super-cool solution for U-Boot.

At the cost of increased memory footprint,
some defconfig could be merged, if Marek likes it.

(U-Boot needs to link boot code for all boards
into a single SPL. That's the difference from Barebox.)



Thanks.
Masahiro Yamada



> 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?
>
> Gentle ping?
>
> >
> > In the next version (v3), Patch 2/5 should be fixed for a10.
>
> In v3, I'd remove patch 2/5 since it is already covered by a different
> series that properly cleans up using full malloc only in SPL [2] and
> I'll make this series depend on v3 of that:
>
> [2] https://patchwork.ozlabs.org/project/uboot/list/?series=97430
>
> Regards,
> Simon
>
> > 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
> >
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot



-- 
Best Regards
Masahiro Yamada


More information about the U-Boot mailing list