[Qemu-devel] Re: [PATCH] Elo touchpad 10 bytes emulator v2
Ricardo Ribalda Delgado ricardo.riba...@gmail.com wrote:
New char device emulating an Elo serial touchpad.
-Emulate id and touch packets
-Absolute Output limited to 96-4000
It misses a SOB line.
diff --git a/hw/elo.c b/hw/elo.c
new file mode 100644
index 000..359333d
--- /dev/null
+++ b/hw/elo.c
+#include stdlib.h
+#include ../qemu-common.h
+#include ../qemu-char.h
+#include ../console.h
You can remove the ../ from those, Makefile sets correct include paths
for this to work.
+/*Move event*/
+if (is_downbuttons_state){
+ bytes[2]=0x2;
Can we use some constant from that bytes[2] values? 0x1, 0x2 and 0x4
don't look specially descriptive.
+static void elo_chr_close (struct CharDriverState *chr)
+{
+qemu_free (chr);
+}
Use coherent indentation. You are ussing here function (args) and in
the rest of the file function(args) without space.
diff --git a/hw/serial.c b/hw/serial.c
index f3ec36a..39c708f 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -92,7 +92,7 @@
#define UART_FCR_RFR0x02/* RCVR Fifo Reset */
#define UART_FCR_FE 0x01/* FIFO Enable */
-#define UART_FIFO_LENGTH16 /* 16550A Fifo Length */
+#define UART_FIFO_LENGTH32 /* 16550A Fifo Length */
#define XMIT_FIFO 0
#define RECV_FIFO 1
Why does the lenght of the FIFO changes here? I think this change in
independent of the rest of the patch (no knowledge about the 16550A to
know if it should be 16 or 32).
Later, Juan.
[Qemu-devel] Re: [PATCH] Elo touchpad 10 bytes emulator v2
Hello Juan
Thanks for your comments. About the indentation error... Do you have
some kind of auto indent script(like the kernel code has). It is
making me crazy trying to collaborate with a lot of projects an all of
them with different styles.
+#include stdlib.h
+#include ../qemu-common.h
+#include ../qemu-char.h
+#include ../console.h
You can remove the ../ from those, Makefile sets correct include paths
for this to work.
Ok. I used the mssmouse.c as reference. I can change that. (I guess
that I should also replace with )
+ /*Move event*/
+ if (is_downbuttons_state){
+ bytes[2]=0x2;
Can we use some constant from that bytes[2] values? 0x1, 0x2 and 0x4
don't look specially descriptive.
ok
Why does the lenght of the FIFO changes here? I think this change in
independent of the rest of the patch (no knowledge about the 16550A to
know if it should be 16 or 32).
I have to send a 2x10 bytes package, and it does not fit the the 16
bytes buffer Any other suggestion about how to do it?
Later, Juan.
Thanks again for your comments
--
Ricardo Ribalda
http://www.eps.uam.es/~rribalda/
[Qemu-devel] Re: [PATCH] Elo touchpad 10 bytes emulator v2
Ricardo Ribalda Delgado ricardo.riba...@gmail.com wrote: Hello Juan Thanks for your comments. About the indentation error... Do you have some kind of auto indent script(like the kernel code has). It is making me crazy trying to collaborate with a lot of projects an all of them with different styles. Don't even start this discussion yet again :) Short answer: we don't have. Long answer: any indenter that would pass qemu code style will be turing complete (at least) and possibly will also pass the Turing test. +#include stdlib.h +#include ../qemu-common.h +#include ../qemu-char.h +#include ../console.h You can remove the ../ from those, Makefile sets correct include paths for this to work. Ok. I used the mssmouse.c as reference. I can change that. (I guess that I should also replace with ) Everything uses . I just looked when reviewing this patch at msmouse, and that one could also take some cleanup. This happens a lot in qemu, you search for another driver for inspiration, and Murphy gets just the one with the wrong examples. Why does the lenght of the FIFO changes here? I think this change in independent of the rest of the patch (no knowledge about the 16550A to know if it should be 16 or 32). I have to send a 2x10 bytes package, and it does not fit the the 16 bytes buffer Any other suggestion about how to do it? Nope, I am not a 16550A guru at all. No sure if your change will break anything else or no, that is why I asked. Later, Juan. Thanks again for your comments You are welcome. Later, Juan.
Re: [Qemu-devel] Re: [PATCH] Elo touchpad 10 bytes emulator v2
Juan Quintela quint...@redhat.com writes: Ricardo Ribalda Delgado ricardo.riba...@gmail.com wrote: Hello Juan Thanks for your comments. About the indentation error... Do you have some kind of auto indent script(like the kernel code has). It is making me crazy trying to collaborate with a lot of projects an all of them with different styles. Don't even start this discussion yet again :) Short answer: we don't have. Long answer: any indenter that would pass qemu code style will be turing complete (at least) and possibly will also pass the Turing test. Among the things I'm paid for is tolerating ugly code. +#include stdlib.h +#include ../qemu-common.h +#include ../qemu-char.h +#include ../console.h You can remove the ../ from those, Makefile sets correct include paths for this to work. Ok. I used the mssmouse.c as reference. I can change that. (I guess that I should also replace with ) Use #include name.h for includes that come with the source or are generated by the build. Everything uses . I just looked when reviewing this patch at msmouse, and that one could also take some cleanup. This happens a lot in qemu, you search for another driver for inspiration, and Murphy gets just the one with the wrong examples. Why does the lenght of the FIFO changes here? I think this change in independent of the rest of the patch (no knowledge about the 16550A to know if it should be 16 or 32). I have to send a 2x10 bytes package, and it does not fit the the 16 bytes buffer Any other suggestion about how to do it? Nope, I am not a 16550A guru at all. No sure if your change will break anything else or no, that is why I asked. The 16550 FIFOs are 16 bytes. The chips with larger FIFOs have different model numbers, e.g. 16650 with 32 byte FIFOs. I can't see how the FIFO length limits your packet size. Could you explain?
[Qemu-devel] Re: [PATCH] Elo touchpad 10 bytes emulator v2
Hello Juan
New patch is on the mail.
It misses a SOB line.
Sorry, I don't understand what you mean here, do you mean the sing off?
You can remove the ../ from those, Makefile sets correct include paths
for this to work.
Done
+ /*Move event*/
+ if (is_downbuttons_state){
+ bytes[2]=0x2;
Can we use some constant from that bytes[2] values? 0x1, 0x2 and 0x4
don't look specially descriptive.
Done
Why does the lenght of the FIFO changes here? I think this change in
independent of the rest of the patch (no knowledge about the 16550A to
know if it should be 16 or 32).
Fixed using qemu_chr_can_read(). Thanks for your help!
Later, Juan.
Hope this time the patch is better.
Regards
--
Ricardo Ribalda
http://www.eps.uam.es/~rribalda/
