Re: [Qemu-devel] [PATCH V2 2/3] usb-gotemp: new module emulating a USB thermometer

Re: [Qemu-devel] [PATCH V2 2/3] usb-gotemp: new module emulating a USB thermometer

[Laurent Vivier]

>>> +    s->temperature++;
>>>
>> You're going to overheat very quickly.
>> Apart from making the driver work, is this actually useful?
>
>I wanted the temperature to change with time to give a sense of
>"something is happening" ^_^
>
>The main user I had in mind was someone new to USB and Linux driver 
>development
>following gregkh's driver tutorial:
>http://www.kroah.com/linux/talks/ols_2005_driver_tutorial/
>My thinking was that if the temperature never changes, all the USB
>packets generated in the 'read_int_callback"
>part of the driver would seem like a waste of effort.
>

Perhaps you could read temperature samples from a file...

Regards,
Laurent

-- 
--------------------- Laurent@vivier.eu  ---------------------
"Tout ce qui est impossible reste à accomplir"    Jules Verne
"Things are only impossible until they're not" Jean-Luc Picard

[Qemu-devel] [PATCH V2 0/3] usb-gotemp: USB thermometer emulation

[Scott Tsai]

Rebased on top of the master branch of git://git.savannah.nongnu.org/qemu.git since
last posting. Any advice on how to get these reviewed and merged or rejected greatly
appreciated!

Greg Kroah-Hartman has been giving a talk titled "Write a Real, Working, Linux Driver"
for the past four years at various conferences.  This patch series enables qemu to emulate
the Vernier Go!Temp USB thermometer used in that talk.

This was motivated by experience from the FreedomHEC Taipei 2009 conference,
where students complained that the gadget was too expensive while professional
developers complained the limited number of devices Greg brought sold out too quickly.

The emulation is complete enough for gregkh's sample driver and 
using the vendor supplied SDK through the in-kernel 'ldusb' module under Linux.
No testing have yet been done with the vendor's fancier Windows software.

Video, slides and code from gregkh's talk:
http://www.kernel.org/pub/media/talks/gregkh/2008_driver_writing_tutorial_gregkh.avi
http://www.archive.org/download/LinuxDriverTutorialFreedomhecTaipei2009_215/06-linux-driver-tutorial.ogg
http://freedomhectaipei.pbworks.com/f/gregkh-driver-tutorial-freedomhectaipei.pdf
http://freedomhectaipei.pbworks.com/f/usb_tutorial.tar.gz

[Qemu-devel] [PATCH V2 1/3] usb: move HID request defines to hw/usb.h

[Scott Tsai]

Move USB HID request constants from hw/usb-hid.c to hw/usb.h
to allow other modules to use them.

Signed-off-by: Scott Tsai <scottt.tw@gmail.com>
---
 hw/usb-hid.c |   20 ++++++--------------
 hw/usb.h     |    8 ++++++++
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/hw/usb-hid.c b/hw/usb-hid.c
index f4a2a48..e263fc0 100644
--- a/hw/usb-hid.c
+++ b/hw/usb-hid.c
@@ -26,14 +26,6 @@
 #include "console.h"
 #include "usb.h"
 
-/* HID interface requests */
-#define GET_REPORT   0xa101
-#define GET_IDLE     0xa102
-#define GET_PROTOCOL 0xa103
-#define SET_REPORT   0x2109
-#define SET_IDLE     0x210a
-#define SET_PROTOCOL 0x210b
-
 /* HID descriptor types */
 #define USB_DT_HID    0x21
 #define USB_DT_REPORT 0x22
@@ -763,7 +755,7 @@ static int usb_hid_handle_control(USBDevice *dev, int request, int value,
             goto fail;
         }
         break;
-    case GET_REPORT:
+    case USB_REQ_HID_GET_REPORT:
 	if (s->kind == USB_MOUSE)
             ret = usb_mouse_poll(s, data, length);
 	else if (s->kind == USB_TABLET)
