2015-11-27 10:38:34

by Rasmus Villemoes

[permalink] [raw]
Subject: [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 <[email protected]>
---
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


2015-11-27 10:39:00

by Rasmus Villemoes

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

aDate is always empty, hence pointless.

Signed-off-by: Rasmus Villemoes <[email protected]>
---
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

2015-11-27 10:38:37

by Rasmus Villemoes

[permalink] [raw]
Subject: [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 <[email protected]>
---
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

2015-11-27 12:23:39

by Sergei Shtylyov

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

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 <[email protected]>

[...]

MBR, Sergei

2015-11-28 00:04:37

by Greg KH

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

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

2015-11-28 10:21:55

by Sergei Shtylyov

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

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

2015-11-30 20:48:36

by Rasmus Villemoes

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

On Sat, Nov 28 2015, Sergei Shtylyov <[email protected]> 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

2015-12-01 15:00:57

by Felipe Balbi

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


Hi,

Rasmus Villemoes <[email protected]> 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 <[email protected]>

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


Attachments:
signature.asc (818.00 B)

2015-12-03 09:31:35

by Rasmus Villemoes

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

On Tue, Dec 01 2015, Felipe Balbi <[email protected]> wrote:

> Hi,
>
> Rasmus Villemoes <[email protected]> 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 <[email protected]>
>
> 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