2011-03-07 11:09:55

by Mohan Pallaka

[permalink] [raw]
Subject: [PATCH 0/2] Support for isa1200 haptic chip

This patchset includes a __weak attributed pwm apis in pwm.h and
a ff-memless based isa1200 driver.

ISA1200 chip can operate in two modes and if the chip has been
configured for pwm-generation mode, which doesn't use pwm standard
apis then it'd encounter compilation errors if the host doesn't
support PWM. This driver is based on Park's isa1200 driver that uses
haptic class.

Kyungmin Park (1):
input: misc: Add support for isa1200 chip

Mohan Pallaka (1):
pwm: Add __weak attributed functions for pwm operations

drivers/input/misc/Kconfig | 12 ++
drivers/input/misc/Makefile | 1 +
drivers/input/misc/isa1200.c | 440 +++++++++++++++++++++++++++++++++++++++++
include/linux/input/isa1200.h | 45 +++++
include/linux/pwm.h | 21 ++-
5 files changed, 514 insertions(+), 5 deletions(-)
create mode 100644 drivers/input/misc/isa1200.c
create mode 100644 include/linux/input/isa1200.h

--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.


2011-03-07 11:10:00

by Mohan Pallaka

[permalink] [raw]
Subject: [PATCH 1/2] pwm: Add __weak attributed functions for pwm operations

For chip drivers that support both pwm and non-pwm modes
would encounter compilation errors if the platform doesn't
have support for pwm though the chip is programmed to work
in non-pwm mode. Add __weak attributed pwm functions to avoid
compilation issues in these scenarios.

Signed-off-by: Mohan Pallaka <[email protected]>
---
include/linux/pwm.h | 21 ++++++++++++++++-----
1 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index 7c77575..3a8c3df 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -3,29 +3,40 @@

struct pwm_device;

+/* Add __weak functions to support PWM */
+
/*
* pwm_request - request a PWM device
*/
-struct pwm_device *pwm_request(int pwm_id, const char *label);
+struct pwm_device __weak *pwm_request(int pwm_id, const char *label)
+{
+ return NULL;
+}

/*
* pwm_free - free a PWM device
*/
-void pwm_free(struct pwm_device *pwm);
+void __weak pwm_free(struct pwm_device *pwm) { }

/*
* pwm_config - change a PWM device configuration
*/
-int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns);
+int __weak pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
+{
+ return -ENODEV;
+}

/*
* pwm_enable - start a PWM output toggling
*/
-int pwm_enable(struct pwm_device *pwm);
+int __weak pwm_enable(struct pwm_device *pwm)
+{
+ return -EINVAL;
+}

/*
* pwm_disable - stop a PWM output toggling
*/
-void pwm_disable(struct pwm_device *pwm);
+void __weak pwm_disable(struct pwm_device *pwm) { }

#endif /* __LINUX_PWM_H */

--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

2011-03-07 11:10:08

by Mohan Pallaka

[permalink] [raw]
Subject: [PATCH 2/2] input: misc: Add support for isa1200 chip

From: Kyungmin Park <[email protected]>

isa1200 chip can be used to generate vibrations to indicate
silent alerts, providing gaming haptic actions or vibrations
in response to touches. It can operate in two modes, pwm
generation and pwm input mode. Pwm generation mode requires
external clock and pwm input mode needs the pwm signal to be
given as input to the chip.

This patch has been derived based on Kyungmin Park's haptic
framework patches at http://lkml.org/lkml/2009/10/7/50 . Park's
isa1200 driver registers to the haptic class, which is also
introduced as part of the same patch series, and userspace will
operate the vibrator through exported sysfs entries. This version
supports only pwm input mode.

This derived patch converts the driver from haptic framework to
the Linux supported force feeback framework. It also supports the
chip operation in both pwm input and generation modes.

Signed-off-by: Kyungmin Park <[email protected]>
Signed-off-by: Mohan Pallaka <[email protected]>
---
drivers/input/misc/Kconfig | 12 ++
drivers/input/misc/Makefile | 1 +
drivers/input/misc/isa1200.c | 440 +++++++++++++++++++++++++++++++++++++++++
include/linux/input/isa1200.h | 45 +++++
4 files changed, 498 insertions(+), 0 deletions(-)
create mode 100644 drivers/input/misc/isa1200.c
create mode 100644 include/linux/input/isa1200.h

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index b0c6772..6d20b8b 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -302,6 +302,18 @@ config HP_SDC_RTC
Say Y here if you want to support the built-in real time clock
of the HP SDC controller.

