On 07/04/2016 02:57 PM, Krzeminski, Marcin (Nokia - PL/Wroclaw) wrote:
>
>
>> -----Original Message-----
>> From: Qemu-devel [mailto:qemu-devel-
>> bounces+marcin.krzeminski=nokia....@nongnu.org] On Behalf Of Cédric Le
>> Goater
>> Sent: Monday, July 04, 2016 2:19 PM
>> To: Peter Maydell <peter.mayd...@linaro.org>; Peter Crosthwaite
>> <crosthwaite.pe...@gmail.com>
>> Cc: Andrew Jeffery <and...@aj.id.au>; qemu-...@nongnu.org; qemu-
>> de...@nongnu.org; mar.krzemin...@gmail.com; Cédric Le Goater
>> <c...@kaod.org>
>> 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.
It is not a fake JEDEC id. It is a real one from the datasheet :)
mx25l25635f and mx25l25635e share the same 0xc22019 JEDEC.
>> Signed-off-by: Cédric Le Goater <c...@kaod.org>
>> ---
>> 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?
You are right. The patch has bit rotted, it still applies on qemu's HEAD
but at the wrong place :/ But, I don't think it is needed anymore, see
below.
> 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.
It is not Linux, it is a user space tool :
https://github.com/open-power/skiboot/blob/master/hw/ast-bmc/ast-sf-ctrl.c#L465
> Could you rebase to master and check if it will boot without your this change?
So the current code is fine for the mx25l25635f, which takes 2 bytes
for the WRSR. But, now, how would the m25p80 object behave with the
mx25l25635e ? Only one byte will be written.
Should we deprecate mx25l25635e ? It is not used in qemu.
Thanks,
C.
