[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] Odp.: Odp.: [Qemu-devel] [PATCH 2/7] m25p80: add mx25l256
From: |
Cédric Le Goater |
Subject: |
Re: [Qemu-arm] Odp.: Odp.: [Qemu-devel] [PATCH 2/7] m25p80: add mx25l25635f chip |
Date: |
Mon, 4 Jul 2016 18:18:06 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.1.0 |
On 07/04/2016 06:03 PM, Krzeminski, Marcin (Nokia - PL/Wroclaw) wrote:
>
>
> W dniu 04.07.2016 o 17:48, Cédric Le Goater pisze:
>> On 07/04/2016 05:23 PM, Krzeminski, Marcin (Nokia - PL/Wroclaw) wrote:
>>>
>>>
>>> W dniu 04.07.2016 o 15:41, Cédric Le Goater pisze:
>>>> On 07/04/2016 02:57 PM, Krzeminski, Marcin (Nokia - PL/Wroclaw) wrote:
>>>>>
>>>>>
>>>>>> -----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.
>>>>
>>>> It is not a fake JEDEC id. It is a real one from the datasheet :)
>>>>
>>>> mx25l25635f and mx25l25635e share the same 0xc22019 JEDEC.
>>> Sorry, I misunderstand and haven't check in data-sheet :)
>>
>> np.
>>
>>
>>>>>> 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?
>>>>
>>>> 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
>>> I meant spi-nor framework in Linux, if you are using different tool it is
>>> different story,
>>> but should work for both (if real hw works for both...).
>>
>> Yes.
>>
>> Linux still does not know about the mx25l25635f and it is behaving
>> fine using the mx25l25635e definition. We should send a patch for
>> it I guess. I have the HW, I will look into that when I have some
>> time.
> Linux detects flash type by JEDEC code, so if the JEDEC is the same
> it will be hard to patch this. They are working on detection by JEDEC216,
> and then probably it will be possible to properly identify the flash.
>
>>>>> 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.
>>>
>>> It should behave properly (as in my case spi-nor framework in Linux writes
>>> only
>>> one byte for Macronix in WSRS). COLLECTING_VAR_LEN_DATA state apply changes
>>> (call complete_collecting_data) when CS go high. If it takes one byte
>>> instead of two,
>>> one byte will be written to SR.
>>
>> ah yes, this is true. so we are fine on that side.
>>
>>>> Should we deprecate mx25l25635e ? It is not used in qemu.
>>>
>>> That is good question, I do not see any flash with same JEDEC in the code,
>>> so
>>> this one could be the first if you leave both of them.
>>> You mentioned about warnings when you configure Qemu to use mx25l25635e,
>>> instead of mx25l25635f, in my it should not matter, what are those warnings?
>>
>> Before your patchset, we could get these:
>>
>> qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Unknown cmd %x\n", value);
>>
>> you have solved that.
>>
>> So I guess we could just add :
>>
>> + { INFO("mx25l25635f", 0xc22019, 0, 64 << 10, 512, 0) },
>>
>> but as we don't have any errors without them, I will just drop
>> those oneliner patches.
>
> I think you should add this line, since this model is supported.
OK. Next round then.
Thanks,
C.
> Thanks,
> Marcin
>>
>>
>> Thanks,
>>
>> C.
>>
>
- Re: [Qemu-arm] [PATCH 1/7] tests: add a m25p80 test, (continued)
[Qemu-arm] [PATCH 2/7] m25p80: add mx25l25635f chip, Cédric Le Goater, 2016/07/04
- Re: [Qemu-arm] [Qemu-devel] [PATCH 2/7] m25p80: add mx25l25635f chip, Krzeminski, Marcin (Nokia - PL/Wroclaw), 2016/07/04
- Re: [Qemu-arm] [Qemu-devel] [PATCH 2/7] m25p80: add mx25l25635f chip, Cédric Le Goater, 2016/07/04
- [Qemu-arm] Odp.: [Qemu-devel] [PATCH 2/7] m25p80: add mx25l25635f chip, Krzeminski, Marcin (Nokia - PL/Wroclaw), 2016/07/04
- Re: [Qemu-arm] Odp.: [Qemu-devel] [PATCH 2/7] m25p80: add mx25l25635f chip, Cédric Le Goater, 2016/07/04
- [Qemu-arm] Odp.: Odp.: [Qemu-devel] [PATCH 2/7] m25p80: add mx25l25635f chip, Krzeminski, Marcin (Nokia - PL/Wroclaw), 2016/07/04
- Re: [Qemu-arm] Odp.: Odp.: [Qemu-devel] [PATCH 2/7] m25p80: add mx25l25635f chip,
Cédric Le Goater <=
[Qemu-arm] [PATCH 3/7] ast2400: use a mx25l25635f chip, Cédric Le Goater, 2016/07/04
[Qemu-arm] [PATCH 4/7] m25p80: add a m25p80_set_rom_storage() routine, Cédric Le Goater, 2016/07/04
Re: [Qemu-arm] [PATCH 4/7] m25p80: add a m25p80_set_rom_storage() routine, Peter Crosthwaite, 2016/07/09
Re: [Qemu-arm] [PATCH 4/7] m25p80: add a m25p80_set_rom_storage() routine, Cédric Le Goater, 2016/07/11
[Qemu-arm] [PATCH 5/7] ast2400: handle SPI flash Command mode (read only), Cédric Le Goater, 2016/07/04