| Message ID | 50492BC5.4090107@web.de |
|---|---|
| State | New |
| Headers | show |
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
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
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
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 >
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
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,
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(-)