| Message ID | 1586176111-31003-1-git-send-email-amittomer25@gmail.com |
|---|---|
| Headers | show |
| Series | Actions S700 SoC support | expand |
Hi Tom, 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). > I've reviewed all patches and tested them on Bubblegum96 board. Since we now have time for the next merge window, can you please apply this series once it is open? Or let me know if I have to send you a pull req. Thanks, Mani > 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 > > MAINTAINERS | 3 +- > arch/arm/Kconfig | 5 +- > arch/arm/dts/Makefile | 4 +- > arch/arm/dts/s700-cubieboard7.dts | 92 +++++++ > arch/arm/dts/s700-u-boot.dtsi | 18 ++ > arch/arm/dts/s700.dtsi | 248 +++++++++++++++++++ > arch/arm/dts/s900-u-boot.dtsi | 17 ++ > arch/arm/dts/s900.dtsi | 322 +++++++++++++++++++++++-- > arch/arm/include/asm/arch-owl/clk_s900.h | 57 ----- > arch/arm/include/asm/arch-owl/regs_s700.h | 56 +++++ > arch/arm/mach-owl/Kconfig | 49 ++-- > arch/arm/mach-owl/Makefile | 3 +- > arch/arm/mach-owl/soc.c | 57 +++++ > arch/arm/mach-owl/sysmap-owl.c | 32 +++ > arch/arm/mach-owl/sysmap-s900.c | 32 --- > board/ucRobotics/bubblegum_96/Kconfig | 15 -- > board/ucRobotics/bubblegum_96/MAINTAINERS | 6 - > board/ucRobotics/bubblegum_96/Makefile | 3 - > board/ucRobotics/bubblegum_96/bubblegum_96.c | 57 ----- > configs/bubblegum_96_defconfig | 12 +- > configs/cubieboard7_defconfig | 9 + > doc/board/actions/cubieboard7.rst | 114 +++++++++ > doc/board/actions/index.rst | 10 + > doc/board/index.rst | 1 + > drivers/clk/owl/Kconfig | 8 +- > drivers/clk/owl/Makefile | 2 +- > drivers/clk/owl/clk_owl.c | 155 ++++++++++++ > drivers/clk/owl/clk_owl.h | 64 +++++ > drivers/clk/owl/clk_s900.c | 137 ----------- > drivers/serial/serial_owl.c | 2 +- > include/configs/bubblegum_96.h | 40 --- > include/configs/owl-common.h | 40 +++ > include/dt-bindings/clock/actions,s700-cmu.h | 118 +++++++++ > include/dt-bindings/clock/actions,s900-cmu.h | 129 ++++++++++ > include/dt-bindings/clock/s900_cmu.h | 77 ------ > include/dt-bindings/reset/actions,s700-reset.h | 34 +++ > include/dt-bindings/reset/actions,s900-reset.h | 65 +++++ > 37 files changed, 1607 insertions(+), 486 deletions(-) > create mode 100644 arch/arm/dts/s700-cubieboard7.dts > create mode 100644 arch/arm/dts/s700-u-boot.dtsi > create mode 100644 arch/arm/dts/s700.dtsi > create mode 100644 arch/arm/dts/s900-u-boot.dtsi > delete mode 100644 arch/arm/include/asm/arch-owl/clk_s900.h > create mode 100644 arch/arm/include/asm/arch-owl/regs_s700.h > create mode 100644 arch/arm/mach-owl/soc.c > create mode 100644 arch/arm/mach-owl/sysmap-owl.c > delete mode 100644 arch/arm/mach-owl/sysmap-s900.c > delete mode 100644 board/ucRobotics/bubblegum_96/Kconfig > delete mode 100644 board/ucRobotics/bubblegum_96/MAINTAINERS > delete mode 100644 board/ucRobotics/bubblegum_96/Makefile > delete mode 100644 board/ucRobotics/bubblegum_96/bubblegum_96.c > create mode 100644 configs/cubieboard7_defconfig > create mode 100644 doc/board/actions/cubieboard7.rst > create mode 100644 doc/board/actions/index.rst > create mode 100644 drivers/clk/owl/clk_owl.c > create mode 100644 drivers/clk/owl/clk_owl.h > delete mode 100644 drivers/clk/owl/clk_s900.c > delete mode 100644 include/configs/bubblegum_96.h > create mode 100644 include/configs/owl-common.h > create mode 100644 include/dt-bindings/clock/actions,s700-cmu.h > create mode 100644 include/dt-bindings/clock/actions,s900-cmu.h > delete mode 100644 include/dt-bindings/clock/s900_cmu.h > create mode 100644 include/dt-bindings/reset/actions,s700-reset.h > create mode 100644 include/dt-bindings/reset/actions,s900-reset.h > > -- > 2.7.4 >
On Mon, Apr 06, 2020 at 11:40:28PM +0530, Manivannan Sadhasivam wrote: > Hi Tom, > > 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). > > > > I've reviewed all patches and tested them on Bubblegum96 board. Since we > now have time for the next merge window, can you please apply this series > once it is open? > > Or let me know if I have to send you a pull req. I will take this for the next merge window/-next branch, thanks!
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. Second, the new defconfig isn't listed under any MAINTAINERS file and needs to be. I had fixed this up while testing but the other problem is more severe and needs looking in to. Thanks!
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)? Cheers, Andre > Second, the new defconfig isn't listed under > any MAINTAINERS file and needs to be. I had fixed this up while testing > but the other problem is more severe and needs looking in to. Thanks! >
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.
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. 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!
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? > 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.
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).
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.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. 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