* [U-Boot] [PATCH v2] cfi_flash: Fix unaligned accesses to cfi_qry structure
@ 2013-05-14 17:27 Andrew Gabbasov
2013-05-22 18:50 ` Albert ARIBAUD
2013-05-23 7:49 ` Stefan Roese
0 siblings, 2 replies; 4+ messages in thread
From: Andrew Gabbasov @ 2013-05-14 17:27 UTC (permalink / raw)
To: u-boot
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.
Since the structure is supposed to replicate the actual data layout
in CFI Flash chips, the alignment issue can't be fixed in the structure.
So, unaligned fields need using of explicit unaligned access macros.
Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
---
drivers/mtd/cfi_flash.c | 15 +++++++++------
include/mtd/cfi_flash.h | 18 +++++++++++-------
2 files changed, 20 insertions(+), 13 deletions(-)
diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
index 22d8440..f6759a8 100644
--- a/drivers/mtd/cfi_flash.c
+++ b/drivers/mtd/cfi_flash.c
@@ -38,6 +38,7 @@
#include <asm/processor.h>
#include <asm/io.h>
#include <asm/byteorder.h>
+#include <asm/unaligned.h>
#include <environment.h>
#include <mtd/cfi_flash.h>
#include <watchdog.h>
@@ -1640,9 +1641,10 @@ static void cfi_reverse_geometry(struct cfi_qry *qry)
u32 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;
+ tmp = get_unaligned(&(qry->erase_region_info[i]));
+ put_unaligned(get_unaligned(&(qry->erase_region_info[j])),
+ &(qry->erase_region_info[i]));
+ put_unaligned(tmp, &(qry->erase_region_info[j]));
}
}
@@ -2073,8 +2075,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 = le16_to_cpu(get_unaligned(&(qry.p_id)));
+ info->ext_addr = le16_to_cpu(get_unaligned(&(qry.p_adr)));
num_erase_regions = qry.num_erase_regions;
if (info->ext_addr) {
@@ -2163,7 +2165,8 @@ ulong flash_get_size (phys_addr_t base, int banknum)
break;
}
- tmp = le32_to_cpu(qry.erase_region_info[i]);
+ tmp = le32_to_cpu(get_unaligned(
+ &(qry.erase_region_info[i])));
debug("erase region %u: 0x%08lx\n", i, tmp);
erase_region_count = (tmp & 0xffff) + 1;
diff --git a/include/mtd/cfi_flash.h b/include/mtd/cfi_flash.h
index 966b5e0..b644b91 100644
--- a/include/mtd/cfi_flash.h
+++ b/include/mtd/cfi_flash.h
@@ -129,12 +129,16 @@ typedef union {
} cfiword_t;
/* CFI standard query structure */
+/* The offsets and sizes of this packed structure members correspond
+ * to the actual layout in CFI Flash chips. Some 16- and 32-bit members
+ * are unaligned and must be accessed with explicit unaligned access macros.
+ */
struct cfi_qry {
u8 qry[3];
- u16 p_id;
- u16 p_adr;
- u16 a_id;
- u16 a_adr;
+ u16 p_id; /* unaligned */
+ u16 p_adr; /* unaligned */
+ u16 a_id; /* unaligned */
+ u16 a_adr; /* unaligned */
u8 vcc_min;
u8 vcc_max;
u8 vpp_min;
@@ -148,10 +152,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;
+ u16 interface_desc; /* aligned */
+ u16 max_buf_write_size; /* aligned */
u8 num_erase_regions;
- u32 erase_region_info[NUM_ERASE_REGIONS];
+ u32 erase_region_info[NUM_ERASE_REGIONS]; /* unaligned */
} __attribute__((packed));
struct cfi_pri_hdr {
--
1.7.10.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* [U-Boot] [PATCH v2] cfi_flash: Fix unaligned accesses to cfi_qry structure
2013-05-14 17:27 [U-Boot] [PATCH v2] cfi_flash: Fix unaligned accesses to cfi_qry structure Andrew Gabbasov
@ 2013-05-22 18:50 ` Albert ARIBAUD
2013-05-23 6:16 ` Stefan Roese
2013-05-23 7:49 ` Stefan Roese
1 sibling, 1 reply; 4+ messages in thread
From: Albert ARIBAUD @ 2013-05-22 18:50 UTC (permalink / raw)
To: u-boot
Hi Andrew,
On Tue, 14 May 2013 12:27:52 -0500, Andrew Gabbasov
<andrew_gabbasov@mentor.com> wrote:
> 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.
>
> Since the structure is supposed to replicate the actual data layout
> in CFI Flash chips, the alignment issue can't be fixed in the structure.
> So, unaligned fields need using of explicit unaligned access macros.
>
> Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
> ---
> drivers/mtd/cfi_flash.c | 15 +++++++++------
> include/mtd/cfi_flash.h | 18 +++++++++++-------
> 2 files changed, 20 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
> index 22d8440..f6759a8 100644
> --- a/drivers/mtd/cfi_flash.c
> +++ b/drivers/mtd/cfi_flash.c
> @@ -38,6 +38,7 @@
> #include <asm/processor.h>
> #include <asm/io.h>
> #include <asm/byteorder.h>
> +#include <asm/unaligned.h>
> #include <environment.h>
> #include <mtd/cfi_flash.h>
> #include <watchdog.h>
> @@ -1640,9 +1641,10 @@ static void cfi_reverse_geometry(struct cfi_qry *qry)
> u32 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;
> + tmp = get_unaligned(&(qry->erase_region_info[i]));
> + put_unaligned(get_unaligned(&(qry->erase_region_info[j])),
> + &(qry->erase_region_info[i]));
> + put_unaligned(tmp, &(qry->erase_region_info[j]));
> }
> }
>
> @@ -2073,8 +2075,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 = le16_to_cpu(get_unaligned(&(qry.p_id)));
> + info->ext_addr = le16_to_cpu(get_unaligned(&(qry.p_adr)));
> num_erase_regions = qry.num_erase_regions;
>
> if (info->ext_addr) {
> @@ -2163,7 +2165,8 @@ ulong flash_get_size (phys_addr_t base, int banknum)
> break;
> }
>
> - tmp = le32_to_cpu(qry.erase_region_info[i]);
> + tmp = le32_to_cpu(get_unaligned(
> + &(qry.erase_region_info[i])));
> debug("erase region %u: 0x%08lx\n", i, tmp);
>
> erase_region_count = (tmp & 0xffff) + 1;
> diff --git a/include/mtd/cfi_flash.h b/include/mtd/cfi_flash.h
> index 966b5e0..b644b91 100644
> --- a/include/mtd/cfi_flash.h
> +++ b/include/mtd/cfi_flash.h
> @@ -129,12 +129,16 @@ typedef union {
> } cfiword_t;
>
> /* CFI standard query structure */
> +/* The offsets and sizes of this packed structure members correspond
> + * to the actual layout in CFI Flash chips. Some 16- and 32-bit members
> + * are unaligned and must be accessed with explicit unaligned access macros.
> + */
> struct cfi_qry {
> u8 qry[3];
> - u16 p_id;
> - u16 p_adr;
> - u16 a_id;
> - u16 a_adr;
> + u16 p_id; /* unaligned */
> + u16 p_adr; /* unaligned */
> + u16 a_id; /* unaligned */
> + u16 a_adr; /* unaligned */
> u8 vcc_min;
> u8 vcc_max;
> u8 vpp_min;
> @@ -148,10 +152,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;
> + u16 interface_desc; /* aligned */
> + u16 max_buf_write_size; /* aligned */
> u8 num_erase_regions;
> - u32 erase_region_info[NUM_ERASE_REGIONS];
> + u32 erase_region_info[NUM_ERASE_REGIONS]; /* unaligned */
> } __attribute__((packed));
>
> struct cfi_pri_hdr {
Reviewed-By: Albert ARIBAUD <albert.u.boot@aribaud.net>
Seems ok to me.
Now, seeing as this is global to all architectures, yet motivated by
ARM alignment considerations, which repo should this go to?
Amicalement,
--
Albert.
^ permalink raw reply [flat|nested] 4+ messages in thread* [U-Boot] [PATCH v2] cfi_flash: Fix unaligned accesses to cfi_qry structure
2013-05-22 18:50 ` Albert ARIBAUD
@ 2013-05-23 6:16 ` Stefan Roese
0 siblings, 0 replies; 4+ messages in thread
From: Stefan Roese @ 2013-05-23 6:16 UTC (permalink / raw)
To: u-boot
Hi Albert,
On 05/22/2013 08:50 PM, Albert ARIBAUD wrote:
<snip>
>> @@ -148,10 +152,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;
>> + u16 interface_desc; /* aligned */
>> + u16 max_buf_write_size; /* aligned */
>> u8 num_erase_regions;
>> - u32 erase_region_info[NUM_ERASE_REGIONS];
>> + u32 erase_region_info[NUM_ERASE_REGIONS]; /* unaligned */
>> } __attribute__((packed));
>>
>> struct cfi_pri_hdr {
>
> Reviewed-By: Albert ARIBAUD <albert.u.boot@aribaud.net>
>
> Seems ok to me.
>
> Now, seeing as this is global to all architectures, yet motivated by
> ARM alignment considerations, which repo should this go to?
I'm responsible for cfi-flash. I'll look at it today.
Thanks,
Stefan
^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH v2] cfi_flash: Fix unaligned accesses to cfi_qry structure
2013-05-14 17:27 [U-Boot] [PATCH v2] cfi_flash: Fix unaligned accesses to cfi_qry structure Andrew Gabbasov
2013-05-22 18:50 ` Albert ARIBAUD
@ 2013-05-23 7:49 ` Stefan Roese
1 sibling, 0 replies; 4+ messages in thread
From: Stefan Roese @ 2013-05-23 7:49 UTC (permalink / raw)
To: u-boot
On 05/14/2013 07:27 PM, Andrew Gabbasov wrote:
> 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.
>
> Since the structure is supposed to replicate the actual data layout
> in CFI Flash chips, the alignment issue can't be fixed in the structure.
> So, unaligned fields need using of explicit unaligned access macros.
Applied to u-boot-cfi-flash/master.
Thanks,
Stefan
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-05-23 7:49 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-14 17:27 [U-Boot] [PATCH v2] cfi_flash: Fix unaligned accesses to cfi_qry structure Andrew Gabbasov
2013-05-22 18:50 ` Albert ARIBAUD
2013-05-23 6:16 ` Stefan Roese
2013-05-23 7:49 ` Stefan Roese
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox