[U-Boot] [PATCH 2/2] ARC: HSDK: add platform-specific commands

[Alexey Brodkin <Alexey.Brodkin@synopsys.com>]

Hi Eugeniy,

Please populate commit message with brief explanation of what
kind of functionality is added by that change and why it is
needed in the first place.

On Fri, 2018-03-23 at 15:35 +0300, Eugeniy Paltsev wrote:
> Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> ---
>  arch/arc/dts/hsdk.dts           |   56 +++
>  board/synopsys/hsdk/MAINTAINERS |    4 +-
>  board/synopsys/hsdk/Makefile    |    2 +
>  board/synopsys/hsdk/clk-lib.c   |   75 +++
>  board/synopsys/hsdk/clk-lib.h   |   38 ++
>  board/synopsys/hsdk/env-lib.c   |  302 ++++++++++++
>  board/synopsys/hsdk/env-lib.h   |   58 +++
>  board/synopsys/hsdk/hsdk.c      | 1034 +++++++++++++++++++++++++++++++++++++--
>  configs/hsdk_defconfig          |   14 +
>  include/configs/hsdk.h          |   56 ++-
>  10 files changed, 1588 insertions(+), 51 deletions(-)
>  create mode 100644 board/synopsys/hsdk/clk-lib.c
>  create mode 100644 board/synopsys/hsdk/clk-lib.h
>  create mode 100644 board/synopsys/hsdk/env-lib.c
>  create mode 100644 board/synopsys/hsdk/env-lib.h
> 
> diff --git a/arch/arc/dts/hsdk.dts b/arch/arc/dts/hsdk.dts
> index 67dfb93ca8..4bb3035d53 100644
> --- a/arch/arc/dts/hsdk.dts
> +++ b/arch/arc/dts/hsdk.dts
> @@ -6,6 +6,7 @@
>  /dts-v1/;
>  
>  #include "skeleton.dtsi"
> +#include "dt-bindings/clock/snps,hsdk-cgu.h"
>  
>  / {
>  	#address-cells = <1>;
> @@ -13,6 +14,7 @@
>  
>  	aliases {
>  		console = &uart0;
> +		spi0 = &spi0;
>  	};

That (together with corresponding defconfig changes) should be put in
a separate commit that adds support of SPI Flash on this board.

Also pls either make sure series this change is depending on get
applied to main git tree of U-Boot or mention them as a prerequisites.

>  	cpu_card {
> @@ -24,6 +26,35 @@
>  		};
>  	};
>  
> +	clk-fmeas {
> +		clocks = <&cgu_clk CLK_ARC_PLL>, <&cgu_clk CLK_SYS_PLL>,
> +			 <&cgu_clk CLK_TUN_PLL>, <&cgu_clk CLK_DDR_PLL>,
> +			 <&cgu_clk CLK_ARC>, <&cgu_clk CLK_HDMI_PLL>,
> +			 <&cgu_clk CLK_TUN_TUN>, <&cgu_clk CLK_HDMI>,
> +			 <&cgu_clk CLK_SYS_APB>, <&cgu_clk CLK_SYS_AXI>,
> +			 <&cgu_clk CLK_SYS_ETH>, <&cgu_clk CLK_SYS_USB>,
> +			 <&cgu_clk CLK_SYS_SDIO>, <&cgu_clk CLK_SYS_HDMI>,
> +			 <&cgu_clk CLK_SYS_GFX_CORE>, <&cgu_clk CLK_SYS_GFX_DMA>,
> +			 <&cgu_clk CLK_SYS_GFX_CFG>, <&cgu_clk CLK_SYS_DMAC_CORE>,
> +			 <&cgu_clk CLK_SYS_DMAC_CFG>, <&cgu_clk CLK_SYS_SDIO_REF>,
> +			 <&cgu_clk CLK_SYS_SPI_REF>, <&cgu_clk CLK_SYS_I2C_REF>,
> +			 <&cgu_clk CLK_SYS_UART_REF>, <&cgu_clk CLK_SYS_EBI_REF>,
> +			 <&cgu_clk CLK_TUN_ROM>, <&cgu_clk CLK_TUN_PWM>;
> +		clock-names = "cpu-pll", "sys-pll",
> +			      "tun-pll", "ddr-clk",
> +			      "cpu-clk", "hdmi-pll",
> +			      "tun-clk", "hdmi-clk",
> +			      "apb-clk", "axi-clk",
> +			      "eth-clk", "usb-clk",
> +			      "sdio-clk", "hdmi-sys-clk",
> +			      "gfx-core-clk", "gfx-dma-clk",
> +			      "gfx-cfg-clk", "dmac-core-clk",
> +			      "dmac-cfg-clk", "sdio-ref-clk",
> +			      "spi-clk", "i2c-clk",
> +			      "uart-clk", "ebi-clk",
> +			      "rom-clk", "pwm-clk";
> +	};
> +
>  	cgu_clk: cgu-clk at f0000000 {
>  		compatible = "snps,hsdk-cgu-clock";
>  		reg = <0xf0000000 0x10>, <0xf00014B8 0x4>;
> @@ -53,4 +84,29 @@
>  		compatible = "generic-ohci";
>  		reg = <0xf0060000 0x100>;
>  	};
> +
> +	spi0: spi at f0020000 {
> +		compatible = "snps,dw-apb-ssi";
> +		reg = <0xf0020000 0x1000>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		spi-max-frequency = <4000000>;
> +		clocks = <&cgu_clk CLK_SYS_SPI_REF>;
> +		clock-names = "spi_clk";
> +		cs-gpio = <&cs_gpio 0>;
> +		spi_flash at 0 {
> +			compatible = "spi-flash";
> +			reg = <0>;
> +			spi-max-frequency = <4000000>;
> +		};
> +	};
> +
> +	cs_gpio: gpio at f00114B0 {
> +		compatible = "snps,hsdk-creg-gpio";
> +		reg = <0xf00014B0 0x4>;
> +		gpio-controller;
> +		#gpio-cells = <1>;
> +		gpio-bank-name = "hsdk-spi-cs";
> +		gpio-count = <1>;

Ditto.
 
> -#define	CREG_BASE	(ARC_PERIPHERAL_BASE + 0x1000)
> -#define	CREG_PAE	(CREG_BASE + 0x180)
> -#define	CREG_PAE_UPDATE	(CREG_BASE + 0x194)
> -#define	CREG_CPU_START	(CREG_BASE + 0x400)
> +#define ALL_CPU_MASK		GENMASK(NR_CPUS - 1, 0)
> +#define MASTER_CPU_ID		0
> +#define APERTURE_SHIFT		28
> +#define NO_CCM			0x10
> +#define SLAVE_CPU_READY		0x12345678
> +#define BOOTSTAGE_1		1
> +#define BOOTSTAGE_2		2
> +#define BOOTSTAGE_3		3
> +#define BOOTSTAGE_4		4
> +#define BOOTSTAGE_5		5

Seems like you don't like an idea of giving meaningful names to these stages.

>  
> -int board_early_init_f(void)
> +#define RESET_VECTOR_ADDR	0x0
> +
> +#define CREG_BASE		(ARC_PERIPHERAL_BASE + 0x1000)
> +#define CREG_CPU_START		(CREG_BASE + 0x400)
> +#define CREG_CPU_START_MASK	0xF
> +
> +#define SDIO_BASE		(ARC_PERIPHERAL_BASE + 0xA000)
> +#define SDIO_UHS_REG_EXT	(SDIO_BASE + 0x108)
> +#define SDIO_UHS_REG_EXT_DIV_2	(2 << 30)
> +
> +/* Uncached access macros */
> +#define arc_read_uncached_32(ptr)	\
> +({					\
> +	unsigned int __ret;		\
> +	__asm__ __volatile__(		\
> +	"	ld.di %0, [%1]	\n"	\
> +	: "=r"(__ret)			\
> +	: "r"(ptr));			\
> +	__ret;				\
> +})
> +
> +#define arc_write_uncached_32(ptr, data)\
> +({					\
> +	__asm__ __volatile__(		\
> +	"	st.di %0, [%1]	\n"	\
> +	:				\
> +	: "r"(data), "r"(ptr));		\
> +})
> +
> +struct hsdk_env_core_ctl {
> +	u32_env entry[NR_CPUS];
> +	u32_env iccm[NR_CPUS];
> +	u32_env dccm[NR_CPUS];
> +};
> +
> +struct hsdk_env_common_ctl {
> +	bool halt_on_boot;
> +	u32_env core_mask;
> +	u32_env cpu_freq;
> +	u32_env axi_freq;
> +	u32_env tun_freq;
> +	u32_env nvlim;
> +	u32_env icache;
> +	u32_env dcache;
> +};
> +
> +/*
> + * Uncached cross-cpu structure, all CPUs must access to this structure fields
> + * only with arc_read_uncached_32() / arc_write_uncached_32() accessors.

... because ...

> + */
> +struct hsdk_cross_cpu {
> +	/* slave CPU ready flag */
> +	u32 ready_flag;
> +	/* address of the area, which can be used for stack by slave CPU */
> +	u32 stack_ptr;
> +	/* slave CPU status - bootstage number */
> +	s32 status[NR_CPUS];
> +
> +	/*
> +	 * Slave CPU data - it is copy of corresponding fields in
> +	 * hsdk_env_core_ctl and hsdk_env_common_ctl structures which are
> +	 * required for slave CPUs initialization.
> +	 * This fields can be populated by copying from hsdk_env_core_ctl
> +	 * and hsdk_env_common_ctl structures with sync_cross_cpu_data()
> +	 * function.
> +	 */
> +	u32 entry[NR_CPUS];
> +	u32 iccm[NR_CPUS];
> +	u32 dccm[NR_CPUS];
> +
> +	u32 core_mask;
> +	u32 icache;
> +	u32 dcache;
> +
> +	u8 cache_padding[ARCH_DMA_MINALIGN];
> +} __aligned(ARCH_DMA_MINALIGN);
> +
> +/* Place for slave CPUs temporary stack */
> +static u32 slave_stack[256 * NR_CPUS] __aligned(4);

__aligned(4) makes no sense for an array of ints on 32 bit arch.

> +
> +/* TODO: add ICCM BCR and DCCM BCR runtime check */
> +static void init_slave_cpu_func(u32 core)
> +{
> +	u32 val;
> +
> +	/* ICCM move if exists */

IMHO it's better to say "Remap xCCM to another memory region".

[snip]

> +static void do_init_master_cpu(void)
> +{
> +	/*
> +	 * Setup master caches even if master isn't used as we want to use
> +	 * same cache configuration on all running CPUs
> +	 */
> +	init_master_icache();
> +	init_master_dcache();
> +}
> +
> +enum hsdk_axi_masters {
> +	M_HS_CORE = 0,
> +	M_HS_RTT,
> +	M_AXI_TUN,
> +	M_HDMI_VIDEO,
> +	M_HDMI_AUDIO,
> +	M_USB_HOST,
> +	M_ETHERNET,
> +	M_SDIO,
> +	M_GPU,
> +	M_DMAC_0,
> +	M_DMAC_1,
> +	M_DVFS
> +};
> +
> +#define UPDATE_VAL	1
> +
> +/*
> + * m	master		AXI_M_m_SLV0	AXI_M_m_SLV1	AXI_M_m_OFFSET0	AXI_M_m_OFFSET1
> + * 0	HS (CBU)	0x11111111	0x63111111	0xFEDCBA98	0x0E543210
> + * 1	HS (RTT)	0x77777777	0x77777777	0xFEDCBA98	0x76543210
> + * 2	AXI Tunnel	0x88888888	0x88888888	0xFEDCBA98	0x76543210
> + * 3	HDMI-VIDEO	0x77777777	0x77777777	0xFEDCBA98	0x76543210
> + * 4	HDMI-ADUIO	0x77777777	0x77777777	0xFEDCBA98	0x76543210
> + * 5	USB-HOST	0x77777777	0x77999999	0xFEDCBA98	0x76DCBA98
> + * 6	ETHERNET	0x77777777	0x77999999	0xFEDCBA98	0x76DCBA98
> + * 7	SDIO		0x77777777	0x77999999	0xFEDCBA98	0x76DCBA98
> + * 8	GPU		0x77777777	0x77777777	0xFEDCBA98	0x76543210
> + * 9	DMAC (port #1)	0x77777777	0x77777777	0xFEDCBA98	0x76543210
> + * 10	DMAC (port #2)	0x77777777	0x77777777	0xFEDCBA98	0x76543210
> + * 11	DVFS		0x00000000	0x60000000	0x00000000	0x00000000
> + *
> + * Please read ARC HS Development IC Specification, section 17.2 for more
> + * information about apertures configuration.
> + * NOTE: we are changing default apertures configuration, specified in

"We intentionally modify default settings in U-boot".

> + * "Table 111 CREG Address Decoder register reset values".
> + */
> +

[snip]

> +static int do_hsdk_clock_print_all(cmd_tbl_t *cmdtp, int flag, int argc,
> +				   char *const argv[])
> +{
> +	/* CPU clock domain */
> +	soc_clk_ctl("cpu-pll", NULL, CLK_PRINT | CLK_MHZ);
> +	soc_clk_ctl("cpu-clk", NULL, CLK_PRINT | CLK_MHZ);
> +	printf("\n");
> +
> +	/* SYS clock domain */
> +	soc_clk_ctl("sys-pll", NULL, CLK_PRINT | CLK_MHZ);
> +	soc_clk_ctl("apb-clk", NULL, CLK_PRINT | CLK_MHZ);
> +	soc_clk_ctl("axi-clk", NULL, CLK_PRINT | CLK_MHZ);
> +	soc_clk_ctl("eth-clk", NULL, CLK_PRINT | CLK_MHZ);
> +	soc_clk_ctl("usb-clk", NULL, CLK_PRINT | CLK_MHZ);
> +	soc_clk_ctl("sdio-clk", NULL, CLK_PRINT | CLK_MHZ);
> +/*	soc_clk_ctl("hdmi-sys-clk", NULL, CLK_PRINT | CLK_MHZ); */
> +	soc_clk_ctl("gfx-core-clk", NULL, CLK_PRINT | CLK_MHZ);
> +	soc_clk_ctl("gfx-dma-clk", NULL, CLK_PRINT | CLK_MHZ);
> +	soc_clk_ctl("gfx-cfg-clk", NULL, CLK_PRINT | CLK_MHZ);
> +	soc_clk_ctl("dmac-core-clk", NULL, CLK_PRINT | CLK_MHZ);
> +	soc_clk_ctl("dmac-cfg-clk", NULL, CLK_PRINT | CLK_MHZ);
> +	soc_clk_ctl("sdio-ref-clk", NULL, CLK_PRINT | CLK_MHZ);
> +	soc_clk_ctl("spi-clk", NULL, CLK_PRINT | CLK_MHZ);
> +	soc_clk_ctl("i2c-clk", NULL, CLK_PRINT | CLK_MHZ);
> +/*	soc_clk_ctl("ebi-clk", NULL, CLK_PRINT | CLK_MHZ); */
> +	soc_clk_ctl("uart-clk", NULL, CLK_PRINT | CLK_MHZ);
> +	printf("\n");
> +
> +	/* DDR clock domain */
> +	soc_clk_ctl("ddr-clk", NULL, CLK_PRINT | CLK_MHZ);
> +	printf("\n");
> +
> +	/* HDMI clock domain */
> +/*	soc_clk_ctl("hdmi-pll", NULL, CLK_PRINT | CLK_MHZ); */
> +/*	soc_clk_ctl("hdmi-clk", NULL, CLK_PRINT | CLK_MHZ); */
> +/*	printf("\n"); */
> +

Please explain why do we want to keep these commented out lines.

>  }
> diff --git a/configs/hsdk_defconfig b/configs/hsdk_defconfig
> index 11cb7e03a6..7656ec9a31 100644
> --- a/configs/hsdk_defconfig
> +++ b/configs/hsdk_defconfig
> @@ -7,13 +7,19 @@ CONFIG_DEFAULT_DEVICE_TREE="hsdk"
>  CONFIG_USE_BOOTARGS=y
>  CONFIG_BOOTARGS="console=ttyS0,115200n8"
>  CONFIG_BOARD_EARLY_INIT_F=y
> +CONFIG_HUSH_PARSER=y
>  CONFIG_SYS_PROMPT="hsdk# "
> +CONFIG_CMD_ENV_FLAGS=y
>  # CONFIG_CMD_FLASH is not set
> +CONFIG_CMD_GPIO=y
>  CONFIG_CMD_MMC=y
> +CONFIG_CMD_SF=y
> +CONFIG_CMD_SPI=y

Move to a separate patch.

>  CONFIG_CMD_USB=y
>  # CONFIG_CMD_SETEXPR is not set
>  CONFIG_CMD_DHCP=y
>  CONFIG_CMD_PING=y
> +CONFIG_CMD_CACHE=y
>  CONFIG_CMD_EXT2=y
>  CONFIG_CMD_EXT4=y
>  CONFIG_CMD_EXT4_WRITE=y
> @@ -25,12 +31,20 @@ CONFIG_ENV_FAT_INTERFACE="mmc"
>  CONFIG_ENV_FAT_DEVICE_AND_PART="0:1"
>  CONFIG_NET_RANDOM_ETHADDR=y
>  CONFIG_DM=y
> +CONFIG_CLK_HSDK=y
> +CONFIG_DM_GPIO=y
> +CONFIG_HSDK_CREG_GPIO=y

Last 2 lines go to a separate patch.

>  CONFIG_MMC=y
>  CONFIG_MMC_DW=y
> +CONFIG_DM_SPI_FLASH=y
> +CONFIG_SPI_FLASH=y
> +CONFIG_SPI_FLASH_SST=y

Ditto

>  CONFIG_DM_ETH=y
>  CONFIG_ETH_DESIGNWARE=y
>  CONFIG_DM_SERIAL=y
>  CONFIG_SYS_NS16550=y
> +CONFIG_DM_SPI=y
> +CONFIG_DESIGNWARE_SPI=y

Ditto.

-Alexey