| Message ID | 1269888491-845-1-git-send-email-ricardo.ribalda@gmail.com |
|---|---|
| State | New |
| Headers | show |
Ricardo Ribalda Delgado <ricardo.ribalda@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 0000000..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_down&&buttons_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_RFR 0x02 /* RCVR Fifo Reset */ > #define UART_FCR_FE 0x01 /* FIFO Enable */ > > -#define UART_FIFO_LENGTH 16 /* 16550A Fifo Length */ > +#define UART_FIFO_LENGTH 32 /* 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.
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_down&&buttons_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 Delgado <ricardo.ribalda@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.
Juan Quintela <quintela@redhat.com> writes: > Ricardo Ribalda Delgado <ricardo.ribalda@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?
Markus Armbruster <armbru@redhat.com> wrote: >>> 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? Ah, I found it. Please look at hw/baum.c to see how to handle bigger packets. You can't write 10 + 10 bytes into 16 bytes in one go, you need to wait for the guest to consume some bits before you sent something more. see how baum_accept_input() handle qemu_chr_can_read() output. Something like that should fix your problems. Later, Juan.
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_down&&buttons_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
diff --git a/Makefile.objs b/Makefile.objs index b73e2cb..07c2e68 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -75,7 +75,7 @@ common-obj-y += bt.o bt-host.o bt-vhci.o bt-l2cap.o bt-sdp.o bt-hci.o bt-hid.o u common-obj-y += bt-hci-csr.o common-obj-y += buffered_file.o migration.o migration-tcp.o qemu-sockets.o common-obj-y += qemu-char.o savevm.o #aio.o -common-obj-y += msmouse.o ps2.o +common-obj-y += msmouse.o ps2.o elo.o common-obj-y += qdev.o qdev-properties.o common-obj-y += qemu-config.o block-migration.o diff --git a/hw/elo.c b/hw/elo.c new file mode 100644 index 0000000..359333d --- /dev/null +++ b/hw/elo.c @@ -0,0 +1,153 @@ +/* + * QEMU ELO Touchpad via serial port + * + * Copyright (c) 2010 Ricardo Ribalda: QTechnology http://qtec.com + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ +#include <stdlib.h> +#include "../qemu-common.h" +#include "../qemu-char.h" +#include "../console.h" +#include "elo.h" + +static void elo_event(void *opaque, + int ax, int ay, int az, int buttons_state) +{ + CharDriverState *chr = (CharDriverState *)opaque; + + unsigned char bytes[10]; + static int is_down=0; + int old_ax=0,old_ay=0; + int i; + + /*A touchpad cannot capture flight events*/ + if ((!is_down)&&(!buttons_state)) + return; + + ax=(ax*(4000-96+1))/0x7fff; + ax+=96; + ay=(ay*(4000-96+1))/0x7fff; + ay+=96; + + /*Move event*/ + if (is_down&&buttons_state){ + bytes[2]=0x2; + is_down++; + if ((old_ay==ay)&&(old_ax==ax)) + return; + old_ay=ay; + old_ax=ax; + } + + /*Click*/ + if ((!is_down)&&buttons_state){ + bytes[2]=0x1; + is_down=1; + old_ay=ay; + old_ax=ax; + } + /*Release*/ + if (is_down&&(!buttons_state)){ + bytes[2]=0x4; + is_down=0; + } + + bytes[0]='U'; + bytes[1]='T'; + bytes[3]=ax&0xff; + bytes[4]=(ax>>8)&0xff; + bytes[5]=ay&0xff; + bytes[6]=(ay>>8)&0xff; + bytes[7]=0x0;/*No presure capabilities*/ + bytes[8]=0x0; + bytes[9]=0xaa; + for(i=0;i<9;i++) + bytes[9]+=bytes[i]; + + qemu_chr_read(chr, bytes, 10); +} + +static int elo_chr_write (struct CharDriverState *s, const uint8_t *buf, int len) +{ + unsigned char bytes[10]; + static int in_cmd=0,i; + + if (buf[0]==0x55) + in_cmd=0; + + /*Only response to ID*/ + in_cmd+=len; + if (in_cmd<10) + return len; + + /* Only respond cmd ID This should be enough at least for linux*/ + /*Full ref at http://tge.cegep-baie-comeau.qc.ca/fichestech/References/Elo%20Touch%20Screen/smartset/pages/chapter_6.htm#command_descriptions*/ + /*ID*/ + in_cmd=0; + bytes[0]='U'; + bytes[1]='I'; + bytes[2]='0';/*Accu*/ + bytes[3]='0';/*Serial*/ + bytes[4]=0;/*No features*/ + bytes[5]=1; + bytes[6]=2; + bytes[7]=1;/*1 response*/ + bytes[8]=0xe; + bytes[9]=0xaa; + for(i=0;i<9;i++) + bytes[9]+=bytes[i]; + qemu_chr_read(s, bytes, 10); + + /*ACK no err*/ + in_cmd=0; + bytes[0]='U'; + bytes[1]='A'; + bytes[2]='0'; + bytes[3]='0'; + bytes[4]='0'; + bytes[5]=0; + bytes[6]=0; + bytes[7]=0; + bytes[8]=0; + bytes[9]=0xaa; + for(i=0;i<9;i++) + bytes[9]+=bytes[i]; + qemu_chr_read(s, bytes, 10); + in_cmd=0; + return len; +} + +static void elo_chr_close (struct CharDriverState *chr) +{ + qemu_free (chr); +} + +CharDriverState *qemu_chr_open_elo(QemuOpts *opts) +{ + CharDriverState *chr; + chr = qemu_mallocz(sizeof(CharDriverState)); + chr->chr_write = elo_chr_write; + chr->chr_close = elo_chr_close; + + fprintf(stderr,"ELO Touchpad\n"); + qemu_add_mouse_event_handler(elo_event, chr, 1, "QEMU Elo Touchpad"); + + return chr; +} diff --git a/hw/elo.h b/hw/elo.h new file mode 100644 index 0000000..4b4e09a --- /dev/null +++ b/hw/elo.h @@ -0,0 +1,2 @@ +/* elo.c */ +CharDriverState *qemu_chr_open_elo(QemuOpts *opts); 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_RFR 0x02 /* RCVR Fifo Reset */ #define UART_FCR_FE 0x01 /* FIFO Enable */ -#define UART_FIFO_LENGTH 16 /* 16550A Fifo Length */ +#define UART_FIFO_LENGTH 32 /* 16550A Fifo Length */ #define XMIT_FIFO 0 #define RECV_FIFO 1 diff --git a/qemu-char.c b/qemu-char.c index 40cfefa..2f95767 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -32,6 +32,7 @@ #include "hw/usb.h" #include "hw/baum.h" #include "hw/msmouse.h" +#include "hw/elo.h" #include "qemu-objects.h" #include <unistd.h> @@ -2278,6 +2279,7 @@ QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename) if (strcmp(filename, "null") == 0 || strcmp(filename, "pty") == 0 || strcmp(filename, "msmouse") == 0 || + strcmp(filename, "elo") == 0 || strcmp(filename, "braille") == 0 || strcmp(filename, "stdio") == 0) { qemu_opt_set(opts, "backend", filename); @@ -2391,6 +2393,7 @@ static const struct { { .name = "socket", .open = qemu_chr_open_socket }, { .name = "udp", .open = qemu_chr_open_udp }, { .name = "msmouse", .open = qemu_chr_open_msmouse }, + { .name = "elo", .open = qemu_chr_open_elo }, { .name = "vc", .open = text_console_init }, #ifdef _WIN32 { .name = "file", .open = qemu_chr_open_win_file_out }, diff --git a/qemu-options.hx b/qemu-options.hx index 7c33736..7ee62cb 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -1183,7 +1183,7 @@ DEF("chardev", HAS_ARG, QEMU_OPTION_chardev, "-chardev socket,id=id,path=path[,server][,nowait][,telnet] (unix)\n" "-chardev udp,id=id[,host=host],port=port[,localaddr=localaddr]\n" " [,localport=localport][,ipv4][,ipv6]\n" - "-chardev msmouse,id=id\n" + "-chardev elo,id=id\n" "-chardev vc,id=id[[,width=width][,height=height]][[,cols=cols][,rows=rows]]\n" "-chardev file,id=id,path=path\n" "-chardev pipe,id=id,path=path\n" @@ -1650,6 +1650,9 @@ or fake device. @item msmouse Three button serial mouse. Configure the guest to use Microsoft protocol. + +@item elo +Elo Touchpad 10bytes emulator. @end table ETEXI