+config INPUT_ISA1200
+ tristate "ISA1200 haptic support"
+ depends on I2C
+ select INPUT_FF_MEMLESS
+ help
+ ISA1200 is a high performance enhanced haptic chip.
+ Say Y here if you want to support ISA1200 connected via I2C,
+ and select N if you are unsure.
+
+ To compile this driver as a module, choose M here: the
+ module will be called isa1200.
+
config INPUT_PCF50633_PMU
tristate "PCF50633 PMU events"
depends on MFD_PCF50633
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 9b47971..4570154 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -23,6 +23,7 @@ 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_ISA1200) += isa1200.o
obj-$(CONFIG_INPUT_IXP4XX_BEEPER) += ixp4xx-beeper.o
obj-$(CONFIG_INPUT_KEYSPAN_REMOTE) += keyspan_remote.o
obj-$(CONFIG_INPUT_M68K_BEEP) += m68kspkr.o
diff --git a/drivers/input/misc/isa1200.c b/drivers/input/misc/isa1200.c
new file mode 100644
index 0000000..bce96f8
--- /dev/null
+++ b/drivers/input/misc/isa1200.c
@@ -0,0 +1,440 @@
+/*
+ * Copyright (C) 2009 Samsung Electronics
+ * Kyungmin Park <[email protected]>
+ *
+ * Copyright (c) 2011, Code Aurora Forum. All rights reserved.
+ *
+ * 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.
+ */
+
+#define pr_fmt(fmt) "%s: " fmt, __func__
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/i2c.h>
+#include <linux/gpio.h>
+#include <linux/workqueue.h>
+#include <linux/delay.h>
+#include <linux/pwm.h>
+#include <linux/input.h>
+#include <linux/slab.h>
+#include <linux/pm.h>
+#include <linux/input/isa1200.h>
+
+#define ISA1200_HCTRL0 0x30
+#define HCTRL0_MODE_CTRL_BIT (3)
+#define HCTRL0_OVERDRIVE_HIGH_BIT (5)
+#define HCTRL0_OVERDRIVE_EN_BIT (6)
+#define HCTRL0_HAP_EN (7)
+#define HCTRL0_RESET 0x01
+#define HCTRL1_RESET 0x4B
+
+#define ISA1200_HCTRL1 0x31
+#define HCTRL1_SMART_ENABLE_BIT (3)
+#define HCTRL1_ERM_BIT (5)
+#define HCTRL1_EXT_CLK_ENABLE_BIT (7)
+
+#define ISA1200_HCTRL5 0x35
+#define HCTRL5_VIB_STRT 0xD5
+#define HCTRL5_VIB_STOP 0x6B
+
+#define DIVIDER_128 (128)
+#define DIVIDER_1024 (1024)
+#define DIVIDE_SHIFTER_128 (7)
+
+#define FREQ_22400 (22400)
+#define FREQ_172600 (172600)
+
+#define POR_DELAY_USEC 250
+
+struct isa1200_chip {
+ const struct isa1200_platform_data *pdata;
+ struct i2c_client *client;
+ struct input_dev *input_device;
+ struct pwm_device *pwm;
+ unsigned int period_ns;
+ unsigned int state;
+ struct work_struct work;
+};
+
+static void isa1200_vib_set(struct isa1200_chip *haptic, int enable)
+{
+ int rc;
+
+ if (enable) {
+ if (haptic->pdata->mode_ctrl == PWM_INPUT_MODE) {
+ rc = pwm_config(haptic->pwm,
+ (haptic->period_ns * haptic->pdata->duty) / 100,
+ haptic->period_ns);
+ if (rc < 0)
+ pr_err("pwm_config fail\n");
+ rc = pwm_enable(haptic->pwm);
+ if (rc < 0)
+ pr_err("pwm_enable fail\n");
+ } else if (haptic->pdata->mode_ctrl == PWM_GEN_MODE) {
+ rc = i2c_smbus_write_byte_data(haptic->client,
+ ISA1200_HCTRL5,
+ HCTRL5_VIB_STRT);
+ if (rc < 0)
+ pr_err("start vibration fail\n");
+ }
+ } else {
+ if (haptic->pdata->mode_ctrl == PWM_INPUT_MODE)
+ pwm_disable(haptic->pwm);
+ else if (haptic->pdata->mode_ctrl == PWM_GEN_MODE) {
+ rc = i2c_smbus_write_byte_data(haptic->client,
+ ISA1200_HCTRL5,
+ HCTRL5_VIB_STOP);
+ if (rc < 0)
+ pr_err("stop vibration fail\n");
+ }
+ }
+}
+
+static int isa1200_setup(struct i2c_client *client)
+{
+ struct isa1200_chip *haptic = i2c_get_clientdata(client);
+ int value, temp, rc;
+
+ gpio_set_value_cansleep(haptic->pdata->hap_en_gpio, 0);
+ udelay(POR_DELAY_USEC);
+ gpio_set_value_cansleep(haptic->pdata->hap_en_gpio, 1);
+
+ value = (haptic->pdata->smart_en << HCTRL1_SMART_ENABLE_BIT) |
+ (haptic->pdata->is_erm << HCTRL1_ERM_BIT) |
+ (haptic->pdata->ext_clk_en << HCTRL1_EXT_CLK_ENABLE_BIT);
+
+ rc = i2c_smbus_write_byte_data(client, ISA1200_HCTRL1, value);
+ if (rc < 0) {
+ pr_err("i2c write failure\n");
+ return rc;
+ }
+
+ if (haptic->pdata->mode_ctrl == PWM_GEN_MODE) {
+ temp = haptic->pdata->pwm_fd.pwm_div;
+ if (temp < DIVIDER_128 || temp > DIVIDER_1024 ||
+ temp % DIVIDER_128) {
+ pr_err("Invalid divider\n");
+ rc = -EINVAL;
+ goto reset_hctrl1;
+ }
+ value = ((temp >> DIVIDE_SHIFTER_128) - 1);
+ } else if (haptic->pdata->mode_ctrl == PWM_INPUT_MODE) {
+ temp = haptic->pdata->pwm_fd.pwm_freq;
+ if (temp < FREQ_22400 || temp > FREQ_172600 ||
+ temp % FREQ_22400) {
+ pr_err("Invalid frequency\n");
+ rc = -EINVAL;
+ goto reset_hctrl1;
+ }
+ value = ((temp / FREQ_22400) - 1);
+ haptic->period_ns = NSEC_PER_SEC / temp;
+ }
+ value |= (haptic->pdata->mode_ctrl << HCTRL0_MODE_CTRL_BIT) |
+ (haptic->pdata->overdrive_high << HCTRL0_OVERDRIVE_HIGH_BIT) |
+ (haptic->pdata->overdrive_en << HCTRL0_OVERDRIVE_EN_BIT) |
+ (haptic->pdata->chip_en << HCTRL0_HAP_EN);
+
+ rc = i2c_smbus_write_byte_data(client, ISA1200_HCTRL0, value);
+ if (rc < 0) {
+ pr_err("i2c write failure\n");
+ goto reset_hctrl1;
+ }
+
+ return 0;
+
+reset_hctrl1:
+ i2c_smbus_write_byte_data(client, ISA1200_HCTRL1,
+ HCTRL1_RESET);
+ return rc;
+}
+
+static void isa1200_worker(struct work_struct *work)
+{
+ struct isa1200_chip *haptic;
+
+ haptic = container_of(work, struct isa1200_chip, work);
+ isa1200_vib_set(haptic, !!haptic->state);
+}
+
+static int isa1200_play_effect(struct input_dev *dev, void *data,
+ struct ff_effect *effect)
+{
+ struct isa1200_chip *haptic = input_get_drvdata(dev);
+
+ /* support basic vibration */
+ haptic->state = effect->u.rumble.strong_magnitude >> 8;
+ if (!haptic->state)
+ haptic->state = effect->u.rumble.weak_magnitude >> 9;
+
+ schedule_work(&haptic->work);
+ return 0;
+}
+
+#ifdef CONFIG_PM
+static int isa1200_suspend(struct device *dev)
+{
+ struct isa1200_chip *haptic = dev_get_drvdata(dev);
+ int rc;
+
+ cancel_work_sync(&haptic->work);
+ /* turn-off current vibration */
+ isa1200_vib_set(haptic, 0);
+
+ if (haptic->pdata->power_on) {
+ rc = haptic->pdata->power_on(0);
+ if (rc) {
+ pr_err("power-down failed\n");
+ return rc;
+ }
+ }
+ return 0;
+}
+
+static int isa1200_resume(struct device *dev)
+{
+ struct isa1200_chip *haptic = dev_get_drvdata(dev);
+ int rc;
+
+ if (haptic->pdata->power_on) {
+ rc = haptic->pdata->power_on(1);
+ if (rc) {
+ pr_err("power-up failed\n");
+ return rc;
+ }
+ }
+
+ isa1200_setup(haptic->client);
+ return 0;
+}
+#else
+#define isa1200_suspend NULL
+#define isa1200_resume NULL
+#endif
+
+static int isa1200_open(struct input_dev *dev)
+{
+ struct isa1200_chip *haptic = input_get_drvdata(dev);
+ int rc;
+
+ /* device setup */
+ if (haptic->pdata->dev_setup) {
+ rc = haptic->pdata->dev_setup(true);
+ if (rc < 0) {
+ pr_err("setup failed!\n");
+ return rc;
+ }
+ }
+
+ /* power on */
+ if (haptic->pdata->power_on) {
+ rc = haptic->pdata->power_on(true);
+ if (rc < 0) {
+ pr_err("power failed\n");
+ goto err_setup;
+ }
+ }
+
+ /* request gpio */
+ rc = gpio_is_valid(haptic->pdata->hap_en_gpio);
+ if (rc) {
+ rc = gpio_request(haptic->pdata->hap_en_gpio, "haptic_gpio");
+ if (rc) {
+ pr_err("gpio %d request failed\n",
+ haptic->pdata->hap_en_gpio);
+ goto err_power_on;
+ }
+ } else {
+ pr_err("Invalid gpio %d\n",
+ haptic->pdata->hap_en_gpio);
+ goto err_power_on;
+ }
+
+ rc = gpio_direction_output(haptic->pdata->hap_en_gpio, 0);
+ if (rc) {
+ pr_err("gpio %d set direction failed\n",
+ haptic->pdata->hap_en_gpio);
+ goto err_gpio_free;
+ }
+
+ /* setup registers */
+ rc = isa1200_setup(haptic->client);
+ if (rc < 0) {
+ pr_err("setup fail %d\n", rc);
+ goto err_gpio_free;
+ }
+
+ if (haptic->pdata->mode_ctrl == PWM_INPUT_MODE) {
+ haptic->pwm = pwm_request(haptic->pdata->pwm_ch_id,
+ haptic->client->driver->id_table->name);
+ if (IS_ERR(haptic->pwm)) {
+ pr_err("pwm request failed\n");
+ rc = PTR_ERR(haptic->pwm);
+ goto err_reset_hctrl0;
+ }
+ }
+
+ /* init workqeueue */
+ INIT_WORK(&haptic->work, isa1200_worker);
+ return 0;
+
+err_reset_hctrl0:
+ i2c_smbus_write_byte_data(haptic->client, ISA1200_HCTRL0,
+ HCTRL0_RESET);
+err_gpio_free:
+ gpio_free(haptic->pdata->hap_en_gpio);
+err_power_on:
+ if (haptic->pdata->power_on)
+ haptic->pdata->power_on(0);
+err_setup:
+ if (haptic->pdata->dev_setup)
+ haptic->pdata->dev_setup(false);
+
+ return rc;
+}
+
+static void isa1200_close(struct input_dev *dev)
+{
+ struct isa1200_chip *haptic = input_get_drvdata(dev);
+
+ /* turn-off current vibration */
+ isa1200_vib_set(haptic, 0);
+
+ if (haptic->pdata->mode_ctrl == PWM_INPUT_MODE)
+ pwm_free(haptic->pwm);
+
+ gpio_free(haptic->pdata->hap_en_gpio);
+
+ /* reset hardware registers */
+ i2c_smbus_write_byte_data(haptic->client, ISA1200_HCTRL0,
+ HCTRL0_RESET);
+ i2c_smbus_write_byte_data(haptic->client, ISA1200_HCTRL1,
+ HCTRL1_RESET);
+
+ if (haptic->pdata->dev_setup)
+ haptic->pdata->dev_setup(false);
+
+ /* power-off the chip */
+ if (haptic->pdata->power_on)
+ haptic->pdata->power_on(0);
+}
+
+static int __devinit isa1200_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct isa1200_chip *haptic;
+ int rc;
+
+ if (!i2c_check_functionality(client->adapter,
+ I2C_FUNC_SMBUS_BYTE_DATA)) {
+ pr_err("i2c is not supported\n");
+ return -EIO;
+ }
+
+ if (!client->dev.platform_data) {
+ pr_err("pdata is not avaiable\n");
+ return -EINVAL;
+ }
+
+ haptic = kzalloc(sizeof(struct isa1200_chip), GFP_KERNEL);
+ if (!haptic) {
+ pr_err("no memory\n");
+ return -ENOMEM;
+ }
+
+ haptic->pdata = client->dev.platform_data;
+ haptic->client = client;
+
+ i2c_set_clientdata(client, haptic);
+
+ haptic->input_device = input_allocate_device();
+ if (!haptic->input_device) {
+ pr_err("input device alloc failed\n");
+ rc = -ENOMEM;
+ goto err_mem_alloc;
+ }
+
+ input_set_drvdata(haptic->input_device, haptic);
+ haptic->input_device->name = haptic->pdata->name ? :
+ "isa1200-ff-memless";
+
+ haptic->input_device->dev.parent = &client->dev;
+
+ input_set_capability(haptic->input_device, EV_FF, FF_RUMBLE);
+
+ haptic->input_device->open = isa1200_open;
+ haptic->input_device->close = isa1200_close;
+
+ rc = input_ff_create_memless(haptic->input_device, NULL,
+ isa1200_play_effect);
+ if (rc < 0) {
+ pr_err("unable to register with ff\n");
+ goto err_free_dev;
+ }
+
+ rc = input_register_device(haptic->input_device);
+ if (rc < 0) {
+ pr_err("unable to register input device\n");
+ goto err_ff_destroy;
+ }
+ return 0;
+
+err_ff_destroy:
+ input_ff_destroy(haptic->input_device);
+err_free_dev:
+ input_free_device(haptic->input_device);
+err_mem_alloc:
+ kfree(haptic);
+ return rc;
+}
+
+static int __devexit isa1200_remove(struct i2c_client *client)
+{
+ struct isa1200_chip *haptic = i2c_get_clientdata(client);
+
+ input_unregister_device(haptic->input_device);
+ kfree(haptic);
+
+ return 0;
+}
+
+static const struct i2c_device_id isa1200_id_table[] = {
+ {"isa1200", 0},
+ { },
+};
+MODULE_DEVICE_TABLE(i2c, isa1200_id_table);
+
+static const struct dev_pm_ops isa1200_pm_ops = {
+ .suspend = isa1200_suspend,
+ .resume = isa1200_resume,
+};
+
+static struct i2c_driver isa1200_driver = {
+ .driver = {
+ .name = "isa1200",
+ .owner = THIS_MODULE,
+ .pm = &isa1200_pm_ops,
+ },
+ .probe = isa1200_probe,
+ .remove = __devexit_p(isa1200_remove),
+ .id_table = isa1200_id_table,
+};
+
+static int __init isa1200_init(void)
+{
+ return i2c_add_driver(&isa1200_driver);
+}
+module_init(isa1200_init);
+
+static void __exit isa1200_exit(void)
+{
+ i2c_del_driver(&isa1200_driver);
+}
+module_exit(isa1200_exit);
+
+MODULE_DESCRIPTION("isa1200 based vibrator chip driver");
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Kyungmin Park <[email protected]>");
+MODULE_AUTHOR("Mohan Pallaka <[email protected]>");
diff --git a/include/linux/input/isa1200.h b/include/linux/input/isa1200.h
new file mode 100644
index 0000000..28c18b7
--- /dev/null
+++ b/include/linux/input/isa1200.h
@@ -0,0 +1,45 @@
+/*
+ * Copyright (C) 2009 Samsung Electronics
+ * Kyungmin Park <[email protected]>
+ *
+ * Copyright (c) 2011, Code Aurora Forum. All rights reserved.
+ *
+ * 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.
+ */
+
+#ifndef __LINUX_ISA1200_H
+#define __LINUX_ISA1200_H
+
+enum mode_control {
+ POWER_DOWN_MODE = 0,
+ PWM_INPUT_MODE,
+ PWM_GEN_MODE,
+ WAVE_GEN_MODE
+};
+
+union pwm_div_freq {
+ unsigned int pwm_div; /* PWM gen mode */
+ unsigned int pwm_freq; /* PWM input mode */
+};
+
+struct isa1200_platform_data {
+ const char *name;
+ unsigned int pwm_ch_id; /* pwm channel id */
+ unsigned int max_timeout;
+ unsigned int hap_en_gpio;
+ bool overdrive_high; /* high/low overdrive */
+ bool overdrive_en; /* enable/disable overdrive */
+ enum mode_control mode_ctrl; /* input/generation/wave */
+ union pwm_div_freq pwm_fd;
+ bool smart_en; /* smart mode enable/disable */
+ bool is_erm;
+ bool ext_clk_en;
+ unsigned int chip_en;
+ unsigned int duty;
+ int (*power_on)(int on);
+ int (*dev_setup)(bool on);
+};
+
+#endif /* __LINUX_ISA1200_H */

--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

2011-03-10 12:22:48

by Mohan Pallaka

[permalink] [raw]
Subject: Re: [PATCH 0/2] Support for isa1200 haptic chip