@@ -771,29 +763,29 @@ static int usb_hid_handle_control(USBDevice *dev, int request, int value,
         else if (s->kind == USB_KEYBOARD)
             ret = usb_keyboard_poll(&s->kbd, data, length);
         break;
-    case SET_REPORT:
+    case USB_REQ_HID_SET_REPORT:
         if (s->kind == USB_KEYBOARD)
             ret = usb_keyboard_write(&s->kbd, data, length);
         else
             goto fail;
         break;
-    case GET_PROTOCOL:
+    case USB_REQ_HID_GET_PROTOCOL:
         if (s->kind != USB_KEYBOARD)
             goto fail;
         ret = 1;
         data[0] = s->protocol;
         break;
-    case SET_PROTOCOL:
+    case USB_REQ_HID_SET_PROTOCOL:
         if (s->kind != USB_KEYBOARD)
             goto fail;
         ret = 0;
         s->protocol = value;
         break;
-    case GET_IDLE:
+    case USB_REQ_HID_GET_IDLE:
         ret = 1;
         data[0] = s->idle;
         break;
-    case SET_IDLE:
+    case USB_REQ_HID_SET_IDLE:
         s->idle = (uint8_t) (value >> 8);
         ret = 0;
         break;
diff --git a/hw/usb.h b/hw/usb.h
index 351c466..7d46931 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -104,6 +104,14 @@
 #define USB_REQ_SET_INTERFACE		0x0B
 #define USB_REQ_SYNCH_FRAME		0x0C
 
+/* HID interface requests */
+#define USB_REQ_HID_GET_REPORT   0xa101
+#define USB_REQ_HID_GET_IDLE     0xa102
+#define USB_REQ_HID_GET_PROTOCOL 0xa103
+#define USB_REQ_HID_SET_REPORT   0x2109
+#define USB_REQ_HID_SET_IDLE     0x210a
+#define USB_REQ_HID_SET_PROTOCOL 0x210b
+
 #define USB_DEVICE_SELF_POWERED		0
 #define USB_DEVICE_REMOTE_WAKEUP	1
 
-- 
1.6.5.2

[Qemu-devel] [PATCH V2 2/3] usb-gotemp: new module emulating a USB thermometer

[Scott Tsai]

Emulate the Vernier Go!Temp USB thermometer
(see: http://www.vernier.com/go/gotemp.html)
used in Greg Kroah-Hartman's "Write a Real, Working, Linux Driver" talk.

The emulation is complete enough for gregkh's sample driver and
using the vendor supplied SDK through the in-kernel 'ldusb' module under Linux.
No testing have yet been done with the vendor's fancier Windows software.

Signed-off-by: Scott Tsai <scottt.tw@gmail.com>
---
 Makefile        |    2 +-
 hw/usb-gotemp.c |  710 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 711 insertions(+), 1 deletions(-)
 create mode 100644 hw/usb-gotemp.c

diff --git a/Makefile b/Makefile
index 30f1c9d..54b8968 100644
--- a/Makefile
+++ b/Makefile
@@ -124,7 +124,7 @@ obj-y += i2c.o smbus.o smbus_eeprom.o
 obj-y += eeprom93xx.o
 obj-y += scsi-disk.o cdrom.o
 obj-y += scsi-generic.o scsi-bus.o
-obj-y += usb.o usb-hub.o usb-$(HOST_USB).o usb-hid.o usb-msd.o usb-wacom.o
+obj-y += usb.o usb-hub.o usb-$(HOST_USB).o usb-hid.o usb-msd.o usb-wacom.o usb-gotemp.o
 obj-y += usb-serial.o usb-net.o usb-bus.o
 obj-$(CONFIG_SSI) += ssi.o
 obj-$(CONFIG_SSI_SD) += ssi-sd.o
diff --git a/hw/usb-gotemp.c b/hw/usb-gotemp.c
new file mode 100644
index 0000000..1db6c62
--- /dev/null
+++ b/hw/usb-gotemp.c
@@ -0,0 +1,710 @@
+/*
+ * Vernier Go!Temp USB thermometer emulation
+ * see: http://www.vernier.com/go/gotemp.html
+ *
+ * Copyright (c) 2009 Scott Tsai <scottt.tw@gmail.com>
+ *
+ *  This program is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "hw.h"
+#include "usb.h"
+
+//#define DEBUG_GOTEMP
+
+#ifdef DEBUG_GOTEMP
+#define DPRINTF(fmt, ...) \
+    do { fprintf(stderr, "usb-gotemp: " fmt , ## __VA_ARGS__); } while (0)
+static void DHEXDUMP(uint8_t *buf, int len)
+{
+    int i;
+    if (!buf || !len) {
+        fprintf(stderr, "(null)\n");
+        return;
+    }
+    for (i = 0; i < len - 1; i++)
+        fprintf(stderr, "0x%02x ", buf[i]);
+    fprintf(stderr, "0x%02x\n", buf[i]);
+}
+#else
+#define DPRINTF(fmt, ...) do {} while(0)
+static void DHEXDUMP(uint8_t *buf, int len) { }
+#endif
+
+/*
+ * This device has three logical packet streams:
+ * 1. Commands in HID SET_REPORT requests to endpoint 0
+ * 2. Command responses in USB interrupt transfers from endpoint 1
+ * 3. Measurements in USB interrupt transfers also from endpoint 1
+ *
+ * All command, response and measurement packets are 8 bytes long.
+ */
+
+#define PACKET_SIZE 8
+#define QUEUE_SIZE 4  /* arbitrary */
+
+typedef struct {
+    uint8_t buf[QUEUE_SIZE][PACKET_SIZE];
+    int wp, rp;
+} Queue;
+
+static int queue_empty(Queue *q)
+{
+    return q->wp == q->rp;
+}
+
+static void queue_put(Queue *q, uint8_t *pkt)
+{
+    int next = (q->wp + 1) % QUEUE_SIZE;
+    if (next == q->rp)
+        return;
+    q->wp = next;
+    memcpy(q->buf[next], pkt, PACKET_SIZE);
+}
+
+static void queue_get(Queue *q, uint8_t *pkt)
+{
+    q->rp = (q->rp + 1) % QUEUE_SIZE;
+    memcpy(pkt, q->buf[q->rp], PACKET_SIZE);
+}
+
+#define LED_COLOR_RED          0x40
+#define LED_COLOR_GREEN        0x80
+#define LED_COLOR_RED_GREEN    0x00
+#define LED_BRIGHTNESS_MIN     0x00
+#define LED_BRIGHTNESS_MAX     0x10
+#define LED_BRIGHTNESS_DEFAULT 0x04
+
+/* Vernier product code names:
+ * Go!Link is also known as Skip.
+ * Go!Temp is also known as Jonah and is the device emulated here.
+ * Go!Motion is also known as Cyclops
+ */
+
+#define MEASUREMENT_TICK_IN_SECONDS 0.000128
+#define MEASUREMENT_PERIOD_DEFAULT_JONAH 0x0f82 /* unit: 0.000128 seconds, about 0.5 seconds */
+
+typedef struct {
+    USBDevice dev;
+    int status;                  /* as reported by CMD_ID_GET_STATUS */
+    int measuring;               /* whether measurement packets should be sent */
+    uint32_t measurement_period; /* unit: 0.000128 seconds */
+    int64_t last_measure_time;   /* unit: milliseconds in qemu_get_lock(rt_clock)  */
+    Queue response_queue;        /* queue of response packets */
+    uint8_t rolling_counter;
+    int16_t temperature;         /* unit: 1/128 Celsius */
+    uint8_t red_led_brightness;
+    uint8_t green_led_brightness;
+} GoTempState;
+
+#define MANUFACTURER_STRING       "Vernier Software & Technology"
+#define MANUFACTURER_STRING_INDEX 1
+#define PRODUCT_STRING            "Go! Temp ver 1.53"
+#define PRODUCT_STRING_INDEX      2
+
+/* MASTER_CPU_VERSION: reported in USB device descriptor and the CMD_ID_GET_STATUS command */
+#define MASTER_CPU_VERSION_MAJOR  0x01
+#define MASTER_CPU_VERSION_MINOR  0x53
+
+static const uint8_t gotemp_dev_descriptor[] = {
+    0x12,       /*  u8 bLength; */
+    0x01,       /*  u8 bDescriptorType; Device */
+    0x10, 0x01, /*  u16 bcdUSB; v1.1 */
+
+    0x00,       /*  u8  bDeviceClass; */
+    0x00,       /*  u8  bDeviceSubClass; */
+    0x00,       /*  u8  bDeviceProtocol; [ low/full speeds only ] */
+    0x08,       /*  u8  bMaxPacketSize0; 8 Bytes */
+
+    0xf7, 0x08, /*  u16 idVendor; */
+    0x02, 0x00, /*  u16 idProduct; */
+    MASTER_CPU_VERSION_MINOR, MASTER_CPU_VERSION_MAJOR, /*  u16 bcdDevice, "ver 1.53", also included in product string */
+
+    MANUFACTURER_STRING_INDEX, /*  u8  iManufacturer; */
+    PRODUCT_STRING_INDEX,      /*  u8  iProduct; */
+    0x00,       /*  u8  iSerialNumber; */
+    0x01        /*  u8  bNumConfigurations; */
+};
+
+static const uint8_t gotemp_config_descriptor[] = {
+    /* one configuration */
+    0x09,       /*  u8  bLength; */
+    0x02,       /*  u8  bDescriptorType; Configuration */
+    0x22, 0x00, /*  u16 wTotalLength; */
+    0x01,       /*  u8  bNumInterfaces; (1) */
+    0x01,       /*  u8  bConfigurationValue; */
+    0x00,       /*  u8  iConfiguration; */
+    0x80,       /*  u8  bmAttributes;
+                    Bit 7: must be set,
+6: Self-powered,
+5: Remote wakeup,
+4..0: resvd */
+    100/2,      /*  u8  MaxPower; 100mA */
+
+    /* one interface */
+    0x09,       /*  u8  if_bLength; */
+    0x04,       /*  u8  if_bDescriptorType; Interface */
+    0x00,       /*  u8  if_bInterfaceNumber; */
+    0x00,       /*  u8  if_bAlternateSetting; */
+    0x01,       /*  u8  if_bNumEndpoints; */
+    0x03,       /*  u8  if_bInterfaceClass; HID */
+    0x00,       /*  u8  if_bInterfaceSubClass; */
+    0x00,       /*  u8  if_bInterfaceProtocol; */
+    0x00,       /*  u8  if_iInterface; */
+
+    /* HID descriptor */
+    0x09,       /*  u8  bLength; */
+    0x21,       /*  u8  bDescriptorType; HID */
+    0x10, 0x01, /*  u16 bcdHID; HCD specification release number */
+    0x00,       /*  u8  bCountryCode; */
+    0x01,       /*  u8  bNumDescriptors; */
+    0x22,       /*  u8  bDescriptorType; report descriptor */
+    0x32, 0x00, /*  u16 wDescriptorLength; length of report descriptor above */
+
+    /* one endpoint (status change endpoint) */
+    0x07,       /*  u8  ep_bLength; */
+    0x05,       /*  u8  ep_bDescriptorType; Endpoint */
+    0x81,       /*  u8  ep_bEndpointAddress; IN Endpoint 1 */
+    0x03,       /*  u8  ep_bmAttributes; Interrupt */
+    0x08, 0x00, /*  u16 ep_wMaxPacketSize; */
+    0x0a        /*  u8  ep_bInterval; 10 milliseconds (low-speed) */
+};
+
+static const uint8_t gotemp_hid_report_descriptor[] = {
+    0x06, 0x00, 0xff, /* Usage Page (Vendor Defined) */
+    0x09, 0x01,       /* Usage (Vendor Defined) */
+    0xa1, 0x01,       /* Collection (Application) */
+    0x05, 0x01,       /*   Usage Page (Generic Desktop) */
+    0x09, 0x46,       /*   Usage (Vector) */
+    0x15, 0x80,       /*   Logical Minimum */
+    0x25, 0x7f,       /*   Logical Maximum */
+    0x95, 0x08,       /*   Report Count */
+    0x75, 0x08,       /*   Report Size */
+    0x81, 0x06,       /*   Input (Data, Variable, Relative) */
+    0x05, 0x01,       /*   Usage Page (Generic Desktop) */
+    0x09, 0x46,       /*   Usage (Vector) */
+    0x15, 0x80,       /*   Logical Minimum */
+    0x25, 0x7f,       /*   Logical Maximum */
+    0x95, 0x08,       /*   Report Count */
+    0x75, 0x08,       /*   Report Size */
+    0xb1, 0x06,       /*   Feature (Data, Variable, Relative) */
+    0x05, 0x08,       /*   Usage Page (LEDs) */
+    0x09, 0x2d,       /*   Usage (Ready) */
+    0x15, 0x80,       /*   Logical Minimum */
+    0x25, 0x7f,       /*   Logical Maximum */
+    0x95, 0x08,       /*   Report Count */
+    0x75, 0x08,       /*   Report Size */
+    0x91, 0x06,       /*   Output (Data, Variable, Relative) */
+    0xc0              /* End Collection */
+};
+
+/* gotemp_nv_mem: writable on real hardware */
+static const uint8_t gotemp_nv_mem[] = {
+    0x01, 0x3c, 0x21, 0x00, 0x00, 0x07,
+    0x51, 0x05, 0x54, 0x65, 0x6d, 0x70, 0x65,
+    0x72, 0x61, 0x74, 0x75, 0x72, 0x65, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x54, 0x65, 0x6d, 0x70, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x34, 0x0f, 0x01, 0x00, 0x00, 0x00, 0x3f,
+    0x00, 0x00, 0x80, 0x3f, 0xb4, 0x00, 0x00,
+    0x00, 0x01, 0x0e, 0x01, 0x00, 0x00, 0xc8,
+    0xc1, 0x00, 0x00, 0xfa, 0x42, 0x04, 0x02,
+    0x00, 0x20, 0xd0, 0x7f, 0xc3, 0xcd, 0xcc,
+    0xcc, 0x42, 0x00, 0x00, 0x00, 0x00, 0x28,
+    0x43, 0x29, 0x00, 0x00, 0x00, 0x00, 0x50,
+    0x3b, 0xd6, 0xc3, 0xec, 0x51, 0x38, 0x43,
+    0x00, 0x00, 0x00, 0x00, 0x28, 0x46, 0x29,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x7f, 0x89,
+    0x41, 0xcd, 0xcc, 0xcc, 0x42, 0x00, 0x00,
+    0x00, 0x00, 0x28, 0x4b, 0x29, 0x00, 0x00,
+    0x00, 0x00, 0x94,
+};
+
+#define MANUFACTURE_DATE_WEEEK_IN_YEAR_IN_BCD        0xff;
+#define MANUFACTURE_DATE_YEAR_LAST_TWO_DIGITS_IN_BCD 0x00;
+#define SERIAL_NUMBER                                 0x0dca1000
+
+#define CMD_ID_GET_STATUS               0x10
+#define CMD_ID_READ_LOCAL_NV_MEM        0x17
+#define CMD_ID_START_MEASUREMENTS       0x18
+#define CMD_ID_STOP_MEASUREMENTS        0x19
+#define CMD_ID_INIT                     0x1a
+#define CMD_ID_SET_MEASUREMENT_PERIOD   0x1b
+#define CMD_ID_GET_MEASUREMENT_PERIOD   0x1c
+#define CMD_ID_SET_LED_STATE            0x1d
+#define CMD_ID_GET_LED_STATE            0x1e
+#define CMD_ID_GET_SERIAL_NUMBER        0x20
+#define CMD_ID_READ_REMOTE_NV_MEM       0x27
+
+#define RESPONSE_HEADER_NV_MEM_READ             0x49
+#define RESPONSE_HEADER_CMD_SUCCESS             0x5a
+#define RESPONSE_HEADER_GET_LED_STATUS          0x5b
+#define RESPONSE_HEADER_GET_STATUS_JONAH        0x5c
+#define RESPONSE_HEADER_GET_MREASUREMENT_PERIOD 0x5d
+#define RESPONSE_HEADER_GET_SERIAL_NUMBER       0x5f
+#define RESPONSE_HEADER_CMD_ERROR               (RESPONSE_HEADER_CMD_SUCCESS | 0x20)
+#define RESPONSE_HEADER_INIT_SUCCESS            0x9a
+
+#define RESPONSE_STATUS_SUCCESS                                             0x00
+#define RESPONSE_STATUS_NOT_READY_FOR_NEW_CMD                               0x30
+#define RESPONSE_STATUS_CMD_NOT_SUPPORTED                                   0x31
+#define RESPONSE_STATUS_INTERNAL_ERROR1                                     0x32
+#define RESPONSE_STATUS_INTERNAL_ERROR2                                     0x33
+#define RESPONSE_STATUS_ERROR_CANNOT_CHANGE_PERIOD_WHILE_COLLECTING         0x34
+#define RESPONSE_STATUS_ERROR_CANNOT_READ_NV_MEM_BLK_WHILE_COLLECTING_FAST  0x35
+#define RESPONSE_STATUS_ERROR_INVALID_PARAMETER                             0x36
+#define RESPONSE_STATUS_ERROR_CANNOT_WRITE_FLASH_WHILE_COLLECTING           0x37
+#define RESPONSE_STATUS_ERROR_CANNOT_WRITE_FLASH_WHILE_HOST_FIFO_BUSY       0x38
+#define RESPONSE_STATUS_ERROR_OP_BLOCKED_WHILE_COLLECTING                   0x39
+#define RESPONSE_STATUS_ERROR_CALCULATOR_CANNOT_MEASURE_WITH_NO_BATTERIES   0x3A
+#define RESPONSE_STATUS_ERROR_SLAVE_POWERUP_INIT                            0x40
+#define RESPONSE_STATUS_ERROR_SLAVE_POWERRESTORE_INIT                       0x41
+
+static void gotemp_get_status(GoTempState *s, uint8_t *pkt)
+{
+    pkt[0] = RESPONSE_HEADER_GET_STATUS_JONAH;
+    pkt[1] = CMD_ID_GET_STATUS;
+    pkt[2] = s->status;
+    pkt[3] = MASTER_CPU_VERSION_MINOR;
+    pkt[4] = MASTER_CPU_VERSION_MAJOR;
+}
+
+static int16_t celsius_to_internal_temperature_unit(int v)
+{
+    return v * 128;
+}
+
+static void gotemp_fill_success_response(GoTempState *s, uint8_t cmd, uint8_t *pkt)
+{
+    /* Response format for most commands:
+     * pkt[0]: header
+     * pkt[1]: cmd
+     * pkt[2]: status
+     */
+
+    memset(pkt, 0, PACKET_SIZE);
+    if (cmd == CMD_ID_INIT)
+        pkt[0] = RESPONSE_HEADER_INIT_SUCCESS;
+    else
+        pkt[0] = RESPONSE_HEADER_CMD_SUCCESS;
+    pkt[1] = cmd;
+    pkt[2] = RESPONSE_STATUS_SUCCESS;
+}
+
+static void gotemp_queue_response(GoTempState *s, uint8_t *pkt)
+{
+    queue_put(&s->response_queue, pkt);
+}
+
+static int gotemp_respond(GoTempState *s, uint8_t *buf, int len)
+{
+    /* All Go!Temp response packets are 8 bytes */
+    uint8_t pkt[PACKET_SIZE];
+    int l;
+    queue_get(&s->response_queue, pkt);
+
+    l = len < PACKET_SIZE ? len : PACKET_SIZE;
+    if (pkt[0] == RESPONSE_HEADER_NV_MEM_READ) {
+        uint8_t cmd = pkt[1], addr = pkt[2], len = pkt[3], offset = pkt[4];
+        int t = len - offset;
+        if (offset == 0) {
+            if (t > 6) {
+                t = 6;
+                buf[0] = 0x49 + 0x06;
+            } else {
+                buf[0] = 0x49 + t + 0x10; /* first packet is also the last packet in NVRAM read */
+            }
+            buf[1] = cmd;
+            memcpy(buf + 2, gotemp_nv_mem + addr + offset, t);
+        } else {
+            if (t > 7) {
+                t = 7;
+                buf[0] = 0x40 + 0x07;
+            } else {
+                buf[0] = 0x40 + t + 0x10; /* last packet in NVRAM read */
+            }
+            memcpy(buf + 1, gotemp_nv_mem + addr + offset, t);
+        }
+        if (!(buf[0] & 0x10)) { /* not last packet, queue next transfer */
+            pkt[4] += t;
+            gotemp_queue_response(s, pkt);
+        }
+    } else {
+        memcpy(buf, pkt, l);
+    }
+    return l;
+}
+
+static void gotemp_read_nv_mem(GoTempState *s, uint8_t gotemp_cmd, uint8_t addr, uint8_t len, uint8_t *pkt)
+{
+    /* Need to send 'len' bytes in 6 (first packet) or 7 byte chunks.
+     * The responses to CMD_ID_*_NVRAM_READ are special cased in gotemp_respond and we're just filling
+     * an internal book keeping record here, not the real packet that gets sent over the wire.
+     * */
+    pkt[0] = RESPONSE_HEADER_NV_MEM_READ;
+    pkt[1] = gotemp_cmd;
+    pkt[2] = addr; /* requestes address */
+    pkt[3] = len;  /* requested length */
+    pkt[4] = 0x00; /* current transfer offset from address */
+}
+
+static void gotemp_set_led(GoTempState *s, uint8_t color, uint8_t brightness)
+{
+    if (brightness > LED_BRIGHTNESS_MAX)
+        brightness = LED_BRIGHTNESS_MAX;
+
+    switch (color) {
+        case LED_COLOR_RED:
+            s->red_led_brightness = brightness;
+            break;
+        case LED_COLOR_GREEN:
+            s->green_led_brightness = brightness;
+            break;
+        case LED_COLOR_RED_GREEN:
+            s->red_led_brightness = brightness;
+            s->green_led_brightness = brightness;
+            break;
+    }
+}
+
+static void gotemp_get_led(GoTempState *s, uint8_t *pkt)
+{
+    pkt[0] = RESPONSE_HEADER_GET_LED_STATUS;
+    pkt[1] = CMD_ID_GET_LED_STATE;
+    if (s->red_led_brightness && s->green_led_brightness) {
+        pkt[2] = LED_COLOR_RED_GREEN;
+        pkt[3] = s->red_led_brightness;
+    } else if (s->red_led_brightness) {
+        pkt[2] = LED_COLOR_RED;
+        pkt[3] = s->red_led_brightness;
+    } else if (s->green_led_brightness) {
+        pkt[2] = LED_COLOR_GREEN;
+        pkt[3] = s->green_led_brightness;
+    } else {
+        pkt[2] = 0x00;
+        pkt[3] = 0x00;
+    }
+}
+
+static uint32_t le32_unpack(uint8_t *buf)
+{
+    return (buf[3] << 24) | (buf[2] << 16) | (buf[1] << 8) | buf[0];
+}
+
+static void le32_pack(uint8_t *buf, uint32_t v)
+{
+    buf[0] = v & 0xff;
+    buf[1] = (v >> 8) & 0xff;
+    buf[2] = (v >> 16) & 0xff;
+    buf[3] = (v >> 24) & 0xff;
+}
+
+static void gotemp_get_serial(GoTempState *s, uint8_t *pkt)
+{
+    pkt[0] = RESPONSE_HEADER_GET_SERIAL_NUMBER;
+    pkt[1] = CMD_ID_GET_SERIAL_NUMBER;
+    pkt[2] = MANUFACTURE_DATE_WEEEK_IN_YEAR_IN_BCD;
+    pkt[3] = MANUFACTURE_DATE_YEAR_LAST_TWO_DIGITS_IN_BCD;
+    le32_pack(pkt + 4, SERIAL_NUMBER);
+}
+
+static int64_t measurement_ticks_to_ms(uint32_t ticks)
+{
+    return ticks * MEASUREMENT_TICK_IN_SECONDS * 1000;
+}
+
+static void gotemp_get_measurement_period(GoTempState *s, uint8_t *pkt)
+{
+    pkt[0] = RESPONSE_HEADER_GET_MREASUREMENT_PERIOD;
+    pkt[1] = CMD_ID_GET_MEASUREMENT_PERIOD;
+    le32_pack(pkt + 2, s->measurement_period);
+}
+
+static void gotemp_reset(GoTempState *s)
+{
+    s->measuring = 1; /* device is measuring upon reset, CMD_ID_INIT stops the measuring */
+    s->measurement_period = MEASUREMENT_PERIOD_DEFAULT_JONAH;
+    s->last_measure_time = qemu_get_clock(rt_clock);
+    gotemp_set_led(s, LED_COLOR_RED_GREEN, LED_BRIGHTNESS_DEFAULT);
+    s->response_queue.wp = s-> response_queue.rp = 0;
+    s->rolling_counter = 0;
+    s->status = RESPONSE_STATUS_SUCCESS;
+    s->temperature = celsius_to_internal_temperature_unit(25);
+}
+
+static void gotemp_handle_reset(USBDevice *dev)
+{
+    GoTempState *s = (GoTempState*)dev;
+    DPRINTF("%s\n", __func__);
+    gotemp_reset(s);
+}
+
+static int gotemp_handle_hid_set_report(GoTempState *s, uint8_t *data)
+{
+    uint8_t pkt[PACKET_SIZE];
+    uint8_t gotemp_cmd = data[0];
+    switch (gotemp_cmd) {
+        case CMD_ID_GET_STATUS:
+            gotemp_get_status(s, pkt);
+            break;
+        case CMD_ID_INIT:
+            s->measuring = 0;
+            s->measurement_period = MEASUREMENT_PERIOD_DEFAULT_JONAH;
+            gotemp_set_led(s, LED_COLOR_RED_GREEN, LED_BRIGHTNESS_DEFAULT);
+            s->response_queue.wp = s->response_queue.rp = 0;
+            s->status = RESPONSE_STATUS_SUCCESS;
+            gotemp_fill_success_response(s, gotemp_cmd, pkt);
+            break;
+        case CMD_ID_START_MEASUREMENTS:
+            s->measuring = 1;
+            s->last_measure_time = qemu_get_clock(rt_clock);
+            gotemp_fill_success_response(s, gotemp_cmd, pkt);
+            break;
+        case CMD_ID_STOP_MEASUREMENTS:
+            s->measuring = 0;
+            gotemp_fill_success_response(s, gotemp_cmd, pkt);
+            break;
+        case CMD_ID_READ_LOCAL_NV_MEM:
+        case CMD_ID_READ_REMOTE_NV_MEM:
+            gotemp_read_nv_mem(s, gotemp_cmd, data[1], data[2], pkt);
+            break;
+        case CMD_ID_SET_MEASUREMENT_PERIOD:
+            s->measurement_period = le32_unpack(data + 1);
+            gotemp_fill_success_response(s, gotemp_cmd, pkt);
+            break;
+        case CMD_ID_GET_MEASUREMENT_PERIOD:
+            gotemp_get_measurement_period(s, pkt);
+            break;
+        case CMD_ID_SET_LED_STATE:
+            gotemp_set_led(s, data[1], data[2]);
+            gotemp_fill_success_response(s, gotemp_cmd, pkt);
+            break;
+        case CMD_ID_GET_LED_STATE:
+            gotemp_get_led(s, pkt);
+            break;
+        case CMD_ID_GET_SERIAL_NUMBER:
+            gotemp_get_serial(s, pkt);
+            break;
+        default:
+            DPRINTF("%s: unsupported gotemp command: 0x%02x\n", __func__, gotemp_cmd);
+            pkt[0] = RESPONSE_HEADER_CMD_ERROR;
+            pkt[1] = gotemp_cmd;
+            pkt[2] = RESPONSE_STATUS_CMD_NOT_SUPPORTED;
+            break;
+    }
+    gotemp_queue_response(s, pkt);
+    return 0;
+}
+
+static int gotemp_handle_control(USBDevice *dev, int request, int value,
+        int index, int length, uint8_t *data)
+{
+    GoTempState *s = (GoTempState*)dev;
+    int ret = 0;
+
+    DPRINTF("%s(request: 0x%04x, value: 0x%04x, index: 0x%04x)\n",  __func__, request, value, index);
+    DPRINTF("\tdata: ");
+    DHEXDUMP(data, length > 32 ? 32 : length);
+    switch (request) {
+        case DeviceRequest | USB_REQ_GET_STATUS:
+            data[0] = (0 << USB_DEVICE_SELF_POWERED) |
+                (dev->remote_wakeup << USB_DEVICE_REMOTE_WAKEUP);
+            data[1] = 0x00;
+            ret = 2;
+            break;
+        case DeviceOutRequest | USB_REQ_CLEAR_FEATURE:
+            if (value == USB_DEVICE_REMOTE_WAKEUP) {
+                dev->remote_wakeup = 0;
+            } else {
+                goto fail;
+            }
+            ret = 0;
+            break;
+        case DeviceOutRequest | USB_REQ_SET_FEATURE:
+            if (value == USB_DEVICE_REMOTE_WAKEUP) {
+                dev->remote_wakeup = 1;
+            } else {
+                goto fail;
+            }
+            ret = 0;
+            break;
+        case DeviceOutRequest | USB_REQ_SET_ADDRESS:
+            dev->addr = value;
+            ret = 0;
+            break;
+        case DeviceRequest | USB_REQ_GET_DESCRIPTOR:
+            switch(value >> 8) {
+                case USB_DT_DEVICE:
+                    memcpy(data, gotemp_dev_descriptor,
+                            sizeof(gotemp_dev_descriptor));
+                    ret = sizeof(gotemp_dev_descriptor);
+                    break;
+                case USB_DT_CONFIG:
+                    memcpy(data, gotemp_config_descriptor,
+                            sizeof(gotemp_config_descriptor));
+                    ret = sizeof(gotemp_config_descriptor);
+                    break;
+                case USB_DT_STRING:
+                    switch(value & 0xff) {
+                        case 0:
+                            /* language ids */
+                            data[0] = 4;
+                            data[1] = 3;
+                            data[2] = 0x09; /* little endian 0x0409: en_US */
+                            data[3] = 0x04;
+                            ret = 4;
+                            break;
+                        case MANUFACTURER_STRING_INDEX:
+                            ret = set_usb_string(data, MANUFACTURER_STRING);
+                            break;
+                        case PRODUCT_STRING_INDEX:
+                            ret = set_usb_string(data, PRODUCT_STRING);
+                            break;
+                        default:
+                            goto fail;
+                    }
+                    break;
+                default:
+                    goto fail;
+            }
+            break;
+        case DeviceRequest | USB_REQ_GET_CONFIGURATION:
+            data[0] = 1;
+            ret = 1;
+            break;
+        case DeviceOutRequest | USB_REQ_SET_CONFIGURATION:
+            ret = 0;
+            break;
+        case DeviceRequest | USB_REQ_GET_INTERFACE:
+            data[0] = 0;
+            ret = 1;
+            break;
+        case InterfaceOutRequest | USB_REQ_SET_INTERFACE:
+            ret = 0;
+            break;
+            /* HID specific requests */
+        case InterfaceRequest | USB_REQ_GET_DESCRIPTOR:
+            if (value >> 8 != 0x22)
+                goto fail;
+            memcpy(data, gotemp_hid_report_descriptor, sizeof(gotemp_hid_report_descriptor));
+            ret =  sizeof(gotemp_hid_report_descriptor);
+            break;
+        case InterfaceRequest | USB_REQ_SET_CONFIGURATION:
+            break;
+        case USB_REQ_HID_GET_REPORT:
+            /* FIXME: mandatory for HID devices, verify behavior on real hardware */
+            break;
+        case USB_REQ_HID_SET_REPORT:
+            if (length < PACKET_SIZE)
+                goto fail;
+            ret = gotemp_handle_hid_set_report(s, data);
+            break;
+        default:
+fail:
+            DPRINTF("%s: unsupported request: 0x%04x, value: 0x%04x, index: 0x%04x\n", __func__, request, value, index);
+            ret = USB_RET_STALL;
+            break;
+    }
+    return ret;
+}
+
+static int gotemp_poll(GoTempState *s, uint8_t *buf, int len)
+{
+    int l;
+    int64_t now;
+    if (!s->measuring)
+        return USB_RET_NAK;
+
+    now = qemu_get_clock(rt_clock);
+    if ((now - s->last_measure_time) < measurement_ticks_to_ms(s->measurement_period))
+        return USB_RET_NAK;
+
+    s->last_measure_time = now;
+    l = 0;
+    if (len > l)
+        buf[l++] = 1; /* measurements in packet */
+    if (len > l)
+        buf[l++] = s->rolling_counter++;
+    if (len > l)
+        buf[l++] = s->temperature & 0xff;
+    if (len > l)
+        buf[l++] = s->temperature >> 8;
+    if (len > l)
+        buf[l++] = 0x00;
+    if (len > l)
+        buf[l++] = 0x00;
+    if (len > l)
+        buf[l++] = 0x00;
+    if (len > l)
+        buf[l++] = 0x00;
+    s->temperature++;
+    return l;
+}
+
+static int gotemp_handle_data(USBDevice *dev, USBPacket *p)
+{
+    GoTempState *s = (GoTempState *)dev;
+    int ret = 0;
+
+    //    DPRINTF("%s: p: {pid: 0x%02x, devep: %d}\n", __func__, p->pid, p->devep);
+    switch(p->pid) {
+        case USB_TOKEN_IN:
+            if (p->devep != 1)
+                goto fail;
+            if (!queue_empty(&s->response_queue))
+                ret =  gotemp_respond(s, p->data, p->len);
+            else
+                ret = gotemp_poll(s, p->data, p->len);
+            break;
+        case USB_TOKEN_OUT:
+        default:
+fail:
+            ret = USB_RET_STALL;
+            break;
+    }
+    return ret;
+}
+
+static void gotemp_handle_destroy(USBDevice *dev)
+{
+}
+
+static int gotemp_initfn(USBDevice *dev)
+{
+    DPRINTF("%s called\n", __func__);
+    GoTempState *s = DO_UPCAST(GoTempState, dev, dev);
+    s->dev.speed = USB_SPEED_LOW;
+    gotemp_reset(s);
+    return 0;
+}
+
+static struct USBDeviceInfo gotemp_info = {
+    .qdev.name      = "QEMU USB Thermometer",
+    .qdev.alias     = "usb-gotemp",
+    .usbdevice_name = "thermometer",
+    .qdev.size      = sizeof(GoTempState),
+    .init           = gotemp_initfn,
+    .handle_packet  = usb_generic_handle_packet,
+    .handle_reset   = gotemp_handle_reset,
+    .handle_control = gotemp_handle_control,
+    .handle_data    = gotemp_handle_data,
+    .handle_destroy = gotemp_handle_destroy,
+};
+
+static void gotemp_register_devices(void)
+{
+    usb_qdev_register(&gotemp_info);
+}
+device_init(gotemp_register_devices)
-- 
1.6.5.2

Re: [Qemu-devel] [PATCH V2 2/3] usb-gotemp: new module emulating a USB thermometer

[Avi Kivity]

On 11/10/2009 11:37 AM, Scott Tsai wrote:
> Emulate the Vernier Go!Temp USB thermometer
> (see: http://www.vernier.com/go/gotemp.html)
> used in Greg Kroah-Hartman's "Write a Real, Working, Linux Driver" talk.
>
> The emulation is complete enough for gregkh's sample driver and
> using the vendor supplied SDK through the in-kernel 'ldusb' module under Linux.
> No testing have yet been done with the vendor's fancier Windows software.
>
> +    s->temperature++;
>    

You're going to overheat very quickly.

Apart from making the driver work, is this actually useful?

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH V2 2/3] usb-gotemp: new module emulating a USB thermometer

[Scott Tsai]

>> +    s->temperature++;
>>
> You're going to overheat very quickly.
> Apart from making the driver work, is this actually useful?

I wanted the temperature to change with time to give a sense of
"something is happening" ^_^

The main user I had in mind was someone new to USB and Linux driver development
following gregkh's driver tutorial:
http://www.kroah.com/linux/talks/ols_2005_driver_tutorial/
My thinking was that if the temperature never changes, all the USB
packets generated in the 'read_int_callback"
part of the driver would seem like a waste of effort.

Re: [Qemu-devel] [PATCH V2 2/3] usb-gotemp: new module emulating a USB thermometer

[Alexander Graf]

On 10.11.2009, at 16:14, Scott Tsai wrote:

>>> +    s->temperature++;
>>>
>> You're going to overheat very quickly.
>> Apart from making the driver work, is this actually useful?
>
> I wanted the temperature to change with time to give a sense of
> "something is happening" ^_^
>
> The main user I had in mind was someone new to USB and Linux driver  
> development
> following gregkh's driver tutorial:
> http://www.kroah.com/linux/talks/ols_2005_driver_tutorial/
> My thinking was that if the temperature never changes, all the USB
> packets generated in the 'read_int_callback"
> part of the driver would seem like a waste of effort.

How about having a monitor command to change the temperature,  
leveraging a "common interface"?
That way in the future real host temperature measurements could maybe  
get forwarded there too. At least for battery I've had several people  
ask already if it's possible to read the host battery status (incl. AC  
status) from inside the VM.

Alex

Re: [Qemu-devel] [PATCH V2 2/3] usb-gotemp: new module emulating a USB thermometer

[Scott Tsai]

On Tue, Nov 10, 2009 at 11:33 PM, Alexander Graf <agraf@suse.de> wrote:
> How about having a monitor command to change the temperature, leveraging a
> "common interface"?
> That way in the future real host temperature measurements could maybe get
> forwarded there too. At least for battery I've had several people ask
> already if it's possible to read the host battery status (incl. AC status)
> from inside the VM.

I'd certainly like to make this code useful for something other than
developer training.
How about a new monitor command "thermometer_set" that works like "mouse_move"?
"thermometer_set" would just set the temperature of the "first"
thermometer device it finds.

Re: [Qemu-devel] [PATCH V2 2/3] usb-gotemp: new module emulating a USB thermometer

[Luiz Capitulino]

On Tue, 10 Nov 2009 23:55:10 +0800
Scott Tsai <scottt.tw@gmail.com> wrote:

> On Tue, Nov 10, 2009 at 11:33 PM, Alexander Graf <agraf@suse.de> wrote:
> > How about having a monitor command to change the temperature, leveraging a
> > "common interface"?
> > That way in the future real host temperature measurements could maybe get
> > forwarded there too. At least for battery I've had several people ask
> > already if it's possible to read the host battery status (incl. AC status)
> > from inside the VM.
> 
> I'd certainly like to make this code useful for something other than
> developer training.
> How about a new monitor command "thermometer_set" that works like "mouse_move"?
> "thermometer_set" would just set the temperature of the "first"
> thermometer device it finds.

 Couldn't the device be a parameter?

 And I'd suggest usb_therm_set for the name.

Re: [Qemu-devel] [PATCH V2 2/3] usb-gotemp: new module emulating a USB thermometer

[Scott Tsai]

On Wed, Nov 11, 2009 at 1:06 AM, Luiz Capitulino <lcapitulino@redhat.com> wrote:
>>
>> I'd certainly like to make this code useful for something other than
>> developer training.
>> How about a new monitor command "thermometer_set" that works like "mouse_move"?
>> "thermometer_set" would just set the temperature of the "first"
>> thermometer device it finds.
>
>  Couldn't the device be a parameter?
>
>  And I'd suggest usb_therm_set for the name.
>

Looking at the existing "mouse_set" and "mouse_move" monitor commands,
they work on USB, PS/2 and other kinds of mice with "mouse_set" selecting
the mouse device affected by  "mouse_move".
So how about a new command "therm_set" which selects the thermometer
affected by "therm_temp" ?

On a separate note, I understand that if a piece of code is not useful enough
we don't want to merge it to add to the maintenance burden.
I still propose 'usb-gotemp' for merging because the fact that gregkh
could give his
driver tutorial several years in a roll to sizable audiences shows
that there are people out there
interested in getting into Linux driver development.
With this code merged, people could follow the video and slides of his talk
without special hardware and this potentially grows the Linux developer pool.

Re: [Qemu-devel] [PATCH V2 2/3] usb-gotemp: new module emulating a USB thermometer

[Luiz Capitulino]

On Wed, 11 Nov 2009 01:52:12 +0800
Scott Tsai <scottt.tw@gmail.com> wrote:

> On Wed, Nov 11, 2009 at 1:06 AM, Luiz Capitulino <lcapitulino@redhat.com> wrote:
> >>
> >> I'd certainly like to make this code useful for something other than
> >> developer training.
> >> How about a new monitor command "thermometer_set" that works like "mouse_move"?
> >> "thermometer_set" would just set the temperature of the "first"
> >> thermometer device it finds.
> >
> >  Couldn't the device be a parameter?
> >
> >  And I'd suggest usb_therm_set for the name.
> >
> 
> Looking at the existing "mouse_set" and "mouse_move" monitor commands,
> they work on USB, PS/2 and other kinds of mice with "mouse_set" selecting
> the mouse device affected by  "mouse_move".
> So how about a new command "therm_set" which selects the thermometer
> affected by "therm_temp" ?

 This mouse command also selects the current mouse, right?

 I think your case is simpler, you just want to say which thermometer
should get the new temp.

> On a separate note, I understand that if a piece of code is not useful enough
> we don't want to merge it to add to the maintenance burden.

 Alex has given a good reason to merge it, I think.

Re: [Qemu-devel] [PATCH V2 2/3] usb-gotemp: new module emulating a USB thermometer

[Anthony Liguori]

Scott Tsai wrote:
> On Wed, Nov 11, 2009 at 1:06 AM, Luiz Capitulino <lcapitulino@redhat.com> wrote:
>   
>>> I'd certainly like to make this code useful for something other than
>>> developer training.
>>> How about a new monitor command "thermometer_set" that works like "mouse_move"?
>>> "thermometer_set" would just set the temperature of the "first"
>>> thermometer device it finds.
>>>       
>>  Couldn't the device be a parameter?
>>
>>  And I'd suggest usb_therm_set for the name.
>>
>>     
>
> Looking at the existing "mouse_set" and "mouse_move" monitor commands,
> they work on USB, PS/2 and other kinds of mice with "mouse_set" selecting
> the mouse device affected by  "mouse_move".
> So how about a new command "therm_set" which selects the thermometer
> affected by "therm_temp" ?
>
> On a separate note, I understand that if a piece of code is not useful enough
> we don't want to merge it to add to the maintenance burden.
> I still propose 'usb-gotemp' for merging because the fact that gregkh
> could give his
> driver tutorial several years in a roll to sizable audiences shows
> that there are people out there
> interested in getting into Linux driver development.
> With this code merged, people could follow the video and slides of his talk
> without special hardware and this potentially grows the Linux developer pool.
>   

And if Greg decides to change the device he uses for the tutorial, then 
in a few years it's not so useful anymore?

That said, if we position this as an example device, I think that makes 
sense.  But that suggests that we should document the heck out of it and 
make it a learning experience for QEMU too.  It could be an example of 
how to write a simple QEMU device emulation.

That would be interesting independent of Greg's tutorial.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH V2 2/3] usb-gotemp: new module emulating a USB thermometer

[Alexander Graf]

On 11.11.2009, at 01:09, Anthony Liguori wrote:

> Scott Tsai wrote:
>> On Wed, Nov 11, 2009 at 1:06 AM, Luiz Capitulino <lcapitulino@redhat.com 
>> > wrote:
>>
>>>> I'd certainly like to make this code useful for something other  
>>>> than
>>>> developer training.
>>>> How about a new monitor command "thermometer_set" that works like  
>>>> "mouse_move"?
>>>> "thermometer_set" would just set the temperature of the "first"
>>>> thermometer device it finds.
>>>>
>>> Couldn't the device be a parameter?
>>>
>>> And I'd suggest usb_therm_set for the name.
>>>
>>>
>>
>> Looking at the existing "mouse_set" and "mouse_move" monitor  
>> commands,
>> they work on USB, PS/2 and other kinds of mice with "mouse_set"  
>> selecting
>> the mouse device affected by  "mouse_move".
>> So how about a new command "therm_set" which selects the thermometer
>> affected by "therm_temp" ?
>>
>> On a separate note, I understand that if a piece of code is not  
>> useful enough
>> we don't want to merge it to add to the maintenance burden.
>> I still propose 'usb-gotemp' for merging because the fact that gregkh
>> could give his
>> driver tutorial several years in a roll to sizable audiences shows
>> that there are people out there
>> interested in getting into Linux driver development.
>> With this code merged, people could follow the video and slides of  
>> his talk
>> without special hardware and this potentially grows the Linux  
>> developer pool.
>>
>
> And if Greg decides to change the device he uses for the tutorial,  
> then in a few years it's not so useful anymore?

Well, why don't we ask him?

> That said, if we position this as an example device, I think that  
> makes sense.

I personally don't think it should be a requirement for inclusion in  
qemu that it's useful for years on for everyone using it. If there's a  
big enough group of people using a feature it seems worthwhile. And if  
Scott went through the trouble of implementing it, I'm pretty sure  
there is enough of an audience around.

>  But that suggests that we should document the heck out of it and  
> make it a learning experience for QEMU too.  It could be an example  
> of how to write a simple QEMU device emulation.

That'd be beneficial nevertheless. I'm doubtful a USB device is the  
proper target here though.

Alex

Re: [Qemu-devel] [PATCH V2 2/3] usb-gotemp: new module emulating a USB thermometer

[Greg KH]

On Wed, Nov 11, 2009 at 01:15:45AM +0100, Alexander Graf wrote:
> 
> On 11.11.2009, at 01:09, Anthony Liguori wrote:
> 
> > Scott Tsai wrote:
> >> On Wed, Nov 11, 2009 at 1:06 AM, Luiz Capitulino <lcapitulino@redhat.com 
> >> > wrote:
> >>
> >>>> I'd certainly like to make this code useful for something other  
> >>>> than
> >>>> developer training.

What code?  Where is it at?

> >>>> How about a new monitor command "thermometer_set" that works like  
> >>>> "mouse_move"?
> >>>> "thermometer_set" would just set the temperature of the "first"
> >>>> thermometer device it finds.
> >>>>
> >>> Couldn't the device be a parameter?
> >>>
> >>> And I'd suggest usb_therm_set for the name.
> >>>
> >>>
> >>
> >> Looking at the existing "mouse_set" and "mouse_move" monitor  
> >> commands,
> >> they work on USB, PS/2 and other kinds of mice with "mouse_set"  
> >> selecting
> >> the mouse device affected by  "mouse_move".
> >> So how about a new command "therm_set" which selects the thermometer
> >> affected by "therm_temp" ?
> >>
> >> On a separate note, I understand that if a piece of code is not  
> >> useful enough
> >> we don't want to merge it to add to the maintenance burden.
> >> I still propose 'usb-gotemp' for merging because the fact that gregkh
> >> could give his
> >> driver tutorial several years in a roll to sizable audiences shows
> >> that there are people out there
> >> interested in getting into Linux driver development.
> >> With this code merged, people could follow the video and slides of  
> >> his talk
> >> without special hardware and this potentially grows the Linux  
> >> developer pool.
> >>
> >
> > And if Greg decides to change the device he uses for the tutorial,  
> > then in a few years it's not so useful anymore?
> 
> Well, why don't we ask him?

I don't understand the context here.

There is already an in-kernel driver for the gotemp usb device, that has
been there for many many years.  Why would you want to write a different
one instead?

Yes, I do use a different one for my "write a driver" tutorial, but when
I give that class, I note the differences between the userspace
interface for that driver, and the one that is already in the kernel.

> > That said, if we position this as an example device, I think that  
> > makes sense.
> 
> I personally don't think it should be a requirement for inclusion in  
> qemu that it's useful for years on for everyone using it. If there's a  
> big enough group of people using a feature it seems worthwhile. And if  
> Scott went through the trouble of implementing it, I'm pretty sure  
> there is enough of an audience around.

Implementing something I wrote 4 years ago?  :)

