[PATCH 6/6] ARM: rmobile: Add Beacon EmbeddedWorks RZG2M Dev Kit

[Marek Vasut]

On 6/19/20 3:58 PM, Adam Ford wrote:
[...]
> diff --git a/board/beacon/beacon-rzg2m/beacon-rzg2m.c b/board/beacon/beacon-rzg2m/beacon-rzg2m.c
> new file mode 100644
> index 0000000000..88702958e0
> --- /dev/null
> +++ b/board/beacon/beacon-rzg2m/beacon-rzg2m.c
> @@ -0,0 +1,112 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2020 Compass Electronics Group, LLC
> + */
> +
> +#include <common.h>
> +#include <asm/io.h>
> +#include <asm/arch/rcar-mstp.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +void s_init(void)
> +{
> +}
> +
> +#define DVFS_MSTP926		BIT(26)
> +#define GPIO2_MSTP910		BIT(10)
> +#define GPIO3_MSTP909		BIT(9)
> +#define GPIO5_MSTP907		BIT(7)
> +#define HSUSB_MSTP704		BIT(4)	/* HSUSB */
> +
> +int board_early_init_f(void)
> +{
> +#if defined(CONFIG_SYS_I2C) && defined(CONFIG_SYS_I2C_SH)
> +	/* DVFS for reset */
> +	mstp_clrbits_le32(SMSTPCR9, SMSTPCR9, DVFS_MSTP926);
> +#endif
> +	return 0;
> +}
> +
> +/* HSUSB block registers */
> +#define HSUSB_REG_LPSTS			0xE6590102
> +#define HSUSB_REG_LPSTS_SUSPM_NORMAL	BIT(14)
> +#define HSUSB_REG_UGCTRL2		0xE6590184
> +#define HSUSB_REG_UGCTRL2_USB0SEL	0x30
> +#define HSUSB_REG_UGCTRL2_USB0SEL_EHCI	0x10
> +
> +int board_init(void)
> +{
> +	/* address of boot parameters */
> +	gd->bd->bi_boot_params = CONFIG_SYS_TEXT_BASE + 0x50000;
> +
> +	/* USB1 pull-up */
> +	setbits_le32(PFC_PUEN6, PUEN_USB1_OVC | PUEN_USB1_PWEN);
> +
> +	/* Configure the HSUSB block */
> +	mstp_clrbits_le32(SMSTPCR7, SMSTPCR7, HSUSB_MSTP704);
> +	/* Choice USB0SEL */
> +	clrsetbits_le32(HSUSB_REG_UGCTRL2, HSUSB_REG_UGCTRL2_USB0SEL,
> +			HSUSB_REG_UGCTRL2_USB0SEL_EHCI);
> +	/* low power status */
> +	setbits_le16(HSUSB_REG_LPSTS, HSUSB_REG_LPSTS_SUSPM_NORMAL);
> +
> +	return 0;
> +}

Is any of this really needed on your board ?

[...]

> +void board_add_ram_info(int use_default)
> +{
> +	int i;
> +
> +	printf("\n");
> +	for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
> +		if (!gd->bd->bi_dram[i].size)
> +			break;
> +		printf("Bank #%d: 0x%09llx - 0x%09llx, ", i,
> +		       (unsigned long long)(gd->bd->bi_dram[i].start),
> +		       (unsigned long long)(gd->bd->bi_dram[i].start
> +		       + gd->bd->bi_dram[i].size - 1));
> +		print_size(gd->bd->bi_dram[i].size, "\n");
> +	};
> +}

Is this needed ?

> +void board_cleanup_before_linux(void)
> +{
> +	/*
> +	 * Turn off the clock that was turned on outside
> +	 * the control of the driver
> +	 */
> +	/* Configure the HSUSB block */
> +	mstp_setbits_le32(SMSTPCR7, SMSTPCR7, HSUSB_MSTP704);
> +
> +	/*
> +	 * Because of the control order dependency,
> +	 * turn off a specific clock at this timing
> +	 */
> +	mstp_setbits_le32(SMSTPCR9, SMSTPCR9,
> +			  GPIO2_MSTP910 | GPIO3_MSTP909 | GPIO5_MSTP907);
> +}

The clock driver should do this clock turning off, should it not ?

> diff --git a/configs/r8a774a1_beacon-rzg2m_defconfig b/configs/r8a774a1_beacon-rzg2m_defconfig
> new file mode 100644
> index 0000000000..b2a699f21a
> --- /dev/null
> +++ b/configs/r8a774a1_beacon-rzg2m_defconfig

[...]

> +CONFIG_BOOTARGS="root=/dev/nfs rw nfsroot=192.168.0.1:/export/rfs ip=192.168.0.20"

Please don't hard-code user-specific configuration.

[...]

> diff --git a/include/configs/beacon-rzg2m.h b/include/configs/beacon-rzg2m.h
> new file mode 100644
> index 0000000000..d451bfc5ee
> --- /dev/null
> +++ b/include/configs/beacon-rzg2m.h

[...]

> +/* Generic Timer Definitions (use in assembler source) */
> +#define COUNTER_FREQUENCY	0xFE502A	/* 16.66MHz from CPclk */

Is this needed ?

> +/* Environment in eMMC, at the end of 2nd "boot sector" */
> +/* #define CONFIG_ENV_OFFSET		(-CONFIG_ENV_SIZE) */
> +#define CONFIG_SYS_MMC_ENV_DEV		1
> +#define CONFIG_SYS_MMC_ENV_PART		2
> +
> +#undef CONFIG_EXTRA_ENV_SETTINGS
> +
> +#define CONFIG_EXTRA_ENV_SETTINGS		\
> +	"usb_pgood_delay=2000\0"	\
> +	"fdt_high=0xffffffffffffffff\0"	\
> +	"initrd_high=0xffffffffffffffff\0" \
> +	"script=boot.scr\0" \
> +	"image=Image\0" \
> +	"console=ttySC0,115200\0" \
> +	"fdt_addr=0x48000000\0" \
> +	"loadaddr=0x48080000\0" \
> +	"fdt_high=0xffffffffffffffff\0"		\

Do not use fdt_high and initrd_high, it breaks booting if the DT is at
4-byte aligned offset instead of 8-byte aligned offset. The gen3 uses
bootm_size to automatically relocate the DT to the correct location.

Also, some of the env entries are defined multiple times.

[...]

> diff --git a/include/dt-bindings/clk/versaclock.h b/include/dt-bindings/clk/versaclock.h

I think this should not be part of this patch.