On 3/7/2011 4:39 PM, Mohan Pallaka wrote:
> This patchset includes a __weak attributed pwm apis in pwm.h and
> a ff-memless based isa1200 driver.
>
> ISA1200 chip can operate in two modes and if the chip has been
> configured for pwm-generation mode, which doesn't use pwm standard
> apis then it'd encounter compilation errors if the host doesn't
> support PWM. This driver is based on Park's isa1200 driver that uses
> haptic class.
>
> Kyungmin Park (1):
> input: misc: Add support for isa1200 chip
>
> Mohan Pallaka (1):
> pwm: Add __weak attributed functions for pwm operations
>
> drivers/input/misc/Kconfig | 12 ++
> drivers/input/misc/Makefile | 1 +
> drivers/input/misc/isa1200.c | 440 +++++++++++++++++++++++++++++++++++++++++
> include/linux/input/isa1200.h | 45 +++++
> include/linux/pwm.h | 21 ++-
> 5 files changed, 514 insertions(+), 5 deletions(-)
> create mode 100644 drivers/input/misc/isa1200.c
> create mode 100644 include/linux/input/isa1200.h
>

Ping.

Dmitry, could you spend sometime to review this.

Thanks,
Mohan.

--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

2011-03-10 12:26:33

by Mohan Pallaka

[permalink] [raw]
Subject: Re: [PATCH 1/2] pwm: Add __weak attributed functions for pwm operations

On 3/7/2011 4:39 PM, Mohan Pallaka wrote:
> For chip drivers that support both pwm and non-pwm modes
> would encounter compilation errors if the platform doesn't
> have support for pwm though the chip is programmed to work
> in non-pwm mode. Add __weak attributed pwm functions to avoid
> compilation issues in these scenarios.
>
> Signed-off-by: Mohan Pallaka <[email protected]>
> ---
> include/linux/pwm.h | 21 ++++++++++++++++-----
> 1 files changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/pwm.h b/include/linux/pwm.h
> index 7c77575..3a8c3df 100644
> --- a/include/linux/pwm.h
> +++ b/include/linux/pwm.h
> @@ -3,29 +3,40 @@
>
> struct pwm_device;
>
> +/* Add __weak functions to support PWM */
> +
> /*
> * pwm_request - request a PWM device
> */
> -struct pwm_device *pwm_request(int pwm_id, const char *label);
> +struct pwm_device __weak *pwm_request(int pwm_id, const char *label)
> +{
> + return NULL;
> +}
>
> /*
> * pwm_free - free a PWM device
> */
> -void pwm_free(struct pwm_device *pwm);
> +void __weak pwm_free(struct pwm_device *pwm) { }
>
> /*
> * pwm_config - change a PWM device configuration
> */
> -int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns);
> +int __weak pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
> +{
> + return -ENODEV;
> +}
>
> /*
> * pwm_enable - start a PWM output toggling
> */
> -int pwm_enable(struct pwm_device *pwm);
> +int __weak pwm_enable(struct pwm_device *pwm)
> +{
> + return -EINVAL;
> +}
>
> /*
> * pwm_disable - stop a PWM output toggling
> */
> -void pwm_disable(struct pwm_device *pwm);
> +void __weak pwm_disable(struct pwm_device *pwm) { }
>
> #endif /* __LINUX_PWM_H */
>

Ping.

Please spend sometime to review this.

Thanks,
Mohan.

--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

2011-03-10 12:27:46

by Mohan Pallaka

[permalink] [raw]
Subject: Re: [PATCH 2/2] input: misc: Add support for isa1200 chip