thanks,

greg k-h

Re: [Qemu-devel] [PATCH V2 2/3] usb-gotemp: new module emulating a USB thermometer

[Alexander Graf]

On 11.11.2009, at 01:57, Greg KH wrote:

> On Wed, Nov 11, 2009 at 01:15:45AM +0100, Alexander Graf wrote:
>>
>> On 11.11.2009, at 01:09, Anthony Liguori wrote:
>>
>>> Scott Tsai wrote:
>>>> On Wed, Nov 11, 2009 at 1:06 AM, Luiz Capitulino <lcapitulino@redhat.com
>>>>> wrote:
>>>>
>>>>>> I'd certainly like to make this code useful for something other
>>>>>> than
>>>>>> developer training.
>
> What code?  Where is it at?

It's in this mail thread. It seems like gmane is behind a bit, but  
it's basically about:

http://thread.gmane.org/gmane.comp.emulators.qemu/55877

>>>>>> How about a new monitor command "thermometer_set" that works like
>>>>>> "mouse_move"?
>>>>>> "thermometer_set" would just set the temperature of the "first"
>>>>>> thermometer device it finds.
>>>>>>
>>>>> Couldn't the device be a parameter?
>>>>>
>>>>> And I'd suggest usb_therm_set for the name.
>>>>>
>>>>>
>>>>
>>>> Looking at the existing "mouse_set" and "mouse_move" monitor
>>>> commands,
>>>> they work on USB, PS/2 and other kinds of mice with "mouse_set"
>>>> selecting
>>>> the mouse device affected by  "mouse_move".
>>>> So how about a new command "therm_set" which selects the  
>>>> thermometer
>>>> affected by "therm_temp" ?
>>>>
>>>> On a separate note, I understand that if a piece of code is not
>>>> useful enough
>>>> we don't want to merge it to add to the maintenance burden.
>>>> I still propose 'usb-gotemp' for merging because the fact that  
>>>> gregkh
>>>> could give his
>>>> driver tutorial several years in a roll to sizable audiences shows
>>>> that there are people out there
>>>> interested in getting into Linux driver development.
>>>> With this code merged, people could follow the video and slides of
>>>> his talk
>>>> without special hardware and this potentially grows the Linux
>>>> developer pool.
>>>>
>>>
>>> And if Greg decides to change the device he uses for the tutorial,
>>> then in a few years it's not so useful anymore?
>>
>> Well, why don't we ask him?
>
> I don't understand the context here.

