[PATCH 22/50] pckbd: implement i8042_mmio_reset() for I8042_MMIO device

Mark Cave-Ayland posted 50 patches 3 years, 8 months ago
Maintainers: Richard Henderson <richard.henderson@linaro.org>, Helge Deller <deller@gmx.de>, "Michael S. Tsirkin" <mst@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, "Hervé Poussineau" <hpoussin@reactos.org>, Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>
There is a newer version of this series
[PATCH 22/50] pckbd: implement i8042_mmio_reset() for I8042_MMIO device
Posted by Mark Cave-Ayland 3 years, 8 months ago
This allows the I8042_MMIO reset function to be registered directly within the
DeviceClass rather than using qemu_register_reset() directly.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/input/pckbd.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
index bbdd3bc568..df443aaff2 100644
--- a/hw/input/pckbd.c
+++ b/hw/input/pckbd.c
@@ -665,10 +665,20 @@ static const MemoryRegionOps i8042_mmio_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
+static void i8042_mmio_reset(DeviceState *dev)
+{
+    MMIOKBDState *s = I8042_MMIO(dev);
+    KBDState *ks = &s->kbd;
+
+    ks->extended_state = true;
+    kbd_reset(ks);
+}
+
 static void i8042_mmio_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
+    dc->reset = i8042_mmio_reset;
     set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
 }
 
@@ -687,15 +697,12 @@ void i8042_mm_init(qemu_irq kbd_irq, qemu_irq mouse_irq,
     s->irq_mouse = mouse_irq;
     s->mask = mask;
 
-    s->extended_state = true;
-
     vmstate_register(NULL, 0, &vmstate_kbd, s);
 
     memory_region_init_io(region, NULL, &i8042_mmio_ops, s, "i8042", size);
 
     s->kbd = ps2_kbd_init(kbd_update_kbd_irq, s);
     s->mouse = ps2_mouse_init(kbd_update_aux_irq, s);
-    qemu_register_reset(kbd_reset, s);
 }
 
 static const TypeInfo i8042_mmio_info = {
-- 
2.20.1
Re: [PATCH 22/50] pckbd: implement i8042_mmio_reset() for I8042_MMIO device
Posted by Peter Maydell 3 years, 8 months ago
On Sun, 22 May 2022 at 19:19, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> This allows the I8042_MMIO reset function to be registered directly within the
> DeviceClass rather than using qemu_register_reset() directly.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/input/pckbd.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
> index bbdd3bc568..df443aaff2 100644
> --- a/hw/input/pckbd.c
> +++ b/hw/input/pckbd.c
> @@ -665,10 +665,20 @@ static const MemoryRegionOps i8042_mmio_ops = {
>      .endianness = DEVICE_NATIVE_ENDIAN,
>  };
>
> +static void i8042_mmio_reset(DeviceState *dev)
> +{
> +    MMIOKBDState *s = I8042_MMIO(dev);
> +    KBDState *ks = &s->kbd;
> +
> +    ks->extended_state = true;
> +    kbd_reset(ks);
> +}

extended_state is not runtime guest-changeable state, it's a
device property that's set at device creation time. So we
shouldn't be setting it to 'true' here, but instead in the
device init or realize function.

thanks
-- PMM
Re: [PATCH 22/50] pckbd: implement i8042_mmio_reset() for I8042_MMIO device
Posted by Mark Cave-Ayland 3 years, 8 months ago
On 09/06/2022 11:49, Peter Maydell wrote:

> On Sun, 22 May 2022 at 19:19, Mark Cave-Ayland
> <mark.cave-ayland@ilande.co.uk> wrote:
>>
>> This allows the I8042_MMIO reset function to be registered directly within the
>> DeviceClass rather than using qemu_register_reset() directly.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>>   hw/input/pckbd.c | 13 ++++++++++---
>>   1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
>> index bbdd3bc568..df443aaff2 100644
>> --- a/hw/input/pckbd.c
>> +++ b/hw/input/pckbd.c
>> @@ -665,10 +665,20 @@ static const MemoryRegionOps i8042_mmio_ops = {
>>       .endianness = DEVICE_NATIVE_ENDIAN,
>>   };
>>
>> +static void i8042_mmio_reset(DeviceState *dev)
>> +{
>> +    MMIOKBDState *s = I8042_MMIO(dev);
>> +    KBDState *ks = &s->kbd;
>> +
>> +    ks->extended_state = true;
>> +    kbd_reset(ks);
>> +}
> 
> extended_state is not runtime guest-changeable state, it's a
> device property that's set at device creation time. So we
> shouldn't be setting it to 'true' here, but instead in the
> device init or realize function.

Ah oops. I'll move it to a corresponding device init function in v2.


ATB,

Mark.