Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932439AbbESMaM (ORCPT ); Tue, 19 May 2015 08:30:12 -0400 Received: from relay1.mentorg.com ([192.94.38.131]:48171 "EHLO relay1.mentorg.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932329AbbESMaL (ORCPT ); Tue, 19 May 2015 08:30:11 -0400 Message-ID: <555B2CCE.50306@mentor.com> Date: Tue, 19 May 2015 15:30:06 +0300 From: Vladimir Zapolskiy User-Agent: Mozilla/5.0 (X11; Linux i686; rv:31.0) Gecko/20100101 Icedove/31.0 MIME-Version: 1.0 To: Philipp Zabel CC: =?UTF-8?B?SGVpa28gU3TDvGJuZXI=?= , Arnd Bergmann , Greg Kroah-Hartman , Subject: Re: [PATCH v3 3/9] misc: sram: use phys_addr_t instead of u32 for physical address References: <1431976122-4228-1-git-send-email-vladimir_zapolskiy@mentor.com> <1431976122-4228-4-git-send-email-vladimir_zapolskiy@mentor.com> <1432031917.15181.20.camel@pengutronix.de> In-Reply-To: <1432031917.15181.20.camel@pengutronix.de> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [137.202.0.76] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4510 Lines: 132 Hi Philipp, thank you for review. On 19.05.2015 13:38, Philipp Zabel wrote: > Hi Vladimir, > > Am Montag, den 18.05.2015, 22:08 +0300 schrieb Vladimir Zapolskiy: >> To avoid any problems on non 32-bit platforms get and store memory >> addresses under phys_addr_t type. >> >> Signed-off-by: Vladimir Zapolskiy >> --- >> Changes from v1 to v2: >> - report size of SRAM in decimal format '%zu' instead of '%zx' >> - replacement of denominator '1024' to SZ_1K requires explicit >> include of linux/sizes.h on some platforms, keep it as a number >> >> drivers/misc/sram.c | 26 +++++++++++++++----------- >> 1 file changed, 15 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c >> index 999684a..b7a3a24 100644 >> --- a/drivers/misc/sram.c >> +++ b/drivers/misc/sram.c >> @@ -41,8 +41,8 @@ struct sram_dev { >> >> struct sram_reserve { >> struct list_head list; >> - u32 start; >> - u32 size; >> + phys_addr_t start; >> + size_t size; >> }; >> >> static int sram_reserve_cmp(void *priv, struct list_head *a, >> @@ -60,7 +60,8 @@ static int sram_probe(struct platform_device *pdev) >> struct sram_dev *sram; >> struct resource *res; >> struct device_node *np = pdev->dev.of_node, *child; >> - unsigned long size, cur_start, cur_size; >> + phys_addr_t cur_start; >> + size_t size, cur_size; >> struct sram_reserve *rblocks, *block; >> struct list_head reserve_list; >> unsigned int nblocks; >> @@ -138,9 +139,9 @@ static int sram_probe(struct platform_device *pdev) >> block->size = resource_size(&child_res); >> list_add_tail(&block->list, &reserve_list); >> >> - dev_dbg(&pdev->dev, "found reserved block 0x%x-0x%x\n", >> - block->start, >> - block->start + block->size); >> + dev_dbg(&pdev->dev, "found reserved block 0x%llx-0x%llx\n", >> + (unsigned long long)block->start, > > Now that block->start is of type phys_addr_t, is there a reason not to > use %pa ? the only reason why I decided not to use %pa in the change is because the value should be passed over a pointer then. For instance right here it is possible to change '"0x%llx", (unsigned long long)block->start' to '"%pa", &block->start', however some lines below the same trick can not be done for '(unsigned long long)cur_start + cur_size' value. Among alternatives to introduce a local variable, use mixed "0x%llx" and "%pa" or use unified "0x%llx" I selected the latter one. >> + (unsigned long long)block->start + block->size); > > phys_addr_t end = block->start + block->size; > > dev_dbg(&pdev->dev, "found reserved block 0x%pa-0x%pa\n", > &block->start, &end); > >> block++; >> } >> @@ -158,8 +159,9 @@ static int sram_probe(struct platform_device *pdev) >> /* can only happen if sections overlap */ >> if (block->start < cur_start) { >> dev_err(&pdev->dev, >> - "block at 0x%x starts after current offset 0x%lx\n", >> - block->start, cur_start); >> + "block at 0x%llx starts after current offset 0x%llx\n", >> + (unsigned long long)block->start, >> + (unsigned long long)cur_start); > > dev_err(&pdev->dev, > "block at 0x%pa starts after current offset 0x%pa\n", > &block->start, &cur_start); > >> ret = -EINVAL; >> goto err_chunks; >> } >> @@ -177,8 +179,9 @@ static int sram_probe(struct platform_device *pdev) >> */ >> cur_size = block->start - cur_start; >> >> - dev_dbg(&pdev->dev, "adding chunk 0x%lx-0x%lx\n", >> - cur_start, cur_start + cur_size); >> + dev_dbg(&pdev->dev, "adding chunk 0x%llx-0x%llx\n", >> + (unsigned long long)cur_start, >> + (unsigned long long)cur_start + cur_size); > > dev_dbg(&pdev->dev, "adding chunk 0x%pa-0x%pa\n", > &cur_start, &block->start); > >> ret = gen_pool_add_virt(sram->pool, >> (unsigned long)virt_base + cur_start, >> res->start + cur_start, cur_size, -1); >> @@ -193,7 +196,8 @@ static int sram_probe(struct platform_device *pdev) >> >> platform_set_drvdata(pdev, sram); >> >> - dev_dbg(&pdev->dev, "SRAM pool: %ld KiB @ 0x%p\n", size / 1024, virt_base); >> + dev_dbg(&pdev->dev, "SRAM pool: %zu KiB @ 0x%p\n", >> + size / 1024, virt_base); >> >> return 0; > > regards > Philipp > -- With best wishes, Vladimir -- 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/