From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751760AbdAPHZs (ORCPT ); Mon, 16 Jan 2017 02:25:48 -0500 Received: from mailout3.samsung.com ([203.254.224.33]:38309 "EHLO mailout3.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751727AbdAPHZk (ORCPT ); Mon, 16 Jan 2017 02:25:40 -0500 X-AuditID: b6c32a3d-f793f6d000001fd5-48-587c7540db9a Date: Mon, 16 Jan 2017 16:24:47 +0900 From: Jaechul Lee To: Dmitry Torokhov Cc: Rob Herring , Mark Rutland , Catalin Marinas , Will Deacon , Kukjin Kim , Krzysztof Kozlowski , Javier Martinez Canillas , Andi Shyti , Chanwoo Choi , beomho.seo@samsung.com, galaxyra@gmail.com, linux-arm-kernel@lists.infradead.org, linux-input@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-samsung-soc@vger.kernel.org Subject: Re: [PATCH v6 2/3] input: tm2-touchkey: Add touchkey driver support for TM2 Message-id: <20170116072447.GA15619@jcsing> MIME-version: 1.0 Content-type: text/plain; charset=us-ascii Content-disposition: inline In-reply-to: <20170115071110.GA24007@dtor-ws> User-Agent: Mutt/1.5.24 (2015-08-30) X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrMJsWRmVeSWpSXmKPExsWy7bCmga5DaU2Ewf9HOhbbjzxjtTj9aRu7 xftlPYwW1788Z7WYf+Qcq8XhRS8YLY6vnMVs8ebtGiaL/sevmS3On9/AbrHp8TVWi5ufvrFa XN41h81ixvl9TBZLr19ksmjde4Td4uXHEywOgh5r5q1h9Ng56y67x6ZVnWwem5fUe2zpB/L6 tqxi9Pi8SS6APSrVJiM1MSW1SCE1Lzk/JTMv3VbJOzjeOd7UzMBQ19DSwlxJIS8xN9VWycUn QNctMwfoAyWFssScUqBQQGJxsZK+nU1RfmlJqkJGfnGJrVK0oaGRnqGBuZ6RkZGeiXGslZEp UElCasb/j+dYCv6lVjTP28fawPgvqIuRk0NCwERiR2cnK4QtJnHh3nq2LkYuDiGBHYwST65u YoJw2pkkVp86xA7TMXfdCqiq5YwSvXNeQTkvGCVef//NCFLFIqAqcapzMthcNgEtif4lPWwg toiAvsT22b8YQRqYBbazSFyd8ZcJJCEsECZx4XcTSxcjBwevgLbE+2VZIGFeAUGJH5PvsYDY zAI6EmePrWOEsKUlHv2dAXYRp4CuxIxll9hBWkUFVCReHawHGS8h0Mgh8eV8E1hcQkBWYtMB ZogHXCR23l7HBmELS7w6vgXqMWmJVf9uMUH0djNKNB/dCeV0MErsu7IAqsNY4v6De8wQR/BJ vPvawwqxgFeio00IwvSQeP6ZCaLaUaJ/11QWSPj8YJR4vrCJeQKj/Cwkr81C8tosJK8tYGRe xSiWWlCcm55abFhgqVecmFtcmpeul5yfu4kRnIC1bHcwfjnnc4hRgINRiYd3wY7qCCHWxLLi ytxDjBIczEoivH9yayKEeFMSK6tSi/Lji0pzUosPMZoCo2cis5Rocj4wO+SVxBuamBmaGJkY GpobGRgpifMua7SOEBJITyxJzU5NLUgtgulj4uCUamCc89xxmbxo063LN20DTQ9utJc6Huru 31EgfFfz7pY1Nis3GuZVTLy716a9UNFhfXGGquMW090l5udex3arprLeUDfR5JmT276uds/m EzPccnMT4kyZN7fEP/uV2qotXeRn5jZtgr/VjXNmFqeFrO4fte1PbfPdveLZVZ1lFvrXZUwF bD5p/FViKc5INNRiLipOBAAZZlWf1gMAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrAIsWRmVeSWpSXmKPExsVy+t9jAV2H0poIgwnfJC22H3nGanH60zZ2 i/fLehgtrn95zmox/8g5VovDi14wWhxfOYvZ4s3bNUwW/Y9fM1ucP7+B3WLT42usFjc/fWO1 uLxrDpvFjPP7mCyWXr/IZNG69wi7xcuPJ1gcBD3WzFvD6LFz1l12j02rOtk8Ni+p99jSD+T1 bVnF6PF5k1wAe5SbTUZqYkpqkUJqXnJ+SmZeuq1SaIibroWSQl5ibqqtUoSub0iQkkJZYk4p kGdkgAYcnAPcg5X07RLcMv5/PMdS8C+1onnePtYGxn9BXYycHBICJhJz161gg7DFJC7cWw9k c3EICSxllLja08MK4bxglNhxbRYrSBWLgKrEqc7JYDabgJZE/5IesG4RAX2J7bN/MYI0MAts Z5HY+G0yE0hCWCBM4sLvJpYuRg4OXgFtiffLsiCG/mCUaFz2lAWkhldAUOLH5HtgNjPQ0PU7 jzNB2NISj/7OYAexOQV0JWYsu8QOMkdUQEXi1cH6CYwCs5B0z0LSPQtJ9wJG5lWMEqkFyQXF Sem5hnmp5XrFibnFpXnpesn5uZsYwbH9TGoH48Fd7ocYBTgYlXh4F+yojhBiTSwrrsw9xCjB wawkwvsntyZCiDclsbIqtSg/vqg0J7X4EKMpMEAmMkuJJucD005eSbyhibmJubGBhbmlpYmR kjhv4+xn4UIC6YklqdmpqQWpRTB9TBycUg2M2nXZB+cJhaurxxjzsrA0TfM63fbCZSPDwSv+ 29Y4FXdsfLQ6U35/4slFx1j2XI9otmGbErZqlpbHmcu780vnuu+W0wlLmuTA1Hzl7h+PLLP8 BhNOm6zbL2TX2jVb9mz13L8rNqVZvm2B78ufhitL2k/cOdMbdrKP492J/JJNQr+P5u3bq+mm xFKckWioxVxUnAgAVlR92gMDAAA= X-MTR: 20000000000000000@CPGS X-CMS-MailID: 20170116072448epcas1p1ab2ce6dfe5253eff9f02c4876a5dd5e3 X-Msg-Generator: CA X-Sender-IP: 203.254.230.26 X-Local-Sender: =?UTF-8?B?7J207J6s7LKgG1RpemVuIFBsYXRmb3JtIExhYihTL1fshLw=?= =?UTF-8?B?7YSwKRvsgrzshLHsoITsnpAbUzUo7LGF7J6EKS/ssYXsnoQ=?= X-Global-Sender: =?UTF-8?B?SmFlQ2h1bCBMZWUbVGl6ZW4gUGxhdGZvcm0gTGFiLhtTYW1z?= =?UTF-8?B?dW5nIEVsZWN0cm9uaWNzG1NlbmlvciBFbmdpbmVlcg==?= X-Sender-Code: =?UTF-8?B?QzEwG1NUQUYbQzEwVjgxMTE=?= CMS-TYPE: 101P DLP-Filter: Pass X-CFilter-Loop: Reflected X-HopCount: 7 X-CMS-RootMailID: 20170109072218epcas1p1f95be49f7f466712c146d1e9163601e1 X-RootMTR: 20170109072218epcas1p1f95be49f7f466712c146d1e9163601e1 References: <1483946535-4703-1-git-send-email-jcsing.lee@samsung.com> <1483946535-4703-3-git-send-email-jcsing.lee@samsung.com> <20170115071110.GA24007@dtor-ws> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Dear Dmitry Torokhov, On Sat, Jan 14, 2017 at 11:11:10PM -0800, Dmitry Torokhov wrote: > Hi Jaechul, > > On Mon, Jan 09, 2017 at 04:22:14PM +0900, Jaechul Lee wrote: > > +static irqreturn_t tm2_touchkey_irq_handler(int irq, void *devid) > > +{ > > + struct tm2_touchkey_data *touchkey = devid; > > + u32 data; > > + > > + data = i2c_smbus_read_byte_data(touchkey->client, > > + TM2_TOUCHKEY_KEYCODE_REG); > > + > > + if (data < 0) { > > You declared data as u32 so it will never be negative. Yes, it won't be negative. > > > + dev_err(&touchkey->client->dev, "Failed to read i2c data\n"); > > + return IRQ_HANDLED; > > + } > > + > > + touchkey->keycode_type = data & TM2_TOUCHKEY_BIT_KEYCODE; > > + touchkey->pressed = !(data & TM2_TOUCHKEY_BIT_PRESS_EV); > > There is no need to store this in touchkey structure as you are not > going to use it past this function. I agree with you. it doesn't need to store variables in touchkey structure. > > Does the version of the patch below work for you? I found that the condition is inverted. if data & TM2_TOUCHKEY_BIT_PRESS_EV is true, it means touchkey is released. it should be changed like this. if (data & TM2_TOUCHKEY_BIT_PRESS_EV) { input_report_key(touchkey->input_dev, KEY_PHONE, 0); input_report_key(touchkey->input_dev, KEY_BACK, 0); } else { input_report_key(touchkey->input_dev, key, 1); } I will prepare for patch v7 based on your modifications. Thank you very much for your reviews. Best Regards, Jaechul > > Thanks. > > -- > Dmitry > > > Input: tm2-touchkey - add touchkey driver support for TM2 > > From: Jaechul Lee > > This patch adds support for the TM2 touch key and led functionality. > > The driver interfaces with userspace through an input device and > reports KEY_PHONE and KEY_BACK event types. LED brightness can be > controlled by "/sys/class/leds/tm2-touchkey/brightness". > > Signed-off-by: Beomho Seo > Signed-off-by: Jaechul Lee > Reviewed-by: Javier Martinez Canillas > Reviewed-by: Andi Shyti > Acked-by: Krzysztof Kozlowski > Patchwork-Id: 9504149 > Signed-off-by: Dmitry Torokhov > --- > drivers/input/keyboard/Kconfig | 11 + > drivers/input/keyboard/Makefile | 1 > drivers/input/keyboard/tm2-touchkey.c | 286 +++++++++++++++++++++++++++++++++ > 3 files changed, 298 insertions(+) > create mode 100644 drivers/input/keyboard/tm2-touchkey.c > > diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig > index cbd75cf44739..97acd6524ad7 100644 > --- a/drivers/input/keyboard/Kconfig > +++ b/drivers/input/keyboard/Kconfig > @@ -666,6 +666,17 @@ config KEYBOARD_TC3589X > To compile this driver as a module, choose M here: the > module will be called tc3589x-keypad. > > +config KEYBOARD_TM2_TOUCHKEY > + tristate "TM2 touchkey support" > + depends on I2C > + depends on LEDS_CLASS > + help > + Say Y here to enable device driver for tm2-touchkey with > + LED control for the Exynos5433 TM2 board. > + > + To compile this driver as a module, choose M here. > + module will be called tm2-touchkey. > + > config KEYBOARD_TWL4030 > tristate "TI TWL4030/TWL5030/TPS659x0 keypad support" > depends on TWL4030_CORE > diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile > index d9f4cfcf3410..7d9acff819a7 100644 > --- a/drivers/input/keyboard/Makefile > +++ b/drivers/input/keyboard/Makefile > @@ -61,6 +61,7 @@ obj-$(CONFIG_KEYBOARD_SUN4I_LRADC) += sun4i-lradc-keys.o > obj-$(CONFIG_KEYBOARD_SUNKBD) += sunkbd.o > obj-$(CONFIG_KEYBOARD_TC3589X) += tc3589x-keypad.o > obj-$(CONFIG_KEYBOARD_TEGRA) += tegra-kbc.o > +obj-$(CONFIG_KEYBOARD_TM2_TOUCHKEY) += tm2-touchkey.o > obj-$(CONFIG_KEYBOARD_TWL4030) += twl4030_keypad.o > obj-$(CONFIG_KEYBOARD_XTKBD) += xtkbd.o > obj-$(CONFIG_KEYBOARD_W90P910) += w90p910_keypad.o > diff --git a/drivers/input/keyboard/tm2-touchkey.c b/drivers/input/keyboard/tm2-touchkey.c > new file mode 100644 > index 000000000000..79bc2d2bd4b9 > --- /dev/null > +++ b/drivers/input/keyboard/tm2-touchkey.c > @@ -0,0 +1,286 @@ > +/* > + * TM2 touchkey device driver > + * > + * Copyright 2005 Phil Blundell > + * Copyright 2016 Samsung Electronics Co., Ltd. > + * > + * Author: Beomho Seo > + * Author: Jaechul Lee > + * > + * 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. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define TM2_TOUCHKEY_DEV_NAME "tm2-touchkey" > +#define TM2_TOUCHKEY_KEYCODE_REG 0x03 > +#define TM2_TOUCHKEY_BASE_REG 0x00 > +#define TM2_TOUCHKEY_CMD_LED_ON 0x10 > +#define TM2_TOUCHKEY_CMD_LED_OFF 0x20 > +#define TM2_TOUCHKEY_BIT_PRESS_EV BIT(3) > +#define TM2_TOUCHKEY_BIT_KEYCODE GENMASK(2, 0) > +#define TM2_TOUCHKEY_LED_VOLTAGE_MIN 2500000 > +#define TM2_TOUCHKEY_LED_VOLTAGE_MAX 3300000 > + > +enum { > + TM2_TOUCHKEY_KEY_MENU = 0x1, > + TM2_TOUCHKEY_KEY_BACK, > +}; > + > +struct tm2_touchkey_data { > + struct i2c_client *client; > + struct input_dev *input_dev; > + struct led_classdev led_dev; > + struct regulator *vdd; > + struct regulator_bulk_data regulators[2]; > +}; > + > +static void tm2_touchkey_led_brightness_set(struct led_classdev *led_dev, > + enum led_brightness brightness) > +{ > + struct tm2_touchkey_data *touchkey = > + container_of(led_dev, struct tm2_touchkey_data, led_dev); > + u32 volt; > + u8 data; > + > + if (brightness == LED_OFF) { > + volt = TM2_TOUCHKEY_LED_VOLTAGE_MIN; > + data = TM2_TOUCHKEY_CMD_LED_OFF; > + } else { > + volt = TM2_TOUCHKEY_LED_VOLTAGE_MAX; > + data = TM2_TOUCHKEY_CMD_LED_ON; > + } > + > + regulator_set_voltage(touchkey->vdd, volt, volt); > + i2c_smbus_write_byte_data(touchkey->client, > + TM2_TOUCHKEY_BASE_REG, data); > +} > + > +static int tm2_touchkey_power_enable(struct tm2_touchkey_data *touchkey) > +{ > + int error; > + > + error = regulator_bulk_enable(ARRAY_SIZE(touchkey->regulators), > + touchkey->regulators); > + if (error) > + return error; > + > + /* waiting for device initialization, at least 150ms */ > + msleep(150); > + > + return 0; > +} > + > +static void tm2_touchkey_power_disable(void *data) > +{ > + struct tm2_touchkey_data *touchkey = data; > + > + regulator_bulk_disable(ARRAY_SIZE(touchkey->regulators), > + touchkey->regulators); > +} > + > +static irqreturn_t tm2_touchkey_irq_handler(int irq, void *devid) > +{ > + struct tm2_touchkey_data *touchkey = devid; > + int data; > + int key; > + > + data = i2c_smbus_read_byte_data(touchkey->client, > + TM2_TOUCHKEY_KEYCODE_REG); > + if (data < 0) { > + dev_err(&touchkey->client->dev, > + "failed to read i2c data: %d\n", data); > + goto out; > + } > + > + switch (data & TM2_TOUCHKEY_BIT_KEYCODE) { > + case TM2_TOUCHKEY_KEY_MENU: > + key = KEY_PHONE; > + break; > + > + case TM2_TOUCHKEY_KEY_BACK: > + key = KEY_BACK; > + break; > + > + default: > + dev_warn(&touchkey->client->dev, > + "unhandled keycode, data %#02x\n", data); > + goto out; > + } > + > + if (data & TM2_TOUCHKEY_BIT_PRESS_EV) { > + input_report_key(touchkey->input_dev, key, 1); > + } else { > + input_report_key(touchkey->input_dev, KEY_PHONE, 0); > + input_report_key(touchkey->input_dev, KEY_BACK, 0); > + } > + > + input_sync(touchkey->input_dev); > + > +out: > + return IRQ_HANDLED; > +} > + > +static int tm2_touchkey_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + struct tm2_touchkey_data *touchkey; > + int error; > + > + if (!i2c_check_functionality(client->adapter, > + I2C_FUNC_SMBUS_BYTE | I2C_FUNC_SMBUS_BYTE_DATA)) { > + dev_err(&client->dev, "incompatible I2C adapter\n"); > + return -EIO; > + } > + > + touchkey = devm_kzalloc(&client->dev, sizeof(*touchkey), GFP_KERNEL); > + if (!touchkey) > + return -ENOMEM; > + > + touchkey->client = client; > + i2c_set_clientdata(client, touchkey); > + > + touchkey->regulators[0].supply = "vcc"; > + touchkey->regulators[1].supply = "vdd"; > + error = devm_regulator_bulk_get(&client->dev, > + ARRAY_SIZE(touchkey->regulators), > + touchkey->regulators); > + if (error) { > + dev_err(&client->dev, "failed to get regulators: %d\n", error); > + return error; > + } > + > + /* Save VDD for easy access */ > + touchkey->vdd = touchkey->regulators[1].consumer; > + > + error = tm2_touchkey_power_enable(touchkey); > + if (error) { > + dev_err(&client->dev, "failed to power up device: %d\n", error); > + return error; > + } > + > + error = devm_add_action_or_reset(&client->dev, > + tm2_touchkey_power_disable, touchkey); > + if (error) { > + dev_err(&client->dev, > + "failed to install poweroff handler: %d\n", error); > + return error; > + } > + > + /* input device */ > + touchkey->input_dev = devm_input_allocate_device(&client->dev); > + if (!touchkey->input_dev) { > + dev_err(&client->dev, "failed to allocate input device\n"); > + return -ENOMEM; > + } > + > + touchkey->input_dev->name = TM2_TOUCHKEY_DEV_NAME; > + touchkey->input_dev->id.bustype = BUS_I2C; > + > + input_set_capability(touchkey->input_dev, EV_KEY, KEY_PHONE); > + input_set_capability(touchkey->input_dev, EV_KEY, KEY_BACK); > + > + input_set_drvdata(touchkey->input_dev, touchkey); > + > + error = input_register_device(touchkey->input_dev); > + if (error) { > + dev_err(&client->dev, > + "failed to register input device: %d\n", error); > + return error; > + } > + > + error = devm_request_threaded_irq(&client->dev, client->irq, > + NULL, tm2_touchkey_irq_handler, > + IRQF_ONESHOT, > + TM2_TOUCHKEY_DEV_NAME, touchkey); > + if (error) { > + dev_err(&client->dev, > + "failed to request threaded irq: %d\n", error); > + return error; > + } > + > + /* led device */ > + touchkey->led_dev.name = TM2_TOUCHKEY_DEV_NAME; > + touchkey->led_dev.brightness = LED_FULL; > + touchkey->led_dev.max_brightness = LED_FULL; > + touchkey->led_dev.brightness_set = tm2_touchkey_led_brightness_set; > + > + error = devm_led_classdev_register(&client->dev, &touchkey->led_dev); > + if (error) { > + dev_err(&client->dev, > + "failed to register touchkey led: %d\n", error); > + return error; > + } > + > + return 0; > +} > + > +static int __maybe_unused tm2_touchkey_suspend(struct device *dev) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct tm2_touchkey_data *touchkey = i2c_get_clientdata(client); > + > + disable_irq(client->irq); > + tm2_touchkey_power_disable(touchkey); > + > + return 0; > +} > + > +static int __maybe_unused tm2_touchkey_resume(struct device *dev) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct tm2_touchkey_data *touchkey = i2c_get_clientdata(client); > + int ret; > + > + enable_irq(client->irq); > + > + ret = tm2_touchkey_power_enable(touchkey); > + if (ret) > + dev_err(dev, "failed to enable power: %d\n", ret); > + > + return ret; > +} > + > +static SIMPLE_DEV_PM_OPS(tm2_touchkey_pm_ops, > + tm2_touchkey_suspend, tm2_touchkey_resume); > + > +static const struct i2c_device_id tm2_touchkey_id_table[] = { > + { TM2_TOUCHKEY_DEV_NAME, 0 }, > + { }, > +}; > +MODULE_DEVICE_TABLE(i2c, tm2_touchkey_id_table); > + > +static const struct of_device_id tm2_touchkey_of_match[] = { > + { .compatible = "cypress,tm2-touchkey", }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, tm2_touchkey_of_match); > + > +static struct i2c_driver tm2_touchkey_driver = { > + .driver = { > + .name = TM2_TOUCHKEY_DEV_NAME, > + .pm = &tm2_touchkey_pm_ops, > + .of_match_table = of_match_ptr(tm2_touchkey_of_match), > + }, > + .probe = tm2_touchkey_probe, > + .id_table = tm2_touchkey_id_table, > +}; > +module_i2c_driver(tm2_touchkey_driver); > + > +MODULE_AUTHOR("Beomho Seo "); > +MODULE_AUTHOR("Jaechul Lee "); > +MODULE_DESCRIPTION("Samsung touchkey driver"); > +MODULE_LICENSE("GPL v2"); > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html