[PATCH] lib/vsprintf.c: add %paD format specifier for dma_addr_t types

[PATCH] lib/vsprintf.c: add %paD format specifier for dma_addr_t types

[user.email]

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Add the %paD format specifier for printing a dma_addr_t type, since the
DMA address size on some platforms can vary based on build options,
regardless of the native integer type.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 Documentation/printk-formats.txt |  7 +++++++
 lib/vsprintf.c                   | 15 ++++++++++++---
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt
index 445ad74..3344ca9 100644
--- a/Documentation/printk-formats.txt
+++ b/Documentation/printk-formats.txt
@@ -63,6 +63,13 @@ Physical addresses:
 	resource_size_t) which can vary based on build options, regardless of
 	the width of the CPU data path. Passed by reference.
 
+DMA addresses:
+
+	%paD	0x01234567 or 0x0123456789abcdef
+
+	For printing a dma_addr_t type which can vary based on build options,
+	regardless of the width of the CPU data path. Passed by reference.
+
 Raw buffer as a hex string:
 	%*ph	00 01 02  ...  3f
 	%*phC	00:01:02: ... :3f
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 10909c5..12ea69d 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1219,6 +1219,7 @@ int kptr_restrict __read_mostly;
  *            The maximum supported length is 64 bytes of the input. Consider
  *            to use print_hex_dump() for the larger input.
  * - 'a' For a phys_addr_t type and its derivative types (passed by reference)
+ * - 'aD' For a dma_addr_t type (passed by reference)
  * - 'd[234]' For a dentry name (optionally 2-4 last components)
  * - 'D[234]' Same as 'd' but for a struct file
  *
@@ -1354,10 +1355,18 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 		break;
 	case 'a':
 		spec.flags |= SPECIAL | SMALL | ZEROPAD;
-		spec.field_width = sizeof(phys_addr_t) * 2 + 2;
 		spec.base = 16;
-		return number(buf, end,
-			      (unsigned long long) *((phys_addr_t *)ptr), spec);
+		switch (fmt[1]) {
+		case 'D':
+			spec.field_width = sizeof(dma_addr_t) * 2 + 2;
+			return number(buf, end,
+				(unsigned long long)*((dma_addr_t *)ptr), spec);
+		default:
+			spec.field_width = sizeof(phys_addr_t) * 2 + 2;
+			return number(buf, end,
+				(unsigned long long)*((phys_addr_t *)ptr), spec);
+		}
+		break;
 	case 'd':
 		return dentry_name(buf, end, ptr, spec, fmt);
 	case 'D':
-- 
1.8.4.4

Re: [PATCH] lib/vsprintf.c: add %paD format specifier for dma_addr_t types

[Joe Perches]

On Thu, 2013-11-28 at 13:28 +0200, user.email wrote:
> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Add the %paD format specifier for printing a dma_addr_t type, since the
> DMA address size on some platforms can vary based on build options,
> regardless of the native integer type.

OK, but I prefer this one because upper case is
sometimes used to specify upper case only output.
ala %x %X and %pUl %pUL.

https://lkml.org/lkml/2013/9/12/483

Re: [PATCH] lib/vsprintf.c: add %paD format specifier for dma_addr_t types

[Andy Shevchenko]

On Thu, 2013-11-28 at 10:32 -0800, Joe Perches wrote:
> On Thu, 2013-11-28 at 13:28 +0200, user.email wrote:
> > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > 
> > Add the %paD format specifier for printing a dma_addr_t type, since the
> > DMA address size on some platforms can vary based on build options,
> > regardless of the native integer type.
> 
> OK, but I prefer this one because upper case is
> sometimes used to specify upper case only output.
> ala %x %X and %pUl %pUL.

No objections.

> https://lkml.org/lkml/2013/9/12/483

Hmm... Still not in kernel. Do you know reason why it so?


-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH] lib/vsprintf.c: add %paD format specifier for dma_addr_t types

[Joe Perches]

