| Message ID | 20210305153815.126937-1-vincent.knecht@mailoo.org |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | [v6,1/2] dt-bindings: input/touchscreen: add bindings for msg2638 | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/dt-meta-schema | success | |
| robh/dtbs-check | success |
On Fri, 05 Mar 2021 16:38:04 +0100, Vincent Knecht wrote: > This adds dts bindings for the mstar msg2638 touchscreen. > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > Signed-off-by: Vincent Knecht <vincent.knecht@mailoo.org> > --- > Changed in v6: > - change touchscreen-size-x/y values in example section to reflect > scaling removal in driver (Dmitry) > - add Linus W. Reviewed-by > Changed in v5: nothing > Changed in v4: > - don't use wildcards in compatible strings (Rob H) > - rename from msg26xx to msg2638 > - rename example pinctrl-0 to &ts_int_reset_default for consistency > Changed in v3: > - added `touchscreen-size-x: true` and `touchscreen-size-y: true` properties > Changed in v2: > - changed M-Star to MStar in title line > - changed reset gpio to active-low in example section > --- > .../input/touchscreen/mstar,msg2638.yaml | 69 +++++++++++++++++++ > 1 file changed, 69 insertions(+) > create mode 100644 Documentation/devicetree/bindings/input/touchscreen/mstar,msg2638.yaml > Reviewed-by: Rob Herring <robh@kernel.org>
On Fri, Mar 5, 2021 at 4:38 PM Vincent Knecht <vincent.knecht@mailoo.org> wrote: > Add support for the msg2638 touchscreen IC from MStar. > Firmware handling, wakeup gestures and other specialties are not supported. > This driver reuses zinitix.c structure, while the checksum and irq handler > functions are based on out-of-tree driver for Alcatel Idol 3 (4.7"). > > Signed-off-by: Vincent Knecht <vincent.knecht@mailoo.org> > --- > Changed in v6: Looks good to me: Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
Le vendredi 05 mars 2021 à 16:38 +0100, Vincent Knecht a écrit : > Add support for the msg2638 touchscreen IC from MStar. > Firmware handling, wakeup gestures and other specialties are not supported. > This driver reuses zinitix.c structure, while the checksum and irq handler > functions are based on out-of-tree driver for Alcatel Idol 3 (4.7"). > > Signed-off-by: Vincent Knecht <vincent.knecht@mailoo.org> > --- > Changed in v6: > - minor formatting changes > - mention wakeup gestures not being supported (yet?) in commit message > - do not scale coordinates in the driver (Dmitry) > - multiple suggestions from Linus W. > - include linux/gpio/consumer.h instead of linux/gpio.h > - rename delay defines to include time unit like _MS and _US > - rename `error` variable to `ret` > - move enable_irq() call from msg2638_power_on() to msg2638_start() > - remove CONFIG_OF #ifdef around of_device_id struct Hello, just a little gentle ping: do I need to change anything to get this merged ? So far seems to work fine with idol347 port of postmarketOS :-)
Hi Vincent, On Fri, Mar 05, 2021 at 04:38:05PM +0100, Vincent Knecht wrote: > Add support for the msg2638 touchscreen IC from MStar. > Firmware handling, wakeup gestures and other specialties are not supported. > This driver reuses zinitix.c structure, while the checksum and irq handler > functions are based on out-of-tree driver for Alcatel Idol 3 (4.7"). > > Signed-off-by: Vincent Knecht <vincent.knecht@mailoo.org> > --- > Changed in v6: > - minor formatting changes > - mention wakeup gestures not being supported (yet?) in commit message > - do not scale coordinates in the driver (Dmitry) > - multiple suggestions from Linus W. > - include linux/gpio/consumer.h instead of linux/gpio.h > - rename delay defines to include time unit like _MS and _US > - rename `error` variable to `ret` > - move enable_irq() call from msg2638_power_on() to msg2638_start() > - remove CONFIG_OF #ifdef around of_device_id struct > Changed in v5: > - use gpiod_set_value_cansleep() (Stephan G) > - use msleep/usleep_range() rathen than mdelay() (Stephan G) > Changed in v4: > - rename from msg26xx to msg2638, following compatible string change > - rename mstar_* functions to msg2638_* for consistency > - add RESET_DELAY define and use it in msg2638_power_on() > - change a few dev_err() calls to be on one line only > - add missing \n in a few dev_err() strings > Changed in v3: > - no change > Changed in v2: > - don't use bitfields in packet struct, to prevent endian-ness related problems (Dmitry) > - fix reset gpiod calls order in mstar_power_on() (Dmitry) > --- > drivers/input/touchscreen/Kconfig | 12 + > drivers/input/touchscreen/Makefile | 1 + > drivers/input/touchscreen/msg2638.c | 354 ++++++++++++++++++++++++++++ > 3 files changed, 367 insertions(+) > create mode 100644 drivers/input/touchscreen/msg2638.c > > diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig > index f012fe746df0..fefbe1c1bb10 100644 > --- a/drivers/input/touchscreen/Kconfig > +++ b/drivers/input/touchscreen/Kconfig > @@ -1334,4 +1334,16 @@ config TOUCHSCREEN_ZINITIX > To compile this driver as a module, choose M here: the > module will be called zinitix. > > +config TOUCHSCREEN_MSG2638 > + tristate "MStar msg2638 touchscreen support" > + depends on I2C > + depends on GPIOLIB || COMPILE_TEST > + help > + Say Y here if you have an I2C touchscreen using MStar msg2638. > + > + If unsure, say N. > + > + To compile this driver as a module, choose M here: the > + module will be called msg2638. > + > endif > diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile > index 6233541e9173..83e516cb3d33 100644 > --- a/drivers/input/touchscreen/Makefile > +++ b/drivers/input/touchscreen/Makefile > @@ -112,3 +112,4 @@ obj-$(CONFIG_TOUCHSCREEN_ROHM_BU21023) += rohm_bu21023.o > obj-$(CONFIG_TOUCHSCREEN_RASPBERRYPI_FW) += raspberrypi-ts.o > obj-$(CONFIG_TOUCHSCREEN_IQS5XX) += iqs5xx.o > obj-$(CONFIG_TOUCHSCREEN_ZINITIX) += zinitix.o > +obj-$(CONFIG_TOUCHSCREEN_MSG2638) += msg2638.o > diff --git a/drivers/input/touchscreen/msg2638.c b/drivers/input/touchscreen/msg2638.c > new file mode 100644 > index 000000000000..8eb3f195d743 > --- /dev/null > +++ b/drivers/input/touchscreen/msg2638.c > @@ -0,0 +1,354 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Driver for MStar msg2638 touchscreens > + * > + * Copyright (c) 2021 Vincent Knecht <vincent.knecht@mailoo.org> > + * > + * Checksum and IRQ handler based on mstar_drv_common.c and mstar_drv_mutual_fw_control.c > + * Copyright (c) 2006-2012 MStar Semiconductor, Inc. > + * > + * Driver structure based on zinitix.c by Michael Srba <Michael.Srba@seznam.cz> > + */ > + > +#include <linux/delay.h> > +#include <linux/gpio/consumer.h> > +#include <linux/i2c.h> > +#include <linux/input.h> > +#include <linux/input/mt.h> > +#include <linux/input/touchscreen.h> > +#include <linux/interrupt.h> > +#include <linux/irq.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/regulator/consumer.h> > +#include <linux/slab.h> > + > +#define MODE_DATA_RAW 0x5A > + > +#define MAX_SUPPORTED_FINGER_NUM 5 > + > +#define CHIP_ON_DELAY_MS 15 > +#define FIRMWARE_ON_DELAY_MS 50 > +#define RESET_DELAY_MIN_US 10000 > +#define RESET_DELAY_MAX_US 11000 > + > +struct point_coord { > + u16 x; > + u16 y; > +}; > + > +struct packet { > + u8 xy_hi; /* higher bits of x and y coordinates */ > + u8 x_low; > + u8 y_low; > + u8 pressure; > +}; > + > +struct touch_event { > + u8 mode; > + struct packet pkt[MAX_SUPPORTED_FINGER_NUM]; > + u8 proximity; > + u8 checksum; > +}; > + > +struct msg2638_ts_data { > + struct i2c_client *client; > + struct input_dev *input_dev; > + struct touchscreen_properties prop; > + struct regulator_bulk_data supplies[2]; > + struct gpio_desc *reset_gpiod; > +}; > + > +static int msg2638_init_regulators(struct msg2638_ts_data *msg2638) > +{ > + struct i2c_client *client = msg2638->client; > + int ret; > + > + msg2638->supplies[0].supply = "vdd"; > + msg2638->supplies[1].supply = "vddio"; > + ret = devm_regulator_bulk_get(&client->dev, > + ARRAY_SIZE(msg2638->supplies), > + msg2638->supplies); > + if (ret < 0) { > + dev_err(&client->dev, "Failed to get regulators: %d\n", ret); > + return ret; > + } > + > + return 0; I do not see benefit of factoring out into separate function so I moved it directly into probe(), similarly how we acquire reset GPIO. > +} > + > +static void msg2638_power_on(struct msg2638_ts_data *msg2638) Renamed to msg2638_reset(). > +{ > + gpiod_set_value_cansleep(msg2638->reset_gpiod, 1); > + usleep_range(RESET_DELAY_MIN_US, RESET_DELAY_MAX_US); > + gpiod_set_value_cansleep(msg2638->reset_gpiod, 0); > + msleep(FIRMWARE_ON_DELAY_MS); > +} > + > +static void msg2638_report_finger(struct msg2638_ts_data *msg2638, int slot, > + const struct point_coord *pc) > +{ > + input_mt_slot(msg2638->input_dev, slot); > + input_mt_report_slot_state(msg2638->input_dev, MT_TOOL_FINGER, true); > + touchscreen_report_pos(msg2638->input_dev, &msg2638->prop, pc->x, pc->y, true); > + input_report_abs(msg2638->input_dev, ABS_MT_TOUCH_MAJOR, 1); The device does not really report "touch major", this is synthetic data, and therefore I removed it. All in all this is quite simple function, I pushed it down into msg2638_ts_irq_handler(). > +} > + > +static u8 msg2638_checksum(u8 *data, u32 length) > +{ > + s32 sum = 0; > + u32 i; > + > + for (i = 0; i < length; i++) > + sum += data[i]; > + > + return (u8)((-sum) & 0xFF); > +} > + > +static irqreturn_t msg2638_ts_irq_handler(int irq, void *msg2638_handler) > +{ > + struct msg2638_ts_data *msg2638 = msg2638_handler; > + struct i2c_client *client = msg2638->client; > + struct touch_event touch_event; > + struct point_coord coord; > + struct i2c_msg msg[1]; > + struct packet *p; > + u32 len; > + int ret; > + int i; > + > + len = sizeof(struct touch_event); > + memset(&touch_event, 0, len); > + > + msg[0].addr = client->addr; > + msg[0].flags = I2C_M_RD; > + msg[0].len = len; > + msg[0].buf = (u8 *)&touch_event; > + > + ret = i2c_transfer(client->adapter, msg, 1); > + if (ret != 1) { > + dev_err(&client->dev, "Failed I2C transfer in irq handler!\n"); > + goto out; > + } > + > + if (touch_event.mode != MODE_DATA_RAW) > + goto out; > + > + if (msg2638_checksum((u8 *)&touch_event, len - 1) != touch_event.checksum) { > + dev_err(&client->dev, "Failed checksum!\n"); > + goto out; > + } > + > + for (i = 0; i < MAX_SUPPORTED_FINGER_NUM; i++) { > + p = &touch_event.pkt[i]; > + /* Ignore non-pressed finger data */ > + if (p->xy_hi == 0xFF && p->x_low == 0xFF && p->y_low == 0xFF) > + continue; > + > + coord.x = (((p->xy_hi & 0xF0) << 4) | p->x_low); > + coord.y = (((p->xy_hi & 0x0F) << 8) | p->y_low); > + msg2638_report_finger(msg2638, i, &coord); > + } > + > + input_mt_sync_frame(msg2638->input_dev); > + input_sync(msg2638->input_dev); > + > +out: > + return IRQ_HANDLED; > +} > + > +static int msg2638_start(struct msg2638_ts_data *msg2638) > +{ > + int ret; > + > + ret = regulator_bulk_enable(ARRAY_SIZE(msg2638->supplies), msg2638->supplies); > + if (ret) { > + dev_err(&msg2638->client->dev, "Failed to enable regulators: %d\n", ret); > + return ret; > + } > + > + msleep(CHIP_ON_DELAY_MS); > + msg2638_power_on(msg2638); > + enable_irq(msg2638->client->irq); > + > + return 0; > +} > + > +static int msg2638_stop(struct msg2638_ts_data *msg2638) > +{ > + int ret; > + > + disable_irq(msg2638->client->irq); > + > + ret = regulator_bulk_disable(ARRAY_SIZE(msg2638->supplies), msg2638->supplies); > + if (ret) { > + dev_err(&msg2638->client->dev, "Failed to disable regulators: %d\n", ret); > + return ret; > + } > + > + return 0; > +} > + > +static int msg2638_input_open(struct input_dev *dev) > +{ > + struct msg2638_ts_data *msg2638 = input_get_drvdata(dev); > + > + return msg2638_start(msg2638); > +} > + > +static void msg2638_input_close(struct input_dev *dev) > +{ > + struct msg2638_ts_data *msg2638 = input_get_drvdata(dev); > + > + msg2638_stop(msg2638); > +} > + > +static int msg2638_init_input_dev(struct msg2638_ts_data *msg2638) > +{ > + struct input_dev *input_dev; > + int ret; > + > + input_dev = devm_input_allocate_device(&msg2638->client->dev); > + if (!input_dev) { > + dev_err(&msg2638->client->dev, "Failed to allocate input device.\n"); > + return -ENOMEM; > + } > + > + input_set_drvdata(input_dev, msg2638); > + msg2638->input_dev = input_dev; > + > + input_dev->name = "MStar TouchScreen"; > + input_dev->phys = "input/ts"; > + input_dev->id.bustype = BUS_I2C; > + input_dev->open = msg2638_input_open; > + input_dev->close = msg2638_input_close; > + > + input_set_capability(input_dev, EV_ABS, ABS_MT_POSITION_X); > + input_set_capability(input_dev, EV_ABS, ABS_MT_POSITION_Y); > + input_set_abs_params(input_dev, ABS_MT_WIDTH_MAJOR, 0, 15, 0, 0); > + input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0); The last 2 events are not reported and should not be advertised in the capabilities. > + > + touchscreen_parse_properties(input_dev, true, &msg2638->prop); > + if (!msg2638->prop.max_x || !msg2638->prop.max_y) { > + dev_err(&msg2638->client->dev, > + "touchscreen-size-x and/or touchscreen-size-y not set in dts\n"); > + return -EINVAL; > + } > + > + ret = input_mt_init_slots(input_dev, MAX_SUPPORTED_FINGER_NUM, > + INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED); > + if (ret) { > + dev_err(&msg2638->client->dev, "Failed to initialize MT slots: %d\n", ret); > + return ret; > + } > + > + ret = input_register_device(input_dev); > + if (ret) { > + dev_err(&msg2638->client->dev, "Failed to register input device: %d\n", ret); > + return ret; > + } > + > + return 0; > +} > + > +static int msg2638_ts_probe(struct i2c_client *client) > +{ > + struct msg2638_ts_data *msg2638; > + int ret; > + > + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) { > + dev_err(&client->dev, "Failed to assert adapter's support for plain I2C.\n"); > + return -ENXIO; > + } > + > + msg2638 = devm_kzalloc(&client->dev, sizeof(*msg2638), GFP_KERNEL); > + if (!msg2638) > + return -ENOMEM; > + > + msg2638->client = client; > + i2c_set_clientdata(client, msg2638); > + > + ret = msg2638_init_regulators(msg2638); > + if (ret) { > + dev_err(&client->dev, "Failed to initialize regulators: %d\n", ret); > + return ret; > + } > + > + msg2638->reset_gpiod = devm_gpiod_get(&client->dev, "reset", GPIOD_OUT_LOW); > + if (IS_ERR(msg2638->reset_gpiod)) { > + ret = PTR_ERR(msg2638->reset_gpiod); > + dev_err(&client->dev, "Failed to request reset GPIO: %d\n", ret); > + return ret; > + } > + > + ret = msg2638_init_input_dev(msg2638); > + if (ret) { > + dev_err(&client->dev, "Failed to initialize input device: %d\n", ret); > + return ret; > + } > + > + irq_set_status_flags(client->irq, IRQ_NOAUTOEN); We now have IRQF_NO_AUTOEN as a flag for request_irq() and friends so I used it instead of irq_set_status_flags(). > + ret = devm_request_threaded_irq(&client->dev, client->irq, > + NULL, msg2638_ts_irq_handler, > + IRQF_ONESHOT, client->name, msg2638); > + if (ret) { > + dev_err(&client->dev, "Failed to request IRQ: %d\n", ret); > + return ret; > + } > + > + return 0; > +} > + > +static int __maybe_unused msg2638_suspend(struct device *dev) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct msg2638_ts_data *msg2638 = i2c_get_clientdata(client); > + > + mutex_lock(&msg2638->input_dev->mutex); > + > + if (input_device_enabled(msg2638->input_dev)) > + msg2638_stop(msg2638); > + > + mutex_unlock(&msg2638->input_dev->mutex); > + > + return 0; > +} > + > +static int __maybe_unused msg2638_resume(struct device *dev) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct msg2638_ts_data *msg2638 = i2c_get_clientdata(client); > + int ret = 0; > + > + mutex_lock(&msg2638->input_dev->mutex); > + > + if (input_device_enabled(msg2638->input_dev)) > + ret = msg2638_start(msg2638); > + > + mutex_unlock(&msg2638->input_dev->mutex); > + > + return ret; > +} > + > +static SIMPLE_DEV_PM_OPS(msg2638_pm_ops, msg2638_suspend, msg2638_resume); > + > +static const struct of_device_id msg2638_of_match[] = { > + { .compatible = "mstar,msg2638" }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, msg2638_of_match); > + > +static struct i2c_driver msg2638_ts_driver = { > + .probe_new = msg2638_ts_probe, > + .driver = { > + .name = "MStar-TS", > + .pm = &msg2638_pm_ops, > + .of_match_table = of_match_ptr(msg2638_of_match), Given that the driver is not depending on OF, when !CONFIG_OF this will cause unused variable msg2638_of_match. Given that we can use OF matchinv with ACPI I dropped of_match_ptr(). > + }, > +}; > +module_i2c_driver(msg2638_ts_driver); > + > +MODULE_AUTHOR("Vincent Knecht <vincent.knecht@mailoo.org>"); > +MODULE_DESCRIPTION("MStar MSG2638 touchscreen driver"); > +MODULE_LICENSE("GPL v2"); > -- > 2.29.2 > > > Thanks.
diff --git a/Documentation/devicetree/bindings/input/touchscreen/mstar,msg2638.yaml b/Documentation/devicetree/bindings/input/touchscreen/mstar,msg2638.yaml new file mode 100644 index 000000000000..3a42c23faf6f --- /dev/null +++ b/Documentation/devicetree/bindings/input/touchscreen/mstar,msg2638.yaml @@ -0,0 +1,69 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/input/touchscreen/mstar,msg2638.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: MStar msg2638 touchscreen controller Bindings + +maintainers: + - Vincent Knecht <vincent.knecht@mailoo.org> + +allOf: + - $ref: touchscreen.yaml# + +properties: + compatible: + const: mstar,msg2638 + + reg: + const: 0x26 + + interrupts: + maxItems: 1 + + reset-gpios: + maxItems: 1 + + vdd-supply: + description: Power supply regulator for the chip + + vddio-supply: + description: Power supply regulator for the I2C bus + + touchscreen-size-x: true + touchscreen-size-y: true + +additionalProperties: false + +required: + - compatible + - reg + - interrupts + - reset-gpios + - touchscreen-size-x + - touchscreen-size-y + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + #include <dt-bindings/interrupt-controller/irq.h> + i2c { + #address-cells = <1>; + #size-cells = <0>; + touchscreen@26 { + compatible = "mstar,msg2638"; + reg = <0x26>; + interrupt-parent = <&msmgpio>; + interrupts = <13 IRQ_TYPE_EDGE_FALLING>; + reset-gpios = <&msmgpio 100 GPIO_ACTIVE_LOW>; + pinctrl-names = "default"; + pinctrl-0 = <&ts_int_reset_default>; + vdd-supply = <&pm8916_l17>; + vddio-supply = <&pm8916_l5>; + touchscreen-size-x = <2048>; + touchscreen-size-y = <2048>; + }; + }; + +...