| Message ID | 1386279359-32286-6-git-send-email-roy.franz@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 5 December 2013 21:35, Roy Franz <roy.franz@linaro.org> 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. */ thanks -- PMM
On 12 December 2013 17:26, Peter Maydell <peter.maydell@linaro.org> wrote: > On 5 December 2013 21:35, Roy Franz <roy.franz@linaro.org> 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
On Thu, Dec 12, 2013 at 9:37 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 12 December 2013 17:26, Peter Maydell <peter.maydell@linaro.org> wrote: >> On 5 December 2013 21:35, Roy Franz <roy.franz@linaro.org> 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
diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c index 82a2519..8f81341 100644 --- a/hw/block/pflash_cfi01.c +++ b/hw/block/pflash_cfi01.c @@ -74,6 +74,7 @@ struct pflash_t { uint64_t sector_len; uint8_t bank_width; uint8_t device_width; /* If 0, device width not specified. */ + uint8_t max_device_width; /* max device width in bytes */ uint8_t be; uint8_t wcycle; /* if 0, the flash is read normally */ int ro; @@ -635,6 +636,13 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp) pfl->ro = 0; } + /* Default to devices being used at their maximum device width. This was + * assumed before the device_width support was added. + */ + if (!pfl->max_device_width) { + pfl->max_device_width = pfl->device_width; + } + pfl->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, pflash_timer, pfl); pfl->wcycle = 0; pfl->cmd = 0; @@ -730,6 +738,7 @@ static Property pflash_cfi01_properties[] = { 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), DEFINE_PROP_UINT8("big-endian", struct pflash_t, be, 0), DEFINE_PROP_UINT16("id0", struct pflash_t, ident0, 0), DEFINE_PROP_UINT16("id1", struct pflash_t, ident1, 0),
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. Signed-off-by: Roy Franz <roy.franz@linaro.org> --- hw/block/pflash_cfi01.c | 9 +++++++++ 1 file changed, 9 insertions(+)