qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] musicpal: Fix flash mapping
@ 2012-09-06 23:03 Jan Kiszka
  2012-09-07 14:41 ` Peter Maydell
  0 siblings, 1 reply; 11+ messages in thread
From: Jan Kiszka @ 2012-09-06 23:03 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-devel, qemu-stable

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

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 hw/musicpal.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/musicpal.c b/hw/musicpal.c
index ad725b5..10c2c16 100644
--- a/hw/musicpal.c
+++ b/hw/musicpal.c
@@ -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,
-- 
1.7.3.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH] musicpal: Fix flash mapping
  2012-09-06 23:03 [Qemu-devel] [PATCH] musicpal: Fix flash mapping Jan Kiszka
@ 2012-09-07 14:41 ` Peter Maydell
  2012-09-07 14:53   ` Jan Kiszka
  0 siblings, 1 reply; 11+ messages in thread
From: Peter Maydell @ 2012-09-07 14:41 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: qemu-devel, qemu-stable

On 7 September 2012 00:03, Jan Kiszka <jan.kiszka@web.de> wrote:
> The old arithmetic assumed 32 physical address bits which is no longer
> true for ARM since 3cc0cd61f4.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
> ---
>  hw/musicpal.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/musicpal.c b/hw/musicpal.c
> index ad725b5..10c2c16 100644
> --- a/hw/musicpal.c
> +++ b/hw/musicpal.c
> @@ -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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH] musicpal: Fix flash mapping
  2012-09-07 14:41 ` Peter Maydell
@ 2012-09-07 14:53   ` Jan Kiszka
  2012-09-07 15:25     ` Peter Maydell
  0 siblings, 1 reply; 11+ messages in thread
From: Jan Kiszka @ 2012-09-07 14:53 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-devel, qemu-stable

[-- Attachment #1: Type: text/plain, Size: 1004 bytes --]

On 2012-09-07 16:41, Peter Maydell wrote:
> On 7 September 2012 00:03, Jan Kiszka <jan.kiszka@web.de> wrote:
>> The old arithmetic assumed 32 physical address bits which is no longer
>> true for ARM since 3cc0cd61f4.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
>> ---
>>  hw/musicpal.c |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/musicpal.c b/hw/musicpal.c
>> index ad725b5..10c2c16 100644
>> --- a/hw/musicpal.c
>> +++ b/hw/musicpal.c
>> @@ -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



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH] musicpal: Fix flash mapping
  2012-09-07 14:53   ` Jan Kiszka
@ 2012-09-07 15:25     ` Peter Maydell
  2012-09-08  8:44       ` Blue Swirl
  0 siblings, 1 reply; 11+ messages in thread
From: Peter Maydell @ 2012-09-07 15:25 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: qemu-devel, qemu-stable

On 7 September 2012 15:53, Jan Kiszka <jan.kiszka@web.de> wrote:
> On 2012-09-07 16:41, Peter Maydell wrote:
>> On 7 September 2012 00:03, Jan Kiszka <jan.kiszka@web.de> wrote:
>>> +        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 <peter.maydell@linaro.org>

-- PMM

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH] musicpal: Fix flash mapping
  2012-09-07 15:25     ` Peter Maydell
@ 2012-09-08  8:44       ` Blue Swirl
  2012-09-08  8:50         ` Jan Kiszka
  0 siblings, 1 reply; 11+ messages in thread
From: Blue Swirl @ 2012-09-08  8:44 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Jan Kiszka, qemu-devel, qemu-stable

On Fri, Sep 7, 2012 at 3:25 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 7 September 2012 15:53, Jan Kiszka <jan.kiszka@web.de> wrote:
>> On 2012-09-07 16:41, Peter Maydell wrote:
>>> On 7 September 2012 00:03, Jan Kiszka <jan.kiszka@web.de> wrote:
>>>> +        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.

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

>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
> -- PMM
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH] musicpal: Fix flash mapping
  2012-09-08  8:44       ` Blue Swirl
@ 2012-09-08  8:50         ` Jan Kiszka
  2012-09-08  9:48           ` [Qemu-devel] [PATCH v2] " Jan Kiszka
  0 siblings, 1 reply; 11+ messages in thread
From: Jan Kiszka @ 2012-09-08  8:50 UTC (permalink / raw)
  To: Blue Swirl; +Cc: Peter Maydell, qemu-devel, qemu-stable

[-- Attachment #1: Type: text/plain, Size: 1149 bytes --]

On 2012-09-08 10:44, Blue Swirl wrote:
> On Fri, Sep 7, 2012 at 3:25 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 7 September 2012 15:53, Jan Kiszka <jan.kiszka@web.de> wrote:
>>> On 2012-09-07 16:41, Peter Maydell wrote:
>>>> On 7 September 2012 00:03, Jan Kiszka <jan.kiszka@web.de> wrote:
>>>>> +        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.
> 
> 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



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH v2] musicpal: Fix flash mapping
  2012-09-08  8:50         ` Jan Kiszka
