Signed-off-by: Mike Nawrocki <michael.nawrocki@gtri.gatech.edu>
---
hw/block/pflash_cfi02.c | 97 +++++++++----------------------------------------
1 file changed, 18 insertions(+), 79 deletions(-)
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index c81ddd3a99..a81df913f6 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -138,12 +138,12 @@ static void pflash_timer (void *opaque)
pfl->cmd = 0;
}
-static uint32_t pflash_read (pflash_t *pfl, hwaddr offset,
- int width, int be)
+static uint64_t pflash_read(pflash_t *pfl, hwaddr offset,
+ int width, int be)
{
hwaddr boff;
- uint32_t ret;
uint8_t *p;
+ uint64_t ret;
DPRINTF("%s: offset " TARGET_FMT_plx "\n", __func__, offset);
ret = -1;
@@ -261,7 +261,7 @@ static void pflash_update(pflash_t *pfl, int offset,
}
static void pflash_write (pflash_t *pfl, hwaddr offset,
- uint32_t value, int width, int be)
+ uint64_t value, int width, int be)
{
hwaddr boff;
uint8_t *p;
@@ -494,102 +494,41 @@ static void pflash_write (pflash_t *pfl, hwaddr offset,
pfl->cmd = 0;
}
-
-static uint32_t pflash_readb_be(void *opaque, hwaddr addr)
-{
- return pflash_read(opaque, addr, 1, 1);
-}
-
-static uint32_t pflash_readb_le(void *opaque, hwaddr addr)
-{
- return pflash_read(opaque, addr, 1, 0);
-}
-
-static uint32_t pflash_readw_be(void *opaque, hwaddr addr)
+static uint64_t pflash_read_le(void *opaque, hwaddr addr, unsigned size)
{
pflash_t *pfl = opaque;
-
- return pflash_read(pfl, addr, 2, 1);
+ return pflash_read(pfl, addr, size, 0);
}
-static uint32_t pflash_readw_le(void *opaque, hwaddr addr)
+static uint64_t pflash_read_be(void *opaque, hwaddr addr, unsigned size)
{
pflash_t *pfl = opaque;
-
- return pflash_read(pfl, addr, 2, 0);
+ return pflash_read(pfl, addr, size, 1);
}
-static uint32_t pflash_readl_be(void *opaque, hwaddr addr)
+static void pflash_write_le(void *opaque, hwaddr addr, uint64_t data,
+ unsigned size)
{
pflash_t *pfl = opaque;
-
- return pflash_read(pfl, addr, 4, 1);
+ pflash_write(pfl, addr, data, size, 0);
}
-static uint32_t pflash_readl_le(void *opaque, hwaddr addr)
+static void pflash_write_be(void *opaque, hwaddr addr, uint64_t data,
+ unsigned size)
{
pflash_t *pfl = opaque;
-
- return pflash_read(pfl, addr, 4, 0);
-}
-
-static void pflash_writeb_be(void *opaque, hwaddr addr,
- uint32_t value)
-{
- pflash_write(opaque, addr, value, 1, 1);
-}
-
-static void pflash_writeb_le(void *opaque, hwaddr addr,
- uint32_t value)
-{
- pflash_write(opaque, addr, value, 1, 0);
-}
-
-static void pflash_writew_be(void *opaque, hwaddr addr,
- uint32_t value)
-{
- pflash_t *pfl = opaque;
-
- pflash_write(pfl, addr, value, 2, 1);
-}
-
-static void pflash_writew_le(void *opaque, hwaddr addr,
- uint32_t value)
-{
- pflash_t *pfl = opaque;
-
- pflash_write(pfl, addr, value, 2, 0);
-}
-
-static void pflash_writel_be(void *opaque, hwaddr addr,
- uint32_t value)
-{
- pflash_t *pfl = opaque;
-
- pflash_write(pfl, addr, value, 4, 1);
-}
-
-static void pflash_writel_le(void *opaque, hwaddr addr,
- uint32_t value)
-{
- pflash_t *pfl = opaque;
-
- pflash_write(pfl, addr, value, 4, 0);
+ pflash_write(pfl, addr, data, size, 1);
}
static const MemoryRegionOps pflash_cfi02_ops_be = {
- .old_mmio = {
- .read = { pflash_readb_be, pflash_readw_be, pflash_readl_be, },
- .write = { pflash_writeb_be, pflash_writew_be, pflash_writel_be, },
- },
+ .read = pflash_read_be,
+ .write = pflash_write_be,
.endianness = DEVICE_NATIVE_ENDIAN,
};
static const MemoryRegionOps pflash_cfi02_ops_le = {
- .old_mmio = {
- .read = { pflash_readb_le, pflash_readw_le, pflash_readl_le, },
- .write = { pflash_writeb_le, pflash_writew_le, pflash_writel_le, },
- },
+ .read = pflash_read_le,
+ .write = pflash_write_le,
.endianness = DEVICE_NATIVE_ENDIAN,
};
--
2.14.2
On 13 November 2017 at 16:14, Mike Nawrocki
<michael.nawrocki@gtri.gatech.edu> wrote:
> Signed-off-by: Mike Nawrocki <michael.nawrocki@gtri.gatech.edu>
> ---
> hw/block/pflash_cfi02.c | 97 +++++++++----------------------------------------
> 1 file changed, 18 insertions(+), 79 deletions(-)
>
> diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
> index c81ddd3a99..a81df913f6 100644
> --- a/hw/block/pflash_cfi02.c
> +++ b/hw/block/pflash_cfi02.c
> @@ -138,12 +138,12 @@ static void pflash_timer (void *opaque)
> pfl->cmd = 0;
> }
>
> -static uint32_t pflash_read (pflash_t *pfl, hwaddr offset,
> - int width, int be)
> +static uint64_t pflash_read(pflash_t *pfl, hwaddr offset,
> + int width, int be)
> {
> hwaddr boff;
> - uint32_t ret;
> uint8_t *p;
> + uint64_t ret;
I suspect you'll find that the change of type for 'ret' here
and the 'value' argument to pflash_write() will break compilation
with PFLASH_DEBUG defined, because the type won't match the DPRINTF
format strings any more.
You could either fix up the format strings, or (since there's a
wrapper function here anyway) leave the types of pflash_read()
and pflash_write() alone and let the wrappers implicitly do
the conversion between uint64_t and uint32_t.
thanks
-- PMM
On 23 November 2017 at 11:26, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 13 November 2017 at 16:14, Mike Nawrocki
> <michael.nawrocki@gtri.gatech.edu> wrote:
>> Signed-off-by: Mike Nawrocki <michael.nawrocki@gtri.gatech.edu>
>> ---
>> hw/block/pflash_cfi02.c | 97 +++++++++----------------------------------------
>> 1 file changed, 18 insertions(+), 79 deletions(-)
>>
>> diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
>> index c81ddd3a99..a81df913f6 100644
>> --- a/hw/block/pflash_cfi02.c
>> +++ b/hw/block/pflash_cfi02.c
>> @@ -138,12 +138,12 @@ static void pflash_timer (void *opaque)
>> pfl->cmd = 0;
>> }
>>
>> -static uint32_t pflash_read (pflash_t *pfl, hwaddr offset,
>> - int width, int be)
>> +static uint64_t pflash_read(pflash_t *pfl, hwaddr offset,
>> + int width, int be)
>> {
>> hwaddr boff;
>> - uint32_t ret;
>> uint8_t *p;
>> + uint64_t ret;
>
> I suspect you'll find that the change of type for 'ret' here
> and the 'value' argument to pflash_write() will break compilation
> with PFLASH_DEBUG defined, because the type won't match the DPRINTF
> format strings any more.
>
> You could either fix up the format strings, or (since there's a
> wrapper function here anyway) leave the types of pflash_read()
> and pflash_write() alone and let the wrappers implicitly do
> the conversion between uint64_t and uint32_t.
...ah, just noticed that in patch 2 you want to add 8-byte
accesses. So you'll need to fix the format strings.
thanks
-- PMM
On 11/23/2017 06:27 AM, Peter Maydell wrote:
> On 23 November 2017 at 11:26, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 13 November 2017 at 16:14, Mike Nawrocki
>> <michael.nawrocki@gtri.gatech.edu> wrote:
>>> Signed-off-by: Mike Nawrocki <michael.nawrocki@gtri.gatech.edu>
>>> ---
>>> hw/block/pflash_cfi02.c | 97 +++++++++----------------------------------------
>>> 1 file changed, 18 insertions(+), 79 deletions(-)
>>>
>>> diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
>>> index c81ddd3a99..a81df913f6 100644
>>> --- a/hw/block/pflash_cfi02.c
>>> +++ b/hw/block/pflash_cfi02.c
>>> @@ -138,12 +138,12 @@ static void pflash_timer (void *opaque)
>>> pfl->cmd = 0;
>>> }
>>>
>>> -static uint32_t pflash_read (pflash_t *pfl, hwaddr offset,
>>> - int width, int be)
>>> +static uint64_t pflash_read(pflash_t *pfl, hwaddr offset,
>>> + int width, int be)
>>> {
>>> hwaddr boff;
>>> - uint32_t ret;
>>> uint8_t *p;
>>> + uint64_t ret;
>>
>> I suspect you'll find that the change of type for 'ret' here
>> and the 'value' argument to pflash_write() will break compilation
>> with PFLASH_DEBUG defined, because the type won't match the DPRINTF
>> format strings any more.
>>
>> You could either fix up the format strings, or (since there's a
>> wrapper function here anyway) leave the types of pflash_read()
>> and pflash_write() alone and let the wrappers implicitly do
>> the conversion between uint64_t and uint32_t.
>
> ...ah, just noticed that in patch 2 you want to add 8-byte
> accesses. So you'll need to fix the format strings.
>
> thanks
> -- PMM
>
Yeah, it definitely doesn't compile with PFLASH_DEBUG defined. I'll
update those format strings for the next revision.
Thanks!
On 11/28/2017 01:47 PM, Michael Nawrocki wrote:
>>> I suspect you'll find that the change of type for 'ret' here
>>> and the 'value' argument to pflash_write() will break compilation
>>> with PFLASH_DEBUG defined, because the type won't match the DPRINTF
>>> format strings any more.
>>>
>>> You could either fix up the format strings, or (since there's a
>>> wrapper function here anyway) leave the types of pflash_read()
>>> and pflash_write() alone and let the wrappers implicitly do
>>> the conversion between uint64_t and uint32_t.
>>
>> ...ah, just noticed that in patch 2 you want to add 8-byte
>> accesses. So you'll need to fix the format strings.
>>
>> thanks
>> -- PMM
>>
>
> Yeah, it definitely doesn't compile with PFLASH_DEBUG defined. I'll
> update those format strings for the next revision.
Better yet, please fix the PFLASH_DEBUG code to avoid bitrot in the
first place, by rewriting the bad pattern:
/* #define PFLASH_DEBUG */
#ifdef PFLASH_DEBUG
#define DPRINTF(fmt, ...) \
do { \
fprintf(stderr, "PFLASH: " fmt , ## __VA_ARGS__); \
} while (0)
#else
#define DPRINTF(fmt, ...) do { } while (0)
#endif
into the good pattern:
#ifdef PFLASH_DEBUG
# define PFLASH_PRINT 1
#else
# define PFLASH_PRINT 0
#endif
#define DPRINTF(fmt, ...) \
do { \
if (PFLASH_PRINT) { \
fprintf(stderr, "PFLASH: " fmt, ## __VA_ARGS__); \
}; \
} while (0)
or even better, using the trace infrastructure instead.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
On 11/28/2017 02:05 PM, Eric Blake wrote:
> Better yet, please fix the PFLASH_DEBUG code to avoid bitrot in the
> first place, by rewriting the bad pattern:
>
> into the good pattern:
>
> #ifdef PFLASH_DEBUG
> # define PFLASH_PRINT 1
> #else
> # define PFLASH_PRINT 0
> #endif
> #define DPRINTF(fmt, ...) \
> do { \
> if (PFLASH_PRINT) { \
> fprintf(stderr, "PFLASH: " fmt, ## __VA_ARGS__); \
> }; \
Serves me right for typing this without testing; the extra ; is spurious.
By the way, this is one of the Bite-Sized Tasks that never seems to get
finished...
https://wiki.qemu.org/BiteSizedTasks#Bitrot_prevention
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
© 2016 - 2026 Red Hat, Inc.