Re: [Qemu-devel] [PATCH 2/7] m25p80: add mx25l25635f chip

[Krzeminski, Marcin (Nokia - PL/Wroclaw)]

> -----Original Message-----
> From: Qemu-devel [mailto:qemu-devel-
> address@hidden On Behalf Of Cédric Le
> Goater
> Sent: Monday, July 04, 2016 2:19 PM
> To: Peter Maydell <address@hidden>; Peter Crosthwaite
> <address@hidden>
> Cc: Andrew Jeffery <address@hidden>; address@hidden; qemu-
> address@hidden; address@hidden; Cédric Le Goater
> <address@hidden>
> Subject: [Qemu-devel] [PATCH 2/7] m25p80: add mx25l25635f chip
> 
> The Macronix chip mx25l25635f used on some OpenPower boxes is very
> similar to the mx25l25635e. They share the same JEDEC identifier but the
> WRSR instruction requires 2 bytes in the mx25l25635f case.
> 
> To prevent some warnings on guests, let's introduce a new chip identifying
> JEDEC 0xc22019 as a MX25L25635F chip.
> 
Hello,

Adding fake JEDEC id is not good idea at all, we should somehow handle this in 
the code.

> Signed-off-by: Cédric Le Goater <address@hidden>
> ---
>  hw/block/m25p80.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c index
> d9b27939dde6..aa964280beab 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -199,6 +199,7 @@ static const FlashPartInfo known_devices[] = {
>      { INFO("mx25l12805d", 0xc22018,      0,  64 << 10, 256, 0) },
>      { INFO("mx25l12855e", 0xc22618,      0,  64 << 10, 256, 0) },
>      { INFO("mx25l25635e", 0xc22019,      0,  64 << 10, 512, 0) },
> +    { INFO("mx25l25635f", 0xc22019,      0,  64 << 10, 512, 0) },
>      { INFO("mx25l25655e", 0xc22619,      0,  64 << 10, 512, 0) },
>      { INFO("mx66u51235f", 0xc2253a,      0,  64 << 10, 1024, ER_4K | ER_32K) 
> },
>      { INFO("mx66u1g45g",  0xc2253b,      0,  64 << 10, 2048, ER_4K | ER_32K) 
> },
> @@ -991,6 +992,9 @@ static void decode_new_cmd(Flash *s, uint32_t
> value)
>              s->pos = 0;
>              s->len = 0;
>              s->state = STATE_COLLECTING_DATA;
> +            if (!strcmp(s->pi->part_name, "mx25l25635f")) {
> +                s->needed_bytes = 2;
> +            }
>          }
>          break;
Is it based on current master?
In my last series (and current master) this case should be handled by this code:
            case MAN_MACRONIX:
                s->needed_bytes = 2;
                s->state = STATE_COLLECTING_VAR_LEN_DATA;
                break;
I do not know what quest you are using, but linux mainline (at least 4.4.0) is 
sending only one byte,
in WRSR so this will not work.
Could you rebase to master and check if it will boot without your this change?

Thanks,
Marcin

>      case RNVCR:
> --
> 2.1.4
>