Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1278708 > unrolled thread

[PATCH 1/3] usb: musb: convert printk to pr_*

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/3] usb: musb: convert printk to pr_* Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-11-27 11:40 +0100
    [PATCH 3/3] usb: musb: remove redundant stack buffer Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-11-27 11:40 +0100
      Re: [PATCH 3/3] usb: musb: remove redundant stack buffer Felipe Balbi <balbi@ti.com> - 2015-12-01 16:10 +0100
        Re: [PATCH 3/3] usb: musb: remove redundant stack buffer Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-12-03 10:40 +0100
    [PATCH 2/3] usb: musb: remove always-empty string from debug output Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-11-27 11:40 +0100
    Re: [PATCH 1/3] usb: musb: convert printk to pr_* Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2015-11-27 13:30 +0100
      Re: [PATCH 1/3] usb: musb: convert printk to pr_* Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2015-11-28 01:10 +0100
        Re: [PATCH 1/3] usb: musb: convert printk to pr_* Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2015-11-28 11:30 +0100
          Re: [PATCH 1/3] usb: musb: convert printk to pr_* Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-11-30 21:50 +0100

#1278708 — [PATCH 1/3] usb: musb: convert printk to pr_*

This file already uses pr_debug in a few places; this converts the
remaining printks.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/usb/musb/musb_core.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 18cfc0a361cb..1ac976332060 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1360,8 +1360,7 @@ static int ep_config_from_table(struct musb *musb)
 		break;
 	}
 
-	printk(KERN_DEBUG "%s: setup fifo_mode %d\n",
-			musb_driver_name, fifo_mode);
+	pr_debug("%s: setup fifo_mode %d\n", musb_driver_name, fifo_mode);
 
 
 done:
@@ -1390,7 +1389,7 @@ done:
 		musb->nr_endpoints = max(epn, musb->nr_endpoints);
 	}
 
