Hi Bin,
On Thu, Dec 10, 2020 at 3:54 PM Bin Meng <bmeng...@gmail.com> wrote:
> Hi Padmarao,
>
> On Thu, Dec 3, 2020 at 4:44 AM Padmarao Begari
> <padmarao.beg...@microchip.com> wrote:
> >
> > This patch adds Microchip MPFS Icicle Kit support. For now, only
> > NS16550 Serial, Microchip clock, Cadence eMMC and MACB drivers are
> > only enabled. The Microchip MPFS Icicle defconfig by default builds
>
> remove "only" because it was already said at the beginning of this sentence
>
>
Ok
> > U-Boot for S-Mode because U-Boot on Microchip PolarFire SoC will run
> > in S-Mode as payload of HSS + OpenSBI.
> >
> > Signed-off-by: Padmarao Begari <padmarao.beg...@microchip.com>
> > Reviewed-by: Anup Patel <anup.pa...@wdc.com>
> > ---
> > board/microchip/mpfs_icicle/Kconfig | 24 ++++++
> > board/microchip/mpfs_icicle/mpfs_icicle.c | 97 ++++++++++++++++++++++-
> > configs/microchip_mpfs_icicle_defconfig | 9 ++-
> > include/configs/microchip_mpfs_icicle.h | 60 +++++---------
> > 4 files changed, 145 insertions(+), 45 deletions(-)
> >
> > diff --git a/board/microchip/mpfs_icicle/Kconfig
> b/board/microchip/mpfs_icicle/Kconfig
> > index bf8e1a13ec..f4e705a07b 100644
> > --- a/board/microchip/mpfs_icicle/Kconfig
> > +++ b/board/microchip/mpfs_icicle/Kconfig
> > @@ -20,7 +20,31 @@ config BOARD_SPECIFIC_OPTIONS # dummy
> > def_bool y
> > select GENERIC_RISCV
> > select BOARD_EARLY_INIT_F
> > + select BOARD_LATE_INIT
> > imply SMP
> > + imply CLK_CCF
> > + imply CLK_MPFS
> > imply SYS_NS16550
> > + imply CMD_DHCP
> > + imply CMD_EXT2
> > + imply CMD_EXT4
> > + imply CMD_FAT
> > + imply CMD_FS_GENERIC
> > + imply CMD_NET
> > + imply CMD_PING
> > + imply CMD_MMC
> > + imply DOS_PARTITION
> > + imply EFI_PARTITION
> > + imply IP_DYN
> > + imply ISO_PARTITION
> > + imply MACB
> > + imply MII
> > + imply NET_RANDOM_ETHADDR
>
> This should not be used since we are reading mac address from SoC.
>
> Ok, I will remove imply NET_RANDOM_ETHADDR
> + imply PHY_LIB
> > + imply PHY_VITESSE
> > + imply MMC
> > + imply MMC_WRITE
> > + imply MMC_SDHCI
> > + imply MMC_SDHCI_CADENCE
> >
> > endif
> > diff --git a/board/microchip/mpfs_icicle/mpfs_icicle.c
> b/board/microchip/mpfs_icicle/mpfs_icicle.c
> > index 8381361ec3..420ea25ac2 100644
> > --- a/board/microchip/mpfs_icicle/mpfs_icicle.c
> > +++ b/board/microchip/mpfs_icicle/mpfs_icicle.c
> > @@ -6,10 +6,47 @@
> >
> > #include <common.h>
> > #include <dm.h>
> > +#include <env.h>
> > #include <init.h>
> > #include <asm/io.h>
> >
> > -#define MPFS_SYSREG_SOFT_RESET ((unsigned int *)0x20002088)
> > +#define MPFS_SYSREG_SOFT_RESET ((unsigned int *)0x20002088)
> > +#define MPFS_SYS_SERVICE_CR ((unsigned int *)0x37020050)
> > +#define MPFS_SYS_SERVICE_SR ((unsigned int *)0x37020054)
> > +#define MPFS_SYS_SERVICE_MAILBOX ((unsigned char *)0x37020800)
> > +
> > +#define PERIPH_RESET_VALUE 0x1e8u
> > +#define SERVICE_CR_REQ 0x1u
> > +#define SERVICE_SR_BUSY 0x2u
> > +
> > +static void read_device_serial_number(u8 *response, u8 response_size)
> > +{
> > + u8 idx;
> > + u8 *response_buf;
> > + unsigned int val;
> > +
> > + response_buf = (u8 *)response;
> > +
> > + writel(SERVICE_CR_REQ, MPFS_SYS_SERVICE_CR);
> > + /*
> > + * REQ bit will remain set till the system controller starts
> > + * processing.
> > + */
> > + do {
> > + val = readl(MPFS_SYS_SERVICE_CR);
> > + } while (SERVICE_CR_REQ == (val & SERVICE_CR_REQ));
> > +
> > + /*
> > + * Once system controller starts processing the busy bit will
> > + * go high and service is completed when busy bit is gone low
> > + */
> > + do {
> > + val = readl(MPFS_SYS_SERVICE_SR);
> > + } while (SERVICE_SR_BUSY == (val & SERVICE_SR_BUSY));
> > +
> > + for (idx = 0; idx < response_size; idx++)
> > + response_buf[idx] = readb(MPFS_SYS_SERVICE_MAILBOX +
> idx);
> > +}
> >
> > int board_init(void)
> > {
> > @@ -22,10 +59,64 @@ int board_early_init_f(void)
> > {
> > unsigned int val;
> >
> > - /* Reset uart peripheral */
> > + /* Reset uart, mmc peripheral */
> > val = readl(MPFS_SYSREG_SOFT_RESET);
> > - val = (val & ~(1u << 5u));
> > + val = (val & ~(PERIPH_RESET_VALUE));
> > writel(val, MPFS_SYSREG_SOFT_RESET);
> >
> > return 0;
> > }
> > +
> > +int board_late_init(void)
> > +{
> > + u32 ret;
> > + u32 node;
> > + u8 idx;
> > + u8 device_serial_number[16] = { 0 };
> > + unsigned char mac_addr[6];
> > + char icicle_mac_addr[20];
> > + void *blob = (void *)gd->fdt_blob;
>
> Missing DECLARE_GLOBAL_DATA_PTR at the beginning of this file?
>
> Ok I will add DECLARE_GLOBAL_DATA_PTR
> > +
> > + node = fdt_path_offset(blob, "ethernet0");
> > + if (node < 0) {
> > + printf("No ethernet0 path offset\n");
> > + return -ENODEV;
> > + }
> > +
> > + ret = fdtdec_get_byte_array(blob, node, "local-mac-address",
> mac_addr, 6);
> > + if (ret) {
> > + printf("No local-mac-address property\n");
> > + return -EINVAL;
> > + }
> > +
> > + read_device_serial_number(device_serial_number, 16);
> > +
> > + /* Update MAC address with device serial number */
> > + mac_addr[0] = 0x00;
> > + mac_addr[1] = 0x04;
> > + mac_addr[2] = 0xA3;
> > + mac_addr[3] = device_serial_number[2];
> > + mac_addr[4] = device_serial_number[1];
> > + mac_addr[5] = device_serial_number[0];
> > +
> > + ret = fdt_setprop(blob, node, "local-mac-address", mac_addr, 6);
> > + if (ret) {
> > + printf("Error setting local-mac-address property\n");
> > + return -ENODEV;
> > + }
> > +
> > + icicle_mac_addr[0] = '[';
> > +
> > + sprintf(&icicle_mac_addr[1], "%pM", mac_addr);
> > +
> > + icicle_mac_addr[18] = ']';
> > + icicle_mac_addr[19] = '\0';
> > +
> > + for (idx = 0; idx < 20; idx++) {
> > + if (icicle_mac_addr[idx] == ':')
> > + icicle_mac_addr[idx] = ' ';
> > + }
> > + env_set("icicle_mac_addr", icicle_mac_addr);
>
> What is this environment variable used for? I think we should set up
> the "ethaddr" variable, like:
>
> eth_env_set_enetaddr("ethaddr", mac_addr);
>
>
The "icicle_mac_addr" environment variable is used for updating the mac
address in the Linux device tree.
The "ethaddr" variable is set with updated 'local-mac-address' of the
device tree so no need to set it with eth_env_set_enetaddr() function.
> +
> > + return 0;
> > +}
> > diff --git a/configs/microchip_mpfs_icicle_defconfig
> b/configs/microchip_mpfs_icicle_defconfig
> > index 2977966473..9abbc18a89 100644
> > --- a/configs/microchip_mpfs_icicle_defconfig
> > +++ b/configs/microchip_mpfs_icicle_defconfig
> > @@ -1,12 +1,15 @@
> > CONFIG_RISCV=y
> > CONFIG_ENV_SIZE=0x2000
> > CONFIG_TARGET_MICROCHIP_ICICLE=y
> > -CONFIG_NR_CPUS=5
> > CONFIG_ARCH_RV64I=y
> > +CONFIG_RISCV_SMODE=y
> > +CONFIG_SBI_V01=y
>
> Why is SBI ver 0.1 used? Can we use SBI ver 0.2?
>
>
The HSS (*Hart Software Services*) is using SBI ver 0.1 now, we will use
SBI ver 0.2 in HSS in future.
Regards
Padmarao
> > +CONFIG_DEFAULT_DEVICE_TREE="microchip-mpfs-icicle-kit"
> > +CONFIG_DISTRO_DEFAULTS=y
> > +CONFIG_DISPLAY_CPUINFO=y
> > +CONFIG_DISPLAY_BOARDINFO=y
> > CONFIG_FIT=y
> > -CONFIG_BOOTDELAY=3
> > CONFIG_SYS_PROMPT="RISC-V # "
> > -CONFIG_OF_PRIOR_STAGE=y
> > CONFIG_SYS_RELOC_GD_ENV_ADDR=y
> > CONFIG_BOOTP_SEND_HOSTNAME=y
> > CONFIG_DM_MTD=y
> > diff --git a/include/configs/microchip_mpfs_icicle.h
> b/include/configs/microchip_mpfs_icicle.h
> > index 8a7470545b..97547057b9 100644
> > --- a/include/configs/microchip_mpfs_icicle.h
> > +++ b/include/configs/microchip_mpfs_icicle.h
> > @@ -7,53 +7,35 @@
> > #ifndef __CONFIG_H
> > #define __CONFIG_H
> >
> > -/*
> > - * CPU and Board Configuration Options
> > - */
> > +#include <linux/sizes.h>
> >
> > -/*
> > - * Miscellaneous configurable options
> > - */
> > -#define CONFIG_SYS_CBSIZE 1024 /* Console I/O Buffer Size */
> > +#define CONFIG_SYS_SDRAM_BASE 0x80000000
> > +#define CONFIG_SYS_INIT_SP_ADDR (CONFIG_SYS_SDRAM_BASE + SZ_2M)
> >
> > -/*
> > - * Print Buffer Size
> > - */
> > -#define CONFIG_SYS_PBSIZE \
> > - (CONFIG_SYS_CBSIZE + sizeof(CONFIG_SYS_PROMPT) + 16)
> > +#define CONFIG_SYS_LOAD_ADDR (CONFIG_SYS_SDRAM_BASE + SZ_2M)
> >
> > -/*
> > - * max number of command args
> > - */
> > -#define CONFIG_SYS_MAXARGS 16
> > +#define CONFIG_SYS_MALLOC_LEN SZ_8M
> >
> > -/*
> > - * Boot Argument Buffer Size
> > - */
> > -#define CONFIG_SYS_BARGSIZE CONFIG_SYS_CBSIZE
> > -
> > -/*
> > - * Size of malloc() pool
> > - * 512kB is suggested, (CONFIG_ENV_SIZE + 128 * 1024) was not enough
> > - */
> > -#define CONFIG_SYS_MALLOC_LEN (512 << 10)
> > +#define CONFIG_SYS_BOOTM_LEN SZ_64M
> >
> > -/*
> > - * Physical Memory Map
> > - */
> > -#define PHYS_SDRAM_0 0x80000000 /* SDRAM Bank #1 */
> > -#define PHYS_SDRAM_0_SIZE 0x40000000 /* 1 GB */
> > -#define CONFIG_SYS_SDRAM_BASE PHYS_SDRAM_0
> > +#define CONFIG_STANDALONE_LOAD_ADDR 0x80200000
> >
> > -/* Init Stack Pointer */
> > -#define CONFIG_SYS_INIT_SP_ADDR (CONFIG_SYS_SDRAM_BASE +
> 0x200000)
> > +/* Environment options */
> >
> > -#define CONFIG_SYS_LOAD_ADDR 0x80000000 /* SDRAM */
> > +#define BOOT_TARGET_DEVICES(func) \
> > + func(MMC, mmc, 0) \
> > + func(DHCP, dhcp, na)
> >
> > -/*
> > - * memtest works on DRAM
> > - */
> > +#include <config_distro_bootcmd.h>
> >
> > -/* When we use RAM as ENV */
> > +#define CONFIG_EXTRA_ENV_SETTINGS \
> > + "fdt_high=0xffffffffffffffff\0" \
> > + "initrd_high=0xffffffffffffffff\0" \
> > + "kernel_addr_r=0x84000000\0" \
> > + "fdt_addr_r=0x88000000\0" \
> > + "scriptaddr=0x88100000\0" \
> > + "pxefile_addr_r=0x88200000\0" \
> > + "ramdisk_addr_r=0x88300000\0" \
> > + BOOTENV
> >
> > #endif /* __CONFIG_H */
>
> Regards,
> Bin
>