Scott implemented a qemu device emulator for the usb thermal device  
you're using in your presentation, so people could learn writing usb  
drivers with qemu.

Alex

Re: [Qemu-devel] [PATCH V2 2/3] usb-gotemp: new module emulating a USB thermometer

[Scott Tsai]

On Wed, Nov 11, 2009 at 8:57 AM, Greg KH <gregkh@suse.de> wrote:
>
> What code?  Where is it at?

http://patchwork.ozlabs.org/patch/38118/

This code emulates a Vernier Go!Temp device in qemu.
I wrote this to enable people to follow your driver tutorial without
buying the gadget.
(I implemented functionality not exercised in your tutorial code like
the LED and NV_MEM commands as well)

The current discussion is about whether this code is useful enough to
be worth to maintenance burden if merged into qemu.

Re: [Qemu-devel] [PATCH V2 2/3] usb-gotemp: new module emulating a USB thermometer

[Greg KH]

On Wed, Nov 11, 2009 at 09:06:58AM +0800, Scott Tsai wrote:
> On Wed, Nov 11, 2009 at 8:57 AM, Greg KH <gregkh@suse.de> wrote:
> >
> > What code? ??Where is it at?
> 
> http://patchwork.ozlabs.org/patch/38118/
> 
> This code emulates a Vernier Go!Temp device in qemu.
> I wrote this to enable people to follow your driver tutorial without
> buying the gadget.
> (I implemented functionality not exercised in your tutorial code like
> the LED and NV_MEM commands as well)
> 
> The current discussion is about whether this code is useful enough to
> be worth to maintenance burden if merged into qemu.

