[PATCH v3 1/2] dt-bindings: input: touchscreen: elants_i2c: Add eth3915n touchscreen chip

[PATCH v3 1/2] dt-bindings: input: touchscreen: elants_i2c: Add eth3915n touchscreen chip

[Yunlong Jia]

Add an elan touch screen chip eth3915n.
This chip requires more delay time than the eth3500.

Signed-off-by: Yunlong Jia <yunlong.jia@ecs.com.tw>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Acked-by: David Heidelberg <david@ixit.cz>
---

(no changes since v1)

 .../devicetree/bindings/input/touchscreen/elan,elants_i2c.yaml   | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/elan,elants_i2c.yaml b/Documentation/devicetree/bindings/input/touchscreen/elan,elants_i2c.yaml
index a9b53c2e6f0ab..d28625372f5ac 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/elan,elants_i2c.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/elan,elants_i2c.yaml
@@ -17,6 +17,7 @@ properties:
     enum:
       - elan,ektf3624
       - elan,ekth3500
+      - elan,ekth3915
 
   reg:
     maxItems: 1
-- 
2.17.1

[PATCH v3 2/2] input: touchscreen: elants_i2c: Add eth3915n touchscreen chip

[Yunlong Jia]

The eth3915n requires more delay time than the eth3500 when poweron
 & reset.
Define EKTH3915_POWERON_DELAY_MSEC as the poweron delay time of eth3915n,
 about 80ms.
Define EKTH3915_RESET_DELAY_MSEC as the reset delay time of eth3915n,
 about 300ms.

Signed-off-by: Yunlong Jia <yunlong.jia@ecs.com.tw>
Reviewed-by: Douglas Anderson <dianders@chromium.org>

---

Changes in v3:
 1. Add poweron delay time.

Changes in v2:
 1. Adjust the 'Signed-off-by'.

 drivers/input/touchscreen/elants_i2c.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c
index c9dd703b0c7d8..fb99dd10b0b6d 100644
--- a/drivers/input/touchscreen/elants_i2c.c
+++ b/drivers/input/touchscreen/elants_i2c.c
@@ -116,6 +116,8 @@
 
 #define ELAN_POWERON_DELAY_USEC	500
 #define ELAN_RESET_DELAY_MSEC	20
+#define EKTH3915_POWERON_DELAY_MSEC    80
+#define EKTH3915_RESET_DELAY_MSEC	300
 
 /* FW boot code version */
 #define BC_VER_H_BYTE_FOR_EKTH3900x1_I2C        0x72
