Re: [PATCH v10 3/3] Input: Add TouchNetix axiom i2c touchscreen driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Le 2024-04-19 11:28, Marco Felsch a écrit :
Hi Kamel,


Hello,

thank you for the patch. Again just a rough review.

On 24-04-19, Kamel Bouhara wrote:
Add a new driver for the TouchNetix's axiom family of
touchscreen controllers. This driver only supports i2c
and can be later adapted for SPI and USB support.

Signed-off-by: Kamel Bouhara <kamel.bouhara@xxxxxxxxxxx>
---

...

+static int axiom_i2c_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct input_dev *input_dev;
+	struct axiom_data *ts;
+	u32 poll_interval;
+	int target;
+	int error;
+
+	ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
+	if (!ts)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, ts);
+	ts->client = client;
+	ts->dev = dev;
+
+	ts->regmap = devm_regmap_init_i2c(client, &axiom_i2c_regmap_config);
+	error = PTR_ERR_OR_ZERO(ts->regmap);
+	if (error) {
+		dev_err(dev, "Failed to initialize regmap: %d\n", error);
+		return error;
+	}
+
+	ts->vddi = devm_regulator_get(dev, "VDDI");

This does not match the dt-bindings.

+	if (IS_ERR(ts->vddi))
+		return dev_err_probe(&client->dev, PTR_ERR(ts->vddi),
+				     "Failed to enable VDDI regulator\n");
+
+	ts->vdda = devm_regulator_get(dev, "VDDA");

Here as well.

+	if (IS_ERR(ts->vdda))
+		return dev_err_probe(&client->dev, PTR_ERR(ts->vdda),
+				     "Failed to enable VDDA regulator\n");

Now we handle it as always but..

+ ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(ts->reset_gpio))
+ return dev_err_probe(dev, PTR_ERR(ts->reset_gpio), "failed to get reset GPIO\n");
+
+	if (ts->reset_gpio)
+		axiom_reset(ts->reset_gpio);
+	else
+		msleep(AXIOM_STARTUP_TIME_MS);

still the reset is useless since you never enabled the regulators. So
either use devm_regulator_get_enable() or you do the enable/disable
separate via regulator_enable()/disable(). If there are no strict
enablement restrictions like orders and timings you could also make use
of the regulator_bulk API (e.g. devm_regulator_bulk_get_enable).


Fixed in v11, thanks !

Regards,
  Marco

+
+	error = axiom_discover(ts);
+	if (error)
+		return dev_err_probe(dev, error, "Failed touchscreen discover\n");
+
+	input_dev = devm_input_allocate_device(ts->dev);
+	if (!input_dev)
+		return -ENOMEM;
+
+	input_dev->name = "TouchNetix axiom Touchscreen";
+	input_dev->phys = "input/axiom_ts";
+
+	input_set_abs_params(input_dev, ABS_MT_POSITION_X, 0, 65535, 0, 0);
+	input_set_abs_params(input_dev, ABS_MT_POSITION_Y, 0, 65535, 0, 0);
+ input_set_abs_params(input_dev, ABS_MT_TOOL_TYPE, 0, MT_TOOL_MAX, 0, 0);
+	input_set_abs_params(input_dev, ABS_MT_DISTANCE, 0, 127, 0, 0);
+	input_set_abs_params(input_dev, ABS_MT_PRESSURE, 0, 127, 0, 0);
+
+	touchscreen_parse_properties(input_dev, true, &ts->prop);
+
+ /* Registers the axiom device as a touchscreen instead of a mouse pointer */ + error = input_mt_init_slots(input_dev, AXIOM_U41_MAX_TARGETS, INPUT_MT_DIRECT);
+	if (error)
+		return error;
+
+ /* Enables the raw data for up to 4 force channels to be sent to the input subsystem */
+	set_bit(EV_REL, input_dev->evbit);
+	set_bit(EV_MSC, input_dev->evbit);
+	/* Declare that we support "RAW" Miscellaneous events */
+	set_bit(MSC_RAW, input_dev->mscbit);
+
+	ts->input_dev = input_dev;
+ input_set_drvdata(ts->input_dev, ts);shfs kamel@xxxxxxxxxxxxxxxxxx:/home/kamel/granitenet/modem/cmux cmux/
+
+	/* Ensure that all reports are initialised to not be present. */
+	for (target = 0; target < AXIOM_U41_MAX_TARGETS; target++)
+		ts->targets[target].state = AXIOM_TARGET_STATE_NOT_PRESENT;
+
+	error = devm_request_threaded_irq(dev, client->irq, NULL,
+					  axiom_irq, IRQF_ONESHOT, dev_name(dev), ts);
+	if (error) {
+		dev_info(dev, "Request irq failed, falling back to polling mode");
+
+		error = input_setup_polling(input_dev, axiom_i2c_poll);
+		if (error)
+ return dev_err_probe(ts->dev, error, "Unable to set up polling mode\n");
+
+ if (!device_property_read_u32(ts->dev, "poll-interval", &poll_interval))
+			input_set_poll_interval(input_dev, poll_interval);
+		else
+			input_set_poll_interval(input_dev, POLL_INTERVAL_DEFAULT_MS);
+	}
+
+	return input_register_device(input_dev);
+}
+
+static const struct i2c_device_id axiom_i2c_id_table[] = {
+	{ "ax54a" },
+	{ },
+};
+MODULE_DEVICE_TABLE(i2c, axiom_i2c_id_table);
+
+static const struct of_device_id axiom_i2c_of_match[] = {
+	{ .compatible = "touchnetix,ax54a", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, axiom_i2c_of_match);
+
+static struct i2c_driver axiom_i2c_driver = {
+	.driver = {
+		   .name = "axiom",
+		   .of_match_table = axiom_i2c_of_match,
+	},
+	.id_table = axiom_i2c_id_table,
+	.probe = axiom_i2c_probe,
+};
+module_i2c_driver(axiom_i2c_driver);
+
+MODULE_AUTHOR("Bart Prescott <bartp@xxxxxxxxxxxxxx>");
+MODULE_AUTHOR("Pedro Torruella <pedro.torruella@xxxxxxxxxxxxxx>");
+MODULE_AUTHOR("Mark Satterthwaite <mark.satterthwaite@xxxxxxxxxxxxxx>");
+MODULE_AUTHOR("Hannah Rossiter <hannah.rossiter@xxxxxxxxxxxxxx>");
+MODULE_AUTHOR("Kamel Bouhara <kamel.bouhara@xxxxxxxxxxx>");
+MODULE_DESCRIPTION("TouchNetix axiom touchscreen I2C bus driver");
+MODULE_LICENSE("GPL");
--
2.25.1



--
--
Kamel Bouhara, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com




[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux