Re: [PATCH v7 1/2] ECI: input: introduce ECI accessory input driver

From: Tapio Vihuri
Date: Mon Feb 21 2011 - 05:29:26 EST

Next message: Herbert Xu: "Re: [PATCH] random: update interface comments to reflect reality"
Previous message: David Cohen: "[PATCH 0/1] Fix linux/wait.h header file"
In reply to: Dmitry Torokhov: "Re: [PATCH v7 1/2] ECI: input: introduce ECI accessory input driver"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi.

Thank you for comments.

On Sat, 2011-02-19 at 01:22 -0800, ext Dmitry Torokhov wrote:
> Hi Tapio,
>
> On Tue, Feb 15, 2011 at 03:11:59PM +0200, tapio.vihuri@xxxxxxxxx wrote:
> > From: Tapio Vihuri <tapio.vihuri@xxxxxxxxx>
> >
> > ECI stands for (Enhancement Control Interface).
> >
> > ECI is better known as Multimedia Headset for Nokia phones.
> > If headset has many buttons, like play, vol+, vol- etc. then it is propably
> > ECI accessory.
> > Among multiple buttons ECI accessory contains memory for storing several
> > parameters.
> >
> > ECI input driver provides the following features:
> > - reading ECI configuration memory
> > - ECI buttons as input events
> > - ECI i2c-controller part as a bridge between host CPU I2C and
> > ECI accessory communication
> >
>
> I think it looks much better now, however I bet we can make it even
> better ;)

Yes.
> > Signed-off-by: Tapio Vihuri <tapio.vihuri@xxxxxxxxx>
> > ---
> > drivers/input/misc/Kconfig | 47 ++
> > drivers/input/misc/Makefile | 3 +-
> > drivers/input/misc/eci.c | 851 +++++++++++++++++++++++++++++++++++++
> > drivers/input/misc/eci_at20-i2c.c | 713 +++++++++++++++++++++++++++++++
> > include/linux/input/eci.h | 189 ++++++++
> > 5 files changed, 1802 insertions(+), 1 deletions(-)
> > create mode 100644 drivers/input/misc/eci.c
> > create mode 100644 drivers/input/misc/eci_at20-i2c.c
> > create mode 100644 include/linux/input/eci.h
> >
> > diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> > index b0c6772..2f02eda 100644
> > --- a/drivers/input/misc/Kconfig
> > +++ b/drivers/input/misc/Kconfig
> > @@ -114,6 +114,53 @@ config INPUT_APANEL
> > To compile this driver as a module, choose M here: the module will
> > be called apanel.
> >
> > +config INPUT_ECI
> > + tristate "AV ECI (Enhancement Control Interface) input driver"
> > + default n
>
> No need to specify "default n" - 'n' is the default.

OK.
>
> > + select INPUT_SPARSEKMAP
> > + help
> > + The Enhancement Control Interface functionality
> > + ECI is better known as Multimedia Headset for Nokia phones.
> > + If headset has many buttons, like play, vol+, vol- etc. then
> > + it is propably ECI accessory.
> > + Among multiple buttons ECI accessory contains memory for storing
> > + several parameters.
> > +
> > + ECI input driver provides the following features:
> > + - reading ECI configuration memory
> > + - ECI buttons as input events
> > +
> > + ECI input driver needs some ECI controller method, select
> > + appropriate controller below.
> > +
> > + Say 'y' here to statically link this module into the kernel or 'm'
> > + to build it as a dynamically loadable module. The module will be
> > + called eci.ko
> > +
> > +config ECI_DEBUG
> > + bool "ECI driver debug"
> > + depends on INPUT_ECI
> > + help
> > + Say Y here to enable debugging messages for ECI input driver.
> > +
> > + Ease understanding ECI accessory configuration embedded in
> > + accessory's memory.
> > + Add memory and buttons parsing info to module eci.ko
> > +
>
> Do we need it as a config option? Do we expect users recompile the
> driver with debug? Maybe just leave #define DEBUG commented out in
> eci.c?

OK for me.
>
> > +config INPUT_ECI_AT20
> > + tristate "ECI controller method"
>
> It should probably say "Support for ECI connected via AT20 I2C chip" or
> something similar.
>

I figure out more descriptive prompt.
> > + depends on INPUT_ECI && I2C && X86_MRST && INPUT_MISC
>
> Does it really have to depend on MRST? Also, dependency on INPUT_MISC is
> not needed, you won't descend here if INPUT_MISC is disabled.
>

Currently it's only in MRST platform, but it could be in other platforms
also.
> > + default y
>
> No, no default 'y' please.

OK.
>
> > + help
> > + This selects the used ECI controller
> > +
> > + ECI controller is kind of bridge between host CPU I2C and
> > + ECI accessory ECI communication.
>
> Need better description as well.

OK.
>
> > +
> > + Say 'y' here to statically link this module into the kernel or 'm'
> > + to build it as a dynamically loadable module. The module will be
> > + called eci_at20-i2c.ko
> > +
> > config INPUT_IXP4XX_BEEPER
> > tristate "IXP4XX Beeper support"
> > depends on ARCH_IXP4XX
> > diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
> > index 9b47971..7631b33 100644
> > --- a/drivers/input/misc/Makefile
> > +++ b/drivers/input/misc/Makefile
> > @@ -23,6 +23,8 @@ obj-$(CONFIG_INPUT_CMA3000_I2C) += cma3000_d0x_i2c.o
> > obj-$(CONFIG_INPUT_COBALT_BTNS) += cobalt_btns.o
> > obj-$(CONFIG_INPUT_DM355EVM) += dm355evm_keys.o
> > obj-$(CONFIG_HP_SDC_RTC) += hp_sdc_rtc.o
> > +obj-$(CONFIG_INPUT_ECI) += eci.o
> > +obj-$(CONFIG_INPUT_ECI_AT20) += eci_at20-i2c.o
> > obj-$(CONFIG_INPUT_IXP4XX_BEEPER) += ixp4xx-beeper.o
> > obj-$(CONFIG_INPUT_KEYSPAN_REMOTE) += keyspan_remote.o
> > obj-$(CONFIG_INPUT_M68K_BEEP) += m68kspkr.o
> > @@ -43,4 +45,3 @@ obj-$(CONFIG_INPUT_UINPUT) += uinput.o
> > obj-$(CONFIG_INPUT_WISTRON_BTNS) += wistron_btns.o
> > obj-$(CONFIG_INPUT_WM831X_ON) += wm831x-on.o
> > obj-$(CONFIG_INPUT_YEALINK) += yealink.o
> > -
> > diff --git a/drivers/input/misc/eci.c b/drivers/input/misc/eci.c
> > new file mode 100644
> > index 0000000..81a5c20
> > --- /dev/null
> > +++ b/drivers/input/misc/eci.c
> > @@ -0,0 +1,851 @@
> > +/*
> > + * This file is part of ECI (Enhancement Control Interface) accessory input
> > + * driver
> > + *
> > + * Copyright (C) 2011 Nokia Corporation and/or its subsidiary(-ies).
> > + *
> > + * Contact: Tapio Vihuri <tapio.vihuri@xxxxxxxxx>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License
> > + * version 2 as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful, but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> > + * 02110-1301 USA
> > + *
>
> Probably should stop quoting the address in case FSF moves.

Company policy ;) I see what I can do.
>
> > + */
> > +
> > +/*
> > + * ECI stands for (Enhancement Control Interface).
> > + *
> > + * ECI is better known as Multimedia Headset for Nokia phones.
> > + * If headset has many buttons, like play, vol+, vol- etc. then it is propably
> > + * ECI accessory.
> > + * Among several buttons ECI accessory contains memory for storing several
> > + * parameters.
> > + *
> > + * ECI input driver provides the following features:
> > + * - reading ECI configuration memory
> > + * - ECI buttons as input events
> > + */
> > +
> > +#if defined(CONFIG_ECI_DEBUG)
> > +#define DEBUG
> > +#endif
> > +
> > +#include <linux/init.h>
> > +#include <linux/device.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/slab.h>
> > +#include <linux/i2c.h>
> > +
> > +#include <linux/input.h>
> > +#include <linux/input/sparse-keymap.h>
> > +#include <linux/input/eci.h>
> > +
> > +#define ECI_DRIVERNAME "ECI_accessory"
> > +
> > +#define ECI_WAIT_SEND_BUTTON 5 /* ms */
> > +#define ECI_WAIT_BUS_SETTLE 40 /* ms */
> > +#define ECI_TRY_GET_MEMORY 2000 /* ms */
> > +#define ECI_TRY_INIT_IO 200 /* ms */
> > +#define ECI_TRY_SET_MIC 200 /* ms */
> > +#define ECI_KEY_REPEAT_INTERVAL 400 /* ms */
> > +
> > +#define ECI_EKEY_BLOCK_ID 0xb3
> > +#define ECI_ENHANCEMENT_FEATURE_BLOCK_ID 0x02
> > +
> > +/* Map ECI inputs to events */
> > +static const struct key_entry eci_keymap[] = {
> > + { KE_KEY, 0x00, { KEY_UNKNOWN } }, /* No feature */
> > + { KE_KEY, 0x01, { KEY_UNKNOWN } }, /* Ignition Sense */
> > + { KE_KEY, 0x02, { KEY_UNKNOWN } }, /* Car-Kit Handset Hook */
> > + { KE_KEY, 0x03, { KEY_BATTERY } }, /* Power Supply/Car Battery Det */
> > + { KE_KEY, 0x06, { KEY_AUDIO } }, /* External audio In */
> > + { KE_KEY, 0x07, { KEY_PHONE } }, /* Send, End, and Voice Recogn */
> > + { KE_VSW, 0x08, { .sw = { SW_HEADPHONE_INSERT} } }, /* Headphone plug */
> > + { KE_KEY, 0x0a, { KEY_UNKNOWN } }, /* Device Power Request */
> > + { KE_KEY, 0x0b, { KEY_VOLUMEUP } }, /* Volume Up */
> > + { KE_KEY, 0x0c, { KEY_VOLUMEDOWN } }, /* Volume Down */
> > + { KE_KEY, 0x0d, { KEY_PLAYPAUSE } }, /* Play / Pause */
> > + { KE_KEY, 0x0e, { KEY_STOP } }, /* Stop */
> > + { KE_KEY, 0x0f, { KEY_FORWARD } }, /* Next/Fast Fward/Autosearch up */
> > + { KE_KEY, 0x10, { KEY_REWIND } }, /* Prev/Rewind/Autosearch down */
> > + { KE_KEY, 0x11, { KEY_UNKNOWN } }, /* Push to Talk over Cellular */
> > + { KE_KEY, 0x14, { KEY_UNKNOWN } }, /* Synchronization Button */
> > + { KE_KEY, 0x15, { KEY_RADIO } }, /* Music/Radio/Off Selector */
> > + { KE_KEY, 0x16, { KEY_UNKNOWN } }, /* Redial */
> > + { KE_KEY, 0x17, { KEY_UNKNOWN } }, /* Left Soft Key */
> > + { KE_KEY, 0x18, { KEY_UNKNOWN } }, /* Right Soft key */
> > + { KE_KEY, 0x19, { KEY_SEND } }, /* Send key */
> > + { KE_KEY, 0x1a, { KEY_END } }, /* End key */
> > + { KE_KEY, 0x1b, { KEY_UNKNOWN } }, /* Middle Soft key */
> > + { KE_KEY, 0x1c, { KEY_UP } }, /* UP key/joystick direction */
> > + { KE_KEY, 0x1d, { KEY_DOWN } }, /* DOWN key/joystick direction */
> > + { KE_KEY, 0x1e, { KEY_RIGHT } }, /* RIGHT key/joystick direction */
> > + { KE_KEY, 0x1f, { KEY_LEFT } }, /* LEFT key/joystick direction */
> > + { KE_KEY, 0x20, { KEY_UNKNOWN } }, /* Symbian Application key */
> > + { KE_KEY, 0x21, { KEY_UNKNOWN } }, /* Terminal Applicat Ctrl Input */
> > + { KE_KEY, 0x23, { KEY_UNKNOWN } }, /* USB Class Switching */
> > + { KE_KEY, 0x24, { KEY_MUTE } }, /* Mute */
> > + { KE_END, 0 },
> > +};
> > +
> > +#define ECI_KEY_MAX 0x24
> > +
> > +#define ECI_INT_ENABLE 1
> > +#define ECI_INT_DELAY_ENABLE (1<<1)
> > +#define ECI_INT_LEN_76MS 0
> > +#define ECI_INT_LEN_82MS (1<<5)
> > +#define ECI_INT_LEN_37MS (2<<5)
> > +#define ECI_INT_LEN_19MS (3<<5)
> > +#define ECI_INT_LEN_10MS (4<<5)
> > +#define ECI_INT_LEN_5MS (5<<5)
> > +#define ECI_INT_LEN_2MS (6<<5)
> > +#define ECI_INT_LEN_120US (7<<5)
> > +
> > +struct eci_mem_block {
> > + u8 id;
> > + u8 len;
> > + u16 size;
> > +};
> > +
> > +static struct eci_cb eci_callback;
> > +static struct audio_hsmic_event hsmic_event;
> > +
> > +static struct eci_data *the_eci;
>
> The statics need to go away.

