Re: [U-Boot] [PATCH] ARM: cfi_flash: Fix unaligned accesses to cfi_qry structure

From: Gabbasov, Andrew Sent: Monday, April 29, 2013 13:34 To: u-boot@lists.denx.de Subject: [U-Boot][PATCH] ARM: cfi_flash: Fix unaligned accesses to cfi_qry structure

Packed structure cfi_qry contains unaligned 16- and 32-bits members, accessing which causes problems when cfi_flash driver is compiled with -munaligned-access option: flash initialization hangs, probably due to data error.

This fix converts 16- and 32-bit members to byte arrays and uses special macros to access such fields. It removes possible unaligned accesses in cfi_flash driver.

Signed-off-by: Andrew Gabbasov andrew_gabbasov@mentor.com

drivers/mtd/cfi_flash.c | 23 +++++++++++++---------- include/mtd/cfi_flash.h | 18 +++++++++++------- 2 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c index 60dbb78..2112ffc 100644 --- a/drivers/mtd/cfi_flash.c +++ b/drivers/mtd/cfi_flash.c @@ -1636,13 +1636,16 @@ void flash_read_factory_serial (flash_info_t * info, void *buffer, int offset, */ static void cfi_reverse_geometry(struct cfi_qry *qry) {

•   unsigned int i, j;
  

•   u32 tmp;
  

•   unsigned int i, j, k;
  

•   u8 tmp;
  
    for (i = 0, j = qry->num_erase_regions - 1; i < j; i++, j--) {
  

•           tmp = qry->erase_region_info[i];
  

•           qry->erase_region_info[i] = qry->erase_region_info[j];
  

•           qry->erase_region_info[j] = tmp;
  

•           for (k = 0; k < 4; k++) {
  

•                   tmp = qry->erase_region_info[k][i];
  

•                   qry->erase_region_info[k][i] =
  

•                           qry->erase_region_info[k][j];
  

•                   qry->erase_region_info[k][j] = tmp;
  

•           }
    }
  

}

@@ -1891,7 +1894,7 @@ static int __flash_detect_cfi (flash_info_t * info, struct cfi_qry *qry) && flash_isequal (info, 0, FLASH_OFFSET_CFI_RESP + 2, 'Y')) { flash_read_cfi(info, qry, FLASH_OFFSET_CFI_RESP, sizeof(struct cfi_qry));

•                   info->interface = le16_to_cpu(qry->interface_desc);
  

•                   info->interface = cfiqry_get16(qry->interface_desc);
  
                    info->cfi_offset = flash_offset_cfi[cfi_offset];
                    debug ("device interface is %d\n",
  

@@ -2053,8 +2056,8 @@ ulong flash_get_size (phys_addr_t base, int banknum) info->start[0] = (ulong)map_physmem(base, info->portwidth, MAP_NOCACHE);

    if (flash_detect_cfi (info, &qry)) {

•           info->vendor = le16_to_cpu(qry.p_id);
  

•           info->ext_addr = le16_to_cpu(qry.p_adr);
  

•           info->vendor = cfiqry_get16(qry.p_id);
  

•           info->ext_addr = cfiqry_get16(qry.p_adr);
            num_erase_regions = qry.num_erase_regions;
  
            if (info->ext_addr) {
  

@@ -2140,7 +2143,7 @@ ulong flash_get_size (phys_addr_t base, int banknum) break; }

•                   tmp = le32_to_cpu(qry.erase_region_info[i]);
  

•                   tmp = cfiqry_get32(qry.erase_region_info[i]);
                    debug("erase region %u: 0x%08lx\n", i, tmp);
  
                    erase_region_count = (tmp & 0xffff) + 1;
  

@@ -2213,7 +2216,7 @@ ulong flash_get_size (phys_addr_t base, int banknum) }

            info->sector_count = sect_cnt;

•           info->buffer_size = 1 << le16_to_cpu(qry.max_buf_write_size);
  

•           info->buffer_size = 1 << cfiqry_get16(qry.max_buf_write_size);
            tmp = 1 << qry.block_erase_timeout_typ;
            info->erase_blk_tout = tmp *
                    (1 << qry.block_erase_timeout_max);
  

diff --git a/include/mtd/cfi_flash.h b/include/mtd/cfi_flash.h index 966b5e0..30d6896 100644 --- a/include/mtd/cfi_flash.h +++ b/include/mtd/cfi_flash.h @@ -131,10 +131,10 @@ typedef union { /* CFI standard query structure */ struct cfi_qry { u8 qry[3];

•   u16     p_id;
  

•   u16     p_adr;
  

•   u16     a_id;
  

•   u16     a_adr;
  

•   u8      p_id[2];
  

•   u8      p_adr[2];
  

•   u8      a_id[2];
  

•   u8      a_adr[2];
    u8      vcc_min;
    u8      vcc_max;
    u8      vpp_min;
  

@@ -148,10 +148,10 @@ struct cfi_qry { u8 block_erase_timeout_max; u8 chip_erase_timeout_max; u8 dev_size;

•   u16     interface_desc;
  

•   u16     max_buf_write_size;
  

•   u8      interface_desc[2];
  

•   u8      max_buf_write_size[2];
    u8      num_erase_regions;
  

•   u32     erase_region_info[NUM_ERASE_REGIONS];
  

•   u8      erase_region_info[4][NUM_ERASE_REGIONS];
  

} __attribute__((packed));

struct cfi_pri_hdr { @@ -160,6 +160,10 @@ struct cfi_pri_hdr { u8 minor_version; } __attribute__((packed));

+#define cfiqry_get16(mp) ((u16)((mp)[0]) | ((u16)((mp)[1]) << 8)) +#define cfiqry_get32(mp) ((u32)((mp)[0]) | ((u32)((mp)[1]) << 8) | \

•                     ((u32)((mp)[2]) << 16) | ((u32)((mp)[3]) << 24))
  

#ifndef CONFIG_SYS_FLASH_BANKS_LIST #define CONFIG_SYS_FLASH_BANKS_LIST { CONFIG_SYS_FLASH_BASE }

#endif

1.7.10.4

Unfortunately, I can't verify this fix for the boards with flash configuration, requiring reversing geometry. So, if anybody could test it on such board, I would greatly appreciate it.

Thanks.

Best regards, Andrew