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

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

[Rasmus Villemoes]

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

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

[Rasmus Villemoes]

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

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

[Rasmus Villemoes]

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

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

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 522 bytes --]


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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

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

[Rasmus Villemoes]

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

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

[Sergei Shtylyov]

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

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

[Greg Kroah-Hartman]

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

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

[Sergei Shtylyov]

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

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

[Rasmus Villemoes]

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