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. 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). -- Tom
signature.asc
Description: PGP signature