Hello Mamta,

> -----Original Message-----
> From: Mamta Shukla <mamta.shu...@leica-geosystems.com>
> Sent: Thursday, July 7, 2022 2:45 PM
> To: u-boot@lists.denx.de
> Cc: sba...@denx.de; peng....@nxp.com; aford...@gmail.com; GEO-CHHER-bsp-
> development <bsp-development....@leica-geosystems.com>; feste...@denx.de; 
> SHUKLA
> Mamta Ramendra <mamta.shu...@leica-geosystems.com>; HAEMMERLE Thomas
> <thomas.haemme...@leica-geosystems.com>
> Subject: [PATCH v4 1/7] tools: mkimage: Add support to generate FlexSPI Header
> for i.MX8m
> 
> Add struct with Flex SPI Configuration Block and enable generating
> fspi header using mkimage.
> 
> Refer i.MX 8M Mini Application Processor Reference Manual for
> detailed information about parameters for FlexSPI Configuration block.
> 
> Signed-off-by: Mamta Shukla <mamta.shu...@leica-geosystems.com>
> Signed-off-by: Thomas Haemmerle <thomas.haemme...@leica-geosystems.com>
> Tested-by: Adam Ford <aford...@gmail.com>
> Reviewed-by: Fabio Estevam <feste...@denx.de>
> ---
> v2:
> -Add check for error in case open() for fspi_fd in imx8mkimage.c fails
> 
> v3:
> -No changes
> 
> v4:
> -No changes
> 
>  include/imximage.h | 38 ++++++++++++++++++++++
>  tools/Kconfig      | 59 +++++++++++++++++++++++++++++++++
>  tools/imx8mimage.c | 81 +++++++++++++++++++++++++++++++++++++++++++---
>  3 files changed, 174 insertions(+), 4 deletions(-)
> 
> diff --git a/include/imximage.h b/include/imximage.h
> index 5a812f5a10..c1ecc0b7cb 100644
> --- a/include/imximage.h
> +++ b/include/imximage.h
> @@ -201,6 +201,44 @@ struct imx_header {
>       } header;
>  };
> 
> +typedef struct {
> +     uint8_t tag[4];
> +     uint8_t version[4];
> +     uint8_t reserved_1[4];
> +     uint8_t read_sample;
> +     uint8_t datahold;
> +     uint8_t datasetup;
> +     uint8_t coladdrwidth;
> +     uint8_t devcfgenable;
> +     uint8_t reserved_2[3];
> +     uint8_t devmodeseq[4];
> +     uint8_t devmodearg[4];
> +     uint8_t cmd_enable;
> +     uint8_t reserved_3[3];
> +     uint8_t cmd_seq[16] ;
> +     uint8_t cmd_arg[16];
> +     uint8_t controllermisc[4];
> +     uint8_t dev_type;
> +     uint8_t sflash_pad;
> +     uint8_t serial_clk;
> +     uint8_t lut_custom ;
> +     uint8_t reserved_4[8];
> +     uint8_t sflashA1[4];
> +     uint8_t sflashA2[4];
> +     uint8_t sflashB1[4];
> +     uint8_t sflashB2[4];
> +     uint8_t cspadover[4];
> +     uint8_t sclkpadover[4];
> +     uint8_t datapadover[4];
> +     uint8_t dqspadover[4];
> +     uint8_t timeout[4];
> +     uint8_t commandInt[4];
> +     uint8_t datavalid[4];
> +     uint8_t busyoffset[2];
> +     uint8_t busybitpolarity[2];
> +     uint8_t lut[256];
> +} __attribute__((packed)) fspi_conf;
> +
>  typedef void (*set_dcd_val_t)(struct imx_header *imxhdr,
>                                       char *name, int lineno,
>                                       int fld, uint32_t value,
> diff --git a/tools/Kconfig b/tools/Kconfig
> index 117c921da3..539708f277 100644
> --- a/tools/Kconfig
> +++ b/tools/Kconfig
> @@ -98,4 +98,63 @@ config TOOLS_MKEFICAPSULE
>         optionally sign that file. If you want to enable UEFI capsule
>         update feature on your target, you certainly need this.
> 
> +menuconfig FSPI_CONF_HEADER
> +     bool "FlexSPI Header Configuration"
> +     help
> +       FSPI Header Configuration
> +
> +config FSPI_CONF_FILE
> +     string "FlexSPI Header File"
> +     depends on FSPI_CONF_HEADER
> +     help
> +       FlexSPI Header File name
> +
> +config READ_CLK_SOURCE
> +     hex "Sampling Clock Source"
> +     default 0x00
> +     depends on FSPI_CONF_HEADER
> +     help
> +       Sample Clock source for Flash, default is internal loopback clock
> +
> +config DEVICE_TYPE
> +     hex "Flash Type"
> +     default 0x01
> +     depends on FSPI_CONF_HEADER
> +     help
> +       Flash type: Serial NOR (0X01) and Serial NAND (0x02)
> +
> +config FLASH_PAD_TYPE
> +     hex "Flash Pad Type"
> +     default 0x01
> +     depends on FSPI_CONF_HEADER
> +     help
> +       Flash Pad type :
> +       Single Pad 0x01
> +       Dual Pads  0x02
> +       Quad Pad   0x04
> +       Octal Pad  0x08
> +
> +config SERIAL_CLK_FREQUENCY
> +     hex "Serial Clock Frequency"
> +     default 0x02
> +     depends on FSPI_CONF_HEADER
> +     help
> +       Chip specific frequency: other value 30MHz
> +       1-30MHz  2-50MHz 3-60MHz 4-75MHz 5-80MHz 6-100MHz 7-133MHz 8-166MHz
> +
> +config LUT_CUSTOM_SEQUENCE
> +     hex "Enable Custom Look Up Table(LUT) Sequence"
> +     default 0x00
> +     depends on FSPI_CONF_HEADER
> +     help
> +       0 - Use predefined LUT Sequence
> +       1 - Use LUT Sequence provided
> +
> +config LUT_SEQUENCE
> +     string "Look Up Table Sequence"
> +     default "0x0b, 0x04, 0x18, 0x08, 0x08, 0x30, 0x04, 0x24"
> +     depends on FSPI_CONF_HEADER
> +     help
> +       Look Up Table Sequence
> +
>  endmenu
> diff --git a/tools/imx8mimage.c b/tools/imx8mimage.c
> index 4eed683396..facf8887a1 100644
> --- a/tools/imx8mimage.c
> +++ b/tools/imx8mimage.c
> @@ -12,7 +12,7 @@
>  #include "compiler.h"
> 
>  static uint32_t ap_start_addr, sld_start_addr, sld_src_off;
> -static char *ap_img, *sld_img, *signed_hdmi;
> +static char *ap_img, *sld_img, *signed_hdmi, *fspi;
>  static imx_header_v3_t imx_header[2]; /* At most there are 3 IVT headers */
>  static uint32_t rom_image_offset;
>  static uint32_t sector_size = 0x200;
> @@ -120,7 +120,6 @@ static void parse_cfg_cmd(int32_t cmd, char *token, char
> *name, int lineno)
>                       rom_version = ROM_V1;
>               }
>               break;
> -
>       }
>  }
> 
> @@ -412,10 +411,70 @@ static void dump_header_v2(imx_header_v3_t *imx_header, 
> int
> index)
>               imx_header[index].boot_data.plugin);
>  }
> 
> +#ifdef CONFIG_FSPI_CONF_HEADER
> +static int generate_fspi_header (int ifd)
> +{
> +     int ret, i = 0;
> +     char *val;
> +     char lut_str[] = CONFIG_LUT_SEQUENCE;
> +
> +     fspi_conf fspi_conf_data = {
> +     .tag = {0x46, 0x43, 0x46, 0x42},
> +     .version = {0x00, 0x00, 0x01, 0x56},
> +     .reserved_1 = {0x00, 0x00, 0x00, 0x00},
> +     .read_sample = CONFIG_READ_CLK_SOURCE,
> +     .datahold =  {0x03},
> +     .datasetup = {0x03},
> +     .coladdrwidth = {0x00},
> +     .devcfgenable = {0x00},
> +     .reserved_2 = {0x00, 0x00, 0x00},
> +     .devmodeseq =  {0x00, 0x00, 0x00, 0x00},
> +     .devmodearg =  {0x00, 0x00, 0x00, 0x00},
> +     .cmd_enable =  {0x00},
> +     .reserved_3 = {0x00},
> +     .cmd_seq = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +                                                     0x00, 0x00, 0x00, 0x00, 
> 0x00,
> 0x00},
> +     .cmd_arg = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +                                                     0x00, 0x00, 0x00, 0x00, 
> 0x00,
> 0x00},
> +     .controllermisc = {0x00, 0x00, 0x00, 0x00},
> +     .dev_type = CONFIG_DEVICE_TYPE,
> +     .sflash_pad = CONFIG_FLASH_PAD_TYPE,
> +     .serial_clk = CONFIG_SERIAL_CLK_FREQUENCY,
> +     .lut_custom = CONFIG_LUT_CUSTOM_SEQUENCE,
> +     .reserved_4 = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
> +     .sflashA1  =  {0x00, 0x00, 0x00, 0x10},
> +     .sflashA2 = {0x00, 0x00, 0x00, 0x00},
> +     .sflashB1 = {0x00, 0x00, 0x00, 0x00},
> +     .sflashB2 =  {0x00, 0x00, 0x00, 0x00},
> +     .cspadover = {0x00, 0x00, 0x00, 0x00},
> +     .sclkpadover = {0x00, 0x00, 0x00, 0x00},
> +     .datapadover = {0x00, 0x00, 0x00, 0x00},
> +     .dqspadover = {0x00, 0x00, 0x00, 0x00},
> +     .timeout =  {0x00, 0x00, 0x00, 0x00},
> +     .commandInt = {0x00, 0x00, 0x00, 0x00},
> +     .datavalid  = {0x00, 0x00, 0x00, 0x00},
> +     .busyoffset = {0x00, 0x00},
> +     .busybitpolarity = {0x00, 0x00},