-	printk(KERN_DEBUG "%s: %d/%d max ep, %d/%d memory\n",
+	pr_debug("%s: %d/%d max ep, %d/%d memory\n",
 			musb_driver_name,
 			n + 1, musb->config->num_eps * 2 - 1,
 			offset, (1 << (musb->config->ram_bits + 2)));
@@ -1491,8 +1490,7 @@ static int musb_core_init(u16 musb_type, struct musb *musb)
 	if (reg & MUSB_CONFIGDATA_SOFTCONE)
 		strcat(aInfo, ", SoftConn");
 
-	printk(KERN_DEBUG "%s: ConfigData=0x%02x (%s)\n",
-			musb_driver_name, reg, aInfo);
+	pr_debug("%s: ConfigData=0x%02x (%s)\n", musb_driver_name, reg, aInfo);
 
 	aDate[0] = 0;
 	if (MUSB_CONTROLLER_MHDRC == musb_type) {
@@ -1502,9 +1500,8 @@ static int musb_core_init(u16 musb_type, struct musb *musb)
 		musb->is_multipoint = 0;
 		type = "";
 #ifndef	CONFIG_USB_OTG_BLACKLIST_HUB
-		printk(KERN_ERR
-			"%s: kernel must blacklist external hubs\n",
-			musb_driver_name);
+		pr_err("%s: kernel must blacklist external hubs\n",
+		       musb_driver_name);
 #endif
 	}
 
@@ -1513,8 +1510,8 @@ static int musb_core_init(u16 musb_type, struct musb *musb)
 	snprintf(aRevision, 32, "%d.%d%s", MUSB_HWVERS_MAJOR(musb->hwvers),
 		MUSB_HWVERS_MINOR(musb->hwvers),
 		(musb->hwvers & MUSB_HWVERS_RC) ? "RC" : "");
-	printk(KERN_DEBUG "%s: %sHDRC RTL version %s %s\n",
-			musb_driver_name, type, aRevision, aDate);
+	pr_debug("%s: %sHDRC RTL version %s %s\n",
+		 musb_driver_name, type, aRevision, aDate);
 
 	/* configure ep0 */
 	musb_configure_ep0(musb);
-- 
2.6.1

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

[toc] | [next] | [standalone]

#1278710 — [PATCH 3/3] usb: musb: remove redundant stack buffer

aRevision is only used once, so we might as well do the formatting as
part of the pr_debug. This eliminates the stack buffer, and avoids
doing the formatting at all when pr_debug is compiled out.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/usb/musb/musb_core.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 86dce9635b14..3e8d1bcfc1b5 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1458,7 +1458,7 @@ static int musb_core_init(u16 musb_type, struct musb *musb)
 {
 	u8 reg;
 	char *type;
-	char aInfo[90], aRevision[32];
+	char aInfo[90];
 	void __iomem	*mbase = musb->mregs;
 	int		status = 0;
 	int		i;
@@ -1506,11 +1506,11 @@ static int musb_core_init(u16 musb_type, struct musb *musb)
 
 	/* log release info */
 	musb->hwvers = musb_read_hwvers(mbase);
-	snprintf(aRevision, 32, "%d.%d%s", MUSB_HWVERS_MAJOR(musb->hwvers),
-		MUSB_HWVERS_MINOR(musb->hwvers),
-		(musb->hwvers & MUSB_HWVERS_RC) ? "RC" : "");
-	pr_debug("%s: %sHDRC RTL version %s\n",
-		 musb_driver_name, type, aRevision);
+	pr_debug("%s: %sHDRC RTL version %d.%d%s\n",
+		 musb_driver_name, type,
+		 MUSB_HWVERS_MAJOR(musb->hwvers),
+		 MUSB_HWVERS_MINOR(musb->hwvers),
+		 (musb->hwvers & MUSB_HWVERS_RC) ? "RC" : "");
 
 	/* configure ep0 */
 	musb_configure_ep0(musb);
-- 
2.6.1

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

[toc] | [prev] | [next] | [standalone]

#1280932 — Re: [PATCH 3/3] usb: musb: remove redundant stack buffer

[Multipart message — attachments visible in raw view] — view raw

Hi,

Rasmus Villemoes <linux@rasmusvillemoes.dk> writes:
> aRevision is only used once, so we might as well do the formatting as
> part of the pr_debug. This eliminates the stack buffer, and avoids
> doing the formatting at all when pr_debug is compiled out.
>
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>

this needs to be rebased on top of my testing/next:

checking file drivers/usb/musb/musb_core.c
Hunk #1 FAILED at 1458.
Hunk #2 FAILED at 1506.
2 out of 2 hunks FAILED

-- 
balbi

[toc] | [prev] | [next] | [standalone]

#1282844 — Re: [PATCH 3/3] usb: musb: remove redundant stack buffer

On Tue, Dec 01 2015, Felipe Balbi <balbi@ti.com> wrote:

> Hi,
>
> Rasmus Villemoes <linux@rasmusvillemoes.dk> writes:
>> aRevision is only used once, so we might as well do the formatting as
>> part of the pr_debug. This eliminates the stack buffer, and avoids
>> doing the formatting at all when pr_debug is compiled out.
>>
>> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
>
> this needs to be rebased on top of my testing/next:

git url, please.

> checking file drivers/usb/musb/musb_core.c
> Hunk #1 FAILED at 1458.
> Hunk #2 FAILED at 1506.
> 2 out of 2 hunks FAILED

Did 1/3 and 2/3 apply? They touch some of the same lines, so clearly 3/3
wouldn't apply by itself.

Rasmus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1278711 — [PATCH 2/3] usb: musb: remove always-empty string from debug output

aDate is always empty, hence pointless.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/usb/musb/musb_core.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 1ac976332060..86dce9635b14 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1458,7 +1458,7 @@ static int musb_core_init(u16 musb_type, struct musb *musb)
 {
 	u8 reg;
 	char *type;
-	char aInfo[90], aRevision[32], aDate[12];
+	char aInfo[90], aRevision[32];
 	void __iomem	*mbase = musb->mregs;
 	int		status = 0;
 	int		i;
@@ -1492,7 +1492,6 @@ static int musb_core_init(u16 musb_type, struct musb *musb)
 
 	pr_debug("%s: ConfigData=0x%02x (%s)\n", musb_driver_name, reg, aInfo);
 
-	aDate[0] = 0;
 	if (MUSB_CONTROLLER_MHDRC == musb_type) {
 		musb->is_multipoint = 1;
 		type = "M";
@@ -1510,8 +1509,8 @@ static int musb_core_init(u16 musb_type, struct musb *musb)
 	snprintf(aRevision, 32, "%d.%d%s", MUSB_HWVERS_MAJOR(musb->hwvers),
 		MUSB_HWVERS_MINOR(musb->hwvers),
 		(musb->hwvers & MUSB_HWVERS_RC) ? "RC" : "");
-	pr_debug("%s: %sHDRC RTL version %s %s\n",
-		 musb_driver_name, type, aRevision, aDate);
+	pr_debug("%s: %sHDRC RTL version %s\n",
+		 musb_driver_name, type, aRevision);
 
 	/* configure ep0 */
 	musb_configure_ep0(musb);
-- 
2.6.1

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

[toc] | [prev] | [next] | [standalone]

#1278781

Hello.

On 11/27/2015 1:38 PM, Rasmus Villemoes wrote:

> This file already uses pr_debug in a few places; this converts the
> remaining printks.

    Are you aware that printk(KERN_DEBUG, ...) and pr_debug() are not equivalent?

> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>

[...]

MBR, Sergei


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

[toc] | [prev] | [next] | [standalone]

#1279045

On Fri, Nov 27, 2015 at 03:23:33PM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 11/27/2015 1:38 PM, Rasmus Villemoes wrote:
> 
> >This file already uses pr_debug in a few places; this converts the
> >remaining printks.
> 
>    Are you aware that printk(KERN_DEBUG, ...) and pr_debug() are not equivalent?

Yes, and that is a good thing, you should be using pr_debug() instead of
printk(KERN_DEBUG...).

Why object to something like this?

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1279122

On 11/28/2015 3:04 AM, Greg Kroah-Hartman wrote:

>>> This file already uses pr_debug in a few places; this converts the
>>> remaining printks.
>>
>>     Are you aware that printk(KERN_DEBUG, ...) and pr_debug() are not equivalent?
>
> Yes, and that is a good thing, you should be using pr_debug() instead of
> printk(KERN_DEBUG...).
>
> Why object to something like this?

    I'm not objecting, just asking. There have been many cases in my practice 
where a patch author wasn't aware of that...
    It's just that these printk()'s could have been intentional (not to depend 
on DEBUG or dynamic debugging).

> greg k-h

MBR, Sergei

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

[toc] | [prev] | [next] | [standalone]

#1280259

On Sat, Nov 28 2015, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote:

> On 11/28/2015 3:04 AM, Greg Kroah-Hartman wrote:
>
>>>> This file already uses pr_debug in a few places; this converts the
>>>> remaining printks.
>>>
>>>     Are you aware that printk(KERN_DEBUG, ...) and pr_debug() are not equivalent?
>>
>> Yes, and that is a good thing, you should be using pr_debug() instead of
>> printk(KERN_DEBUG...).
>>
>> Why object to something like this?
>
>    I'm not objecting, just asking. There have been many cases in my
> practice where a patch author wasn't aware of that...

I was aware, but I can see how the commit message could indicate
otherwise. I don't feel strongly for the conversion, so I could redo 2/3
and 3/3 (which should be uncontroversial cleanups).

Rasmus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web