* [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() @ 2015-12-08 16:07 Masahiro Yamada 2015-12-08 16:16 ` Joe Perches 0 siblings, 1 reply; 23+ messages in thread From: Masahiro Yamada @ 2015-12-08 16:07 UTC (permalink / raw) To: devicetree Cc: Masahiro Yamada, Frank Rowand, Rob Herring, linux-kernel, Grant Likely Trivial changes suggested by checkpatch.pl. Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> --- Changes in v2: - Fix printk(KERN_DEBUG ...) too drivers/of/address.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/of/address.c b/drivers/of/address.c index 9582c57..65fafbb 100644 --- a/drivers/of/address.c +++ b/drivers/of/address.c @@ -23,7 +23,7 @@ static int __of_address_to_resource(struct device_node *dev, #ifdef DEBUG static void of_dump_addr(const char *s, const __be32 *addr, int na) { - printk(KERN_DEBUG "%s", s); + pr_debug("%s", s); while (na--) printk(" %08x", be32_to_cpu(*(addr++))); printk("\n"); @@ -597,7 +597,7 @@ static u64 __of_translate_address(struct device_node *dev, pbus = of_match_bus(parent); pbus->count_cells(dev, &pna, &pns); if (!OF_CHECK_COUNTS(pna, pns)) { - printk(KERN_ERR "prom_parse: Bad cell count for %s\n", + pr_err("prom_parse: Bad cell count for %s\n", of_node_full_name(dev)); break; } -- 1.9.1 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() 2015-12-08 16:07 [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() Masahiro Yamada @ 2015-12-08 16:16 ` Joe Perches 2015-12-08 17:03 ` Joe Perches 0 siblings, 1 reply; 23+ messages in thread From: Joe Perches @ 2015-12-08 16:16 UTC (permalink / raw) To: Masahiro Yamada, devicetree Cc: Frank Rowand, Rob Herring, linux-kernel, Grant Likely On Wed, 2015-12-09 at 01:07 +0900, Masahiro Yamada wrote: > Trivial changes suggested by checkpatch.pl. [] > diff --git a/drivers/of/address.c b/drivers/of/address.c [] > @@ -23,7 +23,7 @@ static int __of_address_to_resource(struct device_node *dev, > #ifdef DEBUG > static void of_dump_addr(const char *s, const __be32 *addr, int na) > { > - printk(KERN_DEBUG "%s", s); > + pr_debug("%s", s); > while (na--) > printk(" %08x", be32_to_cpu(*(addr++))); > printk("\n"); mixing pr_debug and printk doesn't make much sense. It might be nicer to use static void of_dumpaddr(const char *s, const __be32 *addr, int na) { #ifdef DEBUG ... #endif } to avoid the other static declaration ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() 2015-12-08 16:16 ` Joe Perches @ 2015-12-08 17:03 ` Joe Perches 2015-12-08 21:28 ` Rob Herring 0 siblings, 1 reply; 23+ messages in thread From: Joe Perches @ 2015-12-08 17:03 UTC (permalink / raw) To: Masahiro Yamada, devicetree Cc: Frank Rowand, Rob Herring, linux-kernel, Grant Likely On Tue, 2015-12-08 at 08:16 -0800, Joe Perches wrote: > On Wed, 2015-12-09 at 01:07 +0900, Masahiro Yamada wrote: > > Trivial changes suggested by checkpatch.pl. > [] > > diff --git a/drivers/of/address.c b/drivers/of/address.c > [] > > @@ -23,7 +23,7 @@ static int __of_address_to_resource(struct device_node *dev, > > #ifdef DEBUG > > static void of_dump_addr(const char *s, const __be32 *addr, int na) > > { > > - printk(KERN_DEBUG "%s", s); > > + pr_debug("%s", s); > > while (na--) > > printk(" %08x", be32_to_cpu(*(addr++))); > > printk("\n"); > > mixing pr_debug and printk doesn't make much sense. > > It might be nicer to use > > static void of_dumpaddr(const char *s, const __be32 *addr, int na) > { > #ifdef DEBUG > ... > #endif > } > > to avoid the other static declaration Or more simply: static void of_dumpaddr(const char *s, const __be32 *addr, int na) { print_hex_dump_debug(s, DUMP_PREFIX_NONE, 16, 1, addr, na * sizeof(__be32), false); } ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() 2015-12-08 17:03 ` Joe Perches @ 2015-12-08 21:28 ` Rob Herring 2015-12-09 0:03 ` Joe Perches 2015-12-09 21:59 ` [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions Joe Perches 0 siblings, 2 replies; 23+ messages in thread From: Rob Herring @ 2015-12-08 21:28 UTC (permalink / raw) To: Joe Perches Cc: Masahiro Yamada, devicetree, Frank Rowand, linux-kernel, Grant Likely On Tue, Dec 8, 2015 at 11:03 AM, Joe Perches <joe@perches.com> wrote: > On Tue, 2015-12-08 at 08:16 -0800, Joe Perches wrote: >> On Wed, 2015-12-09 at 01:07 +0900, Masahiro Yamada wrote: >> > Trivial changes suggested by checkpatch.pl. >> [] >> > diff --git a/drivers/of/address.c b/drivers/of/address.c >> [] >> > @@ -23,7 +23,7 @@ static int __of_address_to_resource(struct device_node *dev, >> > #ifdef DEBUG >> > static void of_dump_addr(const char *s, const __be32 *addr, int na) >> > { >> > - printk(KERN_DEBUG "%s", s); >> > + pr_debug("%s", s); >> > while (na--) >> > printk(" %08x", be32_to_cpu(*(addr++))); >> > printk("\n"); >> >> mixing pr_debug and printk doesn't make much sense. >> >> It might be nicer to use >> >> static void of_dumpaddr(const char *s, const __be32 *addr, int na) >> { >> #ifdef DEBUG >> ... >> #endif >> } >> >> to avoid the other static declaration > > Or more simply: > > static void of_dumpaddr(const char *s, const __be32 *addr, int na) > { > print_hex_dump_debug(s, DUMP_PREFIX_NONE, 16, 1, > addr, na * sizeof(__be32), false); > } Except that it doesn't do the endian swapping. Looking closer at this, we should just drop this hunk. So I will take v1. Rob ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() 2015-12-08 21:28 ` Rob Herring @ 2015-12-09 0:03 ` Joe Perches 2015-12-09 12:03 ` Andy Shevchenko 2015-12-09 21:59 ` [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions Joe Perches 1 sibling, 1 reply; 23+ messages in thread From: Joe Perches @ 2015-12-09 0:03 UTC (permalink / raw) To: Rob Herring, Andrew Morton Cc: Masahiro Yamada, devicetree, Frank Rowand, linux-kernel, Grant Likely On Tue, 2015-12-08 at 15:28 -0600, Rob Herring wrote: > On Tue, Dec 8, 2015 at 11:03 AM, Joe Perches <joe@perches.com> wrote: > > On Tue, 2015-12-08 at 08:16 -0800, Joe Perches wrote: > > > On Wed, 2015-12-09 at 01:07 +0900, Masahiro Yamada wrote: > > > > Trivial changes suggested by checkpatch.pl. > > > [] > > > > diff --git a/drivers/of/address.c b/drivers/of/address.c > > > [] > > > > @@ -23,7 +23,7 @@ static int __of_address_to_resource(struct device_node *dev, > > > > #ifdef DEBUG > > > > static void of_dump_addr(const char *s, const __be32 *addr, int na) > > > > { > > > > - printk(KERN_DEBUG "%s", s); > > > > + pr_debug("%s", s); > > > > while (na--) > > > > printk(" %08x", be32_to_cpu(*(addr++))); > > > > printk("\n"); [] > > static void of_dumpaddr(const char *s, const __be32 *addr, int na) > > { > > print_hex_dump_debug(s, DUMP_PREFIX_NONE, 16, 1, > > addr, na * sizeof(__be32), false); > > } > > Except that it doesn't do the endian swapping. Looking closer at this, > we should just drop this hunk. So I will take v1. Maybe endian conversions should be added to hex_dump_debug like: (probably doesn't apply. Evolution 3.18 is horrible) --- include/linux/printk.h | 7 +++++++ lib/hexdump.c | 56 +++++++++++++++++++++++++++++++++++--------------- 2 files changed, 47 insertions(+), 16 deletions(-) diff --git a/include/linux/printk.h b/include/linux/printk.h index 9729565..4be190c 100644 --- a/include/linux/printk.h +++ b/include/linux/printk.h @@ -424,6 +424,13 @@ enum { DUMP_PREFIX_ADDRESS, DUMP_PREFIX_OFFSET }; + +enum { + DUMP_TYPE_CPU = 0, + DUMP_TYPE_LE = BIT(30), + DUMP_TYPE_BE = BIT(31) +}; + extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, char *linebuf, size_t linebuflen, bool ascii); diff --git a/lib/hexdump.c b/lib/hexdump.c index 992457b..49113aa 100644 --- a/lib/hexdump.c +++ b/lib/hexdump.c @@ -81,6 +81,7 @@ EXPORT_SYMBOL(bin2hex); * @len: number of bytes in the @buf * @rowsize: number of bytes to print per line; must be 16 or 32 * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1) + * OR'd with DUMP_TYPE_BE or DUMP_TYPE_LE for endian conversions * @linebuf: where to put the converted data * @linebuflen: total size of @linebuf, including space for terminating NUL * @ascii: include ASCII after the hex output @@ -114,19 +115,20 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, int j, lx = 0; int ascii_column; int ret; + int actual_groupsize = groupsize & ~(DUMP_TYPE_LE | DUMP_TYPE_BE); if (rowsize != 16 && rowsize != 32) rowsize = 16; if (len > rowsize) /* limit to one line at a time */ len = rowsize; - if (!is_power_of_2(groupsize) || groupsize > 8) - groupsize = 1; - if ((len % groupsize) != 0) /* no mixed size output */ - groupsize = 1; + if (!is_power_of_2(actual_groupsize) || actual_groupsize > 8) + actual_groupsize = 1; + if ((len % actual_groupsize) != 0) /* no mixed size output */ + actual_groupsize = 1; - ngroups = len / groupsize; - ascii_column = rowsize * 2 + rowsize / groupsize + 1; + ngroups = len / actual_groupsize; + ascii_column = rowsize * 2 + rowsize / actual_groupsize + 1; if (!linebuflen) goto overflow1; @@ -134,35 +136,56 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, if (!len) goto nil; - if (groupsize == 8) { + if (actual_groupsize == 8) { const u64 *ptr8 = buf; for (j = 0; j < ngroups; j++) { + u64 val; + + if (groupsize & DUMP_TYPE_LE) + val = get_unaligned_le64(ptr8 + j); + else if (groupsize & DUMP_TYPE_BE) + val = get_unaligned_be64(ptr8 + j); + else + val = get_unaligned(ptr8 + j); ret = snprintf(linebuf + lx, linebuflen - lx, - "%s%16.16llx", j ? " " : "", - get_unaligned(ptr8 + j)); + "%s%16.16llx", j ? " " : "", val); if (ret >= linebuflen - lx) goto overflow1; lx += ret; } - } else if (groupsize == 4) { + } else if (actual_groupsize == 4) { const u32 *ptr4 = buf; for (j = 0; j < ngroups; j++) { + u32 val; + + if (groupsize & DUMP_TYPE_LE) + val = get_unaligned_le32(ptr4 + j); + else if (groupsize & DUMP_TYPE_BE) + val = get_unaligned_be32(ptr4 + j); + else + val = get_unaligned(ptr4 + j); ret = snprintf(linebuf + lx, linebuflen - lx, - "%s%8.8x", j ? " " : "", - get_unaligned(ptr4 + j)); + "%s%8.8x", j ? " " : "", val); if (ret >= linebuflen - lx) goto overflow1; lx += ret; } - } else if (groupsize == 2) { + } else if (actual_groupsize == 2) { const u16 *ptr2 = buf; for (j = 0; j < ngroups; j++) { + u16 val; + + if (groupsize & DUMP_TYPE_LE) + val = get_unaligned_le16(ptr2 + j); + else if (groupsize & DUMP_TYPE_BE) + val = get_unaligned_be16(ptr2 + j); + else + val = get_unaligned(ptr2 + j); ret = snprintf(linebuf + lx, linebuflen - lx, - "%s%4.4x", j ? " " : "", - get_unaligned(ptr2 + j)); + "%s%4.4x", j ? " " : "", val); if (ret >= linebuflen - lx) goto overflow1; lx += ret; @@ -203,7 +226,8 @@ nil: overflow2: linebuf[lx++] = '\0'; overflow1: - return ascii ? ascii_column + len : (groupsize * 2 + 1) * ngroups - 1; + return ascii ? ascii_column + len + : (actual_groupsize * 2 + 1) * ngroups - 1; } EXPORT_SYMBOL(hex_dump_to_buffer); ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() 2015-12-09 0:03 ` Joe Perches @ 2015-12-09 12:03 ` Andy Shevchenko 2015-12-09 19:28 ` Joe Perches 0 siblings, 1 reply; 23+ messages in thread From: Andy Shevchenko @ 2015-12-09 12:03 UTC (permalink / raw) To: Joe Perches Cc: Rob Herring, Andrew Morton, Masahiro Yamada, devicetree, Frank Rowand, linux-kernel, Grant Likely On Wed, Dec 9, 2015 at 2:03 AM, Joe Perches <joe@perches.com> wrote: > On Tue, 2015-12-08 at 15:28 -0600, Rob Herring wrote: >> On Tue, Dec 8, 2015 at 11:03 AM, Joe Perches <joe@perches.com> wrote: >> > On Tue, 2015-12-08 at 08:16 -0800, Joe Perches wrote: >> > > On Wed, 2015-12-09 at 01:07 +0900, Masahiro Yamada wrote: >> > > > Trivial changes suggested by checkpatch.pl. >> > > [] >> > > > diff --git a/drivers/of/address.c b/drivers/of/address.c >> > > [] >> > > > @@ -23,7 +23,7 @@ static int __of_address_to_resource(struct device_node *dev, >> > > > #ifdef DEBUG >> > > > static void of_dump_addr(const char *s, const __be32 *addr, int na) >> > > > { >> > > > - printk(KERN_DEBUG "%s", s); >> > > > + pr_debug("%s", s); >> > > > while (na--) >> > > > printk(" %08x", be32_to_cpu(*(addr++))); >> > > > printk("\n"); > [] >> > static void of_dumpaddr(const char *s, const __be32 *addr, int na) >> > { >> > print_hex_dump_debug(s, DUMP_PREFIX_NONE, 16, 1, >> > addr, na * sizeof(__be32), false); >> > } >> >> Except that it doesn't do the endian swapping. Looking closer at this, >> we should just drop this hunk. So I will take v1. > > Maybe endian conversions should be added to hex_dump_debug like: > (probably doesn't apply. Evolution 3.18 is horrible) Overall the idea is not bad I think, though few comments below. Besides that, please, update test_hexdump (wrt my update to the test suite [1]) to go through BE test cases. [1] http://www.spinics.net/lists/kernel/msg2139337.html > --- > include/linux/printk.h | 7 +++++++ > lib/hexdump.c | 56 +++++++++++++++++++++++++++++++++++--------------- > 2 files changed, 47 insertions(+), 16 deletions(-) > > diff --git a/include/linux/printk.h b/include/linux/printk.h > index 9729565..4be190c 100644 > --- a/include/linux/printk.h > +++ b/include/linux/printk.h > @@ -424,6 +424,13 @@ enum { > DUMP_PREFIX_ADDRESS, > DUMP_PREFIX_OFFSET > }; > + > +enum { > + DUMP_TYPE_CPU = 0, Do we need explicit enum for that? Documentation update anyway is required. > + DUMP_TYPE_LE = BIT(30), > + DUMP_TYPE_BE = BIT(31) > +}; > + > extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, > int groupsize, char *linebuf, size_t linebuflen, > bool ascii); > diff --git a/lib/hexdump.c b/lib/hexdump.c > index 992457b..49113aa 100644 > --- a/lib/hexdump.c > +++ b/lib/hexdump.c > @@ -81,6 +81,7 @@ EXPORT_SYMBOL(bin2hex); > * @len: number of bytes in the @buf > * @rowsize: number of bytes to print per line; must be 16 or 32 > * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1) > + * OR'd with DUMP_TYPE_BE or DUMP_TYPE_LE for endian conversions > * @linebuf: where to put the converted data > * @linebuflen: total size of @linebuf, including space for terminating NUL > * @ascii: include ASCII after the hex output > @@ -114,19 +115,20 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, > int j, lx = 0; > int ascii_column; > int ret; > + int actual_groupsize = groupsize & ~(DUMP_TYPE_LE | DUMP_TYPE_BE); I would rather prefer to have function parameter to be renamed. E.g. gsflags ? > > if (rowsize != 16 && rowsize != 32) > rowsize = 16; > > if (len > rowsize) /* limit to one line at a time */ > len = rowsize; > - if (!is_power_of_2(groupsize) || groupsize > 8) > - groupsize = 1; > - if ((len % groupsize) != 0) /* no mixed size output */ > - groupsize = 1; > + if (!is_power_of_2(actual_groupsize) || actual_groupsize > 8) > + actual_groupsize = 1; > + if ((len % actual_groupsize) != 0) /* no mixed size output */ > + actual_groupsize = 1; > > - ngroups = len / groupsize; > - ascii_column = rowsize * 2 + rowsize / groupsize + 1; > + ngroups = len / actual_groupsize; > + ascii_column = rowsize * 2 + rowsize / actual_groupsize + 1; > > if (!linebuflen) > goto overflow1; > @@ -134,35 +136,56 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, > if (!len) > goto nil; > > - if (groupsize == 8) { > + if (actual_groupsize == 8) { > const u64 *ptr8 = buf; > > for (j = 0; j < ngroups; j++) { > + u64 val; > + > + if (groupsize & DUMP_TYPE_LE) > + val = get_unaligned_le64(ptr8 + j); > + else if (groupsize & DUMP_TYPE_BE) > + val = get_unaligned_be64(ptr8 + j); > + else > + val = get_unaligned(ptr8 + j); > ret = snprintf(linebuf + lx, linebuflen - lx, > - "%s%16.16llx", j ? " " : "", > - get_unaligned(ptr8 + j)); > + "%s%16.16llx", j ? " " : "", val); > if (ret >= linebuflen - lx) > goto overflow1; > lx += ret; > } > - } else if (groupsize == 4) { > + } else if (actual_groupsize == 4) { > const u32 *ptr4 = buf; > > for (j = 0; j < ngroups; j++) { > + u32 val; > + > + if (groupsize & DUMP_TYPE_LE) > + val = get_unaligned_le32(ptr4 + j); > + else if (groupsize & DUMP_TYPE_BE) > + val = get_unaligned_be32(ptr4 + j); > + else > + val = get_unaligned(ptr4 + j); > ret = snprintf(linebuf + lx, linebuflen - lx, > - "%s%8.8x", j ? " " : "", > - get_unaligned(ptr4 + j)); > + "%s%8.8x", j ? " " : "", val); > if (ret >= linebuflen - lx) > goto overflow1; > lx += ret; > } > - } else if (groupsize == 2) { > + } else if (actual_groupsize == 2) { > const u16 *ptr2 = buf; > > for (j = 0; j < ngroups; j++) { > + u16 val; > + > + if (groupsize & DUMP_TYPE_LE) > + val = get_unaligned_le16(ptr2 + j); > + else if (groupsize & DUMP_TYPE_BE) > + val = get_unaligned_be16(ptr2 + j); > + else > + val = get_unaligned(ptr2 + j); > ret = snprintf(linebuf + lx, linebuflen - lx, > - "%s%4.4x", j ? " " : "", > - get_unaligned(ptr2 + j)); > + "%s%4.4x", j ? " " : "", val); > if (ret >= linebuflen - lx) > goto overflow1; > lx += ret; > @@ -203,7 +226,8 @@ nil: > overflow2: > linebuf[lx++] = '\0'; > overflow1: > - return ascii ? ascii_column + len : (groupsize * 2 + 1) * ngroups - 1; > + return ascii ? ascii_column + len > + : (actual_groupsize * 2 + 1) * ngroups - 1; > } > EXPORT_SYMBOL(hex_dump_to_buffer); > > > -- > 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/ -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() 2015-12-09 12:03 ` Andy Shevchenko @ 2015-12-09 19:28 ` Joe Perches 2015-12-09 20:02 ` Andy Shevchenko 0 siblings, 1 reply; 23+ messages in thread From: Joe Perches @ 2015-12-09 19:28 UTC (permalink / raw) To: Andy Shevchenko Cc: Rob Herring, Andrew Morton, Masahiro Yamada, devicetree, Frank Rowand, linux-kernel, Grant Likely, Rasmus Villemoes On Wed, 2015-12-09 at 14:03 +0200, Andy Shevchenko wrote: > On Wed, Dec 9, 2015 at 2:03 AM, Joe Perches <joe@perches.com> wrote: > > On Tue, 2015-12-08 at 15:28 -0600, Rob Herring wrote: > > > On Tue, Dec 8, 2015 at 11:03 AM, Joe Perches <joe@perches.com> wrote: > > > > On Tue, 2015-12-08 at 08:16 -0800, Joe Perches wrote: > > > > > On Wed, 2015-12-09 at 01:07 +0900, Masahiro Yamada wrote: > > > > > > Trivial changes suggested by checkpatch.pl. > > > > > [] > > > > > > diff --git a/drivers/of/address.c b/drivers/of/address.c > > > > > [] > > > > > > @@ -23,7 +23,7 @@ static int __of_address_to_resource(struct device_node *dev, > > > > > > #ifdef DEBUG > > > > > > static void of_dump_addr(const char *s, const __be32 *addr, int na) > > > > > > { > > > > > > - printk(KERN_DEBUG "%s", s); > > > > > > + pr_debug("%s", s); > > > > > > while (na--) > > > > > > printk(" %08x", be32_to_cpu(*(addr++))); > > > > > > printk("\n"); > > [] > > > > static void of_dumpaddr(const char *s, const __be32 *addr, int na) > > > > { > > > > print_hex_dump_debug(s, DUMP_PREFIX_NONE, 16, 1, > > > > addr, na * sizeof(__be32), false); > > > > } > > > > > > Except that it doesn't do the endian swapping. Looking closer at this, > > > we should just drop this hunk. So I will take v1. > > > > Maybe endian conversions should be added to hex_dump_debug like: > > (probably doesn't apply. Evolution 3.18 is horrible) > > Overall the idea is not bad I think, though few comments below. > Besides that, please, update test_hexdump (wrt my update to the test > suite [1]) to go through BE test cases. > > [1] http://www.spinics.net/lists/kernel/msg2139337.html I don't want to have to learn that code as it seems a bit tricky. Maybe you could do it. > > diff --git a/lib/hexdump.c b/lib/hexdump.c > > index 992457b..49113aa 100644 > > --- a/lib/hexdump.c > > +++ b/lib/hexdump.c > > @@ -81,6 +81,7 @@ EXPORT_SYMBOL(bin2hex); > > * @len: number of bytes in the @buf > > * @rowsize: number of bytes to print per line; must be 16 or 32 > > * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1) > > + * OR'd with DUMP_TYPE_BE or DUMP_TYPE_LE for endian conversions > > * @linebuf: where to put the converted data > > * @linebuflen: total size of @linebuf, including space for terminating NUL > > * @ascii: include ASCII after the hex output > > @@ -114,19 +115,20 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, > > int j, lx = 0; > > int ascii_column; > > int ret; > > + int actual_groupsize = groupsize & ~(DUMP_TYPE_LE | DUMP_TYPE_BE); > > I would rather prefer to have function parameter to be renamed. > > E.g. gsflags ? > Well, it's a bit simpler changelog. --- include/linux/printk.h | 7 +++++++ lib/hexdump.c | 39 +++++++++++++++++++++++++++++++-------- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/include/linux/printk.h b/include/linux/printk.h index 9729565..4be190c 100644 --- a/include/linux/printk.h +++ b/include/linux/printk.h @@ -424,6 +424,13 @@ enum { DUMP_PREFIX_ADDRESS, DUMP_PREFIX_OFFSET }; + +enum { + DUMP_TYPE_CPU = 0, + DUMP_TYPE_LE = BIT(30), + DUMP_TYPE_BE = BIT(31) +}; + extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, char *linebuf, size_t linebuflen, bool ascii); diff --git a/lib/hexdump.c b/lib/hexdump.c index 992457b..5b1eda70 100644 --- a/lib/hexdump.c +++ b/lib/hexdump.c @@ -80,7 +80,8 @@ EXPORT_SYMBOL(bin2hex); * @buf: data blob to dump * @len: number of bytes in the @buf * @rowsize: number of bytes to print per line; must be 16 or 32 - * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1) + * @groupflags: number of bytes to print at a time (1, 2, 4, 8; default = 1) + * OR with DUMP_TYPE_BE or DUMP_TYPE_LE for endian conversions * @linebuf: where to put the converted data * @linebuflen: total size of @linebuf, including space for terminating NUL * @ascii: include ASCII after the hex output @@ -105,7 +106,7 @@ EXPORT_SYMBOL(bin2hex); * (excluding the terminating NUL) which would have been written to the final * string if enough space had been available. */ -int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, +int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupflags, char *linebuf, size_t linebuflen, bool ascii) { const u8 *ptr = buf; @@ -114,6 +115,7 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, int j, lx = 0; int ascii_column; int ret; + int groupsize = groupflags & ~(DUMP_TYPE_LE | DUMP_TYPE_BE); if (rowsize != 16 && rowsize != 32) rowsize = 16; @@ -138,9 +140,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, const u64 *ptr8 = buf; for (j = 0; j < ngroups; j++) { + u64 val; + + if (groupflags & DUMP_TYPE_LE) + val = get_unaligned_le64(ptr8 + j); + else if (groupflags & DUMP_TYPE_BE) + val = get_unaligned_be64(ptr8 + j); + else + val = get_unaligned(ptr8 + j); ret = snprintf(linebuf + lx, linebuflen - lx, - "%s%16.16llx", j ? " " : "", - get_unaligned(ptr8 + j)); + "%s%16.16llx", j ? " " : "", val); if (ret >= linebuflen - lx) goto overflow1; lx += ret; @@ -149,9 +158,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, const u32 *ptr4 = buf; for (j = 0; j < ngroups; j++) { + u32 val; + + if (groupflags & DUMP_TYPE_LE) + val = get_unaligned_le32(ptr4 + j); + else if (groupflags & DUMP_TYPE_BE) + val = get_unaligned_be32(ptr4 + j); + else + val = get_unaligned(ptr4 + j); ret = snprintf(linebuf + lx, linebuflen - lx, - "%s%8.8x", j ? " " : "", - get_unaligned(ptr4 + j)); + "%s%8.8x", j ? " " : "", val); if (ret >= linebuflen - lx) goto overflow1; lx += ret; @@ -160,9 +176,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, const u16 *ptr2 = buf; for (j = 0; j < ngroups; j++) { + u16 val; + + if (groupflags & DUMP_TYPE_LE) + val = get_unaligned_le16(ptr2 + j); + else if (groupflags & DUMP_TYPE_BE) + val = get_unaligned_be16(ptr2 + j); + else + val = get_unaligned(ptr2 + j); ret = snprintf(linebuf + lx, linebuflen - lx, - "%s%4.4x", j ? " " : "", - get_unaligned(ptr2 + j)); + "%s%4.4x", j ? " " : "", val); if (ret >= linebuflen - lx) goto overflow1; lx += ret; ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() 2015-12-09 19:28 ` Joe Perches @ 2015-12-09 20:02 ` Andy Shevchenko 2015-12-09 20:09 ` Joe Perches 2015-12-09 20:17 ` Rasmus Villemoes 0 siblings, 2 replies; 23+ messages in thread From: Andy Shevchenko @ 2015-12-09 20:02 UTC (permalink / raw) To: Joe Perches Cc: Rob Herring, Andrew Morton, Masahiro Yamada, devicetree, Frank Rowand, linux-kernel, Grant Likely, Rasmus Villemoes On Wed, Dec 9, 2015 at 9:28 PM, Joe Perches <joe@perches.com> wrote: > On Wed, 2015-12-09 at 14:03 +0200, Andy Shevchenko wrote: >> On Wed, Dec 9, 2015 at 2:03 AM, Joe Perches <joe@perches.com> wrote: >> > On Tue, 2015-12-08 at 15:28 -0600, Rob Herring wrote: >> > > On Tue, Dec 8, 2015 at 11:03 AM, Joe Perches <joe@perches.com> wrote: >> > > > On Tue, 2015-12-08 at 08:16 -0800, Joe Perches wrote: >> > > > > On Wed, 2015-12-09 at 01:07 +0900, Masahiro Yamada wrote: >> > > > > > Trivial changes suggested by checkpatch.pl. >> > > > > [] >> > > > > > diff --git a/drivers/of/address.c b/drivers/of/address.c >> > > > > [] >> > > > > > @@ -23,7 +23,7 @@ static int __of_address_to_resource(struct device_node *dev, >> > > > > > #ifdef DEBUG >> > > > > > static void of_dump_addr(const char *s, const __be32 *addr, int na) >> > > > > > { >> > > > > > - printk(KERN_DEBUG "%s", s); >> > > > > > + pr_debug("%s", s); >> > > > > > while (na--) >> > > > > > printk(" %08x", be32_to_cpu(*(addr++))); >> > > > > > printk("\n"); >> > [] >> > > > static void of_dumpaddr(const char *s, const __be32 *addr, int na) >> > > > { >> > > > print_hex_dump_debug(s, DUMP_PREFIX_NONE, 16, 1, >> > > > addr, na * sizeof(__be32), false); >> > > > } >> > > >> > > Except that it doesn't do the endian swapping. Looking closer at this, >> > > we should just drop this hunk. So I will take v1. >> > >> > Maybe endian conversions should be added to hex_dump_debug like: >> > (probably doesn't apply. Evolution 3.18 is horrible) >> >> Overall the idea is not bad I think, though few comments below. >> Besides that, please, update test_hexdump (wrt my update to the test >> suite [1]) to go through BE test cases. >> >> [1] http://www.spinics.net/lists/kernel/msg2139337.html > > I don't want to have to learn that code as it seems a bit tricky. > Maybe you could do it. Okay, I'm fine if it at least passes existing test cases. Good to mention this in commit message then. > >> > diff --git a/lib/hexdump.c b/lib/hexdump.c >> > index 992457b..49113aa 100644 >> > --- a/lib/hexdump.c >> > +++ b/lib/hexdump.c >> > @@ -81,6 +81,7 @@ EXPORT_SYMBOL(bin2hex); >> > * @len: number of bytes in the @buf >> > * @rowsize: number of bytes to print per line; must be 16 or 32 >> > * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1) >> > + * OR'd with DUMP_TYPE_BE or DUMP_TYPE_LE for endian conversions >> > * @linebuf: where to put the converted data >> > * @linebuflen: total size of @linebuf, including space for terminating NUL >> > * @ascii: include ASCII after the hex output >> > @@ -114,19 +115,20 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, >> > int j, lx = 0; >> > int ascii_column; >> > int ret; >> > + int actual_groupsize = groupsize & ~(DUMP_TYPE_LE | DUMP_TYPE_BE); >> >> I would rather prefer to have function parameter to be renamed. >> >> E.g. gsflags ? >> > > Well, it's a bit simpler changelog. Generally I'm fine with this version, though take into account above and below. > --- > include/linux/printk.h | 7 +++++++ > lib/hexdump.c | 39 +++++++++++++++++++++++++++++++-------- > 2 files changed, 38 insertions(+), 8 deletions(-) > > diff --git a/include/linux/printk.h b/include/linux/printk.h > index 9729565..4be190c 100644 > --- a/include/linux/printk.h > +++ b/include/linux/printk.h > @@ -424,6 +424,13 @@ enum { > DUMP_PREFIX_ADDRESS, > DUMP_PREFIX_OFFSET > }; > + > +enum { > + DUMP_TYPE_CPU = 0, And still open this, do we need it? I think you may just mention in the documentation that default behaviour is CPU like. > + DUMP_TYPE_LE = BIT(30), > + DUMP_TYPE_BE = BIT(31) > +}; > + > extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, > int groupsize, char *linebuf, size_t linebuflen, Here as well to change. > bool ascii); > diff --git a/lib/hexdump.c b/lib/hexdump.c > index 992457b..5b1eda70 100644 > --- a/lib/hexdump.c > +++ b/lib/hexdump.c > @@ -80,7 +80,8 @@ EXPORT_SYMBOL(bin2hex); > * @buf: data blob to dump > * @len: number of bytes in the @buf > * @rowsize: number of bytes to print per line; must be 16 or 32 > - * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1) > + * @groupflags: number of bytes to print at a time (1, 2, 4, 8; default = 1) > + * OR with DUMP_TYPE_BE or DUMP_TYPE_LE for endian conversions Maybe specify "bitwise OR with.." ? %DUMP_… > * @linebuf: where to put the converted data > * @linebuflen: total size of @linebuf, including space for terminating NUL > * @ascii: include ASCII after the hex output > @@ -105,7 +106,7 @@ EXPORT_SYMBOL(bin2hex); > * (excluding the terminating NUL) which would have been written to the final > * string if enough space had been available. > */ > -int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, > +int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupflags, > char *linebuf, size_t linebuflen, bool ascii) > { > const u8 *ptr = buf; > @@ -114,6 +115,7 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, > int j, lx = 0; > int ascii_column; > int ret; > + int groupsize = groupflags & ~(DUMP_TYPE_LE | DUMP_TYPE_BE); > > if (rowsize != 16 && rowsize != 32) > rowsize = 16; > @@ -138,9 +140,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, > const u64 *ptr8 = buf; > > for (j = 0; j < ngroups; j++) { > + u64 val; > + > + if (groupflags & DUMP_TYPE_LE) > + val = get_unaligned_le64(ptr8 + j); > + else if (groupflags & DUMP_TYPE_BE) > + val = get_unaligned_be64(ptr8 + j); > + else > + val = get_unaligned(ptr8 + j); > ret = snprintf(linebuf + lx, linebuflen - lx, > - "%s%16.16llx", j ? " " : "", > - get_unaligned(ptr8 + j)); > + "%s%16.16llx", j ? " " : "", val); > if (ret >= linebuflen - lx) > goto overflow1; > lx += ret; > @@ -149,9 +158,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, > const u32 *ptr4 = buf; > > for (j = 0; j < ngroups; j++) { > + u32 val; > + > + if (groupflags & DUMP_TYPE_LE) > + val = get_unaligned_le32(ptr4 + j); > + else if (groupflags & DUMP_TYPE_BE) > + val = get_unaligned_be32(ptr4 + j); > + else > + val = get_unaligned(ptr4 + j); > ret = snprintf(linebuf + lx, linebuflen - lx, > - "%s%8.8x", j ? " " : "", > - get_unaligned(ptr4 + j)); > + "%s%8.8x", j ? " " : "", val); > if (ret >= linebuflen - lx) > goto overflow1; > lx += ret; > @@ -160,9 +176,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, > const u16 *ptr2 = buf; > > for (j = 0; j < ngroups; j++) { > + u16 val; > + > + if (groupflags & DUMP_TYPE_LE) > + val = get_unaligned_le16(ptr2 + j); > + else if (groupflags & DUMP_TYPE_BE) > + val = get_unaligned_be16(ptr2 + j); > + else > + val = get_unaligned(ptr2 + j); > ret = snprintf(linebuf + lx, linebuflen - lx, > - "%s%4.4x", j ? " " : "", > - get_unaligned(ptr2 + j)); > + "%s%4.4x", j ? " " : "", val); -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() 2015-12-09 20:02 ` Andy Shevchenko @ 2015-12-09 20:09 ` Joe Perches 2015-12-09 20:17 ` Rasmus Villemoes 1 sibling, 0 replies; 23+ messages in thread From: Joe Perches @ 2015-12-09 20:09 UTC (permalink / raw) To: Andy Shevchenko Cc: Rob Herring, Andrew Morton, Masahiro Yamada, devicetree, Frank Rowand, linux-kernel, Grant Likely, Rasmus Villemoes On Wed, 2015-12-09 at 22:02 +0200, Andy Shevchenko wrote: > On Wed, Dec 9, 2015 at 9:28 PM, Joe Perches <joe@perches.com> wrote: [] > > > > diff --git a/lib/hexdump.c b/lib/hexdump.c > > > > index 992457b..49113aa 100644 > > > > --- a/lib/hexdump.c > > > > +++ b/lib/hexdump.c > > > > @@ -81,6 +81,7 @@ EXPORT_SYMBOL(bin2hex); > > > > * @len: number of bytes in the @buf > > > > * @rowsize: number of bytes to print per line; must be 16 or 32 > > > > * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1) > > > > + * OR'd with DUMP_TYPE_BE or DUMP_TYPE_LE for endian conversions > > > > * @linebuf: where to put the converted data > > > > * @linebuflen: total size of @linebuf, including space for terminating NUL > > > > * @ascii: include ASCII after the hex output > > > > @@ -114,19 +115,20 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, > > > > int j, lx = 0; > > > > int ascii_column; > > > > int ret; > > > > + int actual_groupsize = groupsize & ~(DUMP_TYPE_LE | DUMP_TYPE_BE); > > > > > > I would rather prefer to have function parameter to be renamed. > > > > > > E.g. gsflags ? > > > > > > > Well, it's a bit simpler changelog. > > Generally I'm fine with this version, though take into account above and below. > > > --- > > include/linux/printk.h | 7 +++++++ > > lib/hexdump.c | 39 +++++++++++++++++++++++++++++++-------- > > 2 files changed, 38 insertions(+), 8 deletions(-) > > > > diff --git a/include/linux/printk.h b/include/linux/printk.h > > index 9729565..4be190c 100644 > > --- a/include/linux/printk.h > > +++ b/include/linux/printk.h > > @@ -424,6 +424,13 @@ enum { > > DUMP_PREFIX_ADDRESS, > > DUMP_PREFIX_OFFSET > > }; > > + > > +enum { > > + DUMP_TYPE_CPU = 0, > > And still open this, do we need it? I think you may just mention in > the documentation that default behaviour is CPU like. The only documentation I'm aware of is the kernel-doc > > + DUMP_TYPE_LE = BIT(30), > > + DUMP_TYPE_BE = BIT(31) > > +}; > > + > > extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, > > int groupsize, char *linebuf, size_t linebuflen, > > Here as well to change. Right, thanks. > > bool ascii); > > diff --git a/lib/hexdump.c b/lib/hexdump.c > > index 992457b..5b1eda70 100644 > > --- a/lib/hexdump.c > > +++ b/lib/hexdump.c > > @@ -80,7 +80,8 @@ EXPORT_SYMBOL(bin2hex); > > * @buf: data blob to dump > > * @len: number of bytes in the @buf > > * @rowsize: number of bytes to print per line; must be 16 or 32 > > - * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1) > > + * @groupflags: number of bytes to print at a time (1, 2, 4, 8; default = 1) > > + * OR with DUMP_TYPE_BE or DUMP_TYPE_LE for endian conversions > > Maybe specify "bitwise OR with.." ? ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() 2015-12-09 20:02 ` Andy Shevchenko 2015-12-09 20:09 ` Joe Perches @ 2015-12-09 20:17 ` Rasmus Villemoes 2015-12-09 20:36 ` Andy Shevchenko 1 sibling, 1 reply; 23+ messages in thread From: Rasmus Villemoes @ 2015-12-09 20:17 UTC (permalink / raw) To: Andy Shevchenko Cc: Joe Perches, Rob Herring, Andrew Morton, Masahiro Yamada, devicetree, Frank Rowand, linux-kernel, Grant Likely On Wed, Dec 09 2015, Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Wed, Dec 9, 2015 at 9:28 PM, Joe Perches <joe@perches.com> wrote: >> --- >> include/linux/printk.h | 7 +++++++ >> lib/hexdump.c | 39 +++++++++++++++++++++++++++++++-------- >> 2 files changed, 38 insertions(+), 8 deletions(-) >> >> diff --git a/include/linux/printk.h b/include/linux/printk.h >> index 9729565..4be190c 100644 >> --- a/include/linux/printk.h >> +++ b/include/linux/printk.h >> @@ -424,6 +424,13 @@ enum { >> DUMP_PREFIX_ADDRESS, >> DUMP_PREFIX_OFFSET >> }; >> + >> +enum { >> + DUMP_TYPE_CPU = 0, > > And still open this, do we need it? I think you may just mention in > the documentation that default behaviour is CPU like. I think it's best to have a name for the default. In this case it's unlikely to ever be relevant, but in general one could imagine stuff like #ifdef THIS_OR_THAT #define MY_DUMP_TYPE DUMP_TYPE_LE #else #define MY_DUMP_TYPE DUMP_TYPE_CPU #endif which is a lot more readable than if the latter was a naked 0. The feature seems fine to me. It's always been somewhat annoying that grouping changes the order of the byte values on little-endian machines (so grouping is actually a wrong name for it; it's more like "treat the bytes an array of u16/u32/u64"), and one can now get around that by specifying DUMP_TYPE_BE. Rasmus ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() 2015-12-09 20:17 ` Rasmus Villemoes @ 2015-12-09 20:36 ` Andy Shevchenko 2015-12-09 20:42 ` Joe Perches 0 siblings, 1 reply; 23+ messages in thread From: Andy Shevchenko @ 2015-12-09 20:36 UTC (permalink / raw) To: Rasmus Villemoes Cc: Joe Perches, Rob Herring, Andrew Morton, Masahiro Yamada, devicetree, Frank Rowand, linux-kernel, Grant Likely On Wed, Dec 9, 2015 at 10:17 PM, Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote: > On Wed, Dec 09 2015, Andy Shevchenko <andy.shevchenko@gmail.com> wrote: >> On Wed, Dec 9, 2015 at 9:28 PM, Joe Perches <joe@perches.com> wrote: >>> +enum { >>> + DUMP_TYPE_CPU = 0, >> >> And still open this, do we need it? I think you may just mention in >> the documentation that default behaviour is CPU like. > > I think it's best to have a name for the default. In this case it's > unlikely to ever be relevant, but in general one could imagine stuff > like > > #ifdef THIS_OR_THAT > #define MY_DUMP_TYPE DUMP_TYPE_LE > #else > #define MY_DUMP_TYPE DUMP_TYPE_CPU > #endif > > which is a lot more readable than if the latter was a naked 0. Point taken. Though _CPU suggests user to think if it's equivalent to BE or LE. I'm wondering if _NATIVE is better? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() 2015-12-09 20:36 ` Andy Shevchenko @ 2015-12-09 20:42 ` Joe Perches 0 siblings, 0 replies; 23+ messages in thread From: Joe Perches @ 2015-12-09 20:42 UTC (permalink / raw) To: Andy Shevchenko, Rasmus Villemoes Cc: Rob Herring, Andrew Morton, Masahiro Yamada, devicetree, Frank Rowand, linux-kernel, Grant Likely On Wed, 2015-12-09 at 22:36 +0200, Andy Shevchenko wrote: [] > wondering if _NATIVE is better? I think CPU is better myself and it's already in use like: include/linux/iio/iio.h-enum iio_endian { include/linux/iio/iio.h: IIO_CPU, include/linux/iio/iio.h- IIO_BE, include/linux/iio/iio.h- IIO_LE, ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions 2015-12-08 21:28 ` Rob Herring 2015-12-09 0:03 ` Joe Perches @ 2015-12-09 21:59 ` Joe Perches 2015-12-09 22:09 ` Andrew Morton ` (2 more replies) 1 sibling, 3 replies; 23+ messages in thread From: Joe Perches @ 2015-12-09 21:59 UTC (permalink / raw) To: Andrew Morton, linux-kernel Cc: Andy Shevchenko, Rasmus Villemoes, Rob Herring There are use cases for dumping buffers with specific endian types for 2, 4, and 8 byte variables in arrays. Add an enum for DUMP_TYPE_(LE|BE|CPU) to enable emitting them as such. Rename groupsize to groupflags in the functions and add documentation to the kernel-doc to describe the use of the DUMP_TYPE_ enum. Signed-off-by: Joe Perches <joe@perches.com> --- include/linux/printk.h | 23 +++++++++++++++-------- lib/hexdump.c | 49 ++++++++++++++++++++++++++++++++++++++----------- 2 files changed, 53 insertions(+), 19 deletions(-) diff --git a/include/linux/printk.h b/include/linux/printk.h index 9729565..aac2943 100644 --- a/include/linux/printk.h +++ b/include/linux/printk.h @@ -424,12 +424,19 @@ enum { DUMP_PREFIX_ADDRESS, DUMP_PREFIX_OFFSET }; + +enum { + DUMP_TYPE_CPU = 0, + DUMP_TYPE_LE = BIT(30), + DUMP_TYPE_BE = BIT(31) +}; + extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, - int groupsize, char *linebuf, size_t linebuflen, + int groupflags, char *linebuf, size_t linebuflen, bool ascii); #ifdef CONFIG_PRINTK extern void print_hex_dump(const char *level, const char *prefix_str, - int prefix_type, int rowsize, int groupsize, + int prefix_type, int rowsize, int groupflags, const void *buf, size_t len, bool ascii); #if defined(CONFIG_DYNAMIC_DEBUG) #define print_hex_dump_bytes(prefix_str, prefix_type, buf, len) \ @@ -440,7 +447,7 @@ extern void print_hex_dump_bytes(const char *prefix_str, int prefix_type, #endif /* defined(CONFIG_DYNAMIC_DEBUG) */ #else static inline void print_hex_dump(const char *level, const char *prefix_str, - int prefix_type, int rowsize, int groupsize, + int prefix_type, int rowsize, int groupflags, const void *buf, size_t len, bool ascii) { } @@ -453,17 +460,17 @@ static inline void print_hex_dump_bytes(const char *prefix_str, int prefix_type, #if defined(CONFIG_DYNAMIC_DEBUG) #define print_hex_dump_debug(prefix_str, prefix_type, rowsize, \ - groupsize, buf, len, ascii) \ + groupflags, buf, len, ascii) \ dynamic_hex_dump(prefix_str, prefix_type, rowsize, \ - groupsize, buf, len, ascii) + groupflags, buf, len, ascii) #elif defined(DEBUG) #define print_hex_dump_debug(prefix_str, prefix_type, rowsize, \ - groupsize, buf, len, ascii) \ + groupflags, buf, len, ascii) \ print_hex_dump(KERN_DEBUG, prefix_str, prefix_type, rowsize, \ - groupsize, buf, len, ascii) + groupflags, buf, len, ascii) #else static inline void print_hex_dump_debug(const char *prefix_str, int prefix_type, - int rowsize, int groupsize, + int rowsize, int groupflags, const void *buf, size_t len, bool ascii) { } diff --git a/lib/hexdump.c b/lib/hexdump.c index 992457b..f44608e 100644 --- a/lib/hexdump.c +++ b/lib/hexdump.c @@ -80,7 +80,9 @@ EXPORT_SYMBOL(bin2hex); * @buf: data blob to dump * @len: number of bytes in the @buf * @rowsize: number of bytes to print per line; must be 16 or 32 - * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1) + * @groupflags: number of bytes to print at a time (1, 2, 4, 8; default = 1) + * use a bitwise OR with %DUMP_TYPE_BE or %DUMP_TYPE_LE for endian conversions + * of @buf, otherwise %DUMP_TYPE_CPU is used * @linebuf: where to put the converted data * @linebuflen: total size of @linebuf, including space for terminating NUL * @ascii: include ASCII after the hex output @@ -105,7 +107,7 @@ EXPORT_SYMBOL(bin2hex); * (excluding the terminating NUL) which would have been written to the final * string if enough space had been available. */ -int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, +int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupflags, char *linebuf, size_t linebuflen, bool ascii) { const u8 *ptr = buf; @@ -114,6 +116,7 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, int j, lx = 0; int ascii_column; int ret; + int groupsize = groupflags & ~(DUMP_TYPE_LE | DUMP_TYPE_BE); if (rowsize != 16 && rowsize != 32) rowsize = 16; @@ -138,9 +141,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, const u64 *ptr8 = buf; for (j = 0; j < ngroups; j++) { + u64 val; + + if (groupflags & DUMP_TYPE_LE) + val = get_unaligned_le64(ptr8 + j); + else if (groupflags & DUMP_TYPE_BE) + val = get_unaligned_be64(ptr8 + j); + else + val = get_unaligned(ptr8 + j); ret = snprintf(linebuf + lx, linebuflen - lx, - "%s%16.16llx", j ? " " : "", - get_unaligned(ptr8 + j)); + "%s%16.16llx", j ? " " : "", val); if (ret >= linebuflen - lx) goto overflow1; lx += ret; @@ -149,9 +159,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, const u32 *ptr4 = buf; for (j = 0; j < ngroups; j++) { + u32 val; + + if (groupflags & DUMP_TYPE_LE) + val = get_unaligned_le32(ptr4 + j); + else if (groupflags & DUMP_TYPE_BE) + val = get_unaligned_be32(ptr4 + j); + else + val = get_unaligned(ptr4 + j); ret = snprintf(linebuf + lx, linebuflen - lx, - "%s%8.8x", j ? " " : "", - get_unaligned(ptr4 + j)); + "%s%8.8x", j ? " " : "", val); if (ret >= linebuflen - lx) goto overflow1; lx += ret; @@ -160,9 +177,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, const u16 *ptr2 = buf; for (j = 0; j < ngroups; j++) { + u16 val; + + if (groupflags & DUMP_TYPE_LE) + val = get_unaligned_le16(ptr2 + j); + else if (groupflags & DUMP_TYPE_BE) + val = get_unaligned_be16(ptr2 + j); + else + val = get_unaligned(ptr2 + j); ret = snprintf(linebuf + lx, linebuflen - lx, - "%s%4.4x", j ? " " : "", - get_unaligned(ptr2 + j)); + "%s%4.4x", j ? " " : "", val); if (ret >= linebuflen - lx) goto overflow1; lx += ret; @@ -216,7 +240,10 @@ EXPORT_SYMBOL(hex_dump_to_buffer); * @prefix_type: controls whether prefix of an offset, address, or none * is printed (%DUMP_PREFIX_OFFSET, %DUMP_PREFIX_ADDRESS, %DUMP_PREFIX_NONE) * @rowsize: number of bytes to print per line; must be 16 or 32 - * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1) + * @groupflags: number of bytes to print at a time (1, 2, 4, 8; default = 1) + * @groupflags: number of bytes to print at a time (1, 2, 4, 8; default = 1) + * use a bitwise OR with %DUMP_TYPE_BE or %DUMP_TYPE_LE for endian conversions + * of @buf, otherwise %DUMP_TYPE_CPU is used * @buf: data blob to dump * @len: number of bytes in the @buf * @ascii: include ASCII after the hex output @@ -240,7 +267,7 @@ EXPORT_SYMBOL(hex_dump_to_buffer); * ffffffff88089af0: 73727170 77767574 7b7a7978 7f7e7d7c pqrstuvwxyz{|}~. */ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type, - int rowsize, int groupsize, + int rowsize, int groupflags, const void *buf, size_t len, bool ascii) { const u8 *ptr = buf; @@ -254,7 +281,7 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type, linelen = min(remaining, rowsize); remaining -= rowsize; - hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize, + hex_dump_to_buffer(ptr + i, linelen, rowsize, groupflags, linebuf, sizeof(linebuf), ascii); switch (prefix_type) { -- 2.6.3.368.gf34be46 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions 2015-12-09 21:59 ` [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions Joe Perches @ 2015-12-09 22:09 ` Andrew Morton 2015-12-09 22:16 ` Joe Perches 2015-12-09 22:11 ` Andy Shevchenko 2015-12-10 5:55 ` kbuild test robot 2 siblings, 1 reply; 23+ messages in thread From: Andrew Morton @ 2015-12-09 22:09 UTC (permalink / raw) To: Joe Perches; +Cc: linux-kernel, Andy Shevchenko, Rasmus Villemoes, Rob Herring On Wed, 9 Dec 2015 13:59:30 -0800 Joe Perches <joe@perches.com> wrote: > There are use cases for dumping buffers with specific endian types > for 2, 4, and 8 byte variables in arrays. > > Add an enum for DUMP_TYPE_(LE|BE|CPU) to enable emitting them as such. > > Rename groupsize to groupflags in the functions and add documentation > to the kernel-doc to describe the use of the DUMP_TYPE_ enum. What are these use cases? Will patches be forthcoming to convert them? ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions 2015-12-09 22:09 ` Andrew Morton @ 2015-12-09 22:16 ` Joe Perches 2015-12-09 22:45 ` Andrew Morton 0 siblings, 1 reply; 23+ messages in thread From: Joe Perches @ 2015-12-09 22:16 UTC (permalink / raw) To: Andrew Morton Cc: linux-kernel, Andy Shevchenko, Rasmus Villemoes, Rob Herring On Wed, 2015-12-09 at 14:09 -0800, Andrew Morton wrote: > On Wed, 9 Dec 2015 13:59:30 -0800 Joe Perches <joe@perches.com> wrote: > > > There are use cases for dumping buffers with specific endian types > > for 2, 4, and 8 byte variables in arrays. > > > > Add an enum for DUMP_TYPE_(LE|BE|CPU) to enable emitting them as such. > > > > Rename groupsize to groupflags in the functions and add documentation > > to the kernel-doc to describe the use of the DUMP_TYPE_ enum. > > What are these use cases? Buffers that contain LE or BE arrays that want to be emitted by print_hex_dump. > Will patches be forthcoming to convert them? Sure, after a settling period for Masahiro's patch to be applied upstream. https://lkml.org/lkml/2015/12/8/480 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions 2015-12-09 22:16 ` Joe Perches @ 2015-12-09 22:45 ` Andrew Morton 2015-12-09 23:02 ` Joe Perches 0 siblings, 1 reply; 23+ messages in thread From: Andrew Morton @ 2015-12-09 22:45 UTC (permalink / raw) To: Joe Perches; +Cc: linux-kernel, Andy Shevchenko, Rasmus Villemoes, Rob Herring On Wed, 09 Dec 2015 14:16:36 -0800 Joe Perches <joe@perches.com> wrote: > On Wed, 2015-12-09 at 14:09 -0800, Andrew Morton wrote: > > On Wed,____9 Dec 2015 13:59:30 -0800 Joe Perches <joe@perches.com> wrote: > > > > > There are use cases for dumping buffers with specific endian types > > > for 2, 4, and 8 byte variables in arrays. > > > > > > Add an enum for DUMP_TYPE_(LE|BE|CPU) to enable emitting them as such. > > > > > > Rename groupsize to groupflags in the functions and add documentation > > > to the kernel-doc to describe the use of the DUMP_TYPE_ enum. > > > > What are these use cases? > > Buffers that contain LE or BE arrays that want > to be emitted by print_hex_dump. Make that "where are". > > Will patches be forthcoming to convert them? > > Sure, after a settling period for Masahiro's patch > to be applied upstream. > > https://lkml.org/lkml/2015/12/8/480 One single callsite under #ifdef DEBUG? Doesn't seem to warrant adding code and complexity to core library functions. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions 2015-12-09 22:45 ` Andrew Morton @ 2015-12-09 23:02 ` Joe Perches 0 siblings, 0 replies; 23+ messages in thread From: Joe Perches @ 2015-12-09 23:02 UTC (permalink / raw) To: Andrew Morton Cc: linux-kernel, Andy Shevchenko, Rasmus Villemoes, Rob Herring On Wed, 2015-12-09 at 14:45 -0800, Andrew Morton wrote: > On Wed, 09 Dec 2015 14:16:36 -0800 Joe Perches <joe@perches.com> wrote: > > > Will patches be forthcoming to convert them? > > Sure, after a settling period for Masahiro's patch > > to be applied upstream. > > > > https://lkml.org/lkml/2015/12/8/480 > > One single callsite under #ifdef DEBUG? Doesn't seem to warrant adding > code and complexity to core library functions. It was just a for-instance. It's pretty trivial code and it's quite likely there are other buffers that could be emitted that way. e.g.: look at code like: drivers/message/fusion/mptdebug.h drivers/infiniband/hw/mthca/mthca_mr.c Dunno, I didn't look very hard. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions 2015-12-09 21:59 ` [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions Joe Perches 2015-12-09 22:09 ` Andrew Morton @ 2015-12-09 22:11 ` Andy Shevchenko 2015-12-09 22:22 ` Joe Perches 2015-12-09 23:09 ` Andy Shevchenko 2015-12-10 5:55 ` kbuild test robot 2 siblings, 2 replies; 23+ messages in thread From: Andy Shevchenko @ 2015-12-09 22:11 UTC (permalink / raw) To: Joe Perches; +Cc: Andrew Morton, linux-kernel, Rasmus Villemoes, Rob Herring On Wed, Dec 9, 2015 at 11:59 PM, Joe Perches <joe@perches.com> wrote: > There are use cases for dumping buffers with specific endian types > for 2, 4, and 8 byte variables in arrays. > > Add an enum for DUMP_TYPE_(LE|BE|CPU) to enable emitting them as such. > > Rename groupsize to groupflags in the functions and add documentation > to the kernel-doc to describe the use of the DUMP_TYPE_ enum. One comment below and my tag Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com> > > Signed-off-by: Joe Perches <joe@perches.com> > --- > include/linux/printk.h | 23 +++++++++++++++-------- > lib/hexdump.c | 49 ++++++++++++++++++++++++++++++++++++++----------- > 2 files changed, 53 insertions(+), 19 deletions(-) > > diff --git a/include/linux/printk.h b/include/linux/printk.h > index 9729565..aac2943 100644 > --- a/include/linux/printk.h > +++ b/include/linux/printk.h > @@ -424,12 +424,19 @@ enum { > DUMP_PREFIX_ADDRESS, > DUMP_PREFIX_OFFSET > }; > + > +enum { > + DUMP_TYPE_CPU = 0, > + DUMP_TYPE_LE = BIT(30), > + DUMP_TYPE_BE = BIT(31) > +}; > + > extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, > - int groupsize, char *linebuf, size_t linebuflen, > + int groupflags, char *linebuf, size_t linebuflen, > bool ascii); > #ifdef CONFIG_PRINTK > extern void print_hex_dump(const char *level, const char *prefix_str, > - int prefix_type, int rowsize, int groupsize, > + int prefix_type, int rowsize, int groupflags, > const void *buf, size_t len, bool ascii); > #if defined(CONFIG_DYNAMIC_DEBUG) > #define print_hex_dump_bytes(prefix_str, prefix_type, buf, len) \ > @@ -440,7 +447,7 @@ extern void print_hex_dump_bytes(const char *prefix_str, int prefix_type, > #endif /* defined(CONFIG_DYNAMIC_DEBUG) */ > #else > static inline void print_hex_dump(const char *level, const char *prefix_str, > - int prefix_type, int rowsize, int groupsize, > + int prefix_type, int rowsize, int groupflags, > const void *buf, size_t len, bool ascii) > { > } > @@ -453,17 +460,17 @@ static inline void print_hex_dump_bytes(const char *prefix_str, int prefix_type, > > #if defined(CONFIG_DYNAMIC_DEBUG) > #define print_hex_dump_debug(prefix_str, prefix_type, rowsize, \ > - groupsize, buf, len, ascii) \ > + groupflags, buf, len, ascii) \ > dynamic_hex_dump(prefix_str, prefix_type, rowsize, \ > - groupsize, buf, len, ascii) > + groupflags, buf, len, ascii) > #elif defined(DEBUG) > #define print_hex_dump_debug(prefix_str, prefix_type, rowsize, \ > - groupsize, buf, len, ascii) \ > + groupflags, buf, len, ascii) \ > print_hex_dump(KERN_DEBUG, prefix_str, prefix_type, rowsize, \ > - groupsize, buf, len, ascii) > + groupflags, buf, len, ascii) > #else > static inline void print_hex_dump_debug(const char *prefix_str, int prefix_type, > - int rowsize, int groupsize, > + int rowsize, int groupflags, > const void *buf, size_t len, bool ascii) > { > } > diff --git a/lib/hexdump.c b/lib/hexdump.c > index 992457b..f44608e 100644 > --- a/lib/hexdump.c > +++ b/lib/hexdump.c > @@ -80,7 +80,9 @@ EXPORT_SYMBOL(bin2hex); > * @buf: data blob to dump > * @len: number of bytes in the @buf > * @rowsize: number of bytes to print per line; must be 16 or 32 > - * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1) > + * @groupflags: number of bytes to print at a time (1, 2, 4, 8; default = 1) > + * use a bitwise OR with %DUMP_TYPE_BE or %DUMP_TYPE_LE for endian conversions > + * of @buf, otherwise %DUMP_TYPE_CPU is used > * @linebuf: where to put the converted data > * @linebuflen: total size of @linebuf, including space for terminating NUL > * @ascii: include ASCII after the hex output > @@ -105,7 +107,7 @@ EXPORT_SYMBOL(bin2hex); > * (excluding the terminating NUL) which would have been written to the final > * string if enough space had been available. > */ > -int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, > +int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupflags, > char *linebuf, size_t linebuflen, bool ascii) > { > const u8 *ptr = buf; > @@ -114,6 +116,7 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, > int j, lx = 0; > int ascii_column; > int ret; > + int groupsize = groupflags & ~(DUMP_TYPE_LE | DUMP_TYPE_BE); > > if (rowsize != 16 && rowsize != 32) > rowsize = 16; > @@ -138,9 +141,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, > const u64 *ptr8 = buf; > > for (j = 0; j < ngroups; j++) { > + u64 val; > + > + if (groupflags & DUMP_TYPE_LE) > + val = get_unaligned_le64(ptr8 + j); > + else if (groupflags & DUMP_TYPE_BE) > + val = get_unaligned_be64(ptr8 + j); > + else > + val = get_unaligned(ptr8 + j); > ret = snprintf(linebuf + lx, linebuflen - lx, > - "%s%16.16llx", j ? " " : "", > - get_unaligned(ptr8 + j)); > + "%s%16.16llx", j ? " " : "", val); > if (ret >= linebuflen - lx) > goto overflow1; > lx += ret; > @@ -149,9 +159,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, > const u32 *ptr4 = buf; > > for (j = 0; j < ngroups; j++) { > + u32 val; > + > + if (groupflags & DUMP_TYPE_LE) > + val = get_unaligned_le32(ptr4 + j); > + else if (groupflags & DUMP_TYPE_BE) > + val = get_unaligned_be32(ptr4 + j); > + else > + val = get_unaligned(ptr4 + j); > ret = snprintf(linebuf + lx, linebuflen - lx, > - "%s%8.8x", j ? " " : "", > - get_unaligned(ptr4 + j)); > + "%s%8.8x", j ? " " : "", val); > if (ret >= linebuflen - lx) > goto overflow1; > lx += ret; > @@ -160,9 +177,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, > const u16 *ptr2 = buf; > > for (j = 0; j < ngroups; j++) { > + u16 val; > + > + if (groupflags & DUMP_TYPE_LE) > + val = get_unaligned_le16(ptr2 + j); > + else if (groupflags & DUMP_TYPE_BE) > + val = get_unaligned_be16(ptr2 + j); > + else > + val = get_unaligned(ptr2 + j); > ret = snprintf(linebuf + lx, linebuflen - lx, > - "%s%4.4x", j ? " " : "", > - get_unaligned(ptr2 + j)); > + "%s%4.4x", j ? " " : "", val); > if (ret >= linebuflen - lx) > goto overflow1; > lx += ret; > @@ -216,7 +240,10 @@ EXPORT_SYMBOL(hex_dump_to_buffer); > * @prefix_type: controls whether prefix of an offset, address, or none > * is printed (%DUMP_PREFIX_OFFSET, %DUMP_PREFIX_ADDRESS, %DUMP_PREFIX_NONE) > * @rowsize: number of bytes to print per line; must be 16 or 32 > - * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1) > + * @groupflags: number of bytes to print at a time (1, 2, 4, 8; default = 1) > + * @groupflags: number of bytes to print at a time (1, 2, 4, 8; default = 1) Duplicate. > + * use a bitwise OR with %DUMP_TYPE_BE or %DUMP_TYPE_LE for endian conversions > + * of @buf, otherwise %DUMP_TYPE_CPU is used > * @buf: data blob to dump > * @len: number of bytes in the @buf > * @ascii: include ASCII after the hex output > @@ -240,7 +267,7 @@ EXPORT_SYMBOL(hex_dump_to_buffer); > * ffffffff88089af0: 73727170 77767574 7b7a7978 7f7e7d7c pqrstuvwxyz{|}~. > */ > void print_hex_dump(const char *level, const char *prefix_str, int prefix_type, > - int rowsize, int groupsize, > + int rowsize, int groupflags, > const void *buf, size_t len, bool ascii) > { > const u8 *ptr = buf; > @@ -254,7 +281,7 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type, > linelen = min(remaining, rowsize); > remaining -= rowsize; > > - hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize, > + hex_dump_to_buffer(ptr + i, linelen, rowsize, groupflags, > linebuf, sizeof(linebuf), ascii); > > switch (prefix_type) { > -- > 2.6.3.368.gf34be46 > -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions 2015-12-09 22:11 ` Andy Shevchenko @ 2015-12-09 22:22 ` Joe Perches 2015-12-09 23:09 ` Andy Shevchenko 1 sibling, 0 replies; 23+ messages in thread From: Joe Perches @ 2015-12-09 22:22 UTC (permalink / raw) To: Andy Shevchenko Cc: Andrew Morton, linux-kernel, Rasmus Villemoes, Rob Herring On Thu, 2015-12-10 at 00:11 +0200, Andy Shevchenko wrote: > On Wed, Dec 9, 2015 at 11:59 PM, Joe Perches <joe@perches.com> wrote: [] > > diff --git a/lib/hexdump.c b/lib/hexdump.c [] > > @@ -216,7 +240,10 @@ EXPORT_SYMBOL(hex_dump_to_buffer); > > * @prefix_type: controls whether prefix of an offset, address, or none > > * is printed (%DUMP_PREFIX_OFFSET, %DUMP_PREFIX_ADDRESS, %DUMP_PREFIX_NONE) > > * @rowsize: number of bytes to print per line; must be 16 or 32 > > - * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1) > > + * @groupflags: number of bytes to print at a time (1, 2, 4, 8; default = 1) > > + * @groupflags: number of bytes to print at a time (1, 2, 4, 8; default = 1) > > Duplicate. Ick. Andrew, can you delete one line please or do you want a resend? ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions 2015-12-09 22:11 ` Andy Shevchenko 2015-12-09 22:22 ` Joe Perches @ 2015-12-09 23:09 ` Andy Shevchenko 2015-12-09 23:15 ` Joe Perches 1 sibling, 1 reply; 23+ messages in thread From: Andy Shevchenko @ 2015-12-09 23:09 UTC (permalink / raw) To: Joe Perches; +Cc: Andrew Morton, linux-kernel, Rasmus Villemoes, Rob Herring On Thu, Dec 10, 2015 at 12:11 AM, Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Wed, Dec 9, 2015 at 11:59 PM, Joe Perches <joe@perches.com> wrote: >> There are use cases for dumping buffers with specific endian types >> for 2, 4, and 8 byte variables in arrays. >> >> Add an enum for DUMP_TYPE_(LE|BE|CPU) to enable emitting them as such. >> >> Rename groupsize to groupflags in the functions and add documentation >> to the kernel-doc to describe the use of the DUMP_TYPE_ enum. > > One comment below and my tag > Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com> Couple of additional notes. (Tag still in power) 1. There is similar to print_hex_dump() function in seq_file. Separate patch? 2. What to do with ASCII part? Shall we print it as byte stream? And one more style comment below. > >> >> Signed-off-by: Joe Perches <joe@perches.com> >> --- >> include/linux/printk.h | 23 +++++++++++++++-------- >> lib/hexdump.c | 49 ++++++++++++++++++++++++++++++++++++++----------- >> 2 files changed, 53 insertions(+), 19 deletions(-) >> >> diff --git a/include/linux/printk.h b/include/linux/printk.h > >> index 9729565..aac2943 100644 >> --- a/include/linux/printk.h >> +++ b/include/linux/printk.h >> @@ -424,12 +424,19 @@ enum { >> DUMP_PREFIX_ADDRESS, >> DUMP_PREFIX_OFFSET >> }; >> + >> +enum { >> + DUMP_TYPE_CPU = 0, >> + DUMP_TYPE_LE = BIT(30), >> + DUMP_TYPE_BE = BIT(31) >> +}; >> + >> extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, >> - int groupsize, char *linebuf, size_t linebuflen, >> + int groupflags, char *linebuf, size_t linebuflen, >> bool ascii); >> #ifdef CONFIG_PRINTK >> extern void print_hex_dump(const char *level, const char *prefix_str, >> - int prefix_type, int rowsize, int groupsize, >> + int prefix_type, int rowsize, int groupflags, >> const void *buf, size_t len, bool ascii); >> #if defined(CONFIG_DYNAMIC_DEBUG) >> #define print_hex_dump_bytes(prefix_str, prefix_type, buf, len) \ >> @@ -440,7 +447,7 @@ extern void print_hex_dump_bytes(const char *prefix_str, int prefix_type, >> #endif /* defined(CONFIG_DYNAMIC_DEBUG) */ >> #else >> static inline void print_hex_dump(const char *level, const char *prefix_str, >> - int prefix_type, int rowsize, int groupsize, >> + int prefix_type, int rowsize, int groupflags, >> const void *buf, size_t len, bool ascii) >> { >> } >> @@ -453,17 +460,17 @@ static inline void print_hex_dump_bytes(const char *prefix_str, int prefix_type, >> >> #if defined(CONFIG_DYNAMIC_DEBUG) >> #define print_hex_dump_debug(prefix_str, prefix_type, rowsize, \ >> - groupsize, buf, len, ascii) \ >> + groupflags, buf, len, ascii) \ >> dynamic_hex_dump(prefix_str, prefix_type, rowsize, \ >> - groupsize, buf, len, ascii) >> + groupflags, buf, len, ascii) >> #elif defined(DEBUG) >> #define print_hex_dump_debug(prefix_str, prefix_type, rowsize, \ >> - groupsize, buf, len, ascii) \ >> + groupflags, buf, len, ascii) \ >> print_hex_dump(KERN_DEBUG, prefix_str, prefix_type, rowsize, \ >> - groupsize, buf, len, ascii) >> + groupflags, buf, len, ascii) >> #else >> static inline void print_hex_dump_debug(const char *prefix_str, int prefix_type, >> - int rowsize, int groupsize, >> + int rowsize, int groupflags, >> const void *buf, size_t len, bool ascii) >> { >> } >> diff --git a/lib/hexdump.c b/lib/hexdump.c >> index 992457b..f44608e 100644 >> --- a/lib/hexdump.c >> +++ b/lib/hexdump.c >> @@ -80,7 +80,9 @@ EXPORT_SYMBOL(bin2hex); >> * @buf: data blob to dump >> * @len: number of bytes in the @buf >> * @rowsize: number of bytes to print per line; must be 16 or 32 >> - * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1) >> + * @groupflags: number of bytes to print at a time (1, 2, 4, 8; default = 1) >> + * use a bitwise OR with %DUMP_TYPE_BE or %DUMP_TYPE_LE for endian conversions >> + * of @buf, otherwise %DUMP_TYPE_CPU is used >> * @linebuf: where to put the converted data >> * @linebuflen: total size of @linebuf, including space for terminating NUL >> * @ascii: include ASCII after the hex output >> @@ -105,7 +107,7 @@ EXPORT_SYMBOL(bin2hex); >> * (excluding the terminating NUL) which would have been written to the final >> * string if enough space had been available. >> */ >> -int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, >> +int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupflags, >> char *linebuf, size_t linebuflen, bool ascii) >> { >> const u8 *ptr = buf; >> @@ -114,6 +116,7 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, >> int j, lx = 0; >> int ascii_column; >> int ret; >> + int groupsize = groupflags & ~(DUMP_TYPE_LE | DUMP_TYPE_BE); Can you move this upper to first or second position in the definition block? >> >> if (rowsize != 16 && rowsize != 32) >> rowsize = 16; >> @@ -138,9 +141,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, >> const u64 *ptr8 = buf; >> >> for (j = 0; j < ngroups; j++) { >> + u64 val; >> + >> + if (groupflags & DUMP_TYPE_LE) >> + val = get_unaligned_le64(ptr8 + j); >> + else if (groupflags & DUMP_TYPE_BE) >> + val = get_unaligned_be64(ptr8 + j); >> + else >> + val = get_unaligned(ptr8 + j); >> ret = snprintf(linebuf + lx, linebuflen - lx, >> - "%s%16.16llx", j ? " " : "", >> - get_unaligned(ptr8 + j)); >> + "%s%16.16llx", j ? " " : "", val); >> if (ret >= linebuflen - lx) >> goto overflow1; >> lx += ret; >> @@ -149,9 +159,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, >> const u32 *ptr4 = buf; >> >> for (j = 0; j < ngroups; j++) { >> + u32 val; >> + >> + if (groupflags & DUMP_TYPE_LE) >> + val = get_unaligned_le32(ptr4 + j); >> + else if (groupflags & DUMP_TYPE_BE) >> + val = get_unaligned_be32(ptr4 + j); >> + else >> + val = get_unaligned(ptr4 + j); >> ret = snprintf(linebuf + lx, linebuflen - lx, >> - "%s%8.8x", j ? " " : "", >> - get_unaligned(ptr4 + j)); >> + "%s%8.8x", j ? " " : "", val); >> if (ret >= linebuflen - lx) >> goto overflow1; >> lx += ret; >> @@ -160,9 +177,16 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize, >> const u16 *ptr2 = buf; >> >> for (j = 0; j < ngroups; j++) { >> + u16 val; >> + >> + if (groupflags & DUMP_TYPE_LE) >> + val = get_unaligned_le16(ptr2 + j); >> + else if (groupflags & DUMP_TYPE_BE) >> + val = get_unaligned_be16(ptr2 + j); >> + else >> + val = get_unaligned(ptr2 + j); >> ret = snprintf(linebuf + lx, linebuflen - lx, >> - "%s%4.4x", j ? " " : "", >> - get_unaligned(ptr2 + j)); >> + "%s%4.4x", j ? " " : "", val); >> if (ret >= linebuflen - lx) >> goto overflow1; >> lx += ret; >> @@ -216,7 +240,10 @@ EXPORT_SYMBOL(hex_dump_to_buffer); >> * @prefix_type: controls whether prefix of an offset, address, or none >> * is printed (%DUMP_PREFIX_OFFSET, %DUMP_PREFIX_ADDRESS, %DUMP_PREFIX_NONE) >> * @rowsize: number of bytes to print per line; must be 16 or 32 >> - * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1) >> + * @groupflags: number of bytes to print at a time (1, 2, 4, 8; default = 1) > >> + * @groupflags: number of bytes to print at a time (1, 2, 4, 8; default = 1) > > Duplicate. > >> + * use a bitwise OR with %DUMP_TYPE_BE or %DUMP_TYPE_LE for endian conversions >> + * of @buf, otherwise %DUMP_TYPE_CPU is used >> * @buf: data blob to dump >> * @len: number of bytes in the @buf >> * @ascii: include ASCII after the hex output >> @@ -240,7 +267,7 @@ EXPORT_SYMBOL(hex_dump_to_buffer); >> * ffffffff88089af0: 73727170 77767574 7b7a7978 7f7e7d7c pqrstuvwxyz{|}~. >> */ >> void print_hex_dump(const char *level, const char *prefix_str, int prefix_type, >> - int rowsize, int groupsize, >> + int rowsize, int groupflags, >> const void *buf, size_t len, bool ascii) >> { >> const u8 *ptr = buf; >> @@ -254,7 +281,7 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type, >> linelen = min(remaining, rowsize); >> remaining -= rowsize; >> >> - hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize, >> + hex_dump_to_buffer(ptr + i, linelen, rowsize, groupflags, >> linebuf, sizeof(linebuf), ascii); >> >> switch (prefix_type) { >> -- >> 2.6.3.368.gf34be46 >> > > > > -- > With Best Regards, > Andy Shevchenko -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions 2015-12-09 23:09 ` Andy Shevchenko @ 2015-12-09 23:15 ` Joe Perches 0 siblings, 0 replies; 23+ messages in thread From: Joe Perches @ 2015-12-09 23:15 UTC (permalink / raw) To: Andy Shevchenko Cc: Andrew Morton, linux-kernel, Rasmus Villemoes, Rob Herring On Thu, 2015-12-10 at 01:09 +0200, Andy Shevchenko wrote: > On Thu, Dec 10, 2015 at 12:11 AM, Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: > > On Wed, Dec 9, 2015 at 11:59 PM, Joe Perches <joe@perches.com> wrote: > > > There are use cases for dumping buffers with specific endian types > > > for 2, 4, and 8 byte variables in arrays. > > > > > > Add an enum for DUMP_TYPE_(LE|BE|CPU) to enable emitting them as such. > > > > > > Rename groupsize to groupflags in the functions and add documentation > > > to the kernel-doc to describe the use of the DUMP_TYPE_ enum. > > > > One comment below and my tag > > Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com> > > Couple of additional notes. (Tag still in power) > > 1. There is similar to print_hex_dump() function in seq_file. Separate patch? Let's see if anyone wants this one first. > 2. What to do with ASCII part? Shall we print it as byte stream? It's still emitted as a byte stream. > And one more style comment below. That one's a nit I don't care about. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions 2015-12-09 21:59 ` [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions Joe Perches 2015-12-09 22:09 ` Andrew Morton 2015-12-09 22:11 ` Andy Shevchenko @ 2015-12-10 5:55 ` kbuild test robot 2015-12-10 6:20 ` Joe Perches 2 siblings, 1 reply; 23+ messages in thread From: kbuild test robot @ 2015-12-10 5:55 UTC (permalink / raw) To: Joe Perches Cc: kbuild-all, Andrew Morton, linux-kernel, Andy Shevchenko, Rasmus Villemoes, Rob Herring Hi Joe, [auto build test WARNING on v4.4-rc4] [also build test WARNING on next-20151209] url: https://github.com/0day-ci/linux/commits/Joe-Perches/hexdump-Add-ability-to-do-endian-conversions-in-print_hex_dump-functions/20151210-060244 reproduce: # apt-get install sparse make ARCH=x86_64 allmodconfig make C=1 CF=-D__CHECK_ENDIAN__ sparse warnings: (new ones prefixed by >>) include/linux/compiler.h:228:8: sparse: attribute 'no_sanitize_address': unknown attribute include/linux/printk.h:430:24: sparse: undefined identifier 'BIT' >> include/linux/printk.h:430:27: sparse: bad constant expression type include/linux/printk.h:431:24: sparse: undefined identifier 'BIT' include/linux/printk.h:431:27: sparse: bad constant expression type In file included from lib/decompress.c:19:0: include/linux/printk.h:430:17: error: implicit declaration of function 'BIT' [-Werror=implicit-function-declaration] DUMP_TYPE_LE = BIT(30), ^ include/linux/printk.h:430:2: error: enumerator value for 'DUMP_TYPE_LE' is not an integer constant DUMP_TYPE_LE = BIT(30), ^ include/linux/printk.h:432:1: error: enumerator value for 'DUMP_TYPE_BE' is not an integer constant }; ^ cc1: some warnings being treated as errors vim +430 include/linux/printk.h 414 printk_ratelimited(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__) 415 #else 416 #define pr_debug_ratelimited(fmt, ...) \ 417 no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__) 418 #endif 419 420 extern const struct file_operations kmsg_fops; 421 422 enum { 423 DUMP_PREFIX_NONE, 424 DUMP_PREFIX_ADDRESS, 425 DUMP_PREFIX_OFFSET 426 }; 427 428 enum { 429 DUMP_TYPE_CPU = 0, > 430 DUMP_TYPE_LE = BIT(30), 431 DUMP_TYPE_BE = BIT(31) 432 }; 433 434 extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, 435 int groupflags, char *linebuf, size_t linebuflen, 436 bool ascii); 437 #ifdef CONFIG_PRINTK 438 extern void print_hex_dump(const char *level, const char *prefix_str, --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions 2015-12-10 5:55 ` kbuild test robot @ 2015-12-10 6:20 ` Joe Perches 0 siblings, 0 replies; 23+ messages in thread From: Joe Perches @ 2015-12-10 6:20 UTC (permalink / raw) To: kbuild test robot Cc: kbuild-all, Andrew Morton, linux-kernel, Andy Shevchenko, Rasmus Villemoes, Rob Herring On Thu, 2015-12-10 at 13:55 +0800, kbuild test robot wrote: > Hi Joe, Hello Fengguang. Thanks for the report. I'll fix and resubmit. > > > include/linux/printk.h:430:24: sparse: undefined identifier 'BIT' > > > include/linux/printk.h:430:27: sparse: bad constant expression type ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2015-12-10 6:20 UTC | newest] Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-12-08 16:07 [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() Masahiro Yamada 2015-12-08 16:16 ` Joe Perches 2015-12-08 17:03 ` Joe Perches 2015-12-08 21:28 ` Rob Herring 2015-12-09 0:03 ` Joe Perches 2015-12-09 12:03 ` Andy Shevchenko 2015-12-09 19:28 ` Joe Perches 2015-12-09 20:02 ` Andy Shevchenko 2015-12-09 20:09 ` Joe Perches 2015-12-09 20:17 ` Rasmus Villemoes 2015-12-09 20:36 ` Andy Shevchenko 2015-12-09 20:42 ` Joe Perches 2015-12-09 21:59 ` [PATCH] hexdump: Add ability to do endian conversions in print_hex_dump functions Joe Perches 2015-12-09 22:09 ` Andrew Morton 2015-12-09 22:16 ` Joe Perches 2015-12-09 22:45 ` Andrew Morton 2015-12-09 23:02 ` Joe Perches 2015-12-09 22:11 ` Andy Shevchenko 2015-12-09 22:22 ` Joe Perches 2015-12-09 23:09 ` Andy Shevchenko 2015-12-09 23:15 ` Joe Perches 2015-12-10 5:55 ` kbuild test robot 2015-12-10 6:20 ` Joe Perches
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).