prep: fix keyboard for the 40p machine

[Qemu-devel] [PATCH] prep: fix keyboard for the 40p machine

Posted by Mark Cave-Ayland 7 years, 5 months ago

Commit 72d3d8f052 "hw/isa/superio: Add a keyboard/mouse controller (8042)"
added an 8042 keyboard device to the PC87312 superio device to replace that
being used by the prep machine.

Unfortunately this commit didn't do the same for the 40p machine which broke
the keyboard by registering two 8042 keyboard devices at the same address.

Resolve this by similarly removing the 8042 keyboard from the 40p machine as
done for the prep machine in commit 72d3d8f052.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/ppc/prep.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index a1e7219db6..be4db6a687 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -770,7 +770,6 @@ static void ibm_40p_init(MachineState *machine)
 
     /* add some more devices */
     if (defaults_enabled()) {
-        isa_create_simple(isa_bus, TYPE_I8042);
         m48t59 = NVRAM(isa_create_simple(isa_bus, "isa-m48t59"));
 
         dev = DEVICE(isa_create(isa_bus, "cs4231a"));
-- 
2.11.0

Re: [Qemu-devel] [PATCH] prep: fix keyboard for the 40p machine

Posted by Mark Cave-Ayland 7 years, 5 months ago

On 24/05/18 06:39, Mark Cave-Ayland wrote:

> Commit 72d3d8f052 "hw/isa/superio: Add a keyboard/mouse controller (8042)"
> added an 8042 keyboard device to the PC87312 superio device to replace that
> being used by the prep machine.
> 
> Unfortunately this commit didn't do the same for the 40p machine which broke
> the keyboard by registering two 8042 keyboard devices at the same address.
> 
> Resolve this by similarly removing the 8042 keyboard from the 40p machine as
> done for the prep machine in commit 72d3d8f052.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   hw/ppc/prep.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index a1e7219db6..be4db6a687 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
> @@ -770,7 +770,6 @@ static void ibm_40p_init(MachineState *machine)
>   
>       /* add some more devices */
>       if (defaults_enabled()) {
> -        isa_create_simple(isa_bus, TYPE_I8042);
>           m48t59 = NVRAM(isa_create_simple(isa_bus, "isa-m48t59"));
>   
>           dev = DEVICE(isa_create(isa_bus, "cs4231a"));
> 

FWIW I see this patchset was applied just before the 2.12 release so 
it's probably worth a CC to qemu-stable@ once Hervé is satisfied.


ATB,

Mark.

Re: [Qemu-devel] [PATCH] prep: fix keyboard for the 40p machine

Posted by Philippe Mathieu-Daudé 7 years, 5 months ago

On 05/24/2018 02:39 AM, Mark Cave-Ayland wrote:
> Commit 72d3d8f052 "hw/isa/superio: Add a keyboard/mouse controller (8042)"
> added an 8042 keyboard device to the PC87312 superio device to replace that
> being used by the prep machine.
> 
> Unfortunately this commit didn't do the same for the 40p machine which broke
> the keyboard by registering two 8042 keyboard devices at the same address.

Oops sorry... I have this fixed in the following up series after SuperIO
cleanup, which is SouthBridge cleanup, involving a good rework of the
PIIX and I82378 chipsets, using Hervé Poussineau patches.
But I have 2 more prioritary series to finish before returning to this
one :/

> Resolve this by similarly removing the 8042 keyboard from the 40p machine as
> done for the prep machine in commit 72d3d8f052.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Thanks for fixing this (too bad there are no keyboard qtests and this
got unnoticed).

> ---
>  hw/ppc/prep.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index a1e7219db6..be4db6a687 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
> @@ -770,7 +770,6 @@ static void ibm_40p_init(MachineState *machine)
>  
>      /* add some more devices */
>      if (defaults_enabled()) {
> -        isa_create_simple(isa_bus, TYPE_I8042);
>          m48t59 = NVRAM(isa_create_simple(isa_bus, "isa-m48t59"));
>  
>          dev = DEVICE(isa_create(isa_bus, "cs4231a"));
>

Re: [Qemu-devel] [Qemu-ppc] [PATCH] prep: fix keyboard for the 40p machine

Posted by Thomas Huth 7 years, 5 months ago

On 24.05.2018 18:20, Philippe Mathieu-Daudé wrote:
> On 05/24/2018 02:39 AM, Mark Cave-Ayland wrote:
>> Commit 72d3d8f052 "hw/isa/superio: Add a keyboard/mouse controller (8042)"
>> added an 8042 keyboard device to the PC87312 superio device to replace that
>> being used by the prep machine.
>>
>> Unfortunately this commit didn't do the same for the 40p machine which broke
>> the keyboard by registering two 8042 keyboard devices at the same address.
[...]
> Thanks for fixing this (too bad there are no keyboard qtests and this
> got unnoticed).

Then it's time to write some tests? ;-)

 Thomas

Re: [Qemu-devel] [PATCH] prep: fix keyboard for the 40p machine

Posted by Hervé Poussineau 7 years, 5 months ago

Le 24/05/2018 à 07:39, Mark Cave-Ayland a écrit :
> Commit 72d3d8f052 "hw/isa/superio: Add a keyboard/mouse controller (8042)"
> added an 8042 keyboard device to the PC87312 superio device to replace that
> being used by the prep machine.
> 
> Unfortunately this commit didn't do the same for the 40p machine which broke
> the keyboard by registering two 8042 keyboard devices at the same address.
> 
> Resolve this by similarly removing the 8042 keyboard from the 40p machine as
> done for the prep machine in commit 72d3d8f052.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Hervé Poussineau <hpoussin@reactos.org>

> ---
>   hw/ppc/prep.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index a1e7219db6..be4db6a687 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
> @@ -770,7 +770,6 @@ static void ibm_40p_init(MachineState *machine)
>   
>       /* add some more devices */
>       if (defaults_enabled()) {
> -        isa_create_simple(isa_bus, TYPE_I8042);
>           m48t59 = NVRAM(isa_create_simple(isa_bus, "isa-m48t59"));
>   
>           dev = DEVICE(isa_create(isa_bus, "cs4231a"));
>

Re: [Qemu-devel] [PATCH] prep: fix keyboard for the 40p machine

Posted by David Gibson 7 years, 4 months ago

On Thu, May 24, 2018 at 06:39:58AM +0100, Mark Cave-Ayland wrote:
> Commit 72d3d8f052 "hw/isa/superio: Add a keyboard/mouse controller (8042)"
> added an 8042 keyboard device to the PC87312 superio device to replace that
> being used by the prep machine.
> 
> Unfortunately this commit didn't do the same for the 40p machine which broke
> the keyboard by registering two 8042 keyboard devices at the same address.
> 
> Resolve this by similarly removing the 8042 keyboard from the 40p machine as
> done for the prep machine in commit 72d3d8f052.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Applied to ppc-for-3.0.

> ---
>  hw/ppc/prep.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index a1e7219db6..be4db6a687 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
> @@ -770,7 +770,6 @@ static void ibm_40p_init(MachineState *machine)
>  
>      /* add some more devices */
>      if (defaults_enabled()) {
> -        isa_create_simple(isa_bus, TYPE_I8042);
>          m48t59 = NVRAM(isa_create_simple(isa_bus, "isa-m48t59"));
>  
>          dev = DEVICE(isa_create(isa_bus, "cs4231a"));

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson