On 22 October 2013 17:35, Roy Franz <roy.fr...@linaro.org> 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 <roy.fr...@linaro.org>
> ---
> 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
