* [PATCH v2 0/6] hid: new driver for PicoLCD device
@ 2010-03-20 16:00 Bruno Prémont
[not found] ` <20100320170014.440959a8-hY15tx4IgV39zxVx7UNMDg@public.gmane.org>
` (4 more replies)
0 siblings, 5 replies; 20+ messages in thread
From: Bruno Prémont @ 2010-03-20 16:00 UTC (permalink / raw)
To: Jiri Kosina
Cc: linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rick L. Vinyard Jr.,
Nicu Pavel, Oliver Neukum, Jaya Kumar
This series adds support for PicoLCD USB HID device adding support for
the various features in different patches so each patch concentrates
on a device class.
I implemented the patches so input support is required (this should
make it easy to later on add support for other PicoLCD device that
don't have the same feature set but share compatible HID reports.
This causes presence of a few #ifdef blocks to include support for the
different feature sets when their matching device class has been
selected.
Though to minimize the amount of such #ifs I put all code for a single
class together and defined a few stubs in the #else part so global
device initialization is not filled with #ifs.
I'm not sure which of the following approaches is better (I took the
first one):
- Check for built-in or build-as-module class support
with #if defined():
#if defined(CONFIG_..CLASS) || defined(CONFIG_..CLASS_MODULE)
- Add extra CONFIG_PICOLCD_$CLASS to Kconfig and let Kconfig get
things correctly set having just simple
#ifdef CONFIG_PICOLCD_$CLASS in the code.
The series depends on my previous patch adding HID suspend support
(I've not yet looked at improving it). The patch adding support
for backlight class depends on backlight state as of 2.6.34-rc2.
All the rest should apply against 2.6.33 (unless I did oversee some
detail).
Bruno
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 1/6] hid: new driver for PicoLCD device
[not found] ` <20100320170014.440959a8-hY15tx4IgV39zxVx7UNMDg@public.gmane.org>
@ 2010-03-20 16:02 ` Bruno Prémont
2010-03-21 3:46 ` Dmitry Torokhov
2010-03-20 16:04 ` [PATCH v2 2/6] hid: add framebuffer support to " Bruno Prémont
1 sibling, 1 reply; 20+ messages in thread
From: Bruno Prémont @ 2010-03-20 16:02 UTC (permalink / raw)
To: Jiri Kosina
Cc: linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rick L. Vinyard Jr.,
Nicu Pavel, Oliver Neukum, Jaya Kumar
Add basic driver for PicoLCD graphics device.
Initially support keypad with input device, operation mode switching
and provide support for debugging communication via events file from
debugfs.
Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
---
Documentation/ABI/testing/sysfs-driver-hid-picolcd | 17 +
drivers/hid/Kconfig | 18 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-core.c | 2 +
drivers/hid/hid-ids.h | 2 +
drivers/hid/hid-picolcd.c | 1244 ++++++++++++++++++++
6 files changed, 1284 insertions(+), 0 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-driver-hid-picolcd b/Documentation/ABI/testing/sysfs-driver-hid-picolcd
new file mode 100644
index 0000000..1c63a0f
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-hid-picolcd
@@ -0,0 +1,17 @@
+What: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/operation_mode
+Date: March 2010
+Contact: Bruno Prémont <bonbons@linux-vserver.org>
+Description: Make it possible to switch the PicoLCD device between LCD
+ (firmware) and bootloader (flasher) operation modes.
+
+ Reading: returns list of available modes is show, the active mode
+ being enclosed in brackets ('<' and '>')
+
+ Writing: causes operation mode switch. Permitted values are
+ the non-active mode names listed when read, optionally followed
+ by a delay value expressed in ms.
+
+ Note: when switching mode the current PicoLCD HID device gets
+ disconnected and reconnects after above delay (default value
+ is 5 seconds though this default should not be depended on).
+
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 24d90ea..7097f0a 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -227,6 +227,24 @@ config HID_PETALYNX
---help---
Support for Petalynx Maxter remote control.
+config HID_PICOLCD
+ tristate "PicoLCD (graphic version)"
+ depends on USB_HID
+ ---help---
+ This provides support for Minibox PicoLCD devices, currently
+ only the graphical ones are supported.
+
+ This includes support for the following device features:
+ - Keypad
+ - Switching between Firmware and Flash mode
+ Features that are not (yet) supported:
+ - Framebuffer for monochrome 256x64 display
+ - Backlight control
+ - Contrast control
+ - IR
+ - General purpose outputs
+ - EEProm / Flash access
+
config HID_SAMSUNG
tristate "Samsung" if EMBEDDED
depends on USB_HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 0de2dff..f0a4870 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -36,6 +36,7 @@ obj-$(CONFIG_HID_MONTEREY) += hid-monterey.o
obj-$(CONFIG_HID_NTRIG) += hid-ntrig.o
obj-$(CONFIG_HID_PANTHERLORD) += hid-pl.o
obj-$(CONFIG_HID_PETALYNX) += hid-petalynx.o
+obj-$(CONFIG_HID_PICOLCD) += hid-picolcd.o
obj-$(CONFIG_HID_SAMSUNG) += hid-samsung.o
obj-$(CONFIG_HID_SMARTJOYPLUS) += hid-sjoy.o
obj-$(CONFIG_HID_SONY) += hid-sony.o
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index a50d850..2bd0671 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1330,6 +1330,8 @@ static const struct hid_device_id hid_blacklist[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RUMBLEPAD2) },
{ 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_MICROCHIP, USB_DEVICE_ID_PICOLCD) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_MICROCHIP, USB_DEVICE_ID_PICOLCD_BOOTLOADER) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_SIDEWINDER_GV) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_NE4K) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_LK6K) },
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 010368e..6d67582 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -330,6 +330,8 @@
#define USB_VENDOR_ID_MICROCHIP 0x04d8
#define USB_DEVICE_ID_PICKIT1 0x0032
#define USB_DEVICE_ID_PICKIT2 0x0033
+#define USB_DEVICE_ID_PICOLCD 0xc002
+#define USB_DEVICE_ID_PICOLCD_BOOTLOADER 0xf002
#define USB_VENDOR_ID_MICROSOFT 0x045e
#define USB_DEVICE_ID_SIDEWINDER_GV 0x003b
diff --git a/drivers/hid/hid-picolcd.c b/drivers/hid/hid-picolcd.c
new file mode 100644
index 0000000..30b8e08
--- /dev/null
+++ b/drivers/hid/hid-picolcd.c
@@ -0,0 +1,1244 @@
+/***************************************************************************
+ * Copyright (C) 2010 by Bruno Prémont <bonbons@linux-vserver.org> *
+ * *
+ * Based on Logitech G13 driver (v0.4) *
+ * Copyright (C) 2009 by Rick L. Vinyard, Jr. <rvinyard@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, version 2 of the License. *
+ * *
+ * 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/hid.h>
+#include <linux/hid-debug.h>
+#include <linux/input.h>
+#include "hid-ids.h"
+#include "usbhid/usbhid.h"
+#include <linux/usb.h>
+
+#include <linux/seq_file.h>
+#include <linux/debugfs.h>
+
+#include <linux/completion.h>
+
+#define PICOLCD_NAME "PicoLCD (graphic)"
+
+/* Report numbers */
+#define REPORT_ERROR_CODE 0x10 /* LCD: IN[16] */
+#define ERR_SUCCESS 0x00
+#define ERR_PARAMETER_MISSING 0x01
+#define ERR_DATA_MISSING 0x02
+#define ERR_BLOCK_READ_ONLY 0x03
+#define ERR_BLOCK_NOT_ERASABLE 0x04
+#define ERR_BLOCK_TOO_BIG 0x05
+#define ERR_SECTION_OVERFLOW 0x06
+#define ERR_INVALID_CMD_LEN 0x07
+#define ERR_INVALID_DATA_LEN 0x08
+#define REPORT_KEY_STATE 0x11 /* LCD: IN[2] */
+#define REPORT_IR_DATA 0x21 /* LCD: IN[63] */
+#define REPORT_EE_DATA 0x32 /* LCD: IN[63] */
+#define REPORT_MEMORY 0x41 /* LCD: IN[63] */
+#define REPORT_LED_STATE 0x81 /* LCD: OUT[1] */
+#define REPORT_BRIGHTNESS 0x91 /* LCD: OUT[1] */
+#define REPORT_CONTRAST 0x92 /* LCD: OUT[1] */
+#define REPORT_RESET 0x93 /* LCD: OUT[2] */
+#define REPORT_LCD_CMD 0x94 /* LCD: OUT[63] */
+#define REPORT_LCD_DATA 0x95 /* LCD: OUT[63] */
+#define REPORT_LCD_CMD_DATA 0x96 /* LCD: OUT[63] */
+#define REPORT_EE_READ 0xa3 /* LCD: OUT[63] */
+#define REPORT_EE_WRITE 0xa4 /* LCD: OUT[63] */
+#define REPORT_ERASE_MEMORY 0xb2 /* LCD: OUT[2] */
+#define REPORT_READ_MEMORY 0xb3 /* LCD: OUT[3] */
+#define REPORT_WRITE_MEMORY 0xb4 /* LCD: OUT[63] */
+#define REPORT_SPLASH_RESTART 0xc1 /* LCD: OUT[1] */
+#define REPORT_EXIT_KEYBOARD 0xef /* LCD: OUT[2] */
+#define REPORT_VERSION 0xf1 /* LCD: IN[2],OUT[1] Bootloader: IN[2],OUT[1] */
+#define REPORT_BL_ERASE_MEMORY 0xf2 /* Bootloader: IN[36],OUT[4] */
+#define REPORT_BL_READ_MEMORY 0xf3 /* Bootloader: IN[36],OUT[4] */
+#define REPORT_BL_WRITE_MEMORY 0xf4 /* Bootloader: IN[36],OUT[36] */
+#define REPORT_DEVID 0xf5 /* LCD: IN[5], OUT[1] Bootloader: IN[5],OUT[1] */
+#define REPORT_SPLASH_SIZE 0xf6 /* LCD: IN[4], OUT[1] */
+#define REPORT_HOOK_VERSION 0xf7 /* LCD: IN[2], OUT[1] */
+#define REPORT_EXIT_FLASHER 0xff /* Bootloader: OUT[2] */
+
+/* Input device
+ *
+ * The PicoLCD has an IR receiver header, a built-in keypad with 5 keys
+ * and header for 4x4 key matrix. The built-in keys are part of the matrix.
+ */
+#define PICOLCD_KEYS 17
+
+static const int def_keymap[PICOLCD_KEYS] = {
+ KEY_RESERVED, /* none */
+ KEY_BACK, /* col 4 + row 1 */
+ KEY_HOMEPAGE, /* col 3 + row 1 */
+ KEY_RESERVED, /* col 2 + row 1 */
+ KEY_RESERVED, /* col 1 + row 1 */
+ KEY_SCROLLUP, /* col 4 + row 2 */
+ KEY_OK, /* col 3 + row 2 */
+ KEY_SCROLLDOWN, /* col 2 + row 2 */
+ KEY_RESERVED, /* col 1 + row 2 */
+ KEY_RESERVED, /* col 4 + row 3 */
+ KEY_RESERVED, /* col 3 + row 3 */
+ KEY_RESERVED, /* col 2 + row 3 */
+ KEY_RESERVED, /* col 1 + row 3 */
+ KEY_RESERVED, /* col 4 + row 4 */
+ KEY_RESERVED, /* col 3 + row 4 */
+ KEY_RESERVED, /* col 2 + row 4 */
+ KEY_RESERVED, /* col 1 + row 4 */
+};
+
+/* Description of in-progress IO operation, used for operations
+ * that trigger response from device */
+struct picolcd_pending {
+ struct hid_report *out_report;
+ struct hid_report *in_report;
+ int raw_size;
+ u8 raw_data[64];
+};
+
+/* Per device data structure */
+struct picolcd_data {
+ struct hid_device *hdev;
+#ifdef CONFIG_DEBUG_FS
+ int addr_sz;
+#endif
+ u8 version[2];
+ /* input stuff */
+ u8 pressed_keys[2];
+ struct input_dev *input_keys;
+ struct input_dev *input_cir;
+ int keycode[PICOLCD_KEYS];
+
+ /* Housekeeping stuff */
+ spinlock_t lock;
+ struct picolcd_pending *pending;
+ struct completion ready;
+ int status;
+#define PICOLCD_BUSY 1
+#define PICOLCD_FAILED 4
+#define PICOLCD_BOOTLOADER 8
+};
+
+
+/* Find a given report */
+#define picolcd_in_report(id, dev) picolcd_report(id, dev, HID_INPUT_REPORT)
+#define picolcd_out_report(id, dev) picolcd_report(id, dev, HID_OUTPUT_REPORT)
+
+static struct hid_report *picolcd_report(int id, struct hid_device *hdev, int dir)
+{
+ struct list_head *feature_report_list = &hdev->report_enum[dir].report_list;
+ struct hid_report *report = NULL;
+
+ list_for_each_entry(report, feature_report_list, list) {
+ if (report->id = id)
+ return report;
+ }
+ dev_warn(&hdev->dev, "No report with id 0x%x found\n", id);
+ return NULL;
+}
+
+#ifdef CONFIG_DEBUG_FS
+static void picolcd_debug_out_report(struct picolcd_data *data,
+ struct hid_device *hdev, struct hid_report *report);
+#define usbhid_submit_report(a, b, c) \
+ picolcd_debug_out_report(hid_get_drvdata(a), a, b); \
+ usbhid_submit_report(a, b, c)
+#endif
+
+/* Submit a report and wait for a reply from device - if device fades away
+ * or does not respond in time, return NULL */
+static struct picolcd_pending* picolcd_send_and_wait(struct hid_device *hdev,
+ int report_id, const u8 *raw_data, int size)
+{
+ struct picolcd_data *data = hid_get_drvdata(hdev);
+ struct picolcd_pending *work;
+ struct hid_report *report = picolcd_out_report(report_id, hdev);
+ unsigned long flags;
+ int status, i, j, k;
+
+ if (!report)
+ return NULL;
+ work = kzalloc(sizeof(*work), GFP_KERNEL);
+ if (!work)
+ return NULL;
+
+ work->out_report = report;
+ work->in_report = NULL;
+ work->raw_size = 0;
+
+retry:
+ /* try to get lock and enqueue our job */
+ spin_lock_irqsave(&data->lock, flags);
+ status = data->status;
+ if (data->pending || (status & PICOLCD_FAILED)) {
+ /* some job already pending,
+ * wait for it to complete and try again */
+ spin_unlock_irqrestore(&data->lock, flags);
+ if (status & PICOLCD_FAILED) {
+ kfree(work);
+ return NULL;
+ }
+ wait_for_completion_interruptible_timeout(&data->ready, HZ*2);
+ goto retry;
+ }
+ data->status |= PICOLCD_BUSY;
+ data->pending = work;
+ for (i = k = 0; i < report->maxfield; i++)
+ for (j = 0; j < report->field[i]->report_count; j++) {
+ hid_set_field(report->field[i], j, k < size ? raw_data[k] : 0);
+ k++;
+ }
+ usbhid_submit_report(data->hdev, report, USB_DIR_OUT);
+ complete_all(&data->ready);
+ INIT_COMPLETION(data->ready);
+ spin_unlock_irqrestore(&data->lock, flags);
+ /* wait for our job to complete */
+ wait_for_completion_interruptible_timeout(&data->ready, HZ*2);
+
+ spin_lock_irqsave(&data->lock, flags);
+ if (data->pending = work) {
+ data->status &= ~PICOLCD_BUSY;
+ data->pending = NULL;
+ complete_all(&data->ready);
+ spin_unlock_irqrestore(&data->lock, flags);
+ return work;
+ } else {
+ spin_unlock_irqrestore(&data->lock, flags);
+ kfree(work);
+ return NULL;
+ }
+}
+
+/*
+ * input class device
+ */
+static int picolcd_raw_keypad(struct hid_device *hdev,
+ struct hid_report *report, u8 *raw_data, int size)
+{
+ /*
+ * Keypad event
+ * First and second data bytes list currently pressed keys,
+ * 0x00 means no key and at most 2 keys may be pressed at same time
+ */
+ struct picolcd_data *data = hid_get_drvdata(hdev);
+ int i, j;
+
+ /* determine newly pressed keys */
+ for (i = 0; i < size; i++) {
+ int key_code;
+ if (raw_data[i] = 0)
+ continue;
+ for (j = 0; j < sizeof(data->pressed_keys); j++)
+ if (data->pressed_keys[j] = raw_data[i])
+ goto key_already_down;
+ for (j = 0; j < sizeof(data->pressed_keys); j++)
+ if (data->pressed_keys[j] = 0) {
+ data->pressed_keys[j] = raw_data[i];
+ break;
+ }
+ input_event(data->input_keys, EV_MSC, MSC_SCAN, raw_data[i]);
+ if (input_get_keycode(data->input_keys, raw_data[i], &key_code))
+ key_code = KEY_UNKNOWN;
+ if (key_code != KEY_UNKNOWN) {
+ dbg_hid(PICOLCD_NAME " got key press for %u:%d", raw_data[i], key_code);
+ input_report_key(data->input_keys, key_code, 1);
+ }
+ input_sync(data->input_keys);
+key_already_down:
+ continue;
+ }
+
+ /* determine newly released keys */
+ for (j = 0; j < sizeof(data->pressed_keys); j++) {
+ int key_code;
+ if (data->pressed_keys[j] = 0)
+ continue;
+ for (i = 0; i < size; i++)
+ if (data->pressed_keys[j] = raw_data[i])
+ goto key_still_down;
+ input_event(data->input_keys, EV_MSC, MSC_SCAN, data->pressed_keys[j]);
+ if (input_get_keycode(data->input_keys, data->pressed_keys[j], &key_code))
+ key_code = KEY_UNKNOWN;
+ if (key_code != KEY_UNKNOWN) {
+ dbg_hid(PICOLCD_NAME " got key release for %u:%d", data->pressed_keys[j], key_code);
+ input_report_key(data->input_keys, key_code, 0);
+ }
+ input_sync(data->input_keys);
+ data->pressed_keys[j] = 0;
+key_still_down:
+ continue;
+ }
+ return 1;
+}
+
+static int picolcd_raw_cir(struct hid_device *hdev,
+ struct hid_report *report, u8 *raw_data, int size)
+{
+ /* Need understanding of CIR data format to implement ... */
+ return 1;
+}
+
+
+
+/*
+ * Reset our device and wait for answer to VERSION request
+ */
+static int picolcd_reset(struct hid_device *hdev)
+{
+ struct picolcd_data *data = hid_get_drvdata(hdev);
+ struct hid_report *report = picolcd_out_report(REPORT_RESET, hdev);
+ struct picolcd_pending *verinfo;
+ unsigned long flags;
+
+ if (!data || !report || report->maxfield != 1)
+ return -ENODEV;
+
+ spin_lock_irqsave(&data->lock, flags);
+ complete_all(&data->ready);
+ INIT_COMPLETION(data->ready);
+ if (hdev->product = USB_DEVICE_ID_PICOLCD_BOOTLOADER)
+ data->status |= PICOLCD_BOOTLOADER;
+
+ /* perform the reset */
+ hid_set_field(report->field[0], 0, 1);
+ usbhid_submit_report(hdev, report, USB_DIR_OUT);
+ spin_unlock_irqrestore(&data->lock, flags);
+
+ verinfo = picolcd_send_and_wait(hdev, REPORT_VERSION, NULL, 0);
+ if (verinfo && verinfo->raw_size = 2) {
+ if (data->status & PICOLCD_BOOTLOADER) {
+ dev_info(&hdev->dev, "PicoLCD reset successful, bootloader version %d.%d\n",
+ verinfo->raw_data[0], verinfo->raw_data[1]);
+ data->version[0] = verinfo->raw_data[0];
+ data->version[1] = verinfo->raw_data[1];
+ } else {
+ dev_info(&hdev->dev, "PicoLCD reset successful, firmware version %d.%d\n",
+ verinfo->raw_data[1], verinfo->raw_data[0]);
+ data->version[0] = verinfo->raw_data[1];
+ data->version[1] = verinfo->raw_data[0];
+ }
+ kfree(verinfo);
+ verinfo = NULL;
+ } else if (verinfo) {
+ dev_err(&hdev->dev, "confused, got unexpected version response from PicoLCD after reset\n");
+ kfree(verinfo);
+ verinfo = NULL;
+ } else {
+ dev_err(&hdev->dev, "no version response from PicoLCD after reset");
+ return -EBUSY;
+ }
+
+ return 0;
+}
+
+/*
+ * The "operation_mode" sysfs attribute
+ */
+static ssize_t picolcd_operation_mode_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct picolcd_data *data = dev_get_drvdata(dev);
+
+ if (data->status & PICOLCD_BOOTLOADER)
+ return snprintf(buf, PAGE_SIZE, "<bootloader> lcd\n");
+ else
+ return snprintf(buf, PAGE_SIZE, "bootloader <lcd>\n");
+}
+
+static ssize_t picolcd_operation_mode_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct picolcd_data *data = dev_get_drvdata(dev);
+ struct hid_report *report = NULL;
+ size_t cnt = count;
+ int timeout = 5000;
+ unsigned u;
+ unsigned long flags;
+
+ if (cnt >= 3 && strncmp("lcd", buf, 3) = 0) {
+ if (data->status & PICOLCD_BOOTLOADER)
+ report = picolcd_out_report(REPORT_EXIT_FLASHER, data->hdev);
+ buf += 3;
+ cnt -= 3;
+ } else if (cnt >= 10 && strncmp("bootloader", buf, 10) = 0) {
+ if (!(data->status & PICOLCD_BOOTLOADER))
+ report = picolcd_out_report(REPORT_EXIT_KEYBOARD, data->hdev);
+ buf += 10;
+ cnt -= 10;
+ }
+ if (!report)
+ return -EINVAL;
+
+ while (cnt > 0 && (*buf = ' ' || *buf = '\t')) {
+ buf++;
+ cnt--;
+ }
+ while (cnt > 0 && (buf[cnt-1] = '\n' || buf[cnt-1] = '\r'))
+ cnt--;
+ if (cnt > 0) {
+ if (sscanf(buf, "%u", &u) != 1)
+ return -EINVAL;
+ if (u > 30000)
+ return -EINVAL;
+ else
+ timeout = u;
+ }
+
+ spin_lock_irqsave(&data->lock, flags);
+ hid_set_field(report->field[0], 0, timeout & 0xff);
+ hid_set_field(report->field[0], 1, (timeout >> 8) & 0xff);
+ usbhid_submit_report(data->hdev, report, USB_DIR_OUT);
+ spin_unlock_irqrestore(&data->lock, flags);
+ return count;
+}
+
+static DEVICE_ATTR(operation_mode, 0644, picolcd_operation_mode_show,
+ picolcd_operation_mode_store);
+
+
+#ifdef CONFIG_DEBUG_FS
+/*
+ * Helper code for HID report level dumping/debugging
+ */
+static const char *error_codes[] = {
+ "success", "parameter missing", "data_missing", "block readonly", "block not erasable",
+ "block too big", "section overflow", "invalid command length", "invalid data length",
+};
+
+static void dump_buff_as_hex(char *dst, size_t dst_sz, const u8 *data,
+ const size_t data_len)
+{
+ int i, j;
+ for (i = j = 0; i < data_len && j + 3 < dst_sz; i++) {
+ dst[j++] = hex_asc[(data[i] >> 4) & 0x0f];
+ dst[j++] = hex_asc[data[i] & 0x0f];
+ dst[j++] = ' ';
+ }
+ if (j < dst_sz) {
+ dst[j--] = '\0';
+ dst[j] = '\n';
+ } else
+ dst[j] = '\0';
+}
+
+static void picolcd_debug_out_report(struct picolcd_data *data,
+ struct hid_device *hdev, struct hid_report *report)
+{
+ u8 raw_data[70];
+ int raw_size = (report->size >> 3) + 1;
+ char *buff;
+#define BUFF_SZ 256
+
+ /* Avoid unnecessary overhead if debugfs is disabled */
+ if (!hdev->debug_events)
+ return;
+
+ buff = kmalloc(BUFF_SZ, GFP_ATOMIC);
+ if (!buff)
+ return;
+
+ snprintf(buff, BUFF_SZ, "\nout report %d (size %d) = ",
+ report->id, raw_size);
+ hid_debug_event(hdev, buff);
+ if (raw_size + 5 > sizeof(raw_data)) {
+ hid_debug_event(hdev, " TOO BIG\n");
+ return;
+ } else {
+ raw_data[0] = report->id;
+ hid_output_report(report, raw_data);
+ dump_buff_as_hex(buff, BUFF_SZ, raw_data, raw_size);
+ hid_debug_event(hdev, buff);
+ }
+
+ switch (report->id) {
+ case REPORT_LED_STATE:
+ /* 1 data byte with GPO state */
+ snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
+ "REPORT_LED_STATE", report->id, raw_size-1);
+ hid_debug_event(hdev, buff);
+ snprintf(buff, BUFF_SZ, "\tGPO state: 0x%02x\n", raw_data[1]);
+ hid_debug_event(hdev, buff);
+ break;
+ case REPORT_BRIGHTNESS:
+ /* 1 data byte with brightness */
+ snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
+ "REPORT_BRIGHTNESS", report->id, raw_size-1);
+ hid_debug_event(hdev, buff);
+ snprintf(buff, BUFF_SZ, "\tBrightness: 0x%02x\n", raw_data[1]);
+ hid_debug_event(hdev, buff);
+ break;
+ case REPORT_CONTRAST:
+ /* 1 data byte with contrast */
+ snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
+ "REPORT_CONTRAST", report->id, raw_size-1);
+ hid_debug_event(hdev, buff);
+ snprintf(buff, BUFF_SZ, "\tContrast: 0x%02x\n", raw_data[1]);
+ hid_debug_event(hdev, buff);
+ break;
+ case REPORT_RESET:
+ /* 2 data bytes with reset duration in ms */
+ snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
+ "REPORT_RESET", report->id, raw_size-1);
+ hid_debug_event(hdev, buff);
+ snprintf(buff, BUFF_SZ, "\tDuration: 0x%02x%02x (%dms)\n",
+ raw_data[2], raw_data[1], raw_data[2] << 8 | raw_data[1]);
+ hid_debug_event(hdev, buff);
+ break;
+ case REPORT_LCD_CMD:
+ /* 63 data bytes with LCD commands */
+ snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
+ "REPORT_LCD_CMD", report->id, raw_size-1);
+ hid_debug_event(hdev, buff);
+ /* TODO: format decoding */
+ break;
+ case REPORT_LCD_DATA:
+ /* 63 data bytes with LCD data */
+ snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
+ "REPORT_LCD_CMD", report->id, raw_size-1);
+ /* TODO: format decoding */
+ hid_debug_event(hdev, buff);
+ break;
+ case REPORT_LCD_CMD_DATA:
+ /* 63 data bytes with LCD commands and data */
+ snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
+ "REPORT_LCD_CMD", report->id, raw_size-1);
+ /* TODO: format decoding */
+ hid_debug_event(hdev, buff);
+ break;
+ case REPORT_EE_READ:
+ /* 3 data bytes with read area description */
+ snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
+ "REPORT_EE_READ", report->id, raw_size-1);
+ hid_debug_event(hdev, buff);
+ snprintf(buff, BUFF_SZ, "\tData address: 0x%02x%02x\n", raw_data[2], raw_data[1]);
+ hid_debug_event(hdev, buff);
+ snprintf(buff, BUFF_SZ, "\tData length: %d\n", raw_data[3]);
+ hid_debug_event(hdev, buff);
+ break;
+ case REPORT_EE_WRITE:
+ /* 3+1..20 data bytes with write area description */
+ snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
+ "REPORT_EE_WRITE", report->id, raw_size-1);
+ hid_debug_event(hdev, buff);
+ snprintf(buff, BUFF_SZ, "\tData address: 0x%02x%02x\n", raw_data[2], raw_data[1]);
+ hid_debug_event(hdev, buff);
+ snprintf(buff, BUFF_SZ, "\tData length: %d\n", raw_data[3]);
+ hid_debug_event(hdev, buff);
+ if (raw_data[3] = 0) {
+ snprintf(buff, BUFF_SZ, "\tNo data\n");
+ } else if (raw_data[3] + 4 <= raw_size) {
+ snprintf(buff, BUFF_SZ, "\tData: ");
+ hid_debug_event(hdev, buff);
+ dump_buff_as_hex(buff, BUFF_SZ, raw_data+4, raw_data[3]);
+ } else {
+ snprintf(buff, BUFF_SZ, "\tData overflowed\n");
+ }
+ hid_debug_event(hdev, buff);
+ break;
+ case REPORT_ERASE_MEMORY:
+ case REPORT_BL_ERASE_MEMORY:
+ /* 3 data bytes with pointer inside erase block */
+ snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
+ "REPORT_ERASE_MEMORY", report->id, raw_size-1);
+ hid_debug_event(hdev, buff);
+ switch (data->addr_sz) {
+ case 2:
+ snprintf(buff, BUFF_SZ, "\tAddress inside 64 byte block: 0x%02x%02x\n",
+ raw_data[2], raw_data[1]);
+ break;
+ case 3:
+ snprintf(buff, BUFF_SZ, "\tAddress inside 64 byte block: 0x%02x%02x%02x\n",
+ raw_data[3], raw_data[2], raw_data[1]);
+ break;
+ default:
+ snprintf(buff, BUFF_SZ, "\tNot supported\n");
+ }
+ hid_debug_event(hdev, buff);
+ break;
+ case REPORT_READ_MEMORY:
+ case REPORT_BL_READ_MEMORY:
+ /* 4 data bytes with read area description */
+ snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
+ "REPORT_READ_MEMORY", report->id, raw_size-1);
+ hid_debug_event(hdev, buff);
+ switch (data->addr_sz) {
+ case 2:
+ snprintf(buff, BUFF_SZ, "\tData address: 0x%02x%02x\n",
+ raw_data[2], raw_data[1]);
+ hid_debug_event(hdev, buff);
+ snprintf(buff, BUFF_SZ, "\tData length: %d\n", raw_data[3]);
+ break;
+ case 3:
+ snprintf(buff, BUFF_SZ, "\tData address: 0x%02x%02x%02x\n",
+ raw_data[3], raw_data[2], raw_data[1]);
+ hid_debug_event(hdev, buff);
+ snprintf(buff, BUFF_SZ, "\tData length: %d\n", raw_data[4]);
+ break;
+ default:
+ snprintf(buff, BUFF_SZ, "\tNot supported\n");
+ }
+ hid_debug_event(hdev, buff);
+ break;
+ case REPORT_WRITE_MEMORY:
+ case REPORT_BL_WRITE_MEMORY:
+ /* 4+1..32 data bytes with write adrea description */
+ snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
+ "REPORT_WRITE_MEMORY", report->id, raw_size-1);
+ hid_debug_event(hdev, buff);
+ switch (data->addr_sz) {
+ case 2:
+ snprintf(buff, BUFF_SZ, "\tData address: 0x%02x%02x\n",
+ raw_data[2], raw_data[1]);
+ hid_debug_event(hdev, buff);
+ snprintf(buff, BUFF_SZ, "\tData length: %d\n", raw_data[3]);
+ hid_debug_event(hdev, buff);
+ if (raw_data[3] = 0) {
+ snprintf(buff, BUFF_SZ, "\tNo data\n");
+ } else if (raw_data[3] + 4 <= raw_size) {
+ snprintf(buff, BUFF_SZ, "\tData: ");
+ hid_debug_event(hdev, buff);
+ dump_buff_as_hex(buff, BUFF_SZ, raw_data+4, raw_data[3]);
+ } else {
+ snprintf(buff, BUFF_SZ, "\tData overflowed\n");
+ }
+ break;
+ case 3:
+ snprintf(buff, BUFF_SZ, "\tData address: 0x%02x%02x%02x\n",
+ raw_data[3], raw_data[2], raw_data[1]);
+ hid_debug_event(hdev, buff);
+ snprintf(buff, BUFF_SZ, "\tData length: %d\n", raw_data[4]);
+ hid_debug_event(hdev, buff);
+ if (raw_data[4] = 0) {
+ snprintf(buff, BUFF_SZ, "\tNo data\n");
+ } else if (raw_data[4] + 5 <= raw_size) {
+ snprintf(buff, BUFF_SZ, "\tData: ");
+ hid_debug_event(hdev, buff);
+ dump_buff_as_hex(buff, BUFF_SZ, raw_data+5, raw_data[4]);
+ } else {
+ snprintf(buff, BUFF_SZ, "\tData overflowed\n");
+ }
+ break;
+ default:
+ snprintf(buff, BUFF_SZ, "\tNot supported\n");
+ }
+ hid_debug_event(hdev, buff);
+ break;
+ case REPORT_SPLASH_RESTART:
+ /* TODO */
+ break;
+ case REPORT_EXIT_KEYBOARD:
+ snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
+ "REPORT_EXIT_KEYBOARD", report->id, raw_size-1);
+ hid_debug_event(hdev, buff);
+ snprintf(buff, BUFF_SZ, "\tRestart delay: %dms (0x%02x%02x)\n",
+ raw_data[1] | (raw_data[2] << 8),
+ raw_data[2], raw_data[1]);
+ hid_debug_event(hdev, buff);
+ break;
+ case REPORT_VERSION:
+ snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
+ "REPORT_VERSION", report->id, raw_size-1);
+ hid_debug_event(hdev, buff);
+ break;
+ case REPORT_DEVID:
+ snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
+ "REPORT_DEVID", report->id, raw_size-1);
+ hid_debug_event(hdev, buff);
+ break;
+ case REPORT_SPLASH_SIZE:
+ snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
+ "REPORT_SPLASH_SIZE", report->id, raw_size-1);
+ hid_debug_event(hdev, buff);
+ break;
+ case REPORT_HOOK_VERSION:
+ snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
+ "REPORT_HOOK_VERSION", report->id, raw_size-1);
+ hid_debug_event(hdev, buff);
+ break;
+ case REPORT_EXIT_FLASHER:
+ snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
+ "REPORT_VERSION", report->id, raw_size-1);
+ hid_debug_event(hdev, buff);
+ snprintf(buff, BUFF_SZ, "\tRestart delay: %dms (0x%02x%02x)\n",
+ raw_data[1] | (raw_data[2] << 8),
+ raw_data[2], raw_data[1]);
+ hid_debug_event(hdev, buff);
+ break;
+ default:
+ snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
+ "<unknown>", report->id, raw_size-1);
+ hid_debug_event(hdev, buff);
+ break;
+ }
+ wake_up_interruptible(&hdev->debug_wait);
+ kfree(buff);
+}
+
+static inline void picolcd_debug_raw_event(struct picolcd_data *data,
+ struct hid_device *hdev, struct hid_report *report,
+ u8 *raw_data, int size)
+{
+ char *buff;
+
+#define BUFF_SZ 256
+ /* Avoid unnecessary overhead if debugfs is disabled */
+ if (!hdev->debug_events)
+ return;
+
+ buff = kmalloc(BUFF_SZ, GFP_ATOMIC);
+ if (!buff)
+ return;
+
+ switch (report->id) {
+ case REPORT_ERROR_CODE:
+ /* 2 data bytes with affected report and error code */
+ snprintf(buff, BUFF_SZ, "report %s (%d, size=%d)\n",
+ "REPORT_ERROR_CODE", report->id, size-1);
+ hid_debug_event(hdev, buff);
+ if (raw_data[2] < ARRAY_SIZE(error_codes))
+ snprintf(buff, BUFF_SZ, "\tError code 0x%02x (%s) in reply to report 0x%02x\n",
+ raw_data[2], error_codes[raw_data[2]], raw_data[1]);
+ else
+ snprintf(buff, BUFF_SZ, "\tError code 0x%02x in reply to report 0x%02x\n",
+ raw_data[2], raw_data[1]);
+ hid_debug_event(hdev, buff);
+ break;
+ case REPORT_KEY_STATE:
+ /* 2 data bytes with key state */
+ snprintf(buff, BUFF_SZ, "report %s (%d, size=%d)\n",
+ "REPORT_KEY_STATE", report->id, size-1);
+ hid_debug_event(hdev, buff);
+ if (raw_data[1] = 0)
+ snprintf(buff, BUFF_SZ, "\tNo key pressed\n");
+ else if (raw_data[2] = 0)
+ snprintf(buff, BUFF_SZ, "\tOne key pressed: 0x%02x (%d)\n",
+ raw_data[1], raw_data[1]);
+ else
+ snprintf(buff, BUFF_SZ, "\tTwo keys pressed: 0x%02x (%d), 0x%02x (%d)\n",
+ raw_data[1], raw_data[1], raw_data[2], raw_data[2]);
+ hid_debug_event(hdev, buff);
+ break;
+ case REPORT_IR_DATA:
+ /* Up to 20 byes of IR scancode data */
+ snprintf(buff, BUFF_SZ, "report %s (%d, size=%d)\n",
+ "REPORT_IR_DATA", report->id, size-1);
+ hid_debug_event(hdev, buff);
+ if (raw_data[1] = 0) {
+ snprintf(buff, BUFF_SZ, "\tUnexpectedly 0 data length\n");
+ hid_debug_event(hdev, buff);
+ } else if (raw_data[1] + 1 <= size) {
+ snprintf(buff, BUFF_SZ, "\tData length: %d\n\tIR Data: ",
+ raw_data[1]-1);
+ hid_debug_event(hdev, buff);
+ dump_buff_as_hex(buff, BUFF_SZ, raw_data+2, raw_data[1]-1);
+ hid_debug_event(hdev, buff);
+ } else {
+ snprintf(buff, BUFF_SZ, "\tOverflowing data length: %d\n",
+ raw_data[1]-1);
+ hid_debug_event(hdev, buff);
+ }
+ break;
+ case REPORT_EE_DATA:
+ /* Data buffer in response to REPORT_EE_READ or REPORT_EE_WRITE */
+ snprintf(buff, BUFF_SZ, "report %s (%d, size=%d)\n",
+ "REPORT_EE_DATA", report->id, size-1);
+ hid_debug_event(hdev, buff);
+ snprintf(buff, BUFF_SZ, "\tData address: 0x%02x%02x\n", raw_data[2], raw_data[1]);
+ hid_debug_event(hdev, buff);
+ snprintf(buff, BUFF_SZ, "\tData length: %d\n", raw_data[3]);
+ hid_debug_event(hdev, buff);
+ if (raw_data[3] = 0) {
+ snprintf(buff, BUFF_SZ, "\tNo data\n");
+ hid_debug_event(hdev, buff);
+ } else if (raw_data[3] + 4 <= size) {
+ snprintf(buff, BUFF_SZ, "\tData: ");
+ hid_debug_event(hdev, buff);
+ dump_buff_as_hex(buff, BUFF_SZ, raw_data+4, raw_data[3]);
+ hid_debug_event(hdev, buff);
+ } else {
+ snprintf(buff, BUFF_SZ, "\tData overflowed\n");
+ hid_debug_event(hdev, buff);
+ }
+ break;
+ case REPORT_MEMORY:
+ /* Data buffer in response to REPORT_READ_MEMORY or REPORT_WRTIE_MEMORY */
+ snprintf(buff, BUFF_SZ, "report %s (%d, size=%d)\n",
+ "REPORT_MEMORY", report->id, size-1);
+ hid_debug_event(hdev, buff);
+ switch (data->addr_sz) {
+ case 2:
+ snprintf(buff, BUFF_SZ, "\tData address: 0x%02x%02x\n",
+ raw_data[2], raw_data[1]);
+ hid_debug_event(hdev, buff);
+ snprintf(buff, BUFF_SZ, "\tData length: %d\n", raw_data[3]);
+ hid_debug_event(hdev, buff);
+ if (raw_data[3] = 0) {
+ snprintf(buff, BUFF_SZ, "\tNo data\n");
+ } else if (raw_data[3] + 4 <= size) {
+ snprintf(buff, BUFF_SZ, "\tData: ");
+ hid_debug_event(hdev, buff);
+ dump_buff_as_hex(buff, BUFF_SZ, raw_data+4, raw_data[3]);
+ } else {
+ snprintf(buff, BUFF_SZ, "\tData overflowed\n");
+ }
+ break;
+ case 3:
+ snprintf(buff, BUFF_SZ, "\tData address: 0x%02x%02x%02x\n",
+ raw_data[3], raw_data[2], raw_data[1]);
+ hid_debug_event(hdev, buff);
+ snprintf(buff, BUFF_SZ, "\tData length: %d\n", raw_data[4]);
+ hid_debug_event(hdev, buff);
+ if (raw_data[4] = 0) {
+ snprintf(buff, BUFF_SZ, "\tNo data\n");
+ } else if (raw_data[4] + 5 <= size) {
+ snprintf(buff, BUFF_SZ, "\tData: ");
+ hid_debug_event(hdev, buff);
+ dump_buff_as_hex(buff, BUFF_SZ, raw_data+5, raw_data[4]);
+ } else {
+ snprintf(buff, BUFF_SZ, "\tData overflowed\n");
+ }
+ break;
+ default:
+ snprintf(buff, BUFF_SZ, "\tNot supported\n");
+ }
+ hid_debug_event(hdev, buff);
+ break;
+ case REPORT_VERSION:
+ snprintf(buff, BUFF_SZ, "report %s (%d, size=%d)\n",
+ "REPORT_VERSION", report->id, size-1);
+ hid_debug_event(hdev, buff);
+ snprintf(buff, BUFF_SZ, "\tFirmware version: %d.%d\n",
+ raw_data[2], raw_data[1]);
+ hid_debug_event(hdev, buff);
+ break;
+ case REPORT_BL_ERASE_MEMORY:
+ snprintf(buff, BUFF_SZ, "report %s (%d, size=%d)\n",
+ "REPORT_BL_ERASE_MEMORY", report->id, size-1);
+ hid_debug_event(hdev, buff);
+ /* TODO */
+ break;
+ case REPORT_BL_READ_MEMORY:
+ snprintf(buff, BUFF_SZ, "report %s (%d, size=%d)\n",
+ "REPORT_BL_READ_MEMORY", report->id, size-1);
+ hid_debug_event(hdev, buff);
+ /* TODO */
+ break;
+ case REPORT_BL_WRITE_MEMORY:
+ snprintf(buff, BUFF_SZ, "report %s (%d, size=%d)\n",
+ "REPORT_BL_WRITE_MEMORY", report->id, size-1);
+ hid_debug_event(hdev, buff);
+ /* TODO */
+ break;
+ case REPORT_DEVID:
+ snprintf(buff, BUFF_SZ, "report %s (%d, size=%d)\n",
+ "REPORT_DEVID", report->id, size-1);
+ hid_debug_event(hdev, buff);
+ snprintf(buff, BUFF_SZ, "\tSerial: 0x%02x%02x%02x%02x\n",
+ raw_data[1], raw_data[2], raw_data[3], raw_data[4]);
+ hid_debug_event(hdev, buff);
+ snprintf(buff, BUFF_SZ, "\tType: 0x%02x\n",
+ raw_data[5]);
+ hid_debug_event(hdev, buff);
+ break;
+ case REPORT_SPLASH_SIZE:
+ snprintf(buff, BUFF_SZ, "report %s (%d, size=%d)\n",
+ "REPORT_SPLASH_SIZE", report->id, size-1);
+ hid_debug_event(hdev, buff);
+ snprintf(buff, BUFF_SZ, "\tTotal splash space: %d\n",
+ (raw_data[2] << 8) | raw_data[1]);
+ hid_debug_event(hdev, buff);
+ snprintf(buff, BUFF_SZ, "\tUsed splash space: %d\n",
+ (raw_data[4] << 8) | raw_data[3]);
+ hid_debug_event(hdev, buff);
+ break;
+ case REPORT_HOOK_VERSION:
+ snprintf(buff, BUFF_SZ, "report %s (%d, size=%d)\n",
+ "REPORT_HOOK_VERSION", report->id, size-1);
+ hid_debug_event(hdev, buff);
+ snprintf(buff, BUFF_SZ, "\tFirmware version: %d.%d\n",
+ raw_data[1], raw_data[2]);
+ hid_debug_event(hdev, buff);
+ break;
+ default:
+ snprintf(buff, BUFF_SZ, "report %s (%d, size=%d)\n",
+ "<unknown>", report->id, size-1);
+ hid_debug_event(hdev, buff);
+ break;
+ }
+ wake_up_interruptible(&hdev->debug_wait);
+ kfree(buff);
+}
+#else
+#define picolcd_debug_raw_event(data, hdev, report, raw_data, size)
+#endif
+
+/*
+ * Handle raw report as sent by device
+ */
+static int picolcd_raw_event(struct hid_device *hdev,
+ struct hid_report *report, u8 *raw_data, int size)
+{
+ struct picolcd_data *data = hid_get_drvdata(hdev);
+ unsigned long flags;
+ int ret = 0;
+
+ if (data = NULL)
+ return 1;
+
+ if (report->id = REPORT_KEY_STATE) {
+ if (data->input_keys)
+ ret = picolcd_raw_keypad(hdev, report, raw_data+1, size-1);
+ } else if (report->id = REPORT_IR_DATA) {
+ if (data->input_cir)
+ ret = picolcd_raw_cir(hdev, report, raw_data+1, size-1);
+ } else {
+ spin_lock_irqsave(&data->lock, flags);
+ /*
+ * We let the caller of picolcd_send_and_wait() check if the report
+ * we got is one of the expected ones or not.
+ */
+ if (data->pending) {
+ memcpy(data->pending->raw_data, raw_data+1, size-1);
+ data->pending->raw_size = size-1;
+ data->pending->in_report = report;
+ complete_all(&data->ready);
+ }
+ spin_unlock_irqrestore(&data->lock, flags);
+ }
+
+ picolcd_debug_raw_event(data, hdev, report, raw_data, size);
+ return 1;
+}
+
+#ifdef CONFIG_PM
+static int picolcd_suspend(struct hid_device *hdev)
+{
+ dbg_hid(PICOLCD_NAME " device ready for suspend\n");
+ return 0;
+}
+
+static int picolcd_resume(struct hid_device *hdev)
+{
+ return 0;
+}
+
+static int picolcd_reset_resume(struct hid_device *hdev)
+{
+ int ret;
+ ret = picolcd_reset(hdev);
+ if (ret)
+ dbg_hid(PICOLCD_NAME " resetting our device failed: %d\n", ret);
+ return 0;
+}
+#endif
+
+/* initialize keypad input device */
+static inline int picolcd_init_keys(struct picolcd_data *data,
+ struct hid_report *report)
+{
+ struct hid_device *hdev = data->hdev;
+ struct input_dev *idev;
+ int error, i;
+
+ if (!report)
+ return -ENODEV;
+ if (report->maxfield != 1 || report->field[0]->report_count != 2 ||
+ report->field[0]->report_size != 8) {
+ dev_err(&hdev->dev, "unsupported KEY_STATE report");
+ return -EINVAL;
+ }
+
+ idev = input_allocate_device();
+ if (idev = NULL) {
+ dev_err(&hdev->dev, "failed to allocate input device");
+ return -ENOMEM;
+ }
+ input_set_drvdata(idev, hdev);
+ memcpy(data->keycode, def_keymap, sizeof(def_keymap));
+ idev->name = hdev->name;
+ idev->phys = hdev->phys;
+ idev->uniq = hdev->uniq;
+ idev->id.bustype = hdev->bus;
+ idev->id.vendor = hdev->vendor;
+ idev->id.product = hdev->product;
+ idev->id.version = hdev->version;
+ idev->dev.parent = hdev->dev.parent;
+ idev->keycode = &data->keycode;
+ idev->keycodemax = PICOLCD_KEYS;
+ idev->keycodesize = sizeof(int);
+ input_set_capability(idev, EV_MSC, MSC_SCAN);
+ set_bit(EV_REP, idev->evbit);
+ for (i = 0; i < PICOLCD_KEYS; i++) {
+ int key = ((int *)idev->keycode)[i];
+ if (key < KEY_MAX && key >= 0)
+ input_set_capability(idev, EV_KEY, key);
+ }
+ error = input_register_device(idev);
+ if (error) {
+ dev_err(&hdev->dev, "error registering the input device");
+ input_free_device(idev);
+ return error;
+ }
+ data->input_keys = idev;
+ return 0;
+}
+
+static void picolcd_exit_keys(struct picolcd_data *data)
+{
+ struct input_dev *idev = data->input_keys;
+
+ data->input_keys = NULL;
+ if (idev)
+ input_unregister_device(idev);
+}
+
+/* initialize CIR input device */
+static inline int picolcd_init_cir(struct picolcd_data *data, struct hid_report *report)
+{
+ /* support not implemented yet */
+ return 0;
+}
+
+static void picolcd_exit_cir(struct picolcd_data *data)
+{
+}
+
+static inline int picolcd_probe_lcd(struct hid_device *hdev, struct picolcd_data *data)
+{
+ struct picolcd_pending *verinfo;
+ struct hid_report *report;
+ int error;
+
+ verinfo = picolcd_send_and_wait(hdev, REPORT_VERSION, NULL, 0);
+ if (!verinfo || !verinfo->in_report) {
+ kfree(verinfo);
+ dev_err(&hdev->dev, "failed to query FW version of device\n");
+ return -ENODEV;
+ } else if (verinfo->in_report->id = REPORT_VERSION && verinfo->raw_size = 2) {
+ dev_info(&hdev->dev, "detected PicoLCD with firmware version %d.%d\n",
+ verinfo->raw_data[0], verinfo->raw_data[1]);
+ data->version[0] = verinfo->raw_data[1];
+ data->version[1] = verinfo->raw_data[0];
+ if (data->version[0] != 0 && data->version[1] != 3)
+ dev_info(&hdev->dev, "Device with untested firmware revision, "
+ "please submit /sys/kernel/debug/hid/%s/rdesc for this device.\n",
+ dev_name(&hdev->dev));
+ kfree(verinfo);
+ verinfo = NULL;
+ } else {
+ dev_err(&hdev->dev, "unexpected version response from PicoLCD"
+ " (report=0x%02x, size=%d)\n",
+ verinfo->in_report->id, verinfo->raw_size);
+ kfree(verinfo);
+ verinfo = NULL;
+ return -ENODEV;
+ }
+
+ /* Setup keypad input device */
+ error = picolcd_init_keys(data, picolcd_in_report(REPORT_KEY_STATE, hdev));
+ if (error)
+ goto err;
+
+ /* Setup CIR input device */
+ error = picolcd_init_cir(data, picolcd_in_report(REPORT_IR_DATA, hdev));
+ if (error)
+ goto err;
+
+#ifdef CONFIG_DEBUG_FS
+ report = picolcd_out_report(REPORT_READ_MEMORY, hdev);
+ if (report && report->maxfield = 1 && report->field[0]->report_size = 8)
+ data->addr_sz = report->field[0]->report_count - 1;
+ else
+ data->addr_sz = -1;
+#endif
+ return 0;
+err:
+ picolcd_exit_cir(data);
+ picolcd_exit_keys(data);
+ return error;
+}
+
+static inline int picolcd_probe_bootloader(struct hid_device *hdev, struct picolcd_data *data)
+{
+ struct picolcd_pending *verinfo;
+ struct hid_report *report;
+
+ verinfo = picolcd_send_and_wait(hdev, REPORT_VERSION, NULL, 0);
+ if (!verinfo || !verinfo->in_report) {
+ kfree(verinfo);
+ dev_err(&hdev->dev, "failed to query FW version of device\n");
+ return -ENODEV;
+ } else if (verinfo->in_report->id = REPORT_VERSION && verinfo->raw_size = 2) {
+ dev_info(&hdev->dev, "detected PicoLCD with bootloader version %d.%d\n",
+ verinfo->raw_data[1], verinfo->raw_data[0]);
+ data->version[0] = verinfo->raw_data[1];
+ data->version[1] = verinfo->raw_data[0];
+ if (data->version[0] != 1 && data->version[1] != 0)
+ dev_info(&hdev->dev, "Device with untested bootloader revision, "
+ "please submit /sys/kernel/debug/hid/%s/rdesc for this device.\n",
+ dev_name(&hdev->dev));
+ kfree(verinfo);
+ verinfo = NULL;
+ } else {
+ dev_err(&hdev->dev, "unexpected version response from PicoLCD"
+ " (report=0x%02x, size=%d)\n",
+ verinfo->in_report->id, verinfo->raw_size);
+ kfree(verinfo);
+ verinfo = NULL;
+ return -ENODEV;
+ }
+
+#ifdef CONFIG_DEBUG_FS
+ report = picolcd_out_report(REPORT_BL_READ_MEMORY, hdev);
+ if (report && report->maxfield = 1 && report->field[0]->report_size = 8)
+ data->addr_sz = report->field[0]->report_count - 1;
+ else
+ data->addr_sz = -1;
+#endif
+ return 0;
+}
+
+static int picolcd_probe(struct hid_device *hdev,
+ const struct hid_device_id *id)
+{
+ struct picolcd_data *data;
+ int error = -ENOMEM;
+
+ dbg_hid(PICOLCD_NAME " hardware probe...\n");
+
+ /*
+ * Let's allocate the picolcd data structure, set some reasonable
+ * defaults, and associate it with the device
+ */
+ data = kzalloc(sizeof(struct picolcd_data), GFP_KERNEL);
+ if (data = NULL) {
+ dev_err(&hdev->dev, "can't allocate space for Minibox PicoLCD device data\n");
+ error = -ENOMEM;
+ goto err_no_cleanup;
+ }
+
+ spin_lock_init(&data->lock);
+ init_completion(&data->ready);
+ data->hdev = hdev;
+ if (hdev->product = USB_DEVICE_ID_PICOLCD_BOOTLOADER)
+ data->status |= PICOLCD_BOOTLOADER;
+ hid_set_drvdata(hdev, data);
+
+ /* Parse the device reports and start it up */
+ error = hid_parse(hdev);
+ if (error) {
+ dev_err(&hdev->dev, "device report parse failed\n");
+ goto err_cleanup_data;
+ }
+
+ /* We don't use hidinput but hid_hw_start() fails if nothing is
+ * claimed. So spoof claimed input. */
+ hdev->claimed = HID_CLAIMED_INPUT;
+ error = hid_hw_start(hdev, 0);
+ hdev->claimed = 0;
+ if (error) {
+ dev_err(&hdev->dev, "hardware start failed\n");
+ goto err_cleanup_data;
+ }
+
+ error = hdev->ll_driver->open(hdev);
+ if (error) {
+ dev_err(&hdev->dev, "failed to open input interrupt pipe for key and IR events\n");
+ goto err_cleanup_hid_hw;
+ }
+
+ error = sysfs_create_file(&(hdev->dev.kobj), &dev_attr_operation_mode.attr);
+ if (error) {
+ dev_err(&hdev->dev, "failed to create sysfs attributes\n");
+ goto err_cleanup_hid_ll;
+ }
+
+ if (data->status & PICOLCD_BOOTLOADER)
+ error = picolcd_probe_bootloader(hdev, data);
+ else
+ error = picolcd_probe_lcd(hdev, data);
+ if (error)
+ goto err_cleanup_sysfs;
+
+ dbg_hid(PICOLCD_NAME " activated and initialized\n");
+ return 0;
+
+err_cleanup_sysfs:
+ sysfs_remove_file(&(hdev->dev.kobj), &dev_attr_operation_mode.attr);
+err_cleanup_hid_ll:
+ hdev->ll_driver->close(hdev);
+err_cleanup_hid_hw:
+ hid_hw_stop(hdev);
+err_cleanup_data:
+ kfree(data);
+err_no_cleanup:
+ hid_set_drvdata(hdev, NULL);
+
+ return error;
+}
+
+static void picolcd_remove(struct hid_device *hdev)
+{
+ struct picolcd_data *data = hid_get_drvdata(hdev);
+ unsigned long flags;
+
+ dbg_hid(PICOLCD_NAME " hardware remove...\n");
+ spin_lock_irqsave(&data->lock, flags);
+ data->status |= PICOLCD_FAILED;
+ spin_unlock_irqrestore(&data->lock, flags);
+
+ sysfs_remove_file(&(hdev->dev.kobj), &dev_attr_operation_mode.attr);
+ hdev->ll_driver->close(hdev);
+ hid_hw_stop(hdev);
+
+ /* Cleanup input */
+ picolcd_exit_cir(data);
+ picolcd_exit_keys(data);
+
+ /* Finally, clean up the picolcd data itself */
+ kfree(data);
+}
+
+static const struct hid_device_id picolcd_devices[] = {
+ { HID_USB_DEVICE(USB_VENDOR_ID_MICROCHIP, USB_DEVICE_ID_PICOLCD) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_MICROCHIP, USB_DEVICE_ID_PICOLCD_BOOTLOADER) },
+ { }
+};
+MODULE_DEVICE_TABLE(hid, picolcd_devices);
+
+static struct hid_driver picolcd_driver = {
+ .name = "hid-picolcd",
+ .id_table = picolcd_devices,
+ .probe = picolcd_probe,
+ .remove = picolcd_remove,
+ .raw_event = picolcd_raw_event,
+#ifdef CONFIG_PM
+ .suspend = picolcd_suspend,
+ .resume = picolcd_resume,
+ .reset_resume = picolcd_reset_resume,
+#endif
+};
+
+static int __init picolcd_init(void)
+{
+ return hid_register_driver(&picolcd_driver);
+}
+
+static void __exit picolcd_exit(void)
+{
+ hid_unregister_driver(&picolcd_driver);
+}
+
+module_init(picolcd_init);
+module_exit(picolcd_exit);
+MODULE_DESCRIPTION("Minibox graphics PicoLCD Driver");
+MODULE_LICENSE("GPL v2");
--
1.6.4.4
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 2/6] hid: add framebuffer support to PicoLCD device
[not found] ` <20100320170014.440959a8-hY15tx4IgV39zxVx7UNMDg@public.gmane.org>
2010-03-20 16:02 ` [PATCH v2 1/6] " Bruno Prémont
@ 2010-03-20 16:04 ` Bruno Prémont
2010-03-21 3:25 ` Dmitry Torokhov
2010-03-21 7:24 ` Jaya Kumar
1 sibling, 2 replies; 20+ messages in thread
From: Bruno Prémont @ 2010-03-20 16:04 UTC (permalink / raw)
To: Jiri Kosina
Cc: linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rick L. Vinyard Jr.,
Nicu Pavel, Oliver Neukum, Jaya Kumar
Add framebuffer support to PicoLCD device with use of deferred-io.
Only changed areas of framebuffer get sent to device in order to
save USB bandwidth and especially resources on PicoLCD device or
allow higher refresh rate for a small area.
Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
---
drivers/hid/Kconfig | 7 +-
drivers/hid/hid-picolcd.c | 454 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 460 insertions(+), 1 deletions(-)
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 7097f0a..a474bcd 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -230,6 +230,11 @@ config HID_PETALYNX
config HID_PICOLCD
tristate "PicoLCD (graphic version)"
depends on USB_HID
+ select FB_DEFERRED_IO if FB
+ select FB_SYS_FILLRECT if FB
+ select FB_SYS_COPYAREA if FB
+ select FB_SYS_IMAGEBLIT if FB
+ select FB_SYS_FOPS if FB
---help---
This provides support for Minibox PicoLCD devices, currently
only the graphical ones are supported.
@@ -237,8 +242,8 @@ config HID_PICOLCD
This includes support for the following device features:
- Keypad
- Switching between Firmware and Flash mode
- Features that are not (yet) supported:
- Framebuffer for monochrome 256x64 display
+ Features that are not (yet) supported:
- Backlight control
- Contrast control
- IR
diff --git a/drivers/hid/hid-picolcd.c b/drivers/hid/hid-picolcd.c
index 30b8e08..0c1d293 100644
--- a/drivers/hid/hid-picolcd.c
+++ b/drivers/hid/hid-picolcd.c
@@ -24,6 +24,9 @@
#include "usbhid/usbhid.h"
#include <linux/usb.h>
+#include <linux/fb.h>
+#include <linux/vmalloc.h>
+
#include <linux/seq_file.h>
#include <linux/debugfs.h>
@@ -69,6 +72,59 @@
#define REPORT_HOOK_VERSION 0xf7 /* LCD: IN[2], OUT[1] */
#define REPORT_EXIT_FLASHER 0xff /* Bootloader: OUT[2] */
+#if defined(CONFIG_FB) || defined(CONFIG_FB_MODULE)
+/* Framebuffer
+ *
+ * The PicoLCD use a Topway LCD module of 256x64 pixel
+ * This display area is tiled over 4 controllers with 8 tiles
+ * each. Each tile has 8x64 pixel, each data byte representing
+ * a 1-bit wide vertical line of the tile.
+ *
+ * The display can be updated at a tile granularity.
+ *
+ * Chip 1 Chip 2 Chip 3 Chip 4
+ * +----------------+----------------+----------------+----------------+
+ * | Tile 1 | Tile 1 | Tile 1 | Tile 1 |
+ * +----------------+----------------+----------------+----------------+
+ * | Tile 2 | Tile 2 | Tile 2 | Tile 2 |
+ * +----------------+----------------+----------------+----------------+
+ * ...
+ * +----------------+----------------+----------------+----------------+
+ * | Tile 8 | Tile 8 | Tile 8 | Tile 8 |
+ * +----------------+----------------+----------------+----------------+
+ */
+#define PICOLCDFB_NAME "picolcdfb"
+#define PICOLCDFB_WIDTH (256)
+#define PICOLCDFB_LINE_LENGTH (256/8)
+#define PICOLCDFB_HEIGHT (64)
+#define PICOLCDFB_SIZE (PICOLCDFB_LINE_LENGTH*PICOLCDFB_HEIGHT)
+
+#define PICOLCDFB_UPDATE_RATE_LIMIT 10
+#define PICOLCDFB_UPDATE_RATE_DEFAULT 2
+
+/* Framebuffer visual structures */
+static const struct fb_fix_screeninfo picolcdfb_fix = {
+ .id = PICOLCDFB_NAME,
+ .type = FB_TYPE_PACKED_PIXELS,
+ .visual = FB_VISUAL_MONO01,
+ .xpanstep = 0,
+ .ypanstep = 0,
+ .ywrapstep = 0,
+ .line_length = PICOLCDFB_LINE_LENGTH,
+ .accel = FB_ACCEL_NONE,
+};
+
+static const struct fb_var_screeninfo picolcdfb_var = {
+ .xres = PICOLCDFB_WIDTH,
+ .yres = PICOLCDFB_HEIGHT,
+ .xres_virtual = PICOLCDFB_WIDTH,
+ .yres_virtual = PICOLCDFB_HEIGHT,
+ .width = 103,
+ .height = 26,
+ .bits_per_pixel = 1,
+};
+#endif /* CONFIG_FB */
+
/* Input device
*
* The PicoLCD has an IR receiver header, a built-in keypad with 5 keys
@@ -118,12 +174,22 @@ struct picolcd_data {
struct input_dev *input_cir;
int keycode[PICOLCD_KEYS];
+#if defined(CONFIG_FB) || defined(CONFIG_FB_MODULE)
+ /* Framebuffer stuff */
+ u8 fb_update_rate;
+ u8 *fb_vbitmap; /* local copy of what was sent to PicoLCD */
+ u8 *fb_bitmap; /* framebuffer */
+ struct fb_info *fb_info;
+ struct fb_deferred_io fb_defio;
+#endif /* CONFIG_FB */
+
/* Housekeeping stuff */
spinlock_t lock;
struct picolcd_pending *pending;
struct completion ready;
int status;
#define PICOLCD_BUSY 1
+#define PICOLCD_READY_FB 2
#define PICOLCD_FAILED 4
#define PICOLCD_BOOTLOADER 8
};
@@ -218,6 +284,378 @@ retry:
}
}
+#if defined(CONFIG_FB) || defined(CONFIG_FB_MODULE)
+/* Send a given tile to PicoLCD */
+static inline int picolcd_fb_send_tile(struct hid_device *hdev, int chip,
+ int tile)
+{
+ struct picolcd_data *data = hid_get_drvdata(hdev);
+ struct hid_report *report1 = picolcd_out_report(REPORT_LCD_CMD_DATA, hdev);
+ struct hid_report *report2 = picolcd_out_report(REPORT_LCD_DATA, hdev);
+ unsigned long flags;
+ u8 *tdata;
+ int i;
+
+ if (!report1 || report1->maxfield != 1 || !report2 || report2->maxfield != 1)
+ return -ENODEV;
+
+ spin_lock_irqsave(&data->lock, flags);
+ hid_set_field(report1->field[0], 0, chip << 2);
+ hid_set_field(report1->field[0], 1, 0x02);
+ hid_set_field(report1->field[0], 2, 0x00);
+ hid_set_field(report1->field[0], 3, 0x00);
+ hid_set_field(report1->field[0], 4, 0xb8 | tile);
+ hid_set_field(report1->field[0], 5, 0x00);
+ hid_set_field(report1->field[0], 6, 0x00);
+ hid_set_field(report1->field[0], 7, 0x40);
+ hid_set_field(report1->field[0], 8, 0x00);
+ hid_set_field(report1->field[0], 9, 0x00);
+ hid_set_field(report1->field[0], 10, 32);
+
+ hid_set_field(report2->field[0], 0, (chip << 2) | 0x01);
+ hid_set_field(report2->field[0], 1, 0x00);
+ hid_set_field(report2->field[0], 2, 0x00);
+ hid_set_field(report2->field[0], 3, 32);
+
+ tdata = data->fb_vbitmap + (tile * 4 + chip) * 64;
+ for (i = 0; i < 64; i++)
+ if (i < 32)
+ hid_set_field(report1->field[0], 11 + i, tdata[i]);
+ else
+ hid_set_field(report2->field[0], 4 + i - 32, tdata[i]);
+
+ usbhid_submit_report(data->hdev, report1, USB_DIR_OUT);
+ usbhid_submit_report(data->hdev, report2, USB_DIR_OUT);
+ spin_unlock_irqrestore(&data->lock, flags);
+ return 0;
+}
+
+/* Translate a single tile*/
+static inline int picolcd_fb_update_tile(u8 *vbitmap, const u8 *bitmap,
+ int chip, int tile)
+{
+ int i, b, changed = 0;
+ u8 tdata[64];
+ u8 *vdata = vbitmap + (tile * 4 + chip) * 64;
+
+ for (b = 7; b >= 0; b--) {
+ const u8 *bdata = bitmap + tile * 256 + chip * 8 + b * 32;
+ for (i = 0; i < 64; i++) {
+ tdata[i] <<= 1;
+ tdata[i] |= (bdata[i/8] >> (7 - i % 8)) & 0x01;
+ }
+ }
+ for (i = 0; i < 64; i++)
+ if (tdata[i] != vdata[i]) {
+ changed = 1;
+ vdata[i] = tdata[i];
+ }
+ return changed;
+}
+
+/* Reconfigure LCD display */
+static int picolcd_fb_reset(struct picolcd_data *data, int clear)
+{
+ struct hid_report *report = picolcd_out_report(REPORT_LCD_CMD, data->hdev);
+ int i, j;
+ unsigned long flags;
+ static const u8 mapcmd[8] = { 0x00, 0x02, 0x00, 0x64, 0x3f, 0x00, 0x64, 0xc0 };
+
+ if (!report || report->maxfield != 1)
+ return -ENODEV;
+
+ spin_lock_irqsave(&data->lock, flags);
+ for (i = 0; i < 4; i++) {
+ for (j = 0; j < report->field[0]->maxusage; j++)
+ if (j = 0)
+ hid_set_field(report->field[0], j, i << 2);
+ else if (j < sizeof(mapcmd))
+ hid_set_field(report->field[0], j, mapcmd[j]);
+ else
+ hid_set_field(report->field[0], j, 0);
+ usbhid_submit_report(data->hdev, report, USB_DIR_OUT);
+ }
+
+ data->status |= PICOLCD_READY_FB;
+ spin_unlock_irqrestore(&data->lock, flags);
+
+ if (clear && data->fb_bitmap) {
+ memset(data->fb_bitmap, 0, PICOLCDFB_SIZE);
+ }
+
+ /* schedule first output of framebuffer */
+ if (data->fb_info)
+ schedule_delayed_work(&data->fb_info->deferred_work, 0);
+
+ return 0;
+}
+
+/* Update fb_vbitmap from the screen_base and send changed tiles to device */
+static void picolcd_fb_update(struct picolcd_data *data)
+{
+ int chip, tile;
+ unsigned long flags;
+
+ spin_lock_irqsave(&data->lock, flags);
+ if (!(data->status & PICOLCD_READY_FB)) {
+ spin_unlock_irqrestore(&data->lock, flags);
+ picolcd_fb_reset(data, 0);
+ } else {
+ spin_unlock_irqrestore(&data->lock, flags);
+ }
+
+ /*
+ * Translate the XBM format screen_base into the format needed by the
+ * PicoLCD. See display layout above.
+ * Do this one tile after the other and push those tiles that changed.
+ */
+ for (chip = 0; chip < 4; chip++)
+ for (tile = 0; tile < 8; tile++)
+ if (picolcd_fb_update_tile(data->fb_vbitmap,
+ data->fb_bitmap, chip, tile))
+ picolcd_fb_send_tile(data->hdev, chip, tile);
+}
+
+/* Stub to call the system default and update the image on the picoLCD */
+static void picolcd_fb_fillrect(struct fb_info *info,
+ const struct fb_fillrect *rect)
+{
+ if (!info->par)
+ return;
+ sys_fillrect(info, rect);
+
+ schedule_delayed_work(&info->deferred_work, 0);
+}
+
+/* Stub to call the system default and update the image on the picoLCD */
+static void picolcd_fb_copyarea(struct fb_info *info,
+ const struct fb_copyarea *area)
+{
+ if (!info->par)
+ return;
+ sys_copyarea(info, area);
+
+ schedule_delayed_work(&info->deferred_work, 0);
+}
+
+/* Stub to call the system default and update the image on the picoLCD */
+static void picolcd_fb_imageblit(struct fb_info *info, const struct fb_image *image)
+{
+ if (!info->par)
+ return;
+ sys_imageblit(info, image);
+
+ schedule_delayed_work(&info->deferred_work, 0);
+}
+
+/*
+ * 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 picolcd_fb_write(struct fb_info *info, const char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ ssize_t ret;
+ if (!info->par)
+ return -ENODEV;
+ ret = fb_sys_write(info, buf, count, ppos);
+ if (ret >= 0)
+ schedule_delayed_work(&info->deferred_work, 0);
+ return ret;
+}
+
+static int picolcd_fb_blank(int blank, struct fb_info *info)
+{
+ if (!info->par)
+ return -ENODEV;
+ /* We let fb notification do this for us via lcd/backlight device */
+ return 0;
+}
+
+static void picolcd_fb_destroy(struct fb_info *info)
+{
+ struct picolcd_data *data = info->par;
+ info->par = NULL;
+ if (data)
+ data->fb_info = NULL;
+ fb_deferred_io_cleanup(info);
+ framebuffer_release(info);
+}
+
+/* Note this can't be const because of struct fb_info definition */
+static struct fb_ops picolcdfb_ops = {
+ .owner = THIS_MODULE,
+ .fb_destroy = picolcd_fb_destroy,
+ .fb_read = fb_sys_read,
+ .fb_write = picolcd_fb_write,
+ .fb_blank = picolcd_fb_blank,
+ .fb_fillrect = picolcd_fb_fillrect,
+ .fb_copyarea = picolcd_fb_copyarea,
+ .fb_imageblit = picolcd_fb_imageblit,
+};
+
+
+/* Callback from deferred IO workqueue */
+static void picolcd_fb_deferred_io(struct fb_info *info, struct list_head *pagelist)
+{
+ picolcd_fb_update(info->par);
+}
+
+static const struct fb_deferred_io picolcd_fb_defio = {
+ .delay = HZ / PICOLCDFB_UPDATE_RATE_DEFAULT,
+ .deferred_io = picolcd_fb_deferred_io,
+};
+
+
+/*
+ * The "fb_update_rate" sysfs attribute
+ */
+static ssize_t picolcd_fb_update_rate_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct picolcd_data *data = dev_get_drvdata(dev);
+ unsigned fb_update_rate = data->fb_update_rate;
+
+ return snprintf(buf, PAGE_SIZE, "%u (1..%u)\n", fb_update_rate,
+ PICOLCDFB_UPDATE_RATE_LIMIT);
+}
+
+static ssize_t picolcd_fb_update_rate_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct picolcd_data *data = dev_get_drvdata(dev);
+ int i;
+ unsigned u;
+
+ if (count < 1 || count > 10)
+ return -EINVAL;
+
+ i = sscanf(buf, "%u", &u);
+ if (i != 1)
+ return -EINVAL;
+
+ if (u > PICOLCDFB_UPDATE_RATE_LIMIT)
+ return -ERANGE;
+ else if (u = 0)
+ u = PICOLCDFB_UPDATE_RATE_DEFAULT;
+
+ data->fb_update_rate = u;
+ data->fb_defio.delay = HZ / data->fb_update_rate;
+ return count;
+}
+
+static DEVICE_ATTR(fb_update_rate, 0666, picolcd_fb_update_rate_show,
+ picolcd_fb_update_rate_store);
+
+/* initialize Framebuffer device */
+static inline int picolcd_init_framebuffer(struct picolcd_data *data)
+{
+ struct device *dev = &data->hdev->dev;
+ struct fb_info *info = NULL;
+ int error = -ENOMEM;
+ u8 *fb_vbitmap = NULL;
+ u8 *fb_bitmap = NULL;
+
+ fb_bitmap = vmalloc(PICOLCDFB_SIZE);
+ if (fb_bitmap = NULL) {
+ dev_err(dev, "can't get a free page for framebuffer\n");
+ goto err_nomem;
+ }
+
+ fb_vbitmap = kmalloc(PICOLCDFB_SIZE, GFP_KERNEL);
+ if (fb_vbitmap = NULL) {
+ dev_err(dev, "can't alloc vbitmap image buffer\n");
+ goto err_nomem;
+ }
+
+ data->fb_update_rate = PICOLCDFB_UPDATE_RATE_DEFAULT;
+ data->fb_defio = picolcd_fb_defio;
+ info = framebuffer_alloc(0, dev);
+ if (info = NULL) {
+ dev_err(dev, "failed to allocate a framebuffer\n");
+ goto err_nomem;
+ }
+
+ info->fbdefio = &data->fb_defio;
+ info->screen_base = (char __force __iomem *)fb_bitmap;
+ info->fbops = &picolcdfb_ops;
+ info->var = picolcdfb_var;
+ info->fix = picolcdfb_fix;
+ info->fix.smem_len = PICOLCDFB_SIZE;
+ info->par = data;
+ info->flags = FBINFO_FLAG_DEFAULT;
+
+ data->fb_vbitmap = fb_vbitmap;
+ data->fb_bitmap = fb_bitmap;
+ error = picolcd_fb_reset(data, 1);
+ if (error) {
+ dev_err(dev, "failed to configure display\n");
+ goto err_cleanup;
+ }
+ error = sysfs_create_file(&dev->kobj, &dev_attr_fb_update_rate.attr);
+ if (error) {
+ dev_err(dev, "failed to create sysfs attributes\n");
+ goto err_cleanup;
+ }
+ data->fb_info = info;
+ error = register_framebuffer(info);
+ if (error) {
+ dev_err(dev, "failed to register framebuffer\n");
+ goto err_sysfs;
+ }
+ fb_deferred_io_init(info);
+ /* schedule first output of framebuffer */
+ schedule_delayed_work(&info->deferred_work, 0);
+ return 0;
+
+err_sysfs:
+ sysfs_remove_file(&dev->kobj, &dev_attr_fb_update_rate.attr);
+err_cleanup:
+ data->fb_vbitmap = NULL;
+ data->fb_bitmap = NULL;
+ data->fb_info = NULL;
+
+err_nomem:
+ framebuffer_release(info);
+ vfree(fb_bitmap);
+ kfree(fb_vbitmap);
+ return error;
+}
+
+static void picolcd_exit_framebuffer(struct picolcd_data *data)
+{
+ struct fb_info *info = data->fb_info;
+ u8 *fb_vbitmap = data->fb_vbitmap;
+ u8 *fb_bitmap = data->fb_bitmap;
+
+ if (!info)
+ return;
+
+ data->fb_vbitmap = NULL;
+ data->fb_bitmap = NULL;
+ data->fb_info = NULL;
+ sysfs_remove_file(&(data->hdev->dev.kobj), &dev_attr_fb_update_rate.attr);
+ fb_deferred_io_cleanup(info);
+ unregister_framebuffer(info);
+ vfree(fb_bitmap);
+ kfree(fb_vbitmap);
+}
+
+
+#else
+static inline int picolcd_fb_reset(struct picolcd_data *data, int clear)
+{
+ return 0;
+}
+static inline int picolcd_init_framebuffer(struct picolcd_data *data)
+{
+ return 0;
+}
+static void picolcd_exit_framebuffer(struct picolcd_data *data)
+{
+}
+#endif /* CONFIG_FB */
+
/*
* input class device
*/
@@ -337,6 +775,11 @@ static int picolcd_reset(struct hid_device *hdev)
return -EBUSY;
}
+#if defined(CONFIG_FB) || defined(CONFIG_FB_MODULE)
+ if (data->fb_info)
+ schedule_delayed_work(&data->fb_info->deferred_work, 0);
+#endif /* CONFIG_FB */
+
return 0;
}
@@ -937,6 +1380,9 @@ static int picolcd_reset_resume(struct hid_device *hdev)
ret = picolcd_reset(hdev);
if (ret)
dbg_hid(PICOLCD_NAME " resetting our device failed: %d\n", ret);
+ ret = picolcd_fb_reset(hid_get_drvdata(hdev), 0);
+ if (ret)
+ dbg_hid(PICOLCD_NAME " restoring framebuffer content failed: %d\n", ret);
return 0;
}
#endif
@@ -1053,6 +1499,11 @@ static inline int picolcd_probe_lcd(struct hid_device *hdev, struct picolcd_data
if (error)
goto err;
+ /* Set up the framebuffer device */
+ error = picolcd_init_framebuffer(data);
+ if (error)
+ goto err;
+
#ifdef CONFIG_DEBUG_FS
report = picolcd_out_report(REPORT_READ_MEMORY, hdev);
if (report && report->maxfield = 1 && report->field[0]->report_size = 8)
@@ -1062,6 +1513,7 @@ static inline int picolcd_probe_lcd(struct hid_device *hdev, struct picolcd_data
#endif
return 0;
err:
+ picolcd_exit_framebuffer(data);
picolcd_exit_cir(data);
picolcd_exit_keys(data);
return error;
@@ -1200,6 +1652,8 @@ static void picolcd_remove(struct hid_device *hdev)
hdev->ll_driver->close(hdev);
hid_hw_stop(hdev);
+ /* Clean up the framebuffer */
+ picolcd_exit_framebuffer(data);
/* Cleanup input */
picolcd_exit_cir(data);
picolcd_exit_keys(data);
--
1.6.4.4
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 3/6] hid: add backlight support to PicoLCD device
2010-03-20 16:00 [PATCH v2 0/6] hid: new driver for PicoLCD device Bruno Prémont
[not found] ` <20100320170014.440959a8-hY15tx4IgV39zxVx7UNMDg@public.gmane.org>
@ 2010-03-20 16:06 ` Bruno Prémont
2010-03-22 8:59 ` Jiri Kosina
2010-03-20 16:08 ` [PATCH v2 4/6] hid: add lcd " Bruno Prémont
` (2 subsequent siblings)
4 siblings, 1 reply; 20+ messages in thread
From: Bruno Prémont @ 2010-03-20 16:06 UTC (permalink / raw)
To: Jiri Kosina
Cc: linux-input, linux-usb, linux-fbdev, linux-kernel,
Rick L. Vinyard Jr.,
Nicu Pavel, Oliver Neukum, Jaya Kumar, Richard Purdie
Add backlight support to PicoLCD device.
Backlight support depends on backlight class and is only being
compiled if backlight class has been selected.
Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
---
drivers/hid/Kconfig | 2 +-
drivers/hid/hid-picolcd.c | 134 ++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 134 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index a474bcd..5ec3cb7 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -243,8 +243,8 @@ config HID_PICOLCD
- Keypad
- Switching between Firmware and Flash mode
- Framebuffer for monochrome 256x64 display
+ - Backlight control (needs CONFIG_BACKLIGHT_CLASS_DEVICE)
Features that are not (yet) supported:
- - Backlight control
- Contrast control
- IR
- General purpose outputs
diff --git a/drivers/hid/hid-picolcd.c b/drivers/hid/hid-picolcd.c
index 0c1d293..06cf099 100644
--- a/drivers/hid/hid-picolcd.c
+++ b/drivers/hid/hid-picolcd.c
@@ -26,6 +26,7 @@
#include <linux/fb.h>
#include <linux/vmalloc.h>
+#include <linux/backlight.h>
#include <linux/seq_file.h>
#include <linux/debugfs.h>
@@ -182,6 +183,11 @@ struct picolcd_data {
struct fb_info *fb_info;
struct fb_deferred_io fb_defio;
#endif /* CONFIG_FB */
+#if defined(CONFIG_BACKLIGHT_CLASS_DEVICE) || defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE)
+ struct backlight_device *backlight;
+ u8 lcd_brightness;
+ u8 lcd_power;
+#endif /* CONFIG_BACKLIGHT_CLASS_DEVICE */
/* Housekeeping stuff */
spinlock_t lock;
@@ -641,7 +647,7 @@ static void picolcd_exit_framebuffer(struct picolcd_data *data)
kfree(fb_vbitmap);
}
-
+#define picolcd_fbinfo(d) (d)->fb_info
#else
static inline int picolcd_fb_reset(struct picolcd_data *data, int clear)
{
@@ -654,8 +660,118 @@ static inline int picolcd_init_framebuffer(struct picolcd_data *data)
static void picolcd_exit_framebuffer(struct picolcd_data *data)
{
}
+#define picolcd_fbinfo(d) NULL
#endif /* CONFIG_FB */
+#if defined(CONFIG_BACKLIGHT_CLASS_DEVICE) || defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE)
+/*
+ * backlight class device
+ */
+static int picolcd_get_brightness(struct backlight_device *bdev)
+{
+ struct picolcd_data *data = bl_get_data(bdev);
+ return data->lcd_brightness;
+}
+
+static int picolcd_set_brightness(struct backlight_device *bdev)
+{
+ struct picolcd_data *data = bl_get_data(bdev);
+ struct hid_report *report = picolcd_out_report(REPORT_BRIGHTNESS, data->hdev);
+ unsigned long flags;
+
+ if (!report || report->maxfield != 1 || report->field[0]->report_count != 1)
+ return -ENODEV;
+
+ data->lcd_brightness = bdev->props.brightness & 0x0ff;
+ data->lcd_power = bdev->props.power;
+ spin_lock_irqsave(&data->lock, flags);
+ hid_set_field(report->field[0], 0, data->lcd_power = FB_BLANK_UNBLANK ? data->lcd_brightness : 0);
+ usbhid_submit_report(data->hdev, report, USB_DIR_OUT);
+ spin_unlock_irqrestore(&data->lock, flags);
+ return 0;
+}
+
+static int picolcd_check_bl_fb(struct backlight_device *bdev, struct fb_info *fb)
+{
+ return fb && fb = picolcd_fbinfo((struct picolcd_data *)bl_get_data(bdev));
+}
+
+static const struct backlight_ops picolcd_blops = {
+ .update_status = picolcd_set_brightness,
+ .get_brightness = picolcd_get_brightness,
+ .check_fb = picolcd_check_bl_fb,
+};
+
+static inline int picolcd_init_backlight(struct picolcd_data *data, struct hid_report *report)
+{
+ struct device *dev = &data->hdev->dev;
+ struct backlight_device *bdev;
+ struct backlight_properties props;
+ if (!report)
+ return -ENODEV;
+ if (report->maxfield != 1 || report->field[0]->report_count != 1 ||
+ report->field[0]->report_size != 8) {
+ dev_err(dev, "unsupported BRIGHTNESS report");
+ return -EINVAL;
+ }
+
+ memset(&props, 0, sizeof(props));
+ props.max_brightness = 0xff;
+ bdev = backlight_device_register(dev_name(dev), dev, data, &picolcd_blops, &props);
+ if (IS_ERR(bdev)) {
+ dev_err(dev, "failed to register backlight\n");
+ return PTR_ERR(bdev);
+ }
+ bdev->props.brightness = 0xff;
+ data->lcd_brightness = 0xff;
+ data->backlight = bdev;
+ picolcd_set_brightness(bdev);
+ return 0;
+}
+
+static void picolcd_exit_backlight(struct picolcd_data *data)
+{
+ struct backlight_device *bdev = data->backlight;
+
+ data->backlight = NULL;
+ if (bdev)
+ backlight_device_unregister(bdev);
+}
+
+static inline int picolcd_resume_backlight(struct picolcd_data *data)
+{
+ if (!data->backlight)
+ return 0;
+ return picolcd_set_brightness(data->backlight);
+}
+
+static void picolcd_suspend_backlight(struct picolcd_data *data)
+{
+ int bl_power = data->lcd_power;
+ if (!data->backlight)
+ return;
+
+ data->backlight->props.power = FB_BLANK_POWERDOWN;
+ picolcd_set_brightness(data->backlight);
+ data->lcd_power = data->backlight->props.power = bl_power;
+}
+#else
+static inline int picolcd_init_backlight(struct picolcd_data *data, struct hid_report *report)
+{
+ return 0;
+}
+static inline void picolcd_exit_backlight(struct picolcd_data *data)
+{
+}
+static inline int picolcd_resume_backlight(struct picolcd_data *data)
+{
+ return 0;
+}
+static void picolcd_suspend_backlight(struct picolcd_data *data)
+{
+}
+#endif /* CONFIG_BACKLIGHT_CLASS_DEVICE */
+
/*
* input class device
*/
@@ -775,6 +891,7 @@ static int picolcd_reset(struct hid_device *hdev)
return -EBUSY;
}
+ picolcd_resume_backlight(data);
#if defined(CONFIG_FB) || defined(CONFIG_FB_MODULE)
if (data->fb_info)
schedule_delayed_work(&data->fb_info->deferred_work, 0);
@@ -1365,12 +1482,17 @@ static int picolcd_raw_event(struct hid_device *hdev,
#ifdef CONFIG_PM
static int picolcd_suspend(struct hid_device *hdev)
{
+ picolcd_suspend_backlight(hid_get_drvdata(hdev));
dbg_hid(PICOLCD_NAME " device ready for suspend\n");
return 0;
}
static int picolcd_resume(struct hid_device *hdev)
{
+ int ret;
+ ret = picolcd_resume_backlight(hid_get_drvdata(hdev));
+ if (ret)
+ dbg_hid(PICOLCD_NAME " restoring backlight failed: %d\n", ret);
return 0;
}
@@ -1383,6 +1505,9 @@ static int picolcd_reset_resume(struct hid_device *hdev)
ret = picolcd_fb_reset(hid_get_drvdata(hdev), 0);
if (ret)
dbg_hid(PICOLCD_NAME " restoring framebuffer content failed: %d\n", ret);
+ ret = picolcd_resume_backlight(hid_get_drvdata(hdev));
+ if (ret)
+ dbg_hid(PICOLCD_NAME " restoring backlight failed: %d\n", ret);
return 0;
}
#endif
@@ -1504,6 +1629,11 @@ static inline int picolcd_probe_lcd(struct hid_device *hdev, struct picolcd_data
if (error)
goto err;
+ /* Setup backlight class device */
+ error = picolcd_init_backlight(data, picolcd_out_report(REPORT_BRIGHTNESS, hdev));
+ if (error)
+ goto err;
+
#ifdef CONFIG_DEBUG_FS
report = picolcd_out_report(REPORT_READ_MEMORY, hdev);
if (report && report->maxfield = 1 && report->field[0]->report_size = 8)
@@ -1513,6 +1643,7 @@ static inline int picolcd_probe_lcd(struct hid_device *hdev, struct picolcd_data
#endif
return 0;
err:
+ picolcd_exit_backlight(data);
picolcd_exit_framebuffer(data);
picolcd_exit_cir(data);
picolcd_exit_keys(data);
@@ -1653,6 +1784,7 @@ static void picolcd_remove(struct hid_device *hdev)
hid_hw_stop(hdev);
/* Clean up the framebuffer */
+ picolcd_exit_backlight(data);
picolcd_exit_framebuffer(data);
/* Cleanup input */
picolcd_exit_cir(data);
--
1.6.4.4
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 4/6] hid: add lcd support to PicoLCD device
2010-03-20 16:00 [PATCH v2 0/6] hid: new driver for PicoLCD device Bruno Prémont
[not found] ` <20100320170014.440959a8-hY15tx4IgV39zxVx7UNMDg@public.gmane.org>
2010-03-20 16:06 ` [PATCH v2 3/6] hid: add backlight " Bruno Prémont
@ 2010-03-20 16:08 ` Bruno Prémont
2010-03-20 16:10 ` [PATCH v2 5/6] hid: add GPO (leds) " Bruno Prémont
2010-03-20 16:11 ` [PATCH v2 6/6] hid: add experimental access to PicoLCD device's Bruno Prémont
4 siblings, 0 replies; 20+ messages in thread
From: Bruno Prémont @ 2010-03-20 16:08 UTC (permalink / raw)
To: Jiri Kosina
Cc: linux-input, linux-usb, linux-fbdev, linux-kernel,
Rick L. Vinyard Jr.,
Nicu Pavel, Oliver Neukum, Jaya Kumar
Add lcd support to PicoLCD device.
LCD support depends on lcd class and is only being
compiled if lcd class has been selected.
Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
---
drivers/hid/Kconfig | 2 +-
drivers/hid/hid-picolcd.c | 108 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 109 insertions(+), 1 deletions(-)
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 5ec3cb7..b01de6d 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -244,8 +244,8 @@ config HID_PICOLCD
- Switching between Firmware and Flash mode
- Framebuffer for monochrome 256x64 display
- Backlight control (needs CONFIG_BACKLIGHT_CLASS_DEVICE)
+ - Contrast control (needs CONFIG_LCD_CLASS_DEVICE)
Features that are not (yet) supported:
- - Contrast control
- IR
- General purpose outputs
- EEProm / Flash access
diff --git a/drivers/hid/hid-picolcd.c b/drivers/hid/hid-picolcd.c
index 06cf099..88284ca 100644
--- a/drivers/hid/hid-picolcd.c
+++ b/drivers/hid/hid-picolcd.c
@@ -27,6 +27,7 @@
#include <linux/fb.h>
#include <linux/vmalloc.h>
#include <linux/backlight.h>
+#include <linux/lcd.h>
#include <linux/seq_file.h>
#include <linux/debugfs.h>
@@ -183,6 +184,10 @@ struct picolcd_data {
struct fb_info *fb_info;
struct fb_deferred_io fb_defio;
#endif /* CONFIG_FB */
+#if defined(CONFIG_LCD_CLASS_DEVICE) || defined(CONFIG_LCD_CLASS_DEVICE_MODULE)
+ struct lcd_device *lcd;
+ u8 lcd_contrast;
+#endif
#if defined(CONFIG_BACKLIGHT_CLASS_DEVICE) || defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE)
struct backlight_device *backlight;
u8 lcd_brightness;
@@ -772,6 +777,98 @@ static void picolcd_suspend_backlight(struct picolcd_data *data)
}
#endif /* CONFIG_BACKLIGHT_CLASS_DEVICE */
+#if defined(CONFIG_LCD_CLASS_DEVICE) || defined(CONFIG_LCD_CLASS_DEVICE_MODULE)
+/*
+ * lcd class device
+ */
+static int picolcd_get_contrast(struct lcd_device *ldev)
+{
+ struct picolcd_data *data = lcd_get_data(ldev);
+ return data->lcd_contrast;
+}
+
+static int picolcd_set_contrast(struct lcd_device *ldev, int contrast)
+{
+ struct picolcd_data *data = lcd_get_data(ldev);
+ struct hid_report *report = picolcd_out_report(REPORT_CONTRAST, data->hdev);
+ unsigned long flags;
+
+ if (!report || report->maxfield != 1 || report->field[0]->report_count != 1)
+ return -ENODEV;
+
+ data->lcd_contrast = contrast & 0x0ff;
+ spin_lock_irqsave(&data->lock, flags);
+ hid_set_field(report->field[0], 0, data->lcd_contrast);
+ usbhid_submit_report(data->hdev, report, USB_DIR_OUT);
+ spin_unlock_irqrestore(&data->lock, flags);
+ return 0;
+}
+
+static int picolcd_check_lcd_fb(struct lcd_device *ldev, struct fb_info *fb)
+{
+ return fb && fb = picolcd_fbinfo((struct picolcd_data *)lcd_get_data(ldev));
+}
+
+static struct lcd_ops picolcd_lcdops = {
+ .get_contrast = picolcd_get_contrast,
+ .set_contrast = picolcd_set_contrast,
+ .check_fb = picolcd_check_lcd_fb,
+};
+
+static inline int picolcd_init_lcd(struct picolcd_data *data, struct hid_report *report)
+{
+ struct device *dev = &data->hdev->dev;
+ struct lcd_device *ldev;
+
+ if (!report)
+ return -ENODEV;
+ if (report->maxfield != 1 || report->field[0]->report_count != 1 ||
+ report->field[0]->report_size != 8) {
+ dev_err(dev, "unsupported CONTRAST report");
+ return -EINVAL;
+ }
+
+ ldev = lcd_device_register(dev_name(dev), dev, data, &picolcd_lcdops);
+ if (IS_ERR(ldev)) {
+ dev_err(dev, "failed to register LCD\n");
+ return PTR_ERR(ldev);
+ }
+ ldev->props.max_contrast = 0x0ff;
+ data->lcd_contrast = 0xe5;
+ data->lcd = ldev;
+ picolcd_set_contrast(ldev, 0xe5);
+ return 0;
+}
+
+static void picolcd_exit_lcd(struct picolcd_data *data)
+{
+ struct lcd_device *ldev = data->lcd;
+
+ data->lcd = NULL;
+ if (ldev)
+ lcd_device_unregister(ldev);
+}
+
+static inline int picolcd_resume_lcd(struct picolcd_data *data)
+{
+ if (!data->lcd)
+ return 0;
+ return picolcd_set_contrast(data->lcd, data->lcd_contrast);
+}
+#else
+static inline int picolcd_init_lcd(struct picolcd_data *data, struct hid_report *report)
+{
+ return 0;
+}
+static inline void picolcd_exit_lcd(struct picolcd_data *data)
+{
+}
+static inline int picolcd_resume_lcd(struct picolcd_data *data)
+{
+ return 0;
+}
+#endif /* CONFIG_LCD_CLASS_DEVICE */
+
/*
* input class device
*/
@@ -891,6 +988,7 @@ static int picolcd_reset(struct hid_device *hdev)
return -EBUSY;
}
+ picolcd_resume_lcd(data);
picolcd_resume_backlight(data);
#if defined(CONFIG_FB) || defined(CONFIG_FB_MODULE)
if (data->fb_info)
@@ -1505,6 +1603,9 @@ static int picolcd_reset_resume(struct hid_device *hdev)
ret = picolcd_fb_reset(hid_get_drvdata(hdev), 0);
if (ret)
dbg_hid(PICOLCD_NAME " restoring framebuffer content failed: %d\n", ret);
+ ret = picolcd_resume_lcd(hid_get_drvdata(hdev));
+ if (ret)
+ dbg_hid(PICOLCD_NAME " restoring lcd failed: %d\n", ret);
ret = picolcd_resume_backlight(hid_get_drvdata(hdev));
if (ret)
dbg_hid(PICOLCD_NAME " restoring backlight failed: %d\n", ret);
@@ -1629,6 +1730,11 @@ static inline int picolcd_probe_lcd(struct hid_device *hdev, struct picolcd_data
if (error)
goto err;
+ /* Setup lcd class device */
+ error = picolcd_init_lcd(data, picolcd_out_report(REPORT_CONTRAST, hdev));
+ if (error)
+ goto err;
+
/* Setup backlight class device */
error = picolcd_init_backlight(data, picolcd_out_report(REPORT_BRIGHTNESS, hdev));
if (error)
@@ -1644,6 +1750,7 @@ static inline int picolcd_probe_lcd(struct hid_device *hdev, struct picolcd_data
return 0;
err:
picolcd_exit_backlight(data);
+ picolcd_exit_lcd(data);
picolcd_exit_framebuffer(data);
picolcd_exit_cir(data);
picolcd_exit_keys(data);
@@ -1785,6 +1892,7 @@ static void picolcd_remove(struct hid_device *hdev)
/* Clean up the framebuffer */
picolcd_exit_backlight(data);
+ picolcd_exit_lcd(data);
picolcd_exit_framebuffer(data);
/* Cleanup input */
picolcd_exit_cir(data);
--
1.6.4.4
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 5/6] hid: add GPO (leds) support to PicoLCD device
2010-03-20 16:00 [PATCH v2 0/6] hid: new driver for PicoLCD device Bruno Prémont
` (2 preceding siblings ...)
2010-03-20 16:08 ` [PATCH v2 4/6] hid: add lcd " Bruno Prémont
@ 2010-03-20 16:10 ` Bruno Prémont
2010-03-20 16:11 ` [PATCH v2 6/6] hid: add experimental access to PicoLCD device's Bruno Prémont
4 siblings, 0 replies; 20+ messages in thread
From: Bruno Prémont @ 2010-03-20 16:10 UTC (permalink / raw)
To: Jiri Kosina
Cc: linux-input, linux-usb, linux-fbdev, linux-kernel,
Rick L. Vinyard Jr.,
Nicu Pavel, Oliver Neukum, Jaya Kumar, Richard Purdie
Add leds support to PicoLCD device to drive the GPO pins.
GPO support depends on leds class and is only being
compiled if leds class has been selected.
Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
---
drivers/hid/Kconfig | 6 +-
drivers/hid/hid-picolcd.c | 163 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 166 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index b01de6d..90caaf3 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -243,11 +243,11 @@ config HID_PICOLCD
- Keypad
- Switching between Firmware and Flash mode
- Framebuffer for monochrome 256x64 display
- - Backlight control (needs CONFIG_BACKLIGHT_CLASS_DEVICE)
- - Contrast control (needs CONFIG_LCD_CLASS_DEVICE)
+ - Backlight control (needs CONFIG_BACKLIGHT_CLASS_DEVICE)
+ - Contrast control (needs CONFIG_LCD_CLASS_DEVICE)
+ - General purpose outputs (needs CONFIG_LEDS_CLASS)
Features that are not (yet) supported:
- IR
- - General purpose outputs
- EEProm / Flash access
config HID_SAMSUNG
diff --git a/drivers/hid/hid-picolcd.c b/drivers/hid/hid-picolcd.c
index 88284ca..f3d057c 100644
--- a/drivers/hid/hid-picolcd.c
+++ b/drivers/hid/hid-picolcd.c
@@ -29,6 +29,8 @@
#include <linux/backlight.h>
#include <linux/lcd.h>
+#include <linux/leds.h>
+
#include <linux/seq_file.h>
#include <linux/debugfs.h>
@@ -193,6 +195,11 @@ struct picolcd_data {
u8 lcd_brightness;
u8 lcd_power;
#endif /* CONFIG_BACKLIGHT_CLASS_DEVICE */
+#if defined(CONFIG_LEDS_CLASS) || defined(CONFIG_LEDS_CLASS_MODULE)
+ /* LED stuff */
+ u8 led_state;
+ struct led_classdev *led[8];
+#endif /* CONFIG_LEDS_CLASS */
/* Housekeeping stuff */
spinlock_t lock;
@@ -869,6 +876,152 @@ static inline int picolcd_resume_lcd(struct picolcd_data *data)
}
#endif /* CONFIG_LCD_CLASS_DEVICE */
+#if defined(CONFIG_LEDS_CLASS) || defined(CONFIG_LEDS_CLASS_MODULE)
+/**
+ * LED class device
+ */
+static void picolcd_leds_set(struct picolcd_data *data)
+{
+ struct hid_report *report;
+ unsigned long flags;
+
+ if (!data->led[0])
+ return;
+ report = picolcd_out_report(REPORT_LED_STATE, data->hdev);
+ if (!report || report->maxfield != 1 || report->field[0]->report_count != 1)
+ return;
+
+ spin_lock_irqsave(&data->lock, flags);
+ hid_set_field(report->field[0], 0, data->led_state);
+ usbhid_submit_report(data->hdev, report, USB_DIR_OUT);
+ spin_unlock_irqrestore(&data->lock, flags);
+}
+
+static void picolcd_led_set_brightness(struct led_classdev *led_cdev,
+ enum led_brightness value)
+{
+ struct device *dev;
+ struct hid_device *hdev;
+ struct picolcd_data *data;
+ int i, state = 0;
+
+ dev = led_cdev->dev->parent;
+ hdev = container_of(dev, struct hid_device, dev);
+ data = hid_get_drvdata(hdev);
+ for (i = 0; i < 8; i++) {
+ if (led_cdev != data->led[i])
+ continue;
+ state = (data->led_state >> i) & 1;
+ if (value = LED_OFF && state) {
+ data->led_state &= ~(1 << i);
+ picolcd_leds_set(data);
+ } else if (value != LED_OFF && !state) {
+ data->led_state |= 1 << i;
+ picolcd_leds_set(data);
+ }
+ break;
+ }
+}
+
+static enum led_brightness picolcd_led_get_brightness(struct led_classdev *led_cdev)
+{
+ struct device *dev;
+ struct hid_device *hdev;
+ struct picolcd_data *data;
+ int i, value = 0;
+
+ dev = led_cdev->dev->parent;
+ hdev = container_of(dev, struct hid_device, dev);
+ data = hid_get_drvdata(hdev);
+ for (i = 0; i < 8; i++)
+ if (led_cdev = data->led[i]) {
+ value = (data->led_state >> i) & 1;
+ break;
+ }
+ return value ? LED_FULL : LED_OFF;
+}
+
+static inline int picolcd_init_leds(struct picolcd_data *data, struct hid_report *report)
+{
+ struct device *dev = &data->hdev->dev;
+ struct led_classdev *led;
+ size_t name_sz = strlen(dev_name(dev)) + 8;
+ char *name;
+ int i, ret = 0;
+
+ if (!report)
+ return -ENODEV;
+ if (report->maxfield != 1 || report->field[0]->report_count != 1 ||
+ report->field[0]->report_size != 8) {
+ dev_err(dev, "unsupported LED_STATE report");
+ return -EINVAL;
+ }
+
+ for (i = 0; i < 8; i++) {
+ led = kzalloc(sizeof(struct led_classdev)+name_sz, GFP_KERNEL);
+ if (!led) {
+ dev_err(dev, "can't allocate memory for LED %d\n", i);
+ ret = -ENOMEM;
+ goto err;
+ }
+ name = (void *)(&led[1]);
+ snprintf(name, name_sz, "%s::GPO%d", dev_name(dev), i);
+ led->name = name;
+ led->brightness = 0;
+ led->max_brightness = 1;
+ led->brightness_get = picolcd_led_get_brightness;
+ led->brightness_set = picolcd_led_set_brightness;
+
+ data->led[i] = led;
+ ret = led_classdev_register(dev, data->led[i]);
+ if (ret) {
+ data->led[i] = NULL;
+ kfree(led);
+ dev_err(dev, "can't register LED %d\n", i);
+ goto err;
+ }
+ }
+ return 0;
+err:
+ for (i = 0; i < 8; i++)
+ if (data->led[i]) {
+ led = data->led[i];
+ data->led[i] = NULL;
+ led_classdev_unregister(led);
+ kfree(led);
+ }
+ return ret;
+}
+
+static void picolcd_exit_leds(struct picolcd_data *data)
+{
+ struct led_classdev *led;
+ int i;
+
+ for (i = 0; i < 8; i++) {
+ led = data->led[i];
+ data->led[i] = NULL;
+ if (!led)
+ continue;
+ led_classdev_unregister(led);
+ kfree(led);
+ }
+}
+
+#else
+static inline int picolcd_init_leds(struct picolcd_data *data, struct hid_report *report)
+{
+ return 0;
+}
+static void picolcd_exit_leds(struct picolcd_data *data)
+{
+}
+static inline int picolcd_leds_set(struct picolcd_data *data)
+{
+ return 0;
+}
+#endif /* CONFIG_LEDS_CLASS */
+
/*
* input class device
*/
@@ -995,6 +1148,7 @@ static int picolcd_reset(struct hid_device *hdev)
schedule_delayed_work(&data->fb_info->deferred_work, 0);
#endif /* CONFIG_FB */
+ picolcd_leds_set(data);
return 0;
}
@@ -1609,6 +1763,7 @@ static int picolcd_reset_resume(struct hid_device *hdev)
ret = picolcd_resume_backlight(hid_get_drvdata(hdev));
if (ret)
dbg_hid(PICOLCD_NAME " restoring backlight failed: %d\n", ret);
+ picolcd_leds_set(hid_get_drvdata(hdev));
return 0;
}
#endif
@@ -1740,6 +1895,11 @@ static inline int picolcd_probe_lcd(struct hid_device *hdev, struct picolcd_data
if (error)
goto err;
+ /* Setup the LED class devices */
+ error = picolcd_init_leds(data, picolcd_out_report(REPORT_LED_STATE, hdev));
+ if (error)
+ goto err;
+
#ifdef CONFIG_DEBUG_FS
report = picolcd_out_report(REPORT_READ_MEMORY, hdev);
if (report && report->maxfield = 1 && report->field[0]->report_size = 8)
@@ -1749,6 +1909,7 @@ static inline int picolcd_probe_lcd(struct hid_device *hdev, struct picolcd_data
#endif
return 0;
err:
+ picolcd_exit_leds(data);
picolcd_exit_backlight(data);
picolcd_exit_lcd(data);
picolcd_exit_framebuffer(data);
@@ -1890,6 +2051,8 @@ static void picolcd_remove(struct hid_device *hdev)
hdev->ll_driver->close(hdev);
hid_hw_stop(hdev);
+ /* Cleanup LED */
+ picolcd_exit_leds(data);
/* Clean up the framebuffer */
picolcd_exit_backlight(data);
picolcd_exit_lcd(data);
--
1.6.4.4
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 6/6] hid: add experimental access to PicoLCD device's
2010-03-20 16:00 [PATCH v2 0/6] hid: new driver for PicoLCD device Bruno Prémont
` (3 preceding siblings ...)
2010-03-20 16:10 ` [PATCH v2 5/6] hid: add GPO (leds) " Bruno Prémont
@ 2010-03-20 16:11 ` Bruno Prémont
2010-03-21 3:08 ` Dmitry Torokhov
4 siblings, 1 reply; 20+ messages in thread
From: Bruno Prémont @ 2010-03-20 16:11 UTC (permalink / raw)
To: Jiri Kosina
Cc: linux-input, linux-usb, linux-fbdev, linux-kernel,
Rick L. Vinyard Jr.,
Nicu Pavel, Oliver Neukum, Jaya Kumar
The PicoLCD device has a small amount of EEPROM and also provides
access to its FLASH where firmware and splash image are saved.
In flasher mode FLASH access is the only active feature.
Give read/write access to both via debugfs files.
Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
---
drivers/hid/Kconfig | 2 +-
drivers/hid/hid-picolcd.c | 428 +++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 412 insertions(+), 18 deletions(-)
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 90caaf3..2eec702 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -246,9 +246,9 @@ config HID_PICOLCD
- Backlight control (needs CONFIG_BACKLIGHT_CLASS_DEVICE)
- Contrast control (needs CONFIG_LCD_CLASS_DEVICE)
- General purpose outputs (needs CONFIG_LEDS_CLASS)
+ - EEProm / Flash access (via debugfs)
Features that are not (yet) supported:
- IR
- - EEProm / Flash access
config HID_SAMSUNG
tristate "Samsung" if EMBEDDED
diff --git a/drivers/hid/hid-picolcd.c b/drivers/hid/hid-picolcd.c
index f3d057c..3b9c7f4 100644
--- a/drivers/hid/hid-picolcd.c
+++ b/drivers/hid/hid-picolcd.c
@@ -169,6 +169,9 @@ struct picolcd_pending {
struct picolcd_data {
struct hid_device *hdev;
#ifdef CONFIG_DEBUG_FS
+ struct dentry *debug_reset;
+ struct dentry *debug_eeprom;
+ struct dentry *debug_flash;
int addr_sz;
#endif
u8 version[2];
@@ -1219,6 +1222,337 @@ static DEVICE_ATTR(operation_mode, 0644, picolcd_operation_mode_show,
#ifdef CONFIG_DEBUG_FS
/*
+ * The "reset" file
+ */
+static int picolcd_debug_reset_show(struct seq_file *f, void *p)
+{
+ if (picolcd_fbinfo((struct picolcd_data *)f->private))
+ seq_printf(f, "all fb\n");
+ else
+ seq_printf(f, "all\n");
+ return 0;
+}
+
+static ssize_t picolcd_debug_reset_open(struct inode *inode, struct file *f)
+{
+ return single_open(f, picolcd_debug_reset_show, inode->i_private);
+}
+
+static ssize_t picolcd_debug_reset_write(struct file *f, const char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct picolcd_data *data = ((struct seq_file *)f->private_data)->private;
+ char buf[32];
+ size_t cnt = min(count, sizeof(buf)-1);
+ if (copy_from_user(buf, user_buf, cnt))
+ return -EFAULT;
+
+ while (cnt > 0 && (buf[cnt-1] = ' ' || buf[cnt-1] = '\n'))
+ cnt--;
+ buf[cnt] = '\0';
+ if (strcmp(buf, "all") = 0) {
+ picolcd_reset(data->hdev);
+ picolcd_fb_reset(data, 1);
+ } else if (strcmp(buf, "fb") = 0) {
+ picolcd_fb_reset(data, 1);
+ } else {
+ return -EINVAL;
+ }
+ return count;
+}
+
+static const struct file_operations picolcd_debug_reset_fops = {
+ .owner = THIS_MODULE,
+ .open = picolcd_debug_reset_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .write = picolcd_debug_reset_write,
+ .release = single_release,
+};
+
+/*
+ * The "eeprom" file
+ */
+static int picolcd_debug_eeprom_open(struct inode *i, struct file *f)
+{
+ f->private_data = i->i_private;
+ return 0;
+}
+
+static ssize_t picolcd_debug_eeprom_read(struct file *f, char __user *u,
+ size_t s, loff_t *off)
+{
+ struct picolcd_data *data = f->private_data;
+ struct picolcd_pending *resp;
+ u8 raw_data[3];
+ ssize_t ret = -EIO;
+
+ if (s = 0)
+ return -EINVAL;
+ if (*off > 0x0ff)
+ return 0;
+
+ /* prepare buffer with info about what we want to read (addr & len) */
+ raw_data[0] = *off & 0xff;
+ raw_data[1] = (*off >> 8) && 0xff;
+ raw_data[2] = s < 20 ? s : 20;
+ if (*off + raw_data[2] > 0xff)
+ raw_data[2] = 0x100 - *off;
+ resp = picolcd_send_and_wait(data->hdev, REPORT_EE_READ, raw_data,
+ sizeof(raw_data));
+ if (!resp)
+ return -EIO;
+
+ if (resp->in_report && resp->in_report->id = REPORT_EE_DATA) {
+ /* successful read :) */
+ ret = resp->raw_data[2];
+ if (ret > s)
+ ret = s;
+ if (copy_to_user(u, resp->raw_data+3, ret))
+ ret = -EFAULT;
+ else
+ *off += ret;
+ } /* anything else is some kind of IO error */
+
+ kfree(resp);
+ return ret;
+}
+
+static ssize_t picolcd_debug_eeprom_write(struct file *f, const char __user *u,
+ size_t s, loff_t *off)
+{
+ struct picolcd_data *data = f->private_data;
+ struct picolcd_pending *resp;
+ ssize_t ret = -EIO;
+ u8 raw_data[23];
+
+ if (s = 0)
+ return -EINVAL;
+ if (*off > 0x0ff)
+ return -ENOSPC;
+
+ memset(raw_data, 0, sizeof(raw_data));
+ raw_data[0] = *off & 0xff;
+ raw_data[1] = (*off >> 8) && 0xff;
+ raw_data[2] = s < 20 ? s : 20;
+ if (*off + raw_data[2] > 0xff)
+ raw_data[2] = 0x100 - *off;
+
+ if (copy_from_user(raw_data+3, u, raw_data[2]))
+ return -EFAULT;
+ resp = picolcd_send_and_wait(data->hdev, REPORT_EE_WRITE, raw_data,
+ sizeof(raw_data));
+
+ if (!resp)
+ return -EIO;
+
+ if (resp->in_report && resp->in_report->id = REPORT_EE_DATA) {
+ /* check if written data matches */
+ if (memcmp(raw_data, resp->raw_data, 3+raw_data[2]) = 0) {
+ *off += raw_data[2];
+ ret = raw_data[2];
+ }
+ }
+ kfree(resp);
+ return ret;
+}
+
+static const struct file_operations picolcd_debug_eeprom_fops = {
+ .owner = THIS_MODULE,
+ .open = picolcd_debug_eeprom_open,
+ .read = picolcd_debug_eeprom_read,
+ .write = picolcd_debug_eeprom_write,
+ .llseek = generic_file_llseek,
+};
+
+/*
+ * The "flash" file
+ */
+static int picolcd_debug_flash_open(struct inode *i, struct file *f)
+{
+ f->private_data = i->i_private;
+ return 0;
+}
+
+/* record a flash address to buf (bounds check to be done by caller) */
+static int _picolcd_flash_setaddr(struct picolcd_data *data, u8 *buf, long off)
+{
+ buf[0] = off & 0xff;
+ buf[1] = (off >> 8) & 0xff;
+ if (data->addr_sz = 3)
+ buf[2] = (off >> 16) & 0xff;
+ return data->addr_sz = 2 ? 2 : 3;
+}
+
+/* read a given size of data (bounds check to be done by caller) */
+static ssize_t _picolcd_flash_read(struct picolcd_data *data, int report_id,
+ char __user *u, size_t s, loff_t *off)
+{
+ struct picolcd_pending *resp;
+ u8 raw_data[4];
+ ssize_t ret = 0;
+ int len_off, err = -EIO;
+
+ while (s > 0) {
+ err = -EIO;
+ len_off = _picolcd_flash_setaddr(data, raw_data, *off);
+ raw_data[len_off] = s > 32 ? 32 : s;
+ resp = picolcd_send_and_wait(data->hdev, report_id, raw_data, len_off+1);
+ if (!resp || !resp->in_report)
+ goto skip;
+ if (resp->in_report->id = REPORT_MEMORY ||
+ resp->in_report->id = REPORT_BL_READ_MEMORY) {
+ if (memcmp(raw_data, resp->raw_data, len_off+1) != 0)
+ goto skip;
+ if (copy_to_user(u+ret, resp->raw_data+len_off+1, raw_data[len_off])) {
+ err = -EFAULT;
+ goto skip;
+ }
+ *off += raw_data[len_off];
+ s -= raw_data[len_off];
+ ret += raw_data[len_off];
+ err = 0;
+ }
+skip:
+ kfree(resp);
+ if (err)
+ return ret > 0 ? ret : err;
+ }
+ return ret;
+}
+
+static ssize_t picolcd_debug_flash_read(struct file *f, char __user *u,
+ size_t s, loff_t *off)
+{
+ struct picolcd_data *data = f->private_data;
+
+ if (s = 0)
+ return -EINVAL;
+ if (*off > 0x05fff)
+ return 0;
+ if (*off + s > 0x05fff)
+ s = 0x06000 - *off;
+
+ if (data->status & PICOLCD_BOOTLOADER)
+ return _picolcd_flash_read(data, REPORT_BL_READ_MEMORY, u, s, off);
+ else
+ return _picolcd_flash_read(data, REPORT_READ_MEMORY, u, s, off);
+}
+
+/* erase block aligned to 64bytes boundary */
+static ssize_t _picolcd_flash_erase64(struct picolcd_data *data, int report_id,
+ loff_t *off)
+{
+ struct picolcd_pending *resp;
+ u8 raw_data[3];
+ int len_off;
+ ssize_t ret = -EIO;
+
+ if (*off & 0x3f)
+ return -EINVAL;
+
+ len_off = _picolcd_flash_setaddr(data, raw_data, *off);
+ resp = picolcd_send_and_wait(data->hdev, report_id, raw_data, len_off);
+ if (!resp || !resp->in_report)
+ goto skip;
+ if (resp->in_report->id = REPORT_MEMORY ||
+ resp->in_report->id = REPORT_BL_ERASE_MEMORY) {
+ if (memcmp(raw_data, resp->raw_data, len_off) != 0)
+ goto skip;
+ ret = 0;
+ }
+skip:
+ kfree(resp);
+ return ret;
+}
+
+/* write a given size of data (bounds check to be done by caller) */
+static ssize_t _picolcd_flash_write(struct picolcd_data *data, int report_id,
+ const char __user *u, size_t s, loff_t *off)
+{
+ struct picolcd_pending *resp;
+ u8 raw_data[36];
+ ssize_t ret = 0;
+ int len_off, err = -EIO;
+
+ while (s > 0) {
+ err = -EIO;
+ len_off = _picolcd_flash_setaddr(data, raw_data, *off);
+ raw_data[len_off] = s > 32 ? 32 : s;
+ if (copy_from_user(raw_data+len_off+1, u, raw_data[len_off])) {
+ err = -EFAULT;
+ goto skip;
+ }
+ resp = picolcd_send_and_wait(data->hdev, report_id, raw_data,
+ len_off+1+raw_data[len_off]);
+ if (!resp || !resp->in_report)
+ goto skip;
+ if (resp->in_report->id = REPORT_MEMORY ||
+ resp->in_report->id = REPORT_BL_WRITE_MEMORY) {
+ if (memcmp(raw_data, resp->raw_data, len_off+1+raw_data[len_off]) != 0)
+ goto skip;
+ *off += raw_data[len_off];
+ s -= raw_data[len_off];
+ ret += raw_data[len_off];
+ err = 0;
+ }
+skip:
+ kfree(resp);
+ if (err)
+ break;
+ }
+ return ret > 0 ? ret : err;
+}
+
+static ssize_t picolcd_debug_flash_write(struct file *f, const char __user *u,
+ size_t s, loff_t *off)
+{
+ struct picolcd_data *data = f->private_data;
+ ssize_t err, ret = 0;
+ int report_erase, report_write;
+
+ if (s = 0)
+ return -EINVAL;
+ if (*off > 0x5fff)
+ return -ENOSPC;
+ if (s & 0x3f)
+ return -EINVAL;
+ if (*off & 0x3f)
+ return -EINVAL;
+
+ if (data->status & PICOLCD_BOOTLOADER) {
+ report_erase = REPORT_BL_ERASE_MEMORY;
+ report_write = REPORT_BL_WRITE_MEMORY;
+ } else {
+ report_erase = REPORT_ERASE_MEMORY;
+ report_write = REPORT_WRITE_MEMORY;
+ }
+ while (s > 0) {
+ err = _picolcd_flash_erase64(data, report_erase, off);
+ if (err)
+ break;
+ err = _picolcd_flash_write(data, report_write, u, 64, off);
+ if (err < 0)
+ break;
+ ret += err;
+ *off += err;
+ s -= err;
+ if (err != 64)
+ break;
+ }
+ return ret > 0 ? ret : err;
+}
+
+static const struct file_operations picolcd_debug_flash_fops = {
+ .owner = THIS_MODULE,
+ .open = picolcd_debug_flash_open,
+ .read = picolcd_debug_flash_read,
+ .write = picolcd_debug_flash_write,
+ .llseek = generic_file_llseek,
+};
+
+
+/*
* Helper code for HID report level dumping/debugging
*/
static const char *error_codes[] = {
@@ -1689,9 +2023,76 @@ static inline void picolcd_debug_raw_event(struct picolcd_data *data,
wake_up_interruptible(&hdev->debug_wait);
kfree(buff);
}
+
+static inline int picolcd_init_devfs(struct picolcd_data *data,
+ struct hid_report *eeprom_r, struct hid_report *eeprom_w,
+ struct hid_report *flash_r, struct hid_report *flash_w,
+ struct hid_report *reset)
+{
+ struct hid_device *hdev = data->hdev;
+
+ /* reset */
+ if (reset)
+ data->debug_reset = debugfs_create_file("reset", 0600,
+ hdev->debug_dir, data, &picolcd_debug_reset_fops);
+
+ /* eeprom */
+ if (eeprom_r || eeprom_w)
+ data->debug_eeprom = debugfs_create_file("eeprom",
+ (eeprom_w ? S_IWUSR : 0) | (eeprom_r ? S_IRUSR : 0),
+ hdev->debug_dir, data, &picolcd_debug_eeprom_fops);
+ if (eeprom_r && (eeprom_r->maxfield != 1 ||
+ eeprom_r->field[0]->report_count != 3 ||
+ eeprom_r->field[0]->report_size != 8))
+ dev_warn(&hdev->dev, "Unsupported EEPROM data report (%d, %d, %d)\n",
+ eeprom_r->maxfield, eeprom_r->field[0]->report_count,
+ eeprom_r->field[0]->report_size);
+
+ /* flash */
+ if (flash_r && flash_r->maxfield = 1 && flash_r->field[0]->report_size = 8)
+ data->addr_sz = flash_r->field[0]->report_count - 1;
+ else
+ data->addr_sz = -1;
+ if (data->addr_sz = 2 || data->addr_sz = 3)
+ data->debug_flash = debugfs_create_file("flash",
+ (flash_w ? S_IWUSR : 0) | (flash_r ? S_IRUSR : 0),
+ hdev->debug_dir, data, &picolcd_debug_flash_fops);
+ else if (flash_r || flash_w)
+ dev_warn(&hdev->dev, "Unexpected FLASH access reports, "
+ "please submit rdesc for review\n");
+ return 0;
+}
+
+static void picolcd_exit_devfs(struct picolcd_data *data)
+{
+ struct dentry *dent;
+
+ dent = data->debug_reset;
+ data->debug_reset = NULL;
+ if (dent)
+ debugfs_remove(dent);
+ dent = data->debug_eeprom;
+ data->debug_eeprom = NULL;
+ if (dent)
+ debugfs_remove(dent);
+ dent = data->debug_flash;
+ data->debug_flash = NULL;
+ if (dent)
+ debugfs_remove(dent);
+}
#else
#define picolcd_debug_raw_event(data, hdev, report, raw_data, size)
-#endif
+static inline int picolcd_init_devfs(struct picolcd_data *data,
+ struct hid_report *eeprom_r, struct hid_report *eeprom_w,
+ struct hid_report *flash_r, struct hid_report *flash_w,
+ struct hid_report *reset)
+{
+ return 0;
+}
+static void picolcd_exit_devfs(struct picolcd_data *data)
+{
+}
+#endif /* CONFIG_DEBUG_FS */
/*
* Handle raw report as sent by device
@@ -1842,7 +2243,6 @@ static void picolcd_exit_cir(struct picolcd_data *data)
static inline int picolcd_probe_lcd(struct hid_device *hdev, struct picolcd_data *data)
{
struct picolcd_pending *verinfo;
- struct hid_report *report;
int error;
verinfo = picolcd_send_and_wait(hdev, REPORT_VERSION, NULL, 0);
@@ -1900,13 +2300,11 @@ static inline int picolcd_probe_lcd(struct hid_device *hdev, struct picolcd_data
if (error)
goto err;
-#ifdef CONFIG_DEBUG_FS
- report = picolcd_out_report(REPORT_READ_MEMORY, hdev);
- if (report && report->maxfield = 1 && report->field[0]->report_size = 8)
- data->addr_sz = report->field[0]->report_count - 1;
- else
- data->addr_sz = -1;
-#endif
+ picolcd_init_devfs(data, picolcd_out_report(REPORT_EE_READ, hdev),
+ picolcd_out_report(REPORT_EE_WRITE, hdev),
+ picolcd_out_report(REPORT_READ_MEMORY, hdev),
+ picolcd_out_report(REPORT_WRITE_MEMORY, hdev),
+ picolcd_out_report(REPORT_RESET, hdev));
return 0;
err:
picolcd_exit_leds(data);
@@ -1921,7 +2319,6 @@ err:
static inline int picolcd_probe_bootloader(struct hid_device *hdev, struct picolcd_data *data)
{
struct picolcd_pending *verinfo;
- struct hid_report *report;
verinfo = picolcd_send_and_wait(hdev, REPORT_VERSION, NULL, 0);
if (!verinfo || !verinfo->in_report) {
@@ -1948,13 +2345,9 @@ static inline int picolcd_probe_bootloader(struct hid_device *hdev, struct picol
return -ENODEV;
}
-#ifdef CONFIG_DEBUG_FS
- report = picolcd_out_report(REPORT_BL_READ_MEMORY, hdev);
- if (report && report->maxfield = 1 && report->field[0]->report_size = 8)
- data->addr_sz = report->field[0]->report_count - 1;
- else
- data->addr_sz = -1;
-#endif
+ picolcd_init_devfs(data, NULL, NULL,
+ picolcd_out_report(REPORT_BL_READ_MEMORY, hdev),
+ picolcd_out_report(REPORT_BL_WRITE_MEMORY, hdev), NULL);
return 0;
}
@@ -2047,6 +2440,7 @@ static void picolcd_remove(struct hid_device *hdev)
data->status |= PICOLCD_FAILED;
spin_unlock_irqrestore(&data->lock, flags);
+ picolcd_exit_devfs(data);
sysfs_remove_file(&(hdev->dev.kobj), &dev_attr_operation_mode.attr);
hdev->ll_driver->close(hdev);
hid_hw_stop(hdev);
--
1.6.4.4
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2 6/6] hid: add experimental access to PicoLCD device's
2010-03-20 16:11 ` [PATCH v2 6/6] hid: add experimental access to PicoLCD device's Bruno Prémont
@ 2010-03-21 3:08 ` Dmitry Torokhov
[not found] ` <20100321030802.GB29360-WlK9ik9hQGAhIp7JRqBPierSzoNAToWh@public.gmane.org>
0 siblings, 1 reply; 20+ messages in thread
From: Dmitry Torokhov @ 2010-03-21 3:08 UTC (permalink / raw)
To: Bruno Prémont
Cc: Jiri Kosina, linux-input, linux-usb, linux-fbdev, linux-kernel,
Rick L. Vinyard Jr.,
Nicu Pavel, Oliver Neukum, Jaya Kumar
On Sat, Mar 20, 2010 at 05:11:19PM +0100, Bruno Prémont wrote:
> The PicoLCD device has a small amount of EEPROM and also provides
> access to its FLASH where firmware and splash image are saved.
> In flasher mode FLASH access is the only active feature.
>
> Give read/write access to both via debugfs files.
>
It looks you are allowing multiple users access to these files. What
will happen if 2 processes try to write EEPROM at the same time?
> +
> +static inline int picolcd_init_devfs(struct picolcd_data *data,
> + struct hid_report *eeprom_r, struct hid_report *eeprom_w,
> + struct hid_report *flash_r, struct hid_report *flash_w,
> + struct hid_report *reset)
> +{
I don't think this should be forced inline.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/6] hid: add framebuffer support to PicoLCD device
2010-03-20 16:04 ` [PATCH v2 2/6] hid: add framebuffer support to " Bruno Prémont
@ 2010-03-21 3:25 ` Dmitry Torokhov
2010-03-21 7:24 ` Jaya Kumar
1 sibling, 0 replies; 20+ messages in thread
From: Dmitry Torokhov @ 2010-03-21 3:25 UTC (permalink / raw)
To: Bruno Prémont
Cc: Jiri Kosina, linux-input, linux-usb, linux-fbdev, linux-kernel,
Rick L. Vinyard Jr.,
Nicu Pavel, Oliver Neukum, Jaya Kumar
On Sat, Mar 20, 2010 at 05:04:15PM +0100, Bruno Prémont wrote:
> +static inline int picolcd_fb_send_tile(struct hid_device *hdev, int chip,
> + int tile)
> +{
You seemt o be fond of the 'inline; attribute ;) I don't think that a
40 lines function qualifies. Please lose inlines on functions that are
not in .h files and are not stubs.
--
Dmitry
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/6] hid: new driver for PicoLCD device
2010-03-20 16:02 ` [PATCH v2 1/6] " Bruno Prémont
@ 2010-03-21 3:46 ` Dmitry Torokhov
[not found] ` <20100321034600.GE29360-WlK9ik9hQGAhIp7JRqBPierSzoNAToWh@public.gmane.org>
0 siblings, 1 reply; 20+ messages in thread
From: Dmitry Torokhov @ 2010-03-21 3:46 UTC (permalink / raw)
To: Bruno Prémont
Cc: Jiri Kosina, linux-input, linux-usb, linux-fbdev, linux-kernel,
Rick L. Vinyard Jr.,
Nicu Pavel, Oliver Neukum, Jaya Kumar
On Sat, Mar 20, 2010 at 05:02:41PM +0100, Bruno Prémont wrote:
> +/* Input device
> + *
> + * The PicoLCD has an IR receiver header, a built-in keypad with 5 keys
> + * and header for 4x4 key matrix. The built-in keys are part of the matrix.
> + */
> +#define PICOLCD_KEYS 17
> +
> +static const int def_keymap[PICOLCD_KEYS] = {
def_keymap[] = {
...
};
#define PICOLCD_KEYS ARRAY_SIZE(def_keymap);
would be safe. Also unsigned short should cover it.
> + KEY_RESERVED, /* none */
> + KEY_BACK, /* col 4 + row 1 */
> + KEY_HOMEPAGE, /* col 3 + row 1 */
> + KEY_RESERVED, /* col 2 + row 1 */
> + KEY_RESERVED, /* col 1 + row 1 */
> + KEY_SCROLLUP, /* col 4 + row 2 */
> + KEY_OK, /* col 3 + row 2 */
> + KEY_SCROLLDOWN, /* col 2 + row 2 */
> + KEY_RESERVED, /* col 1 + row 2 */
> + KEY_RESERVED, /* col 4 + row 3 */
> + KEY_RESERVED, /* col 3 + row 3 */
> + KEY_RESERVED, /* col 2 + row 3 */
> + KEY_RESERVED, /* col 1 + row 3 */
> + KEY_RESERVED, /* col 4 + row 4 */
> + KEY_RESERVED, /* col 3 + row 4 */
> + KEY_RESERVED, /* col 2 + row 4 */
> + KEY_RESERVED, /* col 1 + row 4 */
> +};
> +
> +/* Description of in-progress IO operation, used for operations
> + * that trigger response from device */
> +struct picolcd_pending {
> + struct hid_report *out_report;
> + struct hid_report *in_report;
> + int raw_size;
> + u8 raw_data[64];
> +};
> +
> +/* Per device data structure */
> +struct picolcd_data {
> + struct hid_device *hdev;
> +#ifdef CONFIG_DEBUG_FS
> + int addr_sz;
> +#endif
> + u8 version[2];
> + /* input stuff */
> + u8 pressed_keys[2];
> + struct input_dev *input_keys;
> + struct input_dev *input_cir;
> + int keycode[PICOLCD_KEYS];
> +
> + /* Housekeeping stuff */
> + spinlock_t lock;
> + struct picolcd_pending *pending;
> + struct completion ready;
> + int status;
> +#define PICOLCD_BUSY 1
> +#define PICOLCD_FAILED 4
> +#define PICOLCD_BOOTLOADER 8
> +};
> +
> +
> +/* Find a given report */
> +#define picolcd_in_report(id, dev) picolcd_report(id, dev, HID_INPUT_REPORT)
> +#define picolcd_out_report(id, dev) picolcd_report(id, dev, HID_OUTPUT_REPORT)
> +
> +static struct hid_report *picolcd_report(int id, struct hid_device *hdev, int dir)
> +{
> + struct list_head *feature_report_list = &hdev->report_enum[dir].report_list;
> + struct hid_report *report = NULL;
> +
> + list_for_each_entry(report, feature_report_list, list) {
> + if (report->id = id)
> + return report;
> + }
> + dev_warn(&hdev->dev, "No report with id 0x%x found\n", id);
> + return NULL;
> +}
> +
> +#ifdef CONFIG_DEBUG_FS
> +static void picolcd_debug_out_report(struct picolcd_data *data,
> + struct hid_device *hdev, struct hid_report *report);
> +#define usbhid_submit_report(a, b, c) \
> + picolcd_debug_out_report(hid_get_drvdata(a), a, b); \
> + usbhid_submit_report(a, b, c)
> +#endif
> +
> +/* Submit a report and wait for a reply from device - if device fades away
> + * or does not respond in time, return NULL */
> +static struct picolcd_pending* picolcd_send_and_wait(struct hid_device *hdev,
> + int report_id, const u8 *raw_data, int size)
> +{
> + struct picolcd_data *data = hid_get_drvdata(hdev);
> + struct picolcd_pending *work;
> + struct hid_report *report = picolcd_out_report(report_id, hdev);
> + unsigned long flags;
> + int status, i, j, k;
> +
> + if (!report)
> + return NULL;
> + work = kzalloc(sizeof(*work), GFP_KERNEL);
> + if (!work)
> + return NULL;
> +
> + work->out_report = report;
> + work->in_report = NULL;
> + work->raw_size = 0;
> +
> +retry:
> + /* try to get lock and enqueue our job */
> + spin_lock_irqsave(&data->lock, flags);
> + status = data->status;
> + if (data->pending || (status & PICOLCD_FAILED)) {
> + /* some job already pending,
> + * wait for it to complete and try again */
> + spin_unlock_irqrestore(&data->lock, flags);
> + if (status & PICOLCD_FAILED) {
> + kfree(work);
> + return NULL;
> + }
> + wait_for_completion_interruptible_timeout(&data->ready, HZ*2);
> + goto retry;
Umm, can we do this with a mutex? Like you take a mutex and don't
release till you are done talking to the device. So that other guy will
wait on the mutex instead of waking up and rechecking condition.
> + }
> + data->status |= PICOLCD_BUSY;
> + data->pending = work;
> + for (i = k = 0; i < report->maxfield; i++)
> + for (j = 0; j < report->field[i]->report_count; j++) {
> + hid_set_field(report->field[i], j, k < size ? raw_data[k] : 0);
> + k++;
> + }
> + usbhid_submit_report(data->hdev, report, USB_DIR_OUT);
> + complete_all(&data->ready);
> + INIT_COMPLETION(data->ready);
Umm, what does this do, exactly?
> + spin_unlock_irqrestore(&data->lock, flags);
> + /* wait for our job to complete */
> + wait_for_completion_interruptible_timeout(&data->ready, HZ*2);
> +
> + spin_lock_irqsave(&data->lock, flags);
> + if (data->pending = work) {
> + data->status &= ~PICOLCD_BUSY;
> + data->pending = NULL;
> + complete_all(&data->ready);
> + spin_unlock_irqrestore(&data->lock, flags);
> + return work;
> + } else {
> + spin_unlock_irqrestore(&data->lock, flags);
> + kfree(work);
> + return NULL;
> + }
> +}
> +
> +/*
> + * input class device
> + */
> +static int picolcd_raw_keypad(struct hid_device *hdev,
> + struct hid_report *report, u8 *raw_data, int size)
> +{
> + /*
> + * Keypad event
> + * First and second data bytes list currently pressed keys,
> + * 0x00 means no key and at most 2 keys may be pressed at same time
> + */
> + struct picolcd_data *data = hid_get_drvdata(hdev);
> + int i, j;
> +
> + /* determine newly pressed keys */
> + for (i = 0; i < size; i++) {
> + int key_code;
> + if (raw_data[i] = 0)
> + continue;
> + for (j = 0; j < sizeof(data->pressed_keys); j++)
> + if (data->pressed_keys[j] = raw_data[i])
> + goto key_already_down;
> + for (j = 0; j < sizeof(data->pressed_keys); j++)
> + if (data->pressed_keys[j] = 0) {
> + data->pressed_keys[j] = raw_data[i];
> + break;
> + }
> + input_event(data->input_keys, EV_MSC, MSC_SCAN, raw_data[i]);
> + if (input_get_keycode(data->input_keys, raw_data[i], &key_code))
> + key_code = KEY_UNKNOWN;
Just get keycode directly from the driver's table, no need to jump through hoops
here,
> + if (key_code != KEY_UNKNOWN) {
> + dbg_hid(PICOLCD_NAME " got key press for %u:%d", raw_data[i], key_code);
> + input_report_key(data->input_keys, key_code, 1);
> + }
> + input_sync(data->input_keys);
> +key_already_down:
> + continue;
> + }
> +
> + /* determine newly released keys */
> + for (j = 0; j < sizeof(data->pressed_keys); j++) {
> + int key_code;
> + if (data->pressed_keys[j] = 0)
> + continue;
> + for (i = 0; i < size; i++)
> + if (data->pressed_keys[j] = raw_data[i])
> + goto key_still_down;
> + input_event(data->input_keys, EV_MSC, MSC_SCAN, data->pressed_keys[j]);
> + if (input_get_keycode(data->input_keys, data->pressed_keys[j], &key_code))
> + key_code = KEY_UNKNOWN;
> + if (key_code != KEY_UNKNOWN) {
> + dbg_hid(PICOLCD_NAME " got key release for %u:%d", data->pressed_keys[j], key_code);
> + input_report_key(data->input_keys, key_code, 0);
> + }
> + input_sync(data->input_keys);
> + data->pressed_keys[j] = 0;
> +key_still_down:
> + continue;
> + }
> + return 1;
> +}
> +
> +static int picolcd_raw_cir(struct hid_device *hdev,
> + struct hid_report *report, u8 *raw_data, int size)
> +{
> + /* Need understanding of CIR data format to implement ... */
> + return 1;
> +}
> +
> +
> +
> +/*
> + * Reset our device and wait for answer to VERSION request
> + */
> +static int picolcd_reset(struct hid_device *hdev)
> +{
> + struct picolcd_data *data = hid_get_drvdata(hdev);
> + struct hid_report *report = picolcd_out_report(REPORT_RESET, hdev);
> + struct picolcd_pending *verinfo;
> + unsigned long flags;
> +
> + if (!data || !report || report->maxfield != 1)
> + return -ENODEV;
> +
> + spin_lock_irqsave(&data->lock, flags);
> + complete_all(&data->ready);
> + INIT_COMPLETION(data->ready);
> + if (hdev->product = USB_DEVICE_ID_PICOLCD_BOOTLOADER)
> + data->status |= PICOLCD_BOOTLOADER;
> +
> + /* perform the reset */
> + hid_set_field(report->field[0], 0, 1);
> + usbhid_submit_report(hdev, report, USB_DIR_OUT);
> + spin_unlock_irqrestore(&data->lock, flags);
> +
> + verinfo = picolcd_send_and_wait(hdev, REPORT_VERSION, NULL, 0);
> + if (verinfo && verinfo->raw_size = 2) {
> + if (data->status & PICOLCD_BOOTLOADER) {
> + dev_info(&hdev->dev, "PicoLCD reset successful, bootloader version %d.%d\n",
> + verinfo->raw_data[0], verinfo->raw_data[1]);
> + data->version[0] = verinfo->raw_data[0];
> + data->version[1] = verinfo->raw_data[1];
> + } else {
> + dev_info(&hdev->dev, "PicoLCD reset successful, firmware version %d.%d\n",
> + verinfo->raw_data[1], verinfo->raw_data[0]);
> + data->version[0] = verinfo->raw_data[1];
> + data->version[1] = verinfo->raw_data[0];
> + }
> + kfree(verinfo);
> + verinfo = NULL;
> + } else if (verinfo) {
> + dev_err(&hdev->dev, "confused, got unexpected version response from PicoLCD after reset\n");
> + kfree(verinfo);
> + verinfo = NULL;
> + } else {
> + dev_err(&hdev->dev, "no version response from PicoLCD after reset");
> + return -EBUSY;
> + }
> +
I am pretty sure it could be written clearer instead of checking for
verinfo several times...
if (!verinfo) {
dev_err(..);
return -EBUSY;
}
if (verinfo->raw_size = 2) {
...
} else {
dev_err(...)
}
kfree(verinfo);
> + return 0;
> +}
> +
> +/*
> + * The "operation_mode" sysfs attribute
> + */
> +static ssize_t picolcd_operation_mode_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct picolcd_data *data = dev_get_drvdata(dev);
> +
> + if (data->status & PICOLCD_BOOTLOADER)
> + return snprintf(buf, PAGE_SIZE, "<bootloader> lcd\n");
> + else
> + return snprintf(buf, PAGE_SIZE, "bootloader <lcd>\n");
> +}
> +
> +static ssize_t picolcd_operation_mode_store(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t count)
> +{
> + struct picolcd_data *data = dev_get_drvdata(dev);
> + struct hid_report *report = NULL;
> + size_t cnt = count;
> + int timeout = 5000;
> + unsigned u;
> + unsigned long flags;
> +
> + if (cnt >= 3 && strncmp("lcd", buf, 3) = 0) {
> + if (data->status & PICOLCD_BOOTLOADER)
> + report = picolcd_out_report(REPORT_EXIT_FLASHER, data->hdev);
> + buf += 3;
> + cnt -= 3;
> + } else if (cnt >= 10 && strncmp("bootloader", buf, 10) = 0) {
> + if (!(data->status & PICOLCD_BOOTLOADER))
> + report = picolcd_out_report(REPORT_EXIT_KEYBOARD, data->hdev);
> + buf += 10;
> + cnt -= 10;
> + }
> + if (!report)
> + return -EINVAL;
> +
> + while (cnt > 0 && (*buf = ' ' || *buf = '\t')) {
> + buf++;
> + cnt--;
> + }
> + while (cnt > 0 && (buf[cnt-1] = '\n' || buf[cnt-1] = '\r'))
> + cnt--;
> + if (cnt > 0) {
> + if (sscanf(buf, "%u", &u) != 1)
> + return -EINVAL;
> + if (u > 30000)
> + return -EINVAL;
> + else
> + timeout = u;
> + }
> +
> + spin_lock_irqsave(&data->lock, flags);
> + hid_set_field(report->field[0], 0, timeout & 0xff);
> + hid_set_field(report->field[0], 1, (timeout >> 8) & 0xff);
> + usbhid_submit_report(data->hdev, report, USB_DIR_OUT);
> + spin_unlock_irqrestore(&data->lock, flags);
> + return count;
> +}
> +
> +static DEVICE_ATTR(operation_mode, 0644, picolcd_operation_mode_show,
> + picolcd_operation_mode_store);
> +
> +
> +#ifdef CONFIG_DEBUG_FS
> +/*
> + * Helper code for HID report level dumping/debugging
> + */
> +static const char *error_codes[] = {
> + "success", "parameter missing", "data_missing", "block readonly", "block not erasable",
> + "block too big", "section overflow", "invalid command length", "invalid data length",
> +};
> +
> +static void dump_buff_as_hex(char *dst, size_t dst_sz, const u8 *data,
> + const size_t data_len)
> +{
> + int i, j;
> + for (i = j = 0; i < data_len && j + 3 < dst_sz; i++) {
> + dst[j++] = hex_asc[(data[i] >> 4) & 0x0f];
> + dst[j++] = hex_asc[data[i] & 0x0f];
> + dst[j++] = ' ';
> + }
> + if (j < dst_sz) {
> + dst[j--] = '\0';
> + dst[j] = '\n';
> + } else
> + dst[j] = '\0';
> +}
> +
> +static void picolcd_debug_out_report(struct picolcd_data *data,
> + struct hid_device *hdev, struct hid_report *report)
> +{
> + u8 raw_data[70];
> + int raw_size = (report->size >> 3) + 1;
> + char *buff;
> +#define BUFF_SZ 256
> +
> + /* Avoid unnecessary overhead if debugfs is disabled */
> + if (!hdev->debug_events)
> + return;
> +
> + buff = kmalloc(BUFF_SZ, GFP_ATOMIC);
> + if (!buff)
> + return;
> +
> + snprintf(buff, BUFF_SZ, "\nout report %d (size %d) = ",
> + report->id, raw_size);
> + hid_debug_event(hdev, buff);
> + if (raw_size + 5 > sizeof(raw_data)) {
> + hid_debug_event(hdev, " TOO BIG\n");
> + return;
> + } else {
> + raw_data[0] = report->id;
> + hid_output_report(report, raw_data);
> + dump_buff_as_hex(buff, BUFF_SZ, raw_data, raw_size);
> + hid_debug_event(hdev, buff);
> + }
> +
> + switch (report->id) {
> + case REPORT_LED_STATE:
> + /* 1 data byte with GPO state */
> + snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
> + "REPORT_LED_STATE", report->id, raw_size-1);
> + hid_debug_event(hdev, buff);
> + snprintf(buff, BUFF_SZ, "\tGPO state: 0x%02x\n", raw_data[1]);
> + hid_debug_event(hdev, buff);
> + break;
> + case REPORT_BRIGHTNESS:
> + /* 1 data byte with brightness */
> + snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
> + "REPORT_BRIGHTNESS", report->id, raw_size-1);
> + hid_debug_event(hdev, buff);
> + snprintf(buff, BUFF_SZ, "\tBrightness: 0x%02x\n", raw_data[1]);
> + hid_debug_event(hdev, buff);
> + break;
> + case REPORT_CONTRAST:
> + /* 1 data byte with contrast */
> + snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
> + "REPORT_CONTRAST", report->id, raw_size-1);
> + hid_debug_event(hdev, buff);
> + snprintf(buff, BUFF_SZ, "\tContrast: 0x%02x\n", raw_data[1]);
> + hid_debug_event(hdev, buff);
> + break;
> + case REPORT_RESET:
> + /* 2 data bytes with reset duration in ms */
> + snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
> + "REPORT_RESET", report->id, raw_size-1);
> + hid_debug_event(hdev, buff);
> + snprintf(buff, BUFF_SZ, "\tDuration: 0x%02x%02x (%dms)\n",
> + raw_data[2], raw_data[1], raw_data[2] << 8 | raw_data[1]);
> + hid_debug_event(hdev, buff);
> + break;
> + case REPORT_LCD_CMD:
> + /* 63 data bytes with LCD commands */
> + snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
> + "REPORT_LCD_CMD", report->id, raw_size-1);
> + hid_debug_event(hdev, buff);
> + /* TODO: format decoding */
> + break;
> + case REPORT_LCD_DATA:
> + /* 63 data bytes with LCD data */
> + snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
> + "REPORT_LCD_CMD", report->id, raw_size-1);
> + /* TODO: format decoding */
> + hid_debug_event(hdev, buff);
> + break;
> + case REPORT_LCD_CMD_DATA:
> + /* 63 data bytes with LCD commands and data */
> + snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
> + "REPORT_LCD_CMD", report->id, raw_size-1);
> + /* TODO: format decoding */
> + hid_debug_event(hdev, buff);
> + break;
> + case REPORT_EE_READ:
> + /* 3 data bytes with read area description */
> + snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
> + "REPORT_EE_READ", report->id, raw_size-1);
> + hid_debug_event(hdev, buff);
> + snprintf(buff, BUFF_SZ, "\tData address: 0x%02x%02x\n", raw_data[2], raw_data[1]);
> + hid_debug_event(hdev, buff);
> + snprintf(buff, BUFF_SZ, "\tData length: %d\n", raw_data[3]);
> + hid_debug_event(hdev, buff);
> + break;
> + case REPORT_EE_WRITE:
> + /* 3+1..20 data bytes with write area description */
> + snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
> + "REPORT_EE_WRITE", report->id, raw_size-1);
> + hid_debug_event(hdev, buff);
> + snprintf(buff, BUFF_SZ, "\tData address: 0x%02x%02x\n", raw_data[2], raw_data[1]);
> + hid_debug_event(hdev, buff);
> + snprintf(buff, BUFF_SZ, "\tData length: %d\n", raw_data[3]);
> + hid_debug_event(hdev, buff);
> + if (raw_data[3] = 0) {
> + snprintf(buff, BUFF_SZ, "\tNo data\n");
> + } else if (raw_data[3] + 4 <= raw_size) {
> + snprintf(buff, BUFF_SZ, "\tData: ");
> + hid_debug_event(hdev, buff);
> + dump_buff_as_hex(buff, BUFF_SZ, raw_data+4, raw_data[3]);
> + } else {
> + snprintf(buff, BUFF_SZ, "\tData overflowed\n");
> + }
> + hid_debug_event(hdev, buff);
> + break;
> + case REPORT_ERASE_MEMORY:
> + case REPORT_BL_ERASE_MEMORY:
> + /* 3 data bytes with pointer inside erase block */
> + snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
> + "REPORT_ERASE_MEMORY", report->id, raw_size-1);
> + hid_debug_event(hdev, buff);
> + switch (data->addr_sz) {
> + case 2:
> + snprintf(buff, BUFF_SZ, "\tAddress inside 64 byte block: 0x%02x%02x\n",
> + raw_data[2], raw_data[1]);
> + break;
> + case 3:
> + snprintf(buff, BUFF_SZ, "\tAddress inside 64 byte block: 0x%02x%02x%02x\n",
> + raw_data[3], raw_data[2], raw_data[1]);
> + break;
> + default:
> + snprintf(buff, BUFF_SZ, "\tNot supported\n");
> + }
> + hid_debug_event(hdev, buff);
> + break;
> + case REPORT_READ_MEMORY:
> + case REPORT_BL_READ_MEMORY:
> + /* 4 data bytes with read area description */
> + snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
> + "REPORT_READ_MEMORY", report->id, raw_size-1);
> + hid_debug_event(hdev, buff);
> + switch (data->addr_sz) {
> + case 2:
> + snprintf(buff, BUFF_SZ, "\tData address: 0x%02x%02x\n",
> + raw_data[2], raw_data[1]);
> + hid_debug_event(hdev, buff);
> + snprintf(buff, BUFF_SZ, "\tData length: %d\n", raw_data[3]);
> + break;
> + case 3:
> + snprintf(buff, BUFF_SZ, "\tData address: 0x%02x%02x%02x\n",
> + raw_data[3], raw_data[2], raw_data[1]);
> + hid_debug_event(hdev, buff);
> + snprintf(buff, BUFF_SZ, "\tData length: %d\n", raw_data[4]);
> + break;
> + default:
> + snprintf(buff, BUFF_SZ, "\tNot supported\n");
> + }
> + hid_debug_event(hdev, buff);
> + break;
> + case REPORT_WRITE_MEMORY:
> + case REPORT_BL_WRITE_MEMORY:
> + /* 4+1..32 data bytes with write adrea description */
> + snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
> + "REPORT_WRITE_MEMORY", report->id, raw_size-1);
> + hid_debug_event(hdev, buff);
> + switch (data->addr_sz) {
> + case 2:
> + snprintf(buff, BUFF_SZ, "\tData address: 0x%02x%02x\n",
> + raw_data[2], raw_data[1]);
> + hid_debug_event(hdev, buff);
> + snprintf(buff, BUFF_SZ, "\tData length: %d\n", raw_data[3]);
> + hid_debug_event(hdev, buff);
> + if (raw_data[3] = 0) {
> + snprintf(buff, BUFF_SZ, "\tNo data\n");
> + } else if (raw_data[3] + 4 <= raw_size) {
> + snprintf(buff, BUFF_SZ, "\tData: ");
> + hid_debug_event(hdev, buff);
> + dump_buff_as_hex(buff, BUFF_SZ, raw_data+4, raw_data[3]);
> + } else {
> + snprintf(buff, BUFF_SZ, "\tData overflowed\n");
> + }
> + break;
> + case 3:
> + snprintf(buff, BUFF_SZ, "\tData address: 0x%02x%02x%02x\n",
> + raw_data[3], raw_data[2], raw_data[1]);
> + hid_debug_event(hdev, buff);
> + snprintf(buff, BUFF_SZ, "\tData length: %d\n", raw_data[4]);
> + hid_debug_event(hdev, buff);
> + if (raw_data[4] = 0) {
> + snprintf(buff, BUFF_SZ, "\tNo data\n");
> + } else if (raw_data[4] + 5 <= raw_size) {
> + snprintf(buff, BUFF_SZ, "\tData: ");
> + hid_debug_event(hdev, buff);
> + dump_buff_as_hex(buff, BUFF_SZ, raw_data+5, raw_data[4]);
> + } else {
> + snprintf(buff, BUFF_SZ, "\tData overflowed\n");
> + }
> + break;
> + default:
> + snprintf(buff, BUFF_SZ, "\tNot supported\n");
> + }
> + hid_debug_event(hdev, buff);
> + break;
> + case REPORT_SPLASH_RESTART:
> + /* TODO */
> + break;
> + case REPORT_EXIT_KEYBOARD:
> + snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
> + "REPORT_EXIT_KEYBOARD", report->id, raw_size-1);
> + hid_debug_event(hdev, buff);
> + snprintf(buff, BUFF_SZ, "\tRestart delay: %dms (0x%02x%02x)\n",
> + raw_data[1] | (raw_data[2] << 8),
> + raw_data[2], raw_data[1]);
> + hid_debug_event(hdev, buff);
> + break;
> + case REPORT_VERSION:
> + snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
> + "REPORT_VERSION", report->id, raw_size-1);
> + hid_debug_event(hdev, buff);
> + break;
> + case REPORT_DEVID:
> + snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
> + "REPORT_DEVID", report->id, raw_size-1);
> + hid_debug_event(hdev, buff);
> + break;
> + case REPORT_SPLASH_SIZE:
> + snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
> + "REPORT_SPLASH_SIZE", report->id, raw_size-1);
> + hid_debug_event(hdev, buff);
> + break;
> + case REPORT_HOOK_VERSION:
> + snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
> + "REPORT_HOOK_VERSION", report->id, raw_size-1);
> + hid_debug_event(hdev, buff);
> + break;
> + case REPORT_EXIT_FLASHER:
> + snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
> + "REPORT_VERSION", report->id, raw_size-1);
> + hid_debug_event(hdev, buff);
> + snprintf(buff, BUFF_SZ, "\tRestart delay: %dms (0x%02x%02x)\n",
> + raw_data[1] | (raw_data[2] << 8),
> + raw_data[2], raw_data[1]);
> + hid_debug_event(hdev, buff);
> + break;
> + default:
> + snprintf(buff, BUFF_SZ, "out report %s (%d, size=%d)\n",
> + "<unknown>", report->id, raw_size-1);
> + hid_debug_event(hdev, buff);
> + break;
> + }
> + wake_up_interruptible(&hdev->debug_wait);
> + kfree(buff);
> +}
> +
> +static inline void picolcd_debug_raw_event(struct picolcd_data *data,
> + struct hid_device *hdev, struct hid_report *report,
> + u8 *raw_data, int size)
> +{
> + char *buff;
> +
> +#define BUFF_SZ 256
> + /* Avoid unnecessary overhead if debugfs is disabled */
> + if (!hdev->debug_events)
> + return;
> +
> + buff = kmalloc(BUFF_SZ, GFP_ATOMIC);
> + if (!buff)
> + return;
> +
> + switch (report->id) {
> + case REPORT_ERROR_CODE:
> + /* 2 data bytes with affected report and error code */
> + snprintf(buff, BUFF_SZ, "report %s (%d, size=%d)\n",
> + "REPORT_ERROR_CODE", report->id, size-1);
> + hid_debug_event(hdev, buff);
> + if (raw_data[2] < ARRAY_SIZE(error_codes))
> + snprintf(buff, BUFF_SZ, "\tError code 0x%02x (%s) in reply to report 0x%02x\n",
> + raw_data[2], error_codes[raw_data[2]], raw_data[1]);
> + else
> + snprintf(buff, BUFF_SZ, "\tError code 0x%02x in reply to report 0x%02x\n",
> + raw_data[2], raw_data[1]);
> + hid_debug_event(hdev, buff);
> + break;
> + case REPORT_KEY_STATE:
> + /* 2 data bytes with key state */
> + snprintf(buff, BUFF_SZ, "report %s (%d, size=%d)\n",
> + "REPORT_KEY_STATE", report->id, size-1);
> + hid_debug_event(hdev, buff);
> + if (raw_data[1] = 0)
> + snprintf(buff, BUFF_SZ, "\tNo key pressed\n");
> + else if (raw_data[2] = 0)
> + snprintf(buff, BUFF_SZ, "\tOne key pressed: 0x%02x (%d)\n",
> + raw_data[1], raw_data[1]);
> + else
> + snprintf(buff, BUFF_SZ, "\tTwo keys pressed: 0x%02x (%d), 0x%02x (%d)\n",
> + raw_data[1], raw_data[1], raw_data[2], raw_data[2]);
> + hid_debug_event(hdev, buff);
> + break;
> + case REPORT_IR_DATA:
> + /* Up to 20 byes of IR scancode data */
> + snprintf(buff, BUFF_SZ, "report %s (%d, size=%d)\n",
> + "REPORT_IR_DATA", report->id, size-1);
> + hid_debug_event(hdev, buff);
> + if (raw_data[1] = 0) {
> + snprintf(buff, BUFF_SZ, "\tUnexpectedly 0 data length\n");
> + hid_debug_event(hdev, buff);
> + } else if (raw_data[1] + 1 <= size) {
> + snprintf(buff, BUFF_SZ, "\tData length: %d\n\tIR Data: ",
> + raw_data[1]-1);
> + hid_debug_event(hdev, buff);
> + dump_buff_as_hex(buff, BUFF_SZ, raw_data+2, raw_data[1]-1);
> + hid_debug_event(hdev, buff);
> + } else {
> + snprintf(buff, BUFF_SZ, "\tOverflowing data length: %d\n",
> + raw_data[1]-1);
> + hid_debug_event(hdev, buff);
> + }
> + break;
> + case REPORT_EE_DATA:
> + /* Data buffer in response to REPORT_EE_READ or REPORT_EE_WRITE */
> + snprintf(buff, BUFF_SZ, "report %s (%d, size=%d)\n",
> + "REPORT_EE_DATA", report->id, size-1);
> + hid_debug_event(hdev, buff);
> + snprintf(buff, BUFF_SZ, "\tData address: 0x%02x%02x\n", raw_data[2], raw_data[1]);
> + hid_debug_event(hdev, buff);
> + snprintf(buff, BUFF_SZ, "\tData length: %d\n", raw_data[3]);
> + hid_debug_event(hdev, buff);
> + if (raw_data[3] = 0) {
> + snprintf(buff, BUFF_SZ, "\tNo data\n");
> + hid_debug_event(hdev, buff);
> + } else if (raw_data[3] + 4 <= size) {
> + snprintf(buff, BUFF_SZ, "\tData: ");
> + hid_debug_event(hdev, buff);
> + dump_buff_as_hex(buff, BUFF_SZ, raw_data+4, raw_data[3]);
> + hid_debug_event(hdev, buff);
> + } else {
> + snprintf(buff, BUFF_SZ, "\tData overflowed\n");
> + hid_debug_event(hdev, buff);
> + }
> + break;
> + case REPORT_MEMORY:
> + /* Data buffer in response to REPORT_READ_MEMORY or REPORT_WRTIE_MEMORY */
> + snprintf(buff, BUFF_SZ, "report %s (%d, size=%d)\n",
> + "REPORT_MEMORY", report->id, size-1);
> + hid_debug_event(hdev, buff);
> + switch (data->addr_sz) {
> + case 2:
> + snprintf(buff, BUFF_SZ, "\tData address: 0x%02x%02x\n",
> + raw_data[2], raw_data[1]);
> + hid_debug_event(hdev, buff);
> + snprintf(buff, BUFF_SZ, "\tData length: %d\n", raw_data[3]);
> + hid_debug_event(hdev, buff);
> + if (raw_data[3] = 0) {
> + snprintf(buff, BUFF_SZ, "\tNo data\n");
> + } else if (raw_data[3] + 4 <= size) {
> + snprintf(buff, BUFF_SZ, "\tData: ");
> + hid_debug_event(hdev, buff);
> + dump_buff_as_hex(buff, BUFF_SZ, raw_data+4, raw_data[3]);
> + } else {
> + snprintf(buff, BUFF_SZ, "\tData overflowed\n");
> + }
> + break;
> + case 3:
> + snprintf(buff, BUFF_SZ, "\tData address: 0x%02x%02x%02x\n",
> + raw_data[3], raw_data[2], raw_data[1]);
> + hid_debug_event(hdev, buff);
> + snprintf(buff, BUFF_SZ, "\tData length: %d\n", raw_data[4]);
> + hid_debug_event(hdev, buff);
> + if (raw_data[4] = 0) {
> + snprintf(buff, BUFF_SZ, "\tNo data\n");
> + } else if (raw_data[4] + 5 <= size) {
> + snprintf(buff, BUFF_SZ, "\tData: ");
> + hid_debug_event(hdev, buff);
> + dump_buff_as_hex(buff, BUFF_SZ, raw_data+5, raw_data[4]);
> + } else {
> + snprintf(buff, BUFF_SZ, "\tData overflowed\n");
> + }
> + break;
> + default:
> + snprintf(buff, BUFF_SZ, "\tNot supported\n");
> + }
> + hid_debug_event(hdev, buff);
> + break;
> + case REPORT_VERSION:
> + snprintf(buff, BUFF_SZ, "report %s (%d, size=%d)\n",
> + "REPORT_VERSION", report->id, size-1);
> + hid_debug_event(hdev, buff);
> + snprintf(buff, BUFF_SZ, "\tFirmware version: %d.%d\n",
> + raw_data[2], raw_data[1]);
> + hid_debug_event(hdev, buff);
> + break;
> + case REPORT_BL_ERASE_MEMORY:
> + snprintf(buff, BUFF_SZ, "report %s (%d, size=%d)\n",
> + "REPORT_BL_ERASE_MEMORY", report->id, size-1);
> + hid_debug_event(hdev, buff);
> + /* TODO */
> + break;
> + case REPORT_BL_READ_MEMORY:
> + snprintf(buff, BUFF_SZ, "report %s (%d, size=%d)\n",
> + "REPORT_BL_READ_MEMORY", report->id, size-1);
> + hid_debug_event(hdev, buff);
> + /* TODO */
> + break;
> + case REPORT_BL_WRITE_MEMORY:
> + snprintf(buff, BUFF_SZ, "report %s (%d, size=%d)\n",
> + "REPORT_BL_WRITE_MEMORY", report->id, size-1);
> + hid_debug_event(hdev, buff);
> + /* TODO */
> + break;
> + case REPORT_DEVID:
> + snprintf(buff, BUFF_SZ, "report %s (%d, size=%d)\n",
> + "REPORT_DEVID", report->id, size-1);
> + hid_debug_event(hdev, buff);
> + snprintf(buff, BUFF_SZ, "\tSerial: 0x%02x%02x%02x%02x\n",
> + raw_data[1], raw_data[2], raw_data[3], raw_data[4]);
> + hid_debug_event(hdev, buff);
> + snprintf(buff, BUFF_SZ, "\tType: 0x%02x\n",
> + raw_data[5]);
> + hid_debug_event(hdev, buff);
> + break;
> + case REPORT_SPLASH_SIZE:
> + snprintf(buff, BUFF_SZ, "report %s (%d, size=%d)\n",
> + "REPORT_SPLASH_SIZE", report->id, size-1);
> + hid_debug_event(hdev, buff);
> + snprintf(buff, BUFF_SZ, "\tTotal splash space: %d\n",
> + (raw_data[2] << 8) | raw_data[1]);
> + hid_debug_event(hdev, buff);
> + snprintf(buff, BUFF_SZ, "\tUsed splash space: %d\n",
> + (raw_data[4] << 8) | raw_data[3]);
> + hid_debug_event(hdev, buff);
> + break;
> + case REPORT_HOOK_VERSION:
> + snprintf(buff, BUFF_SZ, "report %s (%d, size=%d)\n",
> + "REPORT_HOOK_VERSION", report->id, size-1);
> + hid_debug_event(hdev, buff);
> + snprintf(buff, BUFF_SZ, "\tFirmware version: %d.%d\n",
> + raw_data[1], raw_data[2]);
> + hid_debug_event(hdev, buff);
> + break;
> + default:
> + snprintf(buff, BUFF_SZ, "report %s (%d, size=%d)\n",
> + "<unknown>", report->id, size-1);
> + hid_debug_event(hdev, buff);
> + break;
> + }
> + wake_up_interruptible(&hdev->debug_wait);
> + kfree(buff);
> +}
> +#else
> +#define picolcd_debug_raw_event(data, hdev, report, raw_data, size)
> +#endif
> +
> +/*
> + * Handle raw report as sent by device
> + */
> +static int picolcd_raw_event(struct hid_device *hdev,
> + struct hid_report *report, u8 *raw_data, int size)
> +{
> + struct picolcd_data *data = hid_get_drvdata(hdev);
> + unsigned long flags;
> + int ret = 0;
> +
> + if (data = NULL)
> + return 1;
> +
> + if (report->id = REPORT_KEY_STATE) {
> + if (data->input_keys)
> + ret = picolcd_raw_keypad(hdev, report, raw_data+1, size-1);
> + } else if (report->id = REPORT_IR_DATA) {
> + if (data->input_cir)
> + ret = picolcd_raw_cir(hdev, report, raw_data+1, size-1);
> + } else {
> + spin_lock_irqsave(&data->lock, flags);
> + /*
> + * We let the caller of picolcd_send_and_wait() check if the report
> + * we got is one of the expected ones or not.
> + */
> + if (data->pending) {
> + memcpy(data->pending->raw_data, raw_data+1, size-1);
> + data->pending->raw_size = size-1;
> + data->pending->in_report = report;
> + complete_all(&data->ready);
> + }
> + spin_unlock_irqrestore(&data->lock, flags);
> + }
> +
> + picolcd_debug_raw_event(data, hdev, report, raw_data, size);
> + return 1;
> +}
> +
> +#ifdef CONFIG_PM
> +static int picolcd_suspend(struct hid_device *hdev)
> +{
> + dbg_hid(PICOLCD_NAME " device ready for suspend\n");
> + return 0;
> +}
> +
> +static int picolcd_resume(struct hid_device *hdev)
> +{
> + return 0;
> +}
> +
> +static int picolcd_reset_resume(struct hid_device *hdev)
> +{
> + int ret;
> + ret = picolcd_reset(hdev);
> + if (ret)
> + dbg_hid(PICOLCD_NAME " resetting our device failed: %d\n", ret);
> + return 0;
> +}
> +#endif
> +
> +/* initialize keypad input device */
> +static inline int picolcd_init_keys(struct picolcd_data *data,
> + struct hid_report *report)
> +{
> + struct hid_device *hdev = data->hdev;
> + struct input_dev *idev;
> + int error, i;
> +
> + if (!report)
> + return -ENODEV;
> + if (report->maxfield != 1 || report->field[0]->report_count != 2 ||
> + report->field[0]->report_size != 8) {
> + dev_err(&hdev->dev, "unsupported KEY_STATE report");
> + return -EINVAL;
> + }
> +
> + idev = input_allocate_device();
> + if (idev = NULL) {
> + dev_err(&hdev->dev, "failed to allocate input device");
> + return -ENOMEM;
> + }
> + input_set_drvdata(idev, hdev);
> + memcpy(data->keycode, def_keymap, sizeof(def_keymap));
> + idev->name = hdev->name;
> + idev->phys = hdev->phys;
> + idev->uniq = hdev->uniq;
> + idev->id.bustype = hdev->bus;
> + idev->id.vendor = hdev->vendor;
> + idev->id.product = hdev->product;
> + idev->id.version = hdev->version;
> + idev->dev.parent = hdev->dev.parent;
> + idev->keycode = &data->keycode;
> + idev->keycodemax = PICOLCD_KEYS;
> + idev->keycodesize = sizeof(int);
> + input_set_capability(idev, EV_MSC, MSC_SCAN);
> + set_bit(EV_REP, idev->evbit);
> + for (i = 0; i < PICOLCD_KEYS; i++) {
> + int key = ((int *)idev->keycode)[i];
> + if (key < KEY_MAX && key >= 0)
> + input_set_capability(idev, EV_KEY, key);
> + }
> + error = input_register_device(idev);
> + if (error) {
> + dev_err(&hdev->dev, "error registering the input device");
> + input_free_device(idev);
> + return error;
> + }
> + data->input_keys = idev;
> + return 0;
> +}
> +
> +static void picolcd_exit_keys(struct picolcd_data *data)
> +{
> + struct input_dev *idev = data->input_keys;
> +
> + data->input_keys = NULL;
> + if (idev)
> + input_unregister_device(idev);
> +}
> +
> +/* initialize CIR input device */
> +static inline int picolcd_init_cir(struct picolcd_data *data, struct hid_report *report)
> +{
> + /* support not implemented yet */
> + return 0;
> +}
> +
> +static void picolcd_exit_cir(struct picolcd_data *data)
> +{
> +}
> +
> +static inline int picolcd_probe_lcd(struct hid_device *hdev, struct picolcd_data *data)
> +{
> + struct picolcd_pending *verinfo;
> + struct hid_report *report;
> + int error;
> +
> + verinfo = picolcd_send_and_wait(hdev, REPORT_VERSION, NULL, 0);
> + if (!verinfo || !verinfo->in_report) {
> + kfree(verinfo);
> + dev_err(&hdev->dev, "failed to query FW version of device\n");
> + return -ENODEV;
> + } else if (verinfo->in_report->id = REPORT_VERSION && verinfo->raw_size = 2) {
> + dev_info(&hdev->dev, "detected PicoLCD with firmware version %d.%d\n",
> + verinfo->raw_data[0], verinfo->raw_data[1]);
> + data->version[0] = verinfo->raw_data[1];
> + data->version[1] = verinfo->raw_data[0];
> + if (data->version[0] != 0 && data->version[1] != 3)
> + dev_info(&hdev->dev, "Device with untested firmware revision, "
> + "please submit /sys/kernel/debug/hid/%s/rdesc for this device.\n",
> + dev_name(&hdev->dev));
> + kfree(verinfo);
> + verinfo = NULL;
> + } else {
> + dev_err(&hdev->dev, "unexpected version response from PicoLCD"
> + " (report=0x%02x, size=%d)\n",
> + verinfo->in_report->id, verinfo->raw_size);
> + kfree(verinfo);
> + verinfo = NULL;
> + return -ENODEV;
> + }
Please consolidate freeing of acquired resources.
> +
> + /* Setup keypad input device */
> + error = picolcd_init_keys(data, picolcd_in_report(REPORT_KEY_STATE, hdev));
> + if (error)
> + goto err;
> +
> + /* Setup CIR input device */
> + error = picolcd_init_cir(data, picolcd_in_report(REPORT_IR_DATA, hdev));
> + if (error)
> + goto err;
> +
> +#ifdef CONFIG_DEBUG_FS
> + report = picolcd_out_report(REPORT_READ_MEMORY, hdev);
> + if (report && report->maxfield = 1 && report->field[0]->report_size = 8)
> + data->addr_sz = report->field[0]->report_count - 1;
> + else
> + data->addr_sz = -1;
> +#endif
> + return 0;
> +err:
> + picolcd_exit_cir(data);
> + picolcd_exit_keys(data);
> + return error;
> +}
> +
> +static inline int picolcd_probe_bootloader(struct hid_device *hdev, struct picolcd_data *data)
> +{
> + struct picolcd_pending *verinfo;
> + struct hid_report *report;
> +
> + verinfo = picolcd_send_and_wait(hdev, REPORT_VERSION, NULL, 0);
> + if (!verinfo || !verinfo->in_report) {
> + kfree(verinfo);
> + dev_err(&hdev->dev, "failed to query FW version of device\n");
> + return -ENODEV;
> + } else if (verinfo->in_report->id = REPORT_VERSION && verinfo->raw_size = 2) {
> + dev_info(&hdev->dev, "detected PicoLCD with bootloader version %d.%d\n",
> + verinfo->raw_data[1], verinfo->raw_data[0]);
> + data->version[0] = verinfo->raw_data[1];
> + data->version[1] = verinfo->raw_data[0];
> + if (data->version[0] != 1 && data->version[1] != 0)
> + dev_info(&hdev->dev, "Device with untested bootloader revision, "
> + "please submit /sys/kernel/debug/hid/%s/rdesc for this device.\n",
> + dev_name(&hdev->dev));
> + kfree(verinfo);
> + verinfo = NULL;
> + } else {
> + dev_err(&hdev->dev, "unexpected version response from PicoLCD"
> + " (report=0x%02x, size=%d)\n",
> + verinfo->in_report->id, verinfo->raw_size);
> + kfree(verinfo);
> + verinfo = NULL;
> + return -ENODEV;
> + }
>
Please consolidate freeing of acquired resources. Wait, I just saw
afucntion like that... can we combine them somehow?
+
> +#ifdef CONFIG_DEBUG_FS
> + report = picolcd_out_report(REPORT_BL_READ_MEMORY, hdev);
> + if (report && report->maxfield = 1 && report->field[0]->report_size = 8)
> + data->addr_sz = report->field[0]->report_count - 1;
> + else
> + data->addr_sz = -1;
> +#endif
> + return 0;
> +}
> +
> +static int picolcd_probe(struct hid_device *hdev,
> + const struct hid_device_id *id)
> +{
> + struct picolcd_data *data;
> + int error = -ENOMEM;
> +
> + dbg_hid(PICOLCD_NAME " hardware probe...\n");
> +
> + /*
> + * Let's allocate the picolcd data structure, set some reasonable
> + * defaults, and associate it with the device
> + */
> + data = kzalloc(sizeof(struct picolcd_data), GFP_KERNEL);
> + if (data = NULL) {
> + dev_err(&hdev->dev, "can't allocate space for Minibox PicoLCD device data\n");
> + error = -ENOMEM;
> + goto err_no_cleanup;
> + }
> +
> + spin_lock_init(&data->lock);
> + init_completion(&data->ready);
> + data->hdev = hdev;
> + if (hdev->product = USB_DEVICE_ID_PICOLCD_BOOTLOADER)
> + data->status |= PICOLCD_BOOTLOADER;
> + hid_set_drvdata(hdev, data);
> +
> + /* Parse the device reports and start it up */
> + error = hid_parse(hdev);
> + if (error) {
> + dev_err(&hdev->dev, "device report parse failed\n");
> + goto err_cleanup_data;
> + }
> +
> + /* We don't use hidinput but hid_hw_start() fails if nothing is
> + * claimed. So spoof claimed input. */
> + hdev->claimed = HID_CLAIMED_INPUT;
> + error = hid_hw_start(hdev, 0);
> + hdev->claimed = 0;
> + if (error) {
> + dev_err(&hdev->dev, "hardware start failed\n");
> + goto err_cleanup_data;
> + }
> +
> + error = hdev->ll_driver->open(hdev);
> + if (error) {
> + dev_err(&hdev->dev, "failed to open input interrupt pipe for key and IR events\n");
> + goto err_cleanup_hid_hw;
> + }
> +
> + error = sysfs_create_file(&(hdev->dev.kobj), &dev_attr_operation_mode.attr);
device_create_file?
> + if (error) {
> + dev_err(&hdev->dev, "failed to create sysfs attributes\n");
> + goto err_cleanup_hid_ll;
> + }
> +
> + if (data->status & PICOLCD_BOOTLOADER)
> + error = picolcd_probe_bootloader(hdev, data);
> + else
> + error = picolcd_probe_lcd(hdev, data);
> + if (error)
> + goto err_cleanup_sysfs;
> +
> + dbg_hid(PICOLCD_NAME " activated and initialized\n");
> + return 0;
> +
> +err_cleanup_sysfs:
> + sysfs_remove_file(&(hdev->dev.kobj), &dev_attr_operation_mode.attr);
> +err_cleanup_hid_ll:
> + hdev->ll_driver->close(hdev);
> +err_cleanup_hid_hw:
> + hid_hw_stop(hdev);
> +err_cleanup_data:
> + kfree(data);
> +err_no_cleanup:
> + hid_set_drvdata(hdev, NULL);
> +
> + return error;
> +}
> +
> +static void picolcd_remove(struct hid_device *hdev)
> +{
> + struct picolcd_data *data = hid_get_drvdata(hdev);
> + unsigned long flags;
> +
> + dbg_hid(PICOLCD_NAME " hardware remove...\n");
> + spin_lock_irqsave(&data->lock, flags);
> + data->status |= PICOLCD_FAILED;
> + spin_unlock_irqrestore(&data->lock, flags);
> +
> + sysfs_remove_file(&(hdev->dev.kobj), &dev_attr_operation_mode.attr);
> + hdev->ll_driver->close(hdev);
> + hid_hw_stop(hdev);
> +
> + /* Cleanup input */
> + picolcd_exit_cir(data);
> + picolcd_exit_keys(data);
> +
> + /* Finally, clean up the picolcd data itself */
> + kfree(data);
> +}
> +
> +static const struct hid_device_id picolcd_devices[] = {
> + { HID_USB_DEVICE(USB_VENDOR_ID_MICROCHIP, USB_DEVICE_ID_PICOLCD) },
> + { HID_USB_DEVICE(USB_VENDOR_ID_MICROCHIP, USB_DEVICE_ID_PICOLCD_BOOTLOADER) },
> + { }
> +};
> +MODULE_DEVICE_TABLE(hid, picolcd_devices);
> +
> +static struct hid_driver picolcd_driver = {
> + .name = "hid-picolcd",
> + .id_table = picolcd_devices,
> + .probe = picolcd_probe,
> + .remove = picolcd_remove,
> + .raw_event = picolcd_raw_event,
> +#ifdef CONFIG_PM
> + .suspend = picolcd_suspend,
> + .resume = picolcd_resume,
> + .reset_resume = picolcd_reset_resume,
> +#endif
> +};
> +
> +static int __init picolcd_init(void)
> +{
> + return hid_register_driver(&picolcd_driver);
> +}
> +
> +static void __exit picolcd_exit(void)
> +{
> + hid_unregister_driver(&picolcd_driver);
> +}
> +
> +module_init(picolcd_init);
> +module_exit(picolcd_exit);
> +MODULE_DESCRIPTION("Minibox graphics PicoLCD Driver");
> +MODULE_LICENSE("GPL v2");
> --
> 1.6.4.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Dmitry
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/6] hid: add framebuffer support to PicoLCD device
2010-03-20 16:04 ` [PATCH v2 2/6] hid: add framebuffer support to " Bruno Prémont
2010-03-21 3:25 ` Dmitry Torokhov
@ 2010-03-21 7:24 ` Jaya Kumar
2010-03-21 16:11 ` Bruno Prémont
1 sibling, 1 reply; 20+ messages in thread
From: Jaya Kumar @ 2010-03-21 7:24 UTC (permalink / raw)
To: Bruno Prémont
Cc: Jiri Kosina, linux-input, linux-usb, linux-fbdev, linux-kernel,
Rick L. Vinyard Jr.,
Nicu Pavel, Oliver Neukum
Hi Bruno,
On Sun, Mar 21, 2010 at 12:04 AM, Bruno Prémont
<bonbons@linux-vserver.org> wrote:
> Add framebuffer support to PicoLCD device with use of deferred-io.
>
> Only changed areas of framebuffer get sent to device in order to
> save USB bandwidth and especially resources on PicoLCD device or
> allow higher refresh rate for a small area.
Interesting work. One minor comment, defio doesn't currently guarantee
that it is "changed areas". Just that it is "written" pages which
typically equates to "changed" but does not guarantee this.
>
> Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
> ---
> drivers/hid/Kconfig | 7 +-
> drivers/hid/hid-picolcd.c | 454 +++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 460 insertions(+), 1 deletions(-)
It is customary for framebuffer drivers to live in drivers/video. This
is the first one I've reviewed that is outside of it. Is there a good
reason for this one to be outside of it? If so, could you put it in
the comments.
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 7097f0a..a474bcd 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -230,6 +230,11 @@ config HID_PETALYNX
> config HID_PICOLCD
> tristate "PicoLCD (graphic version)"
> depends on USB_HID
> + select FB_DEFERRED_IO if FB
> + select FB_SYS_FILLRECT if FB
> + select FB_SYS_COPYAREA if FB
> + select FB_SYS_IMAGEBLIT if FB
> + select FB_SYS_FOPS if FB
I think all of that "if FB" stuff looks odd, it would disappear if it
were in the right Kconfig.
> +/* Framebuffer visual structures */
> +static const struct fb_fix_screeninfo picolcdfb_fix = {
> + .id = PICOLCDFB_NAME,
> + .type = FB_TYPE_PACKED_PIXELS,
> + .visual = FB_VISUAL_MONO01,
Interesting choice. Out of curiosity, which fb client application are
you testing/using this with?
> + /*
> + * Translate the XBM format screen_base into the format needed by the
> + * PicoLCD. See display layout above.
> + * Do this one tile after the other and push those tiles that changed.
> + */
I think screen_base is in standard fb format, which you've specified
as MONO01 above. When you say, XBM format, in the above comment is it
exactly the same?
Thanks,
jaya
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 6/6] hid: add experimental access to PicoLCD device's
[not found] ` <20100321030802.GB29360-WlK9ik9hQGAhIp7JRqBPierSzoNAToWh@public.gmane.org>
@ 2010-03-21 10:29 ` Bruno Prémont
0 siblings, 0 replies; 20+ messages in thread
From: Bruno Prémont @ 2010-03-21 10:29 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Jiri Kosina, linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rick L. Vinyard Jr.,
Nicu Pavel, Oliver Neukum, Jaya Kumar
On Sat, 20 March 2010 Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> On Sat, Mar 20, 2010 at 05:11:19PM +0100, Bruno Prémont wrote:
> > The PicoLCD device has a small amount of EEPROM and also provides
> > access to its FLASH where firmware and splash image are saved.
> > In flasher mode FLASH access is the only active feature.
> >
> > Give read/write access to both via debugfs files.
> >
>
> It looks you are allowing multiple users access to these files. What
> will happen if 2 processes try to write EEPROM at the same time?
Writes will be serialized by the wait for response from device with
picolcd_send_and_wait() and it's up to userspace to repeat the
reads/writes in order to get all the data they need if they requested
more that the 20 bytes that can be transferred in a single HID report.
For flash access a concurrent write to same area can cause one of the
writers to conflict with the other one as a write there is a sequence
of operations which can interleave (e.g. A erases, A writes 1/2,
B erases, B writes 1/2, A writes 2nd 1/2, B writes 2nd 1/2 which would
fail).
A single-user open would be a nice work-around for this.
I would prefer to make erase operation explicitly visible to user so
I don't have to do the erase behind the scenes.
I've not yet looked in the area of mtd/nand support if I can use their
interface.
This is the big reason I did put both to debugfs, they (especially
flash access) need to be made visible in a better way.
> > +
> > +static inline int picolcd_init_devfs(struct picolcd_data *data,
> > + struct hid_report *eeprom_r, struct hid_report *eeprom_w,
> > + struct hid_report *flash_r, struct hid_report *flash_w,
> > + struct hid_report *reset)
> > +{
>
> I don't think this should be forced inline.
Ok, will drop most of the 'inline' keywords as you suggested already
on one of the other patches.
Thanks for the review,
Bruno
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/6] hid: add framebuffer support to PicoLCD device
2010-03-21 7:24 ` Jaya Kumar
@ 2010-03-21 16:11 ` Bruno Prémont
2010-03-22 8:56 ` Jiri Kosina
0 siblings, 1 reply; 20+ messages in thread
From: Bruno Prémont @ 2010-03-21 16:11 UTC (permalink / raw)
To: Jaya Kumar
Cc: Jiri Kosina, linux-input, linux-usb, linux-fbdev, linux-kernel,
Rick L. Vinyard Jr.,
Nicu Pavel, Oliver Neukum
On Sun, 21 March 2010 Jaya Kumar <jayakumar.lkml@gmail.com> wrote:
> > Add framebuffer support to PicoLCD device with use of deferred-io.
> >
> > Only changed areas of framebuffer get sent to device in order to
> > save USB bandwidth and especially resources on PicoLCD device or
> > allow higher refresh rate for a small area.
>
> Interesting work. One minor comment, defio doesn't currently guarantee
> that it is "changed areas". Just that it is "written" pages which
> typically equates to "changed" but does not guarantee this.
When copying from framebuffer to shadow buffer I check if any given tile
has changed and only send that tile to the device if it has effectively
changed. So this check is done independently of defio.
(though I have to force-fully transfer the whole framebuffer at probe
or after reset - which is missing)
> > Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
> > ---
> > drivers/hid/Kconfig | 7 +-
> > drivers/hid/hid-picolcd.c | 454 +++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 460 insertions(+), 1 deletions(-)
>
> It is customary for framebuffer drivers to live in drivers/video. This
> is the first one I've reviewed that is outside of it. Is there a good
> reason for this one to be outside of it? If so, could you put it in
> the comments.
I've kept all the pieces of PicoLCD driver together under hid,
as it's a HID based device. Framebuffer is just one of its features and
keeping pieces together makes it easier to handle.
> > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> > index 7097f0a..a474bcd 100644
> > --- a/drivers/hid/Kconfig
> > +++ b/drivers/hid/Kconfig
> > @@ -230,6 +230,11 @@ config HID_PETALYNX
> > config HID_PICOLCD
> > tristate "PicoLCD (graphic version)"
> > depends on USB_HID
> > + select FB_DEFERRED_IO if FB
> > + select FB_SYS_FILLRECT if FB
> > + select FB_SYS_COPYAREA if FB
> > + select FB_SYS_IMAGEBLIT if FB
> > + select FB_SYS_FOPS if FB
>
> I think all of that "if FB" stuff looks odd, it would disappear if it
> were in the right Kconfig.
In the initial patch I did select FB as well though for v2 I decided
to make all the dependencies of non-input features optional. For the FB
case the various helper entries are needed, for the rest it's sufficient
to have #ifdef's for their device class.
Another option would be to distribute the Kconfig entries over the
various device class's Kconfig files though I don't think that would
make things easier to understand...
> > +/* Framebuffer visual structures */
> > +static const struct fb_fix_screeninfo picolcdfb_fix = {
> > + .id = PICOLCDFB_NAME,
> > + .type = FB_TYPE_PACKED_PIXELS,
> > + .visual = FB_VISUAL_MONO01,
>
> Interesting choice. Out of curiosity, which fb client application are
> you testing/using this with?
For now I've just been testing with manual read/write to /dev/fb. I've
not yet played with fb applications.
I've had a look at fb applications, seems that none wants to play with
individual bits, they all bail out requesting 8 or more bpp (some
fail politely, others just exit/segfault and don't undo changes they did)
So I guess I will have to add support for translating 8bpp to device's
1bpp to be able to use any existing fb application and switching between
1bpp and 8bpp framebuffer...
> > + /*
> > + * Translate the XBM format screen_base into the format needed by the
> > + * PicoLCD. See display layout above.
> > + * Do this one tile after the other and push those tiles that changed.
> > + */
>
> I think screen_base is in standard fb format, which you've specified
> as MONO01 above. When you say, XBM format, in the above comment is it
> exactly the same?
It should be the same (from what I read MONO01 versus MONO10 is just
whether 0 or 1 is "black" or "white") and in combination with
PACKED_PIXELS it makes 8 pixels per bytes.
Thanks for the review,
Bruno
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/6] hid: new driver for PicoLCD device
[not found] ` <20100321034600.GE29360-WlK9ik9hQGAhIp7JRqBPierSzoNAToWh@public.gmane.org>
@ 2010-03-21 16:37 ` Bruno Prémont
2010-03-22 4:35 ` Dmitry Torokhov
2010-03-22 8:54 ` Jiri Kosina
0 siblings, 2 replies; 20+ messages in thread
From: Bruno Prémont @ 2010-03-21 16:37 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Jiri Kosina, linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rick L. Vinyard Jr.,
Nicu Pavel, Oliver Neukum, Jaya Kumar
On Sat, 20 March 2010 Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> On Sat, Mar 20, 2010 at 05:02:41PM +0100, Bruno Prémont wrote:
> > +/* Input device
> > + *
> > + * The PicoLCD has an IR receiver header, a built-in keypad with 5 keys
> > + * and header for 4x4 key matrix. The built-in keys are part of the matrix.
> > + */
> > +#define PICOLCD_KEYS 17
> > +
> > +static const int def_keymap[PICOLCD_KEYS] = {
>
> def_keymap[] = {
> ...
> };
> #define PICOLCD_KEYS ARRAY_SIZE(def_keymap);
>
> would be safe. Also unsigned short should cover it.
Ok
> > + KEY_RESERVED, /* none */
> > + KEY_BACK, /* col 4 + row 1 */
> > + KEY_HOMEPAGE, /* col 3 + row 1 */
> > + KEY_RESERVED, /* col 2 + row 1 */
> > + KEY_RESERVED, /* col 1 + row 1 */
> > + KEY_SCROLLUP, /* col 4 + row 2 */
> > + KEY_OK, /* col 3 + row 2 */
> > + KEY_SCROLLDOWN, /* col 2 + row 2 */
> > + KEY_RESERVED, /* col 1 + row 2 */
> > + KEY_RESERVED, /* col 4 + row 3 */
> > + KEY_RESERVED, /* col 3 + row 3 */
> > + KEY_RESERVED, /* col 2 + row 3 */
> > + KEY_RESERVED, /* col 1 + row 3 */
> > + KEY_RESERVED, /* col 4 + row 4 */
> > + KEY_RESERVED, /* col 3 + row 4 */
> > + KEY_RESERVED, /* col 2 + row 4 */
> > + KEY_RESERVED, /* col 1 + row 4 */
> > +};
> > +
> > +/* Description of in-progress IO operation, used for operations
> > + * that trigger response from device */
> > +struct picolcd_pending {
> > + struct hid_report *out_report;
> > + struct hid_report *in_report;
> > + int raw_size;
> > + u8 raw_data[64];
> > +};
> > +
> > +/* Per device data structure */
> > +struct picolcd_data {
> > + struct hid_device *hdev;
> > +#ifdef CONFIG_DEBUG_FS
> > + int addr_sz;
> > +#endif
> > + u8 version[2];
> > + /* input stuff */
> > + u8 pressed_keys[2];
> > + struct input_dev *input_keys;
> > + struct input_dev *input_cir;
> > + int keycode[PICOLCD_KEYS];
> > +
> > + /* Housekeeping stuff */
> > + spinlock_t lock;
> > + struct picolcd_pending *pending;
> > + struct completion ready;
> > + int status;
> > +#define PICOLCD_BUSY 1
> > +#define PICOLCD_FAILED 4
> > +#define PICOLCD_BOOTLOADER 8
> > +};
> > +
> > +
> > +/* Find a given report */
> > +#define picolcd_in_report(id, dev) picolcd_report(id, dev, HID_INPUT_REPORT)
> > +#define picolcd_out_report(id, dev) picolcd_report(id, dev, HID_OUTPUT_REPORT)
> > +
> > +static struct hid_report *picolcd_report(int id, struct hid_device *hdev, int dir)
> > +{
> > + struct list_head *feature_report_list = &hdev->report_enum[dir].report_list;
> > + struct hid_report *report = NULL;
> > +
> > + list_for_each_entry(report, feature_report_list, list) {
> > + if (report->id = id)
> > + return report;
> > + }
> > + dev_warn(&hdev->dev, "No report with id 0x%x found\n", id);
> > + return NULL;
> > +}
> > +
> > +#ifdef CONFIG_DEBUG_FS
> > +static void picolcd_debug_out_report(struct picolcd_data *data,
> > + struct hid_device *hdev, struct hid_report *report);
> > +#define usbhid_submit_report(a, b, c) \
> > + picolcd_debug_out_report(hid_get_drvdata(a), a, b); \
> > + usbhid_submit_report(a, b, c)
> > +#endif
> > +
> > +/* Submit a report and wait for a reply from device - if device fades away
> > + * or does not respond in time, return NULL */
> > +static struct picolcd_pending* picolcd_send_and_wait(struct hid_device *hdev,
> > + int report_id, const u8 *raw_data, int size)
> > +{
> > + struct picolcd_data *data = hid_get_drvdata(hdev);
> > + struct picolcd_pending *work;
> > + struct hid_report *report = picolcd_out_report(report_id, hdev);
> > + unsigned long flags;
> > + int status, i, j, k;
> > +
> > + if (!report)
> > + return NULL;
> > + work = kzalloc(sizeof(*work), GFP_KERNEL);
> > + if (!work)
> > + return NULL;
> > +
> > + work->out_report = report;
> > + work->in_report = NULL;
> > + work->raw_size = 0;
> > +
> > +retry:
> > + /* try to get lock and enqueue our job */
> > + spin_lock_irqsave(&data->lock, flags);
> > + status = data->status;
> > + if (data->pending || (status & PICOLCD_FAILED)) {
> > + /* some job already pending,
> > + * wait for it to complete and try again */
> > + spin_unlock_irqrestore(&data->lock, flags);
> > + if (status & PICOLCD_FAILED) {
> > + kfree(work);
> > + return NULL;
> > + }
> > + wait_for_completion_interruptible_timeout(&data->ready, HZ*2);
> > + goto retry;
>
> Umm, can we do this with a mutex? Like you take a mutex and don't
> release till you are done talking to the device. So that other guy will
> wait on the mutex instead of waking up and rechecking condition.
>
> > + }
> > + data->status |= PICOLCD_BUSY;
> > + data->pending = work;
> > + for (i = k = 0; i < report->maxfield; i++)
> > + for (j = 0; j < report->field[i]->report_count; j++) {
> > + hid_set_field(report->field[i], j, k < size ? raw_data[k] : 0);
> > + k++;
> > + }
> > + usbhid_submit_report(data->hdev, report, USB_DIR_OUT);
> > + complete_all(&data->ready);
> > + INIT_COMPLETION(data->ready);
>
> Umm, what does this do, exactly?
It wakes up anyone waiting on the completion and then resets the completion
as otherwise any future attempt to wait on it would succeed immediately.
The complete_all() instead of just complete() comes from the reset() function.
I can probably reduce it here.
Will check this combined with your mutex suggestion above.
> > + spin_unlock_irqrestore(&data->lock, flags);
> > + /* wait for our job to complete */
> > + wait_for_completion_interruptible_timeout(&data->ready, HZ*2);
> > +
> > + spin_lock_irqsave(&data->lock, flags);
> > + if (data->pending = work) {
> > + data->status &= ~PICOLCD_BUSY;
> > + data->pending = NULL;
> > + complete_all(&data->ready);
> > + spin_unlock_irqrestore(&data->lock, flags);
> > + return work;
> > + } else {
> > + spin_unlock_irqrestore(&data->lock, flags);
> > + kfree(work);
> > + return NULL;
> > + }
> > +}
> > +
> > +/*
> > + * input class device
> > + */
> > +static int picolcd_raw_keypad(struct hid_device *hdev,
> > + struct hid_report *report, u8 *raw_data, int size)
> > +{
> > + /*
> > + * Keypad event
> > + * First and second data bytes list currently pressed keys,
> > + * 0x00 means no key and at most 2 keys may be pressed at same time
> > + */
> > + struct picolcd_data *data = hid_get_drvdata(hdev);
> > + int i, j;
> > +
> > + /* determine newly pressed keys */
> > + for (i = 0; i < size; i++) {
> > + int key_code;
> > + if (raw_data[i] = 0)
> > + continue;
> > + for (j = 0; j < sizeof(data->pressed_keys); j++)
> > + if (data->pressed_keys[j] = raw_data[i])
> > + goto key_already_down;
> > + for (j = 0; j < sizeof(data->pressed_keys); j++)
> > + if (data->pressed_keys[j] = 0) {
> > + data->pressed_keys[j] = raw_data[i];
> > + break;
> > + }
> > + input_event(data->input_keys, EV_MSC, MSC_SCAN, raw_data[i]);
> > + if (input_get_keycode(data->input_keys, raw_data[i], &key_code))
> > + key_code = KEY_UNKNOWN;
>
> Just get keycode directly from the driver's table, no need to jump through hoops
> here,
Ok
> > + if (key_code != KEY_UNKNOWN) {
> > + dbg_hid(PICOLCD_NAME " got key press for %u:%d", raw_data[i], key_code);
> > + input_report_key(data->input_keys, key_code, 1);
> > + }
> > + input_sync(data->input_keys);
> > +key_already_down:
> > + continue;
> > + }
> > +
> > + /* determine newly released keys */
> > + for (j = 0; j < sizeof(data->pressed_keys); j++) {
> > + int key_code;
> > + if (data->pressed_keys[j] = 0)
> > + continue;
> > + for (i = 0; i < size; i++)
> > + if (data->pressed_keys[j] = raw_data[i])
> > + goto key_still_down;
> > + input_event(data->input_keys, EV_MSC, MSC_SCAN, data->pressed_keys[j]);
> > + if (input_get_keycode(data->input_keys, data->pressed_keys[j], &key_code))
> > + key_code = KEY_UNKNOWN;
> > + if (key_code != KEY_UNKNOWN) {
> > + dbg_hid(PICOLCD_NAME " got key release for %u:%d", data->pressed_keys[j], key_code);
> > + input_report_key(data->input_keys, key_code, 0);
> > + }
> > + input_sync(data->input_keys);
> > + data->pressed_keys[j] = 0;
> > +key_still_down:
> > + continue;
> > + }
> > + return 1;
> > +}
> > +
> > +static int picolcd_raw_cir(struct hid_device *hdev,
> > + struct hid_report *report, u8 *raw_data, int size)
> > +{
> > + /* Need understanding of CIR data format to implement ... */
> > + return 1;
> > +}
> > +
> > +
> > +
> > +/*
> > + * Reset our device and wait for answer to VERSION request
> > + */
> > +static int picolcd_reset(struct hid_device *hdev)
> > +{
> > + struct picolcd_data *data = hid_get_drvdata(hdev);
> > + struct hid_report *report = picolcd_out_report(REPORT_RESET, hdev);
> > + struct picolcd_pending *verinfo;
> > + unsigned long flags;
> > +
> > + if (!data || !report || report->maxfield != 1)
> > + return -ENODEV;
> > +
> > + spin_lock_irqsave(&data->lock, flags);
> > + complete_all(&data->ready);
> > + INIT_COMPLETION(data->ready);
> > + if (hdev->product = USB_DEVICE_ID_PICOLCD_BOOTLOADER)
> > + data->status |= PICOLCD_BOOTLOADER;
> > +
> > + /* perform the reset */
> > + hid_set_field(report->field[0], 0, 1);
> > + usbhid_submit_report(hdev, report, USB_DIR_OUT);
> > + spin_unlock_irqrestore(&data->lock, flags);
> > +
> > + verinfo = picolcd_send_and_wait(hdev, REPORT_VERSION, NULL, 0);
> > + if (verinfo && verinfo->raw_size = 2) {
> > + if (data->status & PICOLCD_BOOTLOADER) {
> > + dev_info(&hdev->dev, "PicoLCD reset successful, bootloader version %d.%d\n",
> > + verinfo->raw_data[0], verinfo->raw_data[1]);
> > + data->version[0] = verinfo->raw_data[0];
> > + data->version[1] = verinfo->raw_data[1];
> > + } else {
> > + dev_info(&hdev->dev, "PicoLCD reset successful, firmware version %d.%d\n",
> > + verinfo->raw_data[1], verinfo->raw_data[0]);
> > + data->version[0] = verinfo->raw_data[1];
> > + data->version[1] = verinfo->raw_data[0];
> > + }
> > + kfree(verinfo);
> > + verinfo = NULL;
> > + } else if (verinfo) {
> > + dev_err(&hdev->dev, "confused, got unexpected version response from PicoLCD after reset\n");
> > + kfree(verinfo);
> > + verinfo = NULL;
> > + } else {
> > + dev_err(&hdev->dev, "no version response from PicoLCD after reset");
> > + return -EBUSY;
> > + }
> > +
>
> I am pretty sure it could be written clearer instead of checking for
> verinfo several times...
>
> if (!verinfo) {
> dev_err(..);
> return -EBUSY;
> }
>
> if (verinfo->raw_size = 2) {
> ...
> } else {
> dev_err(...)
> }
>
> kfree(verinfo);
See below
> > + return 0;
> > +}
> > +
> > +/*
> > + * The "operation_mode" sysfs attribute
> > + */
> > +static ssize_t picolcd_operation_mode_show(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct picolcd_data *data = dev_get_drvdata(dev);
> > +
> > + if (data->status & PICOLCD_BOOTLOADER)
> > + return snprintf(buf, PAGE_SIZE, "<bootloader> lcd\n");
> > + else
> > + return snprintf(buf, PAGE_SIZE, "bootloader <lcd>\n");
> > +}
> > +
> > +static ssize_t picolcd_operation_mode_store(struct device *dev,
> > + struct device_attribute *attr, const char *buf, size_t count)
> > +{
> > + struct picolcd_data *data = dev_get_drvdata(dev);
> > + struct hid_report *report = NULL;
> > + size_t cnt = count;
> > + int timeout = 5000;
> > + unsigned u;
> > + unsigned long flags;
> > +
> > + if (cnt >= 3 && strncmp("lcd", buf, 3) = 0) {
> > + if (data->status & PICOLCD_BOOTLOADER)
> > + report = picolcd_out_report(REPORT_EXIT_FLASHER, data->hdev);
> > + buf += 3;
> > + cnt -= 3;
> > + } else if (cnt >= 10 && strncmp("bootloader", buf, 10) = 0) {
> > + if (!(data->status & PICOLCD_BOOTLOADER))
> > + report = picolcd_out_report(REPORT_EXIT_KEYBOARD, data->hdev);
> > + buf += 10;
> > + cnt -= 10;
> > + }
> > + if (!report)
> > + return -EINVAL;
> > +
> > + while (cnt > 0 && (*buf = ' ' || *buf = '\t')) {
> > + buf++;
> > + cnt--;
> > + }
> > + while (cnt > 0 && (buf[cnt-1] = '\n' || buf[cnt-1] = '\r'))
> > + cnt--;
> > + if (cnt > 0) {
> > + if (sscanf(buf, "%u", &u) != 1)
> > + return -EINVAL;
> > + if (u > 30000)
> > + return -EINVAL;
> > + else
> > + timeout = u;
> > + }
> > +
> > + spin_lock_irqsave(&data->lock, flags);
> > + hid_set_field(report->field[0], 0, timeout & 0xff);
> > + hid_set_field(report->field[0], 1, (timeout >> 8) & 0xff);
> > + usbhid_submit_report(data->hdev, report, USB_DIR_OUT);
> > + spin_unlock_irqrestore(&data->lock, flags);
> > + return count;
> > +}
> > +
> > +static DEVICE_ATTR(operation_mode, 0644, picolcd_operation_mode_show,
> > + picolcd_operation_mode_store);
> > +
> > +
> > +#ifdef CONFIG_DEBUG_FS
... snip reports dumping to debugfs's events ...
> > +#else
> > +#define picolcd_debug_raw_event(data, hdev, report, raw_data, size)
> > +#endif
> > +
> > +/*
> > + * Handle raw report as sent by device
> > + */
> > +static int picolcd_raw_event(struct hid_device *hdev,
> > + struct hid_report *report, u8 *raw_data, int size)
> > +{
> > + struct picolcd_data *data = hid_get_drvdata(hdev);
> > + unsigned long flags;
> > + int ret = 0;
> > +
> > + if (data = NULL)
> > + return 1;
> > +
> > + if (report->id = REPORT_KEY_STATE) {
> > + if (data->input_keys)
> > + ret = picolcd_raw_keypad(hdev, report, raw_data+1, size-1);
> > + } else if (report->id = REPORT_IR_DATA) {
> > + if (data->input_cir)
> > + ret = picolcd_raw_cir(hdev, report, raw_data+1, size-1);
> > + } else {
> > + spin_lock_irqsave(&data->lock, flags);
> > + /*
> > + * We let the caller of picolcd_send_and_wait() check if the report
> > + * we got is one of the expected ones or not.
> > + */
> > + if (data->pending) {
> > + memcpy(data->pending->raw_data, raw_data+1, size-1);
> > + data->pending->raw_size = size-1;
> > + data->pending->in_report = report;
> > + complete_all(&data->ready);
> > + }
> > + spin_unlock_irqrestore(&data->lock, flags);
> > + }
> > +
> > + picolcd_debug_raw_event(data, hdev, report, raw_data, size);
> > + return 1;
> > +}
> > +
> > +#ifdef CONFIG_PM
> > +static int picolcd_suspend(struct hid_device *hdev)
> > +{
> > + dbg_hid(PICOLCD_NAME " device ready for suspend\n");
> > + return 0;
> > +}
> > +
> > +static int picolcd_resume(struct hid_device *hdev)
> > +{
> > + return 0;
> > +}
> > +
> > +static int picolcd_reset_resume(struct hid_device *hdev)
> > +{
> > + int ret;
> > + ret = picolcd_reset(hdev);
> > + if (ret)
> > + dbg_hid(PICOLCD_NAME " resetting our device failed: %d\n", ret);
> > + return 0;
> > +}
> > +#endif
> > +
> > +/* initialize keypad input device */
> > +static inline int picolcd_init_keys(struct picolcd_data *data,
> > + struct hid_report *report)
> > +{
> > + struct hid_device *hdev = data->hdev;
> > + struct input_dev *idev;
> > + int error, i;
> > +
> > + if (!report)
> > + return -ENODEV;
> > + if (report->maxfield != 1 || report->field[0]->report_count != 2 ||
> > + report->field[0]->report_size != 8) {
> > + dev_err(&hdev->dev, "unsupported KEY_STATE report");
> > + return -EINVAL;
> > + }
> > +
> > + idev = input_allocate_device();
> > + if (idev = NULL) {
> > + dev_err(&hdev->dev, "failed to allocate input device");
> > + return -ENOMEM;
> > + }
> > + input_set_drvdata(idev, hdev);
> > + memcpy(data->keycode, def_keymap, sizeof(def_keymap));
> > + idev->name = hdev->name;
> > + idev->phys = hdev->phys;
> > + idev->uniq = hdev->uniq;
> > + idev->id.bustype = hdev->bus;
> > + idev->id.vendor = hdev->vendor;
> > + idev->id.product = hdev->product;
> > + idev->id.version = hdev->version;
> > + idev->dev.parent = hdev->dev.parent;
> > + idev->keycode = &data->keycode;
> > + idev->keycodemax = PICOLCD_KEYS;
> > + idev->keycodesize = sizeof(int);
> > + input_set_capability(idev, EV_MSC, MSC_SCAN);
> > + set_bit(EV_REP, idev->evbit);
> > + for (i = 0; i < PICOLCD_KEYS; i++) {
> > + int key = ((int *)idev->keycode)[i];
> > + if (key < KEY_MAX && key >= 0)
> > + input_set_capability(idev, EV_KEY, key);
> > + }
> > + error = input_register_device(idev);
> > + if (error) {
> > + dev_err(&hdev->dev, "error registering the input device");
> > + input_free_device(idev);
> > + return error;
> > + }
> > + data->input_keys = idev;
> > + return 0;
> > +}
> > +
> > +static void picolcd_exit_keys(struct picolcd_data *data)
> > +{
> > + struct input_dev *idev = data->input_keys;
> > +
> > + data->input_keys = NULL;
> > + if (idev)
> > + input_unregister_device(idev);
> > +}
> > +
> > +/* initialize CIR input device */
> > +static inline int picolcd_init_cir(struct picolcd_data *data, struct hid_report *report)
> > +{
> > + /* support not implemented yet */
> > + return 0;
> > +}
> > +
> > +static void picolcd_exit_cir(struct picolcd_data *data)
> > +{
> > +}
> > +
> > +static inline int picolcd_probe_lcd(struct hid_device *hdev, struct picolcd_data *data)
> > +{
> > + struct picolcd_pending *verinfo;
> > + struct hid_report *report;
> > + int error;
> > +
> > + verinfo = picolcd_send_and_wait(hdev, REPORT_VERSION, NULL, 0);
> > + if (!verinfo || !verinfo->in_report) {
> > + kfree(verinfo);
> > + dev_err(&hdev->dev, "failed to query FW version of device\n");
> > + return -ENODEV;
> > + } else if (verinfo->in_report->id = REPORT_VERSION && verinfo->raw_size = 2) {
> > + dev_info(&hdev->dev, "detected PicoLCD with firmware version %d.%d\n",
> > + verinfo->raw_data[0], verinfo->raw_data[1]);
> > + data->version[0] = verinfo->raw_data[1];
> > + data->version[1] = verinfo->raw_data[0];
> > + if (data->version[0] != 0 && data->version[1] != 3)
> > + dev_info(&hdev->dev, "Device with untested firmware revision, "
> > + "please submit /sys/kernel/debug/hid/%s/rdesc for this device.\n",
> > + dev_name(&hdev->dev));
> > + kfree(verinfo);
> > + verinfo = NULL;
> > + } else {
> > + dev_err(&hdev->dev, "unexpected version response from PicoLCD"
> > + " (report=0x%02x, size=%d)\n",
> > + verinfo->in_report->id, verinfo->raw_size);
> > + kfree(verinfo);
> > + verinfo = NULL;
> > + return -ENODEV;
> > + }
>
>
> Please consolidate freeing of acquired resources.
See below
> > +
> > + /* Setup keypad input device */
> > + error = picolcd_init_keys(data, picolcd_in_report(REPORT_KEY_STATE, hdev));
> > + if (error)
> > + goto err;
> > +
> > + /* Setup CIR input device */
> > + error = picolcd_init_cir(data, picolcd_in_report(REPORT_IR_DATA, hdev));
> > + if (error)
> > + goto err;
> > +
> > +#ifdef CONFIG_DEBUG_FS
> > + report = picolcd_out_report(REPORT_READ_MEMORY, hdev);
> > + if (report && report->maxfield = 1 && report->field[0]->report_size = 8)
> > + data->addr_sz = report->field[0]->report_count - 1;
> > + else
> > + data->addr_sz = -1;
> > +#endif
> > + return 0;
> > +err:
> > + picolcd_exit_cir(data);
> > + picolcd_exit_keys(data);
> > + return error;
> > +}
> > +
> > +static inline int picolcd_probe_bootloader(struct hid_device *hdev, struct picolcd_data *data)
> > +{
> > + struct picolcd_pending *verinfo;
> > + struct hid_report *report;
> > +
> > + verinfo = picolcd_send_and_wait(hdev, REPORT_VERSION, NULL, 0);
> > + if (!verinfo || !verinfo->in_report) {
> > + kfree(verinfo);
> > + dev_err(&hdev->dev, "failed to query FW version of device\n");
> > + return -ENODEV;
> > + } else if (verinfo->in_report->id = REPORT_VERSION && verinfo->raw_size = 2) {
> > + dev_info(&hdev->dev, "detected PicoLCD with bootloader version %d.%d\n",
> > + verinfo->raw_data[1], verinfo->raw_data[0]);
> > + data->version[0] = verinfo->raw_data[1];
> > + data->version[1] = verinfo->raw_data[0];
> > + if (data->version[0] != 1 && data->version[1] != 0)
> > + dev_info(&hdev->dev, "Device with untested bootloader revision, "
> > + "please submit /sys/kernel/debug/hid/%s/rdesc for this device.\n",
> > + dev_name(&hdev->dev));
> > + kfree(verinfo);
> > + verinfo = NULL;
> > + } else {
> > + dev_err(&hdev->dev, "unexpected version response from PicoLCD"
> > + " (report=0x%02x, size=%d)\n",
> > + verinfo->in_report->id, verinfo->raw_size);
> > + kfree(verinfo);
> > + verinfo = NULL;
> > + return -ENODEV;
> > + }
> >
>
> Please consolidate freeing of acquired resources. Wait, I just saw
> afucntion like that... can we combine them somehow?
Yeah, there are 3 blocks of very similar code though the important
difference is the operation mode (bootloader versus firmware).
According to the behavior and what bootloader mode displays
the version numbers are not in the same order for both cases.
The code distribution was looking worse some internal revisions ago
as by then I did allocate verinfo->raw_data separately ...
Will try to extract version check to a function.
> > +
> > +#ifdef CONFIG_DEBUG_FS
> > + report = picolcd_out_report(REPORT_BL_READ_MEMORY, hdev);
> > + if (report && report->maxfield = 1 && report->field[0]->report_size = 8)
> > + data->addr_sz = report->field[0]->report_count - 1;
> > + else
> > + data->addr_sz = -1;
> > +#endif
> > + return 0;
> > +}
> > +
> > +static int picolcd_probe(struct hid_device *hdev,
> > + const struct hid_device_id *id)
> > +{
> > + struct picolcd_data *data;
> > + int error = -ENOMEM;
> > +
> > + dbg_hid(PICOLCD_NAME " hardware probe...\n");
> > +
> > + /*
> > + * Let's allocate the picolcd data structure, set some reasonable
> > + * defaults, and associate it with the device
> > + */
> > + data = kzalloc(sizeof(struct picolcd_data), GFP_KERNEL);
> > + if (data = NULL) {
> > + dev_err(&hdev->dev, "can't allocate space for Minibox PicoLCD device data\n");
> > + error = -ENOMEM;
> > + goto err_no_cleanup;
> > + }
> > +
> > + spin_lock_init(&data->lock);
> > + init_completion(&data->ready);
> > + data->hdev = hdev;
> > + if (hdev->product = USB_DEVICE_ID_PICOLCD_BOOTLOADER)
> > + data->status |= PICOLCD_BOOTLOADER;
> > + hid_set_drvdata(hdev, data);
> > +
> > + /* Parse the device reports and start it up */
> > + error = hid_parse(hdev);
> > + if (error) {
> > + dev_err(&hdev->dev, "device report parse failed\n");
> > + goto err_cleanup_data;
> > + }
> > +
> > + /* We don't use hidinput but hid_hw_start() fails if nothing is
> > + * claimed. So spoof claimed input. */
> > + hdev->claimed = HID_CLAIMED_INPUT;
> > + error = hid_hw_start(hdev, 0);
> > + hdev->claimed = 0;
> > + if (error) {
> > + dev_err(&hdev->dev, "hardware start failed\n");
> > + goto err_cleanup_data;
> > + }
> > +
> > + error = hdev->ll_driver->open(hdev);
> > + if (error) {
> > + dev_err(&hdev->dev, "failed to open input interrupt pipe for key and IR events\n");
> > + goto err_cleanup_hid_hw;
> > + }
> > +
> > + error = sysfs_create_file(&(hdev->dev.kobj), &dev_attr_operation_mode.attr);
>
> device_create_file?
Thanks for the pointer, will switch
> > + if (error) {
> > + dev_err(&hdev->dev, "failed to create sysfs attributes\n");
> > + goto err_cleanup_hid_ll;
> > + }
> > +
Thanks for your review,
Bruno
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/6] hid: new driver for PicoLCD device
2010-03-21 16:37 ` Bruno Prémont
@ 2010-03-22 4:35 ` Dmitry Torokhov
[not found] ` <20100322043508.GC31621-WlK9ik9hQGAhIp7JRqBPierSzoNAToWh@public.gmane.org>
2010-03-22 8:54 ` Jiri Kosina
1 sibling, 1 reply; 20+ messages in thread
From: Dmitry Torokhov @ 2010-03-22 4:35 UTC (permalink / raw)
To: Bruno Prémont
Cc: Jiri Kosina, linux-input, linux-usb, linux-fbdev, linux-kernel,
Rick L. Vinyard Jr.,
Nicu Pavel, Oliver Neukum, Jaya Kumar
On Sun, Mar 21, 2010 at 05:37:37PM +0100, Bruno Prémont wrote:
> On Sat, 20 March 2010 Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > > + usbhid_submit_report(data->hdev, report, USB_DIR_OUT);
> > > + complete_all(&data->ready);
> > > + INIT_COMPLETION(data->ready);
> >
> > Umm, what does this do, exactly?
>
> It wakes up anyone waiting on the completion and then resets the completion
> as otherwise any future attempt to wait on it would succeed immediately.
>
You realize that if you re-initialize the completion right after
signalling it there is a big chance the waiters will miss it (they do
check completion->done flags that you reset right away.
In general completions are suited for something that happens once (a
single request - allocated - processed - signalled) but not for
repeating use.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/6] hid: new driver for PicoLCD device
2010-03-21 16:37 ` Bruno Prémont
2010-03-22 4:35 ` Dmitry Torokhov
@ 2010-03-22 8:54 ` Jiri Kosina
1 sibling, 0 replies; 20+ messages in thread
From: Jiri Kosina @ 2010-03-22 8:54 UTC (permalink / raw)
To: Bruno Prémont
Cc: Dmitry Torokhov, linux-input, linux-usb, linux-fbdev,
linux-kernel, Rick L. Vinyard Jr.,
Nicu Pavel, Oliver Neukum, Jaya Kumar
On Sun, 21 Mar 2010, Bruno Prémont wrote:
> > > + }
> > > + data->status |= PICOLCD_BUSY;
> > > + data->pending = work;
> > > + for (i = k = 0; i < report->maxfield; i++)
> > > + for (j = 0; j < report->field[i]->report_count; j++) {
> > > + hid_set_field(report->field[i], j, k < size ? raw_data[k] : 0);
> > > + k++;
> > > + }
> > > + usbhid_submit_report(data->hdev, report, USB_DIR_OUT);
> > > + complete_all(&data->ready);
> > > + INIT_COMPLETION(data->ready);
> >
> > Umm, what does this do, exactly?
> >
> It wakes up anyone waiting on the completion and then resets the completion
> as otherwise any future attempt to wait on it would succeed immediately.
>
> The complete_all() instead of just complete() comes from the reset() function.
> I can probably reduce it here.
>
> Will check this combined with your mutex suggestion above.
I am afraid this code is racy as is. You can't be sure that whoever has
been waiting on the completion has been notified already between
complete_all() and INIT_COMPLETION(), can you?
Thanks for the driver!
--
Jiri Kosina
SUSE Labs, Novell Inc.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/6] hid: add framebuffer support to PicoLCD device
2010-03-21 16:11 ` Bruno Prémont
@ 2010-03-22 8:56 ` Jiri Kosina
0 siblings, 0 replies; 20+ messages in thread
From: Jiri Kosina @ 2010-03-22 8:56 UTC (permalink / raw)
To: Bruno Prémont
Cc: Jaya Kumar, linux-input, linux-usb, linux-fbdev, linux-kernel,
Rick L. Vinyard Jr.,
Nicu Pavel, Oliver Neukum
On Sun, 21 Mar 2010, Bruno Prémont wrote:
> > > ---
> > > drivers/hid/Kconfig | 7 +-
> > > drivers/hid/hid-picolcd.c | 454 +++++++++++++++++++++++++++++++++++++++++++++
> > > 2 files changed, 460 insertions(+), 1 deletions(-)
> >
> > It is customary for framebuffer drivers to live in drivers/video. This
> > is the first one I've reviewed that is outside of it. Is there a good
> > reason for this one to be outside of it? If so, could you put it in
> > the comments.
>
> I've kept all the pieces of PicoLCD driver together under hid,
> as it's a HID based device. Framebuffer is just one of its features and
> keeping pieces together makes it easier to handle.
I think it makes sense for a HID driver for this device to contain the
framebuffer bits as well ... from user perspective, this seems to be HID
device, with "extra" framebuffer display on it.
If framebuffer folks Ack the fb-related code, I am fine keeping it under
drivers/hid.
Thanks,
--
Jiri Kosina
SUSE Labs, Novell Inc.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 3/6] hid: add backlight support to PicoLCD device
2010-03-20 16:06 ` [PATCH v2 3/6] hid: add backlight " Bruno Prémont
@ 2010-03-22 8:59 ` Jiri Kosina
2010-03-22 11:01 ` Bruno Prémont
0 siblings, 1 reply; 20+ messages in thread
From: Jiri Kosina @ 2010-03-22 8:59 UTC (permalink / raw)
To: Bruno Prémont
Cc: linux-input, linux-usb, linux-fbdev, linux-kernel,
Rick L. Vinyard Jr.,
Nicu Pavel, Oliver Neukum, Jaya Kumar, Richard Purdie
On Sat, 20 Mar 2010, Bruno Prémont wrote:
> Add backlight support to PicoLCD device.
>
> Backlight support depends on backlight class and is only being
> compiled if backlight class has been selected.
>
> Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
> ---
> drivers/hid/Kconfig | 2 +-
> drivers/hid/hid-picolcd.c | 134 ++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 134 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index a474bcd..5ec3cb7 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -243,8 +243,8 @@ config HID_PICOLCD
> - Keypad
> - Switching between Firmware and Flash mode
> - Framebuffer for monochrome 256x64 display
> + - Backlight control (needs CONFIG_BACKLIGHT_CLASS_DEVICE)
Wouldn't it be better to have Kconfig rules actually resolve the
dependency?
I don't see any issue with HID_PICOLCD directly selecting
BACKLIGHT_CLASS_DEVICE. Or you can do a separate sub-option for that, if
you really want to avoid the direct dependency of the whole driver.
Thanks,
--
Jiri Kosina
SUSE Labs, Novell Inc.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 3/6] hid: add backlight support to PicoLCD device
2010-03-22 8:59 ` Jiri Kosina
@ 2010-03-22 11:01 ` Bruno Prémont
0 siblings, 0 replies; 20+ messages in thread
From: Bruno Prémont @ 2010-03-22 11:01 UTC (permalink / raw)
To: Jiri Kosina
Cc: linux-input, linux-usb, linux-fbdev, linux-kernel,
Rick L. Vinyard Jr.,
Nicu Pavel, Oliver Neukum, Jaya Kumar, Richard Purdie
On Mon, 22 March 2010 Jiri Kosina <jkosina@suse.cz> wrote:
> On Sat, 20 Mar 2010, Bruno Prémont wrote:
>
> > Add backlight support to PicoLCD device.
> >
> > Backlight support depends on backlight class and is only being
> > compiled if backlight class has been selected.
> >
> > Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
> > ---
> > drivers/hid/Kconfig | 2 +-
> > drivers/hid/hid-picolcd.c | 134 ++++++++++++++++++++++++++++++++++++++++++++-
> > 2 files changed, 134 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> > index a474bcd..5ec3cb7 100644
> > --- a/drivers/hid/Kconfig
> > +++ b/drivers/hid/Kconfig
> > @@ -243,8 +243,8 @@ config HID_PICOLCD
> > - Keypad
> > - Switching between Firmware and Flash mode
> > - Framebuffer for monochrome 256x64 display
> > + - Backlight control (needs CONFIG_BACKLIGHT_CLASS_DEVICE)
>
> Wouldn't it be better to have Kconfig rules actually resolve the
> dependency?
>
> I don't see any issue with HID_PICOLCD directly selecting
> BACKLIGHT_CLASS_DEVICE. Or you can do a separate sub-option for that, if
> you really want to avoid the direct dependency of the whole driver.
I could select both BACKLIGHT_CLASS_DEVICE and LCD_CLASS_DEVICE (as
well as their common parent -- same applies to FB) though for the
LEDS_CLASS I would definitely rather go the path of a sub-option, by
default the GPO pins operated with the LEDs are not connected to
anything.
Thanks for the review,
Bruno
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/6] hid: new driver for PicoLCD device
[not found] ` <20100322043508.GC31621-WlK9ik9hQGAhIp7JRqBPierSzoNAToWh@public.gmane.org>
@ 2010-03-22 11:38 ` Bruno Prémont
0 siblings, 0 replies; 20+ messages in thread
From: Bruno Prémont @ 2010-03-22 11:38 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Jiri Kosina, linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rick L. Vinyard Jr.,
Nicu Pavel, Oliver Neukum, Jaya Kumar
On Sun, 21 March 2010 Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> On Sun, Mar 21, 2010 at 05:37:37PM +0100, Bruno Prémont wrote:
> > On Sat, 20 March 2010 Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > > > + usbhid_submit_report(data->hdev, report, USB_DIR_OUT);
> > > > + complete_all(&data->ready);
> > > > + INIT_COMPLETION(data->ready);
> > >
> > > Umm, what does this do, exactly?
> >
> > It wakes up anyone waiting on the completion and then resets the completion
> > as otherwise any future attempt to wait on it would succeed immediately.
> >
>
> You realize that if you re-initialize the completion right after
> signalling it there is a big chance the waiters will miss it (they do
> check completion->done flags that you reset right away.
>
> In general completions are suited for something that happens once (a
> single request - allocated - processed - signalled) but not for
> repeating use.
Would below approach be more correct?
- move the completion to struct picolcd_pending
so it meets the "happens once" requirement
- protect data->pending with a mutex
(though also use spinlock to prevent race between event which
signals the completion and picolcd_send_and_wait() around timeout)
- use the data->lock spinlock to protect multi-report requests from
interleaving
In speudo-code this would be something like:
picolcd_send_and_wait(...)
{
struct picolcd_pending pending;
...
init_completion(&pending->ready);
aquire_mutex(data->mutex);
spin_lock_irqsave(&data->lock, flags);
... // prepare report
data->pending = &pending;
usbhid_submit_report(data->hdev, report, USB_DIR_OUT);
spin_unlock_irqrestore(&data->lock, flags);
wait_for_completion_interruptible_timeout(&pending->ready, HZ*2);
spin_lock_irqsave(&data->lock, flags);
if (data->pending = &pending)
data->pending = NULL;
spin_unlock_irqrestore(&data->lock, flags);
release_mutext(data->mutex);
...
}
picolcd_raw_event(...)
{
...
spin_lock_irqsave(&data->lock, flags);
if (data->pending) {
// copy event data to pending
...
complete(&pending->ready);
}
spin_unlock_irqrestore(&data->lock, flags);
...
}
In picolcd_remove() and picolcd_reset() I could then do something
similar to picolcd_raw_event() to trigger completion with no
data/error in order to skip timeout on the requester side.
Though how should I prevent races on hot-unplug?
Thanks,
Bruno
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2010-03-22 11:38 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-03-20 16:00 [PATCH v2 0/6] hid: new driver for PicoLCD device Bruno Prémont
[not found] ` <20100320170014.440959a8-hY15tx4IgV39zxVx7UNMDg@public.gmane.org>
2010-03-20 16:02 ` [PATCH v2 1/6] " Bruno Prémont
2010-03-21 3:46 ` Dmitry Torokhov
[not found] ` <20100321034600.GE29360-WlK9ik9hQGAhIp7JRqBPierSzoNAToWh@public.gmane.org>
2010-03-21 16:37 ` Bruno Prémont
2010-03-22 4:35 ` Dmitry Torokhov
[not found] ` <20100322043508.GC31621-WlK9ik9hQGAhIp7JRqBPierSzoNAToWh@public.gmane.org>
2010-03-22 11:38 ` Bruno Prémont
2010-03-22 8:54 ` Jiri Kosina
2010-03-20 16:04 ` [PATCH v2 2/6] hid: add framebuffer support to " Bruno Prémont
2010-03-21 3:25 ` Dmitry Torokhov
2010-03-21 7:24 ` Jaya Kumar
2010-03-21 16:11 ` Bruno Prémont
2010-03-22 8:56 ` Jiri Kosina
2010-03-20 16:06 ` [PATCH v2 3/6] hid: add backlight " Bruno Prémont
2010-03-22 8:59 ` Jiri Kosina
2010-03-22 11:01 ` Bruno Prémont
2010-03-20 16:08 ` [PATCH v2 4/6] hid: add lcd " Bruno Prémont
2010-03-20 16:10 ` [PATCH v2 5/6] hid: add GPO (leds) " Bruno Prémont
2010-03-20 16:11 ` [PATCH v2 6/6] hid: add experimental access to PicoLCD device's Bruno Prémont
2010-03-21 3:08 ` Dmitry Torokhov
[not found] ` <20100321030802.GB29360-WlK9ik9hQGAhIp7JRqBPierSzoNAToWh@public.gmane.org>
2010-03-21 10:29 ` Bruno Prémont
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).