Hi Hans de Goede,
On 5/27/24 03:42, Hans de Goede wrote:
Hi Joel,
On 5/27/24 5:26 AM, Joel Selvaraj via B4 Relay wrote:
From: Joel Selvaraj <joelselvaraj.oss@xxxxxxxxx>
---
drivers/input/touchscreen/novatek-nvt-ts.c | 78 +++++++++++++++++++++++++++---
1 file changed, 72 insertions(+), 6 deletions(-)
diff --git a/drivers/input/touchscreen/novatek-nvt-ts.c b/drivers/input/touchscreen/novatek-nvt-ts.c
index 224fd112b25a9..7a82a1b09f9d5 100644
--- a/drivers/input/touchscreen/novatek-nvt-ts.c
+++ b/drivers/input/touchscreen/novatek-nvt-ts.c
@@ -139,9 +143,23 @@ static irqreturn_t nvt_ts_irq(int irq, void *dev_id)
return IRQ_HANDLED;
}
+static void nvt_ts_disable_regulators(void *_data)
+{
+ struct nvt_ts_data *data = _data;
+
+ regulator_bulk_disable(ARRAY_SIZE(data->regulators), data->regulators);
+}
+
static int nvt_ts_start(struct input_dev *dev)
{
struct nvt_ts_data *data = input_get_drvdata(dev);
+ int error;
+
+ error = regulator_bulk_enable(ARRAY_SIZE(data->regulators), data->regulators);
+ if (error) {
+ dev_err(&data->client->dev, "failed to enable regulators\n");
+ return error;
+ }
This is weird, you already enable the regulators in probe() and
those get disabled again on remove() by the devm action you add.
So there is no need to enable / disable the regulators on start/stop .
If you want the regulators to only be enabled when the touchscreen
is on then you should disable the regulators again in probe()
after the nvt_ts_read_data() call there (and drop the devm action).
Yes, I want the regulators to be enabled only when the touchscreen is
on/active. I will disable the regulators in probe and remove the devm
action in v4.
@@ -277,8 +324,26 @@ static int nvt_ts_probe(struct i2c_client *client)
return 0;
}
+static const struct nvt_ts_i2c_chip_data nvt_nt11205_ts_data = {
+ .wake_type = 0x05,
+ .chip_id = 0x05,
+};
+
+static const struct nvt_ts_i2c_chip_data nvt_nt36672a_ts_data = {
+ .wake_type = 0x01,
+ .chip_id = 0x08,
+};
+
+static const struct of_device_id nvt_ts_of_match[] = {
+ { .compatible = "novatek,nt11205-ts", .data = &nvt_nt11205_ts_data },
+ { .compatible = "novatek,nt36672a-ts", .data = &nvt_nt36672a_ts_data },
+ { }
+};
+MODULE_DEVICE_TABLE(of, nvt_ts_of_match);
+
static const struct i2c_device_id nvt_ts_i2c_id[] = {
- { "NT11205-ts" },
+ { "NT11205-ts", (unsigned long) &nvt_nt11205_ts_data },
+ { "NT36672A-ts", (unsigned long) &nvt_nt36672a_ts_data },
The i2c-subsystem will also match of compatible strings to i2c_device_ids
by looking at the partof the compatible after the ',', so for a compatible
of e.g. "novatek,nt36672a-ts" will match an i2c_device_id of "nt36672a-ts".
So if you change these to lower-case:
{ "nt11205-ts", (unsigned long) &nvt_nt11205_ts_data },
{ "nt36672a-ts", (unsigned long) &nvt_nt36672a_ts_data },
Then you can drop the nvt_ts_of_match table since that is not necessary
then.
Hmm I just realized that this will break module auto-loading though since that
does require of modaliases .
So maybe this is not such a good idea after all. Still switching to lowercase
i2c_device_id-s would be good for consistency and you need to respin
the patch-set for the regulator issue anyways.
Ok. I will change it to lowercase i2c device id in v4.
Regards,
Joel Selvaraj
