2022-06-21 13:06:52

by Dario Binacchi

[permalink] [raw]
Subject: [RESEND PATCH v4 0/6] Input: edt-ft5x06 - Improve configuration

The series was born from the analysis and mitigation of a crc problem
raised by an M06 type device. The added sysfs attributes were helpful
in debugging the problem. Patches that change the report rate on driver
probing, mitigated crc errors on kernel bootup. The patch to get/set
report rate by sysfs for an M12 device, has been tested.

Changes in v4:
- Add Rob Herring 'Acked-by' tag.

Changes in v3:
- Add hz unit suffix.
- Add '|' to description.
- Check the lower and upper limits of the report-rate-hz value
- Convert the M06 report-rate-hz value

Changes in v2:
- Add Oliver Graute's 'Acked-by' tag to:
* Input: edt-ft5x06 - show model name by sysfs
* Input: edt-ft5x06 - show firmware version by sysfs
- Fix yaml file. Tested with `make DT_CHECKER_FLAGS=-m dt_binding_check'.

Dario Binacchi (6):
dt-bindings: input: touchscreen: edt-ft5x06: add report-rate-hz
Input: edt-ft5x06 - get/set M12 report rate by sysfs
Input: edt-ft5x06 - set report rate by dts property
Input: edt-ft5x06 - show model name by sysfs
Input: edt-ft5x06 - show firmware version by sysfs
Input: edt-ft5x06 - show crc and header errors by sysfs

.../input/touchscreen/edt-ft5x06.yaml | 8 ++
drivers/input/touchscreen/edt-ft5x06.c | 103 ++++++++++++++++--
2 files changed, 103 insertions(+), 8 deletions(-)

--
2.32.0


2022-06-21 13:14:12

by Dario Binacchi

[permalink] [raw]
Subject: [RESEND PATCH v4 5/6] Input: edt-ft5x06 - show firmware version by sysfs

The firmware version was printed only if debug mode was enabled. Now you
can always get it from sysfs.

Co-developed-by: Michael Trimarchi <[email protected]>
Signed-off-by: Michael Trimarchi <[email protected]>
Signed-off-by: Dario Binacchi <[email protected]>
Acked-by: Oliver Graute <[email protected]>
---

(no changes since v1)

drivers/input/touchscreen/edt-ft5x06.c | 22 +++++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index 89157deebfcb..bab92344b2ea 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -129,6 +129,7 @@ struct edt_ft5x06_ts_data {
int max_support_points;

char name[EDT_NAME_LEN];
+ char fw_version[EDT_NAME_LEN];

struct edt_reg_addr reg_addr;
enum edt_ver version;
@@ -540,6 +541,17 @@ static ssize_t model_show(struct device *dev, struct device_attribute *attr,

static DEVICE_ATTR_RO(model);

+static ssize_t fw_version_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct edt_ft5x06_ts_data *tsdata = i2c_get_clientdata(client);
+
+ return scnprintf(buf, PAGE_SIZE, "%s\n", tsdata->fw_version);
+}
+
+static DEVICE_ATTR_RO(fw_version);
+
static struct attribute *edt_ft5x06_attrs[] = {
&edt_ft5x06_attr_gain.dattr.attr,
&edt_ft5x06_attr_offset.dattr.attr,
@@ -548,6 +560,7 @@ static struct attribute *edt_ft5x06_attrs[] = {
&edt_ft5x06_attr_threshold.dattr.attr,
&edt_ft5x06_attr_report_rate.dattr.attr,
&dev_attr_model.attr,
+ &dev_attr_fw_version.attr,
NULL
};

@@ -834,13 +847,13 @@ static void edt_ft5x06_ts_teardown_debugfs(struct edt_ft5x06_ts_data *tsdata)
#endif /* CONFIG_DEBUGFS */

static int edt_ft5x06_ts_identify(struct i2c_client *client,
- struct edt_ft5x06_ts_data *tsdata,
- char *fw_version)
+ struct edt_ft5x06_ts_data *tsdata)
{
u8 rdbuf[EDT_NAME_LEN];
char *p;
int error;
char *model_name = tsdata->name;
+ char *fw_version = tsdata->fw_version;

/* see what we find if we assume it is a M06 *
* if we get less than EDT_NAME_LEN, we don't want
@@ -1097,7 +1110,6 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
unsigned long irq_flags;
int error;
u32 report_rate;
- char fw_version[EDT_NAME_LEN];

dev_dbg(&client->dev, "probing for EDT FT5x06 I2C\n");

@@ -1210,7 +1222,7 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
tsdata->input = input;
tsdata->factory_mode = false;

- error = edt_ft5x06_ts_identify(client, tsdata, fw_version);
+ error = edt_ft5x06_ts_identify(client, tsdata);
if (error) {
dev_err(&client->dev, "touchscreen probe failed\n");
return error;
@@ -1257,7 +1269,7 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,

dev_dbg(&client->dev,
"Model \"%s\", Rev. \"%s\", %dx%d sensors\n",
- tsdata->name, fw_version, tsdata->num_x, tsdata->num_y);
+ tsdata->name, tsdata->fw_version, tsdata->num_x, tsdata->num_y);

input->name = tsdata->name;
input->id.bustype = BUS_I2C;
--
2.32.0

2022-06-21 13:36:27

by Dario Binacchi

[permalink] [raw]
Subject: [RESEND PATCH v4 4/6] Input: edt-ft5x06 - show model name by sysfs

The model name was printed only if debug mode was enabled. Now you can
always get it from sysfs.

Co-developed-by: Michael Trimarchi <[email protected]>
Signed-off-by: Michael Trimarchi <[email protected]>
Signed-off-by: Dario Binacchi <[email protected]>
Acked-by: Oliver Graute <[email protected]>
---

(no changes since v1)

drivers/input/touchscreen/edt-ft5x06.c | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index 843e8b0522f7..89157deebfcb 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -529,6 +529,17 @@ static EDT_ATTR(threshold, S_IWUSR | S_IRUGO, WORK_REGISTER_THRESHOLD,
static EDT_ATTR(report_rate, S_IWUSR | S_IRUGO, WORK_REGISTER_REPORT_RATE,
M12_REGISTER_REPORT_RATE, NO_REGISTER, 0, 255);

+static ssize_t model_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct edt_ft5x06_ts_data *tsdata = i2c_get_clientdata(client);
+
+ return scnprintf(buf, PAGE_SIZE, "%s\n", tsdata->name);
+}
+
+static DEVICE_ATTR_RO(model);
+
static struct attribute *edt_ft5x06_attrs[] = {
&edt_ft5x06_attr_gain.dattr.attr,
&edt_ft5x06_attr_offset.dattr.attr,
@@ -536,6 +547,7 @@ static struct attribute *edt_ft5x06_attrs[] = {
&edt_ft5x06_attr_offset_y.dattr.attr,
&edt_ft5x06_attr_threshold.dattr.attr,
&edt_ft5x06_attr_report_rate.dattr.attr,
+ &dev_attr_model.attr,
NULL
};

--
2.32.0

2022-06-29 05:30:12

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [RESEND PATCH v4 0/6] Input: edt-ft5x06 - Improve configuration

On Tue, Jun 21, 2022 at 02:39:31PM +0200, Dario Binacchi wrote:
> The series was born from the analysis and mitigation of a crc problem
> raised by an M06 type device. The added sysfs attributes were helpful
> in debugging the problem. Patches that change the report rate on driver
> probing, mitigated crc errors on kernel bootup. The patch to get/set
> report rate by sysfs for an M12 device, has been tested.
>
> Changes in v4:
> - Add Rob Herring 'Acked-by' tag.
>
> Changes in v3:
> - Add hz unit suffix.
> - Add '|' to description.
> - Check the lower and upper limits of the report-rate-hz value
> - Convert the M06 report-rate-hz value
>
> Changes in v2:
> - Add Oliver Graute's 'Acked-by' tag to:
> * Input: edt-ft5x06 - show model name by sysfs
> * Input: edt-ft5x06 - show firmware version by sysfs
> - Fix yaml file. Tested with `make DT_CHECKER_FLAGS=-m dt_binding_check'.
>
> Dario Binacchi (6):
> dt-bindings: input: touchscreen: edt-ft5x06: add report-rate-hz
> Input: edt-ft5x06 - get/set M12 report rate by sysfs
> Input: edt-ft5x06 - set report rate by dts property
> Input: edt-ft5x06 - show model name by sysfs
> Input: edt-ft5x06 - show firmware version by sysfs
> Input: edt-ft5x06 - show crc and header errors by sysfs
>
> .../input/touchscreen/edt-ft5x06.yaml | 8 ++
> drivers/input/touchscreen/edt-ft5x06.c | 103 ++++++++++++++++--
> 2 files changed, 103 insertions(+), 8 deletions(-)

