| Message ID | 1465999230-28050-1-git-send-email-clg@kaod.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 15 June 2016 at 15:00, Cédric Le Goater <clg@kaod.org> 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> > --- > +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); Why have you removed the FIXME comment ? thanks -- PMM
On 06/15/2016 04:07 PM, Peter Maydell wrote: > On 15 June 2016 at 15:00, Cédric Le Goater <clg@kaod.org> 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> >> --- > >> +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); > > Why have you removed the FIXME comment ? This is an error. I should not have removed this one. Only the one related on late-init :/ C.
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 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; > } > >
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; >> } >> >>
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; }
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> --- 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(-)