musicpal: Fix flash mapping

PATCH Superseded Jan Kiszka 3 participants 11 messages Sep 6, 2012 – Sep 8, 2012

lore

Share & tools

lore

Jan Kiszka

author

2012-09-06 23:03 UTC #0 lore raw

PATCH 1/1 musicpal: Fix flash mapping

The old arithmetic assumed 32 physical address bits which is no longer true for ARM since 3cc0cd61f4.

Signed-off-by Jan Kiszka <[email protected]>

---
 hw/musicpal.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

hw/musicpal.c +2 -2

index ad725b5..10c2c16 100644
@@ -1583,7 +1583,7 @@ static void musicpal_init(ram_addr_t ram_size,
          * image is smaller than 32 MB.
          */
 #ifdef TARGET_WORDS_BIGENDIAN
-        pflash_cfi02_register(0-MP_FLASH_SIZE_MAX, NULL,
+        pflash_cfi02_register(0x100000000-MP_FLASH_SIZE_MAX, NULL,
                               "musicpal.flash", flash_size,
                               dinfo->bdrv, 0x10000,
                               (flash_size + 0xffff) >> 16,

@@ -1591,7 +1591,7 @@ static void musicpal_init(ram_addr_t ram_size,
                               2, 0x00BF, 0x236D, 0x0000, 0x0000,
                               0x5555, 0x2AAA, 1);
 #else
-        pflash_cfi02_register(0-MP_FLASH_SIZE_MAX, NULL,
+        pflash_cfi02_register(0x100000000-MP_FLASH_SIZE_MAX, NULL,
                               "musicpal.flash", flash_size,
                               dinfo->bdrv, 0x10000,
                               (flash_size + 0xffff) >> 16,

Signature · 1 line

1.7.3.4

Peter Maydell

2012-09-07 14:42 UTC #1 lore raw

Quoted 13 lines · Jan Kiszka

The old arithmetic assumed 32 physical address bits which is no longer true for ARM since 3cc0cd61f4.
Signed-off-by Jan Kiszka <[email protected]>
---
 hw/musicpal.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
hw/musicpal.c +0 -0
index ad725b5..10c2c16 100644

hw/musicpal.c static void musicpal_init(ram_addr_t ram_size, · line 1586

Quoted 5 lines

hw/musicpal.c +1 -1

@@ -1583,7 +1583,7 @@ static void musicpal_init(ram_addr_t ram_size,
          * image is smaller than 32 MB.
          */
 #ifdef TARGET_WORDS_BIGENDIAN
-        pflash_cfi02_register(0-MP_FLASH_SIZE_MAX, NULL,
+        pflash_cfi02_register(0x100000000-MP_FLASH_SIZE_MAX, NULL,

I don't think this will compile on a 32 bit system, will it? You probably want an ULL suffix.

-- PMM

Jan Kiszka

author

2012-09-07 14:53 UTC #2 lore raw

Quoted 9 lines · Peter Maydell

Quoted 8 lines · Jan Kiszka
The old arithmetic assumed 32 physical address bits which is no longer true for ARM since 3cc0cd61f4.
Signed-off-by Jan Kiszka <[email protected]>
---
 hw/musicpal.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Quoted 13 lines

hw/musicpal.c static void musicpal_init(ram_addr_t ram_size, · line 1586
Quoted 10 lines
hw/musicpal.c +1 -1
index ad725b5..10c2c16 100644
@@ -1583,7 +1583,7 @@ static void musicpal_init(ram_addr_t ram_size,
          * image is smaller than 32 MB.
          */
 #ifdef TARGET_WORDS_BIGENDIAN
-        pflash_cfi02_register(0-MP_FLASH_SIZE_MAX, NULL,
+        pflash_cfi02_register(0x100000000-MP_FLASH_SIZE_MAX, NULL,
I don't think this will compile on a 32 bit system, will it? You probably want an ULL suffix.

It does as the result always fits in 32 bits. But I can add that if you prefer.

Jan

Peter Maydell

Reviewed-by

2012-09-07 15:26 UTC #3 lore raw

Quoted 9 lines · Jan Kiszka

Quoted 5 lines · Peter Maydell
Quoted 1 line · Jan Kiszka
+ pflash_cfi02_register(0x100000000-MP_FLASH_SIZE_MAX, NULL,
I don't think this will compile on a 32 bit system, will it? You probably want an ULL suffix.
It does as the result always fits in 32 bits. But I can add that if you prefer.

I think I had a misconception of this bit of the C standard. C will pick a type big enough to fit the constant value (which will in this case be a 64 bit type of some kind), even without an ULL suffix. So you're right, it's OK.

Reviewed-by Peter Maydell <[email protected]>

-- PMM

Blue Swirl

2012-09-08 08:44 UTC #4 lore raw

Quoted 8 lines · Peter Maydell

Quoted 7 lines · Jan Kiszka
Quoted 5 lines · Peter Maydell
Quoted 1 line · Jan Kiszka
+ pflash_cfi02_register(0x100000000-MP_FLASH_SIZE_MAX, NULL,
I don't think this will compile on a 32 bit system, will it? You probably want an ULL suffix.

Quoted 7 lines

Quoted 2 lines
It does as the result always fits in 32 bits. But I can add that if you prefer.
I think I had a misconception of this bit of the C standard. C will pick a type big enough to fit the constant value (which will in this case be a 64 bit type of some kind), even without an ULL suffix. So you're right, it's OK.

GCC disagrees:

$ cat u64.c
unsigned int i = 0x100000000 - 1;
$ gcc -m32 -Wall -c u64.c
u64.c:1: warning: integer constant is too large for 'long' type

Clang doesn't care even with --all-warnings:

$ clang -m32 -Wall --all-warnings -c u64.c

Quoted 5 lines

Reviewed-by Peter Maydell <[email protected]>
-- PMM

Jan Kiszka

author

2012-09-08 08:50 UTC #5 lore raw

Quoted 17 lines · Blue Swirl

Quoted 8 lines · Peter Maydell
Quoted 7 lines · Jan Kiszka
Quoted 5 lines · Peter Maydell
Quoted 1 line · Jan Kiszka
+ pflash_cfi02_register(0x100000000-MP_FLASH_SIZE_MAX, NULL,
I don't think this will compile on a 32 bit system, will it? You probably want an ULL suffix.
Quoted 7 lines
Quoted 2 lines
It does as the result always fits in 32 bits. But I can add that if you prefer.
I think I had a misconception of this bit of the C standard. C will pick a type big enough to fit the constant value (which will in this case be a 64 bit type of some kind), even without an ULL suffix. So you're right, it's OK.

Quoted 5 lines

GCC disagrees:

$ cat u64.c
unsigned int i = 0x100000000 - 1;
$ gcc -m32 -Wall -c u64.c
u64.c:1: warning: integer constant is too large for 'long' type

Obviously depends on the compiler version or configuration, mine (4.5 still) does not. I'll send v2 to make them all happy.

Jan

Jan Kiszka

author

2012-09-08 09:48 UTC #6 lore raw

PATCH v2 1/1 musicpal: Fix flash mapping

The old arithmetic assumed 32 physical address bits which is no longer true for ARM since 3cc0cd61f4.

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v2:
- mark large constant ULL

 hw/musicpal.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

hw/musicpal.c +2 -2

index ad725b5..10c2c16 100644
@@ -1583,7 +1583,7 @@ static void musicpal_init(ram_addr_t ram_size,
          * image is smaller than 32 MB.
          */
 #ifdef TARGET_WORDS_BIGENDIAN
-        pflash_cfi02_register(0-MP_FLASH_SIZE_MAX, NULL,
+        pflash_cfi02_register(0x100000000-MP_FLASH_SIZE_MAX, NULL,
                               "musicpal.flash", flash_size,
                               dinfo->bdrv, 0x10000,
                               (flash_size + 0xffff) >> 16,

@@ -1591,7 +1591,7 @@ static void musicpal_init(ram_addr_t ram_size,
                               2, 0x00BF, 0x236D, 0x0000, 0x0000,
                               0x5555, 0x2AAA, 1);
 #else
-        pflash_cfi02_register(0-MP_FLASH_SIZE_MAX, NULL,
+        pflash_cfi02_register(0x100000000-MP_FLASH_SIZE_MAX, NULL,
                               "musicpal.flash", flash_size,
                               dinfo->bdrv, 0x10000,
                               (flash_size + 0xffff) >> 16,

Signature · 1 line

1.7.3.4

Peter Maydell

2012-09-08 09:50 UTC #7 lore raw

hw/musicpal.c static void musicpal_init(ram_addr_t ram_size, · line 1587

Quoted 13 lines · Jan Kiszka

The old arithmetic assumed 32 physical address bits which is no longer true for ARM since 3cc0cd61f4.
Signed-off-by Jan Kiszka <[email protected]>
---
Changes in v2:
- mark large constant ULL
#ifdef TARGET_WORDS_BIGENDIAN
hw/musicpal.c +1 -1
-        pflash_cfi02_register(0-MP_FLASH_SIZE_MAX, NULL,
+        pflash_cfi02_register(0x100000000-MP_FLASH_SIZE_MAX, NULL,
                               "musicpal.flash", flash_size,

...wrong version of patch sent?

-- PMM

Jan Kiszka

author

2012-09-08 09:51 UTC #8 lore raw

Quoted 10 lines · Peter Maydell

Quoted 9 lines · Jan Kiszka
The old arithmetic assumed 32 physical address bits which is no longer true for ARM since 3cc0cd61f4.
Signed-off-by Jan Kiszka <[email protected]>
---
Changes in v2:
- mark large constant ULL

Quoted 6 lines

hw/musicpal.c static void musicpal_init(ram_addr_t ram_size, · line 1587
Quoted 4 lines
#ifdef TARGET_WORDS_BIGENDIAN
hw/musicpal.c +1 -1
-        pflash_cfi02_register(0-MP_FLASH_SIZE_MAX, NULL,
+        pflash_cfi02_register(0x100000000-MP_FLASH_SIZE_MAX, NULL,
                               "musicpal.flash", flash_size,
...wrong version of patch sent?

Grrr...

Jan Kiszka

author

2012-09-08 09:52 UTC #9 lore raw

PATCH 1/1 musicpal: Fix flash mapping

In reply to Jan Kiszka

The old arithmetic assumed 32 physical address bits which is no longer true for ARM since 3cc0cd61f4.

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v2a:
- mark large constant ULL

 hw/musicpal.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

hw/musicpal.c +2 -2

index ad725b5..f305e21 100644
@@ -1583,7 +1583,7 @@ static void musicpal_init(ram_addr_t ram_size,
          * image is smaller than 32 MB.
          */
 #ifdef TARGET_WORDS_BIGENDIAN
-        pflash_cfi02_register(0-MP_FLASH_SIZE_MAX, NULL,
+        pflash_cfi02_register(0x100000000ULL-MP_FLASH_SIZE_MAX, NULL,
                               "musicpal.flash", flash_size,
                               dinfo->bdrv, 0x10000,
                               (flash_size + 0xffff) >> 16,

@@ -1591,7 +1591,7 @@ static void musicpal_init(ram_addr_t ram_size,
                               2, 0x00BF, 0x236D, 0x0000, 0x0000,
                               0x5555, 0x2AAA, 1);
 #else
-        pflash_cfi02_register(0-MP_FLASH_SIZE_MAX, NULL,
+        pflash_cfi02_register(0x100000000ULL-MP_FLASH_SIZE_MAX, NULL,
                               "musicpal.flash", flash_size,
                               dinfo->bdrv, 0x10000,
                               (flash_size + 0xffff) >> 16,

Signature · 1 line

1.7.3.4

Blue Swirl

Applied

2012-09-08 10:19 UTC #10 lore raw

Thanks, applied.

Quoted 36 lines · Jan Kiszka

The old arithmetic assumed 32 physical address bits which is no longer true for ARM since 3cc0cd61f4.

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v2a:
- mark large constant ULL

 hw/musicpal.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

hw/musicpal.c +2 -3

index ad725b5..f305e21 100644
@@ -1583,7 +1583,7 @@ static void musicpal_init(ram_addr_t ram_size,
          * image is smaller than 32 MB.
          */
 #ifdef TARGET_WORDS_BIGENDIAN
-        pflash_cfi02_register(0-MP_FLASH_SIZE_MAX, NULL,
+        pflash_cfi02_register(0x100000000ULL-MP_FLASH_SIZE_MAX, NULL,
                               "musicpal.flash", flash_size,
                               dinfo->bdrv, 0x10000,
                               (flash_size + 0xffff) >> 16,

@@ -1591,7 +1591,7 @@ static void musicpal_init(ram_addr_t ram_size,
                               2, 0x00BF, 0x236D, 0x0000, 0x0000,
                               0x5555, 0x2AAA, 1);
 #else
-        pflash_cfi02_register(0-MP_FLASH_SIZE_MAX, NULL,
+        pflash_cfi02_register(0x100000000ULL-MP_FLASH_SIZE_MAX, NULL,
                               "musicpal.flash", flash_size,
                               dinfo->bdrv, 0x10000,
                               (flash_size + 0xffff) >> 16,
--

1.7.3.4