Re: [RFC PATCH 02/11] sunxi: introduce mach-sunxi

[Jules Maselbas <jmaselbas@zdiv.net>]

On Thu, May 18, 2023 at 08:46:56PM +0200, Ahmad Fatoum wrote:
> On 11.05.23 01:37, Jules Maselbas wrote:
> > Add kbuild boilerplate and some early init functions
> > ---
> >  arch/arm/Kconfig                   | 12 ++++++++++
> >  arch/arm/Makefile                  |  1 +
> >  arch/arm/mach-sunxi/Kconfig        | 16 +++++++++++++
> >  arch/arm/mach-sunxi/Makefile       |  2 ++
> >  arch/arm/mach-sunxi/cpu_init.c     | 33 +++++++++++++++++++++++++++
> >  arch/arm/mach-sunxi/sunxi.c        |  0
> >  images/Makefile                    |  1 +
> >  images/Makefile.sunxi              | 13 +++++++++++
> >  include/mach/sunxi/init.h          |  6 +++++
> >  include/mach/sunxi/sun50i-regs.h   | 36 ++++++++++++++++++++++++++++++
> >  include/mach/sunxi/sunxi-pinctrl.h | 13 +++++++++++
> >  11 files changed, 133 insertions(+)
> >  create mode 100644 arch/arm/mach-sunxi/Kconfig
> >  create mode 100644 arch/arm/mach-sunxi/Makefile
> >  create mode 100644 arch/arm/mach-sunxi/cpu_init.c
> >  create mode 100644 arch/arm/mach-sunxi/sunxi.c
> >  create mode 100644 images/Makefile.sunxi
> >  create mode 100644 include/mach/sunxi/init.h
> >  create mode 100644 include/mach/sunxi/sun50i-regs.h
> >  create mode 100644 include/mach/sunxi/sunxi-pinctrl.h
> > 
> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > index abe649de49..8da03e2703 100644
> > --- a/arch/arm/Kconfig
> > +++ b/arch/arm/Kconfig
> > @@ -144,6 +144,17 @@ config ARCH_SOCFPGA
> >  	select COMMON_CLK
> >  	select CLKDEV_LOOKUP
> >  
> > +config ARCH_SUNXI
> > +	bool "Allwinner SoCs"
> 
> depends on ARCH_MULTIARCH? :-)
I don't recall testing a multiarch build, yet

> That's the new hot stuff where you can build multiple SoC families
> in one go. Also, no HAS_DEBUG_LL?
low-level debug is added in a later patch. my initial though was that
HAS_DEBUG_LL is more board specific as it depends on the acctual UART
used for debugging, that's why it is added in ARCH_SUN50I_A64 Kconfig.
I can change this, add HAS_DEBUG_LL to ARCH_SUNXI instead.

> > +	select OFTREE
> > +	select OFDEVICE
> > +	select COMMON_CLK
> > +	select COMMON_CLK_OF_PROVIDER
> > +	select CLKDEV_LOOKUP
> > +	select GENERIC_GPIO
> 
> This is implied by select GPIOLIB below.
ack

> > +	select GPIOLIB
> > +	select PINCTRL
> > +
> >  config ARCH_VERSATILE
> >  	bool "ARM Versatile boards (ARM926EJ-S)"
> >  	select GPIOLIB
> > @@ -311,6 +322,7 @@ source "arch/arm/mach-omap/Kconfig"
> >  source "arch/arm/mach-pxa/Kconfig"
> >  source "arch/arm/mach-rockchip/Kconfig"
> >  source "arch/arm/mach-socfpga/Kconfig"
> > +source "arch/arm/mach-sunxi/Kconfig"
> >  source "arch/arm/mach-stm32mp/Kconfig"
> >  source "arch/arm/mach-versatile/Kconfig"
> >  source "arch/arm/mach-vexpress/Kconfig"
> > diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> > index a506f1e3a3..bb61392e4c 100644
> > --- a/arch/arm/Makefile
> > +++ b/arch/arm/Makefile
> > @@ -103,6 +103,7 @@ machine-$(CONFIG_ARCH_ROCKCHIP)		+= rockchip
> >  machine-$(CONFIG_ARCH_SAMSUNG)		+= samsung
> >  machine-$(CONFIG_ARCH_SOCFPGA)		+= socfpga
> >  machine-$(CONFIG_ARCH_STM32MP)		+= stm32mp
> > +machine-$(CONFIG_ARCH_SUNXI)		+= sunxi
> >  machine-$(CONFIG_ARCH_VERSATILE)	+= versatile
> >  machine-$(CONFIG_ARCH_VEXPRESS)		+= vexpress
> >  machine-$(CONFIG_ARCH_TEGRA)		+= tegra
> > diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> > new file mode 100644
> > index 0000000000..0e8d83fedd
> > --- /dev/null
> > +++ b/arch/arm/mach-sunxi/Kconfig
> > @@ -0,0 +1,16 @@
> > +if ARCH_SUNXI
> > +
> > +config ARCH_TEXT_BASE
> > +	hex
> > +	default 0x0
> > +
> > +menuconfig SUNXI_MULTI_BOARDS
> > +	bool "Allwinner boards"
> > +	select HAVE_PBL_MULTI_IMAGES
> > +	select RELOCATABLE
> 
> Just make this the default?
by putting this into ARCH_SUNXI ?


> > +
> > +if SUNXI_MULTI_BOARDS
> > +
> > +endif
> > +
> > +endif
> > diff --git a/arch/arm/mach-sunxi/Makefile b/arch/arm/mach-sunxi/Makefile
> > new file mode 100644
> > index 0000000000..d678973ca2
> > --- /dev/null
> > +++ b/arch/arm/mach-sunxi/Makefile
> > @@ -0,0 +1,2 @@
> > +obj-y += sunxi.o
> > +lwl-y += cpu_init.o
> > diff --git a/arch/arm/mach-sunxi/cpu_init.c b/arch/arm/mach-sunxi/cpu_init.c
> > new file mode 100644
> > index 0000000000..f4092d8d5d
> > --- /dev/null
> > +++ b/arch/arm/mach-sunxi/cpu_init.c
> > @@ -0,0 +1,33 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +
> > +#include <common.h>
> > +#include <linux/sizes.h>
> > +#include <asm/barebox-arm-head.h>
> > +// #include <asm/errata.h> TODO: errata 843419 ?
> > +#include <io.h>
> > +#include <debug_ll.h>
> > +#include <mach/sunxi/init.h>
> > +#include <mach/sunxi/sun50i-regs.h>
> > +#include <mach/sunxi/sunxi-pinctrl.h>
> > +
> > +static void sunxi_ccu_init(void __iomem *ccu)
> > +{
> > +	/* APB2 = 24MHz (UART, I2C) */
> > +	writel((1 << 24 /* src: 1=osc24 */) |
> > +	       (0 << 16 /* pre_div (N): 0=/1 1=/2 2=/4 3=/8 */) |
> > +	       (0 << 0) /* M-1 */,
> > +	       ccu + CCU_APB2_CFG);
> > +	set_cntfrq(24 * 1000 * 1000);
> > +
> > +	/* PIO clock enable */
> > +	setbits_le32(ccu + CCU_BUS_CLK_GATE2, 1u << 5);
> 
> Nit: There's a BIT() macro that could be used here.
ack
 
> > +	/* UART0 clock enable */
> > +	setbits_le32(ccu + CCU_BUS_CLK_GATE3, 1u << 16);
> > +	/* UART0 release reset */
> > +	setbits_le32(ccu + CCU_BUS_SOFT_RST4, 1u << 16);
> 
> Can the pads UART0 uses be muxed otherwise? Is it ok to unconditionally
To be clear: nothing touches the pin-muxes here, only clock-gate and reset
for gpio and uart. UART0 isn't muxed by default, it can be either muxed to
PB8/PB9 or PF2/PF4. To enable UART0 on PB8/PB9 you will to write 0x-----33-
into PIO PB_CFG1(0x28) register. Which I didn't do here... I think that's
because the boot ROM has done it... or i am missing something.

> enable UART0 here? I'd expect this to be a separate sunxi_uart_setup(port)
> that can be used by board code after doing lowlevel_init.
okay

> > +}
> > +
> > +static void sunxi_cpu_lowlevel_init(void)
> > +{
> > +	sunxi_ccu_init(IOMEM(SUNXI_CCU_BASE_ADDR));
> > +}
> > diff --git a/arch/arm/mach-sunxi/sunxi.c b/arch/arm/mach-sunxi/sunxi.c
> > new file mode 100644
> > index 0000000000..e69de29bb2
> > diff --git a/images/Makefile b/images/Makefile
> > index aa5814710f..3a10fe1abb 100644
> > --- a/images/Makefile
> > +++ b/images/Makefile
> > @@ -151,6 +151,7 @@ include $(srctree)/images/Makefile.omap3
> >  include $(srctree)/images/Makefile.rockchip
> >  include $(srctree)/images/Makefile.socfpga
> >  include $(srctree)/images/Makefile.stm32mp
> > +include $(srctree)/images/Makefile.sunxi
> >  include $(srctree)/images/Makefile.tegra
> >  include $(srctree)/images/Makefile.vexpress
> >  include $(srctree)/images/Makefile.xburst
> > diff --git a/images/Makefile.sunxi b/images/Makefile.sunxi
> > new file mode 100644
> > index 0000000000..778d6f9bdf
> > --- /dev/null
> > +++ b/images/Makefile.sunxi
> > @@ -0,0 +1,13 @@
> > +#
> > +# barebox image generation Makefile for Allwinner sunxi eGON boot images
> > +#
> > +
> > +# %.egonimg - convert into eGON.BT0 image
> > +# ----------------------------------------------------------------------
> > +quiet_cmd_egon_image = EGON  $@
> > +      cmd_egon_image = $(objtree)/scripts/egon_mkimage $< $@
> > +
> > +$(obj)/%.egonimg: $(obj)/% FORCE
> > +	$(call if_changed,egon_image)
> > +
> > +# ----------------------------------------------------------------------
> > diff --git a/include/mach/sunxi/init.h b/include/mach/sunxi/init.h
> > new file mode 100644
> > index 0000000000..26cc022fde
> > --- /dev/null
> > +++ b/include/mach/sunxi/init.h
> > @@ -0,0 +1,6 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +
> > +#ifndef __MACH_INIT_H
> > +#define __MACH_INIT_H
> > +
> > +#endif
> > diff --git a/include/mach/sunxi/sun50i-regs.h b/include/mach/sunxi/sun50i-regs.h
> > new file mode 100644
> > index 0000000000..68501fa351
> > --- /dev/null
> > +++ b/include/mach/sunxi/sun50i-regs.h
> > @@ -0,0 +1,36 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +
> > +#ifndef __MACH_SUN50I_REGS_H
> > +#define __MACH_SUN50I_REGS_H
> 
> #include <linux/sizes.h>
> 
> > +
> > +#define SUN50I_SRAM_A1_BASE_ADDR 0x00000000
> > +#define SUN50I_SRAM_A1_SIZE SZ_64K
> > +#define SUN50I_SRAM_A2_BASE_ADDR 0x00044000
> > +#define SUN50I_SRAM_A2_SIZE SZ_32K
> > +
> > +#define SUN50I_DRAM_BASE_ADDR	0x40000000
> > +
> > +#define SUN50I_CCU_BASE_ADDR	0x01c20000
> > +#define SUN50I_PIO_BASE_ADDR	0x01c20800
> > +#define SUN50I_MMC0_BASE_ADDR	0x01c0f000
> > +#define SUN50I_MMC1_BASE_ADDR	0x01c10000
> > +#define SUN50I_MMC2_BASE_ADDR	0x01c11000
> > +#define SUN50I_TIMER_BASE_ADDR	0x01c20c00
> 
> I know much existing code doesn't, but you could already use IOMEM
> here to avoid casts in consumer code.
ack

> > +
> > +#define CCU_PLL_CPUX		0x00
> > +#define CCU_PLL_PERIPH0		0x28
> > +#define CCU_CPUX_AXI_CFG	0x50
> > +#define CCU_AHB1_APB1_CFG	0x54
> > +#define CCU_APB2_CFG		0x58
> > +#define CCU_AHB2_CFG		0x5c
> > +#define CCU_BUS_CLK_GATE0	0x60
> > +#define CCU_BUS_CLK_GATE1	0x64
> > +#define CCU_BUS_CLK_GATE2	0x68
> > +#define CCU_BUS_CLK_GATE3	0x6c
> > +#define CCU_CE_CLK		0x9c
> > +#define CCU_MBUS_CLK		0x15c
> > +#define CCU_BUS_SOFT_RST0	0x2c0
> > +#define CCU_BUS_SOFT_RST4	0x2d8
> > +#define CCU_PLL_LOCK_CTRL	0x320
> > +
> > +#endif
> > diff --git a/include/mach/sunxi/sunxi-pinctrl.h b/include/mach/sunxi/sunxi-pinctrl.h
> > new file mode 100644
> > index 0000000000..adb2a24577
> > --- /dev/null
> > +++ b/include/mach/sunxi/sunxi-pinctrl.h
> > @@ -0,0 +1,13 @@
> > +/* pio aka "allwinner,sun8i-h3-pinctrl" */
> 
> No include guard?
... this file could be merged in "sun50i-regs.h", what do you think ?
 
> > +
> > +#define PIO_PA_CFG0 0x00
> > +#define PIO_PB_CFG0 0x24
> > +#define PIO_PB_CFG1 0x28
> > +#define PIO_PD_CFG0 0x6c
> > +#define PIO_PD_CFG1 0x70
> > +#define PIO_PD_CFG2 0x74
> > +#define PIO_PF_CFG0 0xb4
> > +
> > +#define PIO_PA_PULL1 0x20
> > +#define PIO_PB_PULL0 0x40
> > +#define PIO_PD_DATA 0x7c
> 
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 
>