Hi Yassine, On Wed, Sep 01, 2021 at 04:52:58PM +0000, Yassine Oudjana wrote: > This adds support for Cypress StreetFighter touchkey controllers such as sf3155. > This driver supports managing regulators and generating input events. > > Due to lack of documentation, this driver is entirely based on information > gathered from a driver written for an old Android kernel fork[1][2]. > > Signed-off-by: Yassine Oudjana <y.oudjana@xxxxxxxxxxxxxx> Thank you for the patch. Please run your patches through checkpatch.pl. See additional comments below. > > [1] https://github.com/LineageOS/android_kernel_xiaomi_msm8996/blob/lineage-18.1/drivers/input/touchscreen/cyttsp_button.c > [2] https://github.com/LineageOS/android_kernel_xiaomi_msm8996/blob/lineage-18.1/arch/arm/boot/dts/qcom/a4-msm8996-mtp.dtsi#L291-L314 > --- > Changes since v2: > - Code style fixes. > - Added copyright statement. > Changes since v1: > - Changed version variables in probe to int to allow storing error codes. > > drivers/input/keyboard/Kconfig | 10 ++ > drivers/input/keyboard/Makefile | 1 + > drivers/input/keyboard/cypress-sf.c | 223 ++++++++++++++++++++++++++++ > 3 files changed, 234 insertions(+) > create mode 100644 drivers/input/keyboard/cypress-sf.c > > diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig > index 40a070a2e7f5..6f3fbea8b803 100644 > --- a/drivers/input/keyboard/Kconfig > +++ b/drivers/input/keyboard/Kconfig > @@ -788,4 +788,14 @@ config KEYBOARD_MTK_PMIC > To compile this driver as a module, choose M here: the > module will be called pmic-keys. > > +config KEYBOARD_CYPRESS_SF > + tristate "Cypress StreetFighter touchkey support" > + depends on I2C > + help > + Say Y here if you want to enable support for Cypress StreetFighter > + touchkeys. > + > + To compile this driver as a module, choose M here: the > + module will be called cypress-sf. > + > endif > diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile > index 1d689fdd5c00..e3c8648f834e 100644 > --- a/drivers/input/keyboard/Makefile > +++ b/drivers/input/keyboard/Makefile > @@ -17,6 +17,7 @@ obj-$(CONFIG_KEYBOARD_BCM) += bcm-keypad.o > obj-$(CONFIG_KEYBOARD_CAP11XX) += cap11xx.o > obj-$(CONFIG_KEYBOARD_CLPS711X) += clps711x-keypad.o > obj-$(CONFIG_KEYBOARD_CROS_EC) += cros_ec_keyb.o > +obj-$(CONFIG_KEYBOARD_CYPRESS_SF) += cypress-sf.o > obj-$(CONFIG_KEYBOARD_DAVINCI) += davinci_keyscan.o > obj-$(CONFIG_KEYBOARD_DLINK_DIR685) += dlink-dir685-touchkeys.o > obj-$(CONFIG_KEYBOARD_EP93XX) += ep93xx_keypad.o > diff --git a/drivers/input/keyboard/cypress-sf.c b/drivers/input/keyboard/cypress-sf.c > new file mode 100644 > index 000000000000..f2862547f633 > --- /dev/null > +++ b/drivers/input/keyboard/cypress-sf.c > @@ -0,0 +1,223 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Cypress StreetFighter Touchkey Driver > + * > + * Copyright (c) 2021 Yassine Oudjana <y.oudjana@xxxxxxxxxxxxxx> > + */ > + > +#include <linux/bitops.h> > +#include <linux/device.h> > +#include <linux/i2c.h> > +#include <linux/input.h> > +#include <linux/interrupt.h> > +#include <linux/module.h> > +#include <linux/pm.h> > +#include <linux/regulator/consumer.h> > + > +#define CYPRESS_SF_DEV_NAME "cypress-sf" > + > +#define CYPRESS_SF_REG_FW_VERSION 0x46 > +#define CYPRESS_SF_REG_HW_VERSION 0x48 > +#define CYPRESS_SF_REG_BUTTON_STATUS 0x4a > + > +struct cypress_sf_data { > + struct i2c_client *client; > + struct input_dev *input_dev; > + struct regulator_bulk_data regulators[2]; > + u32 *keycodes; > + unsigned long keystates; > + int num_keys; > +}; > + > +static irqreturn_t cypress_sf_irq_handler(int irq, void *devid) > +{ > + struct cypress_sf_data *touchkey = devid; > + unsigned long keystates; > + bool curr_state, new_state; > + int key; > + > + keystates = i2c_smbus_read_byte_data(touchkey->client, > + CYPRESS_SF_REG_BUTTON_STATUS); > + if (keystates < 0) { keystates is declared as unsigned, so this condition will never trigger. > + dev_err(&touchkey->client->dev, "Failed to read button status"); > + return IRQ_NONE; > + } > + > + for(key = 0; key < touchkey->num_keys; ++key) { > + curr_state = test_bit(key, &touchkey->keystates); > + new_state = test_bit(key, &keystates); > + > + if(curr_state ^ new_state) { Instead of this please do bitmap_xor(&changed, &keystates, &touchkey->keystates, touchkey->num_keys); for_each_set_bit(key, &changed, touchkey->num_keys) { ... } > + dev_dbg(&touchkey->client->dev,\ > + "Key %d changed to %d", key, new_state); > + input_report_key(touchkey->input_dev, > + touchkey->keycodes[key], > + new_state); > + } > + } > + input_sync(touchkey->input_dev); > + touchkey->keystates = keystates; > + > + return IRQ_HANDLED; > +} > + > +static int cypress_sf_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + struct cypress_sf_data *touchkey; > + int hw_version, fw_version; > + int i, ret; > + > + 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 = "vdd"; > + touchkey->regulators[1].supply = "avdd"; > + > + ret = devm_regulator_bulk_get(&client->dev, > + ARRAY_SIZE(touchkey->regulators), > + touchkey->regulators); > + if(ret) { > + dev_err(&client->dev, "Failed to get regulators: %d\n", ret); > + return ret; > + } > + > + touchkey->num_keys = of_property_count_elems_of_size(client->dev.of_node, > + "linux,keycodes", > + sizeof(u32)); Please use device_property_* API instead of of_property_*. > + if(touchkey->num_keys < 0) > + /* Default key count */ > + touchkey->num_keys = 2; > + > + touchkey->keycodes = devm_kzalloc(&client->dev, > + sizeof(u32) * touchkey->num_keys, GFP_KERNEL); > + if(!touchkey->keycodes) > + return -ENOMEM; > + > + ret = of_property_read_u32_array(client->dev.of_node, "linux,keycodes", > + touchkey->keycodes, touchkey->num_keys); > + > + if(touchkey->num_keys < 0) { > + /* Default keycodes */ > + touchkey->keycodes[0] = KEY_BACK; > + touchkey->keycodes[1] = KEY_MENU; > + } > + > + ret = regulator_bulk_enable(ARRAY_SIZE(touchkey->regulators), > + touchkey->regulators); Please call variables that carry error core or 0 for success 'error'. > + if(ret) { > + dev_err(&client->dev, "Failed to enable regulators: %d\n", ret); > + return ret; > + } > + > + 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 = CYPRESS_SF_DEV_NAME; > + touchkey->input_dev->id.bustype = BUS_I2C; > + > + hw_version = i2c_smbus_read_byte_data(touchkey->client, > + CYPRESS_SF_REG_HW_VERSION); > + fw_version = i2c_smbus_read_word_data(touchkey->client, > + CYPRESS_SF_REG_FW_VERSION); > + if(hw_version < 0 || fw_version < 0) > + dev_warn(&client->dev, "Failed to read versions\n"); > + else > + dev_info(&client->dev, "HW version %d, FW version %d\n", > + hw_version, fw_version); Why is this needed? > + > + for(i = 0; i < touchkey->num_keys; ++i) > + input_set_capability(touchkey->input_dev, EV_KEY, > + touchkey->keycodes[i]); > + > + ret = input_register_device(touchkey->input_dev); > + if(ret) { > + dev_err(&client->dev, > + "Failed to register input device: %d\n", ret); > + return ret; > + } > + > + ret = devm_request_threaded_irq(&client->dev, client->irq, > + NULL, cypress_sf_irq_handler, > + IRQF_ONESHOT, > + CYPRESS_SF_DEV_NAME, touchkey); > + if(ret) { > + dev_err(&client->dev, > + "Failed to register threaded irq: %d", ret); > + return ret; > + } > + > + return 0; > +}; > + > +static int __maybe_unused cypress_sf_suspend(struct device *dev) { > + struct i2c_client *client = to_i2c_client(dev); > + struct cypress_sf_data *touchkey = i2c_get_clientdata(client); > + int ret; > + > + disable_irq(client->irq); > + ret = regulator_bulk_disable(ARRAY_SIZE(touchkey->regulators), > + touchkey->regulators); > + if(ret) { > + dev_err(dev, "Failed to disable regulators: %d", ret); > + enable_irq(client->irq); > + return ret; > + } > + dev_dbg(dev, "Suspended device"); > + > + return 0; > +} > + > +static int __maybe_unused cypress_sf_resume(struct device *dev) { > + struct i2c_client *client = to_i2c_client(dev); > + struct cypress_sf_data *touchkey = i2c_get_clientdata(client); > + int ret; > + > + ret = regulator_bulk_enable(ARRAY_SIZE(touchkey->regulators), > + touchkey->regulators); > + if(ret) { > + dev_err(dev, "Failed to enable regulators: %d", ret); > + return ret; > + } > + enable_irq(client->irq); > + dev_dbg(dev, "Resumed device"); > + > + return 0; > +} > + > +static SIMPLE_DEV_PM_OPS(cypress_sf_pm_ops, > + cypress_sf_suspend, cypress_sf_resume); > + > +static struct i2c_device_id cypress_sf_id_table[] = { > + { CYPRESS_SF_DEV_NAME, 0 }, > + { } > +}; > +MODULE_DEVICE_TABLE(i2c, cypress_sf_id_table); > + > +static const struct of_device_id cypress_sf_of_match[] = { > + { .compatible = "cypress,sf3155", }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, cypress_sf_of_match); > + > +static struct i2c_driver cypress_sf_driver = { > + .driver = { > + .name = CYPRESS_SF_DEV_NAME, > + .pm = &cypress_sf_pm_ops, > + .of_match_table = of_match_ptr(cypress_sf_of_match), > + }, > + .id_table = cypress_sf_id_table, > + .probe = cypress_sf_probe, > +}; > +module_i2c_driver(cypress_sf_driver); > + > +MODULE_AUTHOR("Yassine Oudjana <y.oudjana@xxxxxxxxxxxxxx>"); > +MODULE_DESCRIPTION("Cypress StreetFighter Touchkey Driver"); > +MODULE_LICENSE("GPL v2"); > -- > 2.33.0 > > Thanks. -- Dmitry
