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

Alexey Brodkin Alexey.Brodkin at synopsys.com
Fri Mar 23 16:49:04 UTC 2018


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 at 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


More information about the U-Boot mailing list