Re: [E3-hacking] [RFC][PATCH 4/5] input: serio: add support for Amstrad Delta serial keyboard port
Friday 11 December 2009 03:36:58 Dmitry Torokhov napisał(a):
On Thu, Dec 10, 2009 at 09:07:50PM +0100, Janusz Krzysztofik wrote:
+ +/* + * This table converts the amstrad mailboard scancodes to normal PC- AT scancodes + * The diagram below shows the amstrad keyboard, with the raw scancodes + * + * (70) (7A) (46) (7C) (77) Amstrad (72) (69) (1A) (2A) (1C) (15) + * [ 71][1:74][2:73][3:6B][4:22][5:1B][6:1D][7:1E][8:79][9:7D] [0:75][ 6C] + * [Q:21][W:23][E:24][R:26][T:52][Y:5D][U:0D][I:0E][O:32][P:34] | return| + * [A:31][S:33][D:35][F:36][G:29][H:5B][J:03][K:76][L:3A][@:3B]
| 2C|
+ * [ 3C][Z:3D][X:4E][C:54][V:0B][B:05][N:41][M:42][.:43][ 3E] [ 55] + * [ 83][ 06][ 49][ 4B ][,:44][ 16][ 2E] [ 09] + * + * These scancodes are then translated to AT scancodes using the following table + * The amstrad keyboard does not produce any extended scancodes, but we need to + * translate some amstrad scancodes to a AT extended scancode, hence the 16bit + * value for the translated scancode + *
No, please write a proper keyboard driver instead of creating this Frankenstain monster. It is not a generic serio-style data source so it should not use serio, should reside in drivers/input/keyboard and create input device by itself.
Thanks.
Hi Dmitry, Thanks for your opinion. Lack of support for Amstrad Delta external keybord is not a problem for me, since I use the machine as an IP phone, not a network terminal. I just thought it would be nice for other users to have this old but perfectly working driver integrated into the mainline. But if it is that bad as you say, I have no problem with dropping it. I'll see if I'm able to create a proper driver myself when I find some spare time. Thanks, Janusz
On Fri, Dec 11, 2009 at 04:09:58AM +0100, Janusz Krzysztofik wrote:
Friday 11 December 2009 03:36:58 Dmitry Torokhov napisał(a):
On Thu, Dec 10, 2009 at 09:07:50PM +0100, Janusz Krzysztofik wrote:
+ +/* + * This table converts the amstrad mailboard scancodes to normal PC- AT scancodes + * The diagram below shows the amstrad keyboard, with the raw scancodes + * + * (70) (7A) (46) (7C) (77) Amstrad (72) (69) (1A) (2A) (1C) (15) + * [ 71][1:74][2:73][3:6B][4:22][5:1B][6:1D][7:1E][8:79][9:7D] [0:75][ 6C] + * [Q:21][W:23][E:24][R:26][T:52][Y:5D][U:0D][I:0E][O:32][P:34] | return| + * [A:31][S:33][D:35][F:36][G:29][H:5B][J:03][K:76][L:3A][@:3B]
| 2C|
+ * [ 3C][Z:3D][X:4E][C:54][V:0B][B:05][N:41][M:42][.:43][ 3E] [ 55] + * [ 83][ 06][ 49][ 4B ][,:44][ 16][ 2E] [ 09] + * + * These scancodes are then translated to AT scancodes using the following table + * The amstrad keyboard does not produce any extended scancodes, but we need to + * translate some amstrad scancodes to a AT extended scancode, hence the 16bit + * value for the translated scancode + *
No, please write a proper keyboard driver instead of creating this Frankenstain monster. It is not a generic serio-style data source so it should not use serio, should reside in drivers/input/keyboard and create input device by itself.
Thanks.
Hi Dmitry,
Thanks for your opinion.
Lack of support for Amstrad Delta external keybord is not a problem for me, since I use the machine as an IP phone, not a network terminal. I just thought it would be nice for other users to have this old but perfectly working driver integrated into the mainline. But if it is that bad as you say, I have no problem with dropping it.
I'll see if I'm able to create a proper driver myself when I find some spare time.
Yes, that would be great, thank you. In fact it should end up about the same as this driver, except instead of creating and registering serio port you will need to register input_dev structure and instead of translating into atkbd scancodes you need to translate directly into Linux keycodes (KEY_A, KEY_1 and so on). -- Dmitry
Friday 11 December 2009 09:01:28 Dmitry Torokhov napisał(a):
On Fri, Dec 11, 2009 at 04:09:58AM +0100, Janusz Krzysztofik wrote:
Friday 11 December 2009 03:36:58 Dmitry Torokhov napisał(a):
On Thu, Dec 10, 2009 at 09:07:50PM +0100, Janusz Krzysztofik wrote:
+ +/* + * This table converts the amstrad mailboard scancodes to normal PC- AT scancodes + * The diagram below shows the amstrad keyboard, with the raw scancodes + * + * (70) (7A) (46) (7C) (77) Amstrad (72) (69) (1A) (2A) (1C) (15) + * [ 71][1:74][2:73][3:6B][4:22][5:1B][6:1D][7:1E][8:79][9:7D] [0:75][ 6C] + * [Q:21][W:23][E:24][R:26][T:52][Y:5D][U:0D][I:0E][O:32][P:34] | return| + * [A:31][S:33][D:35][F:36][G:29][H:5B][J:03][K:76][L:3A][@:3B]
| 2C|
+ * [ 3C][Z:3D][X:4E][C:54][V:0B][B:05][N:41][M:42][.:43][ 3E] [ 55] + * [ 83][ 06][ 49][ 4B ][,:44][ 16][ 2E] [ 09] + * + * These scancodes are then translated to AT scancodes using the following table + * The amstrad keyboard does not produce any extended scancodes, but we need to + * translate some amstrad scancodes to a AT extended scancode, hence the 16bit + * value for the translated scancode + *
No, please write a proper keyboard driver instead of creating this Frankenstain monster. It is not a generic serio-style data source so it should not use serio, should reside in drivers/input/keyboard and create input device by itself.
I'll see if I'm able to create a proper driver myself when I find some spare time.
Yes, that would be great, thank you. In fact it should end up about the same as this driver, except instead of creating and registering serio port you will need to register input_dev structure and instead of translating into atkbd scancodes you need to translate directly into Linux keycodes (KEY_A, KEY_1 and so on).
Dmitry, Thanks for your directions. However, before I get to work, please let me still better understand what I am really supposed to create here, and why you think it should be done one way and not the other. I'd rather avoid submitting an input related code that you might find not adequate in the future. First, I have never submitted a single line of code that would be accepted for inclusion into drivers/input before. Could you please recommend a documentation for me to read, from where I could learn what a proper keyboard driver should look like, and what kind of devices can be considered as generic serio-style data sources and what can't, and more? Or should I better give up being that short of required knowledge? Moreover, please have a look at these two messages posted by Cliff Lawson, who worked for Amstrad while the E3 (codename Delta) was being developed, being involved in that development: http://www.earth.li/pipermail/e3-hacking/2006-April/000445.html http://www.earth.li/pipermail/e3-hacking/2006-April/000449.html As one can read, the Amstrad Delta keyboard was described by Cliff as a PS/2 device, even if not connected to a regular PS/2 port but some MPU GPIO lines directly instead. Furthermore, it has been proven by Matt Callow, who created the serio driver you didn't like, that the atkbd keyboard driver already works fine for this keyboard if fed with scancodes matching what it is told to expect. Please also note that the E3 keyboard port is supposed to be used not only for getting input from the keyboard, but from a bundled gamepad as well. Not yet supported, but who knows if forever. That said, do you still think there is a need to create a new, separate keyboard driver for the device in question? Isn't reusing an existing, proven to be working correctly, keyboard driver module, isn't it a good solution here? If not, could you please elaborate on why it's not? I always thought that being PS/2 compatible is enough for a device to be considered as supported by atkbd, but maybe I just don't understand what the atkbd driver is designed for and what a supported device should look like. Moreover, isn't a port, that is supposed to interact with at least two distinct input devices, isn't it worth of creating a separate driver for it, be it serio module or anything else that would better match the input subsystem design? In your initial answer, you quoted a part of the patch, namely that containing a description of the driver's scancode translation functionality. I guess you tried to say that traslating scancodes inside a port driver is wrong. I would agree with that, without any reservations. But for your other conclusions, could you please reconsider if still valid, and elaborate on them if you think they are? Thanks, Janusz PS. To be honest, I have to inform every E3 hacker still interrested in testing the driver, even if already NAKed upstream, that I happened to fail with testing the final, submitted version of this particular patch from the series. I tested everything, patch by patch, after splitting the initial one into several distinct and then after cleaning them up one by one, but unfortunatelly missed testing this one after being cleaned up finally. As a result, it doesn't build. Please use the extra patch below, applied over the series, if interrested in testing the driver in it's current form. I'm sorry for that. Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> --- diff -upr git.orig/arch/arm/mach-omap1/include/mach/ams-delta-fiq.h git/arch/arm/mach-omap1/include/mach/ams-delta-fiq.h --- git.orig/arch/arm/mach-omap1/include/mach/ams-delta-fiq.h 2009-12-11 18:19:19.000000000 +0100 +++ git/arch/arm/mach-omap1/include/mach/ams-delta-fiq.h 2009-12-11 19:19:04.000000000 +0100 @@ -51,6 +51,7 @@ #define FIQ_CIRC_BUFF 30 /*Start of circular buffer */ +extern unsigned int fiq_buffer[]; extern unsigned char qwerty_fiqin_start, qwerty_fiqin_end; extern void __init ams_delta_init_fiq(void); diff -upr git.orig/drivers/input/serio/ams_delta_keyboard.c git/drivers/input/serio/ams_delta_keyboard.c --- git.orig/drivers/input/serio/ams_delta_keyboard.c 2009-12-11 18:19:19.000000000 +0100 +++ git/drivers/input/serio/ams_delta_keyboard.c 2009-12-11 19:23:48.000000000 +0100 @@ -223,7 +223,7 @@ gpio_data: serio: kfree(ams_delta_kbd_port); err: - return retval; + return err; } module_init(ams_delta_kbd_init);
Hi Janusz, On Sat, Dec 12, 2009 at 09:34:07PM +0100, Janusz Krzysztofik wrote:
Friday 11 December 2009 09:01:28 Dmitry Torokhov napisał(a):
On Fri, Dec 11, 2009 at 04:09:58AM +0100, Janusz Krzysztofik wrote:
Friday 11 December 2009 03:36:58 Dmitry Torokhov napisał(a):
On Thu, Dec 10, 2009 at 09:07:50PM +0100, Janusz Krzysztofik wrote:
+ +/* + * This table converts the amstrad mailboard scancodes to normal PC- AT scancodes + * The diagram below shows the amstrad keyboard, with the raw scancodes + * + * (70) (7A) (46) (7C) (77) Amstrad (72) (69) (1A) (2A) (1C) (15) + * [ 71][1:74][2:73][3:6B][4:22][5:1B][6:1D][7:1E][8:79][9:7D] [0:75][ 6C] + * [Q:21][W:23][E:24][R:26][T:52][Y:5D][U:0D][I:0E][O:32][P:34] | return| + * [A:31][S:33][D:35][F:36][G:29][H:5B][J:03][K:76][L:3A][@:3B]
| 2C|
+ * [ 3C][Z:3D][X:4E][C:54][V:0B][B:05][N:41][M:42][.:43][ 3E] [ 55] + * [ 83][ 06][ 49][ 4B ][,:44][ 16][ 2E] [ 09] + * + * These scancodes are then translated to AT scancodes using the following table + * The amstrad keyboard does not produce any extended scancodes, but we need to + * translate some amstrad scancodes to a AT extended scancode, hence the 16bit + * value for the translated scancode + *
No, please write a proper keyboard driver instead of creating this Frankenstain monster. It is not a generic serio-style data source so it should not use serio, should reside in drivers/input/keyboard and create input device by itself.
I'll see if I'm able to create a proper driver myself when I find some spare time.
Yes, that would be great, thank you. In fact it should end up about the same as this driver, except instead of creating and registering serio port you will need to register input_dev structure and instead of translating into atkbd scancodes you need to translate directly into Linux keycodes (KEY_A, KEY_1 and so on).
Dmitry,
Thanks for your directions. However, before I get to work, please let me still better understand what I am really supposed to create here, and why you think it should be done one way and not the other. I'd rather avoid submitting an input related code that you might find not adequate in the future.
First, I have never submitted a single line of code that would be accepted for inclusion into drivers/input before. Could you please recommend a documentation for me to read, from where I could learn what a proper keyboard driver should look like,
I guess I meant "proper keyboard driver" in the sense of a driver that creates and registeres its own input_dev and perform direct translation of data coming form hardware into linux input events.
and what kind of devices can be considered as generic serio-style data sources and what can't, and more?
If a same device can be used on different platforms and several devices may use the same port then I can see the need of creating a serio abstraction. But if tere is 1:1 relationship then a driver that attaches directly to the hardware might make sense.
Or should I better give up being that short of required knowledge?
Moreover, please have a look at these two messages posted by Cliff Lawson, who worked for Amstrad while the E3 (codename Delta) was being developed, being involved in that development:
http://www.earth.li/pipermail/e3-hacking/2006-April/000445.html http://www.earth.li/pipermail/e3-hacking/2006-April/000449.html
As one can read, the Amstrad Delta keyboard was described by Cliff as a PS/2 device, even if not connected to a regular PS/2 port but some MPU GPIO lines directly instead.
Furthermore, it has been proven by Matt Callow, who created the serio driver you didn't like, that the atkbd keyboard driver already works fine for this keyboard if fed with scancodes matching what it is told to expect.
Of course atkbd would work fine if it is fed the data it expects. The same can be said for any driver in the tree, isn't it? It is possible to write HID-to-atkbd or "gpio matrix to atkbd" "serios" but that does not mean it is the right thing to do.
Please also note that the E3 keyboard port is supposed to be used not only for getting input from the keyboard, but from a bundled gamepad as well. Not yet supported, but who knows if forever.
Can they be used simultaneously? Would not the translation that you perform right now cause issues with the gamepad? Are you thinking about reusing one of the existing gamepad modules in the same fashion as you do for atkbd but translating the data into its native format?
That said, do you still think there is a need to create a new, separate keyboard driver for the device in question? Isn't reusing an existing, proven to be working correctly, keyboard driver module, isn't it a good solution here? If not, could you please elaborate on why it's not? I always thought that being PS/2 compatible is enough for a device to be considered as supported by atkbd, but maybe I just don't understand what the atkbd driver is designed for and what a supported device should look like.
atkbd is supposed to support PS/2 devices speaking AT keyboard protocol. We also do support such devices behind dumb controllers that do not allow querying the device but we expect those to at least produce the standard scancodes.
Moreover, isn't a port, that is supposed to interact with at least two distinct input devices, isn't it worth of creating a separate driver for it, be it serio module or anything else that would better match the input subsystem design?
Yes, if there can be several devices connected to it then it makes sense to create a port abstraction for different drivers to plug into.
In your initial answer, you quoted a part of the patch, namely that containing a description of the driver's scancode translation functionality. I guess you tried to say that traslating scancodes inside a port driver is wrong. I would agree with that, without any reservations.
Yes, exactly.
But for your other conclusions, could you please reconsider if still valid, and elaborate on them if you think they are?
I would need to know a bit more about the gamepad, but so far I still think that current attempot of creating intermediate serio module which allows to use atkbd driver is not the proper solution. -- Dmitry
Hi Dmitry, Sunday 13 December 2009 00:20:03 Dmitry Torokhov napisał(a):
atkbd is supposed to support PS/2 devices speaking AT keyboard protocol. We also do support such devices behind dumb controllers that do not allow querying the device but we expect those to at least produce the standard scancodes.
As far as I can understand, using the atkbd driver is not a good idea in this case because of the keyboard not producing standard scancodes, correct? Isn't the hpps2atkbd.h provided keycode table an already supported way of introducing exceptions to this standard scancodes requirement? Why couldn't this method be reused here? What I am afraid of is if the driver supposed to be created instead wouldn't require reiplementing most of the atkbd code.
I would need to know a bit more about the gamepad, but so far I still think that current attempot of creating intermediate serio module which allows to use atkbd driver is not the proper solution.
I modified the serio driver to send exactly what it gets from the buffer and examined its output with serio_raw. The gamepad (can be connected simultaneously) appeared to send exactly the same scancodes as the keyboard did. I couldn't see how those might be destinguished whether coming from the keybord or from the gamepad. Thus, handling them both together as a single device would probably be the only option here. With both devices connected and keys pressed simultaneously, errors occure at the lowest level: parity check failed, invalid stop bit, etc. I don't think it would be possible to do anything about this. But this also means that trying to query them in order to get a sensible response would probably be not reliable. Any thoughts? Thanks, Janusz
participants (2)
-
Dmitry Torokhov -
Janusz Krzysztofik