Applied the lot with a few small adjustments, thank you.

--
Dmitry

2022-06-29 05:47:25

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [RESEND PATCH v4 4/6] Input: edt-ft5x06 - show model name by sysfs

Hi Dario,

On Tue, Jun 21, 2022 at 02:39:35PM +0200, Dario Binacchi wrote:
> The model name was printed only if debug mode was enabled. Now you can
> always get it from sysfs.
>
> Co-developed-by: Michael Trimarchi <[email protected]>
> Signed-off-by: Michael Trimarchi <[email protected]>
> Signed-off-by: Dario Binacchi <[email protected]>
> Acked-by: Oliver Graute <[email protected]>
> ---
>
> (no changes since v1)
>
> drivers/input/touchscreen/edt-ft5x06.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> index 843e8b0522f7..89157deebfcb 100644
> --- a/drivers/input/touchscreen/edt-ft5x06.c
> +++ b/drivers/input/touchscreen/edt-ft5x06.c
> @@ -529,6 +529,17 @@ static EDT_ATTR(threshold, S_IWUSR | S_IRUGO, WORK_REGISTER_THRESHOLD,
> static EDT_ATTR(report_rate, S_IWUSR | S_IRUGO, WORK_REGISTER_REPORT_RATE,
> M12_REGISTER_REPORT_RATE, NO_REGISTER, 0, 255);
>
> +static ssize_t model_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct edt_ft5x06_ts_data *tsdata = i2c_get_clientdata(client);
> +
> + return scnprintf(buf, PAGE_SIZE, "%s\n", tsdata->name);

There is sysfs_emit() that is preferred to be used in sysfs handlers.
Same comment applies to the patches following this one.

I made the change and applied.

Thanks.

--
Dmitry