@@ -133,6 +135,7 @@
 enum elants_chip_id {
 	EKTH3500,
 	EKTF3624,
+	EKTH3915,
 };
 
 enum elants_state {
@@ -664,6 +667,7 @@ static int elants_i2c_initialize(struct elants_data *ts)
 
 	switch (ts->chip_id) {
 	case EKTH3500:
+	case EKTH3915:
 		if (!error)
 			error = elants_i2c_query_ts_info_ekth(ts);
 		break;
@@ -1331,6 +1335,9 @@ static int elants_i2c_power_on(struct elants_data *ts)
 	if (IS_ERR_OR_NULL(ts->reset_gpio))
 		return 0;
 
+	if (ts->chip_id == EKTH3915)
+		msleep(EKTH3915_POWERON_DELAY_MSEC);
+
 	gpiod_set_value_cansleep(ts->reset_gpio, 1);
 
 	error = regulator_enable(ts->vcc33);
@@ -1361,7 +1368,17 @@ static int elants_i2c_power_on(struct elants_data *ts)
 	if (error)
 		return error;
 
-	msleep(ELAN_RESET_DELAY_MSEC);
+	if (ts->chip_id == EKTH3915)
+		/*
+		 * There need delay 300ms for power on sequence.
+		 * T1 + T2 + T3 >= 305 ms
+		 * T1: 0<time<500us
+		 * T2: >5ms
+		 * T3: >300ms
+		 */
+		msleep(EKTH3915_RESET_DELAY_MSEC);
+	else
+		msleep(ELAN_RESET_DELAY_MSEC);
 
 	return 0;
 }
@@ -1686,6 +1703,7 @@ static const struct i2c_device_id elants_i2c_id[] = {
 	{ DEVICE_NAME, EKTH3500 },
 	{ "ekth3500", EKTH3500 },
 	{ "ektf3624", EKTF3624 },
+	{ "ekth3915", EKTH3915 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, elants_i2c_id);
@@ -1702,6 +1720,7 @@ MODULE_DEVICE_TABLE(acpi, elants_acpi_id);
 static const struct of_device_id elants_of_match[] = {
 	{ .compatible = "elan,ekth3500", .data = (void *)EKTH3500 },
 	{ .compatible = "elan,ektf3624", .data = (void *)EKTF3624 },
+	{ .compatible = "elan,ekth3915", .data = (void *)EKTH3915 },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, elants_of_match);
-- 
2.17.1

Re: [PATCH v3 1/2] dt-bindings: input: touchscreen: elants_i2c: Add eth3915n touchscreen chip

[Doug Anderson]

Hi,

On Fri, Sep 9, 2022 at 3:28 AM Yunlong Jia
<yunlong.jia@ecs.corp-partner.google.com> wrote:
>
> Add an elan touch screen chip eth3915n.
> This chip requires more delay time than the eth3500.
>
> Signed-off-by: Yunlong Jia <yunlong.jia@ecs.com.tw>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> Acked-by: David Heidelberg <david@ixit.cz>

This has the same problem talked about in:

https://lore.kernel.org/r/CAD=FV=XEkPfU9wjwGrp59QJDAspUbWwmOTSZfE1Y5PWt1w309w@mail.gmail.com/

Namely that the Signed-off-by and "From" address don't match. You need
to fix that.

Also note that neither of your patches indicates what changed between v2 and v3.


-Doug

Re: [PATCH v3 2/2] input: touchscreen: elants_i2c: Add eth3915n touchscreen chip

[Doug Anderson]

Hi,

On Fri, Sep 9, 2022 at 3:28 AM Yunlong Jia
<yunlong.jia@ecs.corp-partner.google.com> wrote:
>
> The eth3915n requires more delay time than the eth3500 when poweron
>  & reset.
> Define EKTH3915_POWERON_DELAY_MSEC as the poweron delay time of eth3915n,
>  about 80ms.
> Define EKTH3915_RESET_DELAY_MSEC as the reset delay time of eth3915n,
>  about 300ms.
>
> Signed-off-by: Yunlong Jia <yunlong.jia@ecs.com.tw>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>

Just to be clear, this can't land as long as your Signed-off-by and
"From" address don't match.

-Doug

Re: [PATCH v3 2/2] input: touchscreen: elants_i2c: Add eth3915n touchscreen chip

[Dmitry Torokhov]

Hi Yunlong,

On Fri, Sep 09, 2022 at 10:27:55AM +0000, Yunlong Jia wrote:
> The eth3915n requires more delay time than the eth3500 when poweron
>  & reset.
> Define EKTH3915_POWERON_DELAY_MSEC as the poweron delay time of eth3915n,
>  about 80ms.
> Define EKTH3915_RESET_DELAY_MSEC as the reset delay time of eth3915n,
>  about 300ms.
> 
> Signed-off-by: Yunlong Jia <yunlong.jia@ecs.com.tw>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> 
> ---
> 
> Changes in v3:
>  1. Add poweron delay time.

This materially changes the patch so I do not believe you should have
kept Doug's reviewed-by tag. I also do not understand what this delay is
for. Is this the minimal time for the reset line to stay high? Something
else?

Thanks.

-- 
Dmitry

Re: [PATCH v3 2/2] input: touchscreen: elants_i2c: Add eth3915n touchscreen chip

[Doug Anderson]

Hi,

On Tue, Sep 20, 2022 at 1:38 AM Yunlong Jia
<yunlong.jia@ecs.corp-partner.google.com> wrote:
>
> Dear Dmitry：
>
> Thanks for your reminding.
> After double checked with Elan, we finally decided to cancel this timing delay submission, due to touch panel module has HW modification..

Just to confirm that I understand this. I believe that you're saying
that in the end you decided that you _don't_ need the extra delays for
"ekth3915" and you can just use the normal ones. You only needed the
extra delays because of something that was different on your local
setup. Thus we should consider ${SUBJECT} patch abandoned.

If that's not true then please yell.

-Doug