[U-Boot] [PATCH v2 4/5] arm: socfpga: imply/default common config options
Simon Goldschmidt
simon.k.r.goldschmidt at gmail.com
Thu Mar 21 12:36:55 UTC 2019
Hi,
On Thu, Mar 21, 2019 at 5:29 AM Masahiro Yamada
<yamada.masahiro at socionext.com> wrote:
>
> Hi Simon,
>
> Sorry, I completely missed this thread.
No problem, this is not a too pressing issue, yet, since it's for the
next merge window :-)
>
> 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.)
Ok, thanks for this information! I do think things could be imrpoved.
I monitored the thread you references at the start but somehow got
lost in between... However, I do not have any input right now as to
how to improve the things mentioned in that thread.
Marek, being like that, I'll continue with v3 in the same way as v2,
meaning the config items are duplicated in our mach's Kconfig.
I think this is better than cluttering central Kconfig files with our
defaults. I guess I'll append a patch that re-syncs the defconfigs
if they change at all...
Regards,
Simon
>
>
>
> 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