[PATCH v10 00/12] Actions S700 SoC support

Fri Apr 17 14:44:01 CEST 2020

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.przywara at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200417/c27b1314/attachment.sig>