On 3/7/2011 4:39 PM, Mohan Pallaka wrote:
> From: Kyungmin Park <[email protected]>
>
> isa1200 chip can be used to generate vibrations to indicate
> silent alerts, providing gaming haptic actions or vibrations
> in response to touches. It can operate in two modes, pwm
> generation and pwm input mode. Pwm generation mode requires
> external clock and pwm input mode needs the pwm signal to be
> given as input to the chip.
>
> This patch has been derived based on Kyungmin Park's haptic
> framework patches at http://lkml.org/lkml/2009/10/7/50 . Park's
> isa1200 driver registers to the haptic class, which is also
> introduced as part of the same patch series, and userspace will
> operate the vibrator through exported sysfs entries. This version
> supports only pwm input mode.
>
> This derived patch converts the driver from haptic framework to
> the Linux supported force feeback framework. It also supports the
> chip operation in both pwm input and generation modes.
>
> Signed-off-by: Kyungmin Park <[email protected]>
> Signed-off-by: Mohan Pallaka <[email protected]>
> ---
> drivers/input/misc/Kconfig | 12 ++
> drivers/input/misc/Makefile | 1 +
> drivers/input/misc/isa1200.c | 440 +++++++++++++++++++++++++++++++++++++++++
> include/linux/input/isa1200.h | 45 +++++
> 4 files changed, 498 insertions(+), 0 deletions(-)
> create mode 100644 drivers/input/misc/isa1200.c
> create mode 100644 include/linux/input/isa1200.h
>
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index b0c6772..6d20b8b 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -302,6 +302,18 @@ config HP_SDC_RTC
> Say Y here if you want to support the built-in real time clock
> of the HP SDC controller.
>
> +config INPUT_ISA1200
> + tristate "ISA1200 haptic support"
> + depends on I2C
> + select INPUT_FF_MEMLESS
> + help
> + ISA1200 is a high performance enhanced haptic chip.
> + Say Y here if you want to support ISA1200 connected via I2C,
> + and select N if you are unsure.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called isa1200.
> +
> config INPUT_PCF50633_PMU
> tristate "PCF50633 PMU events"
> depends on MFD_PCF50633
> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
> index 9b47971..4570154 100644
> --- a/drivers/input/misc/Makefile
> +++ b/drivers/input/misc/Makefile
> @@ -23,6 +23,7 @@ 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_ISA1200) += isa1200.o
> obj-$(CONFIG_INPUT_IXP4XX_BEEPER) += ixp4xx-beeper.o
> obj-$(CONFIG_INPUT_KEYSPAN_REMOTE) += keyspan_remote.o
> obj-$(CONFIG_INPUT_M68K_BEEP) += m68kspkr.o
> diff --git a/drivers/input/misc/isa1200.c b/drivers/input/misc/isa1200.c
> new file mode 100644
> index 0000000..bce96f8
> --- /dev/null
> +++ b/drivers/input/misc/isa1200.c
> @@ -0,0 +1,440 @@
> +/*
> + * Copyright (C) 2009 Samsung Electronics
> + * Kyungmin Park <[email protected]>
> + *
> + * Copyright (c) 2011, Code Aurora Forum. All rights reserved.
> + *
> + * 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.
> + */
> +
> +#define pr_fmt(fmt) "%s: " fmt, __func__
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/i2c.h>
> +#include <linux/gpio.h>
> +#include <linux/workqueue.h>
> +#include <linux/delay.h>
> +#include <linux/pwm.h>
> +#include <linux/input.h>
> +#include <linux/slab.h>
> +#include <linux/pm.h>
> +#include <linux/input/isa1200.h>
> +
> +#define ISA1200_HCTRL0 0x30
> +#define HCTRL0_MODE_CTRL_BIT (3)
> +#define HCTRL0_OVERDRIVE_HIGH_BIT (5)
> +#define HCTRL0_OVERDRIVE_EN_BIT (6)
> +#define HCTRL0_HAP_EN (7)
> +#define HCTRL0_RESET 0x01
> +#define HCTRL1_RESET 0x4B
> +
> +#define ISA1200_HCTRL1 0x31
> +#define HCTRL1_SMART_ENABLE_BIT (3)
> +#define HCTRL1_ERM_BIT (5)
> +#define HCTRL1_EXT_CLK_ENABLE_BIT (7)
> +
> +#define ISA1200_HCTRL5 0x35
> +#define HCTRL5_VIB_STRT 0xD5
> +#define HCTRL5_VIB_STOP 0x6B
> +
> +#define DIVIDER_128 (128)
> +#define DIVIDER_1024 (1024)
> +#define DIVIDE_SHIFTER_128 (7)
> +
> +#define FREQ_22400 (22400)
> +#define FREQ_172600 (172600)
> +
> +#define POR_DELAY_USEC 250
> +
> +struct isa1200_chip {
> + const struct isa1200_platform_data *pdata;
> + struct i2c_client *client;
> + struct input_dev *input_device;
> + struct pwm_device *pwm;
> + unsigned int period_ns;
> + unsigned int state;
> + struct work_struct work;
> +};
> +
> +static void isa1200_vib_set(struct isa1200_chip *haptic, int enable)
> +{
> + int rc;
> +
> + if (enable) {
> + if (haptic->pdata->mode_ctrl == PWM_INPUT_MODE) {
> + rc = pwm_config(haptic->pwm,
> + (haptic->period_ns * haptic->pdata->duty) / 100,
> + haptic->period_ns);
> + if (rc < 0)
> + pr_err("pwm_config fail\n");
> + rc = pwm_enable(haptic->pwm);
> + if (rc < 0)
> + pr_err("pwm_enable fail\n");
> + } else if (haptic->pdata->mode_ctrl == PWM_GEN_MODE) {
> + rc = i2c_smbus_write_byte_data(haptic->client,
> + ISA1200_HCTRL5,
> + HCTRL5_VIB_STRT);
> + if (rc < 0)
> + pr_err("start vibration fail\n");
> + }
> + } else {
> + if (haptic->pdata->mode_ctrl == PWM_INPUT_MODE)
> + pwm_disable(haptic->pwm);
> + else if (haptic->pdata->mode_ctrl == PWM_GEN_MODE) {
> + rc = i2c_smbus_write_byte_data(haptic->client,
> + ISA1200_HCTRL5,
> + HCTRL5_VIB_STOP);
> + if (rc < 0)
> + pr_err("stop vibration fail\n");
> + }
> + }
> +}
> +
> +static int isa1200_setup(struct i2c_client *client)
> +{
> + struct isa1200_chip *haptic = i2c_get_clientdata(client);
> + int value, temp, rc;
> +
> + gpio_set_value_cansleep(haptic->pdata->hap_en_gpio, 0);
> + udelay(POR_DELAY_USEC);
> + gpio_set_value_cansleep(haptic->pdata->hap_en_gpio, 1);
> +
> + value = (haptic->pdata->smart_en << HCTRL1_SMART_ENABLE_BIT) |
> + (haptic->pdata->is_erm << HCTRL1_ERM_BIT) |
> + (haptic->pdata->ext_clk_en << HCTRL1_EXT_CLK_ENABLE_BIT);
> +
> + rc = i2c_smbus_write_byte_data(client, ISA1200_HCTRL1, value);
> + if (rc < 0) {
> + pr_err("i2c write failure\n");
> + return rc;
> + }
> +
> + if (haptic->pdata->mode_ctrl == PWM_GEN_MODE) {
> + temp = haptic->pdata->pwm_fd.pwm_div;
> + if (temp < DIVIDER_128 || temp > DIVIDER_1024 ||
> + temp % DIVIDER_128) {
> + pr_err("Invalid divider\n");
> + rc = -EINVAL;
> + goto reset_hctrl1;
> + }
> + value = ((temp >> DIVIDE_SHIFTER_128) - 1);
> + } else if (haptic->pdata->mode_ctrl == PWM_INPUT_MODE) {
> + temp = haptic->pdata->pwm_fd.pwm_freq;
> + if (temp < FREQ_22400 || temp > FREQ_172600 ||
> + temp % FREQ_22400) {
> + pr_err("Invalid frequency\n");
> + rc = -EINVAL;
> + goto reset_hctrl1;
> + }
> + value = ((temp / FREQ_22400) - 1);
> + haptic->period_ns = NSEC_PER_SEC / temp;
> + }
> + value |= (haptic->pdata->mode_ctrl << HCTRL0_MODE_CTRL_BIT) |
> + (haptic->pdata->overdrive_high << HCTRL0_OVERDRIVE_HIGH_BIT) |
> + (haptic->pdata->overdrive_en << HCTRL0_OVERDRIVE_EN_BIT) |
> + (haptic->pdata->chip_en << HCTRL0_HAP_EN);
> +
> + rc = i2c_smbus_write_byte_data(client, ISA1200_HCTRL0, value);
> + if (rc < 0) {
> + pr_err("i2c write failure\n");
> + goto reset_hctrl1;
> + }
> +
> + return 0;
> +
> +reset_hctrl1:
> + i2c_smbus_write_byte_data(client, ISA1200_HCTRL1,
> + HCTRL1_RESET);
> + return rc;
> +}
> +
> +static void isa1200_worker(struct work_struct *work)
> +{
> + struct isa1200_chip *haptic;
> +
> + haptic = container_of(work, struct isa1200_chip, work);
> + isa1200_vib_set(haptic, !!haptic->state);
> +}
> +
> +static int isa1200_play_effect(struct input_dev *dev, void *data,
> + struct ff_effect *effect)
> +{
> + struct isa1200_chip *haptic = input_get_drvdata(dev);
> +
> + /* support basic vibration */
> + haptic->state = effect->u.rumble.strong_magnitude >> 8;
> + if (!haptic->state)
> + haptic->state = effect->u.rumble.weak_magnitude >> 9;
> +
> + schedule_work(&haptic->work);
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM
> +static int isa1200_suspend(struct device *dev)
> +{
> + struct isa1200_chip *haptic = dev_get_drvdata(dev);
> + int rc;
> +
> + cancel_work_sync(&haptic->work);
> + /* turn-off current vibration */
> + isa1200_vib_set(haptic, 0);
> +
> + if (haptic->pdata->power_on) {
> + rc = haptic->pdata->power_on(0);
> + if (rc) {
> + pr_err("power-down failed\n");
> + return rc;
> + }
> + }
> + return 0;
> +}
> +
> +static int isa1200_resume(struct device *dev)
> +{
> + struct isa1200_chip *haptic = dev_get_drvdata(dev);
> + int rc;
> +
> + if (haptic->pdata->power_on) {
> + rc = haptic->pdata->power_on(1);
> + if (rc) {
> + pr_err("power-up failed\n");
> + return rc;
> + }
> + }
> +
> + isa1200_setup(haptic->client);
> + return 0;
> +}
> +#else
> +#define isa1200_suspend NULL
> +#define isa1200_resume NULL
> +#endif
> +
> +static int isa1200_open(struct input_dev *dev)
> +{
> + struct isa1200_chip *haptic = input_get_drvdata(dev);
> + int rc;
> +
> + /* device setup */
> + if (haptic->pdata->dev_setup) {
> + rc = haptic->pdata->dev_setup(true);
> + if (rc < 0) {
> + pr_err("setup failed!\n");
> + return rc;
> + }
> + }
> +
> + /* power on */
> + if (haptic->pdata->power_on) {
> + rc = haptic->pdata->power_on(true);
> + if (rc < 0) {
> + pr_err("power failed\n");
> + goto err_setup;
> + }
> + }
> +
> + /* request gpio */
> + rc = gpio_is_valid(haptic->pdata->hap_en_gpio);
> + if (rc) {
> + rc = gpio_request(haptic->pdata->hap_en_gpio, "haptic_gpio");
> + if (rc) {
> + pr_err("gpio %d request failed\n",
> + haptic->pdata->hap_en_gpio);
> + goto err_power_on;
> + }
> + } else {
> + pr_err("Invalid gpio %d\n",
> + haptic->pdata->hap_en_gpio);
> + goto err_power_on;
> + }
> +
> + rc = gpio_direction_output(haptic->pdata->hap_en_gpio, 0);
> + if (rc) {
> + pr_err("gpio %d set direction failed\n",
> + haptic->pdata->hap_en_gpio);
> + goto err_gpio_free;
> + }
> +
> + /* setup registers */
> + rc = isa1200_setup(haptic->client);
> + if (rc < 0) {
> + pr_err("setup fail %d\n", rc);
> + goto err_gpio_free;
> + }
> +
> + if (haptic->pdata->mode_ctrl == PWM_INPUT_MODE) {
> + haptic->pwm = pwm_request(haptic->pdata->pwm_ch_id,
> + haptic->client->driver->id_table->name);
> + if (IS_ERR(haptic->pwm)) {
> + pr_err("pwm request failed\n");
> + rc = PTR_ERR(haptic->pwm);
> + goto err_reset_hctrl0;
> + }
> + }
> +
> + /* init workqeueue */
> + INIT_WORK(&haptic->work, isa1200_worker);
> + return 0;
> +
> +err_reset_hctrl0:
> + i2c_smbus_write_byte_data(haptic->client, ISA1200_HCTRL0,
> + HCTRL0_RESET);
> +err_gpio_free:
> + gpio_free(haptic->pdata->hap_en_gpio);
> +err_power_on:
> + if (haptic->pdata->power_on)
> + haptic->pdata->power_on(0);
> +err_setup:
> + if (haptic->pdata->dev_setup)
> + haptic->pdata->dev_setup(false);
> +
> + return rc;
> +}
> +
> +static void isa1200_close(struct input_dev *dev)
> +{
> + struct isa1200_chip *haptic = input_get_drvdata(dev);
> +
> + /* turn-off current vibration */
> + isa1200_vib_set(haptic, 0);
> +
> + if (haptic->pdata->mode_ctrl == PWM_INPUT_MODE)
> + pwm_free(haptic->pwm);
> +
> + gpio_free(haptic->pdata->hap_en_gpio);
> +
> + /* reset hardware registers */
> + i2c_smbus_write_byte_data(haptic->client, ISA1200_HCTRL0,
> + HCTRL0_RESET);
> + i2c_smbus_write_byte_data(haptic->client, ISA1200_HCTRL1,
> + HCTRL1_RESET);
> +
> + if (haptic->pdata->dev_setup)
> + haptic->pdata->dev_setup(false);
> +
> + /* power-off the chip */
> + if (haptic->pdata->power_on)
> + haptic->pdata->power_on(0);
> +}
> +
> +static int __devinit isa1200_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct isa1200_chip *haptic;
> + int rc;
> +
> + if (!i2c_check_functionality(client->adapter,
> + I2C_FUNC_SMBUS_BYTE_DATA)) {
> + pr_err("i2c is not supported\n");
> + return -EIO;
> + }
> +
> + if (!client->dev.platform_data) {
> + pr_err("pdata is not avaiable\n");
> + return -EINVAL;
> + }
> +
> + haptic = kzalloc(sizeof(struct isa1200_chip), GFP_KERNEL);
> + if (!haptic) {
> + pr_err("no memory\n");
> + return -ENOMEM;
> + }
> +
> + haptic->pdata = client->dev.platform_data;
> + haptic->client = client;
> +
> + i2c_set_clientdata(client, haptic);
> +
> + haptic->input_device = input_allocate_device();
> + if (!haptic->input_device) {
> + pr_err("input device alloc failed\n");
> + rc = -ENOMEM;
> + goto err_mem_alloc;
> + }
> +
> + input_set_drvdata(haptic->input_device, haptic);
> + haptic->input_device->name = haptic->pdata->name ? :
> + "isa1200-ff-memless";
> +
> + haptic->input_device->dev.parent = &client->dev;
> +
> + input_set_capability(haptic->input_device, EV_FF, FF_RUMBLE);
> +
> + haptic->input_device->open = isa1200_open;
> + haptic->input_device->close = isa1200_close;
> +
> + rc = input_ff_create_memless(haptic->input_device, NULL,
> + isa1200_play_effect);
> + if (rc < 0) {
> + pr_err("unable to register with ff\n");
> + goto err_free_dev;
> + }
> +
> + rc = input_register_device(haptic->input_device);
> + if (rc < 0) {
> + pr_err("unable to register input device\n");
> + goto err_ff_destroy;
> + }
> + return 0;
> +
> +err_ff_destroy:
> + input_ff_destroy(haptic->input_device);
> +err_free_dev:
> + input_free_device(haptic->input_device);
> +err_mem_alloc:
> + kfree(haptic);
> + return rc;
> +}
> +
> +static int __devexit isa1200_remove(struct i2c_client *client)
> +{
> + struct isa1200_chip *haptic = i2c_get_clientdata(client);
> +
> + input_unregister_device(haptic->input_device);
> + kfree(haptic);
> +
> + return 0;
> +}
> +
> +static const struct i2c_device_id isa1200_id_table[] = {
> + {"isa1200", 0},
> + { },
> +};
> +MODULE_DEVICE_TABLE(i2c, isa1200_id_table);
> +
> +static const struct dev_pm_ops isa1200_pm_ops = {
> + .suspend = isa1200_suspend,
> + .resume = isa1200_resume,
> +};
> +
> +static struct i2c_driver isa1200_driver = {
> + .driver = {
> + .name = "isa1200",
> + .owner = THIS_MODULE,
> + .pm = &isa1200_pm_ops,
> + },
> + .probe = isa1200_probe,
> + .remove = __devexit_p(isa1200_remove),
> + .id_table = isa1200_id_table,
> +};
> +
> +static int __init isa1200_init(void)
> +{
> + return i2c_add_driver(&isa1200_driver);
> +}
> +module_init(isa1200_init);
> +
> +static void __exit isa1200_exit(void)
> +{
> + i2c_del_driver(&isa1200_driver);
> +}
> +module_exit(isa1200_exit);
> +
> +MODULE_DESCRIPTION("isa1200 based vibrator chip driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Kyungmin Park <[email protected]>");
> +MODULE_AUTHOR("Mohan Pallaka <[email protected]>");
> diff --git a/include/linux/input/isa1200.h b/include/linux/input/isa1200.h
> new file mode 100644
> index 0000000..28c18b7
> --- /dev/null
> +++ b/include/linux/input/isa1200.h
> @@ -0,0 +1,45 @@
> +/*
> + * Copyright (C) 2009 Samsung Electronics
> + * Kyungmin Park <[email protected]>
> + *
> + * Copyright (c) 2011, Code Aurora Forum. All rights reserved.
> + *
> + * 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.
> + */
> +
> +#ifndef __LINUX_ISA1200_H
> +#define __LINUX_ISA1200_H
> +
> +enum mode_control {
> + POWER_DOWN_MODE = 0,
> + PWM_INPUT_MODE,
> + PWM_GEN_MODE,
> + WAVE_GEN_MODE
> +};
> +
> +union pwm_div_freq {
> + unsigned int pwm_div; /* PWM gen mode */
> + unsigned int pwm_freq; /* PWM input mode */
> +};
> +
> +struct isa1200_platform_data {
> + const char *name;
> + unsigned int pwm_ch_id; /* pwm channel id */
> + unsigned int max_timeout;
> + unsigned int hap_en_gpio;
> + bool overdrive_high; /* high/low overdrive */
> + bool overdrive_en; /* enable/disable overdrive */
> + enum mode_control mode_ctrl; /* input/generation/wave */
> + union pwm_div_freq pwm_fd;
> + bool smart_en; /* smart mode enable/disable */
> + bool is_erm;
> + bool ext_clk_en;
> + unsigned int chip_en;
> + unsigned int duty;
> + int (*power_on)(int on);
> + int (*dev_setup)(bool on);
> +};
> +
> +#endif /* __LINUX_ISA1200_H */
>

