[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] Odp.: [PATCH 6/9] m25p80: Introduce quad and equad mode
From: |
Cédric Le Goater |
Subject: |
Re: [Qemu-devel] Odp.: [PATCH 6/9] m25p80: Introduce quad and equad modes. |
Date: |
Fri, 17 Jun 2016 14:43:09 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.1.0 |
On 06/15/2016 07:40 PM, Krzeminski, Marcin (Nokia - PL/Wroclaw) wrote:
>
>
> W dniu 15.06.2016 o 16:25, Cédric Le Goater pisze:
>> On 06/15/2016 03:41 PM, address@hidden wrote:
>>> From: Marcin Krzeminski <address@hidden>
>>>
>>> Quad and Equad modes for Spansion and Macronix flash devices.
>>> This commit also includes modification and new command to manipulate
>>> quad mode (status registers and dedicated commands).
>>> This work is based on Pawel Lenkow work.
>>>
>>> Signed-off-by: Marcin Krzeminski <address@hidden>
>>> ---
>>> hw/block/m25p80.c | 70
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++----
>>> 1 file changed, 65 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
>>> index 55b4377..d1c4d46 100644
>>> --- a/hw/block/m25p80.c
>>> +++ b/hw/block/m25p80.c
>>> @@ -281,6 +281,7 @@ typedef enum {
>>> JEDEC_READ = 0x9f,
>>> BULK_ERASE = 0xc7,
>>> READ_FSR = 0x70,
>>> + RDCR = 0x15,
>>>
>>> READ = 0x03,
>>> READ4 = 0x13,
>>> @@ -317,6 +318,13 @@ typedef enum {
>>> RESET_ENABLE = 0x66,
>>> RESET_MEMORY = 0x99,
>>>
>>> + /*
>>> + * Micron: 0x35 - enable QPI
>>> + * Spansion: 0x35 - read control register
>>> + */
>>> + RDCR_EQIO = 0x35,
>>> + RSTQIO = 0xf5,
>>> +
>>> RNVCR = 0xB5,
>>> WNVCR = 0xB1,
>>>
>>> @@ -366,6 +374,7 @@ typedef struct Flash {
>>> bool write_enable;
>>> bool four_bytes_address_mode;
>>> bool reset_enable;
>>> + bool quad_enable;
>>> uint8_t ear;
>>>
>>> int64_t dirty_page;
>>> @@ -586,6 +595,16 @@ static void complete_collecting_data(Flash *s)
>>> flash_erase(s, s->cur_addr, s->cmd_in_progress);
>>> break;
>>> case WRSR:
>>> + switch (get_man(s)) {
>>> + case MAN_SPANSION:
>>> + s->quad_enable = !!(s->data[1] & 0x02);
>>> + break;
>>> + case MAN_MACRONIX:
>>> + s->quad_enable = extract32(s->data[0], 6, 1);
>>> + break;
>>> + default:
>>> + break;
>>> + }
>>> if (s->write_enable) {
>>> s->write_enable = false;
>>> }
>>> @@ -619,6 +638,7 @@ static void reset_memory(Flash *s)
>>> s->state = STATE_IDLE;
>>> s->write_enable = false;
>>> s->reset_enable = false;
>>> + s->quad_enable = false;
>>>
>>> switch (get_man(s)) {
>>> case MAN_NUMONYX:
>>> @@ -747,10 +767,20 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>>>
>>> case WRSR:
>>> if (s->write_enable) {
>>> - s->needed_bytes = 1;
>>> + switch (get_man(s)) {
>>> + case MAN_SPANSION:
>>> + s->needed_bytes = 2;
>>> + s->state = STATE_COLLECTING_DATA;
>>> + break;
>>> + case MAN_MACRONIX:
>>> + s->needed_bytes = 2;
>>> + s->state = STATE_COLLECTING_VAR_LEN_DATA;
>>> + break;
>>
>> ah. I am glad you address this topic because I am having a little issue
>> with the mx25l25635e and the mx25l25635f.
>>
>> mx25l25635e is a macronix which supports the WRSR command with one byte
>> and the mx25l25635f needs two. The fun part is that they have the same
>> JEDEC : 0xc22019.
>>
>> So not all macronix need 2 bytes. Should we add a flag for that ?
>>
> I think this case should be partially handled with path 4 and this one.
> The new state (patch 4) was introduced to allow use variable write cmds.
> As in my case mx66u51235f allow write up to 2 bytes, but linux writes only
> one,
> and that is allowed and works on real HW. From the datasheet mx25l25635f
> should behave exactly same way.
> In case you mention, the problem might pop up, when guest will try
> to write 2 bytes to mx25l25635e. Real HW will treat second byte as new
> command,
> but this model accept this situation.
qemu complains a bit but that's OK.
> Generally my approach and I think original author was to not emulate exactly
> real hw behaviour but allow to model it in such way the quest can boot up and
> use it.
> If we go witch very restricted command behaviour we will end up in unreadable
> code
> (eg. newest Spansions/Cypress family does not support extended address
> register,
> but such commands will work in this model). The question is if you really
> need to flag
> and support this. If yes additional if is not a problem.
I will wait for your patchset to be merged and then see if the mx25l25635f
needs anything more. This is really minor. It should be a onliner.
Thanks,
C.
>>
>>> + default:
>>> + s->needed_bytes = 1;
>>> + s->state = STATE_COLLECTING_DATA;
>>> + }
>>> s->pos = 0;
>>> - s->len = 0;
>>> - s->state = STATE_COLLECTING_DATA;
>>> }
>>> break;
>>>
>>> @@ -763,6 +793,9 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>>>
>>> case RDSR:
>>> s->data[0] = (!!s->write_enable) << 1;
>>> + if (get_man(s) == MAN_MACRONIX) {
>>> + s->data[0] |= (!!s->quad_enable) << 6;
>>> + }
>>> s->pos = 0;
>>> s->len = 1;
>>> s->state = STATE_READING_DATA;
>>> @@ -789,6 +822,14 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>>> s->state = STATE_READING_DATA;
>>> break;
>>>
>>> + case RDCR:
>>> + s->data[0] = s->volatile_cfg & 0xFF;
>>> + s->data[0] |= (!!s->four_bytes_address_mode) << 5;
>>> + s->pos = 0;
>>> + s->len = 1;
>>> + s->state = STATE_READING_DATA;
>>> + break;
>>> +
>>> case BULK_ERASE:
>>> if (s->write_enable) {
>>> DB_PRINT_L(0, "chip erase\n");
>>> @@ -828,7 +869,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>>> s->state = STATE_READING_DATA;
>>> break;
>>> case WNVCR:
>>> - if (s->write_enable) {
>>> + if (s->write_enable && get_man(s) == MAN_NUMONYX) {
>>> s->needed_bytes = 2;
>>> s->pos = 0;
>>> s->len = 0;
>>> @@ -871,6 +912,24 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>>> reset_memory(s);
>>> }
>>> break;
>>> + case RDCR_EQIO:
>>> + switch (get_man(s)) {
>>> + case MAN_SPANSION:
>>> + s->data[0] = (!!s->quad_enable) << 1;
>>> + s->pos = 0;
>>> + s->len = 1;
>>> + s->state = STATE_READING_DATA;
>>> + break;
>>> + case MAN_MACRONIX:
>>> + s->quad_enable = true;
>>> + break;
>>> + default:
>>> + break;
>>> + }
>>> + break;
>>> + case RSTQIO:
>>> + s->quad_enable = false;
>>> + break;
>>> default:
>>> qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Unknown cmd %x\n", value);
>>> break;
>>> @@ -999,7 +1058,7 @@ static Property m25p80_properties[] = {
>>>
>>> static const VMStateDescription vmstate_m25p80 = {
>>> .name = "xilinx_spi",
>>> - .version_id = 2,
>>> + .version_id = 3,
>>> .minimum_version_id = 1,
>>> .pre_save = m25p80_pre_save,
>>> .fields = (VMStateField[]) {
>>> @@ -1017,6 +1076,7 @@ static const VMStateDescription vmstate_m25p80 = {
>>> VMSTATE_UINT32_V(nonvolatile_cfg, Flash, 2),
>>> VMSTATE_UINT32_V(volatile_cfg, Flash, 2),
>>> VMSTATE_UINT32_V(enh_volatile_cfg, Flash, 2),
>>> + VMSTATE_BOOL_V(quad_enable, Flash, 3),
>>> VMSTATE_END_OF_LIST()
>>> }
>>> };
>>>
>>
>>
>>
>
[Qemu-devel] [PATCH 2/9] m25p80: Make a table for JEDEC ID., marcin.krzeminski, 2016/06/15
[Qemu-devel] [PATCH 8/9] m25p80: Fast read commands family changes., marcin.krzeminski, 2016/06/15