[Qemu-devel] ODP: [PATCH 02/12] Added reset-pin emulation in model.

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

W dniu 21.12.2015 o 12:04, Peter Crosthwaite pisze:
> On Wed, Dec 16, 2015 at 4:57 AM,  <address@hidden> wrote:
>> From: Marcin Krzeminski <address@hidden>
>>
>> Signed-off-by: Marcin Krzeminski <address@hidden>
>> ---
>>  hw/block/m25p80.c | 38 +++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 37 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
>> index bfd493f..bcb66a5 100644
>> --- a/hw/block/m25p80.c
>> +++ b/hw/block/m25p80.c
>> @@ -258,6 +258,8 @@ typedef struct Flash {
>>      uint8_t cmd_in_progress;
>>      uint64_t cur_addr;
>>      bool write_enable;
>> +    bool initialized;
>> +    uint8_t reset_pin;
>>
>>      int64_t dirty_page;
>>
>> @@ -430,6 +432,19 @@ static void complete_collecting_data(Flash *s)
>>      }
>>  }
>>
>> +static void reset_memory(Flash *s)
>> +{
>> +    s->cmd_in_progress = NOP;
>> +    s->cur_addr = 0;
>> +    s->len = 0;
>> +    s->needed_bytes = 0;
>> +    s->pos = 0;
>> +    s->state = STATE_IDLE;
>> +    s->write_enable = false;
>> +
>> +    DB_PRINT_L(0, "Reset done.\n");
>> +}
>> +
>>  static void decode_new_cmd(Flash *s, uint32_t value)
>>  {
>>      s->cmd_in_progress = value;
>> @@ -620,6 +635,8 @@ static int m25p80_init(SSISlave *ss)
>>      s->size = s->pi->sector_size * s->pi->n_sectors;
>>      s->dirty_page = -1;
>>
>> +    s->initialized = false;
>> +
>
> Init functions should never set runtime state.
>
>>      /* FIXME use a qdev drive property instead of drive_get_next() */
>>      dinfo = drive_get_next(IF_MTD);
>>
>> @@ -650,9 +667,24 @@ static void m25p80_pre_save(void *opaque)
>>      flash_sync_dirty((Flash *)opaque, -1);
>>  }
>>
>> +static void m25p80_reset(DeviceState *d)
>> +{
>> +    Flash *s = M25P80(d);
>> +
>> +    if (s->reset_pin || !s->initialized) {
>> +        reset_memory(s);
>> +        s->initialized = true;
>
> Bus I think the real issue here is that is trying to model something
> other than a power-on-reset. Looks like a a soft reset. the dc->reset
> should be a power-on-reset and not be conditional on anything. Only
> the non-volatile state (the storage data) should persist.
>
> Is your board using qemu_system_reset_request() by any chance for
> something that is not supposed to reset the whole board? If this is
> the case, you need to limit the scope of that reset logic and just set
> your board up to not use the centralized all-devices reset (which is
> pretty broken for these modern boards/SoCs that have multiple reset
> domains).
>
> Regards,
> Peter
That is not exactly my case.
I want to emulate device that have a RESET signal functionality (eg. n25q128 
has such option).
The other think that I want to have is possibility to use this signal by qemu 
model
(eg. pin is connected on board to reset IC, or it is unused).
The use case: qemu boots us powered-up board, then I issue 
qemu_system_reset_request().
Then if the pin is connected (propeti is set, flash wil also reset, in other 
case will not).
I don't see why it against reset signal implementation.

Regards,
Marcin

>
>> +    }
>> +}
>> +
>> +static Property m25p80_properties[] = {
>> +    DEFINE_PROP_UINT8("reset-pin", Flash, reset_pin, 0),
>> +    DEFINE_PROP_END_OF_LIST(),
>> +};
>> +
>>  static const VMStateDescription vmstate_m25p80 = {
>>      .name = "xilinx_spi",
>> -    .version_id = 1,
>> +    .version_id = 2,
>>      .minimum_version_id = 1,
>>      .pre_save = m25p80_pre_save,
>>      .fields = (VMStateField[]) {
>> @@ -664,6 +696,8 @@ static const VMStateDescription vmstate_m25p80 = {
>>          VMSTATE_UINT8(cmd_in_progress, Flash),
>>          VMSTATE_UINT64(cur_addr, Flash),
>>          VMSTATE_BOOL(write_enable, Flash),
>> +        VMSTATE_BOOL(initialized, Flash),
>> +        VMSTATE_UINT8(reset_pin, Flash),
>>          VMSTATE_END_OF_LIST()
>>      }
>>  };
>> @@ -679,6 +713,8 @@ static void m25p80_class_init(ObjectClass *klass, void 
>> *data)
>>      k->set_cs = m25p80_cs;
>>      k->cs_polarity = SSI_CS_LOW;
>>      dc->vmsd = &vmstate_m25p80;
>> +    dc->reset = &m25p80_reset;
>> +    dc->props = m25p80_properties;
>>      mc->pi = data;
>>  }
>>
>> --
>> 2.5.0
>>
>>
>
>