Please spend sometime to review this.

Thanks,
Mohan.

--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

2011-03-21 07:06:17

by Trilok Soni

[permalink] [raw]
Subject: Re: [PATCH 2/2] input: misc: Add support for isa1200 chip

Hi Dmitry,

On 3/7/2011 4:39 PM, Mohan Pallaka wrote:
> From: Kyungmin Park <[email protected]>
>
> isa1200 chip can be used to generate vibrations to indicate
> silent alerts, providing gaming haptic actions or vibrations
> in response to touches. It can operate in two modes, pwm
> generation and pwm input mode. Pwm generation mode requires
> external clock and pwm input mode needs the pwm signal to be
> given as input to the chip.
>

Could you please review this patch series?

---Trilok Soni

--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

2011-03-24 07:49:21

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH 2/2] input: misc: Add support for isa1200 chip

Hi,

On Mon, Mar 07, 2011 at 04:39:42PM +0530, Mohan Pallaka wrote:
> From: Kyungmin Park <[email protected]>
>
> isa1200 chip can be used to generate vibrations to indicate
> silent alerts, providing gaming haptic actions or vibrations
> in response to touches. It can operate in two modes, pwm
> generation and pwm input mode. Pwm generation mode requires
> external clock and pwm input mode needs the pwm signal to be
> given as input to the chip.
>
> This patch has been derived based on Kyungmin Park's haptic
> framework patches at http://lkml.org/lkml/2009/10/7/50 . Park's
> isa1200 driver registers to the haptic class, which is also
> introduced as part of the same patch series, and userspace will
> operate the vibrator through exported sysfs entries. This version
> supports only pwm input mode.
>
> This derived patch converts the driver from haptic framework to
> the Linux supported force feeback framework. It also supports the
> chip operation in both pwm input and generation modes.

Looks fairly reasonable, a few comments below.

>
> Signed-off-by: Kyungmin Park <[email protected]>
> Signed-off-by: Mohan Pallaka <[email protected]>
> ---
> drivers/input/misc/Kconfig | 12 ++
> drivers/input/misc/Makefile | 1 +
> drivers/input/misc/isa1200.c | 440 +++++++++++++++++++++++++++++++++++++++++
> include/linux/input/isa1200.h | 45 +++++
> 4 files changed, 498 insertions(+), 0 deletions(-)
> create mode 100644 drivers/input/misc/isa1200.c
> create mode 100644 include/linux/input/isa1200.h
>
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index b0c6772..6d20b8b 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -302,6 +302,18 @@ config HP_SDC_RTC
> Say Y here if you want to support the built-in real time clock
> of the HP SDC controller.
>
> +config INPUT_ISA1200
> + tristate "ISA1200 haptic support"
> + depends on I2C
> + select INPUT_FF_MEMLESS
> + help
> + ISA1200 is a high performance enhanced haptic chip.
> + Say Y here if you want to support ISA1200 connected via I2C,
> + and select N if you are unsure.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called isa1200.
> +
> config INPUT_PCF50633_PMU
> tristate "PCF50633 PMU events"
> depends on MFD_PCF50633
> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
> index 9b47971..4570154 100644
> --- a/drivers/input/misc/Makefile
> +++ b/drivers/input/misc/Makefile
> @@ -23,6 +23,7 @@ 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_ISA1200) += isa1200.o
> obj-$(CONFIG_INPUT_IXP4XX_BEEPER) += ixp4xx-beeper.o
> obj-$(CONFIG_INPUT_KEYSPAN_REMOTE) += keyspan_remote.o
> obj-$(CONFIG_INPUT_M68K_BEEP) += m68kspkr.o
> diff --git a/drivers/input/misc/isa1200.c b/drivers/input/misc/isa1200.c
> new file mode 100644
> index 0000000..bce96f8
> --- /dev/null
> +++ b/drivers/input/misc/isa1200.c
> @@ -0,0 +1,440 @@
> +/*
> + * Copyright (C) 2009 Samsung Electronics
> + * Kyungmin Park <[email protected]>
> + *
> + * Copyright (c) 2011, Code Aurora Forum. All rights reserved.
> + *
> + * 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.
> + */
> +
> +#define pr_fmt(fmt) "%s: " fmt, __func__
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/i2c.h>
> +#include <linux/gpio.h>
> +#include <linux/workqueue.h>
> +#include <linux/delay.h>
> +#include <linux/pwm.h>
> +#include <linux/input.h>
> +#include <linux/slab.h>
> +#include <linux/pm.h>
> +#include <linux/input/isa1200.h>
> +
> +#define ISA1200_HCTRL0 0x30
> +#define HCTRL0_MODE_CTRL_BIT (3)
> +#define HCTRL0_OVERDRIVE_HIGH_BIT (5)
> +#define HCTRL0_OVERDRIVE_EN_BIT (6)
> +#define HCTRL0_HAP_EN (7)
> +#define HCTRL0_RESET 0x01
> +#define HCTRL1_RESET 0x4B
> +
> +#define ISA1200_HCTRL1 0x31
> +#define HCTRL1_SMART_ENABLE_BIT (3)
> +#define HCTRL1_ERM_BIT (5)
> +#define HCTRL1_EXT_CLK_ENABLE_BIT (7)
> +
> +#define ISA1200_HCTRL5 0x35
> +#define HCTRL5_VIB_STRT 0xD5
> +#define HCTRL5_VIB_STOP 0x6B
> +
> +#define DIVIDER_128 (128)
> +#define DIVIDER_1024 (1024)
> +#define DIVIDE_SHIFTER_128 (7)
> +
> +#define FREQ_22400 (22400)
> +#define FREQ_172600 (172600)
> +
> +#define POR_DELAY_USEC 250
> +
> +struct isa1200_chip {
> + const struct isa1200_platform_data *pdata;
> + struct i2c_client *client;
> + struct input_dev *input_device;
> + struct pwm_device *pwm;
> + unsigned int period_ns;
> + unsigned int state;
> + struct work_struct work;
> +};
> +
> +static void isa1200_vib_set(struct isa1200_chip *haptic, int enable)
> +{
> + int rc;
> +
> + if (enable) {
> + if (haptic->pdata->mode_ctrl == PWM_INPUT_MODE) {
> + rc = pwm_config(haptic->pwm,
> + (haptic->period_ns * haptic->pdata->duty) / 100,
> + haptic->period_ns);
> + if (rc < 0)
> + pr_err("pwm_config fail\n");
> + rc = pwm_enable(haptic->pwm);
> + if (rc < 0)
> + pr_err("pwm_enable fail\n");
> + } else if (haptic->pdata->mode_ctrl == PWM_GEN_MODE) {
> + rc = i2c_smbus_write_byte_data(haptic->client,
> + ISA1200_HCTRL5,
> + HCTRL5_VIB_STRT);
> + if (rc < 0)
> + pr_err("start vibration fail\n");
> + }
> + } else {
> + if (haptic->pdata->mode_ctrl == PWM_INPUT_MODE)
> + pwm_disable(haptic->pwm);
> + else if (haptic->pdata->mode_ctrl == PWM_GEN_MODE) {
> + rc = i2c_smbus_write_byte_data(haptic->client,
> + ISA1200_HCTRL5,
> + HCTRL5_VIB_STOP);
> + if (rc < 0)
> + pr_err("stop vibration fail\n");
> + }
> + }
> +}
> +
> +static int isa1200_setup(struct i2c_client *client)
> +{
> + struct isa1200_chip *haptic = i2c_get_clientdata(client);
> + int value, temp, rc;
> +
> + gpio_set_value_cansleep(haptic->pdata->hap_en_gpio, 0);
> + udelay(POR_DELAY_USEC);
> + gpio_set_value_cansleep(haptic->pdata->hap_en_gpio, 1);
> +
> + value = (haptic->pdata->smart_en << HCTRL1_SMART_ENABLE_BIT) |
> + (haptic->pdata->is_erm << HCTRL1_ERM_BIT) |
> + (haptic->pdata->ext_clk_en << HCTRL1_EXT_CLK_ENABLE_BIT);
> +
> + rc = i2c_smbus_write_byte_data(client, ISA1200_HCTRL1, value);
> + if (rc < 0) {
> + pr_err("i2c write failure\n");
> + return rc;
> + }
> +
> + if (haptic->pdata->mode_ctrl == PWM_GEN_MODE) {
> + temp = haptic->pdata->pwm_fd.pwm_div;
> + if (temp < DIVIDER_128 || temp > DIVIDER_1024 ||
> + temp % DIVIDER_128) {
> + pr_err("Invalid divider\n");

Not "divisor" by any chance?

> + rc = -EINVAL;
> + goto reset_hctrl1;
> + }
> + value = ((temp >> DIVIDE_SHIFTER_128) - 1);
> + } else if (haptic->pdata->mode_ctrl == PWM_INPUT_MODE) {
> + temp = haptic->pdata->pwm_fd.pwm_freq;
> + if (temp < FREQ_22400 || temp > FREQ_172600 ||
> + temp % FREQ_22400) {
> + pr_err("Invalid frequency\n");
> + rc = -EINVAL;
> + goto reset_hctrl1;
> + }
> + value = ((temp / FREQ_22400) - 1);
> + haptic->period_ns = NSEC_PER_SEC / temp;
> + }
> + value |= (haptic->pdata->mode_ctrl << HCTRL0_MODE_CTRL_BIT) |
> + (haptic->pdata->overdrive_high << HCTRL0_OVERDRIVE_HIGH_BIT) |
> + (haptic->pdata->overdrive_en << HCTRL0_OVERDRIVE_EN_BIT) |
> + (haptic->pdata->chip_en << HCTRL0_HAP_EN);
> +
> + rc = i2c_smbus_write_byte_data(client, ISA1200_HCTRL0, value);
> + if (rc < 0) {
> + pr_err("i2c write failure\n");
> + goto reset_hctrl1;
> + }
> +
> + return 0;
> +
> +reset_hctrl1:
> + i2c_smbus_write_byte_data(client, ISA1200_HCTRL1,
> + HCTRL1_RESET);
> + return rc;
> +}
> +
> +static void isa1200_worker(struct work_struct *work)
> +{
> + struct isa1200_chip *haptic;
> +
> + haptic = container_of(work, struct isa1200_chip, work);
> + isa1200_vib_set(haptic, !!haptic->state);
> +}
> +
> +static int isa1200_play_effect(struct input_dev *dev, void *data,
> + struct ff_effect *effect)
> +{
> + struct isa1200_chip *haptic = input_get_drvdata(dev);
> +
> + /* support basic vibration */
> + haptic->state = effect->u.rumble.strong_magnitude >> 8;
> + if (!haptic->state)
> + haptic->state = effect->u.rumble.weak_magnitude >> 9;
> +
> + schedule_work(&haptic->work);
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM

Protect with CONFIG_PM_SLEEP, it will save us a warning if only
CONFIG_PM_RUNTIME is enabled.

> +static int isa1200_suspend(struct device *dev)
> +{
> + struct isa1200_chip *haptic = dev_get_drvdata(dev);
> + int rc;
> +
> + cancel_work_sync(&haptic->work);
> + /* turn-off current vibration */
> + isa1200_vib_set(haptic, 0);
> +
> + if (haptic->pdata->power_on) {
> + rc = haptic->pdata->power_on(0);

->power_on(false)?

Everywhere, when arguments are bool please take care to supply boolena
constants.

> + if (rc) {
> + pr_err("power-down failed\n");
> + return rc;
> + }
> + }
> + return 0;
> +}
> +
> +static int isa1200_resume(struct device *dev)
> +{
> + struct isa1200_chip *haptic = dev_get_drvdata(dev);
> + int rc;
> +
> + if (haptic->pdata->power_on) {
> + rc = haptic->pdata->power_on(1);
> + if (rc) {
> + pr_err("power-up failed\n");
> + return rc;
> + }
> + }
> +
> + isa1200_setup(haptic->client);
> + return 0;
> +}
> +#else
> +#define isa1200_suspend NULL
> +#define isa1200_resume NULL
> +#endif
> +
> +static int isa1200_open(struct input_dev *dev)
> +{
> + struct isa1200_chip *haptic = input_get_drvdata(dev);
> + int rc;
> +
> + /* device setup */
> + if (haptic->pdata->dev_setup) {
> + rc = haptic->pdata->dev_setup(true);
> + if (rc < 0) {
> + pr_err("setup failed!\n");
> + return rc;
> + }
> + }
> +
> + /* power on */
> + if (haptic->pdata->power_on) {
> + rc = haptic->pdata->power_on(true);
> + if (rc < 0) {
> + pr_err("power failed\n");
> + goto err_setup;
> + }
> + }
> +
> + /* request gpio */
> + rc = gpio_is_valid(haptic->pdata->hap_en_gpio);
> + if (rc) {
> + rc = gpio_request(haptic->pdata->hap_en_gpio, "haptic_gpio");
> + if (rc) {
> + pr_err("gpio %d request failed\n",
> + haptic->pdata->hap_en_gpio);
> + goto err_power_on;
> + }
> + } else {
> + pr_err("Invalid gpio %d\n",
> + haptic->pdata->hap_en_gpio);
> + goto err_power_on;
> + }
> +
> + rc = gpio_direction_output(haptic->pdata->hap_en_gpio, 0);
> + if (rc) {
> + pr_err("gpio %d set direction failed\n",
> + haptic->pdata->hap_en_gpio);
> + goto err_gpio_free;
> + }
> +
> + /* setup registers */
> + rc = isa1200_setup(haptic->client);
> + if (rc < 0) {
> + pr_err("setup fail %d\n", rc);
> + goto err_gpio_free;
> + }
> +
> + if (haptic->pdata->mode_ctrl == PWM_INPUT_MODE) {
> + haptic->pwm = pwm_request(haptic->pdata->pwm_ch_id,
> + haptic->client->driver->id_table->name);
> + if (IS_ERR(haptic->pwm)) {
> + pr_err("pwm request failed\n");
> + rc = PTR_ERR(haptic->pwm);
> + goto err_reset_hctrl0;
> + }
> + }
> +
> + /* init workqeueue */
> + INIT_WORK(&haptic->work, isa1200_worker);

Why here and not in probe?

Actually, all resource acquisition (gpio_request, pwm_request, and so
forth) should be done in probe(). open() is supposed to bring device
from low power mode and configure the chip itself.

> + return 0;
> +
> +err_reset_hctrl0:
> + i2c_smbus_write_byte_data(haptic->client, ISA1200_HCTRL0,
> + HCTRL0_RESET);
> +err_gpio_free:
> + gpio_free(haptic->pdata->hap_en_gpio);
> +err_power_on:
> + if (haptic->pdata->power_on)
> + haptic->pdata->power_on(0);
> +err_setup:
> + if (haptic->pdata->dev_setup)
> + haptic->pdata->dev_setup(false);
> +
> + return rc;
> +}
> +
> +static void isa1200_close(struct input_dev *dev)
> +{
> + struct isa1200_chip *haptic = input_get_drvdata(dev);
> +
> + /* turn-off current vibration */
> + isa1200_vib_set(haptic, 0);
> +

Somewhere here should be call to cancel_work_sync() to ensure that play
work does not outlive us.

> + if (haptic->pdata->mode_ctrl == PWM_INPUT_MODE)
> + pwm_free(haptic->pwm);
> +
> + gpio_free(haptic->pdata->hap_en_gpio);
> +
> + /* reset hardware registers */
> + i2c_smbus_write_byte_data(haptic->client, ISA1200_HCTRL0,
> + HCTRL0_RESET);
> + i2c_smbus_write_byte_data(haptic->client, ISA1200_HCTRL1,
> + HCTRL1_RESET);
> +
> + if (haptic->pdata->dev_setup)
> + haptic->pdata->dev_setup(false);
> +
> + /* power-off the chip */
> + if (haptic->pdata->power_on)
> + haptic->pdata->power_on(0);
> +}
> +
> +static int __devinit isa1200_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct isa1200_chip *haptic;
> + int rc;
> +
> + if (!i2c_check_functionality(client->adapter,
> + I2C_FUNC_SMBUS_BYTE_DATA)) {
> + pr_err("i2c is not supported\n");
> + return -EIO;
> + }
> +
> + if (!client->dev.platform_data) {
> + pr_err("pdata is not avaiable\n");
> + return -EINVAL;
> + }
> +
> + haptic = kzalloc(sizeof(struct isa1200_chip), GFP_KERNEL);
> + if (!haptic) {
> + pr_err("no memory\n");
> + return -ENOMEM;
> + }
> +
> + haptic->pdata = client->dev.platform_data;
> + haptic->client = client;
> +
> + i2c_set_clientdata(client, haptic);
> +
> + haptic->input_device = input_allocate_device();
> + if (!haptic->input_device) {
> + pr_err("input device alloc failed\n");
> + rc = -ENOMEM;
> + goto err_mem_alloc;
> + }
> +
> + input_set_drvdata(haptic->input_device, haptic);
> + haptic->input_device->name = haptic->pdata->name ? :
> + "isa1200-ff-memless";

"isa1200-ff-memless" is quite ugly, why don't you give a nicer name to
the device? "ISA1200 Haptic device" or something.

> +
> + haptic->input_device->dev.parent = &client->dev;

input_dev->id.bustype = BUS_I2C;

> +
> + input_set_capability(haptic->input_device, EV_FF, FF_RUMBLE);
> +
> + haptic->input_device->open = isa1200_open;
> + haptic->input_device->close = isa1200_close;
> +
> + rc = input_ff_create_memless(haptic->input_device, NULL,
> + isa1200_play_effect);
> + if (rc < 0) {
> + pr_err("unable to register with ff\n");
> + goto err_free_dev;
> + }
> +
> + rc = input_register_device(haptic->input_device);
> + if (rc < 0) {
> + pr_err("unable to register input device\n");
> + goto err_ff_destroy;
> + }
> + return 0;
> +
> +err_ff_destroy:
> + input_ff_destroy(haptic->input_device);
> +err_free_dev:
> + input_free_device(haptic->input_device);
> +err_mem_alloc:
> + kfree(haptic);
> + return rc;
> +}
> +
> +static int __devexit isa1200_remove(struct i2c_client *client)
> +{
> + struct isa1200_chip *haptic = i2c_get_clientdata(client);
> +
> + input_unregister_device(haptic->input_device);
> + kfree(haptic);
> +
> + return 0;
> +}
> +
> +static const struct i2c_device_id isa1200_id_table[] = {
> + {"isa1200", 0},
> + { },
> +};
> +MODULE_DEVICE_TABLE(i2c, isa1200_id_table);
> +
> +static const struct dev_pm_ops isa1200_pm_ops = {
> + .suspend = isa1200_suspend,
> + .resume = isa1200_resume,
> +};

