From patchwork Wed Nov 27 16:24:43 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 3249011 X-Patchwork-Delegate: vinod.koul@intel.com Return-Path: X-Original-To: patchwork-dmaengine@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 1E7CB9F3A0 for ; Wed, 27 Nov 2013 16:25:02 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id DE384205DC for ; Wed, 27 Nov 2013 16:25:00 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 61B81205DE for ; Wed, 27 Nov 2013 16:24:58 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756938Ab3K0QY4 (ORCPT ); Wed, 27 Nov 2013 11:24:56 -0500 Received: from perceval.ideasonboard.com ([95.142.166.194]:54411 "EHLO perceval.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757290Ab3K0QYo (ORCPT ); Wed, 27 Nov 2013 11:24:44 -0500 Received: from avalon.localnet (unknown [46.178.50.243]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 5425D35A6A; Wed, 27 Nov 2013 17:23:57 +0100 (CET) From: Laurent Pinchart To: "Shevchenko, Andriy" Cc: Laurent Pinchart , "linux-sh@vger.kernel.org" , Guennadi Liakhovetski , "Koul, Vinod" , "dmaengine@vger.kernel.org" Subject: Re: [PATCH 1/3] DMA: shdma: Fix warnings due to improper casts and printk formats Date: Wed, 27 Nov 2013 17:24:43 +0100 Message-ID: <5898520.HHVVHa5ZWj@avalon> User-Agent: KMail/4.10.5 (Linux/3.10.17-gentoo; KDE/4.10.5; x86_64; ; ) In-Reply-To: <2028164.izn6i5W5EG@avalon> References: <1385512192-10303-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com> <1385555235.1871.16.camel@smile> <2028164.izn6i5W5EG@avalon> MIME-Version: 1.0 Sender: dmaengine-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: dmaengine@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_TVD_MIME_EPI, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Andriy, On Wednesday 27 November 2013 17:03:25 Laurent Pinchart wrote: > Hi Andy, Sorry for the misspelling. > On Wednesday 27 November 2013 12:27:41 Shevchenko, Andriy wrote: > > On Wed, 2013-11-27 at 01:29 +0100, Laurent Pinchart wrote: > > > Use the %zu printk specifier to print size_t variables, and cast > > > pointers to unsigned long instead of unsigned int where applicable. This > > > fixes warnings on platforms where pointers and/or dma_addr_t have a > > > different size than int. > > > > Few comments below. > > > > > diff --git a/drivers/dma/sh/shdma-base.c b/drivers/dma/sh/shdma-base.c > > > index 2e7b394..b37d584 100644 > > > --- a/drivers/dma/sh/shdma-base.c > > > +++ b/drivers/dma/sh/shdma-base.c > > > @@ -227,7 +227,7 @@ bool shdma_chan_filter(struct dma_chan *chan, void > > > *arg) > > > > > > struct shdma_chan *schan = to_shdma_chan(chan); > > > struct shdma_dev *sdev = to_shdma_dev(schan->dma_chan.device); > > > const struct shdma_ops *ops = sdev->ops; > > > > > > - int match = (int)arg; > > > + int match = (long)arg; > > > > As far as I understand this will lose data on 64bit platforms. > > Are you aware of it? Otherwise I don't see the advantage of such > > conversion. As far as I understand you may cast void * easily to long > > and back, otherwise you lost data. > > That's correct, but shouldn't be an issue here. The match value is a small > integer. The reason why I cast it to long is to avoid compilation warnings > > > > int ret; > > > > > > if (match < 0) > > > > > > @@ -491,9 +491,9 @@ static struct shdma_desc *shdma_add_desc(struct > > > shdma_chan *schan, > > > > > > } > > > > > > dev_dbg(schan->dev, > > > > > > - "chaining (%u/%u)@%x -> %x with %p, cookie %d\n", > > > - copy_size, *len, *src, *dst, &new->async_tx, > > > - new->async_tx.cookie); > > > + "chaining (%zu/%zu)@%lx -> %lx with %p, cookie %d\n", > > > + copy_size, *len, (unsigned long)*src, (unsigned long)*dst, > > > + &new->async_tx, new->async_tx.cookie); > > > > Instead of dancing with casting (actually for dma_addr_t it should be > > ULL type), we can extend %pa to do this job for us: > > That sounds good to me. Do you plan to submit a patch ? I'd like to get this > series upstream in v3.14. Please use the attached patch if it can help speeding things up, and replace the author and SoB line with your name and address. From df335f75387fcb57bc81259ff6893f4458d47747 Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Wed, 27 Nov 2013 17:19:04 +0100 Subject: [PATCH] lib/vsprintf.c: add %paD format specifier for dma_addr_t types 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: Laurent Pinchart --- Documentation/printk-formats.txt | 7 +++++++ lib/vsprintf.c | 14 +++++++++++--- 2 files changed, 18 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..2e3f7ea 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,17 @@ 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); + } case 'd': return dentry_name(buf, end, ptr, spec, fmt); case 'D': -- 1.8.3.2