Re: [Qemu-devel] [PATCH 1/2 v3] block: Add device-width property to pflash_cfi01

[Peter Maydell]

On 19 October 2013 18:04, Roy Franz <address@hidden> wrote:
> The width of the devices that make up the flash interface
> is required to mask certain commands, in particular the
> write length for buffered writes.  This length will be presented
> to each device on the interface by the program writing the flash,
> and the flash emulation code needs to be able to determine
> the length of the write as recieved by each flash device.
> The device-width defaults to the bank width which should
> maintain existing behavior for platforms that don't need
> this change.
> This change is required to support buffered writes on the
> vexpress platform that has a 32 bit flash interface with 2
> 16 bit devices on it.
>
> Signed-off-by: Roy Franz <address@hidden>
> ---
>  hw/block/pflash_cfi01.c |   21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index 018a967..cda8289 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -71,7 +71,8 @@ struct pflash_t {
>      BlockDriverState *bs;
>      uint32_t nb_blocs;
>      uint64_t sector_len;
> -    uint8_t width;
> +    uint8_t bank_width;

If you want to rename this struct field can you put that in its own patch,
please? Otherwise it's hard to see the actual functional changes.

> +    uint8_t device_width;
>      uint8_t be;
>      uint8_t wcycle; /* if 0, the flash is read normally */
>      int ro;
> @@ -126,9 +127,9 @@ static uint32_t pflash_read (pflash_t *pfl, hwaddr offset,
>      ret = -1;
>      boff = offset & 0xFF; /* why this here ?? */
>
> -    if (pfl->width == 2)
> +    if (pfl->bank_width == 2)
>          boff = boff >> 1;
> -    else if (pfl->width == 4)
> +    else if (pfl->bank_width == 4)
>          boff = boff >> 2;
>
>  #if 0
> @@ -378,6 +379,8 @@ static void pflash_write(pflash_t *pfl, hwaddr offset,
>
>              break;
>          case 0xe8:
> +            /* Mask writeblock size based on device width */
> +            value &= (1ULL << (pfl->device_width * 8)) - 1;

Is this really the only guest visible difference for banked flash devices?

>              DPRINTF("%s: block write of %x bytes\n", __func__, value);
>              pfl->counter = value;
>              pfl->wcycle++;

thanks
-- PMM