Re: [Qemu-devel] [PATCH v3 1/3] Switch AMD CFI flash to use new MMIO API

[Michael Nawrocki <michael.nawrocki@gtri.gatech.edu>]

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!