Le 13/09/2023 à 15:25, Stephan Gerhold a écrit :
From: Jonathan Albrieux <jonathan.albrieux-Re5JQEeQqe8AvxtiuMwx3w@xxxxxxxxxxxxxxxx>
Add a simple driver for the Himax HX852x(ES) touch panel controller,
with support for multi-touch and capacitive touch keys.
The driver is somewhat based on sample code from Himax. However, that
code was so extremely confusing that we spent a significant amount of
time just trying to understand the packet format and register commands.
In this driver they are described with clean structs and defines rather
than lots of magic numbers and offset calculations.
Signed-off-by: Jonathan Albrieux <jonathan.albrieux-Re5JQEeQqe8AvxtiuMwx3w@xxxxxxxxxxxxxxxx>
Co-developed-by: Stephan Gerhold <stephan-3XONVrnlUWDR7s880joybQ@xxxxxxxxxxxxxxxx>
Signed-off-by: Stephan Gerhold <stephan-3XONVrnlUWDR7s880joybQ@xxxxxxxxxxxxxxxx>
---
...
+static irqreturn_t hx852x_interrupt(int irq, void *ptr)
+{
+ struct hx852x *hx = ptr;
+ int error;
+
+ error = hx852x_handle_events(hx);
+ if (error) {
+ dev_err(&hx->client->dev, "failed to handle events: %d\n", error);
Should dev_err_ratelimited() be preferred?
+ return IRQ_NONE;
+ }
+
+ return IRQ_HANDLED;
+}
...
+static int hx852x_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct hx852x *hx;
+ int error, i;
Nit: err or ret is shorter and maybe more "standard".
+
+ if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C |
+ I2C_FUNC_SMBUS_WRITE_BYTE |
+ I2C_FUNC_SMBUS_WRITE_BYTE_DATA |
+ I2C_FUNC_SMBUS_WRITE_WORD_DATA)) {
+ dev_err(dev, "not all i2c functionality supported\n");
+ return -ENXIO;
+ }
+
+ hx = devm_kzalloc(dev, sizeof(*hx), GFP_KERNEL);
+ if (!hx)
+ return -ENOMEM;
+
+ hx->client = client;
+ hx->input_dev = devm_input_allocate_device(dev);
+ if (!hx->input_dev)
+ return -ENOMEM;
+
+ hx->input_dev->name = "Himax HX852x";
+ hx->input_dev->id.bustype = BUS_I2C;
+ hx->input_dev->open = hx852x_input_open;
+ hx->input_dev->close = hx852x_input_close;
+
+ i2c_set_clientdata(client, hx);
+ input_set_drvdata(hx->input_dev, hx);
+
+ hx->supplies[0].supply = "vcca";
+ hx->supplies[1].supply = "vccd";
+ error = devm_regulator_bulk_get(dev, ARRAY_SIZE(hx->supplies), hx->supplies);
+ if (error < 0)
+ return dev_err_probe(dev, error, "failed to get regulators");
+
+ hx->reset_gpiod = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(hx->reset_gpiod))
+ return dev_err_probe(dev, error, "failed to get reset gpio");
+
+ error = devm_request_threaded_irq(dev, client->irq, NULL, hx852x_interrupt,
+ IRQF_ONESHOT | IRQF_NO_AUTOEN, NULL, hx);
+ if (error) {
+ dev_err(dev, "failed to request irq %d: %d\n", client->irq, error);
dev_err_probe() could be used to be consistent with above code.
Same for below dev_err() calls.
+ return error;
+ }
+
+ error = hx852x_read_config(hx);
+ if (error)
+ return error;
+
+ input_set_capability(hx->input_dev, EV_ABS, ABS_MT_POSITION_X);
+ input_set_capability(hx->input_dev, EV_ABS, ABS_MT_POSITION_Y);
+ input_set_abs_params(hx->input_dev, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0);
+
+ touchscreen_parse_properties(hx->input_dev, true, &hx->props);
+ error = hx852x_parse_properties(hx);
+ if (error)
+ return error;
+
+ hx->input_dev->keycode = hx->keycodes;
+ hx->input_dev->keycodemax = hx->keycount;
+ hx->input_dev->keycodesize = sizeof(hx->keycodes[0]);
+ for (i = 0; i < hx->keycount; i++)
+ input_set_capability(hx->input_dev, EV_KEY, hx->keycodes[i]);
+
+ error = input_mt_init_slots(hx->input_dev, hx->max_fingers,
+ INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED);
+ if (error) {
+ dev_err(dev, "failed to init MT slots: %d\n", error);
+ return error;
+ }
+
+ error = input_register_device(hx->input_dev);
+ if (error) {
input_mt_destroy_slots() should be called here, or in an error handling
path below, or via a devm_add_action_or_reset().
It should also be called in a .remove function (unless
devm_add_action_or_reset is prefered)
CJ
+ dev_err(dev, "failed to register input device: %d\n", error);
+ return error;
+ }
+
+ return 0;
+}
...