For some variables, GCC throws following warnings:

u-boot-master/tools/imx8mimage.c: In function ‘generate_fspi_header’:
u-boot-master/tools/imx8mimage.c:426:2: warning: braces around scalar 
initializer
  426 |  .datahold =  {0x03},
      |  ^
u-boot-master/tools/imx8mimage.c:426:2: note: (near initialization for 
‘fspi_conf_data.datahold’)
u-boot-master/tools/imx8mimage.c:427:2: warning: braces around scalar 
initializer
  427 |  .datasetup = {0x03},
      |  ^
u-boot-master/tools/imx8mimage.c:427:2: note: (near initialization for 
‘fspi_conf_data.datasetup’)
u-boot-master/tools/imx8mimage.c:428:2: warning: braces around scalar 
initializer
  428 |  .coladdrwidth = {0x00},
      |  ^
u-boot-master/tools/imx8mimage.c:428:2: note: (near initialization for 
‘fspi_conf_data.coladdrwidth’)
u-boot-master/tools/imx8mimage.c:429:2: warning: braces around scalar 
initializer
  429 |  .devcfgenable = {0x00},
      |  ^
u-boot-master/tools/imx8mimage.c:429:2: note: (near initialization for 
‘fspi_conf_data.devcfgenable’)
u-boot-master/tools/imx8mimage.c:433:2: warning: braces around scalar 
initializer
  433 |  .cmd_enable =  {0x00},
      |  ^