On Fri, 2013-11-29 at 14:56 +0200, Andy Shevchenko wrote:
> On Thu, 2013-11-28 at 10:32 -0800, Joe Perches wrote:
> > On Thu, 2013-11-28 at 13:28 +0200, user.email wrote:
> > > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > 
> > > Add the %paD format specifier for printing a dma_addr_t type, since the
> > > DMA address size on some platforms can vary based on build options,
> > > regardless of the native integer type.
> > 
> > OK, but I prefer this one because upper case is
> > sometimes used to specify upper case only output.
> > ala %x %X and %pUl %pUL.
> 
> No objections.
> 
> > https://lkml.org/lkml/2013/9/12/483
> 
> Hmm... Still not in kernel. Do you know reason why it so?

It was just for discussion and I didn't sign or push it
to anyone else.

What's slightly funny is how far back dma_addr_t printk
discussions go.

http://lkml.indiana.edu/hypermail/linux/kernel/0202.1/1221.html

Anyway, if you think you're going to do a lot of
transforms of printk("%x", dma_addr_t) with or without
(unsigned long long) or (u64) casts, I'll submit a signed
patch to Andrew.

Let me know.

In an aside for Julia Lawall (added to cc's), I tried to
do a spatch grep for dma_addr_t (and phys_addr_t) types
that were cast to (unsigned long) or (u64) but I was
unsuccessful.  Is there something else I need to do?

$ cat dma_addr_t.cocci
@@
dma_addr_t foo;
@@

* (unsigned long long)foo;

$ cat t.c
#include <linux/types.h>
#include <linux/printk.h>

int foo(dma_addr_t a)
{
	printk("test: %llx\n", (unsigned long long)a);
}

$ spatch --all-includes --local-includes  -I include/ --sp-file dma_addr_t.cocci t.c
init_defs_builtins: /usr/local/share/coccinelle/standard.h
HANDLING: t.c
$ 

Re: [PATCH] lib/vsprintf.c: add %paD format specifier for dma_addr_t types

[Lars-Peter Clausen]

On 11/29/2013 11:10 PM, Joe Perches wrote:
> On Fri, 2013-11-29 at 14:56 +0200, Andy Shevchenko wrote:
>> On Thu, 2013-11-28 at 10:32 -0800, Joe Perches wrote:
>>> On Thu, 2013-11-28 at 13:28 +0200, user.email wrote:
>>>> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>>>>
>>>> Add the %paD format specifier for printing a dma_addr_t type, since the
>>>> DMA address size on some platforms can vary based on build options,
>>>> regardless of the native integer type.
>>>
>>> OK, but I prefer this one because upper case is
>>> sometimes used to specify upper case only output.
>>> ala %x %X and %pUl %pUL.
>>
>> No objections.
>>
>>> https://lkml.org/lkml/2013/9/12/483
>>
>> Hmm... Still not in kernel. Do you know reason why it so?
>
> It was just for discussion and I didn't sign or push it
> to anyone else.
>
> What's slightly funny is how far back dma_addr_t printk
> discussions go.
>
> http://lkml.indiana.edu/hypermail/linux/kernel/0202.1/1221.html
>
> Anyway, if you think you're going to do a lot of
> transforms of printk("%x", dma_addr_t) with or without
> (unsigned long long) or (u64) casts, I'll submit a signed
> patch to Andrew.
>
> Let me know.
>
> In an aside for Julia Lawall (added to cc's), I tried to
> do a spatch grep for dma_addr_t (and phys_addr_t) types
> that were cast to (unsigned long) or (u64) but I was
> unsuccessful.  Is there something else I need to do?
>
> $ cat dma_addr_t.cocci
> @@
> dma_addr_t foo;
> @@
>
> * (unsigned long long)foo;

Remove the semicolon, otherwise it will only match statements which are nothing 
but the cast.

>
> $ cat t.c
> #include <linux/types.h>
> #include <linux/printk.h>
>
> int foo(dma_addr_t a)
> {
> 	printk("test: %llx\n", (unsigned long long)a);
> }
>
> $ spatch --all-includes --local-includes  -I include/ --sp-file dma_addr_t.cocci t.c
> init_defs_builtins: /usr/local/share/coccinelle/standard.h
> HANDLING: t.c
> $
>
> --
> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH] lib/vsprintf.c: add %paD format specifier for dma_addr_t types

[Joe Perches]

On Fri, 2013-11-29 at 23:50 +0100, Lars-Peter Clausen wrote:
> On 11/29/2013 11:10 PM, Joe Perches wrote:
[]
> > $ cat dma_addr_t.cocci
> > @@
> > dma_addr_t foo;
> > @@
> >
> > * (unsigned long long)foo;
> 
> Remove the semicolon, otherwise it will only match statements which are nothing 
> but the cast.

Duh, thanks.

Re: [PATCH] lib/vsprintf.c: add %paD format specifier for dma_addr_t types

[Laurent Pinchart]

Hi Joe,

On Friday 29 November 2013 14:10:40 Joe Perches wrote:
> On Fri, 2013-11-29 at 14:56 +0200, Andy Shevchenko wrote:
> > On Thu, 2013-11-28 at 10:32 -0800, Joe Perches wrote:
> > > On Thu, 2013-11-28 at 13:28 +0200, user.email wrote:
> > > > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > > 
> > > > Add the %paD format specifier for printing a dma_addr_t type, since
> > > > the
> > > > DMA address size on some platforms can vary based on build options,
> > > > regardless of the native integer type.
> > > 
> > > OK, but I prefer this one because upper case is
> > > sometimes used to specify upper case only output.
> > > ala %x %X and %pUl %pUL.
> > 
> > No objections.
> > 
> > > https://lkml.org/lkml/2013/9/12/483
> > 
> > Hmm... Still not in kernel. Do you know reason why it so?
> 
> It was just for discussion and I didn't sign or push it
> to anyone else.
> 
> What's slightly funny is how far back dma_addr_t printk
> discussions go.
> 
> http://lkml.indiana.edu/hypermail/linux/kernel/0202.1/1221.html
> 
> Anyway, if you think you're going to do a lot of
> transforms of printk("%x", dma_addr_t) with or without
> (unsigned long long) or (u64) casts, I'll submit a signed
> patch to Andrew.
> 
> Let me know.

Just FYI, I've recently submitted a couple of patches that cast dma_addr_t for 
printk purpose, which resulted in Andy proposing a printk format extension 
instead.

> In an aside for Julia Lawall (added to cc's), I tried to
> do a spatch grep for dma_addr_t (and phys_addr_t) types
> that were cast to (unsigned long) or (u64) but I was
> unsuccessful.  Is there something else I need to do?
> 
> $ cat dma_addr_t.cocci
> @@
> dma_addr_t foo;
> @@
> 
> * (unsigned long long)foo;
> 
> $ cat t.c
> #include <linux/types.h>
> #include <linux/printk.h>
> 
> int foo(dma_addr_t a)
> {
> 	printk("test: %llx\n", (unsigned long long)a);
> }
> 
> $ spatch --all-includes --local-includes  -I include/ --sp-file
> dma_addr_t.cocci t.c init_defs_builtins:
> /usr/local/share/coccinelle/standard.h
> HANDLING: t.c
> $
-- 
Regards,

Laurent Pinchart

Re: [PATCH] lib/vsprintf.c: add %paD format specifier for dma_addr_t types

[Andy Shevchenko]

On Fri, 2013-11-29 at 14:10 -0800, Joe Perches wrote:
> On Fri, 2013-11-29 at 14:56 +0200, Andy Shevchenko wrote:
> > On Thu, 2013-11-28 at 10:32 -0800, Joe Perches wrote:

[]

> > > https://lkml.org/lkml/2013/9/12/483
> > 
> > Hmm... Still not in kernel. Do you know reason why it so?
> 
> It was just for discussion and I didn't sign or push it
> to anyone else.
> 
> What's slightly funny is how far back dma_addr_t printk
> discussions go.
> 
> http://lkml.indiana.edu/hypermail/linux/kernel/0202.1/1221.html

Wow, 2002!
Let's cross over a finish line in 2013.


> Anyway, if you think you're going to do a lot of
> transforms of printk("%x", dma_addr_t) with or without
> (unsigned long long) or (u64) casts, I'll submit a signed
> patch to Andrew.

I think it worse to submit even if we have no more such casting in the
future (which is the case I don't believe in).


-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH] lib/vsprintf.c: add %paD format specifier for dma_addr_t types

[Shevchenko, Andriy]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1078 bytes --]

On Mon, 2013-12-02 at 11:06 +0200, Andy Shevchenko wrote:


> > Anyway, if you think you're going to do a lot of
> > transforms of printk("%x", dma_addr_t) with or without
> > (unsigned long long) or (u64) casts, I'll submit a signed
> > patch to Andrew.
> 
> I think it worse to submit even if we have no more such casting in the
> future (which is the case I don't believe in).

s/worse/worth/

-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

[PATCH -next] vsprintf: add %pad extension for dma_addr_t use

[Joe Perches]

dma_addr_t's can be either u32 or u64 depending
on a CONFIG option.

There are a few hundred dma_addr_t's printed via
either cast to unsigned long long, unsigned long
or no cast at all.

Add %pad to be able to emit them without the cast.

Update Documentation/printk-formats.txt too.

Signed-off-by: Joe Perches <joe@perches.com>
---
 Documentation/printk-formats.txt | 11 +++++++++--
 lib/vsprintf.c                   | 33 +++++++++++++++++++++++++++------
 2 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt
index 445ad74..6f4eb32 100644
--- a/Documentation/printk-formats.txt
+++ b/Documentation/printk-formats.txt
@@ -55,14 +55,21 @@ Struct Resources:
 	For printing struct resources. The 'R' and 'r' specifiers result in a
 	printed resource with ('R') or without ('r') a decoded flags member.
 
-Physical addresses:
+Physical addresses types phys_addr_t:
 
-	%pa	0x01234567 or 0x0123456789abcdef
+	%pa[p]	0x01234567 or 0x0123456789abcdef
 
 	For printing a phys_addr_t type (and its derivatives, such as
 	resource_size_t) which can vary based on build options, regardless of
 	the width of the CPU data path. Passed by reference.
 
+DMA addresses types dma_addr_t:
+
+	%pad	0x01234567 or 0x0123456789abcdef
+
+	For printing a dma_addr_t type which can vary based on build options,
+	regardless of the width of the CPU data path. Passed by reference.
+
 Raw buffer as a hex string:
 	%*ph	00 01 02  ...  3f
 	%*phC	00:01:02: ... :3f
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 10909c5..185b6d3 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1155,6 +1155,30 @@ char *netdev_feature_string(char *buf, char *end, const u8 *addr,
 	return number(buf, end, *(const netdev_features_t *)addr, spec);
 }
 
+static noinline_for_stack
+char *address_val(char *buf, char *end, const void *addr,
+		  struct printf_spec spec, const char *fmt)
+{
+	unsigned long long num;
+
+	spec.flags |= SPECIAL | SMALL | ZEROPAD;
+	spec.base = 16;
+
+	switch (fmt[1]) {
+	case 'd':
+		num = *(const dma_addr_t *)addr;
+		spec.field_width = sizeof(dma_addr_t) * 2 + 2;
+		break;
+	case 'p':
+	default:
+		num = *(const phys_addr_t *)addr;
+		spec.field_width = sizeof(phys_addr_t) * 2 + 2;
+		break;
+	}
+
+	return number(buf, end, num, spec);
+}
+
 int kptr_restrict __read_mostly;
 
 /*
@@ -1218,7 +1242,8 @@ int kptr_restrict __read_mostly;
  *              N no separator
  *            The maximum supported length is 64 bytes of the input. Consider
  *            to use print_hex_dump() for the larger input.
- * - 'a' For a phys_addr_t type and its derivative types (passed by reference)
+ * - 'a[pd]' For address types [p] phys_addr_t, [d] dma_addr_t and derivatives
+ *           (default assumed to be phys_addr_t, passed by reference)
  * - 'd[234]' For a dentry name (optionally 2-4 last components)
  * - 'D[234]' Same as 'd' but for a struct file
  *
@@ -1353,11 +1378,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 		}
 		break;
 	case 'a':
-		spec.flags |= SPECIAL | SMALL | ZEROPAD;
-		spec.field_width = sizeof(phys_addr_t) * 2 + 2;
-		spec.base = 16;
-		return number(buf, end,
-			      (unsigned long long) *((phys_addr_t *)ptr), spec);
+		return address_val(buf, end, ptr, spec, fmt);
 	case 'd':
 		return dentry_name(buf, end, ptr, spec, fmt);
 	case 'D':