Re: [PATCH v2 1/3] ECI: input: introduce ECI accessory input driver

Previous thread: [patch] xenfb: fix xenfb suspend/resume race by Joe Jin on Thursday, December 30, 2010 - 5:56 am. (6 messages)

Next thread: [PATCH v2 0/3] input: Add support for ECI (multimedia) accessories by tapio.vihuri on Thursday, December 30, 2010 - 6:36 am. (1 message)
From: tapio.vihuri
Date: Thursday, December 30, 2010 - 6:36 am

From: Tapio Vihuri <tapio.vihuri@nokia.com>

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

Signed-off-by: Tapio Vihuri <tapio.vihuri@nokia.com>
---
 drivers/input/misc/Kconfig  |   18 +
 drivers/input/misc/Makefile |    2 +-
 drivers/input/misc/eci.c    | 1002 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/input/eci.h   |  157 +++++++
 4 files changed, 1178 insertions(+), 1 deletions(-)
 create mode 100644 drivers/input/misc/eci.c
 create mode 100644 include/linux/input/eci.h

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index b99b8cb..7a15bc6 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -448,4 +448,22 @@ config INPUT_ADXL34X_SPI
 	  To compile this driver as a module, choose M here: the
 	  module will be called adxl34x-spi.
 
+config INPUT_ECI
+	tristate "AV ECI (Enhancement Control Interface) input driver"
+	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 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
+
+	  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
+
 endif
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 1fe1f6c..99d2289 100644
--- ...
From: tapio.vihuri
Date: Thursday, December 30, 2010 - 6:36 am

From: Tapio Vihuri <tapio.vihuri@nokia.com>

Gives platform data for ECI accessory input driver and
ECI bus controller driver.

Signed-off-by: Tapio Vihuri <tapio.vihuri@nokia.com>
---
 arch/x86/platform/mrst/mrst.c |   59 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 59 insertions(+), 0 deletions(-)

diff --git a/arch/x86/platform/mrst/mrst.c b/arch/x86/platform/mrst/mrst.c
index 79ae681..60dca78 100644
--- a/arch/x86/platform/mrst/mrst.c
+++ b/arch/x86/platform/mrst/mrst.c
@@ -14,6 +14,7 @@
 #include <linux/sfi.h>
 #include <linux/irq.h>
 #include <linux/module.h>
+#include <linux/platform_device.h>
 
 #include <asm/setup.h>
 #include <asm/mpspec_def.h>
@@ -309,3 +310,61 @@ static inline int __init setup_x86_mrst_timer(char *arg)
 	return 0;
 }
 __setup("x86_mrst_timer=", setup_x86_mrst_timer);
+
+#if defined(CONFIG_ECI) || defined(CONFIG_ECI_MODULE)
+#include <linux/input/eci.h>
+
+#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 */
+
+static struct ecibus_platform_data medfield_ecibus_control = {
+	.ecibus_rst_gpio = GPIO_ECI_RSTn,
+	.ecibus_sw_ctrl_gpio = GPIO_ECI_SW_CTRL,
+	.ecibus_int_gpio = GPIO_ECI_INT,
+};
+
+/*
+ * This is just example, should be used in platform audio driver
+ * hsmic_event->event(hsmic_event->private, true)
+ */
+static void medfield_register_hsmic_event_cb(struct audio_hsmic_event *event)
+{
+	struct audio_hsmic_event *hsmic_event;
+
+	hsmic_event = event;
+}
+
+static struct eci_platform_data medfield_eci_platform_data = {
+	.register_hsmic_event_cb	= medfield_register_hsmic_event_cb,
+};
+
+static struct platform_device medfield_ecibus_device = {
+	.name	= "ecibus",
+	.id	= 1,
+	.dev	= {
+		.platform_data = &medfield_ecibus_control,
+	},
+};
+
+static struct platform_device medfield_eci_device = {
+	.name	= "ECI_accessory",
+	.dev	= {
+		.platform_data = &medfield_eci_platform_data,
+	},
+};
+
+static ...
From: tapio.vihuri
Date: Thursday, December 30, 2010 - 6:36 am

From: Tapio Vihuri <tapio.vihuri@nokia.com>

ECI bus controller is kind of bridge between host CPU I2C and ECI accessory
ECI communication.

