Re: [PATCH v2 29/49] Input: atmel_mxt_ts - implement debug output for messages

From: Jiada Wang
Date: Tue Sep 03 2019 - 03:39:43 EST

Hi Daniel

On 2019/08/30 0:31, Daniel Thompson wrote:
On Tue, Aug 27, 2019 at 03:29:23PM +0900, Jiada Wang wrote:
From: Nick Dyer <nick.dyer@xxxxxxxxxxx>

Add a debug switch which causes all messages from the touch controller to
be dumped to the dmesg log with a set prefix "MXT MSG:". This is used by
Atmel user-space utilities to debug touch operation. Enabling this output
does impact touch performance.

Signed-off-by: Nick Dyer <nick.dyer@xxxxxxxxxxx>
(cherry picked from ndyer/linux/for-upstream commit 3c3fcfdd4889dfeb1c80ae8cd94a622c6342b06a)
[gdavis: Forward port and fix conflicts.]
Signed-off-by: George G. Davis <george_davis@xxxxxxxxxx>
Signed-off-by: Jiada Wang <jiada_wang@xxxxxxxxxx>
drivers/input/touchscreen/atmel_mxt_ts.c | 44 ++++++++++++++++++++++--
1 file changed, 41 insertions(+), 3 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 2d2e8ea30547..941c6970cb70 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -335,6 +335,7 @@ struct mxt_data {
u8 t100_aux_ampl;
u8 t100_aux_area;
u8 t100_aux_vect;
+ bool debug_enabled;
u8 max_reportid;
u32 config_crc;
u32 info_crc;
@@ -460,8 +461,8 @@ static bool mxt_object_readable(unsigned int type)
static void mxt_dump_message(struct mxt_data *data, u8 *message)
- dev_dbg(&data->client->dev, "message: %*ph\n",
- data->T5_msg_size, message);
+ dev_dbg(&data->client->dev, "MXT MSG: %*ph\n",
+ data->T5_msg_size, message);

I'm not 100% convinced that the kernel should change here (arguably the
userspace utility should be modified instead) but if the messages are
conforming to some sort of vendor specific protocol then some commenting
is needed.
I will update with inline comment

@@ -3538,6 +3573,8 @@ static DEVICE_ATTR(hw_version, S_IRUGO, mxt_hw_version_show, NULL);
static DEVICE_ATTR(object, S_IRUGO, mxt_object_show, NULL);
static DEVICE_ATTR(update_cfg, S_IWUSR, NULL, mxt_update_cfg_store);
static DEVICE_ATTR(config_crc, S_IRUGO, mxt_config_crc_show, NULL);
+static DEVICE_ATTR(debug_enable, S_IWUSR | S_IRUSR, mxt_debug_enable_show,
+ mxt_debug_enable_store);

Why isn't CONFIG_DYNAMIC_DEBUG sufficient to enable/disable the

thanks for the comment, I think the only difference is,
by only using CONFIG_DYNAMC_DEBUG, it's hard to differentiate
the messages between valid report_id and exceptional case
(explicitly set of "dump = true")