On 17/04/2020 13:11, Tom Rini wrote: > On Fri, Apr 17, 2020 at 08:31:38PM +0900, Masahiro Yamada wrote: >> On Fri, Apr 17, 2020 at 6:07 PM André Przywara <andre.przyw...@arm.com> >> wrote: >>> >>> On 17/04/2020 04:05, Tom Rini wrote: >>> >>> (adding Masahiro for Kconfig) >>> >>>> On Mon, Apr 06, 2020 at 05:58:19PM +0530, Amit Singh Tomar wrote: >>>> >>>>> This adds Cubieboard7[1] support based on Action Semi's S700 SoC[2], It's >>>>> Quad-core ARMv8 SoC >>>>> with Cortex-A53 cores. Peripheral like UART seems to be compatible with >>>>> S900 SoC(basic support >>>>> for it is alreay present in u-boot). >>>>> >>>>> This series(v10) takes care the commments provided by Mani and patches >>>>> 04/12, 07/12 and 12/12 >>>>> has been changed to address those comments. >>>>> >>>>> Previous series(v9) fixes a Bug that breaks bubblegum96 board >>>>> boot(reported by Mani). It was >>>>> due to fact that driver data read is not proper in the clock driver. >>>>> There are changes in >>>>> patch 06/12 to fix it. >>>>> >>>>> Series(v8) removes the SoC specific include instead just uses owl-common. >>>>> For this >>>>> patch 01/12 and 09/12 changes a bit. >>>>> >>>>> Series(v7) fixes a serious Bug that breaks S900, it was there since >>>>> v5.Thanks to Andre >>>>> for pointing it out. >>>>> >>>>> Series(v6)[3] does following changes: >>>>> >>>>> * [PATCH v5 06/11] becomes [PATCH v6 03/11] >>>>> * [PATCH v5 03/11] becomes [PATCH v6 04/11] >>>>> * Introduce a new patch to move defconfig options to Kconfig which is >>>>> [PATCH v6 10/12] >>>>> >>>>> Series(v5)[4] just re-orders the patches so that U-BOOT(with >>>>> bubblegum96_defconfig) builds >>>>> after every patch of the series(suggested by Andre). >>>>> >>>>> S700 support is tested[5] on Cubieboard7-lite board and S900 support is >>>>> just compiled tested. >>>>> >>>>> This patch series can be tested using below tree: >>>>> https://github.com/Atomar25/u-boot/commits/s700_v10 >>>>> >>>>> [1]: http://www.cubietech.com/product-detail/cubieboard7/ >>>>> [2]: http://www.actions-semi.com/en/productview.aspx?id=225 >>>>> [3]: >>>>> http://u-boot.10912.n7.nabble.com/PATCH-v6-00-12-Actions-S700-SoC-support-td403562.html#a403567 >>>>> [4]: >>>>> http://u-boot.10912.n7.nabble.com/PATCH-v5-00-11-Actions-S700-SoC-support-td402752.html#a402762 >>>>> [5]: https://paste.ubuntu.com/p/TbBtk5dPGS/ >>>>> >>>>> Amit Singh Tomar (12): >>>>> arm: actions: Add common framework for Actions Owl Semi SoCs >>>>> arm: actions: rename sysmap-s900 to sysmap-owl >>>>> serial: actions: add compatible string >>>>> arm: dts: sync dts for Action Semi S900 >>>>> arm: dts: actions: s900: add u-boot specific dtsi file >>>>> clk: actions: Add common clock driver >>>>> arm: actions: add S700 SoC device tree >>>>> arm: dts: actions: s700: add u-boot specific dtsi file >>>>> arm: add support Actions Semi S700 >>>>> actions: Move defconfig options to Kconfig >>>>> arm: add Cubieboard7 board support >>>>> doc: boards: add Cubieboard7 documentation >>>> >>>> A few problems. First, "actions: Move defconfig options to Kconfig" >>>> breaks a large number of boards including p2371-2180 in one way and >>>> libretech_all_h5_cc_h5 (along with lots of other sunxi platforms) in a >>>> different but related way. >>> >>> (Masahiro: it's about this patch: >>> https://lists.denx.de/pipermail/u-boot/2020-April/405672.html) >>> >>> Tom, many thanks for the heads up, I can confirm the problem, but am >>> totally clueless as of *why* this happens: >>> The changes in this patch add options to arch/arm/mach-owl/Kconfig, and >>> are totally contained inside an "if ARCH_OWL .. endif" clamp, so how >>> could this even affect other platforms (which are clearly not defining >>> ARCH_OWL)? >>> >> >> >> scripts/diffconfig in Linux is useful to >> see how the resulted .config has changed. >> >> This is the before/after diff of p2371-2180. >> >> >> -BOOTCOMMAND "run distro_bootcmd" >> -BOOTP_PXE y >> -BOOTP_PXE_CLIENTARCH 0x16 >> -CMD_EXT4_WRITE y >> -EXT4_WRITE y >> -FAT_WRITE y >> -FS_FAT_MAX_CLUSTSIZE 65536 >> -MENU y >> CMD_DHCP y -> n >> CMD_EXT2 y -> n >> CMD_EXT4 y -> n >> CMD_FAT y -> n >> CMD_FS_GENERIC y -> n >> CMD_MII y -> n >> CMD_PART y -> n >> CMD_PING y -> n >> CMD_PXE y -> n >> CMD_SYSBOOT y -> n >> DISTRO_DEFAULTS y -> n >> DOS_PARTITION y -> n >> ENV_VARS_UBOOT_CONFIG y -> n >> FS_EXT4 y -> n >> FS_FAT y -> n >> HUSH_PARSER y -> n >> SUPPORT_RAW_INITRD y -> n >> USB_STORAGE y -> n >> USE_BOOTCOMMAND y -> n >> >> >> >> It turned off DISTRO_DEFAULTS. >> >> The menuconfig help shows >> it now depends on 'ARM && ARCH_OWL'. >> >> Presumably Kconfig was confused >> by DISTRO_DEFAULTS being defined >> multiple times.
It is, but only for ARCH_OWL, where it actually works as expected. I don't get how the additional listing of just DISTRO_DEFAULTS (without a type!) *guarded by if ARCH_OWL* would affect other platforms. Besides, we do this all over the place for stuff like IDENT_STRING, SYS_CLK_FREQ, and, most prominently SYS_CONFIG_NAME, SYS_SOC and SYS_BOARD. And there it works fine. So what is the difference here? > Ah, right. And they shouldn't be defined twice, it should be imply'd > under ARCH_OWL (and the rest should be in the defconfigs, no re-listed > in arch/arm/mach-owl/Kconfig). Thanks! So yes, the fix is relatively easy (Amit actually had it this way before). It's just that I actually recommended this approach here to Amit, to avoid putting platform specific defaults into generic Kconfig files (like we do for sunxi at the moment). Conceptually I find it cleaner to gather platform specific defaults in the platform Kconfig instead of spreading this out all over the tree. Cheers, Andre.