Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753660AbbDMJWR (ORCPT ); Mon, 13 Apr 2015 05:22:17 -0400 Received: from userp1040.oracle.com ([156.151.31.81]:45242 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752513AbbDMJWP (ORCPT ); Mon, 13 Apr 2015 05:22:15 -0400 Date: Mon, 13 Apr 2015 12:21:53 +0300 From: Dan Carpenter To: Peter Senna Tschudin Cc: driverdevel , Greg Kroah-Hartman , Jun Tian , "linux-kernel@vger.kernel.org" , Garret Kelly , Octavian Purdila , Nick Kralevich , Alan Subject: Re: [PATCH] Fix pointer cast for 32 bits arch Message-ID: <20150413092152.GU10964@mwanda> References: <1428830803-30896-1-git-send-email-peter.senna@gmail.com> <20150413091019.GS10964@mwanda> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-Source-IP: aserv0022.oracle.com [141.146.126.234] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3810 Lines: 88 On Mon, Apr 13, 2015 at 11:16:49AM +0200, Peter Senna Tschudin wrote: > On Mon, Apr 13, 2015 at 11:10 AM, Dan Carpenter > wrote: > > On Sun, Apr 12, 2015 at 11:26:43AM +0200, Peter Senna Tschudin wrote: > >> Sparse compalins about casting void * to u64 on i386. > >> Change the cast to resource_size_t. > >> > >> Signed-off-by: Peter Senna Tschudin > >> --- > >> > >> Tested by compilation only. Tested for x86 and x86_64. > >> > >> drivers/staging/goldfish/goldfish_nand.c | 3 ++- > >> include/linux/goldfish.h | 2 +- > >> 2 files changed, 3 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/staging/goldfish/goldfish_nand.c b/drivers/staging/goldfish/goldfish_nand.c > >> index d68f216..738fdc4 100644 > >> --- a/drivers/staging/goldfish/goldfish_nand.c > >> +++ b/drivers/staging/goldfish/goldfish_nand.c > >> @@ -87,7 +87,8 @@ static u32 goldfish_nand_cmd(struct mtd_info *mtd, enum nand_cmd cmd, > >> writel((u32)(addr >> 32), base + NAND_ADDR_HIGH); > >> writel((u32)addr, base + NAND_ADDR_LOW); > >> writel(len, base + NAND_TRANSFER_SIZE); > >> - gf_write64((u64)ptr, base + NAND_DATA, base + NAND_DATA_HIGH); > >> + gf_write64((resource_size_t)ptr, base + NAND_DATA, > >> + base + NAND_DATA_HIGH); > > > > write64 is a misleading name because it only writes 32 bits on 32 bit > > systems. > > > > Are you sure it's a resource_size_t? If that's really true then this > > code needs a lot more work. The ifdef in gf_write64() should be > > CONFIG_PHYS_ADDR_T_64BIT and ops->datbuf needs to updated. It would be > > a lot of changes... > > > > I *think* but I'm not positive that gf_write64 should just take a void > > pointer like this: > > > > diff --git a/drivers/staging/goldfish/goldfish_nand.c b/drivers/staging/goldfish/goldfish_nand.c > > index 213877a..92d6479 100644 > > --- a/drivers/staging/goldfish/goldfish_nand.c > > +++ b/drivers/staging/goldfish/goldfish_nand.c > > @@ -87,7 +87,7 @@ static u32 goldfish_nand_cmd(struct mtd_info *mtd, enum nand_cmd cmd, > > writel((u32)(addr >> 32), base + NAND_ADDR_HIGH); > > writel((u32)addr, base + NAND_ADDR_LOW); > > writel(len, base + NAND_TRANSFER_SIZE); > > - gf_write64((u64)ptr, base + NAND_DATA, base + NAND_DATA_HIGH); > > + gf_write64(ptr, base + NAND_DATA, base + NAND_DATA_HIGH); > > writel(cmd, base + NAND_COMMAND); > > rv = readl(base + NAND_RESULT); > > } > > diff --git a/include/linux/goldfish.h b/include/linux/goldfish.h > > index 569236e..4cdb913 100644 > > --- a/include/linux/goldfish.h > > +++ b/include/linux/goldfish.h > > @@ -3,12 +3,12 @@ > > > > /* Helpers for Goldfish virtual platform */ > > > > -static inline void gf_write64(unsigned long data, > > +static inline void gf_write64(void *data, > > void __iomem *portl, void __iomem *porth) > > { > > - writel((u32)data, portl); > > + writel((u32)(unsigned long)data, portl); > > #ifdef CONFIG_64BIT > > - writel(data>>32, porth); > > + writel((unsigned long)data >> 32, porth); > > #endif > > } > > > > I like this solution. Should I send a single patch changing > gf_write64() and removing all casts when calling the gf_write64(), or > if to split, how to split the patch? Single patch, please. Maybe rename the function to gf_write_ptr() or something similar. regards, dan carpenter -- 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/