Hi Ley Foon,
> -----Original Message-----
> From: Tan, Ley Foon <ley.foon....@intel.com>
> Sent: Tuesday, March 23, 2021 6:04 PM
> To: Lim, Elly Siew Chin <elly.siew.chin....@intel.com>; u-boot@lists.denx.de
> Cc: Marek Vasut <ma...@denx.de>; See, Chin Liang
> <chin.liang....@intel.com>; Simon Goldschmidt
> <simon.k.r.goldschm...@gmail.com>; Chee, Tien Fong
> <tien.fong.c...@intel.com>; Westergreen, Dalon
> <dalon.westergr...@intel.com>; Simon Glass <s...@chromium.org>; Gan,
> Yau Wai <yau.wai....@intel.com>
> Subject: RE: [v1 5/5] arm: socfpga: Restructure Stratix10 and Agilex handoff
> code
>
>
>
> > -----Original Message-----
> > From: Lim, Elly Siew Chin <elly.siew.chin....@intel.com>
> > Sent: Monday, March 15, 2021 5:43 PM
> > To: u-boot@lists.denx.de
> > Cc: Marek Vasut <ma...@denx.de>; Tan, Ley Foon
> > <ley.foon....@intel.com>; See, Chin Liang <chin.liang....@intel.com>;
> > Simon Goldschmidt <simon.k.r.goldschm...@gmail.com>; Chee, Tien Fong
> > <tien.fong.c...@intel.com>; Westergreen, Dalon
> > <dalon.westergr...@intel.com>; Simon Glass <s...@chromium.org>; Gan,
> > Yau Wai <yau.wai....@intel.com>; Lim, Elly Siew Chin
> > <elly.siew.chin....@intel.com>
> > Subject: [v1 5/5] arm: socfpga: Restructure Stratix10 and Agilex
> > handoff code
> >
> > Restructure Stratix10 and Agilex handoff code to used by all SOC64
> > devices, in preparation to support handoff for Diamond Mesa.
> >
> > Remove wrap_pinmux_config_s10.c. Add wrap_handoff_soc64.c which
> > contains the generic function to parse the handoff data.
> >
> > Update system_manager_soc64.c to use generic handoff function in
> > wrap_handoff_soc64.c.
> >
> > Signed-off-by: Siew Chin Lim <elly.siew.chin....@intel.com>
> > ---
> > arch/arm/mach-socfpga/Makefile | 4 +-
> > arch/arm/mach-socfpga/include/mach/handoff_soc64.h | 21 +++++++
> > .../include/mach/system_manager_soc64.h | 4 --
> > arch/arm/mach-socfpga/system_manager_soc64.c | 53 ++++++++++---
> --
> > -
> > arch/arm/mach-socfpga/wrap_handoff_soc64.c | 73
> > ++++++++++++++++++++++
> >
> > diff --git a/arch/arm/mach-socfpga/include/mach/handoff_soc64.h
> > b/arch/arm/mach-socfpga/include/mach/handoff_soc64.h
> > index 2561255712..f4c03688d6 100644
> > --- a/arch/arm/mach-socfpga/include/mach/handoff_soc64.h
> > +++ b/arch/arm/mach-socfpga/include/mach/handoff_soc64.h
> > @@ -11,6 +11,7 @@
> > * Offset for HW handoff from Quartus tools
> > */
> > /* HPS handoff */
> > +#define SOC64_HANDOFF_MAGIC_BOOT 0x424F4F54
> > #define SOC64_HANDOFF_MAGIC_MUX 0x504D5558
> > #define SOC64_HANDOFF_MAGIC_IOCTL 0x494F4354
> > #define SOC64_HANDOFF_MAGIC_FPGA 0x46504741
> > @@ -38,4 +39,24 @@
> > #define SOC64_HANDOFF_CLOCK_FPGA (SOC64_HANDOFF_BASE +
> > 0x600)
> > #endif
> >
> > +#define SOC64_HANDOFF_MUX_LEN 96
> > +#define SOC64_HANDOFF_IOCTL_LEN 96
> > +#ifdef CONFIG_TARGET_SOCFPGA_STRATIX10
> Can change to #if CONFIG_IS_ENABLED()
>
>
> > +#define SOC64_HANDOFF_FPGA_LEN 42
> > +#else
> > +#define SOC64_HANDOFF_FPGA_LEN 40
> > +#endif
> > +#define SOC64_HANDOFF_DELAY_LEN 96
> > +
> > +#ifndef __ASSEMBLY__
> > +#include <asm/types.h>
> > +enum endianness {
> > + little_endian,
> > + big_endian
> > +};
> Uses capital letter for enum macros.
>
> > +
> > +int socfpga_get_handoff_size(void *handoff_address, enum endianness
> > +endian); int socfpga_handoff_read(void *handoff_address, void *table,
> > u32 table_len,
> > + enum endianness big_endian);
> > +#endif
> > #endif /* _HANDOFF_SOC64_H_ */
> > diff --git a/arch/arm/mach-
> socfpga/include/mach/system_manager_soc64.h
> > b/arch/arm/mach-socfpga/include/mach/system_manager_soc64.h
> > index 4949cae97a..1eb8e7a904 100644
> > --- a/arch/arm/mach-socfpga/include/mach/system_manager_soc64.h
> > +++ b/arch/arm/mach-socfpga/include/mach/system_manager_soc64.h
> > @@ -10,10 +10,6 @@
> > void sysmgr_pinmux_init(void);
> > void populate_sysmgr_fpgaintf_module(void);
> > void populate_sysmgr_pinmux(void);
> > -void sysmgr_pinmux_table_sel(const u32 **table, unsigned int
> > *table_len); -void sysmgr_pinmux_table_ctrl(const u32 **table,
> > unsigned int *table_len); -void sysmgr_pinmux_table_fpga(const u32
> > **table, unsigned int *table_len); -void
> > sysmgr_pinmux_table_delay(const u32 **table, unsigned int *table_len);
> >
> > #define SYSMGR_SOC64_WDDBG 0x08
> > #define SYSMGR_SOC64_DMA 0x20
>
> [...]
>
> > diff --git a/arch/arm/mach-socfpga/wrap_handoff_soc64.c
> > b/arch/arm/mach-socfpga/wrap_handoff_soc64.c
> > new file mode 100644
> > index 0000000000..ba168676e9
> > --- /dev/null
> > +++ b/arch/arm/mach-socfpga/wrap_handoff_soc64.c
> > @@ -0,0 +1,73 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2020 Intel Corporation <www.intel.com>
> > + *
> > + */
> > +
> > +#include <common.h>
> > +#include <asm/arch/handoff_soc64.h>
> > +#include <asm/io.h>
> > +#include <errno.h>
> > +#include "log.h"
> Sort this.
>
>
> > +
> > +int socfpga_get_handoff_size(void *handoff_address, enum endianness
> > +endian) {
> > + u32 handoff_size;
> > +
> > + if (endian == little_endian) {
> > + handoff_size = (readl(handoff_address +
> > SOC64_HANDOFF_OFFSET_LENGTH) -
> > + SOC64_HANDOFF_OFFSET_DATA) /
> > + sizeof(u32);
> > + } else if (endian == big_endian) {
> > + handoff_size = swab32(readl(handoff_address +
> > +
> Can merge this, just need do additional swab32 if it is bit endian.
>
>
> [.....]
>
>
> > +
> > +int socfpga_handoff_read(void *handoff_address, void *table, u32
> > table_len,
> > + enum endianness big_endian)
> > +{
> > + u32 temp, i;
> > + u32 *table_x32 = table;
> > +
> > + debug("%s: handoff addr = 0x%p ", __func__, (u32
> > *)handoff_address);
> > +
> > + if (big_endian) {
> > + if (swab32(readl(SOC64_HANDOFF_BASE)) ==
> > SOC64_HANDOFF_MAGIC_BOOT) {
> > + debug("Handoff table address = 0x%p ", table_x32);
> > + debug("table length = 0x%x\n", table_len);
> > + debug("%s: handoff data =\n{\n", __func__);
> > +
> > + for (i = 0; i < table_len; i++) {
> > + temp = readl(handoff_address +
> > + SOC64_HANDOFF_OFFSET_DATA +
> > + (i * sizeof(u32)));
> > + *table_x32 = swab32(temp);
> > +
> > + if (!(i % 2))
> > + debug(" No.%d Addr 0x%08x: ", i,
> > + *table_x32);
> > + else
> > + debug(" 0x%08x\n", *table_x32);
> > +
> > + table_x32++;
> > + }
> > + debug("\n}\n");
> > + } else {
> > + debug("%s: Cannot find
> > SOC64_HANDOFF_MAGIC_BOOT ", __func__);
> > + debug("at addr 0x%p\n", (u32 *)handoff_address);
> > + return -EPERM;
> > + }
> > + }
> How about if it is little endian, why don't need to fill up table_x32?
>
The little endian only needed in new Intel SOC N5X (Diamond Mesa) , we will
upstream this part together with N5X patches later.
And, I will update the code per comment above in next review. Thanks.
>
>
> Regards
> Ley Foon
