[PATCH] Input - elantech: fix debug dump of the current packet

Subsystems: input (keyboard, mouse, joystick, touchscreen) drivers, the rest

STALE3657d

8 messages, 3 authors, 2016-07-28 · open the first message on its own page

[PATCH] Input - elantech: fix debug dump of the current packet

From: Benjamin Tissoires <hidden>
Date: 2016-07-28 09:59:03

The use of mixed psmouse_printk() and printk creates 2 lines in the log,
while the use of %*ph solves everything.

Signed-off-by: Benjamin Tissoires <redacted>
---
 drivers/input/mouse/elantech.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index 615d23e..3461d04 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -222,12 +222,8 @@ static int elantech_write_reg(struct psmouse *psmouse, unsigned char reg,
  */
 static void elantech_packet_dump(struct psmouse *psmouse)
 {
-	int	i;
-
-	psmouse_printk(KERN_DEBUG, psmouse, "PS/2 packet [");
-	for (i = 0; i < psmouse->pktsize; i++)
-		printk("%s0x%02x ", i ? ", " : " ", psmouse->packet[i]);
-	printk("]\n");
+	psmouse_printk(KERN_DEBUG, psmouse, "PS/2 packet [%*ph]",
+		       psmouse->pktsize, psmouse->packet);
 }
 
 /*
-- 
2.5.5

Re: [PATCH] Input - elantech: fix debug dump of the current packet

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2016-07-28 18:22:36

On Thu, Jul 28, 2016 at 11:58:55AM +0200, Benjamin Tissoires wrote:
The use of mixed psmouse_printk() and printk creates 2 lines in the log,
while the use of %*ph solves everything.

Signed-off-by: Benjamin Tissoires <redacted>
Applied, thank you.
quoted hunk
---
 drivers/input/mouse/elantech.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index 615d23e..3461d04 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -222,12 +222,8 @@ static int elantech_write_reg(struct psmouse *psmouse, unsigned char reg,
  */
 static void elantech_packet_dump(struct psmouse *psmouse)
 {
-	int	i;
-
-	psmouse_printk(KERN_DEBUG, psmouse, "PS/2 packet [");
-	for (i = 0; i < psmouse->pktsize; i++)
-		printk("%s0x%02x ", i ? ", " : " ", psmouse->packet[i]);
-	printk("]\n");
+	psmouse_printk(KERN_DEBUG, psmouse, "PS/2 packet [%*ph]",
+		       psmouse->pktsize, psmouse->packet);
 }
 
 /*
-- 
2.5.5
-- 
Dmitry

Re: [PATCH] Input - elantech: fix debug dump of the current packet

From: Joe Perches <joe@perches.com>
Date: 2016-07-28 18:31:04

On Thu, 2016-07-28 at 11:58 +0200, Benjamin Tissoires wrote:
quoted hunk
The use of mixed psmouse_printk() and printk creates 2 lines in the log,
while the use of %*ph solves everything.

Signed-off-by: Benjamin Tissoires <redacted>
---
 drivers/input/mouse/elantech.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index 615d23e..3461d04 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -222,12 +222,8 @@ static int elantech_write_reg(struct psmouse *psmouse, unsigned char reg,
  */
 static void elantech_packet_dump(struct psmouse *psmouse)
 {
-	int	i;
-
-	psmouse_printk(KERN_DEBUG, psmouse, "PS/2 packet [");
-	for (i = 0; i < psmouse->pktsize; i++)
-		printk("%s0x%02x ", i ? ", " : " ", psmouse->packet[i]);
-	printk("]\n");
+	psmouse_printk(KERN_DEBUG, psmouse, "PS/2 packet [%*ph]",
+		       psmouse->pktsize, psmouse->packet);
 }
 
 /*
This should add a newline "\n" format termination but
how does the original produce multiple lines of output?

It looks more like psmouse_printk should be removed altogether
and dev_debug should be used instead.
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Input - elantech: fix debug dump of the current packet

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2016-07-28 18:40:09

On Thu, Jul 28, 2016 at 11:30:56AM -0700, Joe Perches wrote:
On Thu, 2016-07-28 at 11:58 +0200, Benjamin Tissoires wrote:
quoted
The use of mixed psmouse_printk() and printk creates 2 lines in the log,
while the use of %*ph solves everything.

Signed-off-by: Benjamin Tissoires <redacted>
---
 drivers/input/mouse/elantech.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index 615d23e..3461d04 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -222,12 +222,8 @@ static int elantech_write_reg(struct psmouse *psmouse, unsigned char reg,
  */
 static void elantech_packet_dump(struct psmouse *psmouse)
 {
-	int	i;
-
-	psmouse_printk(KERN_DEBUG, psmouse, "PS/2 packet [");
-	for (i = 0; i < psmouse->pktsize; i++)
-		printk("%s0x%02x ", i ? ", " : " ", psmouse->packet[i]);
-	printk("]\n");
+	psmouse_printk(KERN_DEBUG, psmouse, "PS/2 packet [%*ph]",
+		       psmouse->pktsize, psmouse->packet);
 }
 
 /*
This should add a newline "\n" format termination but
I added it, thanks for alerting me.
how does the original produce multiple lines of output?

It looks more like psmouse_printk should be removed altogether
and dev_debug should be used instead.
That does not produce output by default whereas elantech driver uses
elantech_packet_dump() to drop unexpected packets.

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Input - elantech: fix debug dump of the current packet

From: Joe Perches <joe@perches.com>
Date: 2016-07-28 18:50:06

On Thu, 2016-07-28 at 11:40 -0700, Dmitry Torokhov wrote:
On Thu, Jul 28, 2016 at 11:30:56AM -0700, Joe Perches wrote:
quoted
On Thu, 2016-07-28 at 11:58 +0200, Benjamin Tissoires wrote:
quoted
The use of mixed psmouse_printk() and printk creates 2 lines in the log,
while the use of %*ph solves everything.

Signed-off-by: Benjamin Tissoires <redacted>
---
 drivers/input/mouse/elantech.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index 615d23e..3461d04 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -222,12 +222,8 @@ static int elantech_write_reg(struct psmouse *psmouse, unsigned char reg,
  */
 static void elantech_packet_dump(struct psmouse *psmouse)
 {
-	int	i;
-
-	psmouse_printk(KERN_DEBUG, psmouse, "PS/2 packet [");
-	for (i = 0; i < psmouse->pktsize; i++)
-		printk("%s0x%02x ", i ? ", " : " ", psmouse->packet[i]);
-	printk("]\n");
+	psmouse_printk(KERN_DEBUG, psmouse, "PS/2 packet [%*ph]",
+		       psmouse->pktsize, psmouse->packet);
 }
 
 /*
This should add a newline "\n" format termination but
I added it, thanks for alerting me.
quoted
how does the original produce multiple lines of output?

It looks more like psmouse_printk should be removed altogether
and dev_debug should be used instead.
That does not produce output by default whereas elantech driver uses
elantech_packet_dump() to drop unexpected packets.
So use dev_printk(KERN_DEBUG, etc...


But I still don't see a mechanism for the original code to be
emitted on multiple lines.

How long is the longest packet?

This might truncate it.

Should this be ratelimited?

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Input - elantech: fix debug dump of the current packet

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2016-07-28 18:54:19

On Thu, Jul 28, 2016 at 11:49:57AM -0700, Joe Perches wrote:
On Thu, 2016-07-28 at 11:40 -0700, Dmitry Torokhov wrote:
quoted
On Thu, Jul 28, 2016 at 11:30:56AM -0700, Joe Perches wrote:
quoted
On Thu, 2016-07-28 at 11:58 +0200, Benjamin Tissoires wrote:
quoted
The use of mixed psmouse_printk() and printk creates 2 lines in the log,
while the use of %*ph solves everything.

Signed-off-by: Benjamin Tissoires <redacted>
---
 drivers/input/mouse/elantech.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index 615d23e..3461d04 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -222,12 +222,8 @@ static int elantech_write_reg(struct psmouse *psmouse, unsigned char reg,
  */
 static void elantech_packet_dump(struct psmouse *psmouse)
 {
-	int	i;
-
-	psmouse_printk(KERN_DEBUG, psmouse, "PS/2 packet [");
-	for (i = 0; i < psmouse->pktsize; i++)
-		printk("%s0x%02x ", i ? ", " : " ", psmouse->packet[i]);
-	printk("]\n");
+	psmouse_printk(KERN_DEBUG, psmouse, "PS/2 packet [%*ph]",
+		       psmouse->pktsize, psmouse->packet);
 }
 
 /*
This should add a newline "\n" format termination but
I added it, thanks for alerting me.
quoted
how does the original produce multiple lines of output?

It looks more like psmouse_printk should be removed altogether
and dev_debug should be used instead.
That does not produce output by default whereas elantech driver uses
elantech_packet_dump() to drop unexpected packets.
So use dev_printk(KERN_DEBUG, etc...
#define psmouse_printk(level, psmouse, format, ...)	\
	dev_printk(level,				\
		   &(psmouse)->ps2dev.serio->dev,	\
		   psmouse_fmt(format), ##__VA_ARGS__)

I want to see current protocol module in th eoutput and I do not want to
write "&(psmouse)->ps2dev.serio->dev" all the time.

But I still don't see a mechanism for the original code to be
emitted on multiple lines.
Message from another driver snuck in?
How long is the longest packet?

This might truncate it.
No, buffer is 8 bytes.
Should this be ratelimited?
So far there wasn't a need.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Input - elantech: fix debug dump of the current packet

From: Joe Perches <joe@perches.com>
Date: 2016-07-28 18:58:38

On Thu, 2016-07-28 at 11:54 -0700, Dmitry Torokhov wrote:
On Thu, Jul 28, 2016 at 11:49:57AM -0700, Joe Perches wrote:
[]
quoted
So use dev_printk(KERN_DEBUG, etc...
#define psmouse_printk(level, psmouse, format, ...)	\
	dev_printk(level,				\
		   &(psmouse)->ps2dev.serio->dev,	\
		   psmouse_fmt(format), ##__VA_ARGS__)

I want to see current protocol module in th eoutput and I do not want to
write "&(psmouse)->ps2dev.serio->dev" all the time.
There are only 2 uses of the macro.

$ git grep psmouse_printk
drivers/input/mouse/elantech.c:                 psmouse_printk(KERN_DEBUG, psmouse,             \
drivers/input/mouse/elantech.c: psmouse_printk(KERN_DEBUG, psmouse, "PS/2 packet [");
drivers/input/mouse/psmouse.h:#define psmouse_printk(level, psmouse, format, ...)       \


--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Input - elantech: fix debug dump of the current packet

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2016-07-28 19:03:40

On July 28, 2016 11:58:29 AM PDT, Joe Perches [off-list ref] wrote:
On Thu, 2016-07-28 at 11:54 -0700, Dmitry Torokhov wrote:
quoted
On Thu, Jul 28, 2016 at 11:49:57AM -0700, Joe Perches wrote:
[]
quoted
quoted
So use dev_printk(KERN_DEBUG, etc...
#define psmouse_printk(level, psmouse, format, ...)	\
	dev_printk(level,				\
		   &(psmouse)->ps2dev.serio->dev,	\
		   psmouse_fmt(format), ##__VA_ARGS__)

I want to see current protocol module in th eoutput and I do not want
to
quoted
write "&(psmouse)->ps2dev.serio->dev" all the time.
There are only 2 uses of the macro.

$ git grep psmouse_printk
drivers/input/mouse/elantech.c:                 psmouse_printk(KERN_DEBUG,
psmouse,             \
drivers/input/mouse/elantech.c: psmouse_printk(KERN_DEBUG, psmouse,
"PS/2 packet [");
drivers/input/mouse/psmouse.h:#define psmouse_printk(level, psmouse,
format, ...)       \
Yes, but many more of psmouse_<severity> calls and I like consistency.


Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help