lkml.org 
[lkml]   [2012]   [Oct]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH v1] i2c-hid: introduce HID over i2c specification implementation
    Date
    Hi Benjamin,

    I have one little question about __i2chid_command(), please see below.

    "benjamin.tissoires" <benjamin.tissoires@gmail.com> writes:
    > From: Benjamin Tissoires <benjamin.tissoires@enac.fr>
    >
    > Microsoft published the protocol specification of HID over i2c:
    > http://msdn.microsoft.com/en-us/library/windows/hardware/hh852380.aspx
    >
    > This patch introduces an implementation of this protocol.
    >
    > This implementation does not includes the ACPI part of the specification.
    > This will come when ACPI 5.0 devices will be available.
    >
    > Once the ACPI part will be done, OEM will not have to declare HID over I2C
    > devices in their platform specific driver.
    >
    > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@enac.fr>
    > ---
    >
    > Hi,
    >
    > this is finally my first implementation of HID over I2C.
    >
    > This has been tested on an Elan Microelectronics HID over I2C device, with
    > a Samsung Exynos 4412 board.
    >
    > Any comments are welcome.
    >
    > Cheers,
    > Benjamin
    >
    > drivers/i2c/Kconfig | 8 +
    > drivers/i2c/Makefile | 1 +
    > drivers/i2c/i2c-hid.c | 1027 +++++++++++++++++++++++++++++++++++++++++++
    > include/linux/i2c/i2c-hid.h | 35 ++
    > 4 files changed, 1071 insertions(+)
    > create mode 100644 drivers/i2c/i2c-hid.c
    > create mode 100644 include/linux/i2c/i2c-hid.h
    >
    > diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
    > index 5a3bb3d..5adf65a 100644
    > --- a/drivers/i2c/Kconfig
    > +++ b/drivers/i2c/Kconfig
    > @@ -47,6 +47,14 @@ config I2C_CHARDEV
    > This support is also available as a module. If so, the module
    > will be called i2c-dev.
    >
    > +config I2C_HID
    > + tristate "HID over I2C bus"
    > + help
    > + Say Y here to use the HID over i2c protocol implementation.
    > +
    > + This support is also available as a module. If so, the module
    > + will be called i2c-hid.
    > +
    > config I2C_MUX
    > tristate "I2C bus multiplexing support"
    > help
    > diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
    > index beee6b2..8f38116 100644
    > --- a/drivers/i2c/Makefile
    > +++ b/drivers/i2c/Makefile
    > @@ -6,6 +6,7 @@ obj-$(CONFIG_I2C_BOARDINFO) += i2c-boardinfo.o
    > obj-$(CONFIG_I2C) += i2c-core.o
    > obj-$(CONFIG_I2C_SMBUS) += i2c-smbus.o
    > obj-$(CONFIG_I2C_CHARDEV) += i2c-dev.o
    > +obj-$(CONFIG_I2C_HID) += i2c-hid.o
    > obj-$(CONFIG_I2C_MUX) += i2c-mux.o
    > obj-y += algos/ busses/ muxes/
    >
    > diff --git a/drivers/i2c/i2c-hid.c b/drivers/i2c/i2c-hid.c
    > new file mode 100644
    > index 0000000..eb17d8c
    > --- /dev/null
    > +++ b/drivers/i2c/i2c-hid.c
    > @@ -0,0 +1,1027 @@
    > +/*
    > + * HID over I2C protocol implementation
    > + *
    > + * Copyright (c) 2012 Benjamin Tissoires <benjamin.tissoires@gmail.com>
    > + * Copyright (c) 2012 Ecole Nationale de l'Aviation Civile, France
    > + *
    > + * This code is partly based on "USB HID support for Linux":
    > + *
    > + * Copyright (c) 1999 Andreas Gal
    > + * Copyright (c) 2000-2005 Vojtech Pavlik <vojtech@suse.cz>
    > + * Copyright (c) 2005 Michael Haboustak <mike-@cinci.rr.com> for Concept2, Inc
    > + * Copyright (c) 2007-2008 Oliver Neukum
    > + * Copyright (c) 2006-2010 Jiri Kosina
    > + *
    > + * This file is subject to the terms and conditions of the GNU General Public
    > + * License. See the file COPYING in the main directory of this archive for
    > + * more details.
    > + */
    > +
    > +#include <linux/module.h>
    > +#include <linux/i2c.h>
    > +#include <linux/irq.h>
    > +#include <linux/gpio.h>
    > +#include <linux/interrupt.h>
    > +#include <linux/input.h>
    > +#include <linux/delay.h>
    > +#include <linux/slab.h>
    > +#include <linux/pm.h>
    > +
    > +#include <linux/i2c/i2c-hid.h>
    > +
    > +#include <linux/hid.h>
    > +#include <linux/hiddev.h>
    > +#include <linux/hidraw.h>
    > +
    > +#define DRIVER_NAME "i2chid"
    > +#define DRIVER_DESC "HID over I2C core driver"
    > +
    > +#define I2C_HID_COMMAND_TRIES 3
    > +
    > +/* flags */
    > +#define I2C_HID_STARTED (1 << 0)
    > +#define I2C_HID_OUT_RUNNING (1 << 1)
    > +#define I2C_HID_IN_RUNNING (1 << 2)
    > +#define I2C_HID_RESET_PENDING (1 << 3)
    > +#define I2C_HID_SUSPENDED (1 << 4)
    > +
    > +#define I2C_HID_PWR_ON 0x00
    > +#define I2C_HID_PWR_SLEEP 0x01
    > +
    > +/* debug option */
    > +static bool debug = false;
    > +module_param(debug, bool, 0444);
    > +MODULE_PARM_DESC(debug, "print a lot of debug informations");
    > +
    > +struct i2chid_desc {
    > + __le16 wHIDDescLength;
    > + __le16 bcdVersion;
    > + __le16 wReportDescLength;
    > + __le16 wReportDescRegister;
    > + __le16 wInputRegister;
    > + __le16 wMaxInputLength;
    > + __le16 wOutputRegister;
    > + __le16 wMaxOutputLength;
    > + __le16 wCommandRegister;
    > + __le16 wDataRegister;
    > + __le16 wVendorID;
    > + __le16 wProductID;
    > + __le16 wVersionID;
    > +} __packed;
    > +
    > +struct i2chid_cmd {
    > + enum {
    > + /* fecth HID descriptor */
    > + HID_DESCR_CMD,
    > +
    > + /* fetch report descriptors */
    > + HID_REPORT_DESCR_CMD,
    > +
    > + /* commands */
    > + HID_RESET_CMD,
    > + HID_GET_REPORT_CMD,
    > + HID_SET_REPORT_CMD,
    > + HID_GET_IDLE_CMD,
    > + HID_SET_IDLE_CMD,
    > + HID_GET_PROTOCOL_CMD,
    > + HID_SET_PROTOCOL_CMD,
    > + HID_SET_POWER_CMD,
    > +
    > + /* read/write data register */
    > + HID_DATA_CMD,
    > + } name;
    > + unsigned int registerIndex;
    > + __u8 opcode;
    > + unsigned int length;
    > + bool wait;
    > +};
    > +
    > +union command {
    > + u8 data[0];
    > + struct cmd {
    > + __le16 reg;
    > + __u8 reportTypeID;
    > + __u8 opcode;
    > + } __packed c;
    > +};
    > +
    > +#define I2C_HID_CMD(name_, opcode_) \
    > + .name = name_, .opcode = opcode_, .length = 4, \
    > + .registerIndex = offsetof(struct i2chid_desc, wCommandRegister)
    > +
    > +static const struct i2chid_cmd cmds[] = {
    > + { I2C_HID_CMD(HID_RESET_CMD, 0x01), .wait = true },
    > + { I2C_HID_CMD(HID_GET_REPORT_CMD, 0x02) },
    > + { I2C_HID_CMD(HID_SET_REPORT_CMD, 0x03) },
    > + { I2C_HID_CMD(HID_GET_IDLE_CMD, 0x04) },
    > + { I2C_HID_CMD(HID_SET_IDLE_CMD, 0x05) },
    > + { I2C_HID_CMD(HID_GET_PROTOCOL_CMD, 0x06) },
    > + { I2C_HID_CMD(HID_SET_PROTOCOL_CMD, 0x07) },
    > + { I2C_HID_CMD(HID_SET_POWER_CMD, 0x08) },
    > +
    > + {.name = HID_DESCR_CMD,
    > + .length = 2},
    > +
    > + {.name = HID_REPORT_DESCR_CMD,
    > + .registerIndex = offsetof(struct i2chid_desc, wReportDescRegister),
    > + .opcode = 0x00,
    > + .length = 2},
    > +
    > + {.name = HID_DATA_CMD,
    > + .registerIndex = offsetof(struct i2chid_desc, wDataRegister),
    > + .opcode = 0x00,
    > + .length = 2},
    > +
    > + {}, /* terminating */
    > +};
    > +
    > +/* The main device structure */
    > +struct i2chid {
    > + struct i2c_client *client; /* i2c client */
    > + struct hid_device *hid; /* pointer to corresponding HID dev */
    > + union {
    > + __u8 hdesc_buffer[sizeof(struct i2chid_desc)];
    > + struct i2chid_desc hdesc; /* the HID Descriptor */
    > + };
    > + __le16 wHIDDescRegister; /* location of the i2c
    > + * register of the HID
    > + * descriptor. */
    > + unsigned int bufsize; /* i2c buffer size */
    > + char *inbuf; /* Input buffer */
    > +
    > + spinlock_t flock; /* flags spinlock */
    > + unsigned long flags; /* device flags */
    > +
    > + int irq; /* the interrupt line irq */
    > +
    > + wait_queue_head_t wait; /* For waiting the interrupt */
    > +};
    > +
    > +void i2chid_print_buffer(struct i2chid *ihid, u8 *buf, unsigned int n)
    > +{
    > + int i;
    > + char *string = kzalloc((n*3+1)*sizeof(char), GFP_KERNEL);
    > + char tmpbuf[4] = "";
    > +
    > + for (i = 0; i < n; ++i) {
    > + sprintf(tmpbuf, "%02x ", buf[i] & 0xFF);
    > + strcat(string, tmpbuf);
    > + }
    > +
    > + dev_err(&ihid->client->dev, "%s\n", string);
    > + kfree(string);
    > +}
    > +
    > +static int __i2chid_command(struct i2c_client *client, int command, u8 reportID,
    > + u8 reportType, u8 *args, int args_len,
    > + unsigned char *buf_recv, int data_len)
    > +{
    > + struct i2chid *ihid = i2c_get_clientdata(client);
    > + union command *cmd;
    > + unsigned char *rec_buf = buf_recv;
    > + int ret;
    > + int tries = I2C_HID_COMMAND_TRIES;
    > + int length = 0;
    > + struct i2c_msg msg[2];
    > + int msg_num = 0;
    > + int i;
    > + bool wait = false;
    > +
    > + if (WARN_ON(!ihid))
    > + return -ENODEV;
    > +
    > + cmd = kmalloc(args_len + sizeof(union command), GFP_KERNEL);
    > + if (!cmd)
    > + return -ENOMEM;
    > +
    > + for (i = 0; cmds[i].length; i++) {
    > + unsigned int registerIndex;
    > +
    > + if (cmds[i].name != command)
    > + continue;
    > +
    > + registerIndex = cmds[i].registerIndex;
    > + cmd->data[0] = ihid->hdesc_buffer[registerIndex];
    > + cmd->data[1] = ihid->hdesc_buffer[registerIndex + 1];
    > + cmd->c.opcode = cmds[i].opcode;
    > + length = cmds[i].length;
    > + wait = cmds[i].wait;
    > + }
    > +
    > + /* special case for HID_DESCR_CMD */
    > + if (command == HID_DESCR_CMD)
    > + cmd->c.reg = ihid->wHIDDescRegister;
    > +
    > + cmd->c.reportTypeID = reportID | reportType << 4;
    > +
    > + memcpy(cmd->data + length, args, args_len);
    > + length += args_len;
    > +
    > + if (debug) {
    > + char tmpstr[4] = "";
    > + char strbuf[50] = "";
    > + int i;
    > + for (i = 0; i < length; i++) {
    > + sprintf(tmpstr, "%02x ", cmd->data[i] & 0xFF);
    > + strcat(strbuf, tmpstr);
    > + }
    > + dev_err(&client->dev, "%s, cmd=%s\n", __func__, strbuf);
    > + }
    > +
    > + msg[0].addr = client->addr;
    > + msg[0].flags = client->flags & I2C_M_TEN;
    > + msg[0].len = length;
    > + msg[0].buf = (char *) cmd->data;
    > + msg[1].addr = client->addr;
    > + msg[1].flags = client->flags & I2C_M_TEN;
    > + msg[1].flags |= I2C_M_RD;
    > + msg[1].len = data_len;
    > + msg[1].buf = rec_buf;
    > +
    > + msg_num = data_len > 0 ? 2 : 1;
    > +
    > + if (wait) {
    > + spin_lock_irq(&ihid->flock);
    > + set_bit(I2C_HID_RESET_PENDING, &ihid->flags);
    > + spin_unlock_irq(&ihid->flock);
    > + }
    > +
    > + do {
    > + ret = i2c_transfer(client->adapter, msg, msg_num);
    > + if (ret > 0)
    > + break;
    > + tries--;
    > + dev_dbg(&client->dev, "retrying i2chid_command:%d (%d)\n",
    > + command, tries);
    > + } while (tries > 0);

    Do we have to do this retry here? In i2c_transfer() it already does
    retry automatically on arbitration loss (please see __i2c_transfer() in
    drivers/i2c/i2c-core.c) that's defined by individual bus driver. Maybe
    we don't have to retry here and make the code a bit simpler.

    Thanks,
    JJ

    > + if (wait && ret > 0) {
    > + if (debug)
    > + dev_err(&client->dev, "%s: waiting....\n", __func__);
    > + if (!wait_event_timeout(ihid->wait,
    > + !test_bit(I2C_HID_RESET_PENDING, &ihid->flags),
    > + msecs_to_jiffies(5000)))
    > + ret = -ENODATA;
    > + if (debug)
    > + dev_err(&client->dev, "%s: finished.\n", __func__);
    > + }
    > +
    > + kfree(cmd);
    > +
    > + return ret > 0 ? ret != msg_num : ret;
    > +}
    > +
    > +static int i2chid_command(struct i2c_client *client, int command,
    > + unsigned char *buf_recv, int data_len)
    > +{
    > + return __i2chid_command(client, command, 0, 0, NULL, 0,
    > + buf_recv, data_len);
    > +}


    \
     
     \ /
      Last update: 2012-10-03 05:41    [W:2.964 / U:0.184 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site