Re: [Qemu-devel] [PATCH 3/4 v4] block: return status for each device

[Peter Maydell]

On 22 October 2013 17:35, Roy Franz <address@hidden> wrote:
> Now that we know how wide each flash device that makes up the bank is,
> return status for each device in the bank.  Leave existing code
> that treats 32 bit wide banks as composed of two 16 bit devices as otherwise
> we may break configurations that do not set the device_width propery.
>
> Signed-off-by: Roy Franz <address@hidden>
> ---
>  hw/block/pflash_cfi01.c |   16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index cda8289..aa2cbbc 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -191,9 +191,21 @@ static uint32_t pflash_read (pflash_t *pfl, hwaddr 
> offset,
>      case 0x60: /* Block /un)lock */
>      case 0x70: /* Status Register */
>      case 0xe8: /* Write block */
> -        /* Status register read */
> +        /* Status register read.  Return status from each device in
> +         * bank.
> +         */
>          ret = pfl->status;
> -        if (width > 2) {
> +        if (width > pfl->device_width) {
> +            int shift = pfl->device_width * 8;
> +            while (shift + pfl->device_width * 8 <= width * 8) {
> +                ret |= pfl->status << (shift);

The brackets here are superfluous.

> +                shift += pfl->device_width * 8;
> +            }

If we end up with several things that all want to get
this "replicate into all lanes" behaviour then a helper
function is probably going to be worthwhile (see comments
on other patch).

> +        } else if (width > 2) {
> +            /* Handle 32 bit flash cases where device width may be
> +             * improperly set. (Existing behavior before device width
> +             * added.)
> +             */
>              ret |= pfl->status << 16;

Maybe we should have 'device_width == 0' mean 'same as
bank width and with all the legacy workaround code', so
device_width can be specifically set same as bank_width
for new platforms to get the actual correct behaviour for
a full 32 bit wide flash device.

I don't care very much though: up to you.

>          }
>          DPRINTF("%s: status %x\n", __func__, ret);
> --
> 1.7.10.4
>

thanks
-- PMM