Re: [Qemu-devel] [PATCH V5 5/7] Add max device width parameter for NOR devices

[Roy Franz]

On Thu, Dec 12, 2013 at 9:37 AM, Peter Maydell <address@hidden> wrote:
> On 12 December 2013 17:26, Peter Maydell <address@hidden> wrote:
>> On 5 December 2013 21:35, Roy Franz <address@hidden> wrote:
>>> For handling CFI and device ID reads, we need to not only know the
>>> width that a NOR flash device is configured for, but also its maximum
>>> width.  The maximum width addressing mode is used for multi-width
>>> parts no matter which width they are configured for.  The most common
>>> case is x16 parts that also support x8 mode.  When configured for x8
>>> operation these devices respond to CFI and device ID requests differently
>>> than native x8 NOR parts.
>>
>>>      DEFINE_PROP_UINT64("sector-length", struct pflash_t, sector_len, 0),
>>>      DEFINE_PROP_UINT8("width", struct pflash_t, bank_width, 0),
>>>      DEFINE_PROP_UINT8("device-width", struct pflash_t, device_width, 0),
>>> +    DEFINE_PROP_UINT8("max-device-width", struct pflash_t, 
>>> max_device_width, 0),
>>
>> So I think that given we now have three width related properties
>> we could use a comment here about what they mean. Do I have
>> this right?
>>
>> /* width here is the overall width of this QEMU device in bytes.
>>  * The QEMU device may be emulating a number of flash devices
>>  * wired up in parallel; the width of each individual flash
>>  * device should be specified via device-width. If the individual
>>  * devices have a maximum width which is greater than the width
>>  * they are being used for, this maximum width should be set via
>>  * max-device-width (which otherwise defaults to device-width).
>>  * So for instance a 32-bit wide QEMU flash device made from four
>>  * 16-bit flash devices used in 8-bit wide mode would be configured
>>  * with width = 4, device-width = 1, max-device-width = 2.
>>  *
>>  * If device-width is not specified we default to backwards
>>  * compatible behaviour which is a bad emulation of two
>>  * 16 bit devices making up a 32 bit wide QEMU device. This
>>  * is deprecated for new uses of this device.
>>  */
>
> PS: if you're happy that the comment above is correct, I
> can just add it locally (and fix up the format nits in
> the other patch), to save you having to respin the series,
> and stick it in the target-arm.next queue.
>
> thanks
> -- PMM

Hi Peter,

   Your comment explains it very nicely, and please go ahead
and fix the formatting issues.  This is the last QEMU patchset
I have for UEFI support, so once this is in UEFI should boot on the A15
QEMU platforms.

Thanks,
Roy