[v2 04/17] arm: socfpga: Add handoff data support for Intel N5X device
Chee, Tien Fong
tien.fong.chee at intel.com
Mon May 31 13:39:14 CEST 2021
Hi Ley Foon,
> -----Original Message-----
> From: Ley Foon Tan <lftan.linux at gmail.com>
> Sent: Friday, 14 May, 2021 5:13 PM
> To: Lim, Elly Siew Chin <elly.siew.chin.lim at intel.com>
> Cc: ZY - u-boot <u-boot at lists.denx.de>; Marek Vasut <marex at denx.de>;
> Tan, Ley Foon <ley.foon.tan at intel.com>; See, Chin Liang
> <chin.liang.see at intel.com>; Simon Goldschmidt
> <simon.k.r.goldschmidt at gmail.com>; Chee, Tien Fong
> <tien.fong.chee at intel.com>; Westergreen, Dalon
> <dalon.westergreen at intel.com>; Simon Glass <sjg at chromium.org>; Gan,
> Yau Wai <yau.wai.gan at intel.com>
> Subject: Re: [v2 04/17] arm: socfpga: Add handoff data support for Intel N5X
> device
>
> On Fri, Apr 30, 2021 at 3:39 PM Siew Chin Lim <elly.siew.chin.lim at intel.com>
> wrote:
> >
> > N5X support both HPS handoff data and DDR handoff data.
> > Existing HPS handoff functions are restructured to support both
> > existing devices and N5X device.
> >
> > Signed-off-by: Siew Chin Lim <elly.siew.chin.lim at intel.com>
> > Signed-off-by: Tien Fong Chee <tien.fong.chee at intel.com>
> >
> > ---
> > v2:
> > - Enabled auto detect the endianness from the magic word
> > - Merged and simplifying the big and little endian flow
> > ---
> > .../mach-socfpga/include/mach/handoff_soc64.h | 38 +++++-
> > arch/arm/mach-socfpga/system_manager_soc64.c | 18 +--
> > arch/arm/mach-socfpga/wrap_handoff_soc64.c | 126 +++++++++++++--
> ---
> > 3 files changed, 136 insertions(+), 46 deletions(-)
>
> [...]
>
> >
> > @@ -10,12 +10,54 @@
> > #include <errno.h>
> > #include "log.h"
> >
> > -int socfpga_get_handoff_size(void *handoff_address, enum endianness
> > endian)
> > +static enum endianness check_endianness(u32 handoff) {
> > + switch (handoff) {
> > + case SOC64_HANDOFF_MAGIC_BOOT:
> > + case SOC64_HANDOFF_MAGIC_MUX:
> > + case SOC64_HANDOFF_MAGIC_IOCTL:
> > + case SOC64_HANDOFF_MAGIC_FPGA:
> > + case SOC64_HANDOFF_MAGIC_DELAY:
> > + case SOC64_HANDOFF_MAGIC_CLOCK:
> > + case SOC64_HANDOFF_MAGIC_MISC:
> > + return BIG_ENDIAN;
> > +#if IS_ENABLED(CONFIG_TARGET_SOCFPGA_N5X)
> > + case SOC64_HANDOFF_DDR_UMCTL2_MAGIC:
> > + debug("%s: umctl2 handoff data\n", __func__);
> > + return LITTLE_ENDIAN;
> > + case SOC64_HANDOFF_DDR_PHY_MAGIC:
> > + debug("%s: PHY handoff data\n", __func__);
> > + return LITTLE_ENDIAN;
> > + case SOC64_HANDOFF_DDR_PHY_INIT_ENGINE_MAGIC:
> > + debug("%s: PHY engine handoff data\n", __func__);
> > + return LITTLE_ENDIAN;
> Can merge to one 'return' and print the 'handoff' if needed.
I can merge to one 'return' but has to compromise accuracy of DDR handoff type debug print out. So, I don’t see any benefit of doing these.
Do you have any suggestion?
>
> > +#endif
> > + default:
> > + debug("%s: Unknown endianness!!\n", __func__);
> > + return UNKNOWN_ENDIANNESS;
> > + }
> > +}
> > +
> > +int socfpga_get_handoff_size(void *handoff_address)
> > {
> > u32 size;
> > + enum endianness endian_t;
> > +
> > + /* Checking handoff data is little endian ? */
> > + endian_t = check_endianness(readl(handoff_address));
> > +
> > + if (endian_t == UNKNOWN_ENDIANNESS) {
> > + /* Trying to check handoff data is big endian? */
> > + endian_t = check_endianness(swab32(readl(handoff_address)));
> > + if (endian_t == UNKNOWN_ENDIANNESS) {
> > + debug("%s: Cannot find HANDOFF MAGIC ", __func__);
> > + debug("at addr 0x%p\n", (u32 *)handoff_address);
> > + return -EPERM;
> > + }
> > + }
> >
> > size = readl(handoff_address + SOC64_HANDOFF_OFFSET_LENGTH);
> > - if (endian == BIG_ENDIAN)
> > + if (endian_t == BIG_ENDIAN)
> > size = swab32(size);
> >
> > size = (size - SOC64_HANDOFF_OFFSET_DATA) / sizeof(u32); @@
> > -26,41 +68,61 @@ int socfpga_get_handoff_size(void *handoff_address,
> enum endianness endian)
> > return size;
> > }
> >
> > -int socfpga_handoff_read(void *handoff_address, void *table, u32
> table_len,
> > - enum endianness big_endian)
> > +int socfpga_handoff_read(void *handoff_address, void *table, u32
> > +table_len)
> > {
> > - u32 temp, i;
> > + u32 temp;
> > u32 *table_x32 = table;
> > + u32 i = 0;
> > + enum endianness endian_t;
> > +
> > + /* Checking handoff data is little endian ? */
> > + endian_t = check_endianness(readl(handoff_address));
> This code is similar in socfpga_get_handoff_size(). Can have a function to get
> the endianness.
Okay.
>
> >
> > - 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);
> > + if (endian_t == UNKNOWN_ENDIANNESS) {
> > + /* Trying to check handoff data is big endian? */
> > + endian_t = check_endianness(swab32(readl(handoff_address)));
> > + if (endian_t == UNKNOWN_ENDIANNESS) {
> > + debug("%s: Cannot find HANDOFF MAGIC ", __func__);
> > + debug("at addr 0x%p\n", (u32
> > + *)handoff_address);
> > return -EPERM;
> > }
> > }
> >
> > + temp = readl(handoff_address + SOC64_HANDOFF_OFFSET_DATA +
> > + (i * sizeof(u32)));
> When this can't be done in the 'for' loop below?
The first iteration is moved out from below loop for supporting correct debug message format and also to avoid reading the same thing twice.
>
> > +
> > + if (endian_t == BIG_ENDIAN) {
> > + debug("%s: Handoff addr = 0x%p ", __func__, (u32
> *)handoff_address);
> > + debug("Handoff table address = 0x%p ", table_x32);
> > + debug("table length = 0x%x\n", table_len);
> > + debug("%s: handoff data =\n{\n", __func__);
> > + *table_x32 = swab32(temp);
> > + } else if (endian_t == LITTLE_ENDIAN) {
> > + debug(" {\n");
> > + *table_x32 = temp;
> > + }
> > +
> > + debug(" No.%d Addr 0x%08x: ", i, *table_x32);
> > +
> > + for (i = 1; i < table_len; i++) {
> > + table_x32++;
> > +
> > + temp = readl(handoff_address +
> > + SOC64_HANDOFF_OFFSET_DATA +
> > + (i * sizeof(u32)));
> > +
> > + if (endian_t == BIG_ENDIAN)
> > + *table_x32 = swab32(temp);
> > + else if (endian_t == LITTLE_ENDIAN)
> > + *table_x32 = temp;
> > +
> > + if (!(i % 2))
> > + debug(" No.%d Addr 0x%08x: ", i,
> > + *table_x32);
> > + else
> > + debug(" 0x%08x\n", *table_x32);
> > + }
> > + debug("\n}\n");
> > +
> > return 0;
> > }
>
> Regards
> Ley Foon
More information about the U-Boot
mailing list