| Message ID | 20190729145654.14644-9-damien.hedde@greensocs.com |
|---|---|
| State | New |
| Headers | show |
| Series | Multi-phase reset mechanism | expand |
On Mon, Jul 29, 2019 at 04:56:29PM +0200, Damien Hedde wrote: > It adds the possibility to add 2 gpios to control the warm and cold reset. > With theses ios, the reset can be maintained during some time. > Each io is associated with a state to detect level changes. > > Vmstate subsections are also added to the existsing device_reset > subsection. This doesn't seem like a thing that should be present on every single DeviceState. > > Signed-off-by: Damien Hedde <damien.hedde@greensocs.com> > --- > hw/core/qdev-vmstate.c | 15 ++++++++++ > hw/core/qdev.c | 65 ++++++++++++++++++++++++++++++++++++++++++ > include/hw/qdev-core.h | 57 ++++++++++++++++++++++++++++++++++++ > 3 files changed, 137 insertions(+) > > diff --git a/hw/core/qdev-vmstate.c b/hw/core/qdev-vmstate.c > index 24f8465c61..72f84c6cee 100644 > --- a/hw/core/qdev-vmstate.c > +++ b/hw/core/qdev-vmstate.c > @@ -24,10 +24,23 @@ static int device_vmstate_reset_post_load(void *opaque, int version_id) > { > DeviceState *dev = (DeviceState *) opaque; > BusState *bus; > + uint32_t io_count = 0; > + > QLIST_FOREACH(bus, &dev->child_bus, sibling) { > bus->resetting = dev->resetting; > bus->reset_is_cold = dev->reset_is_cold; > } > + > + if (dev->cold_reset_input.state) { > + io_count += 1; > + } > + if (dev->warm_reset_input.state) { > + io_count += 1; > + } > + /* ensure resetting count is coherent with io states */ > + if (dev->resetting < io_count) { > + return -1; > + } > return 0; > } > > @@ -40,6 +53,8 @@ const struct VMStateDescription device_vmstate_reset = { > .fields = (VMStateField[]) { > VMSTATE_UINT32(resetting, DeviceState), > VMSTATE_BOOL(reset_is_cold, DeviceState), > + VMSTATE_BOOL(cold_reset_input.state, DeviceState), > + VMSTATE_BOOL(warm_reset_input.state, DeviceState), > VMSTATE_END_OF_LIST() > }, > }; > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > index 88387d3743..11a4de55ea 100644 > --- a/hw/core/qdev.c > +++ b/hw/core/qdev.c > @@ -450,6 +450,67 @@ void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n) > qdev_init_gpio_in_named(dev, handler, NULL, n); > } > > +static DeviceResetInputState *device_get_reset_input_state(DeviceState *dev, > + bool cold) > +{ > + return cold ? &dev->cold_reset_input : &dev->warm_reset_input; > +} > + > +static void device_reset_handler(DeviceState *dev, bool cold, bool level) > +{ > + DeviceResetInputState *dris = device_get_reset_input_state(dev, cold); > + > + if (dris->type == DEVICE_RESET_ACTIVE_LOW) { > + level = !level; > + } > + > + if (dris->state == level) { > + /* io state has not changed */ > + return; > + } > + > + dris->state = level; > + > + if (level) { > + resettable_assert_reset(OBJECT(dev), cold); > + } else { > + resettable_deassert_reset(OBJECT(dev)); > + } > +} > + > +static void device_cold_reset_handler(void *opaque, int n, int level) > +{ > + device_reset_handler((DeviceState *) opaque, true, level); > +} > + > +static void device_warm_reset_handler(void *opaque, int n, int level) > +{ > + device_reset_handler((DeviceState *) opaque, false, level); > +} > + > +void qdev_init_reset_gpio_in_named(DeviceState *dev, const char *name, > + bool cold, DeviceResetActiveType type) > +{ > + DeviceResetInputState *dris = device_get_reset_input_state(dev, cold); > + qemu_irq_handler handler; > + > + switch (type) { > + case DEVICE_RESET_ACTIVE_LOW: > + case DEVICE_RESET_ACTIVE_HIGH: > + break; > + default: > + assert(false); > + break; > + } > + > + assert(!dris->exists); > + dris->exists = true; > + dris->type = type; > + > + handler = cold ? device_cold_reset_handler : device_warm_reset_handler; > + qdev_init_gpio_in_named(dev, handler, name, 1); > +} > + > void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq *pins, > const char *name, int n) > { > @@ -1007,6 +1068,10 @@ static void device_initfn(Object *obj) > dev->instance_id_alias = -1; > dev->realized = false; > dev->resetting = 0; > + dev->cold_reset_input.exists = false; > + dev->cold_reset_input.state = false; > + dev->warm_reset_input.exists = false; > + dev->warm_reset_input.state = false; > > object_property_add_bool(obj, "realized", > device_get_realized, device_set_realized, NULL); > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h > index 926d4bbcb1..f724ddc8f4 100644 > --- a/include/hw/qdev-core.h > +++ b/include/hw/qdev-core.h > @@ -136,6 +136,23 @@ struct NamedGPIOList { > QLIST_ENTRY(NamedGPIOList) node; > }; > > +typedef enum DeviceResetActiveType { > + DEVICE_RESET_ACTIVE_LOW, > + DEVICE_RESET_ACTIVE_HIGH, > +} DeviceResetActiveType; > + > +/** > + * DeviceResetInputState: > + * @exists: tell if io exists > + * @type: tell whether the io active low or high > + * @state: true if reset is currently active > + */ > +typedef struct DeviceResetInputState { > + bool exists; > + DeviceResetActiveType type; > + bool state; > +} DeviceResetInputState; > + > /** > * DeviceState: > * @realized: Indicates whether the device has been fully constructed. > @@ -143,6 +160,8 @@ struct NamedGPIOList { > * used to count how many times reset has been initiated on the device. > * @reset_is_cold: If the device is under reset, indicates whether it is cold > * or warm. > + * @cold_reset_input: state data for cold reset io > + * @warm_reset_input: state data for warm reset io > * > * This structure should not be accessed directly. We declare it here > * so that it can be embedded in individual device state structures. > @@ -167,6 +186,8 @@ struct DeviceState { > uint32_t resetting; > bool reset_is_cold; > bool reset_hold_needed; > + DeviceResetInputState cold_reset_input; > + DeviceResetInputState warm_reset_input; > }; > > struct DeviceListener { > @@ -372,6 +393,42 @@ static inline void qdev_init_gpio_in_named(DeviceState *dev, > void qdev_pass_gpios(DeviceState *dev, DeviceState *container, > const char *name); > > +/** > + * qdev_init_reset_gpio_in_named: > + * Create a gpio controlling the warm or cold reset of the device. > + * > + * @cold: specify whether it triggers cold or warm reset > + * @type: what kind of reset io it is > + * > + * Note: the io is considered created in its inactive state. No reset > + * is started by this function. > + */ > +void qdev_init_reset_gpio_in_named(DeviceState *dev, const char *name, > + bool cold, DeviceResetActiveType type); > + > +/** > + * qdev_init_warm_reset_gpio: > + * Create the input to control the device warm reset. > + */ > +static inline void qdev_init_warm_reset_gpio(DeviceState *dev, > + const char *name, > + DeviceResetActiveType type) > +{ > + qdev_init_reset_gpio_in_named(dev, name, false, type); > +} > + > +/** > + * qdev_init_cold_reset_gpio: > + * Create the input to control the device cold reset. > + * It can also be used as a power gate control. > + */ > +static inline void qdev_init_cold_reset_gpio(DeviceState *dev, > + const char *name, > + DeviceResetActiveType type) > +{ > + qdev_init_reset_gpio_in_named(dev, name, true, type); > +} > + > BusState *qdev_get_parent_bus(DeviceState *dev); > > /*** BUS API. ***/
On 7/31/19 8:11 AM, David Gibson wrote: > On Mon, Jul 29, 2019 at 04:56:29PM +0200, Damien Hedde wrote: >> It adds the possibility to add 2 gpios to control the warm and cold reset. >> With theses ios, the reset can be maintained during some time. >> Each io is associated with a state to detect level changes. >> >> Vmstate subsections are also added to the existsing device_reset >> subsection. > > This doesn't seem like a thing that should be present on every single > DeviceState. I can revert to previous version where the io state has to be explicitly added in devices using it. Damien
On Wed, 31 Jul 2019 at 07:33, David Gibson <david@gibson.dropbear.id.au> wrote: > > On Mon, Jul 29, 2019 at 04:56:29PM +0200, Damien Hedde wrote: > > It adds the possibility to add 2 gpios to control the warm and cold reset. > > With theses ios, the reset can be maintained during some time. > > Each io is associated with a state to detect level changes. > > > > Vmstate subsections are also added to the existsing device_reset > > subsection. > > This doesn't seem like a thing that should be present on every single > DeviceState. It's a facility that's going to be useful to multiple different subclasses of DeviceState, so it seems to me cleaner to have base class support for the common feature rather than to reimplement it entirely from scratch in every subclass that wants it. (The patch as written breaks migration compatibility -- the new fields need to go in their own vmstate subsection with a suitable 'needed' function -- but that can be fixed.) thanks -- PMM
On Mon, 29 Jul 2019 at 15:59, Damien Hedde <damien.hedde@greensocs.com> wrote: > > It adds the possibility to add 2 gpios to control the warm and cold reset. > With theses ios, the reset can be maintained during some time. > Each io is associated with a state to detect level changes. > > Vmstate subsections are also added to the existsing device_reset > subsection. > > Signed-off-by: Damien Hedde <damien.hedde@greensocs.com> > --- > hw/core/qdev-vmstate.c | 15 ++++++++++ > hw/core/qdev.c | 65 ++++++++++++++++++++++++++++++++++++++++++ > include/hw/qdev-core.h | 57 ++++++++++++++++++++++++++++++++++++ > 3 files changed, 137 insertions(+) > > diff --git a/hw/core/qdev-vmstate.c b/hw/core/qdev-vmstate.c > index 24f8465c61..72f84c6cee 100644 > --- a/hw/core/qdev-vmstate.c > +++ b/hw/core/qdev-vmstate.c > @@ -24,10 +24,23 @@ static int device_vmstate_reset_post_load(void *opaque, int version_id) > { > DeviceState *dev = (DeviceState *) opaque; > BusState *bus; > + uint32_t io_count = 0; > + > QLIST_FOREACH(bus, &dev->child_bus, sibling) { > bus->resetting = dev->resetting; > bus->reset_is_cold = dev->reset_is_cold; > } > + > + if (dev->cold_reset_input.state) { > + io_count += 1; > + } > + if (dev->warm_reset_input.state) { > + io_count += 1; > + } > + /* ensure resetting count is coherent with io states */ > + if (dev->resetting < io_count) { > + return -1; > + } > return 0; > } > > @@ -40,6 +53,8 @@ const struct VMStateDescription device_vmstate_reset = { > .fields = (VMStateField[]) { > VMSTATE_UINT32(resetting, DeviceState), > VMSTATE_BOOL(reset_is_cold, DeviceState), > + VMSTATE_BOOL(cold_reset_input.state, DeviceState), > + VMSTATE_BOOL(warm_reset_input.state, DeviceState), If we're just adding these fields to this VMStateDescription then this patch should come earlier in the series than the patch where we create and start using the fields. Otherwise there's a migration compat break between a QEMU just before this patch and a QEMU with it. I think the simplest fix is to put this patch before patches 6/7 and have a note in the commit message that this functionality can't be used until after the patch which adds the migration support. > VMSTATE_END_OF_LIST() > }, > }; > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > index 88387d3743..11a4de55ea 100644 > --- a/hw/core/qdev.c > +++ b/hw/core/qdev.c > @@ -450,6 +450,67 @@ void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n) > qdev_init_gpio_in_named(dev, handler, NULL, n); > } > > +static DeviceResetInputState *device_get_reset_input_state(DeviceState *dev, > + bool cold) > +{ > + return cold ? &dev->cold_reset_input : &dev->warm_reset_input; > +} > + > +static void device_reset_handler(DeviceState *dev, bool cold, bool level) > +{ > + DeviceResetInputState *dris = device_get_reset_input_state(dev, cold); > + > + if (dris->type == DEVICE_RESET_ACTIVE_LOW) { > + level = !level; > + } > + > + if (dris->state == level) { > + /* io state has not changed */ > + return; > + } > + > + dris->state = level; > + > + if (level) { > + resettable_assert_reset(OBJECT(dev), cold); > + } else { > + resettable_deassert_reset(OBJECT(dev)); > + } > +} > + > +static void device_cold_reset_handler(void *opaque, int n, int level) > +{ > + device_reset_handler((DeviceState *) opaque, true, level); > +} > + > +static void device_warm_reset_handler(void *opaque, int n, int level) > +{ > + device_reset_handler((DeviceState *) opaque, false, level); > +} > + > +void qdev_init_reset_gpio_in_named(DeviceState *dev, const char *name, > + bool cold, DeviceResetActiveType type) > +{ > + DeviceResetInputState *dris = device_get_reset_input_state(dev, cold); > + qemu_irq_handler handler; > + > + switch (type) { > + case DEVICE_RESET_ACTIVE_LOW: > + case DEVICE_RESET_ACTIVE_HIGH: > + break; > + default: > + assert(false); > + break; The usual way to write this is g_assert_not_reached(); (and no following 'break'). But the whole switch statement seems to be a complicated way of writing assert(type == DEVICE_RESET_ACTIVE_LOW || type == DEVICE_RESET_ACTIVE_HIGH); > + } > + > + assert(!dris->exists); > + dris->exists = true; > + dris->type = type; > + > + handler = cold ? device_cold_reset_handler : device_warm_reset_handler; > + qdev_init_gpio_in_named(dev, handler, name, 1); > +} thanks -- PMM
On 8/7/19 5:18 PM, Peter Maydell wrote: > On Mon, 29 Jul 2019 at 15:59, Damien Hedde <damien.hedde@greensocs.com> wrote: >> >> It adds the possibility to add 2 gpios to control the warm and cold reset. >> With theses ios, the reset can be maintained during some time. >> Each io is associated with a state to detect level changes. >> >> Vmstate subsections are also added to the existsing device_reset >> subsection. >> >> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com> >> --- >> hw/core/qdev-vmstate.c | 15 ++++++++++ >> hw/core/qdev.c | 65 ++++++++++++++++++++++++++++++++++++++++++ >> include/hw/qdev-core.h | 57 ++++++++++++++++++++++++++++++++++++ >> 3 files changed, 137 insertions(+) >> >> diff --git a/hw/core/qdev-vmstate.c b/hw/core/qdev-vmstate.c >> index 24f8465c61..72f84c6cee 100644 >> --- a/hw/core/qdev-vmstate.c >> +++ b/hw/core/qdev-vmstate.c >> @@ -24,10 +24,23 @@ static int device_vmstate_reset_post_load(void *opaque, int version_id) >> { >> DeviceState *dev = (DeviceState *) opaque; >> BusState *bus; >> + uint32_t io_count = 0; >> + >> QLIST_FOREACH(bus, &dev->child_bus, sibling) { >> bus->resetting = dev->resetting; >> bus->reset_is_cold = dev->reset_is_cold; >> } >> + >> + if (dev->cold_reset_input.state) { >> + io_count += 1; >> + } >> + if (dev->warm_reset_input.state) { >> + io_count += 1; >> + } >> + /* ensure resetting count is coherent with io states */ >> + if (dev->resetting < io_count) { >> + return -1; >> + } >> return 0; >> } >> >> @@ -40,6 +53,8 @@ const struct VMStateDescription device_vmstate_reset = { >> .fields = (VMStateField[]) { >> VMSTATE_UINT32(resetting, DeviceState), >> VMSTATE_BOOL(reset_is_cold, DeviceState), >> + VMSTATE_BOOL(cold_reset_input.state, DeviceState), >> + VMSTATE_BOOL(warm_reset_input.state, DeviceState), > > If we're just adding these fields to this VMStateDescription > then this patch should come earlier in the series than the > patch where we create and start using the fields. Otherwise > there's a migration compat break between a QEMU just > before this patch and a QEMU with it. I think the simplest > fix is to put this patch before patches 6/7 and have a note > in the commit message that this functionality can't be used > until after the patch which adds the migration support. Independently of the compat break you mention, maybe it is better to have 'conditional' subsection for each input ? > >> VMSTATE_END_OF_LIST() >> }, >> }; >> diff --git a/hw/core/qdev.c b/hw/core/qdev.c >> index 88387d3743..11a4de55ea 100644 >> --- a/hw/core/qdev.c >> +++ b/hw/core/qdev.c >> @@ -450,6 +450,67 @@ void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n) >> qdev_init_gpio_in_named(dev, handler, NULL, n); >> } >> >> +static DeviceResetInputState *device_get_reset_input_state(DeviceState *dev, >> + bool cold) >> +{ >> + return cold ? &dev->cold_reset_input : &dev->warm_reset_input; >> +} >> + >> +static void device_reset_handler(DeviceState *dev, bool cold, bool level) >> +{ >> + DeviceResetInputState *dris = device_get_reset_input_state(dev, cold); >> + >> + if (dris->type == DEVICE_RESET_ACTIVE_LOW) { >> + level = !level; >> + } >> + >> + if (dris->state == level) { >> + /* io state has not changed */ >> + return; >> + } >> + >> + dris->state = level; >> + >> + if (level) { >> + resettable_assert_reset(OBJECT(dev), cold); >> + } else { >> + resettable_deassert_reset(OBJECT(dev)); >> + } >> +} >> + >> +static void device_cold_reset_handler(void *opaque, int n, int level) >> +{ >> + device_reset_handler((DeviceState *) opaque, true, level); >> +} >> + >> +static void device_warm_reset_handler(void *opaque, int n, int level) >> +{ >> + device_reset_handler((DeviceState *) opaque, false, level); >> +} >> + >> +void qdev_init_reset_gpio_in_named(DeviceState *dev, const char *name, >> + bool cold, DeviceResetActiveType type) >> +{ >> + DeviceResetInputState *dris = device_get_reset_input_state(dev, cold); >> + qemu_irq_handler handler; >> + >> + switch (type) { >> + case DEVICE_RESET_ACTIVE_LOW: >> + case DEVICE_RESET_ACTIVE_HIGH: >> + break; >> + default: >> + assert(false); >> + break; > > The usual way to write this is > g_assert_not_reached(); > (and no following 'break'). > > > But the whole switch statement seems to be a complicated way > of writing > assert(type == DEVICE_RESET_ACTIVE_LOW || type == DEVICE_RESET_ACTIVE_HIGH); > >> + } >> + >> + assert(!dris->exists); >> + dris->exists = true; >> + dris->type = type; >> + >> + handler = cold ? device_cold_reset_handler : device_warm_reset_handler; >> + qdev_init_gpio_in_named(dev, handler, name, 1); >> +} > > thanks > -- PMM >
On Wed, Aug 07, 2019 at 11:37:51AM +0100, Peter Maydell wrote: > On Wed, 31 Jul 2019 at 07:33, David Gibson <david@gibson.dropbear.id.au> wrote: > > > > On Mon, Jul 29, 2019 at 04:56:29PM +0200, Damien Hedde wrote: > > > It adds the possibility to add 2 gpios to control the warm and cold reset. > > > With theses ios, the reset can be maintained during some time. > > > Each io is associated with a state to detect level changes. > > > > > > Vmstate subsections are also added to the existsing device_reset > > > subsection. > > > > This doesn't seem like a thing that should be present on every single > > DeviceState. > > It's a facility that's going to be useful to multiple different > subclasses of DeviceState, so it seems to me cleaner to > have base class support for the common feature rather than > to reimplement it entirely from scratch in every subclass > that wants it. Hm, I suppose so. Would it really have to be from scratch, though? Couldn't some suitable helper functions make adding such GPIOs to a device pretty straightforward?
On 8/9/19 7:51 AM, David Gibson wrote: > On Wed, Aug 07, 2019 at 11:37:51AM +0100, Peter Maydell wrote: >> On Wed, 31 Jul 2019 at 07:33, David Gibson <david@gibson.dropbear.id.au> wrote: >>> >>> On Mon, Jul 29, 2019 at 04:56:29PM +0200, Damien Hedde wrote: >>>> It adds the possibility to add 2 gpios to control the warm and cold reset. >>>> With theses ios, the reset can be maintained during some time. >>>> Each io is associated with a state to detect level changes. >>>> >>>> Vmstate subsections are also added to the existsing device_reset >>>> subsection. >>> >>> This doesn't seem like a thing that should be present on every single >>> DeviceState. >> >> It's a facility that's going to be useful to multiple different >> subclasses of DeviceState, so it seems to me cleaner to >> have base class support for the common feature rather than >> to reimplement it entirely from scratch in every subclass >> that wants it. > > Hm, I suppose so. Would it really have to be from scratch, though? > Couldn't some suitable helper functions make adding such GPIOs to a > device pretty straightforward? > This patch does that. A device does have to use the helper to add the gpio. Either qdev_init_warm_reset_gpio(...) or qdev_init_cold_reset_gpio(...) , like any another gpio. The mechanics to control the reset with gpio change is done in the base class and there is some state pre-allocated (and associated vmstate description) to it. If that's a problem I can only provide helpers and let devices handle state allocation and vmstate addition. Damien
On Fri, Aug 09, 2019 at 10:45:43AM +0200, Damien Hedde wrote: > > > On 8/9/19 7:51 AM, David Gibson wrote: > > On Wed, Aug 07, 2019 at 11:37:51AM +0100, Peter Maydell wrote: > >> On Wed, 31 Jul 2019 at 07:33, David Gibson <david@gibson.dropbear.id.au> wrote: > >>> > >>> On Mon, Jul 29, 2019 at 04:56:29PM +0200, Damien Hedde wrote: > >>>> It adds the possibility to add 2 gpios to control the warm and cold reset. > >>>> With theses ios, the reset can be maintained during some time. > >>>> Each io is associated with a state to detect level changes. > >>>> > >>>> Vmstate subsections are also added to the existsing device_reset > >>>> subsection. > >>> > >>> This doesn't seem like a thing that should be present on every single > >>> DeviceState. > >> > >> It's a facility that's going to be useful to multiple different > >> subclasses of DeviceState, so it seems to me cleaner to > >> have base class support for the common feature rather than > >> to reimplement it entirely from scratch in every subclass > >> that wants it. > > > > Hm, I suppose so. Would it really have to be from scratch, though? > > Couldn't some suitable helper functions make adding such GPIOs to a > > device pretty straightforward? > > > > This patch does that. A device does have to use the helper to add the > gpio. Either qdev_init_warm_reset_gpio(...) or > qdev_init_cold_reset_gpio(...) , like any another gpio. Ah, sorry, I misunderstood. That seems ok then. > > The mechanics to control the reset with gpio change is done in the base > class and there is some state pre-allocated (and associated vmstate > description) to it. > > If that's a problem I can only provide helpers and let devices handle > state allocation and vmstate addition. > > Damien >
diff --git a/hw/core/qdev-vmstate.c b/hw/core/qdev-vmstate.c index 24f8465c61..72f84c6cee 100644 --- a/hw/core/qdev-vmstate.c +++ b/hw/core/qdev-vmstate.c @@ -24,10 +24,23 @@ static int device_vmstate_reset_post_load(void *opaque, int version_id) { DeviceState *dev = (DeviceState *) opaque; BusState *bus; + uint32_t io_count = 0; + QLIST_FOREACH(bus, &dev->child_bus, sibling) { bus->resetting = dev->resetting; bus->reset_is_cold = dev->reset_is_cold; } + + if (dev->cold_reset_input.state) { + io_count += 1; + } + if (dev->warm_reset_input.state) { + io_count += 1; + } + /* ensure resetting count is coherent with io states */ + if (dev->resetting < io_count) { + return -1; + } return 0; } @@ -40,6 +53,8 @@ const struct VMStateDescription device_vmstate_reset = { .fields = (VMStateField[]) { VMSTATE_UINT32(resetting, DeviceState), VMSTATE_BOOL(reset_is_cold, DeviceState), + VMSTATE_BOOL(cold_reset_input.state, DeviceState), + VMSTATE_BOOL(warm_reset_input.state, DeviceState), VMSTATE_END_OF_LIST() }, }; diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 88387d3743..11a4de55ea 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -450,6 +450,67 @@ void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n) qdev_init_gpio_in_named(dev, handler, NULL, n); } +static DeviceResetInputState *device_get_reset_input_state(DeviceState *dev, + bool cold) +{ + return cold ? &dev->cold_reset_input : &dev->warm_reset_input; +} + +static void device_reset_handler(DeviceState *dev, bool cold, bool level) +{ + DeviceResetInputState *dris = device_get_reset_input_state(dev, cold); + + if (dris->type == DEVICE_RESET_ACTIVE_LOW) { + level = !level; + } + + if (dris->state == level) { + /* io state has not changed */ + return; + } + + dris->state = level; + + if (level) { + resettable_assert_reset(OBJECT(dev), cold); + } else { + resettable_deassert_reset(OBJECT(dev)); + } +} + +static void device_cold_reset_handler(void *opaque, int n, int level) +{ + device_reset_handler((DeviceState *) opaque, true, level); +} + +static void device_warm_reset_handler(void *opaque, int n, int level) +{ + device_reset_handler((DeviceState *) opaque, false, level); +} + +void qdev_init_reset_gpio_in_named(DeviceState *dev, const char *name, + bool cold, DeviceResetActiveType type) +{ + DeviceResetInputState *dris = device_get_reset_input_state(dev, cold); + qemu_irq_handler handler; + + switch (type) { + case DEVICE_RESET_ACTIVE_LOW: + case DEVICE_RESET_ACTIVE_HIGH: + break; + default: + assert(false); + break; + } + + assert(!dris->exists); + dris->exists = true; + dris->type = type; + + handler = cold ? device_cold_reset_handler : device_warm_reset_handler; + qdev_init_gpio_in_named(dev, handler, name, 1); +} + void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq *pins, const char *name, int n) { @@ -1007,6 +1068,10 @@ static void device_initfn(Object *obj) dev->instance_id_alias = -1; dev->realized = false; dev->resetting = 0; + dev->cold_reset_input.exists = false; + dev->cold_reset_input.state = false; + dev->warm_reset_input.exists = false; + dev->warm_reset_input.state = false; object_property_add_bool(obj, "realized", device_get_realized, device_set_realized, NULL); diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index 926d4bbcb1..f724ddc8f4 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -136,6 +136,23 @@ struct NamedGPIOList { QLIST_ENTRY(NamedGPIOList) node; }; +typedef enum DeviceResetActiveType { + DEVICE_RESET_ACTIVE_LOW, + DEVICE_RESET_ACTIVE_HIGH, +} DeviceResetActiveType; + +/** + * DeviceResetInputState: + * @exists: tell if io exists + * @type: tell whether the io active low or high + * @state: true if reset is currently active + */ +typedef struct DeviceResetInputState { + bool exists; + DeviceResetActiveType type; + bool state; +} DeviceResetInputState; + /** * DeviceState: * @realized: Indicates whether the device has been fully constructed. @@ -143,6 +160,8 @@ struct NamedGPIOList { * used to count how many times reset has been initiated on the device. * @reset_is_cold: If the device is under reset, indicates whether it is cold * or warm. + * @cold_reset_input: state data for cold reset io + * @warm_reset_input: state data for warm reset io * * This structure should not be accessed directly. We declare it here * so that it can be embedded in individual device state structures. @@ -167,6 +186,8 @@ struct DeviceState { uint32_t resetting; bool reset_is_cold; bool reset_hold_needed; + DeviceResetInputState cold_reset_input; + DeviceResetInputState warm_reset_input; }; struct DeviceListener { @@ -372,6 +393,42 @@ static inline void qdev_init_gpio_in_named(DeviceState *dev, void qdev_pass_gpios(DeviceState *dev, DeviceState *container, const char *name); +/** + * qdev_init_reset_gpio_in_named: + * Create a gpio controlling the warm or cold reset of the device. + * + * @cold: specify whether it triggers cold or warm reset + * @type: what kind of reset io it is + * + * Note: the io is considered created in its inactive state. No reset + * is started by this function. + */ +void qdev_init_reset_gpio_in_named(DeviceState *dev, const char *name, + bool cold, DeviceResetActiveType type); + +/** + * qdev_init_warm_reset_gpio: + * Create the input to control the device warm reset. + */ +static inline void qdev_init_warm_reset_gpio(DeviceState *dev, + const char *name, + DeviceResetActiveType type) +{ + qdev_init_reset_gpio_in_named(dev, name, false, type); +} + +/** + * qdev_init_cold_reset_gpio: + * Create the input to control the device cold reset. + * It can also be used as a power gate control. + */ +static inline void qdev_init_cold_reset_gpio(DeviceState *dev, + const char *name, + DeviceResetActiveType type) +{ + qdev_init_reset_gpio_in_named(dev, name, true, type); +} + BusState *qdev_get_parent_bus(DeviceState *dev); /*** BUS API. ***/
It adds the possibility to add 2 gpios to control the warm and cold reset. With theses ios, the reset can be maintained during some time. Each io is associated with a state to detect level changes. Vmstate subsections are also added to the existsing device_reset subsection. Signed-off-by: Damien Hedde <damien.hedde@greensocs.com> --- hw/core/qdev-vmstate.c | 15 ++++++++++ hw/core/qdev.c | 65 ++++++++++++++++++++++++++++++++++++++++++ include/hw/qdev-core.h | 57 ++++++++++++++++++++++++++++++++++++ 3 files changed, 137 insertions(+)