OK, will do.
>
> > +
> > +/* Returns size of accessory memory or error */
> > +static int eci_get_ekey(struct eci_data *eci, int *key)
> > +{
> > + u8 buf[4];
> > + struct eci_mem_block *ekey = (void *)buf;
>
> Instead of casting maybe define eci_mem_block as an union?

Hm, I take K&R and see what I can do.
>
> > + int ret;
> > +
> > + /* Read always four bytes */
> > + ret = eci->eci_hw_ops->acc_read_direct(0, buf);
> > +
> > + if (ret)
> > + return ret;
> > +
> > + if (ekey->id != ECI_EKEY_BLOCK_ID)
> > + return -ENODEV;
> > +
> > + *key = cpu_to_be16(ekey->size);
>
> I know I already asked this, but it still does not look right to me. You
> read some data off the wire and you assume it to be in _CPU_ endianness but
> convert to be16? And later you use the data (compare, etc) as if it was
> in CPU endianness.
>
> I think the structure definition should be as follows:
>
> union eci_mem_block {
> struct {
> u8 id;
> u8 len;
> __be16 size;
> };
> u8 raw[4];
> };
>
> and then you do
>
> *mem_size = be16_to_cpu(ekey.size);

So I made it wrong way, I see. OK, I verify my logic and ashame a litle
bit.
> BTW, why is it called eci_get_ekey() and not eci_get_memory_size()?

The block name in ECI specification is ekey, altough it's really just
mem size.
>
> > +
> > + return 0;
> > +}
> > +
> > +/* Read ECI device memory into buffer */
> > +static int eci_get_memory(struct eci_data *eci, int *restart)
> > +{
> > + int i, ret;
> > +
> > + for (i = *restart; i < eci->mem_size; i += 4) {
> > + ret = eci->eci_hw_ops->acc_read_direct(i, eci->memory + i);
> > + *restart = i;
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +/*
> > + * This should be really init_features, but most oftens these are just buttons
> > + */
> > +static int eci_init_buttons(struct eci_data *eci)
> > +{
> > + struct enchancement_features_fixed *eff = eci->e_features_fix;
> > + u8 n, mireg;
> > + int ret;
> > + u8 buf[4];
> > +
> > + n = eff->number_of_features;
> > +
> > + if (n > ECI_MAX_FEATURE_COUNT)
> > + return -EINVAL;
> > +
> > + ret = eci->eci_hw_ops->acc_write_reg(ECICMD_MIC_CTRL, ECI_MIC_OFF);
> > + if (ret)
> > + return ret;
> > +
> > + ret = eci->eci_hw_ops->acc_read_reg(ECICMD_MASTER_INT_REG, buf, 1);
> > + if (ret)
> > + return ret;
> > +
> > + mireg = buf[0];
> > + mireg &= ~ECI_INT_ENABLE;
> > + mireg |= ECI_INT_LEN_120US | ECI_INT_DELAY_ENABLE;
> > +
> > + ret = eci->eci_hw_ops->acc_write_reg(ECICMD_MASTER_INT_REG, mireg);
> > + if (ret)
> > + return ret;
> > +
> > + msleep(ECI_WAIT_BUS_SETTLE);
> > + mireg |= ECI_INT_ENABLE;
> > + ret = eci->eci_hw_ops->acc_write_reg(ECICMD_MASTER_INT_REG, mireg);
> > + if (ret)
> > + return ret;
> > +
> > + msleep(ECI_WAIT_BUS_SETTLE);
> > +
> > + return ret;
> > +}
> > +
> > +/* Find "enchangement features" block from buffer */
> > +static int eci_get_enchancement_features(struct eci_data *eci)
> > +{
> > + u8 *mem = (void *)eci->memory;
> > + struct eci_mem_block *b = (void *)mem;
> > + struct eci_mem_block *mem_end = (void *)(eci->memory + eci->mem_size);
> > +
> > + if (b->id != ECI_EKEY_BLOCK_ID)
> > + return -ENODEV;
> > +
> > + do {
> > + dev_dbg(eci->dev, "skip BLOCK 0x%02x, LEN 0x%02x\n",
> > + b->id, b->len);
> > + if (!b->len)
> > + return -EINVAL;
> > +
> > + mem += b->len;
> > + b = (void *)mem;
> > + eci->e_features_fix = (void *)b;
>
> Amount of casting here makes me cry :(

Now I'm plushing a litle bit :). OK, I try to focus on readability and
find better solution.
>
> > + dev_dbg(eci->dev, "found BLOCK 0x%02x, LEN 0x%02x\n",
> > + b->id, b->len);
> > + if (b->id == ECI_ENHANCEMENT_FEATURE_BLOCK_ID)
> > + return 0;
> > + } while (b < mem_end);
> > +
> > + dev_dbg(eci->dev, "can't find ECI enchangement features block\n");
> > +
> > + return -ENFILE;
> > +}
> > +
> > +/*
> > + * Find out ECI features.
> > + * All ECI memory block parsing are done here, be carefull as
> > + * pointers to memory tend to go wrong easily.
> > + * ECI "Enhancement Features block has variable size, so we try to
> > + * catch pointers out of block due memory reading errors etc.
> > + *
> > + * I/O support field is not implemented.
> > + * Data direction field is not implemented, nor writing to the ECI I/O
> > + */
> > +static int eci_parse_enchancement_features(struct eci_data *eci)
> > +{
> > + struct enchancement_features_fixed *eff = eci->e_features_fix;
> > + struct enchancement_features_variable *efv = &eci->e_features_var;
> > + int i;
> > + u8 n, k;
> > + void *mem_end = (void *)((u8 *)eff + eff->length);
> > +
> > + dev_dbg(eci->dev, "block id 0x%02x length 0x%02x connector "
> > + "configuration 0x%02x\n", eff->block_id, eff->length,
> > + eff->connector_conf);
> > + n = eff->number_of_features;
> > + dev_dbg(eci->dev, "number of features %d\n", n);
> > +
> > + if (n > ECI_MAX_FEATURE_COUNT) {
> > + dev_dbg(eci->dev, "number of features out of range\n");
> > + return -EINVAL;
> > + }
> > +
> > + k = DIV_ROUND_UP(n, 8);
> > + dev_dbg(eci->dev, "I/O support bytes count %d\n", k);
> > +
> > + efv->io_support = &eff->number_of_features + 1;
> > + /* efv->io_functionality[0] is not used! pins are in 1..31 range */
> > + efv->io_functionality = efv->io_support + k - 1;
> > + efv->active_state = efv->io_functionality + n + 1;
> > +
> > + if ((void *)&efv->active_state[k] > mem_end) {
> > + dev_dbg(eci->dev, "I/O support bytes points out of memory\n");
> > + return -EINVAL;
> > + }
> > +
> > + /* Last part of block */
> > + for (i = 0; i < k; i++)
> > + dev_dbg(eci->dev, "active_state[%d] 0x%02x\n", i,
> > + efv->active_state[i]);
> > +
> > + eci->buttons_data.buttons_up_mask =
> > + ~(u32)(cpu_to_le32(*(u32 *)efv->active_state));
>
> Hmm... you need to properly annotate all members that are not in cpu
> endianness.

OK.
>
> > + dev_dbg(eci->dev, "buttons mask 0x%08x\n",
> > + eci->buttons_data.buttons_up_mask);
> > +
> > + /*
> > + * ECI accessory responces as many bytes needed for used I/O pins
> > + * up to four bytes, when lines 24..31 are used
> > + * all tested ECI accessories how ever return two data bytes
> > + * event though there are less than eight I/O pins
> > + *
> > + * so we get alway reading error if there are less than eight I/Os
> > + * meanwhile just use this kludge, FIXME
> > + */
> > + k = DIV_ROUND_UP(n + 1, 8);
> > + if (k > 4) {
> > + dev_dbg(eci->dev, "maximum port reg count is four\n");
> > + return -EINVAL;
> > + }
> > +
> > + if (k == 1)
> > + k = 2;
> > + eci->port_reg_count = k;
> > +
> > + return 0;
> > +}
> > +
> > +static int eci_init_accessory(struct eci_data *eci)
> > +{
> > + int ret, key = 0, restart = 0;
> > + unsigned long future;
> > +
> > + eci->mem_ok = false;
> > +
> > + if (!eci->eci_hw_ops)
> > + return -ENXIO;
>
> Shouldn't you check this earlier, when registering the accessory?

Yes. I make sure that there is not possibility to get here if not
registered properly and remove this checking.
>
> > +
> > + ret = eci->eci_hw_ops->acc_reset();
> > + if (ret)
> > + return ret;
> > +
> > + msleep(ECI_WAIT_BUS_SETTLE);
> > +
> > + ret = eci->eci_hw_ops->acc_write_reg(ECICMD_MIC_CTRL, ECI_MIC_OFF);
> > + if (ret) {
> > + dev_dbg(eci->dev, "unable to turn ECI mic off\n");
> > + return ret;
> > + }
> > +
> > + /* Get ECI ekey block to determine memory size */
> > + future = jiffies + msecs_to_jiffies(ECI_TRY_GET_MEMORY);
> > + do {
> > + ret = eci_get_ekey(eci, &key);
> > + if (time_is_before_jiffies(future))
> > + break;
> > + } while (ret);
>
> So why do you need the retries? How often does the device go out for
> lunch?

Way too often. This is related to the used controller.
>
> > +
> > + if (ret) {
> > + dev_dbg(eci->dev, "can't read ECI memory ekey block\n");
> > + return ret;
> > + }
> > +
> > + eci->mem_size = key;
> > + if (eci->mem_size > ECI_MAX_MEM_SIZE) {
> > + dev_dbg(eci->dev, "reported ECI memory size out of range\n");
> > + return -EINVAL;
> > + }
> > +
> > + /* Get ECI memory */
> > + future = jiffies + msecs_to_jiffies(ECI_TRY_GET_MEMORY);
> > + do {
> > + ret = eci_get_memory(eci, &restart);
> > + if (time_is_before_jiffies(future))
> > + break;
> > + } while (ret);
> > +
> > + if (ret) {
> > + dev_dbg(eci->dev, "can't read ECI memory\n");
> > + return ret;
> > + }
> > +
> > + if (eci_get_enchancement_features(eci))
> > + return -EIO;
> > +
> > + if (eci_parse_enchancement_features(eci))
> > + return -EIO;
> > +
> > + /*
> > + * Now that enchancement features table has been parsed,
> > + * we can configure ECI buttons.
> > + */
> > + msleep(ECI_WAIT_BUS_SETTLE);
> > + future = jiffies + msecs_to_jiffies(ECI_TRY_INIT_IO);
> > + do {
> > + ret = eci_init_buttons(eci);
> > + if (time_is_before_jiffies(future))
> > + break;
> > + } while (ret);
> > +
> > + if (ret) {
> > + dev_dbg(eci->dev, "can't init ECI buttons\n");
> > + return ret;
> > + }
> > +
> > + eci->mem_ok = true;
> > + msleep(ECI_WAIT_BUS_SETTLE);
> > +
> > + if (eci->eci_hw_ops->acc_write_reg(ECICMD_MIC_CTRL, eci->mic_state))
> > + dev_err(eci->dev, "Unable to control headset microphone\n");
> > +
> > + return 0;
> > +}
> > +
> > +/* Press/release accessory button(s) */
> > +static int eci_get_button(struct eci_data *eci)
> > +{
> > + struct enchancement_features_fixed *eff = eci->e_features_fix;
> > + struct eci_buttons_data *b = &eci->buttons_data;
> > +
> > + if (!eci->mem_ok)
> > + return -ENXIO;
> > +
> > + if (((b->buttons & b->buttons_up_mask) == 0) &&
> > + (eff->number_of_features > 2)) {
> > + dev_err(eci->dev, "ECI report all buttons down, rejected\n");
> > + return -EINVAL;
> > + }
> > +
> > + if (b->windex < ECI_BUTTON_BUF_SIZE) {
> > + if (b->buttons_buf[b->windex] == 0)
> > + b->buttons_buf[b->windex] = b->buttons;
> > + else
> > + dev_err(eci->dev, "ECI button queue owerflow\n");
> > + }
> > + b->windex++;
> > + if (b->windex == ECI_BUTTON_BUF_SIZE)
> > + b->windex = 0;
> > +
> > + return 0;
> > +}
> > +
> > +/* Intended to use ONLY inside eci_parse_button() ! */
> > +#define ACTIVE_STATE(x) (u32)(cpu_to_le32(*(u32 *)efv->active_state) & BIT(x-1))
> > +#define BUTTON_STATE(x) ((buttons & BIT(x))>>1)
> > +
> > +static int eci_parse_button(struct eci_data *eci, u32 buttons)
> > +{
> > + int pin, state;
> > + u8 n, io_fun;
> > + struct enchancement_features_variable *efv = &eci->e_features_var;
> > + struct enchancement_features_fixed *eff = eci->e_features_fix;
> > +
> > + if (!eci->mem_ok)
> > + return -ENXIO;
> > +
> > + n = eff->number_of_features;
> > +
> > + for (pin = 1; pin <= n; pin++) {
> > + io_fun = efv->io_functionality[pin] & ~BIT(7);
> > + if (io_fun > ECI_KEY_MAX)
> > + break;
> > + state = (BUTTON_STATE(pin) == ACTIVE_STATE(pin));
> > + if (state)
> > + dev_dbg(eci->dev, "I/O functionality 0x%02x\n", io_fun);
> > +
> > + sparse_keymap_report_event(eci->acc_input, io_fun, state,
> > + false);
> > + }
> > + input_sync(eci->acc_input);
> > +
> > + return 0;
> > +}
> > +static int eci_send_button(struct eci_data *eci)
> > +{
> > + int i;
> > + struct enchancement_features_fixed *eff = eci->e_features_fix;
> > + struct eci_buttons_data *b = &eci->buttons_data;
> > + u8 n;
> > +
> > + if (!eci->mem_ok)
> > + return -ENXIO;
> > +
> > + n = eff->number_of_features;
> > +
> > + if (n > ECI_MAX_FEATURE_COUNT)
> > + return -EINVAL;
> > +
> > + /* Let input system take care multiple key events */
> > + for (i = 0; i < ECI_BUTTON_BUF_SIZE; i++) {
> > + if (b->buttons_buf[b->rindex] == 0)
> > + break;
> > +
> > + if (eci_parse_button(eci, b->buttons_buf[b->rindex]))
> > + return -ENXIO;
> > +
> > + b->buttons_buf[b->rindex] = 0;
> > + b->rindex++;
> > + if (b->rindex == ECI_BUTTON_BUF_SIZE)
> > + b->rindex = 0;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +/* General work func (eci_ws) for several tasks */
> > +static void eci_work(struct work_struct *ws)
> > +{
> > + struct eci_data *eci;
> > + int ret;
> > +
> > + eci = container_of((struct delayed_work *)ws, struct eci_data, eci_ws);
>
> No, please do not case struct work_struct to struct delayed_work. Use
>
> eci = container_of(ws, struct eci_data, eci_ws.work);

This was due sparse reported error. I check what is the situation with
latest kernel.
>
> > +
> > + ret = eci_send_button(eci);
> > + if (ret)
> > + dev_err(eci->dev, "Error sending event\n");
> > +}
> > +
> > +/* This is called from platform audio driver when microphone routings changes */
> > +static void eci_hsmic_event(void *priv, bool on)
>
> eci_hsmic_control() would probably be better.

I guess the name was due ALSA naming conventions, but I check if it's OK
to change thge name.
>
> > +{
> > + struct eci_data *eci = priv;
> > + unsigned long future;
> > + int ret;
> > +
> > + if (!eci)
> > + return;
>
> How can we lose eci?

I think I was afraid this get called before ECI was initialized. I check
is it even possible.
>
> > +
> > + if (!eci->plugged)
> > + return;
> > +
> > + if (on)
> > + eci->mic_state = ECI_MIC_AUTO;
> > + else
> > + eci->mic_state = ECI_MIC_OFF;
>
>
> eci->mix_state = on ? ECI_MIC_AUTO : ECI_MIC_OFF;

OK.
>
> > +
> > + future = jiffies + msecs_to_jiffies(ECI_TRY_SET_MIC);
> > + do {
> > + ret = eci->eci_hw_ops->acc_write_reg(ECICMD_MIC_CTRL,
> > + eci->mic_state);
> > + if (time_is_before_jiffies(future))
> > + break;
> > + } while (ret);
>
> Hmm, is it in the spec that you have to retry every operation for sevral
> times until it "stick"?

More due inapropriate HW.
>
> > +
> > + if (ret)
> > + dev_err(eci->dev, "Unable to control headset microphone\n");
> > +}
> > +
> > +/*
> > + * Other driver(s) can call this after registering themselves using
> > + * eci_register()
> > + */
> > +static void eci_accessory_event(int event, void *priv)
> > +{
> > + struct eci_data *eci = priv;
> > + struct eci_buttons_data *b = &eci->buttons_data;
> > + int delay = 0;
> > + int ret = 0;
> > +
> > + eci->event = event;
> > + switch (event) {
> > + case ECI_EVENT_IS_ECI:
> > + eci->is_eci = true;
> > + ret = eci->eci_hw_ops->acc_reset();
> > + if (ret)
> > + eci->is_eci = false;
> > + break;
> > + case ECI_EVENT_PLUG_IN:
> > + eci->first_event = true;
> > + ret = eci_init_accessory(eci);
> > + if (ret < 0)
> > + ret = eci_init_accessory(eci);
> > + if (ret) {
> > + dev_err(eci->dev, "Accessory init %s%s%s%s\n",
> > + ret & ACI_COMMERR ? "COMMERR " : "",
> > + ret & ACI_FRAERR ? "FRAERR " : "",
> > + ret & ACI_RESERR ? "RESERR " : "",
> > + ret & ACI_COLL ? "COLLERR " : "");
> > + break;
> > + }
> > + break;
> > + case ECI_EVENT_PLUG_OUT:
> > + /* send all buttons up as last event */
> > + b->buttons = b->buttons_up_mask;
> > + eci_get_button(eci);
> > + eci_send_button(eci);
> > + eci->mem_ok = false;
>
> Instead of checking mem_ok everywhere maybe you should simply unregister
> the device? We already have bind/unbind in sysfs to do just that.

I check out the proper method.
>
> > + break;
> > + case ECI_EVENT_BUTTON:
> > + /*
> > + * First event might not be valid due plug insertion
> > + * so we filter it out if it seems garbage
> > + */
> > + if (eci->first_event) {
> > + eci->first_event = false;
> > + if ((b->buttons & 0xff) == 0)
> > + break;
> > + }
> > + eci_get_button(eci);
> > + delay = msecs_to_jiffies(ECI_WAIT_SEND_BUTTON);
> > + schedule_delayed_work(&eci->eci_ws, delay);
>
> Why do you need to use work? I traced through the calls and it simply
> reports events through input subsystem which can be called from atomic
> contexts.

I put events to the buffer, and send them asynchronously. There was some
good reason to do it, but I just can't remember it now.

But I check it out and create desent comment about it. Or if the
mechanism is obsolete now, I remove it.

>
> Also, do we really need to have this demultiplexor? Can't we have 4
> functions doing one thing and doing it well?

For other tasks, we need to wait specified amount of time. I just didn't
want to create many delayed works.

Otherwise I like basic unix philosophy ;)
>
> > + break;
> > + default:
> > + dev_err(eci->dev, "Unknown accessory event %d\n", event);
> > + break;
> > + }
> > +
> > + return;
> > +}
> > +
> > +static void __eci_disable(struct eci_data *eci)
> > +{
> > + /* Disconnect ECI accessory's microphone */
> > + eci_hsmic_event(eci, false);
> > +}
> > +
> > +static void __eci_enable(struct eci_data *eci)
> > +{
> > + /* ECI accessory's microphone into auto mode */
> > + eci_hsmic_event(eci, true);
> > +}
> > +
> > +void eci_suspend(struct eci_data *eci)
> > +{
> > + mutex_lock(&eci->mutex);
> > +
> > + if (!eci->suspended && eci->opened)
> > + __eci_disable(eci);
> > +
> > + eci->suspended = true;
> > +
> > + mutex_unlock(&eci->mutex);
> > +}
> > +EXPORT_SYMBOL(eci_suspend);
> > +
> > +void eci_resume(struct eci_data *eci)
> > +{
> > + mutex_lock(&eci->mutex);
> > +
> > + if (eci->suspended && eci->opened)
> > + __eci_enable(eci);
> > +
> > + eci->suspended = false;
> > +
> > + mutex_unlock(&eci->mutex);
> > +}
> > +EXPORT_SYMBOL(eci_resume);
> > +
> > +/*
> > + * sysfs entries:
> > + * memory R ECI accessory memory content
> > + * cable RW accessory plugged/unplugged 0/1
> > + */
> > +static ssize_t show_eci_memory(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct eci_data *eci = the_eci;
>
> You should be able to access eci structure through dev_get_drvdata()
> instead of relying on global.

I make it so.
>
> > +
> > + if (!eci->mem_ok)
> > + return -ENXIO;
> > +
> > + memcpy(buf, eci->memory, eci->mem_size);
> > +
> > + return eci->mem_size;
> > +}
> > +
> > +static ssize_t show_cable_plugged(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct eci_data *eci = the_eci;
> > +
> > + return snprintf(buf, sizeof(buf), "%d\n", eci->plugged);
> > +}
> > +
> > +static ssize_t store_cable_plugged(struct device *dev,
> > + struct device_attribute *attr,
> > + const char *buf, size_t len)
> > +{
> > + struct eci_data *eci = the_eci;
> > + long unsigned int tmp;
> > +
> > + if (strict_strtoul(buf, 0, &tmp))
> > + return -EINVAL;
> > +
> > + eci->plugged = !!tmp;
> > + if (eci->plugged)
> > + eci_accessory_event(ECI_EVENT_PLUG_IN, eci);
> > + else
> > + eci_accessory_event(ECI_EVENT_PLUG_OUT, eci);
> > +
> > + return len;
> > +}
> > +
> > +static DEVICE_ATTR(memory, S_IRUGO, show_eci_memory, NULL);
> > +static DEVICE_ATTR(cable, S_IRUGO | S_IWUSR , show_cable_plugged,
> > + store_cable_plugged);
> > +
> > +static struct attribute *eci_attributes[] = {
> > + &dev_attr_memory.attr,
> > + &dev_attr_cable.attr,
> > + NULL
> > +};
> > +
> > +static struct attribute_group eci_attr_group = {
> > + .attrs = eci_attributes
> > +};
> > +
> > +static int eci_input_open(struct input_dev *input)
> > +{
> > + struct eci_data *eci = the_eci;
>
> struct eci_data *eci = input_get_drvdata(input);

As stated.
>
> > +
> > + mutex_lock(&eci->mutex);
> > +
> > + if (!eci->suspended)
> > + __eci_enable(eci);
> > +
> > + eci->opened = true;
> > +
> > + mutex_unlock(&eci->mutex);
> > +
> > + return 0;
> > +}
> > +
> > +static void eci_input_close(struct input_dev *input)
> > +{
> > + struct eci_data *eci = the_eci;
> > +
> > + mutex_lock(&eci->mutex);
> > +
> > + if (!eci->suspended)
> > + __eci_disable(eci);
> > +
> > + eci->opened = false;
> > +
> > + mutex_unlock(&eci->mutex);
> > +}
> > +
> > +static int init_accessory_input(struct eci_data *eci)
> > +{
> > + int err;
> > +
> > + eci->acc_input = input_allocate_device();
> > + if (!eci->acc_input) {
> > + dev_err(eci->dev, "Error allocating input device\n");
> > + return -ENOMEM;
> > + }
> > +
> > + eci->acc_input->name = "ECI Accessory";
> > + eci->acc_input->open = eci_input_open;
> > + eci->acc_input->close = eci_input_close;
>
> You also need to set eci_acc_input->dev.parent to ensure we are in
> proper place in sysfs tree.

I see to it.
>
> > +
> > + input_set_drvdata(eci->acc_input, eci);
> > +
> > + err = sparse_keymap_setup(eci->acc_input, eci_keymap, NULL);
> > + if (err)
> > + goto err_free_dev;
> > +
> > + __set_bit(EV_REP, eci->acc_input->evbit);
> > +
> > + err = input_register_device(eci->acc_input);
> > + if (err) {
> > + dev_err(eci->dev, "Error registering input device\n");
> > + goto err_free_keymap;
> > + }
> > +
> > + /* Must set after input_register_device() to take effect */
> > + eci->acc_input->rep[REP_PERIOD] = ECI_KEY_REPEAT_INTERVAL;
> > +
> > + return 0;
> > +
> > +err_free_keymap:
> > + sparse_keymap_free(eci->acc_input);
> > +err_free_dev:
> > + input_free_device(eci->acc_input);
> > + return err;
> > +}
> > +
> > +struct eci_cb *eci_register(struct device *dev, struct eci_hw_ops *eci_ops)
> > +{
> > + struct eci_data *eci;
> > + int ret;
> > +
> > + eci = kzalloc(sizeof(*eci), GFP_KERNEL);
> > + if (!eci)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + eci->dev = dev;
> > +
> > + the_eci = eci;
> > +
> > + if (!eci_ops || !eci_ops->acc_read_direct ||
> > + !eci_ops->acc_read_reg || !eci_ops->acc_write_reg ||
> > + !eci_ops->acc_reset ||
> > + !eci_ops->register_hsmic_event_cb)
> > + return ERR_PTR(-EINVAL);
> > +
> > + eci->eci_hw_ops = eci_ops;
> > + /*
> > + * If platform machine has audio driver providing
> > + * register_hsmic_event_cb, we should give accessory microphone control,
> > + * ie. eci_hsmic_event to it.
> > + * This way audio driver get control to ECI accessory microphone and
> > + * we can save power
> > + */
> > + if (eci_ops->register_hsmic_event_cb) {
> > + hsmic_event.private = eci;
> > + hsmic_event.event = eci_hsmic_event;
> > + eci_ops->register_hsmic_event_cb(&hsmic_event);
> > + }
> > +
> > + mutex_init(&eci->mutex);
> > +
> > + eci_callback.event = eci_accessory_event;
> > + eci_callback.priv = eci;
> > +
> > + /* /sys/bus/i2c/devices/<adapter>-<address> */
> > + ret = sysfs_create_group(&dev->kobj, &eci_attr_group);
> > + if (ret) {
> > + dev_err(eci->dev, "Could not create sysfs entries\n");
> > + goto err_sysfs;
> > + }
> > +
> > + ret = init_accessory_input(eci);
> > + if (ret) {
> > + dev_err(eci->dev, "ERROR initializing accessory input\n");
> > + goto err_input;
> > + }
> > +
> > + init_waitqueue_head(&eci->wait);
> > + INIT_DELAYED_WORK(&eci->eci_ws, eci_work);
> > +
> > + eci->plugged = 0;
> > + eci->mem_ok = false;
> > + /*
> > + * By default ECI driver leaves microphone off, to save power.
> > + * Audio driver can set microphone on by using
> > + * hsmic_event.event
> > + */
> > + eci->mic_state = ECI_MIC_OFF;
> > +
> > + /* Init buttons_data indexes and buffer */
> > + memset(&eci->buttons_data, 0, sizeof(struct eci_buttons_data));
> > + eci->buttons_data.buttons = 0xffffffff;
> > +
> > + return &eci_callback;
> > +
> > +err_input:
> > + sysfs_remove_group(&dev->kobj, &eci_attr_group);
> > +
> > +err_sysfs:
> > + kfree(eci);
> > +
> > + return ERR_PTR(-EBUSY);
> > +}
> > +EXPORT_SYMBOL(eci_register);
> > +
> > +int eci_remove(struct eci_data *eci)
> > +{
> > + eci->eci_hw_ops->register_hsmic_event_cb(NULL);
> > + cancel_delayed_work_sync(&eci->eci_ws);
> > + sysfs_remove_group(&eci->dev->kobj, &eci_attr_group);
> > + input_unregister_device(eci->acc_input);
> > + kfree(eci);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(eci_remove);
> > +
> > +MODULE_ALIAS("i2c:" ECI_DRIVERNAME);
> > +MODULE_AUTHOR("Nokia Corporation");
> > +MODULE_DESCRIPTION("ECI accessory driver");
> > +MODULE_LICENSE("GPL");
> > diff --git a/drivers/input/misc/eci_at20-i2c.c b/drivers/input/misc/eci_at20-i2c.c
> > new file mode 100644
> > index 0000000..9f9bf53
> > --- /dev/null
> > +++ b/drivers/input/misc/eci_at20-i2c.c
> > @@ -0,0 +1,713 @@
> > +/*
> > + * This file is part of ECI (Enhancement Control Interface) controller
> > + * driver
> > + *
> > + * Copyright (C) 2011 Nokia Corporation and/or its subsidiary(-ies).
> > + *
> > + * Contact: Tapio Vihuri <tapio.vihuri@xxxxxxxxx>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License
> > + * version 2 as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful, but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> > + * 02110-1301 USA
> > + *
> > + */
> > +
> > +/*
> > + * ECI stands for (Enhancement Control Interface).
> > + *
> > + * ECI is better known as Multimedia Headset for Nokia phones.
> > + * If headset has many buttons, like play, vol+, vol- etc. then it is propably
> > + * ECI accessory.
> > + *
> > + * ECI controller is kind of bridge between host CPU I2C and ECI accessory
> > + * ECI communication.
> > + */
> > +
> > +#include <linux/init.h>
> > +#include <linux/device.h>
> > +#include <linux/gpio.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/slab.h>
> > +#include <linux/i2c.h>
> > +#include <linux/delay.h>
> > +#include <linux/debugfs.h>
> > +#include <linux/input/eci.h>
> > +
> > +#define DRIVER_NAME "eci_ctrl"
> > +
> > +#define ECICTRL_STATUS_DATA_READY 0x01
> > +#define ECICTRL_STATUS_ACCESSORY_INT 0x02
> > +
> > +#define ECICTRL_WAIT_IRQ 100 /* msec */
> > +#define ECI_RST_MIN 62
> > +#define ECI_RST_WAIT 10 /* msec */
> > +
> > +struct eci_at20_data {
> > + struct device *dev;
> > + struct eci_cb *eci_callback;
> > + int eci_rst_gpio;
> > + int eci_sw_ctrl_gpio;
> > + int eci_int_gpio;
> > + wait_queue_head_t wait;
> > + bool wait_eci_buttons;
> > + bool wait_data;
> > +};
> > +
> > +static struct eci_at20_data *the_ed;
> > +
> > +/* For eci_at20 controller internal registers */
> > +static int eci_at20_read_reg(u8 reg)
> > +{
> > + struct i2c_client *client = to_i2c_client(the_ed->dev);
> > +
> > + if (reg <= ECICMD_RESERVED)
> > + return -EINVAL;
> > +
> > + return i2c_smbus_read_byte_data(client, reg);
> > +}
> > +
> > +static int eci_at20_write_reg(u8 reg, u8 param)
> > +{
> > + struct i2c_client *client = to_i2c_client(the_ed->dev);
> > +
> > + if (reg <= ECICMD_RESERVED)
> > + return -EINVAL;
> > +
> > + return i2c_smbus_write_byte_data(client, reg, param);
> > +}
> > +
> > +/*
> > + * Struct eci_data is ECI input driver (dealing ECI accessories) data.
> > + * Struct eci_at20_data is this driver data, dealing just ECI communication.
> > + * Global eci_register() pairs structs so that we can call ECI input driver
> > + * event function with eci_data
> > + */
> > +static void eci_at20_emit_buttons(struct eci_at20_data *ed, bool force_up)
> > +{
> > + struct eci_data *eci = ed->eci_callback->priv;
> > + struct eci_buttons_data *b = &eci->buttons_data;
> > +
> > + if (force_up)
> > + b->buttons = b->buttons_up_mask;
> > +
> > + ed->eci_callback->event(ECI_EVENT_BUTTON, eci);
> > +}
> > +
> > +/* Trigger ECI accessory register data write (from accessory) */
> > +static int eci_fire_acc_read_reg(u8 reg, int count)
> > +{
> > + struct i2c_client *client = to_i2c_client(the_ed->dev);
> > +
> > + if (!eci_at20_write_reg(ECIREG_READ_COUNT, count))
> > + return i2c_smbus_read_byte_data(client, reg);
> > + else
> > + return -EIO;
> > +}
> > +
> > +/* For ECI accessory internal registers */
> > +static int eci_acc_read_reg(u8 reg, u8 *buf, int count)
> > +{
> > + s32 ret;
> > + int i;
> > +
> > + if (reg > ECICMD_RESERVED)
> > + return -EINVAL;
> > +
> > + the_ed->wait_data = false;
> > + if (eci_fire_acc_read_reg(reg, count))
> > + return -EIO;
> > +
> > + if (!wait_event_timeout(the_ed->wait, the_ed->wait_data == true,
> > + msecs_to_jiffies(ECICTRL_WAIT_IRQ)))
> > + return -EIO;
> > +
> > + for (i = 0; i < count; i++) {
> > + ret = eci_at20_read_reg(ECIREG_READ_DIRECT + i);
> > + if (ret < 0)
> > + return ret;
> > +
> > + buf[i] = ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +/* ECI accessory register write */
> > +static int eci_acc_write_reg(u8 reg, u8 param)
> > +{
> > + struct i2c_client *client = to_i2c_client(the_ed->dev);
> > +
> > + if (reg > ECICMD_RESERVED)
> > + return -EINVAL;
> > +
> > + return i2c_smbus_write_byte_data(client, reg, param);
> > +}
>
> How much of this code is actually generic vs. being controller specific?

I'm planning adding the second controller, and it's very different.

Basically only interface is the same, internal logic is completely
something else.
>
> > +
> > +/*
> > + * debugfs entries:
> > + * reset RW ECI controller reset line
> > + * button W fake button data
> > + * debug RW ECI controller test register
> > + * mic RW accessory's microphone auto/on/off
> > + */
> > +#ifdef CONFIG_DEBUG_FS
> > +static struct dentry *eci_at20_debugfs_dir;
> > +
> > +static ssize_t reset_read(struct file *file, char __user *user_buf,
> > + size_t count, loff_t *ppos)
> > +{
> > + struct eci_at20_data *ed = file->private_data;
> > + char buf[80];
> > + int len = 0;
> > + int ret;
> > +
> > + if (*ppos == 0) {
> > + ret = !!gpio_get_value(ed->eci_rst_gpio);
> > + len = snprintf(buf, sizeof(buf), "%d\n", ret);
> > + }
> > + ret = simple_read_from_buffer(user_buf, count, ppos, buf, len);
> > +
> > + return ret;
> > +}
> > +
> > +static ssize_t reset_write(struct file *file, const char __user *user_buf,
> > + size_t count, loff_t *ppos)
> > +{
> > + /* Assosiated in default_open() */
> > + struct eci_at20_data *ed = file->private_data;
> > + char buf[32];
> > + int buf_size;
> > +
> > + buf_size = min(count, (sizeof(buf)-1));
> > + if (copy_from_user(buf, user_buf, buf_size))
> > + return -EFAULT;
> > +
> > + if (!memcmp(buf, "0", 1))
> > + gpio_set_value(ed->eci_rst_gpio, 0);
> > + else if (!memcmp(buf, "1", 1))
> > + gpio_set_value(ed->eci_rst_gpio, 1);
> > +
> > + return count;
> > +}
> > +
> > +static ssize_t button_write(struct file *file, const char __user *user_buf,
> > + size_t count, loff_t *ppos)
> > +{
> > + /* Assosiated in default_open() */
> > + struct eci_at20_data *ed = file->private_data;
> > + struct eci_data *eci = ed->eci_callback->priv;
> > + struct eci_buttons_data *b = &eci->buttons_data;
> > + char buf[32];
> > + int buf_size, ret;
> > + unsigned long val;
> > +
> > + buf_size = min(count, (sizeof(buf)-1));
> > + if (copy_from_user(buf, user_buf, buf_size))
> > + return -EFAULT;
> > +
> > + ret = strict_strtoul(buf, 0, &val);
> > + if (ret)
> > + return ret;
> > +
> > + b->buttons = val;
> > + eci_at20_emit_buttons(ed, ECI_REAL_BUTTONS);
> > +
> > + return count;
> > +}
> > +
> > +static ssize_t debug_read(struct file *file, char __user *user_buf,
> > + size_t count, loff_t *ppos)
> > +{
> > + char buf[80];
> > + int len = 0;
> > + int ret;
> > +
> > + if (*ppos == 0) {
> > + ret = eci_at20_read_reg(ECIREG_TEST_IN);
> > + if (ret < 0)
> > + return ret;
> > + len += snprintf(buf + len, sizeof(buf) - len, "%02x\n", ret);
> > + }
> > +
> > + ret = simple_read_from_buffer(user_buf, count, ppos, buf, len);
> > +
> > + return ret;
> > +}
> > +
> > +static ssize_t debug_write(struct file *file, const char __user *user_buf,
> > + size_t count, loff_t *ppos)
> > +{
> > + char buf[80];
> > + int buf_size;
> > + unsigned long val;
> > +
> > + buf_size = min(count, (sizeof(buf)-1));
> > + if (copy_from_user(buf, user_buf, buf_size))
> > + return -EFAULT;
> > +
> > + buf[buf_size] = '\0';
> > + if (!strict_strtoul(buf, 0, &val))
> > + eci_at20_write_reg(ECIREG_TEST_OUT, val);
> > + else
> > + return -EINVAL;
> > +
> > + return count;
> > +}
> > +
> > +static ssize_t mic_read(struct file *file, char __user *user_buf,
> > + size_t count, loff_t *ppos)
> > +{
> > + /* Assosiated in default_open() */
> > + struct eci_at20_data *ed = file->private_data;
> > + struct eci_data *eci = ed->eci_callback->priv;
> > + char buf[80], *state;
> > + int len = 0;
> > + int ret;
> > +
> > + if (!eci->mem_ok)
> > + return -ENODEV;
> > +
> > + /* Do not run twice */
> > + if (*ppos == 0) {
> > + ret = eci_acc_read_reg(ECICMD_MIC_CTRL, buf, 1);
> > + if (ret)
> > + return ret;
> > +
> > + eci->mic_state = buf[0];
> > + switch (eci->mic_state) {
> > + case ECI_MIC_AUTO:
> > + state = "auto";
> > + break;
> > + case ECI_MIC_OFF:
> > + state = "off";
> > + break;
> > + case ECI_MIC_ON:
> > + state = "on";
> > + break;
> > + default:
> > + state = "unknown";
> > + break;
> > + }
> > +
> > + len = snprintf(buf, sizeof(buf), "microphone %s\n", state);
> > + }
> > +
> > + ret = simple_read_from_buffer(user_buf, count, ppos, buf, len);
> > +
> > + return ret;
> > +}
>
> Is this really controller specific? I'd expect you could move most
> (all?) debugfs into core.
>
> This should simplify the dirver as I expect it will allow you to get rid
> of most of your "callbacks".
>

I check out proper location for each debugfs-entries.
> > +
> > +static ssize_t mic_write(struct file *file, const char __user *user_buf,
> > + size_t count, loff_t *ppos)
> > +{
> > + /* Assosiated in default_open() */
> > + struct eci_at20_data *ed = file->private_data;
> > + struct eci_data *eci = ed->eci_callback->priv;
> > + char buf[80];
> > + int buf_size;
> > +
> > + buf_size = min(count, (sizeof(buf) - 1));
> > + if (copy_from_user(buf, user_buf, buf_size))
> > + return -EFAULT;
> > +
> > + if (!memcmp(buf, "auto", 4))
> > + eci->mic_state = ECI_MIC_AUTO;
> > + else if (!memcmp(buf, "off", 3))
> > + eci->mic_state = ECI_MIC_OFF;
> > + else if (!memcmp(buf, "on", 2))
> > + eci->mic_state = ECI_MIC_ON;
> > +
> > + if (eci_acc_write_reg(ECICMD_MIC_CTRL, eci->mic_state))
> > + dev_err(eci->dev, "Unable to control headset microphone\n");
> > +
> > + return count;
> > +}
> > +
> > +static int default_open(struct inode *inode, struct file *file)
> > +{
> > + /* Assosiated in debugfs_create_file() */
> > + if (inode->i_private)
> > + file->private_data = inode->i_private;
> > +
> > + return 0;
> > +}
> > +
> > +static const struct file_operations reset_fops = {
> > + .open = default_open,
> > + .read = reset_read,
> > + .write = reset_write,
> > +};
> > +
> > +static const struct file_operations button_fops = {
> > + .open = default_open,
> > + .write = button_write,
> > +};
> > +
> > +static const struct file_operations debug_fops = {
> > + .open = default_open,
> > + .read = debug_read,
> > + .write = debug_write,
> > +};
> > +
> > +static const struct file_operations mic_fops = {
> > + .open = default_open,
> > + .read = mic_read,
> > + .write = mic_write,
> > +};
> > +
> > +static void eci_at20_uninitialize_debugfs(void)
> > +{
> > + if (eci_at20_debugfs_dir)
> > + debugfs_remove_recursive(eci_at20_debugfs_dir);
> > +}
> > +
> > +static long eci_at20_initialize_debugfs(struct eci_at20_data *ed)
> > +{
> > + void *ok;
> > +
> > + /* /sys/kernel/debug/eci_ctrl */
> > + eci_at20_debugfs_dir = debugfs_create_dir(ed->dev->driver->name, NULL);
> > + if (!eci_at20_debugfs_dir)
> > + return -ENOENT;
> > +
> > + /* Struct ed assosiated to inode->i_private */
> > + ok = debugfs_create_file("reset", S_IRUGO | S_IWUSR,
> > + eci_at20_debugfs_dir, ed, &reset_fops);
> > + if (!ok)
> > + goto fail;
> > +
> > + ok = debugfs_create_file("button", S_IWUSR,
> > + eci_at20_debugfs_dir, ed, &button_fops);
> > + if (!ok)
> > + goto fail;
> > +
> > + ok = debugfs_create_file("debug", S_IRUGO,
> > + eci_at20_debugfs_dir, ed, &debug_fops);
> > + if (!ok)
> > + goto fail;
> > +
> > + ok = debugfs_create_file("mic", S_IRUGO | S_IWUSR,
> > + eci_at20_debugfs_dir, ed, &mic_fops);
> > + if (!ok)
> > + goto fail;
> > +
> > + return 0;
> > +fail:
> > + eci_at20_uninitialize_debugfs();
> > + return -ENOENT;
> > +}
> > +#else
> > +#define eci_at20_initialize_debugfs(ed) 1
> > +#define eci_at20_uninitialize_debugfs()
> > +#endif
> > +
> > +/* Reset and learn ECI accessory, ie. get speed */
> > +static int eci_acc_reset(void)
> > +{
> > + s32 ret;
> > +
> > + eci_at20_write_reg(ECIREG_RST_LEARN, 0);
> > +
> > + msleep(ECI_RST_WAIT);
> > +
> > + ret = eci_at20_read_reg(ECIREG_RST_LEARN);
> > + if (ret < ECI_RST_MIN)
> > + return -EIO;
> > +
> > + return 0;
> > +}
> > +
> > +/* Read always four bytes, as stated in ECI specification */
> > +static int eci_acc_read_direct(u8 addr, char *buf)
> > +{
> > + s32 ret;
> > + int i;
> > +
> > + /* Initiate ECI accessory memory read */
> > + the_ed->wait_data = false;
> > + if (!eci_at20_write_reg(ECIREG_READ_COUNT, 4))
> > + if (eci_at20_write_reg(ECIREG_READ_DIRECT, addr))
> > + return -EIO;
> > +
> > + if (!wait_event_timeout(the_ed->wait, the_ed->wait_data == true,
> > + msecs_to_jiffies(ECICTRL_WAIT_IRQ)))
> > + return -EIO;
> > +
> > + for (i = 0; i < 4; i++) {
> > + ret = eci_at20_read_reg(ECIREG_READ_DIRECT + i);
> > + if (ret < 0)
> > + return ret;
> > + buf[i] = ret;
> > + usleep_range(2000, 10000);
> > + }
> > + return 0;
> > +}
> > +
> > +static void eci_at20_get_buttons(u8 *buf, u8 count)
> > +{
> > + int i, ret;
> > +
> > + if (count > 4) {
> > + dev_err(the_ed->dev, "Maximum four bytes allowed\n");
> > + return;
> > + }
> > +
> > + for (i = 0; i <= count; i++) {
> > + ret = eci_at20_read_reg(ECIREG_READ_DIRECT + i);
> > + buf[i] = ret;
> > + }
> > +}
> > +
> > +static irqreturn_t eci_at20_irq_handler(int irq, void *_ed)
> > +{
> > + struct eci_at20_data *ed = _ed;
> > + struct eci_data *eci = ed->eci_callback->priv;
> > + struct eci_buttons_data *b = &eci->buttons_data;
> > + int status;
> > + char buf[4];
> > +
> > + /* Clears eci_at20 DATA interrupt */
> > + status = eci_at20_read_reg(ECIREG_STATUS);
> > +
> > + if (status & ECICTRL_STATUS_DATA_READY) {
> > + /*
> > + * Buttons are special case as we want be fast with them
> > + * and this way we cope with nested button and data interrupts
> > + * The number of bytes needed to read is parsed in ECI
> > + * input driver, based on data in ECI accessory.
> > + * Maximum four bytes.
> > + */
> > + if (ed->wait_eci_buttons) {
> > + eci_at20_get_buttons(buf, eci->port_reg_count);
> > + b->buttons = cpu_to_le32(*(u32 *)buf);
> > + eci_at20_emit_buttons(ed, ECI_REAL_BUTTONS);
> > + ed->wait_eci_buttons = false;
> > + }
> > + /* Complete ECI data reading */
> > + ed->wait_data = true;
> > + wake_up(&ed->wait);
> > + }
> > +
> > + /* Accessory interrupt, ie. button pressed */
> > + if (status & ECICTRL_STATUS_ACCESSORY_INT) {
> > + if (eci->mem_ok) {
> > + eci_fire_acc_read_reg(ECICMD_PORT_DATA_0, 2);
> > + ed->wait_eci_buttons = true;
> > + }
> > + }
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static struct eci_at20_data *eci_at20_initialize(struct device *dev)
> > +{
> > + struct eci_at20_data *ed;
> > + struct eci_platform_data *pd = dev->platform_data;
> > + int ret;
> > +
> > + if (!pd) {
> > + dev_err(dev, "platform_data not available\n");
> > + return ERR_PTR(-EINVAL);
> > + }
> > +
> > + ed = kzalloc(sizeof(*ed), GFP_KERNEL);
> > + if (!ed)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + ed->dev = dev;
> > + ed->eci_rst_gpio = pd->eci_rst_gpio;
> > + ed->eci_sw_ctrl_gpio = pd->eci_sw_ctrl_gpio;
> > + ed->eci_int_gpio = pd->eci_int_gpio;
> > +
> > + if (ed->eci_rst_gpio == 0 || ed->eci_sw_ctrl_gpio == 0 ||
> > + ed->eci_int_gpio == 0) {
> > + ret = -ENXIO;
> > + goto gpio_err;
> > + }
> > +
> > + the_ed = ed;
> > +
> > + ret = eci_at20_initialize_debugfs(ed);
> > + if (ret)
> > + dev_err(dev, "could not create debugfs entries\n");
> > +
> > + ret = gpio_request(ed->eci_rst_gpio, "ECI_RSTn");
> > + if (ret) {
> > + dev_err(dev, "could not request ECI_RSTn gpio %d\n",
> > + ed->eci_rst_gpio);
> > + goto rst_gpio_err;
> > + }
> > +
> > + gpio_direction_output(ed->eci_rst_gpio, 0);
> > + gpio_set_value(ed->eci_rst_gpio, 1);
> > +
> > + ret = gpio_request(ed->eci_sw_ctrl_gpio, "ECI_SW_CTRL");
> > + if (ret) {
> > + dev_err(dev, "could not request ECI_SW_CTRL gpio %d\n",
> > + ed->eci_sw_ctrl_gpio);
> > + goto sw_ctrl_gpio_err;
> > + }
> > +
> > + gpio_direction_input(ed->eci_sw_ctrl_gpio);
> > +
> > + ret = gpio_request(ed->eci_int_gpio, "ECI_INT");
> > + if (ret) {
> > + dev_err(dev, "could not request ECI_INT gpio %d\n",
> > + ed->eci_int_gpio);
> > + goto int_gpio_err;
> > + }
> > +
> > + gpio_direction_input(ed->eci_int_gpio);
> > +
> > + ret = request_threaded_irq(gpio_to_irq(ed->eci_int_gpio), NULL,
> > + eci_at20_irq_handler, IRQF_TRIGGER_RISING,
> > + "ECI_INT", ed);
> > + if (ret) {
> > + dev_err(dev, "could not request irq %d\n",
> > + gpio_to_irq(ed->eci_int_gpio));
> > + goto int_irq_err;
> > + }
> > +
> > + init_waitqueue_head(&ed->wait);
> > +
> > + return ed;
> > +
> > +int_irq_err:
> > + gpio_free(ed->eci_int_gpio);
> > +int_gpio_err:
> > + gpio_free(ed->eci_sw_ctrl_gpio);
> > +sw_ctrl_gpio_err:
> > + gpio_set_value(ed->eci_rst_gpio, 0);
> > + gpio_free(ed->eci_rst_gpio);
> > +rst_gpio_err:
> > + eci_at20_uninitialize_debugfs();
> > +gpio_err:
> > + kfree(ed);
> > +
> > + return ERR_PTR(ret);
> > +}
> > +
> > +static struct eci_hw_ops eci_at20_hw_ops = {
> > + .acc_reset = eci_acc_reset,
> > + .acc_read_direct = eci_acc_read_direct,
> > + .acc_read_reg = eci_acc_read_reg,
> > + .acc_write_reg = eci_acc_write_reg,
> > +};
> > +
> > +static int __devinit eci_at20_i2c_probe(struct i2c_client *client,
> > + const struct i2c_device_id *id)
> > +{
> > + struct eci_at20_data *ed;
> > + struct eci_platform_data *pd = client->dev.platform_data;
> > +
> > + if (!i2c_check_functionality(client->adapter,
> > + I2C_FUNC_SMBUS_BYTE_DATA)) {
> > + dev_err(&client->dev, "SMBUS byte data not supported\n");
> > + return -EIO;
> > + }
> > +
> > + ed = eci_at20_initialize(&client->dev);
> > +
> > + if (IS_ERR(ed))
> > + return PTR_ERR(ed);
> > +
> > + eci_at20_hw_ops.register_hsmic_event_cb = pd->register_hsmic_event_cb;
> > + /* Register itself to the ECI core */
> > + ed->eci_callback = eci_register(&client->dev, &eci_at20_hw_ops);
> > +
> > + i2c_set_clientdata(client, ed);
> > +
> > + return 0;
> > +}
> > +
> > +static int __devexit eci_at20_remove(struct i2c_client *client)
> > +{
> > + struct eci_at20_data *ed = i2c_get_clientdata(client);
> > + struct eci_data *eci = ed->eci_callback->priv;
> > +
> > + eci_remove(eci);
> > +
> > + gpio_set_value(ed->eci_rst_gpio, 0);
> > + gpio_free(ed->eci_rst_gpio);
> > +
> > + gpio_free(ed->eci_sw_ctrl_gpio);
> > +
> > + free_irq(gpio_to_irq(ed->eci_int_gpio), ed);
> > + gpio_free(ed->eci_int_gpio);
> > +
> > + eci_at20_uninitialize_debugfs();
> > +
> > + kfree(ed);
> > +
> > + return 0;
> > +}
> > +
> > +#ifdef CONFIG_PM
> > +static int eci_at20_suspend(struct i2c_client *client, pm_message_t message)
> > +{
> > + struct eci_at20_data *ed = i2c_get_clientdata(client);
> > + struct eci_data *eci = ed->eci_callback->priv;
> > +
> > + eci_suspend(eci);
> > +
> > + return 0;
> > +}
> > +
> > +static int eci_at20_resume(struct i2c_client *client)
> > +{
> > + struct eci_at20_data *ed = i2c_get_clientdata(client);
> > + struct eci_data *eci = ed->eci_callback->priv;
> > +
> > + eci_resume(eci);
> > +
> > + return 0;
> > +}
> > +#else
> > +#define eci_at20_suspend NULL
> > +#define eci_at20_resume NULL
> > +#endif
> > +
> > +static const struct i2c_device_id eci_id[] = {
> > + { DRIVER_NAME, 0 },
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(i2c, eci_id);
> > +
> > +static struct i2c_driver eci_i2c_driver = {
> > + .driver = {
> > + .name = DRIVER_NAME,
> > + .owner = THIS_MODULE,
> > + },
> > + .probe = eci_at20_i2c_probe,
> > + .remove = __devexit_p(eci_at20_remove),
> > + .suspend = eci_at20_suspend,
> > + .resume = eci_at20_resume,
> > + .id_table = eci_id,
> > +};
> > +
> > +static int __init eci_at20_init(void)
> > +{
> > + return i2c_add_driver(&eci_i2c_driver);
> > +}
> > +module_init(eci_at20_init);
> > +
> > +static void __devexit eci_at20_exit(void)
> > +{
> > + i2c_del_driver(&eci_i2c_driver);
> > +}
> > +module_exit(eci_at20_exit);
> > +
> > +MODULE_DESCRIPTION("ECI accessory controller driver");
> > +MODULE_AUTHOR("Nokia Corporation");
> > +MODULE_ALIAS("i2c:eci_ctrl");
> > +MODULE_LICENSE("GPL");
> > diff --git a/include/linux/input/eci.h b/include/linux/input/eci.h
> > new file mode 100644
> > index 0000000..60b820e
> > --- /dev/null
> > +++ b/include/linux/input/eci.h
> > @@ -0,0 +1,189 @@
> > +/*
> > + * This file is part of ECI (Enhancement Control Interface) driver
> > + *
> > + * Copyright (C) 2011 Nokia Corporation and/or its subsidiary(-ies).
> > + *
> > + * Contact: Tapio Vihuri <tapio.vihuri@xxxxxxxxx>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License
> > + * version 2 as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful, but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> > + * 02110-1301 USA
> > + *
> > + */
> > +#ifndef __ECI_H__
> > +#define __ECI_H__
> > +
> > +#define ECI_MAX_MEM_SIZE 0x7c
> > +#define ECI_BUTTON_BUF_SIZE 32
> > +#define ECI_MAX_FEATURE_COUNT 31
> > +
> > +#define ACI_COMMERR 0x010
> > +#define ACI_FRAERR 0x020
> > +#define ACI_RESERR 0x040
> > +#define ACI_COLL 0x080
> > +
> > +#define ECI_REAL_BUTTONS 0
> > +#define ECI_FORCE_BUTTONS_UP 1
> > +
> > +/* ECI accessory register's bits */
> > +#define ECI_MIC_AUTO 0x00
> > +#define ECI_MIC_OFF 0x5a
> > +#define ECI_MIC_ON 0xff
> > +
> > +/*
> > + * VPROG2CNT - VPROG2 Control Register
> > + * 2.5V | normal | normal
> > + * 10 | 111 | 111
> > + * 2.5V | off | off
> > + * 10 | 100 | 100
> > + */
> > +#define AvP_MSIC_VPROG2 0xd7
> > +#define AvP_MSIC_VPROG2_2V5_ON 0xbf
> > +#define AvP_MSIC_VPROG2_2V5_OFF 0xa4
> > +
> > +#define GPIO_ECI_RSTn 126 /* GP_CORE_030 + 96 */
> > +#define GPIO_ECI_SW_CTRL 178 /* GP_CORE_082 + 96 */
> > +#define GPIO_ECI_INT 16 /* GP_AON_016 */
> > +
> > +/* fixed in ECI HW, do not change */
> > +enum {
> > + ECICMD_HWID,
> > + ECICMD_SWID,
> > + ECICMD_ECI_BUS_SPEED,
> > + ECICMD_MIC_CTRL,
> > + ECICMD_MASTER_INT_REG,
> > + ECICMD_HW_CONF_MEM_ACCESS,
> > + ECICMD_EXTENDED_MEM_ACCESS,
> > + ECICMD_INDIRECT_MEM_ACCESS,
> > + ECICMD_PORT_DATA_0,
> > + ECICMD_PORT_DATA_1,
> > + ECICMD_PORT_DATA_2,
> > + ECICMD_PORT_DATA_3,
> > + ECICMD_LATCHED_PORT_DATA_0,
> > + ECICMD_LATCHED_PORT_DATA_1,
> > + ECICMD_LATCHED_PORT_DATA_2,
> > + ECICMD_LATCHED_PORT_DATA_3,
> > + ECICMD_DATA_DIR_0,
> > + ECICMD_DATA_DIR_1,
> > + ECICMD_DATA_DIR_2,
> > + ECICMD_DATA_DIR_3,
> > + ECICMD_INT_CONFIG_0_LOW,
> > + ECICMD_INT_CONFIG_0_HIGH,
> > + ECICMD_INT_CONFIG_1_LOW,
> > + ECICMD_INT_CONFIG_1_HIGH,
> > + ECICMD_INT_CONFIG_2_LOW,
> > + ECICMD_INT_CONFIG_2_HIGH,
> > + ECICMD_INT_CONFIG_3_LOW,
> > + ECICMD_INT_CONFIG_3_HIGH,
> > + /*
> > + * 0x1c - 0x2f reserved for future
> > + * 0x30 - 0x3d reserved
> > + */
> > + ECICMD_EEPROM_LOCK = 0x3e,
> > + ECICMD_RESERVED, /* 0x3f */
> > + ECIREG_STATUS, /* 0x40 */
> > + ECIREG_READ_COUNT, /* 0x41 */
> > + ECIREG_BUF_COUNT, /* 0x42 */
> > + ECIREG_RST_LEARN, /* 0x43 */
> > + /* 0x44 - 0xdf as data buffer */
> > + ECIREG_READ_DIRECT, /* 0x44 */
> > + ECIREG_HW_ID = 0xe0, /* 0xe0 */
> > + ECIREG_FW_ID, /* 0xe1 */
> > + ECIREG_TEST_IN, /* 0xe2 */
> > + ECIREG_TEST_OUT, /* 0xe3 */
> > +};
> > +
> > +enum {
> > + ECI_EVENT_IS_ECI,
> > + ECI_EVENT_PLUG_IN,
> > + ECI_EVENT_PLUG_OUT,
> > + ECI_EVENT_BUTTON,
> > + ECI_EVENT_NO,
> > +};
> > +
> > +struct audio_hsmic_event {
> > + void *private;
> > + void (*event)(void *priv, bool on);
> > +};
> > +
> > +struct eci_hw_ops {
> > + int (*acc_reset)(void);
> > + int (*acc_read_direct)(u8 addr, char *buf);
> > + int (*acc_read_reg)(u8 reg, u8 *buf, int count);
> > + int (*acc_write_reg)(u8 reg, u8 param);
> > + void (*register_hsmic_event_cb)(struct audio_hsmic_event *);
> > +};
> > +
> > +struct eci_cb {
> > + void *priv;
> > + void (*event)(int event, void *priv);
> > +};
> > +
> > +struct enchancement_features_fixed {
> > + u8 block_id;
> > + u8 length;
> > + u8 connector_conf;
> > + u8 number_of_features;
> > +};
> > +
> > +struct enchancement_features_variable {
> > + u8 *io_support;
> > + u8 *io_functionality;
> > + u8 *active_state;
> > +};
> > +
> > +struct eci_buttons_data {
> > + u32 buttons;
> > + int windex;
> > + int rindex;
> > + u32 buttons_up_mask;
> > + u32 buttons_buf[ECI_BUTTON_BUF_SIZE];
> > +};
> > +
> > +struct eci_data {
> > + struct device *dev;
> > + struct delayed_work eci_ws;
> > + wait_queue_head_t wait;
> > + struct input_dev *acc_input;
> > + int event;
> > + bool first_event;
> > + bool mem_ok;
> > + u16 mem_size;
> > + u8 memory[ECI_MAX_MEM_SIZE];
> > + struct enchancement_features_fixed *e_features_fix;
> > + struct enchancement_features_variable e_features_var;
> > + u8 port_reg_count;
> > + struct eci_buttons_data buttons_data;
> > + struct eci_hw_ops *eci_hw_ops;
> > + u8 mic_state;
> > + u8 plugged;
> > + bool is_eci;
> > + struct mutex mutex;
> > + bool opened;
> > + bool suspended;
> > +};
> > +
> > +/* Exported functions */
> > +void eci_suspend(struct eci_data *eci);
> > +void eci_resume(struct eci_data *eci);
> > +struct eci_cb *eci_register(struct device *dev, struct eci_hw_ops *eci_ops);
> > +int eci_remove(struct eci_data *eci);
> > +
> > +/* eci paltform related data */
> > +struct eci_platform_data {
> > + int eci_rst_gpio;
> > + int eci_sw_ctrl_gpio;
> > + int eci_int_gpio;
> > + void (*register_hsmic_event_cb)(struct audio_hsmic_event *);
> > +};
> > +#endif
> > --
> > 1.6.5
> >
>
> Thanks.
>

Thank you, there was good points.

Tapio

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Next message: Herbert Xu: "Re: [PATCH] random: update interface comments to reflect reality"
Previous message: David Cohen: "[PATCH 0/1] Fix linux/wait.h header file"
In reply to: Dmitry Torokhov: "Re: [PATCH v7 1/2] ECI: input: introduce ECI accessory input driver"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]