* [PATCH] iqrdomain: Improve formatting in debugfs.
@ 2012-04-10 0:06 David Daney
2012-04-11 7:03 ` Grant Likely
0 siblings, 1 reply; 11+ messages in thread
From: David Daney @ 2012-04-10 0:06 UTC (permalink / raw)
To: Grant Likely, Benjamin Herrenschmidt, Thomas Gleixner
Cc: linux-kernel, David Daney
From: David Daney <david.daney@cavium.com>
The irq_domain_mapping file has some interesting output when chip_data
contains leading zeros:
virq hwirq chip name chip data domain name
.
.
.
103 0x00000 CIU2-W 0x 5e0000000000000 none
104 0x00000 CIU2-W 0x 5f0000000000000 none
105 0x00000 CIU2-M 0x (null) none
113 0x00000 CIU2-E 0x1080000000000000 none
.
.
.
I think there should be no space in there between the "0x" and the
rest. Also the '(null)' entry doesn't make much sense at all with a
"0x".
Instead I suggest:
.
.
.
103 0x00000 CIU2-W 0x05e0000000000000 none
104 0x00000 CIU2-W 0x05f0000000000000 none
105 0x00000 CIU2-M 0x0000000000000000 none
113 0x00000 CIU2-E 0x1080000000000000 none
.
.
.
This patch also prints the "chip data" as an 8 character wide number
for 32-bit kernels, instead of the 16 characters wide in the 64-bit
case.
Signed-off-by: David Daney <david.daney@cavium.com>
---
kernel/irq/irqdomain.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 481f2da..83474bf 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -642,9 +642,10 @@ static int virq_debug_show(struct seq_file *m, void *private)
static const char none[] = "none";
void *data;
int i;
+ int ptr_width = sizeof(void *) * 2;
- seq_printf(m, "%-5s %-7s %-15s %-18s %s\n", "virq", "hwirq",
- "chip name", "chip data", "domain name");
+ seq_printf(m, "%-5s %-7s %-15s %-*s %s\n", "virq", "hwirq",
+ "chip name", ptr_width + 2, "chip data", "domain name");
for (i = 1; i < nr_irqs; i++) {
desc = irq_to_desc(i);
@@ -667,7 +668,8 @@ static int virq_debug_show(struct seq_file *m, void *private)
seq_printf(m, "%-15s ", p);
data = irq_desc_get_chip_data(desc);
- seq_printf(m, "0x%16p ", data);
+ seq_printf(m, "0x%0*lx ",
+ ptr_width, (unsigned long)data);
if (desc->irq_data.domain && desc->irq_data.domain->of_node)
p = desc->irq_data.domain->of_node->full_name;
--
1.7.2.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] iqrdomain: Improve formatting in debugfs.
2012-04-10 0:06 [PATCH] iqrdomain: Improve formatting in debugfs David Daney
@ 2012-04-11 7:03 ` Grant Likely
2012-04-11 9:16 ` Andreas Schwab
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Grant Likely @ 2012-04-11 7:03 UTC (permalink / raw)
To: David Daney, Benjamin Herrenschmidt, Thomas Gleixner
Cc: linux-kernel, David Daney
On Mon, 9 Apr 2012 17:06:57 -0700, David Daney <ddaney.cavm@gmail.com> wrote:
> From: David Daney <david.daney@cavium.com>
>
> The irq_domain_mapping file has some interesting output when chip_data
> contains leading zeros:
>
> virq hwirq chip name chip data domain name
> .
> .
> .
> 103 0x00000 CIU2-W 0x 5e0000000000000 none
> 104 0x00000 CIU2-W 0x 5f0000000000000 none
> 105 0x00000 CIU2-M 0x (null) none
> 113 0x00000 CIU2-E 0x1080000000000000 none
> .
> .
> .
>
> I think there should be no space in there between the "0x" and the
> rest. Also the '(null)' entry doesn't make much sense at all with a
> "0x".
Yeah, it doesn't make much sense with the '0x', but I don't want to
supress the (null) output because it is really important to highlight
when a domain isn't assigned to an active irq.
Fixing the zero pad is trivial to fix though. Just remove the
precision from the %p format. Here's my counter-patch:
---
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 9310a8d..eb05e40 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -643,8 +643,8 @@ static int virq_debug_show(struct seq_file *m, void *private)
void *data;
int i;
- seq_printf(m, "%-5s %-7s %-15s %-18s %s\n", "virq", "hwirq",
- "chip name", "chip data", "domain name");
+ seq_printf(m, "%-5s %-7s %-15s %-*s %s\n", "irq", "hwirq",
+ "chip name", 2 * sizeof(void *) + 2, "chip data", "domain name");
for (i = 1; i < nr_irqs; i++) {
desc = irq_to_desc(i);
@@ -667,7 +667,7 @@ static int virq_debug_show(struct seq_file *m, void *private)
seq_printf(m, "%-15s ", p);
data = irq_desc_get_chip_data(desc);
- seq_printf(m, "0x%16p ", data);
+ seq_printf(m, data ? "0x%p " : " %p ", data);
if (desc->irq_data.domain && desc->irq_data.domain->of_node)
p = desc->irq_data.domain->of_node->full_name;
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] iqrdomain: Improve formatting in debugfs.
2012-04-11 7:03 ` Grant Likely
@ 2012-04-11 9:16 ` Andreas Schwab
2012-04-11 14:52 ` Grant Likely
2012-04-11 15:33 ` David Daney
2012-04-12 19:20 ` Andreas Schwab
2 siblings, 1 reply; 11+ messages in thread
From: Andreas Schwab @ 2012-04-11 9:16 UTC (permalink / raw)
To: Grant Likely
Cc: David Daney, Benjamin Herrenschmidt, Thomas Gleixner,
linux-kernel, David Daney
Grant Likely <grant.likely@secretlab.ca> writes:
> @@ -667,7 +667,7 @@ static int virq_debug_show(struct seq_file *m, void *private)
> seq_printf(m, "%-15s ", p);
>
> data = irq_desc_get_chip_data(desc);
> - seq_printf(m, "0x%16p ", data);
> + seq_printf(m, data ? "0x%p " : " %p ", data);
You should be able to use %#p instead to let it add the prefix.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] iqrdomain: Improve formatting in debugfs.
2012-04-11 9:16 ` Andreas Schwab
@ 2012-04-11 14:52 ` Grant Likely
2012-04-11 14:57 ` Grant Likely
2012-04-16 22:01 ` Andreas Schwab
0 siblings, 2 replies; 11+ messages in thread
From: Grant Likely @ 2012-04-11 14:52 UTC (permalink / raw)
To: Andreas Schwab
Cc: David Daney, Benjamin Herrenschmidt, Thomas Gleixner,
linux-kernel, David Daney
On Wed, Apr 11, 2012 at 3:16 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> Grant Likely <grant.likely@secretlab.ca> writes:
>
>> @@ -667,7 +667,7 @@ static int virq_debug_show(struct seq_file *m, void *private)
>> seq_printf(m, "%-15s ", p);
>>
>> data = irq_desc_get_chip_data(desc);
>> - seq_printf(m, "0x%16p ", data);
>> + seq_printf(m, data ? "0x%p " : " %p ", data);
>
> You should be able to use %#p instead to let it add the prefix.
That mostly works, but it doesn't round up the field size to account
for the 2 characters in '0x'. Looks like the %p code needs to be
tweaked to fix that.
Also, gcc complains about it:
/home/grant/hacking/linux/kernel/irq/irqdomain.c: In function ‘virq_debug_show’:
/home/grant/hacking/linux/kernel/irq/irqdomain.c:670:4: warning: '#'
flag used with ‘%p’ gnu_printf format [-Wformat]
Do you know how to suppress that?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] iqrdomain: Improve formatting in debugfs.
2012-04-11 14:52 ` Grant Likely
@ 2012-04-11 14:57 ` Grant Likely
2012-04-16 22:01 ` Andreas Schwab
1 sibling, 0 replies; 11+ messages in thread
From: Grant Likely @ 2012-04-11 14:57 UTC (permalink / raw)
To: Andreas Schwab
Cc: David Daney, Benjamin Herrenschmidt, Thomas Gleixner,
linux-kernel, David Daney
On Wed, Apr 11, 2012 at 8:52 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Wed, Apr 11, 2012 at 3:16 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:
>> Grant Likely <grant.likely@secretlab.ca> writes:
>>
>>> @@ -667,7 +667,7 @@ static int virq_debug_show(struct seq_file *m, void *private)
>>> seq_printf(m, "%-15s ", p);
>>>
>>> data = irq_desc_get_chip_data(desc);
>>> - seq_printf(m, "0x%16p ", data);
>>> + seq_printf(m, data ? "0x%p " : " %p ", data);
>>
>> You should be able to use %#p instead to let it add the prefix.
>
> That mostly works, but it doesn't round up the field size to account
> for the 2 characters in '0x'. Looks like the %p code needs to be
> tweaked to fix that.
which this change does:
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index abbabec..4c4f2ce 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -946,6 +946,8 @@ char *pointer(const char *fmt, char *buf, char
*end, void *ptr,
if (spec.field_width == -1) {
spec.field_width = 2 * sizeof(void *);
spec.flags |= ZEROPAD;
+ if (spec.flags & SPECIAL)
+ spec.field_width += 2;
}
spec.base = 16;
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] iqrdomain: Improve formatting in debugfs.
2012-04-11 14:52 ` Grant Likely
2012-04-11 14:57 ` Grant Likely
@ 2012-04-16 22:01 ` Andreas Schwab
2012-04-16 22:25 ` Grant Likely
1 sibling, 1 reply; 11+ messages in thread
From: Andreas Schwab @ 2012-04-16 22:01 UTC (permalink / raw)
To: Grant Likely
Cc: David Daney, Benjamin Herrenschmidt, Thomas Gleixner,
linux-kernel, David Daney
Grant Likely <grant.likely@secretlab.ca> writes:
> Also, gcc complains about it:
>
> /home/grant/hacking/linux/kernel/irq/irqdomain.c: In function ‘virq_debug_show’:
> /home/grant/hacking/linux/kernel/irq/irqdomain.c:670:4: warning: '#'
> flag used with ‘%p’ gnu_printf format [-Wformat]
Then it's probably not such a good idea after all.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] iqrdomain: Improve formatting in debugfs.
2012-04-16 22:01 ` Andreas Schwab
@ 2012-04-16 22:25 ` Grant Likely
0 siblings, 0 replies; 11+ messages in thread
From: Grant Likely @ 2012-04-16 22:25 UTC (permalink / raw)
To: Andreas Schwab
Cc: David Daney, Benjamin Herrenschmidt, Thomas Gleixner,
linux-kernel, David Daney
On Mon, Apr 16, 2012 at 4:01 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> Grant Likely <grant.likely@secretlab.ca> writes:
>
>> Also, gcc complains about it:
>>
>> /home/grant/hacking/linux/kernel/irq/irqdomain.c: In function ‘virq_debug_show’:
>> /home/grant/hacking/linux/kernel/irq/irqdomain.c:670:4: warning: '#'
>> flag used with ‘%p’ gnu_printf format [-Wformat]
>
> Then it's probably not such a good idea after all.
It would be a good idea if the warning can be suppressed.
g.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] iqrdomain: Improve formatting in debugfs.
2012-04-11 7:03 ` Grant Likely
2012-04-11 9:16 ` Andreas Schwab
@ 2012-04-11 15:33 ` David Daney
2012-04-12 19:20 ` Andreas Schwab
2 siblings, 0 replies; 11+ messages in thread
From: David Daney @ 2012-04-11 15:33 UTC (permalink / raw)
To: Grant Likely
Cc: Benjamin Herrenschmidt, Thomas Gleixner, linux-kernel, David Daney
On 04/11/2012 12:03 AM, Grant Likely wrote:
> On Mon, 9 Apr 2012 17:06:57 -0700, David Daney<ddaney.cavm@gmail.com> wrote:
>> From: David Daney<david.daney@cavium.com>
>>
>> The irq_domain_mapping file has some interesting output when chip_data
>> contains leading zeros:
>>
>> virq hwirq chip name chip data domain name
>> .
>> .
>> .
>> 103 0x00000 CIU2-W 0x 5e0000000000000 none
>> 104 0x00000 CIU2-W 0x 5f0000000000000 none
>> 105 0x00000 CIU2-M 0x (null) none
>> 113 0x00000 CIU2-E 0x1080000000000000 none
>> .
>> .
>> .
>>
>> I think there should be no space in there between the "0x" and the
>> rest. Also the '(null)' entry doesn't make much sense at all with a
>> "0x".
>
> Yeah, it doesn't make much sense with the '0x', but I don't want to
> supress the (null) output because it is really important to highlight
> when a domain isn't assigned to an active irq.
>
> Fixing the zero pad is trivial to fix though. Just remove the
> precision from the %p format. Here's my counter-patch:
This is good too.
FWIW:
Acked-by: David Daney <david.daney@cavium.com>
>
> ---
>
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 9310a8d..eb05e40 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -643,8 +643,8 @@ static int virq_debug_show(struct seq_file *m, void *private)
> void *data;
> int i;
>
> - seq_printf(m, "%-5s %-7s %-15s %-18s %s\n", "virq", "hwirq",
> - "chip name", "chip data", "domain name");
> + seq_printf(m, "%-5s %-7s %-15s %-*s %s\n", "irq", "hwirq",
> + "chip name", 2 * sizeof(void *) + 2, "chip data", "domain name");
>
> for (i = 1; i< nr_irqs; i++) {
> desc = irq_to_desc(i);
> @@ -667,7 +667,7 @@ static int virq_debug_show(struct seq_file *m, void *private)
> seq_printf(m, "%-15s ", p);
>
> data = irq_desc_get_chip_data(desc);
> - seq_printf(m, "0x%16p ", data);
> + seq_printf(m, data ? "0x%p " : " %p ", data);
>
> if (desc->irq_data.domain&& desc->irq_data.domain->of_node)
> p = desc->irq_data.domain->of_node->full_name;
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] iqrdomain: Improve formatting in debugfs.
2012-04-11 7:03 ` Grant Likely
2012-04-11 9:16 ` Andreas Schwab
2012-04-11 15:33 ` David Daney
@ 2012-04-12 19:20 ` Andreas Schwab
2012-04-12 20:38 ` David Daney
2 siblings, 1 reply; 11+ messages in thread
From: Andreas Schwab @ 2012-04-12 19:20 UTC (permalink / raw)
To: Grant Likely
Cc: David Daney, Benjamin Herrenschmidt, Thomas Gleixner,
linux-kernel, David Daney
Grant Likely <grant.likely@secretlab.ca> writes:
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 9310a8d..eb05e40 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -643,8 +643,8 @@ static int virq_debug_show(struct seq_file *m, void *private)
> void *data;
> int i;
>
> - seq_printf(m, "%-5s %-7s %-15s %-18s %s\n", "virq", "hwirq",
> - "chip name", "chip data", "domain name");
> + seq_printf(m, "%-5s %-7s %-15s %-*s %s\n", "irq", "hwirq",
> + "chip name", 2 * sizeof(void *) + 2, "chip data", "domain name");
kernel/irq/irqdomain.c:647:9: warning: field width specifier ‘*’ expects argument of type ‘int’, but argument 6 has type ‘long unsigned int’ [-Wformat]
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] iqrdomain: Improve formatting in debugfs.
2012-04-12 19:20 ` Andreas Schwab
@ 2012-04-12 20:38 ` David Daney
2012-04-12 20:46 ` Grant Likely
0 siblings, 1 reply; 11+ messages in thread
From: David Daney @ 2012-04-12 20:38 UTC (permalink / raw)
To: Andreas Schwab, Grant Likely
Cc: Benjamin Herrenschmidt, Thomas Gleixner, linux-kernel, David Daney
On 04/12/2012 12:20 PM, Andreas Schwab wrote:
> Grant Likely<grant.likely@secretlab.ca> writes:
>
>> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
>> index 9310a8d..eb05e40 100644
>> --- a/kernel/irq/irqdomain.c
>> +++ b/kernel/irq/irqdomain.c
>> @@ -643,8 +643,8 @@ static int virq_debug_show(struct seq_file *m, void *private)
>> void *data;
>> int i;
>>
>> - seq_printf(m, "%-5s %-7s %-15s %-18s %s\n", "virq", "hwirq",
>> - "chip name", "chip data", "domain name");
>> + seq_printf(m, "%-5s %-7s %-15s %-*s %s\n", "irq", "hwirq",
>> + "chip name", 2 * sizeof(void *) + 2, "chip data", "domain name");
>
> kernel/irq/irqdomain.c:647:9: warning: field width specifier ‘*’ expects argument of type ‘int’, but argument 6 has type ‘long unsigned int’ [-Wformat]
>
My original patch assigned the size to an int variable and then passed
that to the seq_printf(), thus avoiding this issue. I suspect something
like:
s/2 * sizeof(void *) + 2/(int)(2 * sizeof(void *) + 2)/
Would do the trick.
David Daney
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] iqrdomain: Improve formatting in debugfs.
2012-04-12 20:38 ` David Daney
@ 2012-04-12 20:46 ` Grant Likely
0 siblings, 0 replies; 11+ messages in thread
From: Grant Likely @ 2012-04-12 20:46 UTC (permalink / raw)
To: David Daney
Cc: Andreas Schwab, Benjamin Herrenschmidt, Thomas Gleixner,
linux-kernel, David Daney
On Thu, Apr 12, 2012 at 2:38 PM, David Daney <ddaney.cavm@gmail.com> wrote:
> On 04/12/2012 12:20 PM, Andreas Schwab wrote:
>>
>> Grant Likely<grant.likely@secretlab.ca> writes:
>>
>>> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
>>> index 9310a8d..eb05e40 100644
>>> --- a/kernel/irq/irqdomain.c
>>> +++ b/kernel/irq/irqdomain.c
>>> @@ -643,8 +643,8 @@ static int virq_debug_show(struct seq_file *m, void
>>> *private)
>>> void *data;
>>> int i;
>>>
>>> - seq_printf(m, "%-5s %-7s %-15s %-18s %s\n", "virq", "hwirq",
>>> - "chip name", "chip data", "domain name");
>>> + seq_printf(m, "%-5s %-7s %-15s %-*s %s\n", "irq", "hwirq",
>>> + "chip name", 2 * sizeof(void *) + 2, "chip data",
>>> "domain name");
>>
>>
>> kernel/irq/irqdomain.c:647:9: warning: field width specifier ‘*’ expects
>> argument of type ‘int’, but argument 6 has type ‘long unsigned int’
>> [-Wformat]
>>
>
> My original patch assigned the size to an int variable and then passed that
> to the seq_printf(), thus avoiding this issue. I suspect something like:
>
> s/2 * sizeof(void *) + 2/(int)(2 * sizeof(void *) + 2)/
>
> Would do the trick.
Yeah, I'll push out a fixup patch.
g
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2012-04-16 22:25 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-10 0:06 [PATCH] iqrdomain: Improve formatting in debugfs David Daney
2012-04-11 7:03 ` Grant Likely
2012-04-11 9:16 ` Andreas Schwab
2012-04-11 14:52 ` Grant Likely
2012-04-11 14:57 ` Grant Likely
2012-04-16 22:01 ` Andreas Schwab
2012-04-16 22:25 ` Grant Likely
2012-04-11 15:33 ` David Daney
2012-04-12 19:20 ` Andreas Schwab
2012-04-12 20:38 ` David Daney
2012-04-12 20:46 ` Grant Likely
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.