On 5/6/19 5:17 PM, Laszlo Ersek wrote: > On 05/05/19 22:06, Philippe Mathieu-Daudé wrote: >> The pflash device is a child of TYPE_DEVICE, so it can implement >> the DeviceReset handler. Actually it has to implement it, else >> on machine reset it might stay in an incoherent state, as it has >> been reported in the buglink listed below. >> >> Add the DeviceReset handler and remove its call from the realize() >> function. >> >> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1678713 >> Reported-by: Laszlo Ersek <ler...@redhat.com> > > - IMO, the above two tags should be dropped from the commit message, as > they are specific to CFI01.
OK. > - Additionally, the commit message references the realize() function > (correctly), but the patch doesn't change that function. That is, the > patch doesn't remove the pflash_reset() call from pflash_cfi02_realize() > that was introduced in the last patch. You found a bug, having your reviewing CFI02 is useful :) I had it correct in a staged branch, then messed while rebasing. Thanks for noticing this, Phil. > > Thanks > Laszlo > >> Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com> >> --- >> hw/block/pflash_cfi02.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c >> index f321b74433c..5af367d1563 100644 >> --- a/hw/block/pflash_cfi02.c >> +++ b/hw/block/pflash_cfi02.c >> @@ -674,6 +674,11 @@ static void pflash_cfi02_realize(DeviceState *dev, >> Error **errp) >> pfl->cfi_table[0x3c] = 0x00; >> } >> >> +static void pflash_cfi02_reset(DeviceState *dev) >> +{ >> + pflash_reset(PFLASH_CFI02(dev)); >> +} >> + >> static Property pflash_cfi02_properties[] = { >> DEFINE_PROP_DRIVE("drive", PFlashCFI02, blk), >> DEFINE_PROP_UINT32("num-blocks", PFlashCFI02, nb_blocs, 0), >> @@ -701,6 +706,7 @@ static void pflash_cfi02_class_init(ObjectClass *klass, >> void *data) >> { >> DeviceClass *dc = DEVICE_CLASS(klass); >> >> + dc->reset = pflash_cfi02_reset; >> dc->realize = pflash_cfi02_realize; >> dc->unrealize = pflash_cfi02_unrealize; >> dc->props = pflash_cfi02_properties; >> >