static SIMPLE_DEV_PM_OPS(isa1200_pm_ops, isa1200_suspend, isa1200_resume);

> +
> +static struct i2c_driver isa1200_driver = {
> + .driver = {
> + .name = "isa1200",
> + .owner = THIS_MODULE,
> + .pm = &isa1200_pm_ops,
> + },
> + .probe = isa1200_probe,
> + .remove = __devexit_p(isa1200_remove),
> + .id_table = isa1200_id_table,
> +};
> +
> +static int __init isa1200_init(void)
> +{
> + return i2c_add_driver(&isa1200_driver);
> +}
> +module_init(isa1200_init);
> +
> +static void __exit isa1200_exit(void)
> +{
> + i2c_del_driver(&isa1200_driver);
> +}
> +module_exit(isa1200_exit);
> +
> +MODULE_DESCRIPTION("isa1200 based vibrator chip driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Kyungmin Park <[email protected]>");
> +MODULE_AUTHOR("Mohan Pallaka <[email protected]>");
> diff --git a/include/linux/input/isa1200.h b/include/linux/input/isa1200.h
> new file mode 100644
> index 0000000..28c18b7
> --- /dev/null
> +++ b/include/linux/input/isa1200.h
> @@ -0,0 +1,45 @@
> +/*
> + * Copyright (C) 2009 Samsung Electronics
> + * Kyungmin Park <[email protected]>
> + *
> + * Copyright (c) 2011, Code Aurora Forum. All rights reserved.
> + *
> + * 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.
> + */
> +
> +#ifndef __LINUX_ISA1200_H
> +#define __LINUX_ISA1200_H
> +
> +enum mode_control {
> + POWER_DOWN_MODE = 0,
> + PWM_INPUT_MODE,
> + PWM_GEN_MODE,
> + WAVE_GEN_MODE
> +};
> +
> +union pwm_div_freq {
> + unsigned int pwm_div; /* PWM gen mode */
> + unsigned int pwm_freq; /* PWM input mode */
> +};
> +
> +struct isa1200_platform_data {
> + const char *name;
> + unsigned int pwm_ch_id; /* pwm channel id */
> + unsigned int max_timeout;
> + unsigned int hap_en_gpio;
> + bool overdrive_high; /* high/low overdrive */
> + bool overdrive_en; /* enable/disable overdrive */
> + enum mode_control mode_ctrl; /* input/generation/wave */
> + union pwm_div_freq pwm_fd;
> + bool smart_en; /* smart mode enable/disable */
> + bool is_erm;
> + bool ext_clk_en;
> + unsigned int chip_en;
> + unsigned int duty;
> + int (*power_on)(int on);

bool on?

> + int (*dev_setup)(bool on);
> +};
> +
> +#endif /* __LINUX_ISA1200_H */
>

Thanks.

--
Dmitry

2011-04-06 06:03:59

by Mohan Pallaka

[permalink] [raw]
Subject: Re: [PATCH 2/2] input: misc: Add support for isa1200 chip

Thanks for review. The new patch with addressed comments is in internal review phase.
I'd post it as soon as it gets cleared internally.

