From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753916AbbLIT2q (ORCPT ); Wed, 9 Dec 2015 14:28:46 -0500 Received: from smtprelay0133.hostedemail.com ([216.40.44.133]:35545 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753394AbbLIT2n (ORCPT ); Wed, 9 Dec 2015 14:28:43 -0500 X-Session-Marker: 6A6F6540706572636865732E636F6D X-Spam-Summary: 50,0,0,,d41d8cd98f00b204,joe@perches.com,:::::::::::::::::,RULES_HIT:2:41:355:379:541:599:800:960:967:973:988:989:1260:1263:1277:1311:1313:1314:1345:1359:1373:1437:1515:1516:1518:1535:1593:1594:1605:1730:1747:1777:1792:2393:2525:2553:2560:2563:2682:2685:2693:2828:2859:2895:2933:2937:2939:2942:2945:2947:2951:2954:3022:3138:3139:3140:3141:3142:3622:3743:3865:3866:3867:3870:3871:3872:3873:3874:3934:3936:3938:3941:3944:3947:3950:3953:3956:3959:4050:4119:4250:4321:5007:6117:6119:6261:6690:7903:7904:8599:8603:9025:9388:10004:10848:11026:11232:11473:11658:11783:11889:11914:12043:12291:12295:12296:12438:12517:12519:12555:12683:12740:12776:13894:14659:21080:30003:30012:30034:30054:30056:30070:30090:30091,0,RBL:none,CacheIP:none,Bayesian:0.5,0.5,0.5,Netcheck:none,DomainCache:0,MSF:not bulk,SPF:fn,MSBL:0,DNSBL:none,Custom_rules:0:0:0,LFtime:3,LUA_SUMMARY:none X-HE-Tag: pull46_2ea168901313a X-Filterd-Recvd-Size: 8324 Message-ID: <1449689319.25389.28.camel@perches.com> Subject: Re: [PATCH v2] of/address: replace printk() with pr_debug() / pr_err() From: Joe Perches To: Andy Shevchenko Cc: Rob Herring , Andrew Morton , Masahiro Yamada , "devicetree@vger.kernel.org" , Frank Rowand , "linux-kernel@vger.kernel.org" , Grant Likely , Rasmus Villemoes Date: Wed, 09 Dec 2015 11:28:39 -0800 In-Reply-To: References: <1449590868-19070-1-git-send-email-yamada.masahiro@socionext.com> <1449591386.3315.2.camel@perches.com> <1449594231.13275.2.camel@perches.com> <1449619439.18646.20.camel@perches.com> Content-Type: text/plain; charset="ISO-8859-1" X-Mailer: Evolution 3.18.2-0ubuntu2 Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2015-12-09 at 14:03 +0200, Andy Shevchenko wrote: > On Wed, Dec 9, 2015 at 2:03 AM, Joe Perches wrote: > > On Tue, 2015-12-08 at 15:28 -0600, Rob Herring wrote: > > > On Tue, Dec 8, 2015 at 11:03 AM, Joe Perches 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;