hid: Logitech G13 driver 0.0.3
in [Kernel]
|
Prev: [PATCH 4/7] Input: wacom - Convert remaining ids, part one
Next: mpt2sas: Fix &&/|| confusion in _scsih_sas_device_status_change_event()
From: Pavel Machek on 7 Jan 2010 14:40 On Thu 2010-01-07 09:23:24, Rick L. Vinyard Jr. wrote: > > This is a driver for the Logitech G13 gamepad, and contains three key > parts. Through the USB reports the device identifies itself as a HID, and as > a result this driver is under the HID framework. .... > Although identified as a HID, the device does not support standard HID > input messages. As a result, a sub-input device is allocated and > registered separately in g13_probe(). The raw events are monitored Ok, so the device pretends to be a HID device, but it is not. that seems like rather poor reason... > --- /dev/null > +++ b/drivers/hid/hid-g13.c > @@ -0,0 +1,1598 @@ ....to put it into hid directory. > +#define G13_G1 0 > +#define G13_G2 1 > +#define G13_G3 2 > +#define G13_G4 3 > +#define G13_G5 4 > +#define G13_G6 5 > +#define G13_G7 6 > +#define G13_G8 7 > +#define G13_G9 8 > +#define G13_G10 9 > +#define G13_G11 10 > +#define G13_G12 11 > +#define G13_G13 12 > +#define G13_G14 13 > +#define G13_G15 14 > +#define G13_G16 15 > +#define G13_G17 16 > +#define G13_G18 17 > +#define G13_G19 18 > +#define G13_G20 19 > +#define G13_G21 20 > +#define G13_G22 21 This looks like waste of space. > +static unsigned char g13_lcd_bits[] = { > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0f, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x3c, 0x00, 0x00, 0x00, 0xc0, 0x70, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xc3, 0x01, 0x00, > + 0x00, 0x20, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x80, 0x00, 0x02, 0x00, 0x00, 0x20, 0x10, 0x01, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x80, 0x40, 0x04, 0x00, 0x00, 0x10, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x40, 0x00, 0x04, 0x00, > + 0x00, 0x10, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x40, 0x00, 0x08, 0x00, 0x00, 0xd0, 0x18, 0x02, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x40, 0x63, 0x08, 0x00, 0x00, 0xd0, 0x1c, 0x02, 0x00, 0xf0, 0xff, 0xff, > + 0x83, 0xff, 0x01, 0xf8, 0xff, 0xff, 0x03, 0x00, 0x40, 0x73, 0x08, 0x00, > + 0x00, 0x10, 0x25, 0x02, 0x00, 0xf8, 0xff, 0xff, 0x83, 0xff, 0x01, 0xf8, > + 0xff, 0xff, 0x07, 0x00, 0x40, 0x94, 0x08, 0x00, 0x00, 0x10, 0x20, 0x02, > + 0x00, 0xfc, 0xff, 0xff, 0x83, 0xff, 0x01, 0xf8, 0xff, 0xff, 0x0f, 0x00, > + 0x40, 0x80, 0x08, 0x00, 0x00, 0xd0, 0x1e, 0x02, 0x00, 0xfe, 0xff, 0xff, > + 0x83, 0xff, 0x01, 0xf8, 0xff, 0xff, 0x1f, 0x00, 0x40, 0x7b, 0x08, 0x00, > + 0x00, 0xd0, 0x27, 0x02, 0x00, 0xfe, 0xff, 0xff, 0x83, 0xff, 0x01, 0xf8, > + 0xff, 0xff, 0x1f, 0x00, 0x40, 0x9f, 0x08, 0x00, 0x00, 0x90, 0x1b, 0x02, > + 0x00, 0x3e, 0x00, 0x00, 0x00, 0xf0, 0x01, 0x00, 0x00, 0x00, 0x1f, 0x00, > + 0x40, 0x6e, 0x08, 0x00, 0x00, 0x10, 0x24, 0x02, 0x00, 0x3e, 0x00, 0x00, > + 0x00, 0xf0, 0x01, 0x00, 0x00, 0x00, 0x1f, 0x00, 0x40, 0x90, 0x08, 0x00, > + 0x00, 0x48, 0x3b, 0x05, 0x00, 0x3e, 0x00, 0x00, 0x00, 0xf0, 0x01, 0x00, > + 0x00, 0x00, 0x1f, 0x00, 0x20, 0xed, 0x14, 0x00, 0x00, 0xc8, 0x7c, 0x08, > + 0x00, 0x3e, 0x00, 0x00, 0x00, 0xf0, 0x01, 0x00, 0x00, 0x00, 0x1f, 0x00, > + 0x20, 0xf3, 0x21, 0x00, 0x00, 0xe4, 0x7f, 0x10, 0x00, 0x3e, 0x00, 0x00, > + 0x00, 0xf0, 0x01, 0x00, 0x00, 0x00, 0x1f, 0x00, 0x90, 0xff, 0x41, 0x00, > + 0x00, 0xe2, 0xff, 0x20, 0x00, 0x3e, 0x00, 0x00, 0x00, 0xf0, 0x01, 0x00, > + 0x00, 0x00, 0x1f, 0x00, 0x88, 0xff, 0x83, 0x00, 0x00, 0xc2, 0x9c, 0x40, > + 0x00, 0x3e, 0xf8, 0xff, 0x07, 0xf0, 0x01, 0xf8, 0xff, 0xff, 0x1f, 0x00, > + 0x08, 0x73, 0x02, 0x01, 0x00, 0xf1, 0x3e, 0x41, 0x00, 0x3e, 0xf8, 0xff, > + 0x07, 0xf0, 0x01, 0xf8, 0xff, 0xff, 0x0f, 0x00, 0xc4, 0xfb, 0x04, 0x01, > + 0x00, 0xf1, 0x7f, 0x40, 0x00, 0x3e, 0xf8, 0xff, 0x07, 0xf0, 0x01, 0xf8, > + 0xff, 0xff, 0x07, 0x00, 0xc4, 0xff, 0x01, 0x01, 0x80, 0xf8, 0xff, 0x89, > + 0x00, 0x3e, 0xf8, 0xff, 0x07, 0xf0, 0x01, 0xf8, 0xff, 0xff, 0x0f, 0x00, > + 0xe2, 0xff, 0x27, 0x02, 0x80, 0xf8, 0xff, 0x93, 0x00, 0x3e, 0xf8, 0xff, > + 0x07, 0xf0, 0x01, 0xf8, 0xff, 0xff, 0x0f, 0x00, 0xe2, 0xff, 0x4f, 0x02, > + 0x40, 0xfc, 0xff, 0x93, 0x00, 0x3e, 0x00, 0xc0, 0x07, 0xf0, 0x01, 0x00, > + 0x00, 0x00, 0x1f, 0x00, 0xf1, 0xff, 0x4f, 0x02, 0x40, 0xfc, 0xff, 0x13, > + 0x01, 0x3e, 0x00, 0xc0, 0x07, 0xf0, 0x01, 0x00, 0x00, 0x00, 0x1f, 0x00, > + 0xf1, 0xff, 0x4f, 0x04, 0x20, 0x7c, 0xff, 0x13, 0x01, 0x3e, 0x00, 0xc0, > + 0x07, 0xf0, 0x01, 0x00, 0x00, 0x00, 0x1f, 0x80, 0xf0, 0xfd, 0x4f, 0x04, > + 0x20, 0x7c, 0xff, 0x13, 0x01, 0x3e, 0x00, 0xc0, 0x07, 0xf0, 0x01, 0x00, > + 0x00, 0x00, 0x1f, 0x80, 0xf0, 0xfd, 0x4f, 0x04, 0x20, 0x7c, 0xff, 0x3b, > + 0x01, 0x3e, 0x00, 0xc0, 0x07, 0xf0, 0x01, 0x00, 0x00, 0x00, 0x1f, 0x80, > + 0xf0, 0xfd, 0xef, 0x04, 0xa0, 0xfc, 0xff, 0x00, 0x01, 0x3e, 0x00, 0xc0, > + 0x07, 0xf0, 0x01, 0x00, 0x00, 0x00, 0x1f, 0x80, 0xf2, 0xff, 0x03, 0x04, > + 0xc0, 0xfb, 0x7f, 0x83, 0x00, 0xfe, 0xff, 0xff, 0x87, 0xff, 0x7f, 0xf8, > + 0xff, 0xff, 0x1f, 0x00, 0xef, 0xff, 0x0d, 0x02, 0xe0, 0xe7, 0x7f, 0xc7, > + 0x00, 0xfe, 0xff, 0xff, 0x87, 0xff, 0x7f, 0xf8, 0xff, 0xff, 0x1f, 0x80, > + 0x9f, 0xff, 0x1d, 0x03, 0xf0, 0xc7, 0x7f, 0xff, 0x00, 0xfc, 0xff, 0xff, > + 0x87, 0xff, 0x7f, 0xf8, 0xff, 0xff, 0x0f, 0xc0, 0x1f, 0xff, 0xfd, 0x03, > + 0xf8, 0xcf, 0x7f, 0xff, 0x03, 0xfc, 0xff, 0xff, 0x87, 0xff, 0x7f, 0xf8, > + 0xff, 0xff, 0x0f, 0xe0, 0x3f, 0xff, 0xfd, 0x0f, 0xf8, 0xcf, 0x7f, 0xff, > + 0x03, 0xf0, 0xff, 0xff, 0x87, 0xff, 0x7f, 0xf8, 0xff, 0xff, 0x03, 0xe0, > + 0x3f, 0xff, 0xfd, 0x0f, 0xfc, 0xef, 0x7f, 0xff, 0x07, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xf0, 0xbf, 0xff, 0xfd, 0x1f, > + 0xfc, 0xdf, 0x9f, 0xff, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0xf0, 0x7f, 0x7f, 0xfe, 0x0f, 0xfc, 0x3f, 0x80, 0xff, > + 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xf0, > + 0xff, 0x00, 0xfe, 0x07, 0xf8, 0x3f, 0x80, 0x7f, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xe0, 0xff, 0x00, 0xfe, 0x01, > + 0xc0, 0xdf, 0x7f, 0x3f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x7f, 0xff, 0xfd, 0x00, 0x00, 0x1c, 0x00, 0x1f, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x70, 0x00, 0x7c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }; ....as does this. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Rick L. Vinyard, Jr. on 7 Jan 2010 16:30 Pavel Machek wrote: > On Thu 2010-01-07 09:23:24, Rick L. Vinyard Jr. wrote: >> >> This is a driver for the Logitech G13 gamepad, and contains three key >> parts. Through the USB reports the device identifies itself as a HID, >> and as >> a result this driver is under the HID framework. > ... >> Although identified as a HID, the device does not support standard HID >> input messages. As a result, a sub-input device is allocated and >> registered separately in g13_probe(). The raw events are monitored > > Ok, so the device pretends to be a HID device, but it is not. that > seems like rather poor reason... > >> --- /dev/null >> +++ b/drivers/hid/hid-g13.c >> @@ -0,0 +1,1598 @@ > > ...to put it into hid directory. > From what I can tell it is compliant with the USB HID 1.11 standard. However, it doesn't support standard HID input messages like a typical mouse or keyboard device. Thus, there isn't anything for the generic parser to parse. That's why everything (related to input interrupts) has to be handled in the raw events. While many aspects of the device, such as the M1-MR LED's don't provide an LED (or similar) HID usage, they can be controlled with the generalized get/set reports. For these I've used usbhid_submit_report() where possible, so the driver itself relies most heavily on the usbhid framework first and framebuffer second. >> +#define G13_G1 0 >> +#define G13_G2 1 >> +#define G13_G3 2 >> +#define G13_G4 3 >> +#define G13_G5 4 >> +#define G13_G6 5 >> +#define G13_G7 6 >> +#define G13_G8 7 >> +#define G13_G9 8 >> +#define G13_G10 9 >> +#define G13_G11 10 >> +#define G13_G12 11 >> +#define G13_G13 12 >> +#define G13_G14 13 >> +#define G13_G15 14 >> +#define G13_G16 15 >> +#define G13_G17 16 >> +#define G13_G18 17 >> +#define G13_G19 18 >> +#define G13_G20 19 >> +#define G13_G21 20 >> +#define G13_G22 21 > > This looks like waste of space. > You're right. Those were used in an early version and are no longer needed and can be replaced with a bit of documentation indicating which key maps to which location in the keymap array. Especially for the ones that follow such as: #define G13_G19 18 #define G13_G20 19 #define G13_G21 20 #define G13_G22 21 #define G13_FUNC 22 #define G13_LCD1 23 #define G13_LCD2 24 #define G13_LCD3 25 #define G13_LCD4 26 #define G13_M1 27 #define G13_M2 28 #define G13_M3 29 #define G13_MR 30 #define G13_BTN_LEFT 31 #define G13_BTN_DOWN 32 #define G13_BTN_STICK 33 #define G13_LIGHT 34 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Jaya Kumar on 7 Jan 2010 21:40 On Fri, Jan 8, 2010 at 12:23 AM, Rick L. Vinyard Jr. <rvinyard(a)cs.nmsu.edu> wrote: > + � � � /* > + � � � �* Translate the XBM format screen_base into the format needed by the > + � � � �* G13. This format places the pixels in a vertical rather than > + � � � �* horizontal format. Assuming a grid with 0,0 in the upper left corner > + � � � �* and 159,42 in the lower right corner, the first byte contains the > + � � � �* pixels 0,0 through 0,7 and the second byte contains the pixels 1,0 > + � � � �* through 1,7. Within the byte, bit 0 represents 0,0; bit 1 0,1; etc. > + � � � �* > + � � � �* This loop operates in reverse to shift the lower bits into their > + � � � �* respective positions, shifting the lower rows into the higher bits. > + � � � �* > + � � � �* The offset is calculated for every 8 rows and is adjusted by 32 since > + � � � �* that is what the G13 image message expects. > + � � � �*/ > + � � � for (row = G13FB_HEIGHT-1; row >= 0; row--) { > + � � � � � � � offset = 32 + row/8 * G13FB_WIDTH; > + � � � � � � � u = data->fb_vbitmap + offset; > + � � � � � � � /* > + � � � � � � � �* Iterate across the screen_base columns to get the > + � � � � � � � �* individual bits > + � � � � � � � �*/ > + � � � � � � � for (col = 0; col < G13FB_LINE_LENGTH; col++) { > + � � � � � � � � � � � /* > + � � � � � � � � � � � �* We will work with a temporary value since we don't > + � � � � � � � � � � � �* want to modify screen_base as we shift each bit > + � � � � � � � � � � � �* downward. > + � � � � � � � � � � � �*/ > + � � � � � � � � � � � temp = data->fb_bitmap[row * G13FB_LINE_LENGTH + col]; > + > + � � � � � � � � � � � /* > + � � � � � � � � � � � �* For each bit in the pixel row we will shift it onto > + � � � � � � � � � � � �* the appropriate by by shift the g13 byte up by 1 and > + � � � � � � � � � � � �* simply doing a bitwise or of the low byte > + � � � � � � � � � � � �*/ > + � � � � � � � � � � � for (bit = 0; bit < 8; bit++) { > + � � � � � � � � � � � � � � � /*Shift the g13 byte up by 1 for this new row*/ > + � � � � � � � � � � � � � � � u[bit] <<= 1; > + � � � � � � � � � � � � � � � /* Bring in the new pixel of temp */ > + � � � � � � � � � � � � � � � u[bit] |= (temp & 0x01); > + � � � � � � � � � � � � � � � /* > + � � � � � � � � � � � � � � � �* Shift temp down so the low pixel is ready > + � � � � � � � � � � � � � � � �* for the next byte > + � � � � � � � � � � � � � � � �*/ > + � � � � � � � � � � � � � � � temp >>= 1; > + � � � � � � � � � � � } > + > + � � � � � � � � � � � /* > + � � � � � � � � � � � �* The last byte represented 8 vertical pixels so we'll > + � � � � � � � � � � � �* jump ahead 8 > + � � � � � � � � � � � �*/ > + � � � � � � � � � � � u += 8; > + � � � � � � � } > + � � � } I didn't read this portion carefully. I assume everything above is being accessed bytewise and that there is no chance of any unaligned access. I didn't see any endian code so can we also assume above is also endian safe. Is there a way that you could use the standard fbmem logo infrastructure in fbdev? If it is lacking in some way, maybe the right thing to do is to improve that so that other devices like this one could use it in future. > +/* > + * The "fb_node" attribute > + */ > +static ssize_t g13_fb_node_show(struct device *dev, > + � � � � � � � � � � � � � � � struct device_attribute *attr, > + � � � � � � � � � � � � � � � char *buf) > +{ > + � � � unsigned fb_node; > + � � � struct g13_data *data = dev_get_drvdata(dev); > + > + � � � if (data == NULL) > + � � � � � � � return -ENODATA; > + > + � � � fb_node = data->fb_info->node; > + > + � � � return sprintf(buf, "%u\n", fb_node); > +} > + > +static DEVICE_ATTR(fb_node, 0444, g13_fb_node_show, NULL); I didn't quite understand the purpose of above and how it would be used to the benefit of userspace. > + > + > +/* > + * The "fb_update_rate" attribute > + */ > +static ssize_t g13_fb_update_rate_show(struct device *dev, > + � � � � � � � � � � � � � � � � � � �struct device_attribute *attr, > + � � � � � � � � � � � � � � � � � � �char *buf) > +{ > + � � � unsigned fb_update_rate; > + � � � struct g13_data *data = dev_get_drvdata(dev); > + > + � � � if (data == NULL) > + � � � � � � � return -ENODATA; > + > + � � � fb_update_rate = data->fb_update_rate; > + > + � � � return sprintf(buf, "%u\n", fb_update_rate); > +} > + > +static ssize_t g13_set_fb_update_rate(struct hid_device *hdev, > + � � � � � � � � � � � � � � � � � � unsigned fb_update_rate) > +{ > + � � � struct g13_data *data = hid_get_g13data(hdev); > + > + � � � if (data == NULL) > + � � � � � � � return -ENODATA; > + > + � � � if (fb_update_rate > G13FB_UPDATE_RATE_LIMIT) > + � � � � � � � data->fb_update_rate = G13FB_UPDATE_RATE_LIMIT; > + � � � else if (fb_update_rate == 0) > + � � � � � � � data->fb_update_rate = 1; > + � � � else > + � � � � � � � data->fb_update_rate = fb_update_rate; > + > + � � � data->fb_defio.delay = HZ / data->fb_update_rate; > + > + � � � return 0; > +} > + > +static ssize_t g13_fb_update_rate_store(struct device *dev, > + � � � � � � � � � � � � � � � � � � � struct device_attribute *attr, > + � � � � � � � � � � � � � � � � � � � const char *buf, size_t count) > +{ > + � � � struct hid_device *hdev; > + � � � int i; > + � � � unsigned u; > + � � � ssize_t set_result; > + > + � � � /* Get the hid associated with the device */ > + � � � hdev = container_of(dev, struct hid_device, dev); > + > + � � � /* If we have an invalid pointer we'll return ENODATA */ > + � � � if (hdev == NULL || &(hdev->dev) != dev) > + � � � � � � � return -ENODATA; > + > + � � � i = sscanf(buf, "%u", &u); > + � � � if (i != 1) { > + � � � � � � � printk(KERN_ERR "unrecognized input: %s", buf); > + � � � � � � � return -1; > + � � � } > + > + � � � set_result = g13_set_fb_update_rate(hdev, u); > + > + � � � if (set_result < 0) > + � � � � � � � return set_result; > + > + � � � return count; > +} > + > +static DEVICE_ATTR(fb_update_rate, 0666, > + � � � � � � � � �g13_fb_update_rate_show, > + � � � � � � � � �g13_fb_update_rate_store); > + > + Okay, this update rate stuff and rate limit is interesting. I assume you are using this fbdev interface with some kind of custom fbdev client. Or are you using X or dfb with it? Thinking about it, I didn't quite understand why there's a rate limit since if you set your defio delay, then isn't that automatically limiting the rate? You might want to document what these sysfs attrs do exactly and how you want it to be used since it is exposed to userspace. > +/* > + * The "mled" attribute > + * on or off for each of the four M led's (M1 M2 M3 MR) > + */ > +static ssize_t g13_mled_show(struct device *dev, > + � � � � � � � � � � � � � �struct device_attribute *attr, > + � � � char *buf) > +{ > + � � � unsigned mled; > + � � � struct g13_data *data = dev_get_drvdata(dev); > + > + � � � if (data == NULL) > + � � � � � � � return -ENODATA; > + > + � � � mled = data->mled; > + > + � � � return sprintf(buf, "%u\n", mled); > +} > + > +static ssize_t g13_set_mled(struct hid_device *hdev, unsigned mled) > +{ > + � � � struct g13_data *data = hid_get_g13data(hdev); > + > + � � � if (data == NULL || data->mled_report == NULL) > + � � � � � � � return -ENODATA; > + > + � � � data->mled_report->field[0]->value[0] = mled&0x0F; > + � � � data->mled_report->field[0]->value[1] = 0x00; > + � � � data->mled_report->field[0]->value[2] = 0x00; > + � � � data->mled_report->field[0]->value[3] = 0x00; > + > + � � � usbhid_submit_report(hdev, data->mled_report, USB_DIR_OUT); > + > + � � � data->mled = mled; > + > + � � � return 0; > +} > + > +static ssize_t g13_mled_store(struct device *dev, > + � � � � � � � � � � � � � � struct device_attribute *attr, > + � � � �const char *buf, size_t count) > +{ > + � � � struct hid_device *hdev; > + � � � int i; > + � � � unsigned m[4]; > + � � � unsigned mled; > + � � � ssize_t set_result; > + > + � � � /* Get the hid associated with the device */ > + � � � hdev = container_of(dev, struct hid_device, dev); > + > + � � � /* If we have an invalid pointer we'll return ENODATA */ > + � � � if (hdev == NULL || &(hdev->dev) != dev) > + � � � � � � � return -ENODATA; > + > + � � � i = sscanf(buf, "%u %u %u %u", m, m+1, m+2, m+3); > + � � � if (!(i == 4 || i == 1)) { > + � � � � � � � printk(KERN_ERR "unrecognized input: %s", buf); > + � � � � � � � return -1; > + � � � } > + > + � � � if (i == 1) > + � � � � � � � mled = m[0]; > + � � � else > + � � � � � � � mled = (m[0] ? 1 : 0) | (m[1] ? 2 : 0) | > + � � � � � � � � � � � � � � � (m[2] ? 4 : 0) | (m[3] ? 8 : 0); > + > + � � � set_result = g13_set_mled(hdev, mled); > + > + � � � if (set_result < 0) > + � � � � � � � return set_result; > + > + � � � return count; > +} > + > +static DEVICE_ATTR(mled, 0666, g13_mled_show, g13_mled_store); Have you considered the use of the LED class driver as an alternative to introducing these sysfs led controls for the device? > + > +static int g13_input_setkeycode(struct input_dev *dev, > + � � � � � � � � � � � � � � � int scancode, > + � � � � � � � � � � � � � � � int keycode) > +{ > + � � � int old_keycode; > + � � � int i; > + � � � struct g13_data *data = input_get_g13data(dev); > + > + � � � if (data == NULL) > + � � � � � � � return -EINVAL; > + > + � � � if (scancode >= dev->keycodemax) > + � � � � � � � return -EINVAL; > + > + � � � if (!dev->keycodesize) > + � � � � � � � return -EINVAL; > + > + � � � if (dev->keycodesize < sizeof(keycode) && > + � � � � � (keycode >> (dev->keycodesize * 8))) > + � � � � � � � return -EINVAL; > + > + � � � write_lock(&data->lock); > + > + � � � old_keycode = data->keycode[scancode]; > + � � � data->keycode[scancode] = keycode; > + > + � � � clear_bit(old_keycode, dev->keybit); > + � � � set_bit(keycode, dev->keybit); > + > + � � � for (i = 0; i < dev->keycodemax; i++) { > + � � � � � � � if (data->keycode[i] == old_keycode) { > + � � � � � � � � � � � set_bit(old_keycode, dev->keybit); > + � � � � � � � � � � � break; /* Setting the bit twice is useless, so break */ > + � � � � � � � } > + � � � } > + > + � � � write_unlock(&data->lock); > + > + � � � return 0; > +} > + > +static int g13_input_getkeycode(struct input_dev *dev, > + � � � � � � � � � � � � � � � int scancode, > + � � � � � � � � � � � � � � � int *keycode) > +{ > + � � � struct g13_data *data = input_get_g13data(dev); > + > + � � � if (!dev->keycodesize) > + � � � � � � � return -EINVAL; > + > + � � � if (scancode >= dev->keycodemax) > + � � � � � � � return -EINVAL; > + > + � � � read_lock(&data->lock); > + > + � � � *keycode = data->keycode[scancode]; > + > + � � � read_unlock(&data->lock); > + > + � � � return 0; > +} > + > + > +/* > + * The "keymap" attribute > + */ > +static ssize_t g13_keymap_index_show(struct device *dev, > + � � � � � � � � � � � � � � � � � �struct device_attribute *attr, > + � � � � � � � � � � � � � � � � � �char *buf) > +{ > + � � � struct g13_data *data = dev_get_drvdata(dev); > + > + � � � if (data == NULL) > + � � � � � � � return -ENODATA; > + > + � � � return sprintf(buf, "%u\n", data->curkeymap); > +} > + > +static ssize_t g13_set_keymap_index(struct hid_device *hdev, unsigned k) > +{ > + � � � struct g13_data *data = hid_get_g13data(hdev); > + > + � � � if (data == NULL) > + � � � � � � � return -ENODATA; > + > + � � � if (k > 2) > + � � � � � � � return -EINVAL; > + > + � � � data->curkeymap = k; > + > + � � � if (data->keymap_switching) > + � � � � � � � g13_set_mled(hdev, 1<<k); > + > + � � � return 0; > +} > + > +static ssize_t g13_keymap_index_store(struct device *dev, > + � � � � � � � � � � � � � � � � � � struct device_attribute *attr, > + � � � � � � � � � � � � � � � � � � const char *buf, size_t count) > +{ > + � � � struct hid_device *hdev; > + � � � int i; > + � � � unsigned k; > + � � � ssize_t set_result; > + > + � � � /* Get the hid associated with the device */ > + � � � hdev = container_of(dev, struct hid_device, dev); > + > + � � � /* If we have an invalid pointer we'll return ENODATA */ > + � � � if (hdev == NULL || &(hdev->dev) != dev) > + � � � � � � � return -ENODATA; > + > + � � � i = sscanf(buf, "%u", &k); > + � � � if (i != 1) { > + � � � � � � � printk(KERN_ERR "unrecognized input: %s", buf); > + � � � � � � � return -1; > + � � � } > + > + � � � set_result = g13_set_keymap_index(hdev, k); > + > + � � � if (set_result < 0) > + � � � � � � � return set_result; > + > + � � � return count; > +} > + > +static DEVICE_ATTR(keymap_index, 0666, > + � � � � � � � � �g13_keymap_index_show, > + � � � � � � � � �g13_keymap_index_store); > + > +/* > + * The "keycode" attribute > + */ > +static ssize_t g13_keymap_show(struct device *dev, > + � � � � � � � � � � � � � � �struct device_attribute *attr, > + � � � � � � � � � � � � � � �char *buf) > +{ > + � � � int i; > + � � � int offset = 0; > + � � � int result; > + > + � � � struct g13_data *data = dev_get_drvdata(dev); > + > + � � � if (data == NULL) > + � � � � � � � return -ENODATA; > + > + � � � for (i = 0; i < G13_KEYMAP_SIZE; i++) { > + � � � � � � � result = sprintf(buf+offset, > + � � � � � � � � � � � � � � � �"0x%03x 0x%04x\n", > + � � � � � � � � � � � � � � � �i, data->keycode[i]); > + � � � � � � � if (result < 0) > + � � � � � � � � � � � return -EINVAL; > + � � � � � � � offset += result; > + � � � } > + > + � � � return offset+1; > +} > + > +static ssize_t g13_keymap_store(struct device *dev, > + � � � � � � � � � � � � � � � struct device_attribute *attr, > + � � � � � � � � � � � � � � � const char *buf, size_t count) > +{ > + � � � struct hid_device *hdev; > + � � � int scanned; > + � � � int consumed; > + � � � int scancd; > + � � � int keycd; > + � � � int set_result; > + � � � int set = 0; > + � � � int gkey; > + � � � int index; > + � � � int good; > + � � � struct g13_data *data; > + > + � � � /* Get the hid associated with the device */ > + � � � hdev = container_of(dev, struct hid_device, dev); > + > + � � � /* If we have an invalid pointer we'll return ENODATA */ > + � � � if (hdev == NULL || &(hdev->dev) != dev) > + � � � � � � � return -ENODATA; > + > + � � � /* Now, let's get the data structure */ > + � � � data = hid_get_g13data(hdev); > + � � � if (data == NULL) > + � � � � � � � return -ENODATA; > + > + � � � do { > + � � � � � � � good = 0; > + > + � � � � � � � /* Look for scancode keycode pair in hex */ > + � � � � � � � scanned = sscanf(buf, "%x %x%n", &scancd, &keycd, &consumed); > + � � � � � � � if (scanned == 2) { > + � � � � � � � � � � � buf += consumed; > + � � � � � � � � � � � set_result = g13_input_setkeycode(data->input_dev, scancd, keycd); > + � � � � � � � � � � � if (set_result < 0) > + � � � � � � � � � � � � � � � return set_result; > + � � � � � � � � � � � set++; > + � � � � � � � � � � � good = 1; > + � � � � � � � } else { > + � � � � � � � � � � � /* > + � � � � � � � � � � � �* Look for Gkey keycode pair and assign to current > + � � � � � � � � � � � �* keymap > + � � � � � � � � � � � �*/ > + � � � � � � � � � � � scanned = sscanf(buf, "G%d %x%n", &gkey, &keycd, &consumed); > + � � � � � � � � � � � if (scanned == 2 && gkey > 0 && gkey <= G13_KEYS) { > + � � � � � � � � � � � � � � � buf += consumed; > + � � � � � � � � � � � � � � � scancd = data->curkeymap * G13_KEYS + gkey - 1; > + � � � � � � � � � � � � � � � set_result = g13_input_setkeycode(data->input_dev, scancd, keycd); > + � � � � � � � � � � � � � � � if (set_result < 0) > + � � � � � � � � � � � � � � � � � � � return set_result; > + � � � � � � � � � � � � � � � set++; > + � � � � � � � � � � � � � � � good = 1; > + � � � � � � � � � � � } else { > + � � � � � � � � � � � � � � � /* > + � � � � � � � � � � � � � � � �* Look for Gkey-index keycode pair and assign > + � � � � � � � � � � � � � � � �* to indexed keymap > + � � � � � � � � � � � � � � � �*/ > + � � � � � � � � � � � � � � � scanned = sscanf(buf, "G%d-%d %x%n", &gkey, &index, &keycd, &consumed); > + � � � � � � � � � � � � � � � if (scanned == 3 && > + � � � � � � � � � � � � � � � � � gkey > 0 && gkey <= G13_KEYS && > + � � � � � � � � � � � � � � � � � index >= 0 && index <= 2) { > + � � � � � � � � � � � � � � � � � � � buf += consumed; > + � � � � � � � � � � � � � � � � � � � scancd = index * G13_KEYS + gkey - 1; > + � � � � � � � � � � � � � � � � � � � set_result = g13_input_setkeycode(data->input_dev, scancd, keycd); > + � � � � � � � � � � � � � � � � � � � if (set_result < 0) > + � � � � � � � � � � � � � � � � � � � � � � � return set_result; > + � � � � � � � � � � � � � � � � � � � set++; > + � � � � � � � � � � � � � � � � � � � good = 1; > + � � � � � � � � � � � � � � � } > + � � � � � � � � � � � } > + � � � � � � � } > + > + � � � } while (good); > + > + � � � if (set == 0) { > + � � � � � � � printk(KERN_ERR "unrecognized keycode input: %s", buf); > + � � � � � � � return -1; > + � � � } > + > + � � � return count; > +} > + > +static DEVICE_ATTR(keymap, 0666, g13_keymap_show, g13_keymap_store); > + > +/* > + * The "emit_msc_raw" attribute > + */ > +static ssize_t g13_emit_msc_raw_show(struct device *dev, > + � � � � � � � � � � � � � � � � � �struct device_attribute *attr, > + � � � � � � � � � � � � � � � � � �char *buf) > +{ > + � � � struct g13_data *data = dev_get_drvdata(dev); > + > + � � � if (data == NULL) > + � � � � � � � return -ENODATA; > + > + � � � return sprintf(buf, "%u\n", data->emit_msc_raw); > +} > + > +static ssize_t g13_set_emit_msc_raw(struct hid_device *hdev, unsigned k) > +{ > + � � � struct g13_data *data = hid_get_g13data(hdev); > + > + � � � if (data == NULL) > + � � � � � � � return -ENODATA; > + > + � � � data->emit_msc_raw = k; > + > + � � � return 0; > +} > + > +static ssize_t g13_emit_msc_raw_store(struct device *dev, > + � � � � � � � � � � � � � � � � � � struct device_attribute *attr, > + � � � � � � � � � � � � � � � � � � const char *buf, size_t count) > +{ > + � � � struct hid_device *hdev; > + � � � int i; > + � � � unsigned k; > + � � � ssize_t set_result; > + > + � � � /* Get the hid associated with the device */ > + � � � hdev = container_of(dev, struct hid_device, dev); > + > + � � � /* If we have an invalid pointer we'll return ENODATA */ > + � � � if (hdev == NULL || &(hdev->dev) != dev) > + � � � � � � � return -ENODATA; > + > + � � � i = sscanf(buf, "%u", &k); > + � � � if (i != 1) { > + � � � � � � � printk(KERN_ERR "unrecognized input: %s", buf); > + � � � � � � � return -1; > + � � � } > + > + � � � set_result = g13_set_emit_msc_raw(hdev, k); > + > + � � � if (set_result < 0) > + � � � � � � � return set_result; > + > + � � � return count; > +} > + > +static DEVICE_ATTR(emit_msc_raw, 0666, > + � � � � � � � � �g13_emit_msc_raw_show, > + � � � � � � � � �g13_emit_msc_raw_store); > + > + > +/* > + * The "keymap_switching" attribute > + */ > +static ssize_t g13_keymap_switching_show(struct device *dev, > + � � � � � � � � � � � � � � � � � � � �struct device_attribute *attr, > + � � � � � � � � � � � � � � � � � � � �char *buf) > +{ > + � � � struct g13_data *data = dev_get_drvdata(dev); > + > + � � � if (data == NULL) > + � � � � � � � return -ENODATA; > + > + � � � return sprintf(buf, "%u\n", data->keymap_switching); > +} > + > +static ssize_t g13_set_keymap_switching(struct hid_device *hdev, unsigned k) > +{ > + � � � struct g13_data *data = hid_get_g13data(hdev); > + > + � � � if (data == NULL) > + � � � � � � � return -ENODATA; > + > + � � � data->keymap_switching = k; > + > + � � � if (data->keymap_switching) > + � � � � � � � g13_set_mled(hdev, 1<<(data->curkeymap)); > + > + � � � return 0; > +} > + > +static ssize_t g13_keymap_switching_store(struct device *dev, > + � � � � � � � � � � � � � � � � � � � � struct device_attribute *attr, > + � � � � � � � � � � � � � � � � � � � � const char *buf, size_t count) > +{ > + � � � struct hid_device *hdev; > + � � � int i; > + � � � unsigned k; > + � � � ssize_t set_result; > + > + � � � /* Get the hid associated with the device */ > + � � � hdev = container_of(dev, struct hid_device, dev); > + > + � � � /* If we have an invalid pointer we'll return ENODATA */ > + � � � if (hdev == NULL || &(hdev->dev) != dev) > + � � � � � � � return -ENODATA; > + > + � � � i = sscanf(buf, "%u", &k); > + � � � if (i != 1) { > + � � � � � � � printk(KERN_ERR "unrecognized input: %s", buf); > + � � � � � � � return -1; > + � � � } > + > + � � � set_result = g13_set_keymap_switching(hdev, k); > + > + � � � if (set_result < 0) > + � � � � � � � return set_result; > + > + � � � return count; > +} > + > +static DEVICE_ATTR(keymap_switching, 0666, > + � � � � � � � � �g13_keymap_switching_show, > + � � � � � � � � �g13_keymap_switching_store); > + > + > +static ssize_t g13_name_show(struct device *dev, > + � � � � � � � � � � � � � �struct device_attribute *attr, > + � � � � � � � � � � � � � �char *buf) > +{ > + � � � struct g13_data *data = dev_get_drvdata(dev); > + � � � int result; > + > + � � � if (data == NULL) > + � � � � � � � return -ENODATA; > + > + � � � if (data->name == NULL) { > + � � � � � � � buf[0] = 0x00; > + � � � � � � � return 1; > + � � � } > + > + � � � read_lock(&data->lock); > + � � � result = sprintf(buf, "%s", data->name); > + � � � read_unlock(&data->lock); > + > + � � � return result; > +} > + > +static ssize_t g13_name_store(struct device *dev, > + � � � � � � � � � � � � � � struct device_attribute *attr, > + � � � � � � � � � � � � � � const char *buf, size_t count) > +{ > + � � � struct g13_data *data = dev_get_drvdata(dev); > + � � � size_t limit = count; > + � � � char *end; > + > + � � � if (data == NULL) > + � � � � � � � return -ENODATA; > + > + � � � write_lock(&data->lock); > + > + � � � if (data->name != NULL) { > + � � � � � � � kfree(data->name); > + � � � � � � � data->name = NULL; > + � � � } > + > + � � � end = strpbrk(buf, "\n\r"); > + � � � if (end != NULL) > + � � � � � � � limit = end - buf; > + > + � � � if (end != buf) { > + > + � � � � � � � if (limit > 100) > + � � � � � � � � � � � limit = 100; > + > + � � � � � � � data->name = kzalloc(limit+1, GFP_KERNEL); > + > + � � � � � � � strncpy(data->name, buf, limit); > + � � � } > + > + � � � write_unlock(&data->lock); > + > + � � � return count; > +} > + > +static DEVICE_ATTR(name, 0666, g13_name_show, g13_name_store); > + > +/* > + * The "rgb" attribute > + * red green blue > + * each with values 0 - 255 (black - full intensity) > + */ > +static ssize_t g13_rgb_show(struct device *dev, > + � � � � � � � � � � � � � struct device_attribute *attr, > + � � � � � � � � � � � � � char *buf) > +{ > + � � � unsigned r, g, b; > + � � � struct g13_data *data = dev_get_drvdata(dev); > + > + � � � if (data == NULL) > + � � � � � � � return -ENODATA; > + > + � � � r = data->rgb[0]; > + � � � g = data->rgb[1]; > + � � � b = data->rgb[2]; > + > + � � � return sprintf(buf, "%u %u %u\n", r, g, b); > +} > + > +static ssize_t g13_set_rgb(struct hid_device *hdev, > + � � � � � � � � � � � � �unsigned r, unsigned g, unsigned b) > +{ > + � � � struct g13_data *data = hid_get_g13data(hdev); > + > + � � � if (data == NULL || data->backlight_report == NULL) > + � � � � � � � return -ENODATA; > + > + � � � data->backlight_report->field[0]->value[0] = r; > + � � � data->backlight_report->field[0]->value[1] = g; > + � � � data->backlight_report->field[0]->value[2] = b; > + � � � data->backlight_report->field[0]->value[3] = 0x00; > + > + � � � usbhid_submit_report(hdev, data->backlight_report, USB_DIR_OUT); > + > + � � � data->rgb[0] = r; > + � � � data->rgb[1] = g; > + � � � data->rgb[2] = b; > + > + � � � return 0; > +} > + > +static ssize_t g13_rgb_store(struct device *dev, > + � � � � � � � � � � � � � �struct device_attribute *attr, > + � � � � � � � � � � � � � �const char *buf, size_t count) > +{ > + � � � struct hid_device *hdev; > + � � � int i; > + � � � unsigned r; > + � � � unsigned g; > + � � � unsigned b; > + � � � ssize_t set_result; > + > + � � � /* Get the hid associated with the device */ > + � � � hdev = container_of(dev, struct hid_device, dev); > + > + � � � /* If we have an invalid pointer we'll return ENODATA */ > + � � � if (hdev == NULL || &(hdev->dev) != dev) > + � � � � � � � return -ENODATA; > + > + � � � i = sscanf(buf, "%u %u %u", &r, &g, &b); > + � � � if (i != 3) { > + � � � � � � � printk(KERN_ERR "unrecognized input: %s", buf); > + � � � � � � � return -1; > + � � � } > + > + � � � set_result = g13_set_rgb(hdev, r, g, b); > + > + � � � if (set_result < 0) > + � � � � � � � return set_result; > + > + � � � return count; > +} > + > +static DEVICE_ATTR(rgb, 0666, g13_rgb_show, g13_rgb_store); > + > +/* > + * Create a group of attributes so that we can create and destroy them all > + * at once. > + */ > +static struct attribute *g13_attrs[] = { > + � � � &dev_attr_name.attr, > + � � � &dev_attr_rgb.attr, > + � � � &dev_attr_mled.attr, > + � � � &dev_attr_keymap_index.attr, > + � � � &dev_attr_emit_msc_raw.attr, > + � � � &dev_attr_keymap_switching.attr, > + � � � &dev_attr_keymap.attr, > + � � � &dev_attr_fb_update_rate.attr, > + � � � &dev_attr_fb_node.attr, > + � � � NULL, � �/* need to NULL terminate the list of attributes */ > +}; Hmm, that's a lot of stuff that you're exposing to userspace. I didn't read through the code carefully so I don't have a good understanding of why all that is necessary, some of them (led at least), seem to me like duplicates of existing userspace interfaces. I have to head out now, sorry for the incremental feedback, but I hope it is useful. Thanks, jaya -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Rick L. Vinyard, Jr. on 8 Jan 2010 00:30 Jaya Kumar wrote: > On Fri, Jan 8, 2010 at 12:23 AM, Rick L. Vinyard Jr. > <rvinyard(a)cs.nmsu.edu> wrote: >> + � � � /* >> + � � � �* Translate the XBM format screen_base into the format needed >> by the >> + � � � �* G13. This format places the pixels in a vertical rather than >> + � � � �* horizontal format. Assuming a grid with 0,0 in the upper left >> corner >> + � � � �* and 159,42 in the lower right corner, the first byte contains >> the >> + � � � �* pixels 0,0 through 0,7 and the second byte contains the >> pixels 1,0 >> + � � � �* through 1,7. Within the byte, bit 0 represents 0,0; bit 1 >> 0,1; etc. >> + � � � �* >> + � � � �* This loop operates in reverse to shift the lower bits into >> their >> + � � � �* respective positions, shifting the lower rows into the higher >> bits. >> + � � � �* >> + � � � �* The offset is calculated for every 8 rows and is adjusted by >> 32 since >> + � � � �* that is what the G13 image message expects. >> + � � � �*/ >> + � � � for (row = G13FB_HEIGHT-1; row >= 0; row--) { >> + � � � � � � � offset = 32 + row/8 * G13FB_WIDTH; >> + � � � � � � � u = data->fb_vbitmap + offset; >> + � � � � � � � /* >> + � � � � � � � �* Iterate across the screen_base columns to get the >> + � � � � � � � �* individual bits >> + � � � � � � � �*/ >> + � � � � � � � for (col = 0; col < G13FB_LINE_LENGTH; col++) { >> + � � � � � � � � � � � /* >> + � � � � � � � � � � � �* We will work with a temporary value since we >> don't >> + � � � � � � � � � � � �* want to modify screen_base as we shift each >> bit >> + � � � � � � � � � � � �* downward. >> + � � � � � � � � � � � �*/ >> + � � � � � � � � � � � temp = data->fb_bitmap[row * G13FB_LINE_LENGTH + >> col]; >> + >> + � � � � � � � � � � � /* >> + � � � � � � � � � � � �* For each bit in the pixel row we will shift >> it onto >> + � � � � � � � � � � � �* the appropriate by by shift the g13 byte up >> by 1 and >> + � � � � � � � � � � � �* simply doing a bitwise or of the low byte >> + � � � � � � � � � � � �*/ >> + � � � � � � � � � � � for (bit = 0; bit < 8; bit++) { >> + � � � � � � � � � � � � � � � /*Shift the g13 byte up by 1 for this >> new row*/ >> + � � � � � � � � � � � � � � � u[bit] <<= 1; >> + � � � � � � � � � � � � � � � /* Bring in the new pixel of temp */ >> + � � � � � � � � � � � � � � � u[bit] |= (temp & 0x01); >> + � � � � � � � � � � � � � � � /* >> + � � � � � � � � � � � � � � � �* Shift temp down so the low pixel is >> ready >> + � � � � � � � � � � � � � � � �* for the next byte >> + � � � � � � � � � � � � � � � �*/ >> + � � � � � � � � � � � � � � � temp >>= 1; >> + � � � � � � � � � � � } >> + >> + � � � � � � � � � � � /* >> + � � � � � � � � � � � �* The last byte represented 8 vertical pixels >> so we'll >> + � � � � � � � � � � � �* jump ahead 8 >> + � � � � � � � � � � � �*/ >> + � � � � � � � � � � � u += 8; >> + � � � � � � � } >> + � � � } > > I didn't read this portion carefully. I assume everything above is > being accessed bytewise and that there is no chance of any unaligned > access. I didn't see any endian code so can we also assume above is > also endian safe. Correct. All access is one u8 at a time so neither unaligned access nor endianness should be an issue. I couldn't think of any other way to do this since each byte in the G13 image format consists of one bit from eight bytes in the framebuffer. > Is there a way that you could use the standard fbmem > logo infrastructure in fbdev? If it is lacking in some way, maybe the > right thing to do is to improve that so that other devices like this > one could use it in future. > Ideally, we could add a 40x40 logo. But, I didn't know if anyone really wanted to go down that path until other devices needed it. The 40x40 penguin logo takes up 200 bytes. The g13 text 300 or so bytes. So there is a way the default image could be created for ~500 bytes rather than the 860 bytes the current version uses. > >> +/* >> + * The "fb_node" attribute >> + */ >> +static ssize_t g13_fb_node_show(struct device *dev, >> + � � � � � � � � � � � � � � � struct device_attribute *attr, >> + � � � � � � � � � � � � � � � char *buf) >> +{ >> + � � � unsigned fb_node; >> + � � � struct g13_data *data = dev_get_drvdata(dev); >> + >> + � � � if (data == NULL) >> + � � � � � � � return -ENODATA; >> + >> + � � � fb_node = data->fb_info->node; >> + >> + � � � return sprintf(buf, "%u\n", fb_node); >> +} >> + >> +static DEVICE_ATTR(fb_node, 0444, g13_fb_node_show, NULL); > > I didn't quite understand the purpose of above and how it would be > used to the benefit of userspace. > From libsysfs I didn't see any other way to open the framebuffer device associated with a g13 device. An example is here: http://g13.svn.sourceforge.net/viewvc/g13/trunk/g13/framebuffer.c?view=markup In particular the g13_device_framebuffer_open() function starting at line 55 uses this attribute. >> + >> + >> +/* >> + * The "fb_update_rate" attribute >> + */ >> +static ssize_t g13_fb_update_rate_show(struct device *dev, >> + � � � � � � � � � � � � � � � � � � �struct device_attribute *attr, >> + � � � � � � � � � � � � � � � � � � �char *buf) >> +{ >> + � � � unsigned fb_update_rate; >> + � � � struct g13_data *data = dev_get_drvdata(dev); >> + >> + � � � if (data == NULL) >> + � � � � � � � return -ENODATA; >> + >> + � � � fb_update_rate = data->fb_update_rate; >> + >> + � � � return sprintf(buf, "%u\n", fb_update_rate); >> +} >> + >> +static ssize_t g13_set_fb_update_rate(struct hid_device *hdev, >> + � � � � � � � � � � � � � � � � � � unsigned fb_update_rate) >> +{ >> + � � � struct g13_data *data = hid_get_g13data(hdev); >> + >> + � � � if (data == NULL) >> + � � � � � � � return -ENODATA; >> + >> + � � � if (fb_update_rate > G13FB_UPDATE_RATE_LIMIT) >> + � � � � � � � data->fb_update_rate = G13FB_UPDATE_RATE_LIMIT; >> + � � � else if (fb_update_rate == 0) >> + � � � � � � � data->fb_update_rate = 1; >> + � � � else >> + � � � � � � � data->fb_update_rate = fb_update_rate; >> + >> + � � � data->fb_defio.delay = HZ / data->fb_update_rate; >> + >> + � � � return 0; >> +} >> + >> +static ssize_t g13_fb_update_rate_store(struct device *dev, >> + � � � � � � � � � � � � � � � � � � � struct device_attribute *attr, >> + � � � � � � � � � � � � � � � � � � � const char *buf, size_t count) >> +{ >> + � � � struct hid_device *hdev; >> + � � � int i; >> + � � � unsigned u; >> + � � � ssize_t set_result; >> + >> + � � � /* Get the hid associated with the device */ >> + � � � hdev = container_of(dev, struct hid_device, dev); >> + >> + � � � /* If we have an invalid pointer we'll return ENODATA */ >> + � � � if (hdev == NULL || &(hdev->dev) != dev) >> + � � � � � � � return -ENODATA; >> + >> + � � � i = sscanf(buf, "%u", &u); >> + � � � if (i != 1) { >> + � � � � � � � printk(KERN_ERR "unrecognized input: %s", buf); >> + � � � � � � � return -1; >> + � � � } >> + >> + � � � set_result = g13_set_fb_update_rate(hdev, u); >> + >> + � � � if (set_result < 0) >> + � � � � � � � return set_result; >> + >> + � � � return count; >> +} >> + >> +static DEVICE_ATTR(fb_update_rate, 0666, >> + � � � � � � � � �g13_fb_update_rate_show, >> + � � � � � � � � �g13_fb_update_rate_store); >> + >> + > > Okay, this update rate stuff and rate limit is interesting. I assume > you are using this fbdev interface with some kind of custom fbdev > client. Or are you using X or dfb with it? At the current time I'm using it as a mmapped region. I've actually got it working with both cairo and papyrus. I don't believe dfb supports monochrome and I haven't tried X. I thought consolefb would be cool though. I looked at it, but I couldn't immediately figure out how to start a console on the fly and it hasn't been an extremely high priority. With cairo and papyrus working I can see various applets claiming segments of the LCD and using it for their display. > Thinking about it, I didn't > quite understand why there's a rate limit since if you set your defio > delay, then isn't that automatically limiting the rate? You might want > to document what these sysfs attrs do exactly and how you want it to > be used since it is exposed to userspace. > The rate limit ensures that the defio delay remains sane. >> +/* >> + * The "mled" attribute >> + * on or off for each of the four M led's (M1 M2 M3 MR) >> + */ >> +static ssize_t g13_mled_show(struct device *dev, >> + � � � � � � � � � � � � � �struct device_attribute *attr, >> + � � � char *buf) >> +{ >> + � � � unsigned mled; >> + � � � struct g13_data *data = dev_get_drvdata(dev); >> + >> + � � � if (data == NULL) >> + � � � � � � � return -ENODATA; >> + >> + � � � mled = data->mled; >> + >> + � � � return sprintf(buf, "%u\n", mled); >> +} >> + >> +static ssize_t g13_set_mled(struct hid_device *hdev, unsigned mled) >> +{ >> + � � � struct g13_data *data = hid_get_g13data(hdev); >> + >> + � � � if (data == NULL || data->mled_report == NULL) >> + � � � � � � � return -ENODATA; >> + >> + � � � data->mled_report->field[0]->value[0] = mled&0x0F; >> + � � � data->mled_report->field[0]->value[1] = 0x00; >> + � � � data->mled_report->field[0]->value[2] = 0x00; >> + � � � data->mled_report->field[0]->value[3] = 0x00; >> + >> + � � � usbhid_submit_report(hdev, data->mled_report, USB_DIR_OUT); >> + >> + � � � data->mled = mled; >> + >> + � � � return 0; >> +} >> + >> +static ssize_t g13_mled_store(struct device *dev, >> + � � � � � � � � � � � � � � struct device_attribute *attr, >> + � � � �const char *buf, size_t count) >> +{ >> + � � � struct hid_device *hdev; >> + � � � int i; >> + � � � unsigned m[4]; >> + � � � unsigned mled; >> + � � � ssize_t set_result; >> + >> + � � � /* Get the hid associated with the device */ >> + � � � hdev = container_of(dev, struct hid_device, dev); >> + >> + � � � /* If we have an invalid pointer we'll return ENODATA */ >> + � � � if (hdev == NULL || &(hdev->dev) != dev) >> + � � � � � � � return -ENODATA; >> + >> + � � � i = sscanf(buf, "%u %u %u %u", m, m+1, m+2, m+3); >> + � � � if (!(i == 4 || i == 1)) { >> + � � � � � � � printk(KERN_ERR "unrecognized input: %s", buf); >> + � � � � � � � return -1; >> + � � � } >> + >> + � � � if (i == 1) >> + � � � � � � � mled = m[0]; >> + � � � else >> + � � � � � � � mled = (m[0] ? 1 : 0) | (m[1] ? 2 : 0) | >> + � � � � � � � � � � � � � � � (m[2] ? 4 : 0) | (m[3] ? 8 : 0); >> + >> + � � � set_result = g13_set_mled(hdev, mled); >> + >> + � � � if (set_result < 0) >> + � � � � � � � return set_result; >> + >> + � � � return count; >> +} >> + >> +static DEVICE_ATTR(mled, 0666, g13_mled_show, g13_mled_store); > > Have you considered the use of the LED class driver as an alternative > to introducing these sysfs led controls for the device? > I did, but this seemed a simpler approach to let user space (such as a daemon) manage the leds. In particular this could be used by a user space program to map the keys. The MR led could be used to indicate an active record mode, etc. >> + >> +static int g13_input_setkeycode(struct input_dev *dev, >> + � � � � � � � � � � � � � � � int scancode, >> + � � � � � � � � � � � � � � � int keycode) >> +{ >> + � � � int old_keycode; >> + � � � int i; >> + � � � struct g13_data *data = input_get_g13data(dev); >> + >> + � � � if (data == NULL) >> + � � � � � � � return -EINVAL; >> + >> + � � � if (scancode >= dev->keycodemax) >> + � � � � � � � return -EINVAL; >> + >> + � � � if (!dev->keycodesize) >> + � � � � � � � return -EINVAL; >> + >> + � � � if (dev->keycodesize < sizeof(keycode) && >> + � � � � � (keycode >> (dev->keycodesize * 8))) >> + � � � � � � � return -EINVAL; >> + >> + � � � write_lock(&data->lock); >> + >> + � � � old_keycode = data->keycode[scancode]; >> + � � � data->keycode[scancode] = keycode; >> + >> + � � � clear_bit(old_keycode, dev->keybit); >> + � � � set_bit(keycode, dev->keybit); >> + >> + � � � for (i = 0; i < dev->keycodemax; i++) { >> + � � � � � � � if (data->keycode[i] == old_keycode) { >> + � � � � � � � � � � � set_bit(old_keycode, dev->keybit); >> + � � � � � � � � � � � break; /* Setting the bit twice is useless, so >> break */ >> + � � � � � � � } >> + � � � } >> + >> + � � � write_unlock(&data->lock); >> + >> + � � � return 0; >> +} >> + >> +static int g13_input_getkeycode(struct input_dev *dev, >> + � � � � � � � � � � � � � � � int scancode, >> + � � � � � � � � � � � � � � � int *keycode) >> +{ >> + � � � struct g13_data *data = input_get_g13data(dev); >> + >> + � � � if (!dev->keycodesize) >> + � � � � � � � return -EINVAL; >> + >> + � � � if (scancode >= dev->keycodemax) >> + � � � � � � � return -EINVAL; >> + >> + � � � read_lock(&data->lock); >> + >> + � � � *keycode = data->keycode[scancode]; >> + >> + � � � read_unlock(&data->lock); >> + >> + � � � return 0; >> +} >> + >> + >> +/* >> + * The "keymap" attribute >> + */ >> +static ssize_t g13_keymap_index_show(struct device *dev, >> + � � � � � � � � � � � � � � � � � �struct device_attribute *attr, >> + � � � � � � � � � � � � � � � � � �char *buf) >> +{ >> + � � � struct g13_data *data = dev_get_drvdata(dev); >> + >> + � � � if (data == NULL) >> + � � � � � � � return -ENODATA; >> + >> + � � � return sprintf(buf, "%u\n", data->curkeymap); >> +} >> + >> +static ssize_t g13_set_keymap_index(struct hid_device *hdev, unsigned >> k) >> +{ >> + � � � struct g13_data *data = hid_get_g13data(hdev); >> + >> + � � � if (data == NULL) >> + � � � � � � � return -ENODATA; >> + >> + � � � if (k > 2) >> + � � � � � � � return -EINVAL; >> + >> + � � � data->curkeymap = k; >> + >> + � � � if (data->keymap_switching) >> + � � � � � � � g13_set_mled(hdev, 1<<k); >> + >> + � � � return 0; >> +} >> + >> +static ssize_t g13_keymap_index_store(struct device *dev, >> + � � � � � � � � � � � � � � � � � � struct device_attribute *attr, >> + � � � � � � � � � � � � � � � � � � const char *buf, size_t count) >> +{ >> + � � � struct hid_device *hdev; >> + � � � int i; >> + � � � unsigned k; >> + � � � ssize_t set_result; >> + >> + � � � /* Get the hid associated with the device */ >> + � � � hdev = container_of(dev, struct hid_device, dev); >> + >> + � � � /* If we have an invalid pointer we'll return ENODATA */ >> + � � � if (hdev == NULL || &(hdev->dev) != dev) >> + � � � � � � � return -ENODATA; >> + >> + � � � i = sscanf(buf, "%u", &k); >> + � � � if (i != 1) { >> + � � � � � � � printk(KERN_ERR "unrecognized input: %s", buf); >> + � � � � � � � return -1; >> + � � � } >> + >> + � � � set_result = g13_set_keymap_index(hdev, k); >> + >> + � � � if (set_result < 0) >> + � � � � � � � return set_result; >> + >> + � � � return count; >> +} >> + >> +static DEVICE_ATTR(keymap_index, 0666, >> + � � � � � � � � �g13_keymap_index_show, >> + � � � � � � � � �g13_keymap_index_store); >> + >> +/* >> + * The "keycode" attribute >> + */ >> +static ssize_t g13_keymap_show(struct device *dev, >> + � � � � � � � � � � � � � � �struct device_attribute *attr, >> + � � � � � � � � � � � � � � �char *buf) >> +{ >> + � � � int i; >> + � � � int offset = 0; >> + � � � int result; >> + >> + � � � struct g13_data *data = dev_get_drvdata(dev); >> + >> + � � � if (data == NULL) >> + � � � � � � � return -ENODATA; >> + >> + � � � for (i = 0; i < G13_KEYMAP_SIZE; i++) { >> + � � � � � � � result = sprintf(buf+offset, >> + � � � � � � � � � � � � � � � �"0x%03x 0x%04x\n", >> + � � � � � � � � � � � � � � � �i, data->keycode[i]); >> + � � � � � � � if (result < 0) >> + � � � � � � � � � � � return -EINVAL; >> + � � � � � � � offset += result; >> + � � � } >> + >> + � � � return offset+1; >> +} >> + >> +static ssize_t g13_keymap_store(struct device *dev, >> + � � � � � � � � � � � � � � � struct device_attribute *attr, >> + � � � � � � � � � � � � � � � const char *buf, size_t count) >> +{ >> + � � � struct hid_device *hdev; >> + � � � int scanned; >> + � � � int consumed; >> + � � � int scancd; >> + � � � int keycd; >> + � � � int set_result; >> + � � � int set = 0; >> + � � � int gkey; >> + � � � int index; >> + � � � int good; >> + � � � struct g13_data *data; >> + >> + � � � /* Get the hid associated with the device */ >> + � � � hdev = container_of(dev, struct hid_device, dev); >> + >> + � � � /* If we have an invalid pointer we'll return ENODATA */ >> + � � � if (hdev == NULL || &(hdev->dev) != dev) >> + � � � � � � � return -ENODATA; >> + >> + � � � /* Now, let's get the data structure */ >> + � � � data = hid_get_g13data(hdev); >> + � � � if (data == NULL) >> + � � � � � � � return -ENODATA; >> + >> + � � � do { >> + � � � � � � � good = 0; >> + >> + � � � � � � � /* Look for scancode keycode pair in hex */ >> + � � � � � � � scanned = sscanf(buf, "%x %x%n", &scancd, &keycd, >> &consumed); >> + � � � � � � � if (scanned == 2) { >> + � � � � � � � � � � � buf += consumed; >> + � � � � � � � � � � � set_result = >> g13_input_setkeycode(data->input_dev, scancd, keycd); >> + � � � � � � � � � � � if (set_result < 0) >> + � � � � � � � � � � � � � � � return set_result; >> + � � � � � � � � � � � set++; >> + � � � � � � � � � � � good = 1; >> + � � � � � � � } else { >> + � � � � � � � � � � � /* >> + � � � � � � � � � � � �* Look for Gkey keycode pair and assign to >> current >> + � � � � � � � � � � � �* keymap >> + � � � � � � � � � � � �*/ >> + � � � � � � � � � � � scanned = sscanf(buf, "G%d %x%n", &gkey, &keycd, >> &consumed); >> + � � � � � � � � � � � if (scanned == 2 && gkey > 0 && gkey <= >> G13_KEYS) { >> + � � � � � � � � � � � � � � � buf += consumed; >> + � � � � � � � � � � � � � � � scancd = data->curkeymap * G13_KEYS + >> gkey - 1; >> + � � � � � � � � � � � � � � � set_result = >> g13_input_setkeycode(data->input_dev, scancd, keycd); >> + � � � � � � � � � � � � � � � if (set_result < 0) >> + � � � � � � � � � � � � � � � � � � � return set_result; >> + � � � � � � � � � � � � � � � set++; >> + � � � � � � � � � � � � � � � good = 1; >> + � � � � � � � � � � � } else { >> + � � � � � � � � � � � � � � � /* >> + � � � � � � � � � � � � � � � �* Look for Gkey-index keycode pair and >> assign >> + � � � � � � � � � � � � � � � �* to indexed keymap >> + � � � � � � � � � � � � � � � �*/ >> + � � � � � � � � � � � � � � � scanned = sscanf(buf, "G%d-%d %x%n", >> &gkey, &index, &keycd, &consumed); >> + � � � � � � � � � � � � � � � if (scanned == 3 && >> + � � � � � � � � � � � � � � � � � gkey > 0 && gkey <= G13_KEYS && >> + � � � � � � � � � � � � � � � � � index >= 0 && index <= 2) { >> + � � � � � � � � � � � � � � � � � � � buf += consumed; >> + � � � � � � � � � � � � � � � � � � � scancd = index * G13_KEYS + gkey >> - 1; >> + � � � � � � � � � � � � � � � � � � � set_result = >> g13_input_setkeycode(data->input_dev, scancd, keycd); >> + � � � � � � � � � � � � � � � � � � � if (set_result < 0) >> + � � � � � � � � � � � � � � � � � � � � � � � return set_result; >> + � � � � � � � � � � � � � � � � � � � set++; >> + � � � � � � � � � � � � � � � � � � � good = 1; >> + � � � � � � � � � � � � � � � } >> + � � � � � � � � � � � } >> + � � � � � � � } >> + >> + � � � } while (good); >> + >> + � � � if (set == 0) { >> + � � � � � � � printk(KERN_ERR "unrecognized keycode input: %s", buf); >> + � � � � � � � return -1; >> + � � � } >> + >> + � � � return count; >> +} >> + >> +static DEVICE_ATTR(keymap, 0666, g13_keymap_show, g13_keymap_store); >> + >> +/* >> + * The "emit_msc_raw" attribute >> + */ >> +static ssize_t g13_emit_msc_raw_show(struct device *dev, >> + � � � � � � � � � � � � � � � � � �struct device_attribute *attr, >> + � � � � � � � � � � � � � � � � � �char *buf) >> +{ >> + � � � struct g13_data *data = dev_get_drvdata(dev); >> + >> + � � � if (data == NULL) >> + � � � � � � � return -ENODATA; >> + >> + � � � return sprintf(buf, "%u\n", data->emit_msc_raw); >> +} >> + >> +static ssize_t g13_set_emit_msc_raw(struct hid_device *hdev, unsigned >> k) >> +{ >> + � � � struct g13_data *data = hid_get_g13data(hdev); >> + >> + � � � if (data == NULL) >> + � � � � � � � return -ENODATA; >> + >> + � � � data->emit_msc_raw = k; >> + >> + � � � return 0; >> +} >> + >> +static ssize_t g13_emit_msc_raw_store(struct device *dev, >> + � � � � � � � � � � � � � � � � � � struct device_attribute *attr, >> + � � � � � � � � � � � � � � � � � � const char *buf, size_t count) >> +{ >> + � � � struct hid_device *hdev; >> + � � � int i; >> + � � � unsigned k; >> + � � � ssize_t set_result; >> + >> + � � � /* Get the hid associated with the device */ >> + � � � hdev = container_of(dev, struct hid_device, dev); >> + >> + � � � /* If we have an invalid pointer we'll return ENODATA */ >> + � � � if (hdev == NULL || &(hdev->dev) != dev) >> + � � � � � � � return -ENODATA; >> + >> + � � � i = sscanf(buf, "%u", &k); >> + � � � if (i != 1) { >> + � � � � � � � printk(KERN_ERR "unrecognized input: %s", buf); >> + � � � � � � � return -1; >> + � � � } >> + >> + � � � set_result = g13_set_emit_msc_raw(hdev, k); >> + >> + � � � if (set_result < 0) >> + � � � � � � � return set_result; >> + >> + � � � return count; >> +} >> + >> +static DEVICE_ATTR(emit_msc_raw, 0666, >> + � � � � � � � � �g13_emit_msc_raw_show, >> + � � � � � � � � �g13_emit_msc_raw_store); >> + >> + >> +/* >> + * The "keymap_switching" attribute >> + */ >> +static ssize_t g13_keymap_switching_show(struct device *dev, >> + � � � � � � � � � � � � � � � � � � � �struct device_attribute *attr, >> + � � � � � � � � � � � � � � � � � � � �char *buf) >> +{ >> + � � � struct g13_data *data = dev_get_drvdata(dev); >> + >> + � � � if (data == NULL) >> + � � � � � � � return -ENODATA; >> + >> + � � � return sprintf(buf, "%u\n", data->keymap_switching); >> +} >> + >> +static ssize_t g13_set_keymap_switching(struct hid_device *hdev, >> unsigned k) >> +{ >> + � � � struct g13_data *data = hid_get_g13data(hdev); >> + >> + � � � if (data == NULL) >> + � � � � � � � return -ENODATA; >> + >> + � � � data->keymap_switching = k; >> + >> + � � � if (data->keymap_switching) >> + � � � � � � � g13_set_mled(hdev, 1<<(data->curkeymap)); >> + >> + � � � return 0; >> +} >> + >> +static ssize_t g13_keymap_switching_store(struct device *dev, >> + � � � � � � � � � � � � � � � � � � � � struct device_attribute *attr, >> + � � � � � � � � � � � � � � � � � � � � const char *buf, size_t count) >> +{ >> + � � � struct hid_device *hdev; >> + � � � int i; >> + � � � unsigned k; >> + � � � ssize_t set_result; >> + >> + � � � /* Get the hid associated with the device */ >> + � � � hdev = container_of(dev, struct hid_device, dev); >> + >> + � � � /* If we have an invalid pointer we'll return ENODATA */ >> + � � � if (hdev == NULL || &(hdev->dev) != dev) >> + � � � � � � � return -ENODATA; >> + >> + � � � i = sscanf(buf, "%u", &k); >> + � � � if (i != 1) { >> + � � � � � � � printk(KERN_ERR "unrecognized input: %s", buf); >> + � � � � � � � return -1; >> + � � � } >> + >> + � � � set_result = g13_set_keymap_switching(hdev, k); >> + >> + � � � if (set_result < 0) >> + � � � � � � � return set_result; >> + >> + � � � return count; >> +} >> + >> +static DEVICE_ATTR(keymap_switching, 0666, >> + � � � � � � � � �g13_keymap_switching_show, >> + � � � � � � � � �g13_keymap_switching_store); >> + >> + >> +static ssize_t g13_name_show(struct device *dev, >> + � � � � � � � � � � � � � �struct device_attribute *attr, >> + � � � � � � � � � � � � � �char *buf) >> +{ >> + � � � struct g13_data *data = dev_get_drvdata(dev); >> + � � � int result; >> + >> + � � � if (data == NULL) >> + � � � � � � � return -ENODATA; >> + >> + � � � if (data->name == NULL) { >> + � � � � � � � buf[0] = 0x00; >> + � � � � � � � return 1; >> + � � � } >> + >> + � � � read_lock(&data->lock); >> + � � � result = sprintf(buf, "%s", data->name); >> + � � � read_unlock(&data->lock); >> + >> + � � � return result; >> +} >> + >> +static ssize_t g13_name_store(struct device *dev, >> + � � � � � � � � � � � � � � struct device_attribute *attr, >> + � � � � � � � � � � � � � � const char *buf, size_t count) >> +{ >> + � � � struct g13_data *data = dev_get_drvdata(dev); >> + � � � size_t limit = count; >> + � � � char *end; >> + >> + � � � if (data == NULL) >> + � � � � � � � return -ENODATA; >> + >> + � � � write_lock(&data->lock); >> + >> + � � � if (data->name != NULL) { >> + � � � � � � � kfree(data->name); >> + � � � � � � � data->name = NULL; >> + � � � } >> + >> + � � � end = strpbrk(buf, "\n\r"); >> + � � � if (end != NULL) >> + � � � � � � � limit = end - buf; >> + >> + � � � if (end != buf) { >> + >> + � � � � � � � if (limit > 100) >> + � � � � � � � � � � � limit = 100; >> + >> + � � � � � � � data->name = kzalloc(limit+1, GFP_KERNEL); >> + >> + � � � � � � � strncpy(data->name, buf, limit); >> + � � � } >> + >> + � � � write_unlock(&data->lock); >> + >> + � � � return count; >> +} >> + >> +static DEVICE_ATTR(name, 0666, g13_name_show, g13_name_store); >> + >> +/* >> + * The "rgb" attribute >> + * red green blue >> + * each with values 0 - 255 (black - full intensity) >> + */ >> +static ssize_t g13_rgb_show(struct device *dev, >> + � � � � � � � � � � � � � struct device_attribute *attr, >> + � � � � � � � � � � � � � char *buf) >> +{ >> + � � � unsigned r, g, b; >> + � � � struct g13_data *data = dev_get_drvdata(dev); >> + >> + � � � if (data == NULL) >> + � � � � � � � return -ENODATA; >> + >> + � � � r = data->rgb[0]; >> + � � � g = data->rgb[1]; >> + � � � b = data->rgb[2]; >> + >> + � � � return sprintf(buf, "%u %u %u\n", r, g, b); >> +} >> + >> +static ssize_t g13_set_rgb(struct hid_device *hdev, >> + � � � � � � � � � � � � �unsigned r, unsigned g, unsigned b) >> +{ >> + � � � struct g13_data *data = hid_get_g13data(hdev); >> + >> + � � � if (data == NULL || data->backlight_report == NULL) >> + � � � � � � � return -ENODATA; >> + >> + � � � data->backlight_report->field[0]->value[0] = r; >> + � � � data->backlight_report->field[0]->value[1] = g; >> + � � � data->backlight_report->field[0]->value[2] = b; >> + � � � data->backlight_report->field[0]->value[3] = 0x00; >> + >> + � � � usbhid_submit_report(hdev, data->backlight_report, USB_DIR_OUT); >> + >> + � � � data->rgb[0] = r; >> + � � � data->rgb[1] = g; >> + � � � data->rgb[2] = b; >> + >> + � � � return 0; >> +} >> + >> +static ssize_t g13_rgb_store(struct device *dev, >> + � � � � � � � � � � � � � �struct device_attribute *attr, >> + � � � � � � � � � � � � � �const char *buf, size_t count) >> +{ >> + � � � struct hid_device *hdev; >> + � � � int i; >> + � � � unsigned r; >> + � � � unsigned g; >> + � � � unsigned b; >> + � � � ssize_t set_result; >> + >> + � � � /* Get the hid associated with the device */ >> + � � � hdev = container_of(dev, struct hid_device, dev); >> + >> + � � � /* If we have an invalid pointer we'll return ENODATA */ >> + � � � if (hdev == NULL || &(hdev->dev) != dev) >> + � � � � � � � return -ENODATA; >> + >> + � � � i = sscanf(buf, "%u %u %u", &r, &g, &b); >> + � � � if (i != 3) { >> + � � � � � � � printk(KERN_ERR "unrecognized input: %s", buf); >> + � � � � � � � return -1; >> + � � � } >> + >> + � � � set_result = g13_set_rgb(hdev, r, g, b); >> + >> + � � � if (set_result < 0) >> + � � � � � � � return set_result; >> + >> + � � � return count; >> +} >> + >> +static DEVICE_ATTR(rgb, 0666, g13_rgb_show, g13_rgb_store); >> + >> +/* >> + * Create a group of attributes so that we can create and destroy them >> all >> + * at once. >> + */ >> +static struct attribute *g13_attrs[] = { >> + � � � &dev_attr_name.attr, >> + � � � &dev_attr_rgb.attr, >> + � � � &dev_attr_mled.attr, >> + � � � &dev_attr_keymap_index.attr, >> + � � � &dev_attr_emit_msc_raw.attr, >> + � � � &dev_attr_keymap_switching.attr, >> + � � � &dev_attr_keymap.attr, >> + � � � &dev_attr_fb_update_rate.attr, >> + � � � &dev_attr_fb_node.attr, >> + � � � NULL, � �/* need to NULL terminate the list of attributes */ >> +}; > > Hmm, that's a lot of stuff that you're exposing to userspace. I didn't > read through the code carefully so I don't have a good understanding > of why all that is necessary, some of them (led at least), seem to me > like duplicates of existing userspace interfaces. There is alot exposed to userspace, but there is a use for everything exposed. The rgb is so that a userspace app can set the backlight colors. The driver supports three keymaps for fast switching using the M1, M2 and M3 keys. But, with the keymap, emit_msc_raw and mled attributes exposed a userspace app can support other combinations such a M1, M2, M3, M1+M2, M1+M3, M2+M3, etc. I also think it would be useful if the keymap set switched out according to the application that has the current focus (and perhaps the backlight color were set as well). That would require the aforementioned to be exposed to something that watched the focus from userspace and took the necessary actions. > I have to head out > now, sorry for the incremental feedback, but I hope it is useful. > No problem with incremental feedback. It's definitely appreciated. Thanks again, Rick -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Dmitry Torokhov on 8 Jan 2010 03:40
Hi Rick, On Thu, Jan 07, 2010 at 09:23:24AM -0700, Rick L. Vinyard Jr. wrote: > > This is a driver for the Logitech G13 gamepad, and contains three key > parts. Through the USB reports the device identifies itself as a HID, and as > a result this driver is under the HID framework. > > There are two primary sub-components to this driver; an input device and a > framebuffer device. > > Although identified as a HID, the device does not support standard HID > input messages. As a result, a sub-input device is allocated and > registered separately in g13_probe(). The raw events are monitored and > key presses/joystick activity is reported through the input device after > referencing an indexed keymap. Finally had some time to look through the patch, comments below. > > Additionally, this device contains a 160x43 monochrome LCD display. A > registered framebuffer device manages this display. The design of this > portion of the driver was based on the design of the hecubafb driver with > deferred framebuffer I/O since there is no real memory to map. > > Changes since 0.0.2: > - Moved the module out of the logitech objs to build on its own > > - Added dependency on FB_DEFFERRED_IO > > - Added explanation as to why the load image is used and how to view it > outside the code. > > - Changed the load image to text "G13" with default penguins resized, > cleaned up and inverted from framebuffer monochrome penguin logo. > > - Cleaned up the raw event callback by moving the key event processing to a > separate function. > > - Removed several of the line breaks introduced to accomodate 80-column lines > to improve readability. > > - Reordeded event processing and utility functions to eliminate the need for > a g13_set_mled() prototype. > > - Removed nonstd flag from framebuffer screeninfo structure > > Signed-off-by: Rick L. Vinyard, Jr <rvinyard(a)cs.nmsu.edu> > --- > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig > index 24d90ea..548dd4a 100644 > --- a/drivers/hid/Kconfig > +++ b/drivers/hid/Kconfig > @@ -183,6 +183,20 @@ config LOGIRUMBLEPAD2_FF > Say Y here if you want to enable force feedback support for Logitech > Rumblepad 2 devices. > > +config HID_LOGITECH_G13 > + tristate "Logitech G13 gameboard support" > + depends on FB > + depends on FB_DEFERRED_IO > + select FB_SYS_FILLRECT > + select FB_SYS_COPYAREA > + select FB_SYS_IMAGEBLIT > + select FB_SYS_FOPS > + help > + This provides support for Logitech G13 gameboard > + devices. This includes support for the device > + as a keypad input with mappable keys as well as > + a framebuffer for the LCD display. > + > config HID_MICROSOFT > tristate "Microsoft" if EMBEDDED > depends on USB_HID > diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile > index 0de2dff..d39dc5b 100644 > --- a/drivers/hid/Makefile > +++ b/drivers/hid/Makefile > @@ -31,6 +31,7 @@ obj-$(CONFIG_HID_GYRATION) += hid-gyration.o > obj-$(CONFIG_HID_KENSINGTON) += hid-kensington.o > obj-$(CONFIG_HID_KYE) += hid-kye.o > obj-$(CONFIG_HID_LOGITECH) += hid-logitech.o > +obj-$(CONFIG_HID_LOGITECH_G13) += hid-g13.o > obj-$(CONFIG_HID_MICROSOFT) += hid-microsoft.o > obj-$(CONFIG_HID_MONTEREY) += hid-monterey.o > obj-$(CONFIG_HID_NTRIG) += hid-ntrig.o > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c > index 80792d3..eeae383 100644 > --- a/drivers/hid/hid-core.c > +++ b/drivers/hid/hid-core.c > @@ -1325,6 +1325,7 @@ static const struct hid_device_id hid_blacklist[] = { > { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_MOMO_WHEEL2) }, > { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_G25_WHEEL) }, > { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RUMBLEPAD2) }, > + { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_G13) }, > { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_SPACETRAVELLER) }, > { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_SPACENAVIGATOR) }, > { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_SIDEWINDER_GV) }, > diff --git a/drivers/hid/hid-g13.c b/drivers/hid/hid-g13.c > new file mode 100644 > index 0000000..63c3044 > --- /dev/null > +++ b/drivers/hid/hid-g13.c > @@ -0,0 +1,1598 @@ > +/*************************************************************************** > + * Copyright (C) 2009 by Rick L. Vinyard, Jr. * > + * rvinyard(a)cs.nmsu.edu * > + * * > + * 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 driver 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 software. If not see <http://www.gnu.org/licenses/>. * > + ***************************************************************************/ > +#include <linux/fb.h> > +#include <linux/hid.h> > +#include <linux/init.h> > +#include <linux/input.h> > +#include <linux/mm.h> > +#include <linux/sysfs.h> > +#include <linux/uaccess.h> > +#include <linux/usb.h> > +#include <linux/vmalloc.h> > + > +#include "hid-ids.h" > +#include "usbhid/usbhid.h" > + > +#define G13_NAME "Logitech G13" > + > +/* Key defines */ > +#define G13_KEYS 35 > +#define G13_KEYMAP_SIZE (G13_KEYS*3) > + > +/* G1-G22 indices */ > +#define G13_G1 0 > +#define G13_G2 1 > +#define G13_G3 2 > +#define G13_G4 3 > +#define G13_G5 4 > +#define G13_G6 5 > +#define G13_G7 6 > +#define G13_G8 7 > +#define G13_G9 8 > +#define G13_G10 9 > +#define G13_G11 10 > +#define G13_G12 11 > +#define G13_G13 12 > +#define G13_G14 13 > +#define G13_G15 14 > +#define G13_G16 15 > +#define G13_G17 16 > +#define G13_G18 17 > +#define G13_G19 18 > +#define G13_G20 19 > +#define G13_G21 20 > +#define G13_G22 21 > +#define G13_FUNC 22 > +#define G13_LCD1 23 > +#define G13_LCD2 24 > +#define G13_LCD3 25 > +#define G13_LCD4 26 > +#define G13_M1 27 > +#define G13_M2 28 > +#define G13_M3 29 > +#define G13_MR 30 > +#define G13_BTN_LEFT 31 > +#define G13_BTN_DOWN 32 > +#define G13_BTN_STICK 33 > +#define G13_LIGHT 34 > + > +/* Framebuffer defines */ > +#define G13FB_NAME "g13fb" > +#define G13FB_WIDTH (160) > +#define G13FB_LINE_LENGTH (160/8) > +#define G13FB_HEIGHT (43) > +#define G13FB_SIZE (G13FB_LINE_LENGTH*G13FB_HEIGHT) > + > +#define G13FB_UPDATE_RATE_LIMIT (20) > +#define G13FB_UPDATE_RATE_DEFAULT (10) > + > +/* > + * The native G13 format uses vertical bits. Therefore the number of bytes > + * needed to represent the first column is 43/8 (rows/bits) rounded up. > + * Additionally, the format requires a padding of 32 bits in front of the > + * image data. > + * > + * Therefore the vbitmap size must be: > + * 160 * ceil(43/8) + 32 = 160 * 6 + 32 = 992 > + */ > +#define G13_VBITMAP_SIZE (992) > + > +/* Backlight defaults */ > +#define G13_DEFAULT_RED (0) > +#define G13_DEFAULT_GREEN (255) > +#define G13_DEFAULT_BLUE (0) > + > +/* Per device data structure */ > +struct g13_data { > + /* HID reports */ > + struct hid_device *hdev; > + struct hid_report *backlight_report; > + struct hid_report *start_input_report; > + struct hid_report *report_4; > + struct hid_report *mled_report; > + struct input_dev *input_dev; > + > + char *name; > + int ready; > + int ready2; > + u32 keycode[G13_KEYMAP_SIZE]; > + u8 rgb[3]; > + u8 mled; > + u8 curkeymap; > + u8 emit_msc_raw; > + u8 keymap_switching; > + > + /* Framebuffer stuff */ > + u8 fb_update_rate; > + u8 *fb_vbitmap; > + u8 *fb_bitmap; > + struct fb_info *fb_info; > + struct fb_deferred_io fb_defio; > + > + /* Housekeeping stuff */ > + rwlock_t lock; > +}; > + > +/* Convenience macros */ > +#define hid_get_g13data(hdev) \ > + ((hdev == NULL) ? NULL : (struct g13_data *)(hid_get_drvdata(hdev))) > + > +#define input_get_hdev(idev) \ > + ((idev == NULL) ? NULL : (struct hid_device *)(input_get_drvdata(idev))) > + > +#define input_get_g13data(idev) (hid_get_g13data(input_get_hdev(idev))) I wonder whether these NULL tests are helpful or if they will simply mask potential bugs in the driver. For example - when (in this particular driver) input_get_drvdata would return NULL? You set it before registering the device so any input device method will have it set. I'd remove them. > + > +static unsigned int g13_default_key_map[G13_KEYS] = { const. > + /* first row g1 - g7 */ > + KEY_F1, KEY_F2, KEY_F3, KEY_F4, KEY_F5, KEY_F6, KEY_F7, > + /* second row g8 - g11 */ > + KEY_UNKNOWN, KEY_UNKNOWN, KEY_BACK, KEY_UP, > + /* second row g12 - g13 */ > + KEY_FORWARD, KEY_UNKNOWN, KEY_UNKNOWN, > + /* third row g15 - g19 */ > + KEY_UNKNOWN, KEY_LEFT, KEY_DOWN, KEY_RIGHT, KEY_UNKNOWN, > + /* fourth row g20 - g22 */ > + KEY_BACKSPACE, KEY_ENTER, KEY_SPACE, > + /* next, light left, light center left, light center right, light right */ > + BTN_0, BTN_1, BTN_2, BTN_3, BTN_4, > + /* M1, M2, M3, MR */ > + KEY_RESERVED, KEY_RESERVED, KEY_RESERVED, KEY_RESERVED, > + /* button left, button down, button stick, light */ > + BTN_LEFT, BTN_RIGHT, BTN_MIDDLE, KEY_RESERVED, > +}; > + > +/* Framebuffer visual structures */ > +static struct fb_fix_screeninfo g13fb_fix = { > + .id = G13FB_NAME, > + .type = FB_TYPE_PACKED_PIXELS, > + .visual = FB_VISUAL_MONO01, > + .xpanstep = 0, > + .ypanstep = 0, > + .ywrapstep = 0, > + .line_length = G13FB_LINE_LENGTH, > + .accel = FB_ACCEL_NONE, > +}; > + > +static struct fb_var_screeninfo g13fb_var = { > + .xres = G13FB_WIDTH, > + .yres = G13FB_HEIGHT, > + .xres_virtual = G13FB_WIDTH, > + .yres_virtual = G13FB_HEIGHT, > + .bits_per_pixel = 1, > +}; > + > +/* > + * This is a default logo to be loaded upon driver initialization > + * replacing the default Logitech G13 image loaded on device > + * initialization. This is to provide the user a cue that the > + * Linux driver is loaded and ready. > + * > + * This logo contains the text G13 in the center with two penguins > + * on each side of the text. The penguins are a 40x40 rendition of > + * the default framebuffer 80x80 monochrome image scaled down and > + * cleaned up to retain something that looks a little better than > + * a simple scaling. > + * > + * This logo is a simple xbm image created in GIMP and exported. > + * To view the image copy the following two #defines plus the > + * g13_lcd_bits to an ASCII text file and save with extension > + * .xbm, then open with GIMP or any other graphical editor > + * such as eog that recognizes the .xbm format. > + */ > +#define g13_lcd_width 160 > +#define g13_lcd_height 43 > +static unsigned char g13_lcd_bits[] = { > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0f, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x3c, 0x00, 0x00, 0x00, 0xc0, 0x70, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xc3, 0x01, 0x00, > + 0x00, 0x20, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x80, 0x00, 0x02, 0x00, 0x00, 0x20, 0x10, 0x01, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x80, 0x40, 0x04, 0x00, 0x00, 0x10, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x40, 0x00, 0x04, 0x00, > + 0x00, 0x10, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x40, 0x00, 0x08, 0x00, 0x00, 0xd0, 0x18, 0x02, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x40, 0x63, 0x08, 0x00, 0x00, 0xd0, 0x1c, 0x02, 0x00, 0xf0, 0xff, 0xff, > + 0x83, 0xff, 0x01, 0xf8, 0xff, 0xff, 0x03, 0x00, 0x40, 0x73, 0x08, 0x00, > + 0x00, 0x10, 0x25, 0x02, 0x00, 0xf8, 0xff, 0xff, 0x83, 0xff, 0x01, 0xf8, > + 0xff, 0xff, 0x07, 0x00, 0x40, 0x94, 0x08, 0x00, 0x00, 0x10, 0x20, 0x02, > + 0x00, 0xfc, 0xff, 0xff, 0x83, 0xff, 0x01, 0xf8, 0xff, 0xff, 0x0f, 0x00, > + 0x40, 0x80, 0x08, 0x00, 0x00, 0xd0, 0x1e, 0x02, 0x00, 0xfe, 0xff, 0xff, > + 0x83, 0xff, 0x01, 0xf8, 0xff, 0xff, 0x1f, 0x00, 0x40, 0x7b, 0x08, 0x00, > + 0x00, 0xd0, 0x27, 0x02, 0x00, 0xfe, 0xff, 0xff, 0x83, 0xff, 0x01, 0xf8, > + 0xff, 0xff, 0x1f, 0x00, 0x40, 0x9f, 0x08, 0x00, 0x00, 0x90, 0x1b, 0x02, > + 0x00, 0x3e, 0x00, 0x00, 0x00, 0xf0, 0x01, 0x00, 0x00, 0x00, 0x1f, 0x00, > + 0x40, 0x6e, 0x08, 0x00, 0x00, 0x10, 0x24, 0x02, 0x00, 0x3e, 0x00, 0x00, > + 0x00, 0xf0, 0x01, 0x00, 0x00, 0x00, 0x1f, 0x00, 0x40, 0x90, 0x08, 0x00, > + 0x00, 0x48, 0x3b, 0x05, 0x00, 0x3e, 0x00, 0x00, 0x00, 0xf0, 0x01, 0x00, > + 0x00, 0x00, 0x1f, 0x00, 0x20, 0xed, 0x14, 0x00, 0x00, 0xc8, 0x7c, 0x08, > + 0x00, 0x3e, 0x00, 0x00, 0x00, 0xf0, 0x01, 0x00, 0x00, 0x00, 0x1f, 0x00, > + 0x20, 0xf3, 0x21, 0x00, 0x00, 0xe4, 0x7f, 0x10, 0x00, 0x3e, 0x00, 0x00, > + 0x00, 0xf0, 0x01, 0x00, 0x00, 0x00, 0x1f, 0x00, 0x90, 0xff, 0x41, 0x00, > + 0x00, 0xe2, 0xff, 0x20, 0x00, 0x3e, 0x00, 0x00, 0x00, 0xf0, 0x01, 0x00, > + 0x00, 0x00, 0x1f, 0x00, 0x88, 0xff, 0x83, 0x00, 0x00, 0xc2, 0x9c, 0x40, > + 0x00, 0x3e, 0xf8, 0xff, 0x07, 0xf0, 0x01, 0xf8, 0xff, 0xff, 0x1f, 0x00, > + 0x08, 0x73, 0x02, 0x01, 0x00, 0xf1, 0x3e, 0x41, 0x00, 0x3e, 0xf8, 0xff, > + 0x07, 0xf0, 0x01, 0xf8, 0xff, 0xff, 0x0f, 0x00, 0xc4, 0xfb, 0x04, 0x01, > + 0x00, 0xf1, 0x7f, 0x40, 0x00, 0x3e, 0xf8, 0xff, 0x07, 0xf0, 0x01, 0xf8, > + 0xff, 0xff, 0x07, 0x00, 0xc4, 0xff, 0x01, 0x01, 0x80, 0xf8, 0xff, 0x89, > + 0x00, 0x3e, 0xf8, 0xff, 0x07, 0xf0, 0x01, 0xf8, 0xff, 0xff, 0x0f, 0x00, > + 0xe2, 0xff, 0x27, 0x02, 0x80, 0xf8, 0xff, 0x93, 0x00, 0x3e, 0xf8, 0xff, > + 0x07, 0xf0, 0x01, 0xf8, 0xff, 0xff, 0x0f, 0x00, 0xe2, 0xff, 0x4f, 0x02, > + 0x40, 0xfc, 0xff, 0x93, 0x00, 0x3e, 0x00, 0xc0, 0x07, 0xf0, 0x01, 0x00, > + 0x00, 0x00, 0x1f, 0x00, 0xf1, 0xff, 0x4f, 0x02, 0x40, 0xfc, 0xff, 0x13, > + 0x01, 0x3e, 0x00, 0xc0, 0x07, 0xf0, 0x01, 0x00, 0x00, 0x00, 0x1f, 0x00, > + 0xf1, 0xff, 0x4f, 0x04, 0x20, 0x7c, 0xff, 0x13, 0x01, 0x3e, 0x00, 0xc0, > + 0x07, 0xf0, 0x01, 0x00, 0x00, 0x00, 0x1f, 0x80, 0xf0, 0xfd, 0x4f, 0x04, > + 0x20, 0x7c, 0xff, 0x13, 0x01, 0x3e, 0x00, 0xc0, 0x07, 0xf0, 0x01, 0x00, > + 0x00, 0x00, 0x1f, 0x80, 0xf0, 0xfd, 0x4f, 0x04, 0x20, 0x7c, 0xff, 0x3b, > + 0x01, 0x3e, 0x00, 0xc0, 0x07, 0xf0, 0x01, 0x00, 0x00, 0x00, 0x1f, 0x80, > + 0xf0, 0xfd, 0xef, 0x04, 0xa0, 0xfc, 0xff, 0x00, 0x01, 0x3e, 0x00, 0xc0, > + 0x07, 0xf0, 0x01, 0x00, 0x00, 0x00, 0x1f, 0x80, 0xf2, 0xff, 0x03, 0x04, > + 0xc0, 0xfb, 0x7f, 0x83, 0x00, 0xfe, 0xff, 0xff, 0x87, 0xff, 0x7f, 0xf8, > + 0xff, 0xff, 0x1f, 0x00, 0xef, 0xff, 0x0d, 0x02, 0xe0, 0xe7, 0x7f, 0xc7, > + 0x00, 0xfe, 0xff, 0xff, 0x87, 0xff, 0x7f, 0xf8, 0xff, 0xff, 0x1f, 0x80, > + 0x9f, 0xff, 0x1d, 0x03, 0xf0, 0xc7, 0x7f, 0xff, 0x00, 0xfc, 0xff, 0xff, > + 0x87, 0xff, 0x7f, 0xf8, 0xff, 0xff, 0x0f, 0xc0, 0x1f, 0xff, 0xfd, 0x03, > + 0xf8, 0xcf, 0x7f, 0xff, 0x03, 0xfc, 0xff, 0xff, 0x87, 0xff, 0x7f, 0xf8, > + 0xff, 0xff, 0x0f, 0xe0, 0x3f, 0xff, 0xfd, 0x0f, 0xf8, 0xcf, 0x7f, 0xff, > + 0x03, 0xf0, 0xff, 0xff, 0x87, 0xff, 0x7f, 0xf8, 0xff, 0xff, 0x03, 0xe0, > + 0x3f, 0xff, 0xfd, 0x0f, 0xfc, 0xef, 0x7f, 0xff, 0x07, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xf0, 0xbf, 0xff, 0xfd, 0x1f, > + 0xfc, 0xdf, 0x9f, 0xff, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0xf0, 0x7f, 0x7f, 0xfe, 0x0f, 0xfc, 0x3f, 0x80, 0xff, > + 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xf0, > + 0xff, 0x00, 0xfe, 0x07, 0xf8, 0x3f, 0x80, 0x7f, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xe0, 0xff, 0x00, 0xfe, 0x01, > + 0xc0, 0xdf, 0x7f, 0x3f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x7f, 0xff, 0xfd, 0x00, 0x00, 0x1c, 0x00, 0x1f, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x70, 0x00, 0x7c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }; > + > + > +/* Send the current framebuffer vbitmap as an interrupt message */ > +static int g13_fb_vbitmap_send(struct hid_device *hdev) > +{ > + struct usb_interface *intf; > + struct usb_device *usbdev; > + struct g13_data *data = hid_get_g13data(hdev); > + > + /* Get the usb device to send the image on */ > + intf = to_usb_interface(hdev->dev.parent); > + usbdev = interface_to_usbdev(intf); > + > + return usb_interrupt_msg(usbdev, usb_sndintpipe(usbdev, 0x02), > + data->fb_vbitmap, G13_VBITMAP_SIZE, > + NULL, USB_CTRL_SET_TIMEOUT*2); > +} > + > +/* Update fb_vbitmap from the screen_base and send to the device */ > +static void g13_fb_update(struct g13_data *data) > +{ > + int row; > + int col; > + int bit; > + u8 *u; > + size_t offset; > + u8 temp; > + > + /* Clear the vbitmap and set the necessary magic number */ > + memset(data->fb_vbitmap, 0x00, G13_VBITMAP_SIZE); > + data->fb_vbitmap[0] = 0x03; > + > + /* > + * Translate the XBM format screen_base into the format needed by the > + * G13. This format places the pixels in a vertical rather than > + * horizontal format. Assuming a grid with 0,0 in the upper left corner > + * and 159,42 in the lower right corner, the first byte contains the > + * pixels 0,0 through 0,7 and the second byte contains the pixels 1,0 > + * through 1,7. Within the byte, bit 0 represents 0,0; bit 1 0,1; etc. > + * > + * This loop operates in reverse to shift the lower bits into their > + * respective positions, shifting the lower rows into the higher bits. > + * > + * The offset is calculated for every 8 rows and is adjusted by 32 since > + * that is what the G13 image message expects. > + */ > + for (row = G13FB_HEIGHT-1; row >= 0; row--) { > + offset = 32 + row/8 * G13FB_WIDTH; > + u = data->fb_vbitmap + offset; > + /* > + * Iterate across the screen_base columns to get the > + * individual bits > + */ > + for (col = 0; col < G13FB_LINE_LENGTH; col++) { > + /* > + * We will work with a temporary value since we don't > + * want to modify screen_base as we shift each bit > + * downward. > + */ > + temp = data->fb_bitmap[row * G13FB_LINE_LENGTH + col]; > + > + /* > + * For each bit in the pixel row we will shift it onto > + * the appropriate by by shift the g13 byte up by 1 and > + * simply doing a bitwise or of the low byte > + */ > + for (bit = 0; bit < 8; bit++) { > + /*Shift the g13 byte up by 1 for this new row*/ > + u[bit] <<= 1; > + /* Bring in the new pixel of temp */ > + u[bit] |= (temp & 0x01); > + /* > + * Shift temp down so the low pixel is ready > + * for the next byte > + */ > + temp >>= 1; > + } > + > + /* > + * The last byte represented 8 vertical pixels so we'll > + * jump ahead 8 > + */ > + u += 8; > + } > + } > + > + /* > + * Now that we have translated screen_base into a format expected by > + * the g13 let's send out the vbitmap > + */ > + g13_fb_vbitmap_send(data->hdev); > + > +} > + > +/* Callback from deferred IO workqueue */ > +static void g13_fb_deferred_io(struct fb_info *info, struct list_head *pagelist) > +{ > + g13_fb_update(info->par); > +} > + > +/* Stub to call the system default and update the image on the g13 */ > +static void g13_fb_fillrect(struct fb_info *info, > + const struct fb_fillrect *rect) > +{ > + struct g13_data *par = info->par; > + > + sys_fillrect(info, rect); > + > + g13_fb_update(par); > +} > + > +/* Stub to call the system default and update the image on the g13 */ > +static void g13_fb_copyarea(struct fb_info *info, > + const struct fb_copyarea *area) > +{ > + struct g13_data *par = info->par; > + > + sys_copyarea(info, area); > + > + g13_fb_update(par); > +} > + > +/* Stub to call the system default and update the image on the g13 */ > +static void g13_fb_imageblit(struct fb_info *info, const struct fb_image *image) > +{ > + struct g13_data *par = info->par; > + > + sys_imageblit(info, image); > + > + g13_fb_update(par); > +} > + > +/* > + * this is the slow path from userspace. they can seek and write to > + * the fb. it's inefficient to do anything less than a full screen draw > + */ > +static ssize_t g13_fb_write(struct fb_info *info, const char __user *buf, > + size_t count, loff_t *ppos) > +{ > + struct g13_data *par = info->par; > + unsigned long p = *ppos; > + void *dst; > + int err = 0; > + unsigned long total_size; > + > + if (info->state != FBINFO_STATE_RUNNING) > + return -EPERM; > + > + total_size = info->fix.smem_len; > + > + if (p > total_size) > + return -EFBIG; > + > + if (count > total_size) { > + err = -EFBIG; > + count = total_size; > + } > + > + if (count + p > total_size) { > + if (!err) > + err = -ENOSPC; > + > + count = total_size - p; > + } > + > + dst = (void __force *)(info->screen_base + p); > + > + if (copy_from_user(dst, buf, count)) > + err = -EFAULT; > + > + if (!err) > + *ppos += count; > + > + g13_fb_update(par); > + > + return (err) ? err : count; > +} > + > +static struct fb_ops g13fb_ops = { const? > + .owner = THIS_MODULE, > + .fb_read = fb_sys_read, > + .fb_write = g13_fb_write, > + .fb_fillrect = g13_fb_fillrect, > + .fb_copyarea = g13_fb_copyarea, > + .fb_imageblit = g13_fb_imageblit, > +}; > + > +/* > + * The "fb_node" attribute > + */ > +static ssize_t g13_fb_node_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + unsigned fb_node; > + struct g13_data *data = dev_get_drvdata(dev); > + > + if (data == NULL) > + return -ENODATA; > + > + fb_node = data->fb_info->node; > + > + return sprintf(buf, "%u\n", fb_node); > +} > + > +static DEVICE_ATTR(fb_node, 0444, g13_fb_node_show, NULL); > + > + > +/* > + * The "fb_update_rate" attribute > + */ > +static ssize_t g13_fb_update_rate_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + unsigned fb_update_rate; > + struct g13_data *data = dev_get_drvdata(dev); > + > + if (data == NULL) > + return -ENODATA; > + > + fb_update_rate = data->fb_update_rate; > + > + return sprintf(buf, "%u\n", fb_update_rate); > +} > + > +static ssize_t g13_set_fb_update_rate(struct hid_device *hdev, > + unsigned fb_update_rate) > +{ > + struct g13_data *data = hid_get_g13data(hdev); > + > + if (data == NULL) > + return -ENODATA; > + > + if (fb_update_rate > G13FB_UPDATE_RATE_LIMIT) > + data->fb_update_rate = G13FB_UPDATE_RATE_LIMIT; > + else if (fb_update_rate == 0) > + data->fb_update_rate = 1; > + else > + data->fb_update_rate = fb_update_rate; > + > + data->fb_defio.delay = HZ / data->fb_update_rate; > + > + return 0; > +} > + > +static ssize_t g13_fb_update_rate_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct hid_device *hdev; > + int i; > + unsigned u; > + ssize_t set_result; > + > + /* Get the hid associated with the device */ > + hdev = container_of(dev, struct hid_device, dev); > + > + /* If we have an invalid pointer we'll return ENODATA */ > + if (hdev == NULL || &(hdev->dev) != dev) > + return -ENODATA; > + > + i = sscanf(buf, "%u", &u); > + if (i != 1) { > + printk(KERN_ERR "unrecognized input: %s", buf); > + return -1; > + } > + > + set_result = g13_set_fb_update_rate(hdev, u); > + > + if (set_result < 0) > + return set_result; > + > + return count; > +} > + > +static DEVICE_ATTR(fb_update_rate, 0666, > + g13_fb_update_rate_show, > + g13_fb_update_rate_store); > + > + > +/* > + * The "mled" attribute > + * on or off for each of the four M led's (M1 M2 M3 MR) > + */ > +static ssize_t g13_mled_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + unsigned mled; > + struct g13_data *data = dev_get_drvdata(dev); > + > + if (data == NULL) > + return -ENODATA; > + > + mled = data->mled; > + > + return sprintf(buf, "%u\n", mled); > +} > + > +static ssize_t g13_set_mled(struct hid_device *hdev, unsigned mled) int. It does not return size but error code. > +{ > + struct g13_data *data = hid_get_g13data(hdev); > + > + if (data == NULL || data->mled_report == NULL) > + return -ENODATA; > + > + data->mled_report->field[0]->value[0] = mled&0x0F; > + data->mled_report->field[0]->value[1] = 0x00; > + data->mled_report->field[0]->value[2] = 0x00; > + data->mled_report->field[0]->value[3] = 0x00; > + > + usbhid_submit_report(hdev, data->mled_report, USB_DIR_OUT); > + > + data->mled = mled; > + > + return 0; > +} > + > +static ssize_t g13_mled_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct hid_device *hdev; > + int i; > + unsigned m[4]; > + unsigned mled; > + ssize_t set_result; > + > + /* Get the hid associated with the device */ > + hdev = container_of(dev, struct hid_device, dev); > + > + /* If we have an invalid pointer we'll return ENODATA */ > + if (hdev == NULL || &(hdev->dev) != dev) > + return -ENODATA; > + > + i = sscanf(buf, "%u %u %u %u", m, m+1, m+2, m+3); > + if (!(i == 4 || i == 1)) { > + printk(KERN_ERR "unrecognized input: %s", buf); > + return -1; > + } > + > + if (i == 1) > + mled = m[0]; > + else > + mled = (m[0] ? 1 : 0) | (m[1] ? 2 : 0) | > + (m[2] ? 4 : 0) | (m[3] ? 8 : 0); > + > + set_result = g13_set_mled(hdev, mled); > + > + if (set_result < 0) > + return set_result; > + > + return count; > +} > + > +static DEVICE_ATTR(mled, 0666, g13_mled_show, g13_mled_store); Like other people said, you should be using LED framework - uiserspace can still control LED state, but in a standard way. > + > +static int g13_input_setkeycode(struct input_dev *dev, > + int scancode, > + int keycode) > +{ > + int old_keycode; > + int i; > + struct g13_data *data = input_get_g13data(dev); > + > + if (data == NULL) > + return -EINVAL; > + > + if (scancode >= dev->keycodemax) > + return -EINVAL; > + > + if (!dev->keycodesize) > + return -EINVAL; > + > + if (dev->keycodesize < sizeof(keycode) && > + (keycode >> (dev->keycodesize * 8))) > + return -EINVAL; > + > + write_lock(&data->lock); > + > + old_keycode = data->keycode[scancode]; > + data->keycode[scancode] = keycode; > + > + clear_bit(old_keycode, dev->keybit); > + set_bit(keycode, dev->keybit); > + > + for (i = 0; i < dev->keycodemax; i++) { > + if (data->keycode[i] == old_keycode) { > + set_bit(old_keycode, dev->keybit); > + break; /* Setting the bit twice is useless, so break */ > + } > + } > + > + write_unlock(&data->lock); > + > + return 0; > +} > + > +static int g13_input_getkeycode(struct input_dev *dev, > + int scancode, > + int *keycode) > +{ > + struct g13_data *data = input_get_g13data(dev); > + > + if (!dev->keycodesize) > + return -EINVAL; > + > + if (scancode >= dev->keycodemax) > + return -EINVAL; > + > + read_lock(&data->lock); > + > + *keycode = data->keycode[scancode]; > + > + read_unlock(&data->lock); > + > + return 0; > +} Default input core methods should cover this, no? > + > + > +/* > + * The "keymap" attribute > + */ > +static ssize_t g13_keymap_index_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct g13_data *data = dev_get_drvdata(dev); > + > + if (data == NULL) > + return -ENODATA; > + > + return sprintf(buf, "%u\n", data->curkeymap); > +} > + > +static ssize_t g13_set_keymap_index(struct hid_device *hdev, unsigned k) > +{ > + struct g13_data *data = hid_get_g13data(hdev); > + > + if (data == NULL) > + return -ENODATA; > + > + if (k > 2) > + return -EINVAL; > + > + data->curkeymap = k; What if you change keymap while a key is pressed and new keymap does not have anything in that position? The key may get "stuck" for a bit... > + > + if (data->keymap_switching) > + g13_set_mled(hdev, 1<<k); > + > + return 0; > +} > + > +static ssize_t g13_keymap_index_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct hid_device *hdev; > + int i; > + unsigned k; > + ssize_t set_result; > + > + /* Get the hid associated with the device */ > + hdev = container_of(dev, struct hid_device, dev); > + > + /* If we have an invalid pointer we'll return ENODATA */ > + if (hdev == NULL || &(hdev->dev) != dev) > + return -ENODATA; > + > + i = sscanf(buf, "%u", &k); > + if (i != 1) { > + printk(KERN_ERR "unrecognized input: %s", buf); > + return -1; > + } > + > + set_result = g13_set_keymap_index(hdev, k); > + > + if (set_result < 0) > + return set_result; > + > + return count; > +} > + > +static DEVICE_ATTR(keymap_index, 0666, > + g13_keymap_index_show, > + g13_keymap_index_store); > + > +/* > + * The "keycode" attribute > + */ > +static ssize_t g13_keymap_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + int i; > + int offset = 0; > + int result; > + > + struct g13_data *data = dev_get_drvdata(dev); > + > + if (data == NULL) > + return -ENODATA; > + > + for (i = 0; i < G13_KEYMAP_SIZE; i++) { > + result = sprintf(buf+offset, > + "0x%03x 0x%04x\n", > + i, data->keycode[i]); > + if (result < 0) > + return -EINVAL; > + offset += result; > + } > + > + return offset+1; > +} > + > +static ssize_t g13_keymap_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct hid_device *hdev; > + int scanned; > + int consumed; > + int scancd; > + int keycd; > + int set_result; > + int set = 0; > + int gkey; > + int index; > + int good; > + struct g13_data *data; > + > + /* Get the hid associated with the device */ > + hdev = container_of(dev, struct hid_device, dev); > + > + /* If we have an invalid pointer we'll return ENODATA */ > + if (hdev == NULL || &(hdev->dev) != dev) > + return -ENODATA; > + > + /* Now, let's get the data structure */ > + data = hid_get_g13data(hdev); > + if (data == NULL) > + return -ENODATA; > + > + do { > + good = 0; > + > + /* Look for scancode keycode pair in hex */ > + scanned = sscanf(buf, "%x %x%n", &scancd, &keycd, &consumed); > + if (scanned == 2) { > + buf += consumed; > + set_result = g13_input_setkeycode(data->input_dev, scancd, keycd); > + if (set_result < 0) > + return set_result; > + set++; > + good = 1; > + } else { > + /* > + * Look for Gkey keycode pair and assign to current > + * keymap > + */ > + scanned = sscanf(buf, "G%d %x%n", &gkey, &keycd, &consumed); > + if (scanned == 2 && gkey > 0 && gkey <= G13_KEYS) { > + buf += consumed; > + scancd = data->curkeymap * G13_KEYS + gkey - 1; > + set_result = g13_input_setkeycode(data->input_dev, scancd, keycd); > + if (set_result < 0) > + return set_result; > + set++; > + good = 1; > + } else { > + /* > + * Look for Gkey-index keycode pair and assign > + * to indexed keymap > + */ > + scanned = sscanf(buf, "G%d-%d %x%n", &gkey, &index, &keycd, &consumed); > + if (scanned == 3 && > + gkey > 0 && gkey <= G13_KEYS && > + index >= 0 && index <= 2) { > + buf += consumed; > + scancd = index * G13_KEYS + gkey - 1; > + set_result = g13_input_setkeycode(data->input_dev, scancd, keycd); > + if (set_result < 0) > + return set_result; > + set++; > + good = 1; > + } > + } > + } > + > + } while (good); > + > + if (set == 0) { > + printk(KERN_ERR "unrecognized keycode input: %s", buf); > + return -1; > + } > + > + return count; > +} > + > +static DEVICE_ATTR(keymap, 0666, g13_keymap_show, g13_keymap_store); > No. Just use EVIOCSKEYCODE. > +/* > + * The "emit_msc_raw" attribute > + */ > +static ssize_t g13_emit_msc_raw_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct g13_data *data = dev_get_drvdata(dev); > + > + if (data == NULL) > + return -ENODATA; > + > + return sprintf(buf, "%u\n", data->emit_msc_raw); > +} > + > +static ssize_t g13_set_emit_msc_raw(struct hid_device *hdev, unsigned k) > +{ > + struct g13_data *data = hid_get_g13data(hdev); > + > + if (data == NULL) > + return -ENODATA; > + > + data->emit_msc_raw = k; > + > + return 0; > +} > + > +static ssize_t g13_emit_msc_raw_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct hid_device *hdev; > + int i; > + unsigned k; > + ssize_t set_result; > + > + /* Get the hid associated with the device */ > + hdev = container_of(dev, struct hid_device, dev); > + > + /* If we have an invalid pointer we'll return ENODATA */ > + if (hdev == NULL || &(hdev->dev) != dev) > + return -ENODATA; > + > + i = sscanf(buf, "%u", &k); > + if (i != 1) { > + printk(KERN_ERR "unrecognized input: %s", buf); > + return -1; > + } > + > + set_result = g13_set_emit_msc_raw(hdev, k); > + > + if (set_result < 0) > + return set_result; > + > + return count; > +} > + > +static DEVICE_ATTR(emit_msc_raw, 0666, > + g13_emit_msc_raw_show, > + g13_emit_msc_raw_store); > + I do no see the need for this attribute, simply emit MSC_RAW always. > + > +/* > + * The "keymap_switching" attribute > + */ > +static ssize_t g13_keymap_switching_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct g13_data *data = dev_get_drvdata(dev); > + > + if (data == NULL) > + return -ENODATA; > + > + return sprintf(buf, "%u\n", data->keymap_switching); > +} > + > +static ssize_t g13_set_keymap_switching(struct hid_device *hdev, unsigned k) > +{ > + struct g13_data *data = hid_get_g13data(hdev); > + > + if (data == NULL) > + return -ENODATA; > + > + data->keymap_switching = k; > + > + if (data->keymap_switching) > + g13_set_mled(hdev, 1<<(data->curkeymap)); > + > + return 0; > +} > + > +static ssize_t g13_keymap_switching_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct hid_device *hdev; > + int i; > + unsigned k; > + ssize_t set_result; > + > + /* Get the hid associated with the device */ > + hdev = container_of(dev, struct hid_device, dev); > + > + /* If we have an invalid pointer we'll return ENODATA */ > + if (hdev == NULL || &(hdev->dev) != dev) > + return -ENODATA; > + > + i = sscanf(buf, "%u", &k); > + if (i != 1) { > + printk(KERN_ERR "unrecognized input: %s", buf); > + return -1; > + } > + > + set_result = g13_set_keymap_switching(hdev, k); > + > + if (set_result < 0) > + return set_result; > + > + return count; > +} > + > +static DEVICE_ATTR(keymap_switching, 0666, > + g13_keymap_switching_show, > + g13_keymap_switching_store); Hmm, attributes that are changing device state are usually 0644. > + > + > +static ssize_t g13_name_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct g13_data *data = dev_get_drvdata(dev); > + int result; > + > + if (data == NULL) > + return -ENODATA; > + > + if (data->name == NULL) { > + buf[0] = 0x00; > + return 1; > + } > + > + read_lock(&data->lock); > + result = sprintf(buf, "%s", data->name); > + read_unlock(&data->lock); > + > + return result; > +} > + > +static ssize_t g13_name_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct g13_data *data = dev_get_drvdata(dev); > + size_t limit = count; > + char *end; > + > + if (data == NULL) > + return -ENODATA; > + > + write_lock(&data->lock); > + > + if (data->name != NULL) { > + kfree(data->name); > + data->name = NULL; > + } > + > + end = strpbrk(buf, "\n\r"); > + if (end != NULL) > + limit = end - buf; > + > + if (end != buf) { > + > + if (limit > 100) > + limit = 100; > + > + data->name = kzalloc(limit+1, GFP_KERNEL); > + > + strncpy(data->name, buf, limit); > + } > + > + write_unlock(&data->lock); > + > + return count; > +} > + > +static DEVICE_ATTR(name, 0666, g13_name_show, g13_name_store); What this attribute is for? > + > +/* > + * The "rgb" attribute > + * red green blue > + * each with values 0 - 255 (black - full intensity) > + */ > +static ssize_t g13_rgb_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + unsigned r, g, b; > + struct g13_data *data = dev_get_drvdata(dev); > + > + if (data == NULL) > + return -ENODATA; > + > + r = data->rgb[0]; > + g = data->rgb[1]; > + b = data->rgb[2]; > + > + return sprintf(buf, "%u %u %u\n", r, g, b); > +} > + > +static ssize_t g13_set_rgb(struct hid_device *hdev, > + unsigned r, unsigned g, unsigned b) > +{ > + struct g13_data *data = hid_get_g13data(hdev); > + > + if (data == NULL || data->backlight_report == NULL) > + return -ENODATA; > + > + data->backlight_report->field[0]->value[0] = r; > + data->backlight_report->field[0]->value[1] = g; > + data->backlight_report->field[0]->value[2] = b; > + data->backlight_report->field[0]->value[3] = 0x00; > + > + usbhid_submit_report(hdev, data->backlight_report, USB_DIR_OUT); > + > + data->rgb[0] = r; > + data->rgb[1] = g; > + data->rgb[2] = b; > + > + return 0; > +} > + > +static ssize_t g13_rgb_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct hid_device *hdev; > + int i; > + unsigned r; > + unsigned g; > + unsigned b; > + ssize_t set_result; > + > + /* Get the hid associated with the device */ > + hdev = container_of(dev, struct hid_device, dev); > + > + /* If we have an invalid pointer we'll return ENODATA */ > + if (hdev == NULL || &(hdev->dev) != dev) > + return -ENODATA; > + > + i = sscanf(buf, "%u %u %u", &r, &g, &b); > + if (i != 3) { > + printk(KERN_ERR "unrecognized input: %s", buf); > + return -1; > + } > + > + set_result = g13_set_rgb(hdev, r, g, b); > + > + if (set_result < 0) > + return set_result; > + > + return count; > +} > + > +static DEVICE_ATTR(rgb, 0666, g13_rgb_show, g13_rgb_store); > + > +/* > + * Create a group of attributes so that we can create and destroy them all > + * at once. > + */ > +static struct attribute *g13_attrs[] = { > + &dev_attr_name.attr, > + &dev_attr_rgb.attr, > + &dev_attr_mled.attr, > + &dev_attr_keymap_index.attr, > + &dev_attr_emit_msc_raw.attr, > + &dev_attr_keymap_switching.attr, > + &dev_attr_keymap.attr, > + &dev_attr_fb_update_rate.attr, > + &dev_attr_fb_node.attr, > + NULL, /* need to NULL terminate the list of attributes */ > +}; > + > +/* > + * An unnamed attribute group will put all of the attributes directly in > + * the kobject directory. If we specify a name, a subdirectory will be > + * created for the attributes with the directory being the name of the > + * attribute group. > + */ > +static struct attribute_group g13_attr_group = { > + .attrs = g13_attrs, > +}; > + > +static struct fb_deferred_io g13_fb_defio = { > + .delay = HZ / G13FB_UPDATE_RATE_DEFAULT, > + .deferred_io = g13_fb_deferred_io, > +}; > + > +static void g13_raw_event_process_input(struct hid_device *hdev, > + struct g13_data *g13data, > + u8 *raw_data) > +{ > + struct input_dev *idev = g13data->input_dev; > + unsigned int code; > + int value; > + int i; > + int mask; > + int offset; > + u8 val; > + > + g13data->ready2 = 1; > + > + if (idev == NULL) > + return; > + > + if (g13data->curkeymap < 3) > + offset = G13_KEYS * g13data->curkeymap; > + else > + offset = 0; > + > + for (i = 0, mask = 0x01; i < 8; i++, mask <<= 1) { > + /* Keys G1 through G8 */ > + code = g13data->keycode[i+offset]; > + value = raw_data[3] & mask; > + if (g13data->keycode[i] != KEY_RESERVED) > + input_report_key(idev, code, value); > + input_event(idev, EV_MSC, MSC_SCAN, i); That means these MSC_SCAN events are emitted _always_. Not sure if that is too useful. If you were to detect the state change and emit MSC_SCAN for changed keys, that would be better. > + > + /* Keys G9 through G16 */ > + code = g13data->keycode[i+8+offset]; > + value = raw_data[4] & mask; > + if (g13data->keycode[i+8] != KEY_RESERVED) > + input_report_key(idev, code, value); > + input_event(idev, EV_MSC, MSC_SCAN, i+8); > + > + /* Keys G17 through G22 */ > + code = g13data->keycode[i+16+offset]; > + value = raw_data[5] & mask; > + if (i <= 5 && g13data->keycode[i+16] != KEY_RESERVED) > + input_report_key(idev, code, value); > + input_event(idev, EV_MSC, MSC_SCAN, i+16); > + > + /* Keys FUNC through M3 */ > + code = g13data->keycode[i+22+offset]; > + value = raw_data[6] & mask; > + if (g13data->keycode[i+22] != KEY_RESERVED) > + input_report_key(idev, code, value); > + input_event(idev, EV_MSC, MSC_SCAN, i+22); > + > + /* Keys MR through LIGHT */ > + code = g13data->keycode[i+30+offset]; > + value = raw_data[7] & mask; > + if (i <= 4 && g13data->keycode[i+30] != KEY_RESERVED) > + input_report_key(idev, code, value); > + input_event(idev, EV_MSC, MSC_SCAN, i+30); > + } > + > + if (g13data->emit_msc_raw) { > + /* > + * Outputs an MSC_RAW value with the low four > + * bits = M1-MR, Low bit = M1 > + */ > + val = raw_data[6] >> 5; > + val |= (raw_data[7] & 0x01 << 3); > + input_event(idev, EV_MSC, MSC_RAW, val); > + } > + > + if (g13data->keymap_switching) { > + if (raw_data[6] & 0x20) { > + g13data->curkeymap = 0; > + g13_set_mled(hdev, 0x01); > + } else if (raw_data[6] & 0x40) { > + g13data->curkeymap = 1; > + g13_set_mled(hdev, 0x02); > + } else if (raw_data[6] & 0x80) { > + g13data->curkeymap = 2; > + g13_set_mled(hdev, 0x04); > + } > + } > + > + input_report_abs(idev, ABS_X, raw_data[1]); > + input_report_abs(idev, ABS_Y, raw_data[2]); > + input_sync(idev); > +} > + > +static int g13_raw_event(struct hid_device *hdev, > + struct hid_report *report, > + u8 *raw_data, int size) > +{ > + /* > + * On initialization receive a 258 byte message with > + * data = 6 0 255 255 255 255 255 255 255 255 ... > + */ > + struct g13_data *g13data; > + g13data = dev_get_drvdata(&hdev->dev); > + > + if (g13data == NULL) > + return 1; > + > + switch (report->id) { > + case 6: > + g13data->ready = 1; > + break; > + case 1: > + g13_raw_event_process_input(hdev, g13data, raw_data); > + break; > + default: > + return 0; > + } > + > + return 1; > +} > + > +static void g13_initialize_keymap(struct g13_data *data) > +{ > + int i; > + > + write_lock(&data->lock); Why do you take this lock? What else could be accessing the structure at this point? > + > + for (i = 0; i < G13_KEYS; i++) { > + data->keycode[i] = g13_default_key_map[i]; > + set_bit(data->keycode[i], data->input_dev->keybit); > + } > + > + clear_bit(0, data->input_dev->keybit); Use KEY_RESERVED instead of 0 so readers know what is going on. Also no need to use locked variants, __set_bit and __clear_bit should suffice. > + > + write_unlock(&data->lock); > + > +} > + > +static int g13_probe(struct hid_device *hdev, > + const struct hid_device_id *id) > +{ > + int error; > + struct g13_data *data; > + int i; > + struct list_head *feature_report_list = > + &hdev->report_enum[HID_FEATURE_REPORT].report_list; > + struct hid_report *report; > + > + dev_dbg(&hdev->dev, "Logitech G13 HID hardware probe..."); > + > + /* > + * Let's allocate the g13 data structure, set some reasonable > + * defaults, and associate it with the device > + */ > + data = kzalloc(sizeof(struct g13_data), GFP_KERNEL); > + if (data == NULL) { > + dev_err(&hdev->dev, "can't allocate space for Logitech G13 device attributes\n"); > + error = -ENOMEM; > + goto err_no_cleanup; > + } > + > + rwlock_init(&data->lock); rwlock is usually slower than spinlock... There is a handful of places where use of rwlock is warranted. > + > + data->hdev = hdev; > + > + data->fb_bitmap = vmalloc(G13FB_SIZE); > + if (data->fb_bitmap == NULL) { > + dev_err(&hdev->dev, G13_NAME ": ERROR: can't get a free page for framebuffer\n"); > + error = -ENOMEM; > + goto err_cleanup_data; > + } > + memcpy(data->fb_bitmap, g13_lcd_bits, G13FB_SIZE); > + > + data->fb_vbitmap = kmalloc(sizeof(u8) * G13_VBITMAP_SIZE, GFP_KERNEL); > + if (data->fb_vbitmap == NULL) { > + dev_err(&hdev->dev, G13_NAME ": ERROR: can't alloc vbitmap image buffer\n"); > + error = -ENOMEM; > + goto err_cleanup_fb_bitmap; > + } > + > + hid_set_drvdata(hdev, data); > + > + dbg_hid("Preparing to parse " G13_NAME " hid reports\n"); > + > + /* Parse the device reports and start it up */ > + error = hid_parse(hdev); > + if (error) { > + dev_err(&hdev->dev, G13_NAME " device report parse failed\n"); > + error = -EINVAL; > + goto err_cleanup_fb_vbitmap; > + } > + > + mdelay(10); Why is this needed? > + > + error = hid_hw_start(hdev, HID_CONNECT_DEFAULT | HID_CONNECT_HIDINPUT_FORCE); > + if (error) { > + dev_err(&hdev->dev, G13_NAME " hardware start failed\n"); > + error = -EINVAL; > + goto err_cleanup_fb_vbitmap; > + } > + > + dbg_hid(G13_NAME " claimed: %d\n", hdev->claimed); > + > + error = hdev->ll_driver->open(hdev); > + if (error) { > + dev_err(&hdev->dev, G13_NAME " failed to open input interrupt pipe for key and joystick events\n"); > + error = -EINVAL; > + goto err_cleanup_fb_vbitmap; > + } > + > + /* Set up the input device for the key I/O */ > + data->input_dev = input_allocate_device(); > + if (data->input_dev == NULL) { > + dev_err(&hdev->dev, G13_NAME " error initializing the input device"); > + error = -ENOMEM; > + goto err_cleanup_fb_vbitmap; > + } > + > + input_set_drvdata(data->input_dev, hdev); > + > + data->input_dev->name = G13_NAME; > + data->input_dev->phys = hdev->phys; > + data->input_dev->uniq = hdev->uniq; > + data->input_dev->id.bustype = hdev->bus; > + data->input_dev->id.vendor = hdev->vendor; > + data->input_dev->id.product = hdev->product; > + data->input_dev->id.version = hdev->version; > + data->input_dev->dev.parent = hdev->dev.parent; > + data->input_dev->keycode = data->keycode; > + data->input_dev->keycodemax = G13_KEYMAP_SIZE; > + data->input_dev->keycodesize = sizeof(u32); > + data->input_dev->setkeycode = g13_input_setkeycode; > + data->input_dev->getkeycode = g13_input_getkeycode; > + > + input_set_capability(data->input_dev, EV_ABS, ABS_X); > + input_set_capability(data->input_dev, EV_ABS, ABS_Y); > + input_set_capability(data->input_dev, EV_MSC, MSC_SCAN); > + input_set_capability(data->input_dev, EV_KEY, KEY_UNKNOWN); > + data->input_dev->evbit[0] |= BIT_MASK(EV_REP); > + > + /* 4 center values */ > + input_set_abs_params(data->input_dev, ABS_X, 0, 0xff, 0, 4); > + input_set_abs_params(data->input_dev, ABS_Y, 0, 0xff, 0, 4); > + > + g13_initialize_keymap(data); > + > + error = input_register_device(data->input_dev); > + if (error) { > + dev_err(&hdev->dev, G13_NAME " error registering the input device"); > + error = -EINVAL; > + goto err_cleanup_input_dev; > + } > + > + /* Set up the framebuffer device */ > + data->fb_update_rate = G13FB_UPDATE_RATE_DEFAULT; > + data->fb_info = framebuffer_alloc(0, &hdev->dev); > + if (data->fb_info == NULL) { > + dev_err(&hdev->dev, G13_NAME " failed to allocate a framebuffer\n"); > + goto err_cleanup_input_dev; > + } > + > + dbg_hid(KERN_INFO G13_NAME " allocated framebuffer\n"); > + > + data->fb_defio = g13_fb_defio; > + data->fb_info->fbdefio = &data->fb_defio; > + > + dbg_hid(KERN_INFO G13_NAME " allocated deferred IO structure\n"); > + > + data->fb_info->screen_base = (char __force __iomem *) data->fb_bitmap; > + data->fb_info->fbops = &g13fb_ops; > + data->fb_info->var = g13fb_var; > + data->fb_info->fix = g13fb_fix; > + data->fb_info->fix.smem_len = G13FB_SIZE; > + data->fb_info->par = data; > + data->fb_info->flags = FBINFO_FLAG_DEFAULT; > + > + fb_deferred_io_init(data->fb_info); > + > + if (register_framebuffer(data->fb_info) < 0) > + goto err_cleanup_fb; > + > + /* Add the sysfs attributes */ > + error = sysfs_create_group(&(hdev->dev.kobj), &g13_attr_group); > + if (error) { > + dev_err(&hdev->dev, "Logitech G13 failed to create sysfs group attributes\n"); > + goto err_cleanup_fb; > + } > + > + dbg_hid("Waiting for G13 to activate\n"); > + > + if (list_empty(feature_report_list)) { > + dev_err(&hdev->dev, "no feature report found\n"); > + error = -ENODEV; > + goto err_cleanup_fb; > + } > + dbg_hid("G13 feature report found\n"); > + > + list_for_each_entry(report, feature_report_list, list) { > + switch (report->id) { > + case 0x04: > + data->report_4 = report; > + break; > + case 0x05: > + data->mled_report = report; > + break; > + case 0x06: > + data->start_input_report = report; > + break; > + case 0x07: > + data->backlight_report = report; > + break; > + default: > + break; > + } > + dbg_hid("G13 Feature report: id=%u type=%u size=%u maxfield=%u report_count=%u\n", > + report->id, report->type, report->size, > + report->maxfield, report->field[0]->report_count); > + } > + > + dbg_hid("Found all reports\n"); > + > + for (i = 0; i < 20; i++) { > + if (data->ready && data->ready2) > + break; > + mdelay(10); > + } Consider using completion? > + > + if (!(data->ready && data->ready2)) > + printk(KERN_ERR "G13 hasn't responded yet, forging ahead with initialization\n"); > + else > + dbg_hid("G13 initialized\n"); > + > + /* > + * Set the initial color and load the linux logo > + * We're going to ignore the error values. If there is an error at this > + * point we'll forge ahead. > + */ > + > + dbg_hid("Set default color\n"); > + > + error = g13_set_rgb(hdev, G13_DEFAULT_RED, G13_DEFAULT_GREEN, G13_DEFAULT_BLUE); And...? > + > + usbhid_submit_report(hdev, data->start_input_report, USB_DIR_IN); > + > + g13_fb_update(data); > + > + dbg_hid("G13 activated and initialized\n"); > + > + /* Everything went well */ > + return 0; > + > +err_cleanup_fb: > + framebuffer_release(data->fb_info); input device was registered here so you need to unregister it (and suppress call to input_free_device below). > + > +err_cleanup_input_dev: > + input_free_device(data->input_dev); > + > +err_cleanup_fb_vbitmap: > + kfree(data->fb_vbitmap); > + > +err_cleanup_fb_bitmap: > + vfree(data->fb_bitmap); > + > +err_cleanup_data: > + /* Make sure we clean up the allocated data structure */ > + kfree(data); > + > +err_no_cleanup: > + > + hid_set_drvdata(hdev, NULL); > + I do not see sysfs group being destroyed. > + return error; > +} > + > +static void g13_remove(struct hid_device *hdev) > +{ > + struct g13_data *data; > + > + hdev->ll_driver->close(hdev); > + > + hid_hw_stop(hdev); > + > + /* Get the internal g13 data buffer */ > + data = hid_get_drvdata(hdev); > + > + /* Clean up the buffer */ > + if (data != NULL) { Can it ever be NULL? > + write_lock(&data->lock); Why? > + input_unregister_device(data->input_dev); > + kfree(data->name); > + write_unlock(&data->lock); > + if (data->fb_info != NULL) { > + fb_deferred_io_cleanup(data->fb_info); > + unregister_framebuffer(data->fb_info); > + framebuffer_release(data->fb_info); > + } > + vfree(data->fb_bitmap); > + kfree(data->fb_vbitmap); > + kfree(data); > + } > + I do not see sysfs group being destroyed. > +} > + > +static const struct hid_device_id g13_devices[] = { > + { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_G13) > + }, > + { } > +}; > +MODULE_DEVICE_TABLE(hid, g13_devices); > + > +static struct hid_driver g13_driver = { > + .name = "hid-g13", > + .id_table = g13_devices, > + .probe = g13_probe, > + .remove = g13_remove, > + .raw_event = g13_raw_event, > +}; > + > +static int __init g13_init(void) > +{ > + return hid_register_driver(&g13_driver); > +} > + > +static void __exit g13_exit(void) > +{ > + hid_unregister_driver(&g13_driver); > +} > + > +module_init(g13_init); > +module_exit(g13_exit); > +MODULE_DESCRIPTION("Logitech G13 HID Driver"); > +MODULE_AUTHOR("Rick L Vinyard Jr (rvinyard(a)cs.nmsu.edu)"); > +MODULE_LICENSE("GPL"); > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h > index 3839340..f3e27d3 100644 > --- a/drivers/hid/hid-ids.h > +++ b/drivers/hid/hid-ids.h > @@ -295,6 +295,7 @@ > #define USB_DEVICE_ID_LOGITECH_EXTREME_3D 0xc215 > #define USB_DEVICE_ID_LOGITECH_RUMBLEPAD2 0xc218 > #define USB_DEVICE_ID_LOGITECH_RUMBLEPAD2_2 0xc219 > +#define USB_DEVICE_ID_LOGITECH_G13 0xc21c > #define USB_DEVICE_ID_LOGITECH_WINGMAN_F3D 0xc283 > #define USB_DEVICE_ID_LOGITECH_FORCE3D_PRO 0xc286 > #define USB_DEVICE_ID_LOGITECH_WHEEL 0xc294 Thanks. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |