| Message ID | 20221013174042.1602926-1-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | hw/ide/microdrive: Use device_cold_reset() for self-resets | expand |
On 13/10/22 19:40, Peter Maydell wrote: > Currently the microdrive code uses device_legacy_reset() to reset > itself, and has its reset method call reset on the IDE bus as the > last thing it does. Switch to using device_cold_reset(). > > The only concrete microdrive device is the TYPE_DSCM1XXXX; it is not > command-line pluggable, so it is used only by the old pxa2xx Arm > boards 'akita', 'borzoi', 'spitz', 'terrier' and 'tosa'. Candidates for deprecation? > You might think that this would result in the IDE bus being > reset automatically, but it does not, because the IDEBus type > does not set the BusClass::reset method. Instead the controller > must explicitly call ide_bus_reset(). We therefore leave that > call in md_reset(). > > Note also that because the PCMCIA card device is a direct subclass of > TYPE_DEVICE and we don't model the PCMCIA controller-to-card > interface as a qbus, PCMCIA cards are not on any qbus and so they > don't get reset when the system is reset. The reset only happens via > the dscm1xxxx_attach() and dscm1xxxx_detach() functions during > machine creation. > > Because our aim here is merely to try to get rid of calls to the > device_legacy_reset() function, we leave these other dubious > reset-related issues alone. (They all stem from this code being > absolutely ancient.) > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/ide/microdrive.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/hw/ide/microdrive.c b/hw/ide/microdrive.c > index 6df9b4cbbe1..56c5be36551 100644 > --- a/hw/ide/microdrive.c > +++ b/hw/ide/microdrive.c > @@ -175,7 +175,7 @@ static void md_attr_write(PCMCIACardState *card, uint32_t at, uint8_t value) > case 0x00: /* Configuration Option Register */ > s->opt = value & 0xcf; > if (value & OPT_SRESET) { > - device_legacy_reset(DEVICE(s)); > + device_cold_reset(DEVICE(s)); I agree this one is dubious. > } > md_interrupt_update(s); > break; > @@ -318,7 +318,7 @@ static void md_common_write(PCMCIACardState *card, uint32_t at, uint16_t value) > case 0xe: /* Device Control */ > s->ctrl = value; > if (value & CTRL_SRST) { > - device_legacy_reset(DEVICE(s)); > + device_cold_reset(DEVICE(s)); Ditto (dubious). > } > md_interrupt_update(s); > break; > @@ -543,7 +543,7 @@ static int dscm1xxxx_attach(PCMCIACardState *card) > md->attr_base = pcc->cis[0x74] | (pcc->cis[0x76] << 8); > md->io_base = 0x0; > > - device_legacy_reset(DEVICE(md)); > + device_cold_reset(DEVICE(md)); This one makes sense. > md_interrupt_update(md); > > return 0; > @@ -553,7 +553,7 @@ static int dscm1xxxx_detach(PCMCIACardState *card) > { > MicroDriveState *md = MICRODRIVE(card); > > - device_legacy_reset(DEVICE(md)); > + device_cold_reset(DEVICE(md)); Ditto (correct). > return 0; > } > Hoping this help to get ride of device_legacy_reset(): Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
diff --git a/hw/ide/microdrive.c b/hw/ide/microdrive.c index 6df9b4cbbe1..56c5be36551 100644 --- a/hw/ide/microdrive.c +++ b/hw/ide/microdrive.c @@ -175,7 +175,7 @@ static void md_attr_write(PCMCIACardState *card, uint32_t at, uint8_t value) case 0x00: /* Configuration Option Register */ s->opt = value & 0xcf; if (value & OPT_SRESET) { - device_legacy_reset(DEVICE(s)); + device_cold_reset(DEVICE(s)); } md_interrupt_update(s); break; @@ -318,7 +318,7 @@ static void md_common_write(PCMCIACardState *card, uint32_t at, uint16_t value) case 0xe: /* Device Control */ s->ctrl = value; if (value & CTRL_SRST) { - device_legacy_reset(DEVICE(s)); + device_cold_reset(DEVICE(s)); } md_interrupt_update(s); break; @@ -543,7 +543,7 @@ static int dscm1xxxx_attach(PCMCIACardState *card) md->attr_base = pcc->cis[0x74] | (pcc->cis[0x76] << 8); md->io_base = 0x0; - device_legacy_reset(DEVICE(md)); + device_cold_reset(DEVICE(md)); md_interrupt_update(md); return 0; @@ -553,7 +553,7 @@ static int dscm1xxxx_detach(PCMCIACardState *card) { MicroDriveState *md = MICRODRIVE(card); - device_legacy_reset(DEVICE(md)); + device_cold_reset(DEVICE(md)); return 0; }
Currently the microdrive code uses device_legacy_reset() to reset itself, and has its reset method call reset on the IDE bus as the last thing it does. Switch to using device_cold_reset(). The only concrete microdrive device is the TYPE_DSCM1XXXX; it is not command-line pluggable, so it is used only by the old pxa2xx Arm boards 'akita', 'borzoi', 'spitz', 'terrier' and 'tosa'. You might think that this would result in the IDE bus being reset automatically, but it does not, because the IDEBus type does not set the BusClass::reset method. Instead the controller must explicitly call ide_bus_reset(). We therefore leave that call in md_reset(). Note also that because the PCMCIA card device is a direct subclass of TYPE_DEVICE and we don't model the PCMCIA controller-to-card interface as a qbus, PCMCIA cards are not on any qbus and so they don't get reset when the system is reset. The reset only happens via the dscm1xxxx_attach() and dscm1xxxx_detach() functions during machine creation. Because our aim here is merely to try to get rid of calls to the device_legacy_reset() function, we leave these other dubious reset-related issues alone. (They all stem from this code being absolutely ancient.) Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/ide/microdrive.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)