| Message ID | 20191022205941.23152-4-svens@stackframe.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
Hi Sven, (Please Cc reviewers who previously commented your patch) On 10/22/19 10:59 PM, Sven Schnelle wrote: > HP-UX sends both the 'Set key make and break (0xfc) and > 'Set all key typematic make and break' (0xfa). QEMU response > with 'Resend' as it doesn't handle these commands. HP-UX than > reports an PS/2 max retransmission exceeded error. Add these > commands and just reply with ACK. > > Signed-off-by: Sven Schnelle <svens@stackframe.org> > --- > hw/input/ps2.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/hw/input/ps2.c b/hw/input/ps2.c > index 67f92f6112..0b671b6339 100644 > --- a/hw/input/ps2.c > +++ b/hw/input/ps2.c > @@ -49,6 +49,8 @@ > #define KBD_CMD_RESET_DISABLE 0xF5 /* reset and disable scanning */ > #define KBD_CMD_RESET_ENABLE 0xF6 /* reset and enable scanning */ > #define KBD_CMD_RESET 0xFF /* Reset */ > +#define KBD_CMD_SET_MAKE_BREAK 0xFC /* Set Make and Break mode */ > +#define KBD_CMD_SET_TYPEMATIC 0xFA /* Set Typematic Make and Break mode */ > > /* Keyboard Replies */ > #define KBD_REPLY_POR 0xAA /* Power on reset */ > @@ -573,6 +575,7 @@ void ps2_write_keyboard(void *opaque, int val) > case KBD_CMD_SCANCODE: > case KBD_CMD_SET_LEDS: > case KBD_CMD_SET_RATE: > + case KBD_CMD_SET_MAKE_BREAK: OK > s->common.write_cmd = val; > ps2_queue(&s->common, KBD_REPLY_ACK); > break; > @@ -592,11 +595,18 @@ void ps2_write_keyboard(void *opaque, int val) > KBD_REPLY_ACK, > KBD_REPLY_POR); > break; > + case KBD_CMD_SET_TYPEMATIC: > + ps2_queue(&s->common, KBD_REPLY_ACK); I'm not sure, can't this loop? Can you fold it with the '0x00' case? > + break; > default: > ps2_queue(&s->common, KBD_REPLY_RESEND); > break; > } > break; > + case KBD_CMD_SET_MAKE_BREAK: We can reorder this one in the same case with: case KBD_CMD_SET_LEDS: case KBD_CMD_SET_RATE: > + ps2_queue(&s->common, KBD_REPLY_ACK); > + s->common.write_cmd = -1; > + break; > case KBD_CMD_SCANCODE: > if (val == 0) { > if (s->common.queue.count <= PS2_QUEUE_SIZE - 2) { >
Hi Philippe, On Wed, Oct 23, 2019 at 01:08:35PM +0200, Philippe Mathieu-Daudé wrote: > Hi Sven, > > (Please Cc reviewers who previously commented your patch) > > On 10/22/19 10:59 PM, Sven Schnelle wrote: > > HP-UX sends both the 'Set key make and break (0xfc) and > > 'Set all key typematic make and break' (0xfa). QEMU response > > with 'Resend' as it doesn't handle these commands. HP-UX than > > reports an PS/2 max retransmission exceeded error. Add these > > commands and just reply with ACK. > > > > Signed-off-by: Sven Schnelle <svens@stackframe.org> > > --- > > hw/input/ps2.c | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/hw/input/ps2.c b/hw/input/ps2.c > > index 67f92f6112..0b671b6339 100644 > > --- a/hw/input/ps2.c > > +++ b/hw/input/ps2.c > > @@ -49,6 +49,8 @@ > > #define KBD_CMD_RESET_DISABLE 0xF5 /* reset and disable scanning */ > > #define KBD_CMD_RESET_ENABLE 0xF6 /* reset and enable scanning */ > > #define KBD_CMD_RESET 0xFF /* Reset */ > > +#define KBD_CMD_SET_MAKE_BREAK 0xFC /* Set Make and Break mode */ > > +#define KBD_CMD_SET_TYPEMATIC 0xFA /* Set Typematic Make and Break mode */ > > /* Keyboard Replies */ > > #define KBD_REPLY_POR 0xAA /* Power on reset */ > > @@ -573,6 +575,7 @@ void ps2_write_keyboard(void *opaque, int val) > > case KBD_CMD_SCANCODE: > > case KBD_CMD_SET_LEDS: > > case KBD_CMD_SET_RATE: > > + case KBD_CMD_SET_MAKE_BREAK: > > OK > > > s->common.write_cmd = val; > > ps2_queue(&s->common, KBD_REPLY_ACK); > > break; > > @@ -592,11 +595,18 @@ void ps2_write_keyboard(void *opaque, int val) > > KBD_REPLY_ACK, > > KBD_REPLY_POR); > > break; > > + case KBD_CMD_SET_TYPEMATIC: > > + ps2_queue(&s->common, KBD_REPLY_ACK); > > I'm not sure, can't this loop? I don't see how? > Can you fold it with the '0x00' case? Ok. > > + break; > > default: > > ps2_queue(&s->common, KBD_REPLY_RESEND); > > break; > > } > > break; > > + case KBD_CMD_SET_MAKE_BREAK: > > We can reorder this one in the same case with: > > case KBD_CMD_SET_LEDS: > case KBD_CMD_SET_RATE: Ok. > > + ps2_queue(&s->common, KBD_REPLY_ACK); > > + s->common.write_cmd = -1; > > + break; > > case KBD_CMD_SCANCODE: > > if (val == 0) { > > if (s->common.queue.count <= PS2_QUEUE_SIZE - 2) { > > > Regards Sven
On 10/23/19 2:08 PM, Sven Schnelle wrote: > Hi Philippe, > > On Wed, Oct 23, 2019 at 01:08:35PM +0200, Philippe Mathieu-Daudé wrote: >> Hi Sven, >> >> (Please Cc reviewers who previously commented your patch) >> >> On 10/22/19 10:59 PM, Sven Schnelle wrote: >>> HP-UX sends both the 'Set key make and break (0xfc) and >>> 'Set all key typematic make and break' (0xfa). QEMU response >>> with 'Resend' as it doesn't handle these commands. HP-UX than >>> reports an PS/2 max retransmission exceeded error. Add these >>> commands and just reply with ACK. >>> >>> Signed-off-by: Sven Schnelle <svens@stackframe.org> >>> --- >>> hw/input/ps2.c | 10 ++++++++++ >>> 1 file changed, 10 insertions(+) >>> >>> diff --git a/hw/input/ps2.c b/hw/input/ps2.c >>> index 67f92f6112..0b671b6339 100644 >>> --- a/hw/input/ps2.c >>> +++ b/hw/input/ps2.c >>> @@ -49,6 +49,8 @@ >>> #define KBD_CMD_RESET_DISABLE 0xF5 /* reset and disable scanning */ >>> #define KBD_CMD_RESET_ENABLE 0xF6 /* reset and enable scanning */ >>> #define KBD_CMD_RESET 0xFF /* Reset */ >>> +#define KBD_CMD_SET_MAKE_BREAK 0xFC /* Set Make and Break mode */ >>> +#define KBD_CMD_SET_TYPEMATIC 0xFA /* Set Typematic Make and Break mode */ >>> /* Keyboard Replies */ >>> #define KBD_REPLY_POR 0xAA /* Power on reset */ >>> @@ -573,6 +575,7 @@ void ps2_write_keyboard(void *opaque, int val) >>> case KBD_CMD_SCANCODE: >>> case KBD_CMD_SET_LEDS: >>> case KBD_CMD_SET_RATE: >>> + case KBD_CMD_SET_MAKE_BREAK: >> >> OK >> >>> s->common.write_cmd = val; >>> ps2_queue(&s->common, KBD_REPLY_ACK); >>> break; >>> @@ -592,11 +595,18 @@ void ps2_write_keyboard(void *opaque, int val) >>> KBD_REPLY_ACK, >>> KBD_REPLY_POR); >>> break; >>> + case KBD_CMD_SET_TYPEMATIC: >>> + ps2_queue(&s->common, KBD_REPLY_ACK); >> >> I'm not sure, can't this loop? > > I don't see how? I misunderstood the two switch statements. >> Can you fold it with the '0x00' case? > > Ok. > >>> + break; >>> default: >>> ps2_queue(&s->common, KBD_REPLY_RESEND); >>> break; >>> } >>> break; >>> + case KBD_CMD_SET_MAKE_BREAK: >> >> We can reorder this one in the same case with: >> >> case KBD_CMD_SET_LEDS: >> case KBD_CMD_SET_RATE: > > Ok. With these changes: Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > >>> + ps2_queue(&s->common, KBD_REPLY_ACK); >>> + s->common.write_cmd = -1; >>> + break; >>> case KBD_CMD_SCANCODE: >>> if (val == 0) { >>> if (s->common.queue.count <= PS2_QUEUE_SIZE - 2) { >>> >> > > Regards > Sven >
diff --git a/hw/input/ps2.c b/hw/input/ps2.c index 67f92f6112..0b671b6339 100644 --- a/hw/input/ps2.c +++ b/hw/input/ps2.c @@ -49,6 +49,8 @@ #define KBD_CMD_RESET_DISABLE 0xF5 /* reset and disable scanning */ #define KBD_CMD_RESET_ENABLE 0xF6 /* reset and enable scanning */ #define KBD_CMD_RESET 0xFF /* Reset */ +#define KBD_CMD_SET_MAKE_BREAK 0xFC /* Set Make and Break mode */ +#define KBD_CMD_SET_TYPEMATIC 0xFA /* Set Typematic Make and Break mode */ /* Keyboard Replies */ #define KBD_REPLY_POR 0xAA /* Power on reset */ @@ -573,6 +575,7 @@ void ps2_write_keyboard(void *opaque, int val) case KBD_CMD_SCANCODE: case KBD_CMD_SET_LEDS: case KBD_CMD_SET_RATE: + case KBD_CMD_SET_MAKE_BREAK: s->common.write_cmd = val; ps2_queue(&s->common, KBD_REPLY_ACK); break; @@ -592,11 +595,18 @@ void ps2_write_keyboard(void *opaque, int val) KBD_REPLY_ACK, KBD_REPLY_POR); break; + case KBD_CMD_SET_TYPEMATIC: + ps2_queue(&s->common, KBD_REPLY_ACK); + break; default: ps2_queue(&s->common, KBD_REPLY_RESEND); break; } break; + case KBD_CMD_SET_MAKE_BREAK: + ps2_queue(&s->common, KBD_REPLY_ACK); + s->common.write_cmd = -1; + break; case KBD_CMD_SCANCODE: if (val == 0) { if (s->common.queue.count <= PS2_QUEUE_SIZE - 2) {
HP-UX sends both the 'Set key make and break (0xfc) and 'Set all key typematic make and break' (0xfa). QEMU response with 'Resend' as it doesn't handle these commands. HP-UX than reports an PS/2 max retransmission exceeded error. Add these commands and just reply with ACK. Signed-off-by: Sven Schnelle <svens@stackframe.org> --- hw/input/ps2.c | 10 ++++++++++ 1 file changed, 10 insertions(+)