Re: [Qemu-devel] [PATCH] m25p80: provide a realize to support late inits.

["Cédric Le Goater" <clg@kaod.org>]

On 06/15/2016 04:20 PM, Paolo Bonzini wrote:
> 
> 
> On 15/06/2016 16:00, Cédric Le Goater wrote:
>> We also need to realize() the SSISlave part of the object. This is why
>> the previous realize() ops is stored in M25P80Class and called in the
>> object realize() ops.
>>
>> This is fully compatible with the existing users of m25p80 and it
>> provides a way to handle errors on the drive backend.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> 
> I think you should instead:
> 
> 1) change hw/ssi/ssi.c's ssi_slave_init to be an override of dc->realize
> 
> 2) change SSISlaveClass's init member to a realize function

OK. I will look into that. 

Thanks,

C.


> Thanks,
> 
> Paolo
> 
>> ---
>>
>>  Should apply on top of :
>>  
>>     m25p80: fix test on blk_pread() return value
>>     https://lists.gnu.org/archive/html/qemu-devel/2016-05/msg05574.html
>>
>>  hw/block/m25p80.c | 24 +++++++++++++++++-------
>>  1 file changed, 17 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
>> index 51d85960566f..c47722d3a3e5 100644
>> --- a/hw/block/m25p80.c
>> +++ b/hw/block/m25p80.c
>> @@ -28,6 +28,7 @@
>>  #include "hw/ssi/ssi.h"
>>  #include "qemu/bitops.h"
>>  #include "qemu/log.h"
>> +#include "qapi/error.h"
>>  
>>  #ifndef M25P80_ERR_DEBUG
>>  #define M25P80_ERR_DEBUG 0
>> @@ -339,6 +340,7 @@ typedef struct Flash {
>>  
>>  typedef struct M25P80Class {
>>      SSISlaveClass parent_class;
>> +    DeviceRealize parent_dc_realize;
>>      FlashPartInfo *pi;
>>  } M25P80Class;
>>  
>> @@ -880,7 +882,6 @@ static uint32_t m25p80_transfer8(SSISlave *ss, uint32_t tx)
>>  
>>  static int m25p80_init(SSISlave *ss)
>>  {
>> -    DriveInfo *dinfo;
>>      Flash *s = M25P80(ss);
>>      M25P80Class *mc = M25P80_GET_CLASS(s);
>>  
>> @@ -888,8 +889,18 @@ static int m25p80_init(SSISlave *ss)
>>  
>>      s->size = s->pi->sector_size * s->pi->n_sectors;
>>      s->dirty_page = -1;
>> +    return 0;
>> +}
>> +
>> +static void m25p80_realize(DeviceState *dev, Error **errp)
>> +{
>> +    Flash *s = M25P80(dev);
>> +    M25P80Class *mc = M25P80_GET_CLASS(s);
>> +    DriveInfo *dinfo;
>> +
>> +    /* initialize the SSISlave part */
>> +    mc->parent_dc_realize(dev, errp);
>>  
>> -    /* FIXME use a qdev drive property instead of drive_get_next() */
>>      dinfo = drive_get_next(IF_MTD);
>>  
>>      if (dinfo) {
>> @@ -899,18 +910,15 @@ static int m25p80_init(SSISlave *ss)
>>  
>>          s->storage = blk_blockalign(s->blk, s->size);
>>  
>> -        /* FIXME: Move to late init */
>>          if (blk_pread(s->blk, 0, s->storage, s->size) != s->size) {
>> -            fprintf(stderr, "Failed to initialize SPI flash!\n");
>> -            return 1;
>> +            error_setg(errp, "failed to read the initial flash content");
>> +            return;
>>          }
>>      } else {
>>          DB_PRINT_L(0, "No BDRV - binding to RAM\n");
>>          s->storage = blk_blockalign(NULL, s->size);
>>          memset(s->storage, 0xFF, s->size);
>>      }
>> -
>> -    return 0;
>>  }
>>  
>>  static void m25p80_reset(DeviceState *d)
>> @@ -967,6 +975,8 @@ static void m25p80_class_init(ObjectClass *klass, void *data)
>>      dc->vmsd = &vmstate_m25p80;
>>      dc->props = m25p80_properties;
>>      dc->reset = m25p80_reset;
>> +    mc->parent_dc_realize = dc->realize;
>> +    dc->realize = m25p80_realize;
>>      mc->pi = data;
>>  }
>>  
>>