On 17/04/2020 13:44, Tom Rini wrote: > On Fri, Apr 17, 2020 at 01:34:36PM +0100, André Przywara wrote: >> 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? > > It doesn't quite work fine, and DISTRO_DEFAULTS is the case where it > shows up badly (which is why everyone else does imply/select). > >>> 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. > > The problem is that Kconfig doesn't work that way. All of the SYS_foo > stuff we have in board/SoC Kconfig files has some unfortunate > side-effects that Yamada-san has noted elsewhere.
Ah, OK, thanks for the heads up. I just found those examples, but didn't know that they are actually considered somewhat broken. > The long list of > "default ... if ...." aren't ideal, but my hope is that by consolidating > who wants/needs what values we can first come up with better defaults > and then perhaps come up with a better tool for everyone (how do you > manage kernel defconfigs is its own problem). Fair enough, if those "default ... if" sequences are the way to go, then so be it. I was just hoping there would be a cleaner way of expressing: "this *platform* relies on/always sets this option". Unfortunately both select and imply seem to come with their own set of issues, at least for certain kind of options. Cheers, Andre