@ 2012-09-08  9:48           ` Jan Kiszka
  2012-09-08  9:50             ` Peter Maydell
  2012-09-08  9:52             ` [Qemu-devel] [PATCH v2a] " Jan Kiszka
  0 siblings, 2 replies; 11+ messages in thread
From: Jan Kiszka @ 2012-09-08  9:48 UTC (permalink / raw)
  To: Blue Swirl; +Cc: Peter Maydell, qemu-devel, qemu-stable

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

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---

Changes in v2:
- mark large constant ULL

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

diff --git a/hw/musicpal.c b/hw/musicpal.c
index ad725b5..10c2c16 100644
--- a/hw/musicpal.c
+++ b/hw/musicpal.c
@@ -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,
-- 
1.7.3.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH v2] musicpal: Fix flash mapping
  2012-09-08  9:48           ` [Qemu-devel] [PATCH v2] " Jan Kiszka
@ 2012-09-08  9:50             ` Peter Maydell
  2012-09-08  9:51               ` Jan Kiszka
  2012-09-08  9:52             ` [Qemu-devel] [PATCH v2a] " Jan Kiszka
  1 sibling, 1 reply; 11+ messages in thread
From: Peter Maydell @ 2012-09-08  9:50 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Blue Swirl, qemu-devel, qemu-stable

On 8 September 2012 10:48, Jan Kiszka <jan.kiszka@web.de> wrote:
> The old arithmetic assumed 32 physical address bits which is no longer
> true for ARM since 3cc0cd61f4.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
> ---
>
> Changes in v2:
> - mark large constant ULL
>
>  #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,

...wrong version of patch sent?

-- PMM

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH v2] musicpal: Fix flash mapping
  2012-09-08  9:50             ` Peter Maydell
@ 2012-09-08  9:51               ` Jan Kiszka
  0 siblings, 0 replies; 11+ messages in thread
From: Jan Kiszka @ 2012-09-08  9:51 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Blue Swirl, qemu-devel, qemu-stable

[-- Attachment #1: Type: text/plain, Size: 626 bytes --]

On 2012-09-08 11:50, Peter Maydell wrote:
> On 8 September 2012 10:48, Jan Kiszka <jan.kiszka@web.de> wrote:
>> The old arithmetic assumed 32 physical address bits which is no longer
>> true for ARM since 3cc0cd61f4.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
>> ---
>>
>> Changes in v2:
>> - mark large constant ULL
>>
>>  #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,
> 
> ...wrong version of patch sent?

Grrr...


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH v2a] musicpal: Fix flash mapping
  2012-09-08  9:48           ` [Qemu-devel] [PATCH v2] " Jan Kiszka
  2012-09-08  9:50             ` Peter Maydell
@ 2012-09-08  9:52             ` Jan Kiszka
  2012-09-08 10:19               ` Blue Swirl
  1 sibling, 1 reply; 11+ messages in thread
From: Jan Kiszka @ 2012-09-08  9:52 UTC (permalink / raw)
  To: Blue Swirl; +Cc: Peter Maydell, qemu-devel, qemu-stable

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

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---

Changes in v2a:
- mark large constant ULL

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

diff --git a/hw/musicpal.c b/hw/musicpal.c
index ad725b5..f305e21 100644
--- a/hw/musicpal.c
+++ b/hw/musicpal.c
@@ -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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH v2a] musicpal: Fix flash mapping
  2012-09-08  9:52             ` [Qemu-devel] [PATCH v2a] " Jan Kiszka
@ 2012-09-08 10:19               ` Blue Swirl
  0 siblings, 0 replies; 11+ messages in thread
From: Blue Swirl @ 2012-09-08 10:19 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Peter Maydell, qemu-devel, qemu-stable

Thanks, applied.

On Sat, Sep 8, 2012 at 9:52 AM, Jan Kiszka <jan.kiszka@web.de> wrote:
> The old arithmetic assumed 32 physical address bits which is no longer
> true for ARM since 3cc0cd61f4.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
> ---
>
> Changes in v2a:
> - mark large constant ULL
>
>  hw/musicpal.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/musicpal.c b/hw/musicpal.c
> index ad725b5..f305e21 100644
> --- a/hw/musicpal.c
> +++ b/hw/musicpal.c
> @@ -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

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2012-09-08 10:19 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-06 23:03 [Qemu-devel] [PATCH] musicpal: Fix flash mapping Jan Kiszka
2012-09-07 14:41 ` Peter Maydell
2012-09-07 14:53   ` Jan Kiszka
2012-09-07 15:25     ` Peter Maydell
2012-09-08  8:44       ` Blue Swirl
2012-09-08  8:50         ` Jan Kiszka
2012-09-08  9:48           ` [Qemu-devel] [PATCH v2] " Jan Kiszka
2012-09-08  9:50             ` Peter Maydell
2012-09-08  9:51               ` Jan Kiszka
2012-09-08  9:52             ` [Qemu-devel] [PATCH v2a] " Jan Kiszka
2012-09-08 10:19               ` Blue Swirl

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).