From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 04A8FEB64DC for ; Mon, 3 Jul 2023 15:53:27 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id E01E2865B6; Mon, 3 Jul 2023 17:53:25 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: by phobos.denx.de (Postfix, from userid 109) id D07B186619; Mon, 3 Jul 2023 17:53:24 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by phobos.denx.de (Postfix) with ESMTP id ED93A8619E for ; Mon, 3 Jul 2023 17:53:21 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=abdellatif.elkhlifi@arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 1C95A2F4; Mon, 3 Jul 2023 08:54:04 -0700 (PDT) Received: from e130802.arm.com (unknown [10.57.77.143]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 160FA3F663; Mon, 3 Jul 2023 08:53:19 -0700 (PDT) Date: Mon, 3 Jul 2023 16:53:14 +0100 From: Abdellatif El Khlifi To: Simon Glass , ilias.apalodimas@linaro.org Cc: nd@arm.com, u-boot@lists.denx.de Subject: Re: [PATCH v13 05/10] arm_ffa: introduce armffa command Message-ID: <20230703155314.GA324536@e130802.arm.com> References: <20230606134856.GA1871110@bill-the-cat> <20230616152817.319869-1-abdellatif.elkhlifi@arm.com> <20230616152817.319869-6-abdellatif.elkhlifi@arm.com> <20230703095507.GA54322@e130802.arm.com> <20230703120842.GA82669@e130802.arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.8 at phobos.denx.de X-Virus-Status: Clean Hi Simon, Ilias, On Mon, Jul 03, 2023 at 02:30:51PM +0100, Simon Glass wrote: ... > > > > > > + log_info("device name %s, dev %p, driver name %s, ops %p\n", > > > > > > + dev->name, > > > > > > + (void *)map_to_sysmem(dev), > > > > > > + dev->driver->name, > > > > > > + (void *)map_to_sysmem(dev->driver->ops)); > > > > > > > > > > Isn't it more useful to print the physical address map_to_sysmem() retuns? > > > > > > > > That's what map_to_sysmem() does, it returns a physical address and it's shown in the log. > > > > > > I dont have access to u-boot source right, but why do you need all > > > these void * casts then? > > > > Because map_to_sysmem() returns an 'phys_addr_t' (aka 'long long unsigned int') . However, %p expects 'void *'. > > > > Compilation warning: > > > > cmd/armffa.c:181:18: warning: format ‘%p’ expects argument of type ‘void *’, but argument 3 has type ‘phys_addr_t’ {aka ‘long long unsigned int’} [8; > > ;https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wformat=-Wformat=8;;] > > 181 | log_info("device name %s, dev %p, driver name %s, ops %p\n", > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > 182 | dev->name, > > 183 | map_to_sysmem(dev), > > | ~~~~~~~~~~~~~~~~~~ > > | | > > | phys_addr_t {aka long long unsigned int} > > You should be using %lx with map_to_sysmen() since it returns a . Use > %p for pointers. > > In general in U-Boot we use addresses (ulong) rather than pointers. > Unfortunately the phys_addr_t type can be larger than the word size. I > suggest creating a printf prefix for that type. See the top of blk.h > for an example of how to do it. Perhaps in a follow-up patch? > Sounds good. I'm gonna add it in a generic way under include/log.h if that makes sense: #ifdef CONFIG_PHYS_64BIT #define PhysAddrLength "ll" #else #define PhysAddrLength "" #endif #define PHYS_ADDR_LN "%" PhysAddrLength "x" #define PHYS_ADDR_LNU "%" PhysAddrLength "u" #endif Note: In a 32-bit platform phys_addr_t is "unsigned int", in a 64-bit platform it is "long long unsigned int" Then using it this way: log_info("device %s, addr " PHYS_ADDR_LN ", driver %s, ops " PHYS_ADDR_LN "\n", dev->name, map_to_sysmem(dev), dev->driver->name, map_to_sysmem(dev->driver->ops)); Cheers Abdellatif