u-boot-master/tools/imx8mimage.c:433:2: note: (near initialization for 
‘fspi_conf_data.cmd_enable’)

Since above reported variables are indeed declared as scalars above, you might 
want to drop the usage of braces.

> +     };
> +
> +     for (val = strtok(lut_str, ","); val; val = strtok(NULL, ",")) {
> +             fspi_conf_data.lut[i++] = strtoul(val, NULL, 16);
> +     }
> +
> +     ret = lseek(ifd, 0, SEEK_CUR);
> +     if (write(ifd, &fspi_conf_data, sizeof(fspi_conf_data)) == -1)
> +             exit(EXIT_FAILURE);
> +
> +     ret = lseek(ifd, sizeof(fspi_conf_data), SEEK_CUR);
> +
> +     return ret;
> +}
> +#endif
> +
>  void build_image(int ofd)
>  {
> -     int file_off, header_hdmi_off = 0, header_image_off;
> -     int hdmi_fd, ap_fd, sld_fd;
> +     int file_off, header_hdmi_off = 0, header_image_off, fspi_off;
> +     int hdmi_fd, ap_fd, sld_fd, fspi_fd;
>       uint32_t sld_load_addr = 0;
>       uint32_t csf_off, sld_csf_off = 0;
>       int ret;
> @@ -455,6 +514,20 @@ void build_image(int ofd)
> 
>       header_image_off = file_off + ivt_offset;
> 
> +#ifdef CONFIG_FSPI_CONF_HEADER
> +     fspi = CONFIG_FSPI_CONF_FILE;
> +     fspi_fd = open(fspi, O_RDWR | O_CREAT, S_IRWXU);
> +     if (fspi_fd < 0) {
> +             fprintf(stderr, "Can't open %s: %s\n",
> +                     fspi, strerror(errno));
> +             exit(EXIT_FAILURE);
> +     }
> +
> +     fspi_off = generate_fspi_header(fspi_fd);
> +     file_off = header_image_off + fspi_off;
> +     close(fspi_fd);
> +
> +#endif
>       ap_fd = open(ap_img, O_RDONLY | O_BINARY);
>       if (ap_fd < 0) {
>               fprintf(stderr, "%s: Can't open: %s\n",
> --
> 2.25.1

Reviewed-by: Andrey Zhizhikin <andrey.zhizhi...@leica-geosystems.com>

Reply via email to