Re: [PATCH v2 53/58] firewire: don't break long lines
On Oct 19 Mauro Carvalho Chehab wrote: > Em Wed, 19 Oct 2016 08:03:01 +0900 > Takashi Sakamoto escreveu: > > A structure for firedtv (struct firedtv) has a member for a pointer to > > struct device. In this case, we can use dev_dbg() for debug printing. [...] > Stefan, > > Would the above be OK for you? ACK. -- Stefan Richter -==- =-=- =-=-- http://arcgraph.de/sr/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 53/58] firewire: don't break long lines
Em Wed, 19 Oct 2016 08:03:01 +0900 Takashi Sakamoto escreveu: > From: Takashi Sakamoto > Date: Wed, 19 Oct 2016 07:53:35 +0900 > Subject: [PATCH] [media] firewire: use dev_dbg() instead of printk() > > A structure for firedtv (struct firedtv) has a member for a pointer to > struct device. In this case, we can use dev_dbg() for debug printing. > This is more preferrable behaviour in device driver development. > > Signed-off-by: Takashi Sakamoto > --- > drivers/media/firewire/firedtv-rc.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/firewire/firedtv-rc.c > b/drivers/media/firewire/firedtv-rc.c > index f82d4a9..04dea2a 100644 > --- a/drivers/media/firewire/firedtv-rc.c > +++ b/drivers/media/firewire/firedtv-rc.c > @@ -184,8 +184,9 @@ void fdtv_handle_rc(struct firedtv *fdtv, unsigned int > code) > else if (code >= 0x4540 && code <= 0x4542) > code = oldtable[code - 0x4521]; > else { > - printk(KERN_DEBUG "firedtv: invalid key code 0x%04x " > -"from remote control\n", code); > + dev_dbg(fdtv->device, > + "invalid key code 0x%04x from remote control\n", > + code); > return; > } Looks good to me. Applied to my development tree: https://git.linuxtv.org/mchehab/experimental.git/commit/?h=printk&id=e0de6d90145753bf40415d670471fcc536b2a26c https://git.linuxtv.org/mchehab/experimental.git/commit/?h=printk&id=9532ba4af6a7619bb028ddd3b829e6f163917b79 Stefan, Would the above be OK for you? Thanks, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 53/58] firewire: don't break long lines
Em Wed, 19 Oct 2016 09:56:25 +0200 Stefan Richter escreveu: > On Oct 19 Takashi Sakamoto wrote: > > --- a/drivers/media/firewire/firedtv-rc.c > > +++ b/drivers/media/firewire/firedtv-rc.c > > @@ -184,8 +184,9 @@ void fdtv_handle_rc(struct firedtv *fdtv, unsigned > > int code) > > else if (code >= 0x4540 && code <= 0x4542) > > code = oldtable[code - 0x4521]; > > else { > > - printk(KERN_DEBUG "firedtv: invalid key code 0x%04x " > > - "from remote control\n", code); > > + dev_dbg(fdtv->device, > > + "invalid key code 0x%04x from remote control\n", > > + code); > > return; > > } > > > > Yes, dev_XYZ(fdtv->device, ...) is better here and is already used this > way throughout the firedtv driver. firedtv-rc.c somehow fell through the > cracks when firedtv was made to use dev_XYZ(). > > (On an unrelated note, this reminds me that I still need to take care of > Mauro's patches "Add a keymap for FireDTV board" and "firedtv: Port it to > use rc_core" from May 28, 2012.) Oh! I forgot about those a long time ago ;) Yeah, it would be great if you could look into those patches when you have some time. Thanks, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 53/58] firewire: don't break long lines
Em Wed, 19 Oct 2016 10:01:13 +0200 Stefan Richter escreveu: > On Oct 18 Mauro Carvalho Chehab wrote: > [...] > > The patch was generated via the script below, and manually > > adjusted if needed. > [...] > > Reviewed-by: Takashi Sakamoto > > Acked-by: Stefan Richter > > Signed-off-by: Mauro Carvalho Chehab > > --- > > drivers/media/firewire/firedtv-rc.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > [...] > > Patch v1 also had a hunk in drivers/media/firewire/firedtv-avc.c. > What happened to it? There was some mistake when I manipulated it manually, after re-run the script. Btw, I liked more the Takashi's dev_dbg() patch for firedtv-rc.c. So, I guess the better would be to apply his patch and the one enclosed here, with just the firedtv-avc.c change. Thanks, Mauro [PATCH] firewire: don't break long lines Due to the 80-cols restrictions, and latter due to checkpatch warnings, several strings were broken into multiple lines. This is not considered a good practice anymore, as it makes harder to grep for strings at the source code. As we're right now fixing other drivers due to KERN_CONT, we need to be able to identify what printk strings don't end with a "\n". It is a way easier to detect those if we don't break long lines. So, join those continuation lines. The patch was generated via the script below, and manually adjusted if needed. use Text::Tabs; while (<>) { if ($next ne "") { $c=$_; if ($c =~ /^\s+\"(.*)/) { $c2=$1; $next =~ s/\"\n$//; $n = expand($next); $funpos = index($n, '('); $pos = index($c2, '",'); if ($funpos && $pos > 0) { $s1 = substr $c2, 0, $pos + 2; $s2 = ' ' x ($funpos + 1) . substr $c2, $pos + 2; $s2 =~ s/^\s+//; $s2 = ' ' x ($funpos + 1) . $s2 if ($s2 ne ""); print unexpand("$next$s1\n"); print unexpand("$s2\n") if ($s2 ne ""); } else { print "$next$c2\n"; } $next=""; next; } else { print $next; } $next=""; } else { if (m/\"$/) { if (!m/\\n\"$/) { $next=$_; next; } } } print $_; } Reviewed-by: Takashi Sakamoto Acked-by: Stefan Richter Signed-off-by: Mauro Carvalho Chehab diff --git a/drivers/media/firewire/firedtv-avc.c b/drivers/media/firewire/firedtv-avc.c index 251a556112a9..5bde6c209cd7 100644 --- a/drivers/media/firewire/firedtv-avc.c +++ b/drivers/media/firewire/firedtv-avc.c @@ -1181,8 +1181,8 @@ int avc_ca_pmt(struct firedtv *fdtv, char *msg, int length) if (es_info_length > 0) { pmt_cmd_id = msg[read_pos++]; if (pmt_cmd_id != 1 && pmt_cmd_id != 4) - dev_err(fdtv->device, "invalid pmt_cmd_id %d " - "at stream level\n", pmt_cmd_id); + dev_err(fdtv->device, "invalid pmt_cmd_id %d at stream level\n", + pmt_cmd_id); if (es_info_length > sizeof(c->operand) - 4 - write_pos) { -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 53/58] firewire: don't break long lines
On Oct 19 Takashi Sakamoto wrote: > --- a/drivers/media/firewire/firedtv-rc.c > +++ b/drivers/media/firewire/firedtv-rc.c > @@ -184,8 +184,9 @@ void fdtv_handle_rc(struct firedtv *fdtv, unsigned > int code) > else if (code >= 0x4540 && code <= 0x4542) > code = oldtable[code - 0x4521]; > else { > - printk(KERN_DEBUG "firedtv: invalid key code 0x%04x " > -"from remote control\n", code); > + dev_dbg(fdtv->device, > + "invalid key code 0x%04x from remote control\n", > + code); > return; > } > Yes, dev_XYZ(fdtv->device, ...) is better here and is already used this way throughout the firedtv driver. firedtv-rc.c somehow fell through the cracks when firedtv was made to use dev_XYZ(). (On an unrelated note, this reminds me that I still need to take care of Mauro's patches "Add a keymap for FireDTV board" and "firedtv: Port it to use rc_core" from May 28, 2012.) -- Stefan Richter -==- =-=- =--== http://arcgraph.de/sr/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 53/58] firewire: don't break long lines
Hi, On Oct 19 2016 05:46, Mauro Carvalho Chehab wrote: > Due to the 80-cols restrictions, and latter due to checkpatch > warnings, several strings were broken into multiple lines. This > is not considered a good practice anymore, as it makes harder > to grep for strings at the source code. > > As we're right now fixing other drivers due to KERN_CONT, we need > to be able to identify what printk strings don't end with a "\n". > It is a way easier to detect those if we don't break long lines. > > So, join those continuation lines. > > The patch was generated via the script below, and manually > adjusted if needed. > > > use Text::Tabs; > while (<>) { > if ($next ne "") { > $c=$_; > if ($c =~ /^\s+\"(.*)/) { > $c2=$1; > $next =~ s/\"\n$//; > $n = expand($next); > $funpos = index($n, '('); > $pos = index($c2, '",'); > if ($funpos && $pos > 0) { > $s1 = substr $c2, 0, $pos + 2; > $s2 = ' ' x ($funpos + 1) . substr $c2, $pos + > 2; > $s2 =~ s/^\s+//; > > $s2 = ' ' x ($funpos + 1) . $s2 if ($s2 ne ""); > > print unexpand("$next$s1\n"); > print unexpand("$s2\n") if ($s2 ne ""); > } else { > print "$next$c2\n"; > } > $next=""; > next; > } else { > print $next; > } > $next=""; > } else { > if (m/\"$/) { > if (!m/\\n\"$/) { > $next=$_; > next; > } > } > } > print $_; > } > > > Reviewed-by: Takashi Sakamoto > Acked-by: Stefan Richter > Signed-off-by: Mauro Carvalho Chehab > --- > drivers/media/firewire/firedtv-rc.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/firewire/firedtv-rc.c > b/drivers/media/firewire/firedtv-rc.c > index f82d4a93feb3..46dde73944df 100644 > --- a/drivers/media/firewire/firedtv-rc.c > +++ b/drivers/media/firewire/firedtv-rc.c > @@ -184,8 +184,8 @@ void fdtv_handle_rc(struct firedtv *fdtv, unsigned int > code) > else if (code >= 0x4540 && code <= 0x4542) > code = oldtable[code - 0x4521]; > else { > - printk(KERN_DEBUG "firedtv: invalid key code 0x%04x " > -"from remote control\n", code); > + printk(KERN_DEBUG "firedtv: invalid key code 0x%04x from remote > control\n", > +code); > return; > } I realized that we can use dev_dbg() instead of the printk(). What do you think about this patch? Anyway, the line is within 80 characters. 8< >From da3289a04226450d6dbabb5c81155ac17c11374d Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Wed, 19 Oct 2016 07:53:35 +0900 Subject: [PATCH] [media] firewire: use dev_dbg() instead of printk() A structure for firedtv (struct firedtv) has a member for a pointer to struct device. In this case, we can use dev_dbg() for debug printing. This is more preferrable behaviour in device driver development. Signed-off-by: Takashi Sakamoto --- drivers/media/firewire/firedtv-rc.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/media/firewire/firedtv-rc.c b/drivers/media/firewire/firedtv-rc.c index f82d4a9..04dea2a 100644 --- a/drivers/media/firewire/firedtv-rc.c +++ b/drivers/media/firewire/firedtv-rc.c @@ -184,8 +184,9 @@ void fdtv_handle_rc(struct firedtv *fdtv, unsigned int code) else if (code >= 0x4540 && code <= 0x4542) code = oldtable[code - 0x4521]; else { - printk(KERN_DEBUG "firedtv: invalid key code 0x%04x " - "from remote control\n", code); + dev_dbg(fdtv->device, + "invalid key code 0x%04x from remote control\n", + code); return; } -- 2.7.4 Regards Takashi Sakamoto -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html