Ah, ok, nevermind then, that sounds fine to me.  If someone wants to
maintain it, go ahead :)

thanks,

greg k-h

Re: [Qemu-devel] [PATCH V2 2/3] usb-gotemp: new module emulating a USB thermometer

[Scott Tsai]

On Wed, Nov 11, 2009 at 8:09 AM, Anthony Liguori <anthony@codemonkey.ws> wrote:
> That said, if we position this as an example device, I think that makes
> sense.  But that suggests that we should document the heck out of it and
> make it a learning experience for QEMU too.  It could be an example of how
> to write a simple QEMU device emulation.
>
> That would be interesting independent of Greg's tutorial.

Thinking about how to document the 'usb-gotemp' code better:
I could improve the comments describing the packet format used by the device.

While writing the code, I remember thinking the methods in
USBDeviceInfo are fairly straight forward and getting into USB device
development through QEMU is much more pleasant then writing firmware
for memory constrained PIC and AVR microcontrollers.

I'll go off to run some errands now, I'm very willing to make any
suggested changes.

Re: [Qemu-devel] [PATCH V2 2/3] usb-gotemp: new module emulating a USB thermometer

[Avi Kivity]

On 11/10/2009 05:33 PM, Alexander Graf wrote:
> How about having a monitor command to change the temperature, 
> leveraging a "common interface"?
> That way in the future real host temperature measurements could maybe 
> get forwarded there too. At least for battery I've had several people 
> ask already if it's possible to read the host battery status (incl. AC 
> status) from inside the VM.

More ACPI functionality, so we can forward ACPI events from the host to 
the guest, would be welcome indeed.

-- 
error compiling committee.c: too many arguments to function