[PATCH v11 00/56] atmel_mxt_ts misc

Hello Dmitry

On 2020/05/12 7:19, Dmitry Torokhov wrote:

Hi Jiada, Nick,

On Thu, May 07, 2020 at 10:56:04PM -0700, Jiada Wang wrote:

quoted

From: Nick Dyer <redacted>

On some firmware variants, the size of the info block exceeds what can
be read in a single transfer.

Is this limitation of the mXT controller or maybe it is issue with
implementation of the particular i2c adapter and should be dealt with
there?

This patch was authored by Nick,
but I assume it is trying to address issue due to I2C adapter limitation
which following patch in this series is already doing
"Input: atmel_mxt_ts: Limit the max bytes transferred in an i2c transaction"

I will extend patch "Input: atmel_mxt_ts: Limit the max bytes transferred in an i2c transaction"
to also cover this case.

Thanks,
Jiada

Thanks.

Hello Dmitry

On 2020/05/12 7:23, Dmitry Torokhov wrote:

On Thu, May 07, 2020 at 10:56:05PM -0700, Jiada Wang wrote:

quoted

From: Nick Dyer <redacted>

This patch outputs status from T48 Noise Supression

Signed-off-by: Nick Dyer <redacted>
Acked-by: Benson Leung <bleung@chromium.org>
Acked-by: Yufeng Shen <redacted>
(cherry picked from ndyer/linux/for-upstream commit 2895a6ff150a49f27a02938f8d262be238b296d8)
Signed-off-by: George G. Davis <redacted>
Signed-off-by: Jiada Wang <redacted>
---
  drivers/input/touchscreen/atmel_mxt_ts.c | 25 ++++++++++++++++++++++++
  1 file changed, 25 insertions(+)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 7e6a66e3e1e0..a53985a7736f 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c

@@ -324,6 +324,7 @@ struct mxt_data {
  	u16 T18_address;
  	u8 T19_reportid;
  	u16 T44_address;
+	u8 T48_reportid;
  	u8 T100_reportid_min;
  	u8 T100_reportid_max;
  

@@ -978,6 +979,24 @@ static void mxt_proc_t100_message(struct mxt_data *data, u8 *message)
  	data->update_input = true;
  }
  
+static int mxt_proc_t48_messages(struct mxt_data *data, u8 *msg)
+{
+	struct device *dev = &data->client->dev;
+	u8 status, state;
+
+	status = msg[1];
+	state  = msg[4];
+
+	dev_dbg(dev, "T48 state %d status %02X %s%s%s%s%s\n", state, status,
+		status & 0x01 ? "FREQCHG " : "",
+		status & 0x02 ? "APXCHG " : "",
+		status & 0x04 ? "ALGOERR " : "",
+		status & 0x10 ? "STATCHG " : "",
+		status & 0x20 ? "NLVLCHG " : "");

Should we define symbolic names for these bits, like you did for T42
in the next patch?

will replace with symbolic names in next version

Thanks,
Jiada

Thanks.

Hello Dmitry

On 2020/05/12 7:53, Dmitry Torokhov wrote:

On Thu, May 07, 2020 at 10:56:07PM -0700, Jiada Wang wrote:

quoted

From: Nick Dyer <redacted>

The atmel touch messages contain orientation information as a byte in a
packed format which can be passed straight on to Android if the input
device configuration is correct.

No, unfortunately I can not accept this. Please convert to the proper
format for ABS_MT_ORIENTATION as defined in
Documentation/input/multi-touch-protocol.rst

I will remove this patch in next version

Thanks,
Jiada

Thanks.

Hello Dmitry

I am working on refining this series,
regarding your comment about drop changes related to
upload firmware and config during boot.

I found currently only config is uploaded during every boot.
but firmware is only uploaded when userspace asks to do so via
sysfs interface.

Could you help to confirm if this is the case?

Thanks,
Jiada

On 2020/06/25 22:50, Wang, Jiada wrote:

Hello Dmitry

sorry for the delay,

On 2020/05/27 15:43, Dmitry Torokhov wrote:

quoted

Hi Jiada,

On Thu, May 07, 2020 at 10:56:00PM -0700, Jiada Wang wrote:

quoted

This patch-set forward ports Nick Dyer's work in ndyer/linux github
repository as long as some other features and fixes

Sorry for ignoring the series for quite a while. I guess my biggest
issue with the series is that quite a bit of patches are trying to
handle the fallout from a very unfortunate design decision in the
driver: the fact that it attempts to automatically upload firmware and
config on every boot/probe. This design was done at my urging because I
did not have access to the technical documentation and did not realize
that the controller has non-volatile memory for both firmware and
configuration. We should only attempt to automatically load firmware
where device does not have non-volatile memory and is unable function
otherwise, in all other cases we better leave it to userspace to decide
whether to execute firmware update and when. The kernel should only
provide facilities so that userspace can initiate firmware update. This
design has worked well for Chrome OS for many years (it used Atmel
controllers in several products), and I would like to bring it to the
mainline.

I agree with you, I will review the patch-set,
and only pick these not related to firmware/cfg upload

Thanks,
jiada

quoted

Thanks.

Hello All

I am thinking it doesn't make sense to keep the series
with such a big chunk of patches,
I will divide the series into several small series

Thanks,
Jiada

On 2020/07/08 22:05, Wang, Jiada wrote:

Hello Dmitry

I am working on refining this series,
regarding your comment about drop changes related to
upload firmware and config during boot.

I found currently only config is uploaded during every boot.
but firmware is only uploaded when userspace asks to do so via
sysfs interface.

Could you help to confirm if this is the case?

Thanks,
Jiada

On 2020/06/25 22:50, Wang, Jiada wrote:

quoted

Hello Dmitry

sorry for the delay,

On 2020/05/27 15:43, Dmitry Torokhov wrote:

quoted

Hi Jiada,

On Thu, May 07, 2020 at 10:56:00PM -0700, Jiada Wang wrote:

quoted

This patch-set forward ports Nick Dyer's work in ndyer/linux github
repository as long as some other features and fixes

Sorry for ignoring the series for quite a while. I guess my biggest
issue with the series is that quite a bit of patches are trying to
handle the fallout from a very unfortunate design decision in the
driver: the fact that it attempts to automatically upload firmware and
config on every boot/probe. This design was done at my urging because I
did not have access to the technical documentation and did not realize
that the controller has non-volatile memory for both firmware and
configuration. We should only attempt to automatically load firmware
where device does not have non-volatile memory and is unable function
otherwise, in all other cases we better leave it to userspace to decide
whether to execute firmware update and when. The kernel should only
provide facilities so that userspace can initiate firmware update. This
design has worked well for Chrome OS for many years (it used Atmel
controllers in several products), and I would like to bring it to the
mainline.

I agree with you, I will review the patch-set,
and only pick these not related to firmware/cfg upload

Thanks,
jiada

quoted

Thanks.