On Mon, 29 Jul 2019 at 15:59, Damien Hedde <damien.he...@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.he...@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
