Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754026AbbDMLO1 (ORCPT ); Mon, 13 Apr 2015 07:14:27 -0400 Received: from mail-lb0-f175.google.com ([209.85.217.175]:35219 "EHLO mail-lb0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753437AbbDMLOY (ORCPT ); Mon, 13 Apr 2015 07:14:24 -0400 MIME-Version: 1.0 In-Reply-To: References: <1428830803-30896-1-git-send-email-peter.senna@gmail.com> Date: Mon, 13 Apr 2015 13:14:22 +0200 Message-ID: Subject: Re: [PATCH] Fix pointer cast for 32 bits arch From: Peter Senna Tschudin To: Geert Uytterhoeven Cc: Greg Kroah-Hartman , Loic Pefferkorn , Alan , Jun Tian , Octavian Purdila , Garret Kelly , =?UTF-8?Q?Kristina_Mart=C5=A1enko?= , Nick Kralevich , driverdevel , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4465 Lines: 114 On Sun, Apr 12, 2015 at 5:38 PM, Geert Uytterhoeven wrote: > On Sun, Apr 12, 2015 at 3:48 PM, Peter Senna Tschudin > wrote: >> On Sun, Apr 12, 2015 at 3:05 PM, Geert Uytterhoeven >> wrote: >>> On Sun, Apr 12, 2015 at 11:26 AM, 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); >>> >>> I guess sparse still complains if CONFIG_X86_PAE=y, which makes >>> resource_size_t u64? >> >> Yes, when CONFIG_X86_PAE=y the patch doesn't fix the warning(gcc >> warning). What is the correct/portable way of fixing this? > > As gf_write64() takes "unsigned long data" for its first argument, why not > simply cast ptr to "unsigned long"? "void *" and "unsigned long" have the same > size on all Linux platforms. > > Seems like there a several silly casting tricks being done in the goldfish > drivers: > > $ git grep -w gf_write64drivers/platform/goldfish/goldfish_pipe.c: > gf_write64((u64)(unsigned long)p > drivers/platform/goldfish/goldfish_pipe.c: gf_write64((u64)(unsigned long)p > drivers/platform/goldfish/goldfish_pipe.c: gf_write64((u64) > drivers/platform/goldfish/goldfish_pipe.c: gf_write64(addre > drivers/staging/goldfish/goldfish_audio.c: (gf_write64((u64)(x), data->reg_ > drivers/staging/goldfish/goldfish_nand.c: gf_write64((u64)ptr, bas > drivers/tty/goldfish.c: gf_write64((u64)buf, base + GOLDFISH_TTY_DATA_PTR, > drivers/tty/goldfish.c: gf_write64((u64)buf, base + GOLDFISH_TTY_DATA_PTR, > include/linux/goldfish.h:static inline void gf_write64(unsigned long data, > > with interesting history showing one wrong cast being "fixed" using another > wrong cast: > > commit f4e131dc38d34469b9fee1840ad237324831bce3 > Author: Octavian Purdila > Date: Fri May 16 08:24:59 2014 +0300 > > goldfish: pipe: fix warnings for 32bit builds > > - gf_write64((u64)pipe, dev->base + PIPE_REG_CHANNEL, > + gf_write64((u64)(unsigned long)pipe, dev->base + PIPE_REG_CHANNEL, > > As in general casts don't belong in drivers[*], what about creating an inline > helper gf_set_pipe() that converts the pointer to unsigned long and calls > gf_write64()? > > Perhaps checkpatch should complain about casts outside header files? Cocinelle found 664 double casts: http://pastebin.com/2bi9Dg7k and 9 triple casts: http://pastebin.com/RkJhPTTV Those are 'not' patches, just the output of Coccinelle for analysis. The .cocci for triple cast: @@ expression e1, e2; type t1, t2, t3; @@ e1 = - (t1)(t2)(t3) e2 So if there are cast patterns known to be wrong, it is easy to find and probably fix them with Coccinelle. > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds -- Peter -- 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/