Signed-off-by: Tapio Vihuri <tapio.vihuri@nokia.com>
---
 drivers/Kconfig           |    2 +
 drivers/Makefile          |    1 +
 drivers/ecibus/Kconfig    |   35 +++
 drivers/ecibus/Makefile   |   10 +
 drivers/ecibus/ecibus.c   |  583 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/input/eci.h |    8 +
 6 files changed, 639 insertions(+), 0 deletions(-)
 create mode 100644 drivers/ecibus/Kconfig
 create mode 100644 drivers/ecibus/Makefile
 create mode 100644 drivers/ecibus/ecibus.c

diff --git a/drivers/Kconfig b/drivers/Kconfig
index a2b902f..f450f98 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -111,4 +111,6 @@ source "drivers/xen/Kconfig"
 source "drivers/staging/Kconfig"
 
 source "drivers/platform/Kconfig"
+
+source "drivers/ecibus/Kconfig"
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index f3ebb30..11f5d57 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -113,5 +113,6 @@ obj-$(CONFIG_SSB)		+= ssb/
 obj-$(CONFIG_VHOST_NET)		+= vhost/
 obj-$(CONFIG_VLYNQ)		+= vlynq/
 obj-$(CONFIG_STAGING)		+= staging/
+obj-$(CONFIG_ECI)		+= ecibus/
 obj-y				+= platform/
 obj-y				+= ieee802154/
diff --git a/drivers/ecibus/Kconfig b/drivers/ecibus/Kconfig
new file mode 100644
index 0000000..f2fc8a4
--- /dev/null
+++ b/drivers/ecibus/Kconfig
@@ -0,0 +1,35 @@
+#
+# ECI driver configuration
+#
+menuconfig ECI
+	bool "ECI support"
+	help
+	  ECI (Enhancement Control Interface) accessory support
+
+	  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 several buttons ECI accessory contains memory for storing
+	  several parameters.
+
+	  Enable ECI support in terminal so that ECI input driver is able
+	  to communicate ...
From: Peter Ujfalusi
Date: Monday, January 3, 2011 - 1:42 am

Hi,


It is kind of misleading to call this driver as a bus driver IMHO.
For what I gathered, this is a driver for a micro-controller, which
implements the ECI communication.
This could be a generic driver for the micro-controller, but two lines
makes it x86 specific...



What happens if the mc is on another bus?
Shall you have platform data for this?

Is this some sort of power enable for the controller itself?
If it is, why not use a callback from the platform to enable/disable the
power, so the driver will be platform independent, and you do this intel
specific thing in arch/ platform code.
If this is not related to the mc itself, then this shall not be part of
this driver.


Same applies here for the intel dependency.

-- 
Péter
--

From: Dmitry Torokhov
Date: Sunday, January 2, 2011 - 1:19 am

Hi Tapio,



Consider switching to sparse keymap library. You won't be needing ugly
KEY_MAX + SW_XXX hacks and it will also support remapping keys from

Do not break strings on 80 column boundaries, wither align the arguments
differently or just go past 80 columns, like this:

			dev_err(eci->dev,

Should be 0 I think, and not have a parameter, preferably

static inline int eci_initialize_debugfs(void)
{
	return 0;







Thank you.

-- 
Dmitry
--

From: Tapio Vihuri
Date: Tuesday, January 4, 2011 - 7:01 am

Hi Dmitry

Thank you for good comments. I sent v3 patch set right after this email.


Corrected in patch set.


Much nicer solution, thank you.

Corrected in patch set.


I take this static inline solution, but I needed the parameter.


The ECI specification says that data in ECI accessory's memory is in big
endian order. This simply get 16-bit size parameter right in any
endianes machine.


I made it now as 0/1. It's in sysfs as this is user space's interface
for ECI acessories inserting.

The other way is via ALSA sound driver providing jack detection, but
it's not there yet.


I have loose some word somewhere... Now it goes:
 	/*
         * Configure ECI buttons now as we know how after
         * enchancement features table has been parsed
         */


Corrected in patch set.


Nothing, and I can't even remember why I have put it there first place.
Removed now.


Thank you, now driver is better.

-- 
Tapsa

--

Previous thread: [patch] xenfb: fix xenfb suspend/resume race by Joe Jin on Thursday, December 30, 2010 - 5:56 am. (6 messages)

Next thread: [PATCH v2 0/3] input: Add support for ECI (multimedia) accessories by tapio.vihuri on Thursday, December 30, 2010 - 6:36 am. (1 message)