On 3/24/2011 1:19 PM, Dmitry Torokhov wrote:
> Hi,
>
> On Mon, Mar 07, 2011 at 04:39:42PM +0530, Mohan Pallaka wrote:
>> From: Kyungmin Park <[email protected]>
>>
>> isa1200 chip can be used to generate vibrations to indicate
>> silent alerts, providing gaming haptic actions or vibrations
>> in response to touches. It can operate in two modes, pwm
>> generation and pwm input mode. Pwm generation mode requires
>> external clock and pwm input mode needs the pwm signal to be
>> given as input to the chip.
>>
>> This patch has been derived based on Kyungmin Park's haptic
>> framework patches at http://lkml.org/lkml/2009/10/7/50 . Park's
>> isa1200 driver registers to the haptic class, which is also
>> introduced as part of the same patch series, and userspace will
>> operate the vibrator through exported sysfs entries. This version
>> supports only pwm input mode.
>>
>> This derived patch converts the driver from haptic framework to
>> the Linux supported force feeback framework. It also supports the
>> chip operation in both pwm input and generation modes.
> Looks fairly reasonable, a few comments below.
>
>> Signed-off-by: Kyungmin Park <[email protected]>
>> Signed-off-by: Mohan Pallaka <[email protected]>
>> ---
>> drivers/input/misc/Kconfig | 12 ++
>> drivers/input/misc/Makefile | 1 +
>> drivers/input/misc/isa1200.c | 440 +++++++++++++++++++++++++++++++++++++++++
>> include/linux/input/isa1200.h | 45 +++++
>> 4 files changed, 498 insertions(+), 0 deletions(-)
>> create mode 100644 drivers/input/misc/isa1200.c
>> create mode 100644 include/linux/input/isa1200.h
>>
>> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
>> index b0c6772..6d20b8b 100644
>> --- a/drivers/input/misc/Kconfig
>> +++ b/drivers/input/misc/Kconfig
>> @@ -302,6 +302,18 @@ config HP_SDC_RTC
>> Say Y here if you want to support the built-in real time clock
>> of the HP SDC controller.
>>
>> +config INPUT_ISA1200
>> + tristate "ISA1200 haptic support"
>> + depends on I2C
>> + select INPUT_FF_MEMLESS
>> + help
>> + ISA1200 is a high performance enhanced haptic chip.
>> + Say Y here if you want to support ISA1200 connected via I2C,
>> + and select N if you are unsure.
>> +
>> + To compile this driver as a module, choose M here: the
>> + module will be called isa1200.
>> +
>> config INPUT_PCF50633_PMU
>> tristate "PCF50633 PMU events"
>> depends on MFD_PCF50633
>> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
>> index 9b47971..4570154 100644
>> --- a/drivers/input/misc/Makefile
>> +++ b/drivers/input/misc/Makefile
>> @@ -23,6 +23,7 @@ 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_ISA1200) += isa1200.o
>> obj-$(CONFIG_INPUT_IXP4XX_BEEPER) += ixp4xx-beeper.o
>> obj-$(CONFIG_INPUT_KEYSPAN_REMOTE) += keyspan_remote.o
>> obj-$(CONFIG_INPUT_M68K_BEEP) += m68kspkr.o
>> diff --git a/drivers/input/misc/isa1200.c b/drivers/input/misc/isa1200.c
>> new file mode 100644
>> index 0000000..bce96f8
>> --- /dev/null
>> +++ b/drivers/input/misc/isa1200.c
>> @@ -0,0 +1,440 @@
>> +/*
>> + * Copyright (C) 2009 Samsung Electronics
>> + * Kyungmin Park <[email protected]>
>> + *
>> + * Copyright (c) 2011, Code Aurora Forum. All rights reserved.
>> + *
>> + * 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.
>> + */
>> +
>> +#define pr_fmt(fmt) "%s: " fmt, __func__
>> +
>> +#include <linux/module.h>
>> +#include <linux/init.h>
>> +#include <linux/i2c.h>
>> +#include <linux/gpio.h>
>> +#include <linux/workqueue.h>
>> +#include <linux/delay.h>
>> +#include <linux/pwm.h>
>> +#include <linux/input.h>
>> +#include <linux/slab.h>
>> +#include <linux/pm.h>
>> +#include <linux/input/isa1200.h>
>> +
>> +#define ISA1200_HCTRL0 0x30
>> +#define HCTRL0_MODE_CTRL_BIT (3)
>> +#define HCTRL0_OVERDRIVE_HIGH_BIT (5)
>> +#define HCTRL0_OVERDRIVE_EN_BIT (6)
>> +#define HCTRL0_HAP_EN (7)
>> +#define HCTRL0_RESET 0x01
>> +#define HCTRL1_RESET 0x4B
>> +
>> +#define ISA1200_HCTRL1 0x31
>> +#define HCTRL1_SMART_ENABLE_BIT (3)
>> +#define HCTRL1_ERM_BIT (5)
>> +#define HCTRL1_EXT_CLK_ENABLE_BIT (7)
>> +
>> +#define ISA1200_HCTRL5 0x35
>> +#define HCTRL5_VIB_STRT 0xD5
>> +#define HCTRL5_VIB_STOP 0x6B
>> +
>> +#define DIVIDER_128 (128)
>> +#define DIVIDER_1024 (1024)
>> +#define DIVIDE_SHIFTER_128 (7)
>> +
>> +#define FREQ_22400 (22400)
>> +#define FREQ_172600 (172600)
>> +
>> +#define POR_DELAY_USEC 250
>> +
>> +struct isa1200_chip {
>> + const struct isa1200_platform_data *pdata;
>> + struct i2c_client *client;
>> + struct input_dev *input_device;
>> + struct pwm_device *pwm;
>> + unsigned int period_ns;
>> + unsigned int state;
>> + struct work_struct work;
>> +};
>> +
>> +static void isa1200_vib_set(struct isa1200_chip *haptic, int enable)
>> +{
>> + int rc;
>> +
>> + if (enable) {
>> + if (haptic->pdata->mode_ctrl == PWM_INPUT_MODE) {
>> + rc = pwm_config(haptic->pwm,
>> + (haptic->period_ns * haptic->pdata->duty) / 100,
>> + haptic->period_ns);
>> + if (rc < 0)
>> + pr_err("pwm_config fail\n");
>> + rc = pwm_enable(haptic->pwm);
>> + if (rc < 0)
>> + pr_err("pwm_enable fail\n");
>> + } else if (haptic->pdata->mode_ctrl == PWM_GEN_MODE) {
>> + rc = i2c_smbus_write_byte_data(haptic->client,
>> + ISA1200_HCTRL5,
>> + HCTRL5_VIB_STRT);
>> + if (rc < 0)
>> + pr_err("start vibration fail\n");
>> + }
>> + } else {
>> + if (haptic->pdata->mode_ctrl == PWM_INPUT_MODE)
>> + pwm_disable(haptic->pwm);
>> + else if (haptic->pdata->mode_ctrl == PWM_GEN_MODE) {
>> + rc = i2c_smbus_write_byte_data(haptic->client,
>> + ISA1200_HCTRL5,
>> + HCTRL5_VIB_STOP);
>> + if (rc < 0)
>> + pr_err("stop vibration fail\n");
>> + }
>> + }
>> +}
>> +
>> +static int isa1200_setup(struct i2c_client *client)
>> +{
>> + struct isa1200_chip *haptic = i2c_get_clientdata(client);
>> + int value, temp, rc;
>> +
>> + gpio_set_value_cansleep(haptic->pdata->hap_en_gpio, 0);
>> + udelay(POR_DELAY_USEC);
>> + gpio_set_value_cansleep(haptic->pdata->hap_en_gpio, 1);
>> +
>> + value = (haptic->pdata->smart_en << HCTRL1_SMART_ENABLE_BIT) |
>> + (haptic->pdata->is_erm << HCTRL1_ERM_BIT) |
>> + (haptic->pdata->ext_clk_en << HCTRL1_EXT_CLK_ENABLE_BIT);
>> +
>> + rc = i2c_smbus_write_byte_data(client, ISA1200_HCTRL1, value);
>> + if (rc < 0) {
>> + pr_err("i2c write failure\n");
>> + return rc;
>> + }
>> +
>> + if (haptic->pdata->mode_ctrl == PWM_GEN_MODE) {
>> + temp = haptic->pdata->pwm_fd.pwm_div;
>> + if (temp < DIVIDER_128 || temp > DIVIDER_1024 ||
>> + temp % DIVIDER_128) {
>> + pr_err("Invalid divider\n");
> Not "divisor" by any chance?
>
>> + rc = -EINVAL;
>> + goto reset_hctrl1;
>> + }
>> + value = ((temp >> DIVIDE_SHIFTER_128) - 1);
>> + } else if (haptic->pdata->mode_ctrl == PWM_INPUT_MODE) {
>> + temp = haptic->pdata->pwm_fd.pwm_freq;
>> + if (temp < FREQ_22400 || temp > FREQ_172600 ||
>> + temp % FREQ_22400) {
>> + pr_err("Invalid frequency\n");
>> + rc = -EINVAL;
>> + goto reset_hctrl1;
>> + }
>> + value = ((temp / FREQ_22400) - 1);
>> + haptic->period_ns = NSEC_PER_SEC / temp;
>> + }
>> + value |= (haptic->pdata->mode_ctrl << HCTRL0_MODE_CTRL_BIT) |
>> + (haptic->pdata->overdrive_high << HCTRL0_OVERDRIVE_HIGH_BIT) |
>> + (haptic->pdata->overdrive_en << HCTRL0_OVERDRIVE_EN_BIT) |
>> + (haptic->pdata->chip_en << HCTRL0_HAP_EN);
>> +
>> + rc = i2c_smbus_write_byte_data(client, ISA1200_HCTRL0, value);
>> + if (rc < 0) {
>> + pr_err("i2c write failure\n");
>> + goto reset_hctrl1;
>> + }
>> +
>> + return 0;
>> +
>> +reset_hctrl1:
>> + i2c_smbus_write_byte_data(client, ISA1200_HCTRL1,
>> + HCTRL1_RESET);
>> + return rc;
>> +}
>> +
>> +static void isa1200_worker(struct work_struct *work)
>> +{
>> + struct isa1200_chip *haptic;
>> +
>> + haptic = container_of(work, struct isa1200_chip, work);
>> + isa1200_vib_set(haptic, !!haptic->state);
>> +}
>> +
>> +static int isa1200_play_effect(struct input_dev *dev, void *data,
>> + struct ff_effect *effect)
>> +{
>> + struct isa1200_chip *haptic = input_get_drvdata(dev);
>> +
>> + /* support basic vibration */
>> + haptic->state = effect->u.rumble.strong_magnitude >> 8;
>> + if (!haptic->state)
>> + haptic->state = effect->u.rumble.weak_magnitude >> 9;
>> +
>> + schedule_work(&haptic->work);
>> + return 0;
>> +}
>> +
>> +#ifdef CONFIG_PM
> Protect with CONFIG_PM_SLEEP, it will save us a warning if only
> CONFIG_PM_RUNTIME is enabled.
>
>> +static int isa1200_suspend(struct device *dev)
>> +{
>> + struct isa1200_chip *haptic = dev_get_drvdata(dev);
>> + int rc;
>> +
>> + cancel_work_sync(&haptic->work);
>> + /* turn-off current vibration */
>> + isa1200_vib_set(haptic, 0);
>> +
>> + if (haptic->pdata->power_on) {
>> + rc = haptic->pdata->power_on(0);
> ->power_on(false)?
>
> Everywhere, when arguments are bool please take care to supply boolena
> constants.
>
>> + if (rc) {
>> + pr_err("power-down failed\n");
>> + return rc;
>> + }
>> + }
>> + return 0;
>> +}
>> +
>> +static int isa1200_resume(struct device *dev)
>> +{
>> + struct isa1200_chip *haptic = dev_get_drvdata(dev);
>> + int rc;
>> +
>> + if (haptic->pdata->power_on) {
>> + rc = haptic->pdata->power_on(1);
>> + if (rc) {
>> + pr_err("power-up failed\n");
>> + return rc;
>> + }
>> + }
>> +
>> + isa1200_setup(haptic->client);
>> + return 0;
>> +}
>> +#else
>> +#define isa1200_suspend NULL
>> +#define isa1200_resume NULL
>> +#endif
>> +
>> +static int isa1200_open(struct input_dev *dev)
>> +{
>> + struct isa1200_chip *haptic = input_get_drvdata(dev);
>> + int rc;
>> +
>> + /* device setup */
>> + if (haptic->pdata->dev_setup) {
>> + rc = haptic->pdata->dev_setup(true);
>> + if (rc < 0) {
>> + pr_err("setup failed!\n");
>> + return rc;
>> + }
>> + }
>> +
>> + /* power on */
>> + if (haptic->pdata->power_on) {
>> + rc = haptic->pdata->power_on(true);
>> + if (rc < 0) {
>> + pr_err("power failed\n");
>> + goto err_setup;
>> + }
>> + }
>> +
>> + /* request gpio */
>> + rc = gpio_is_valid(haptic->pdata->hap_en_gpio);
>> + if (rc) {
>> + rc = gpio_request(haptic->pdata->hap_en_gpio, "haptic_gpio");
>> + if (rc) {
>> + pr_err("gpio %d request failed\n",
>> + haptic->pdata->hap_en_gpio);
>> + goto err_power_on;
>> + }
>> + } else {
>> + pr_err("Invalid gpio %d\n",
>> + haptic->pdata->hap_en_gpio);
>> + goto err_power_on;
>> + }
>> +
>> + rc = gpio_direction_output(haptic->pdata->hap_en_gpio, 0);
>> + if (rc) {
>> + pr_err("gpio %d set direction failed\n",
>> + haptic->pdata->hap_en_gpio);
>> + goto err_gpio_free;
>> + }
>> +
>> + /* setup registers */
>> + rc = isa1200_setup(haptic->client);
>> + if (rc < 0) {
>> + pr_err("setup fail %d\n", rc);
>> + goto err_gpio_free;
>> + }
>> +
>> + if (haptic->pdata->mode_ctrl == PWM_INPUT_MODE) {
>> + haptic->pwm = pwm_request(haptic->pdata->pwm_ch_id,
>> + haptic->client->driver->id_table->name);
>> + if (IS_ERR(haptic->pwm)) {
>> + pr_err("pwm request failed\n");
>> + rc = PTR_ERR(haptic->pwm);
>> + goto err_reset_hctrl0;
>> + }
>> + }
>> +
>> + /* init workqeueue */
>> + INIT_WORK(&haptic->work, isa1200_worker);
> Why here and not in probe?
>
> Actually, all resource acquisition (gpio_request, pwm_request, and so
> forth) should be done in probe(). open() is supposed to bring device
> from low power mode and configure the chip itself.
>
>> + return 0;
>> +
>> +err_reset_hctrl0:
>> + i2c_smbus_write_byte_data(haptic->client, ISA1200_HCTRL0,
>> + HCTRL0_RESET);
>> +err_gpio_free:
>> + gpio_free(haptic->pdata->hap_en_gpio);
>> +err_power_on:
>> + if (haptic->pdata->power_on)
>> + haptic->pdata->power_on(0);
>> +err_setup:
>> + if (haptic->pdata->dev_setup)
>> + haptic->pdata->dev_setup(false);
>> +
>> + return rc;
>> +}
>> +
>> +static void isa1200_close(struct input_dev *dev)
>> +{
>> + struct isa1200_chip *haptic = input_get_drvdata(dev);
>> +
>> + /* turn-off current vibration */
>> + isa1200_vib_set(haptic, 0);
>> +
> Somewhere here should be call to cancel_work_sync() to ensure that play
> work does not outlive us.
>
>> + if (haptic->pdata->mode_ctrl == PWM_INPUT_MODE)
>> + pwm_free(haptic->pwm);
>> +
>> + gpio_free(haptic->pdata->hap_en_gpio);
>> +
>> + /* reset hardware registers */
>> + i2c_smbus_write_byte_data(haptic->client, ISA1200_HCTRL0,
>> + HCTRL0_RESET);
>> + i2c_smbus_write_byte_data(haptic->client, ISA1200_HCTRL1,
>> + HCTRL1_RESET);
>> +
>> + if (haptic->pdata->dev_setup)
>> + haptic->pdata->dev_setup(false);
>> +
>> + /* power-off the chip */
>> + if (haptic->pdata->power_on)
>> + haptic->pdata->power_on(0);
>> +}
>> +
>> +static int __devinit isa1200_probe(struct i2c_client *client,
>> + const struct i2c_device_id *id)
>> +{
>> + struct isa1200_chip *haptic;
>> + int rc;
>> +
>> + if (!i2c_check_functionality(client->adapter,
>> + I2C_FUNC_SMBUS_BYTE_DATA)) {
>> + pr_err("i2c is not supported\n");
>> + return -EIO;
>> + }
>> +
>> + if (!client->dev.platform_data) {
>> + pr_err("pdata is not avaiable\n");
>> + return -EINVAL;
>> + }
>> +
>> + haptic = kzalloc(sizeof(struct isa1200_chip), GFP_KERNEL);
>> + if (!haptic) {
>> + pr_err("no memory\n");
>> + return -ENOMEM;
>> + }
>> +
>> + haptic->pdata = client->dev.platform_data;
>> + haptic->client = client;
>> +
>> + i2c_set_clientdata(client, haptic);
>> +
>> + haptic->input_device = input_allocate_device();
>> + if (!haptic->input_device) {
>> + pr_err("input device alloc failed\n");
>> + rc = -ENOMEM;
>> + goto err_mem_alloc;
>> + }
>> +
>> + input_set_drvdata(haptic->input_device, haptic);
>> + haptic->input_device->name = haptic->pdata->name ? :
>> + "isa1200-ff-memless";
> "isa1200-ff-memless" is quite ugly, why don't you give a nicer name to
> the device? "ISA1200 Haptic device" or something.
>
>> +
>> + haptic->input_device->dev.parent = &client->dev;
> input_dev->id.bustype = BUS_I2C;
>
>> +
>> + input_set_capability(haptic->input_device, EV_FF, FF_RUMBLE);
>> +
>> + haptic->input_device->open = isa1200_open;
>> + haptic->input_device->close = isa1200_close;
>> +
>> + rc = input_ff_create_memless(haptic->input_device, NULL,
>> + isa1200_play_effect);
>> + if (rc < 0) {
>> + pr_err("unable to register with ff\n");
>> + goto err_free_dev;
>> + }
>> +
>> + rc = input_register_device(haptic->input_device);
>> + if (rc < 0) {
>> + pr_err("unable to register input device\n");
>> + goto err_ff_destroy;
>> + }
>> + return 0;
>> +
>> +err_ff_destroy:
>> + input_ff_destroy(haptic->input_device);
>> +err_free_dev:
>> + input_free_device(haptic->input_device);
>> +err_mem_alloc:
>> + kfree(haptic);
>> + return rc;
>> +}
>> +
>> +static int __devexit isa1200_remove(struct i2c_client *client)
>> +{
>> + struct isa1200_chip *haptic = i2c_get_clientdata(client);
>> +
>> + input_unregister_device(haptic->input_device);
>> + kfree(haptic);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct i2c_device_id isa1200_id_table[] = {
>> + {"isa1200", 0},
>> + { },
>> +};
>> +MODULE_DEVICE_TABLE(i2c, isa1200_id_table);
>> +
>> +static const struct dev_pm_ops isa1200_pm_ops = {
>> + .suspend = isa1200_suspend,
>> + .resume = isa1200_resume,
>> +};
> static SIMPLE_DEV_PM_OPS(isa1200_pm_ops, isa1200_suspend, isa1200_resume);
>
>> +
>> +static struct i2c_driver isa1200_driver = {
>> + .driver = {
>> + .name = "isa1200",
>> + .owner = THIS_MODULE,
>> + .pm = &isa1200_pm_ops,
>> + },
>> + .probe = isa1200_probe,
>> + .remove = __devexit_p(isa1200_remove),
>> + .id_table = isa1200_id_table,
>> +};
>> +
>> +static int __init isa1200_init(void)
>> +{
>> + return i2c_add_driver(&isa1200_driver);
>> +}
>> +module_init(isa1200_init);
>> +
>> +static void __exit isa1200_exit(void)
>> +{
>> + i2c_del_driver(&isa1200_driver);
>> +}
>> +module_exit(isa1200_exit);
>> +
>> +MODULE_DESCRIPTION("isa1200 based vibrator chip driver");
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_AUTHOR("Kyungmin Park <[email protected]>");
>> +MODULE_AUTHOR("Mohan Pallaka <[email protected]>");
>> diff --git a/include/linux/input/isa1200.h b/include/linux/input/isa1200.h
>> new file mode 100644
>> index 0000000..28c18b7
>> --- /dev/null
>> +++ b/include/linux/input/isa1200.h
>> @@ -0,0 +1,45 @@
>> +/*
>> + * Copyright (C) 2009 Samsung Electronics
>> + * Kyungmin Park <[email protected]>
>> + *
>> + * Copyright (c) 2011, Code Aurora Forum. All rights reserved.
>> + *
>> + * 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.
>> + */
>> +
>> +#ifndef __LINUX_ISA1200_H
>> +#define __LINUX_ISA1200_H
>> +
>> +enum mode_control {
>> + POWER_DOWN_MODE = 0,
>> + PWM_INPUT_MODE,
>> + PWM_GEN_MODE,
>> + WAVE_GEN_MODE
>> +};
>> +
>> +union pwm_div_freq {
>> + unsigned int pwm_div; /* PWM gen mode */
>> + unsigned int pwm_freq; /* PWM input mode */
>> +};
>> +
>> +struct isa1200_platform_data {
>> + const char *name;
>> + unsigned int pwm_ch_id; /* pwm channel id */
>> + unsigned int max_timeout;
>> + unsigned int hap_en_gpio;
>> + bool overdrive_high; /* high/low overdrive */
>> + bool overdrive_en; /* enable/disable overdrive */
>> + enum mode_control mode_ctrl; /* input/generation/wave */
>> + union pwm_div_freq pwm_fd;
>> + bool smart_en; /* smart mode enable/disable */
>> + bool is_erm;
>> + bool ext_clk_en;
>> + unsigned int chip_en;
>> + unsigned int duty;
>> + int (*power_on)(int on);
> bool on?
>
>> + int (*dev_setup)(bool on);
>> +};
>> +
>> +#endif /* __LINUX_ISA1200_H */
>>
> Thanks.
>
Thanks, Mohan.

--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.