| Message ID | 1469699935-23056-1-git-send-email-benjamin.tissoires@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
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 <benjamin.tissoires@redhat.com> Applied, thank you. > --- > 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 >
On Thu, 2016-07-28 at 11:58 +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 <benjamin.tissoires@redhat.com> > --- > 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
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: > > 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 <benjamin.tissoires@redhat.com> > > --- > > 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.
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: > > > > On Thu, 2016-07-28 at 11:58 +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 <benjamin.tissoires@redhat.com> > > > --- > > > 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. 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
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: > > 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: > > > > > > > > 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 <benjamin.tissoires@redhat.com> > > > > --- > > > > 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. > > 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.
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: [] > > 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
On July 28, 2016 11:58:29 AM PDT, Joe Perches <joe@perches.com> wrote: >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: >[] >> > 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, ...) \ Yes, but many more of psmouse_<severity> calls and I like consistency. Thanks.
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); } /*
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 <benjamin.tissoires@redhat.com> --- drivers/input/mouse/elantech.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)