2009-11-17 16:03:05

by Alan

[permalink] [raw]
Subject: [RESEND PATCH] gpio_addr_flash: Fix warnings

Signed-off-by: Alan Cox <[email protected]>
---

drivers/mtd/maps/gpio-addr-flash.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)


diff --git a/drivers/mtd/maps/gpio-addr-flash.c b/drivers/mtd/maps/gpio-addr-flash.c
index 1ad5caf..9760e31 100644
--- a/drivers/mtd/maps/gpio-addr-flash.c
+++ b/drivers/mtd/maps/gpio-addr-flash.c
@@ -208,7 +208,7 @@ static int __devinit gpio_flash_probe(struct platform_device *pdev)
return -ENOMEM;

state->gpio_count = gpios->end;
- state->gpio_addrs = (void *)gpios->start;
+ state->gpio_addrs = (void *)(unsigned long)gpios->start;
state->gpio_values = (void *)(state + 1);
state->win_size = memory->end - memory->start + 1;
memset(state->gpio_values, 0xff, arr_size);
@@ -220,7 +220,7 @@ static int __devinit gpio_flash_probe(struct platform_device *pdev)
state->map.copy_to = gf_copy_to;
state->map.bankwidth = pdata->width;
state->map.size = state->win_size * (1 << state->gpio_count);
- state->map.virt = (void __iomem *)memory->start;
+ state->map.virt = (void __iomem *)(unsigned long)memory->start;
state->map.phys = NO_XIP;
state->map.map_priv_1 = (unsigned long)state;


2009-11-17 16:50:41

by Mike Frysinger

[permalink] [raw]
Subject: Re: [RESEND PATCH] gpio_addr_flash: Fix warnings

On Tue, Nov 17, 2009 at 10:46, Alan Cox wrote:
> --- a/drivers/mtd/maps/gpio-addr-flash.c
> +++ b/drivers/mtd/maps/gpio-addr-flash.c
> -       state->gpio_addrs     = (void *)gpios->start;
> +       state->gpio_addrs     = (void *)(unsigned long)gpios->start;
> -       state->map.virt       = (void __iomem *)memory->start;
> +       state->map.virt       = (void __iomem *)(unsigned long)memory->start;

on what system do you see warnings ? start should be resource_size_t
which should be phys_addr_t which should cast fine to a void*.
-mike

2009-11-17 16:56:53

by Alan

[permalink] [raw]
Subject: Re: [RESEND PATCH] gpio_addr_flash: Fix warnings

On Tue, 17 Nov 2009 11:50:24 -0500
Mike Frysinger <[email protected]> wrote:

> On Tue, Nov 17, 2009 at 10:46, Alan Cox wrote:
> > --- a/drivers/mtd/maps/gpio-addr-flash.c
> > +++ b/drivers/mtd/maps/gpio-addr-flash.c
> > - ? ? ? state->gpio_addrs ? ? = (void *)gpios->start;
> > + ? ? ? state->gpio_addrs ? ? = (void *)(unsigned long)gpios->start;
> > - ? ? ? state->map.virt ? ? ? = (void __iomem *)memory->start;
> > + ? ? ? state->map.virt ? ? ? = (void __iomem *)(unsigned long)memory->start;
>
> on what system do you see warnings ? start should be resource_size_t
> which should be phys_addr_t which should cast fine to a void*.

32bit x86 shows it up because resource_size_t is a physaddr_t which is
64bit (because of PAE). Now given you don't seem likely to be using the
top bits for any real world example that looks ok but really casting a
physical address to a pointer type needs care and the compiler can't
deduce your intent there.

2009-11-17 23:22:15

by Mike Frysinger

[permalink] [raw]
Subject: Re: [RESEND PATCH] gpio_addr_flash: Fix warnings

On Tue, Nov 17, 2009 at 11:58, Alan Cox wrote:
> On Tue, 17 Nov 2009 11:50:24 -0500 Mike Frysinger wrote:
>> On Tue, Nov 17, 2009 at 10:46, Alan Cox wrote:
>> > --- a/drivers/mtd/maps/gpio-addr-flash.c
>> > +++ b/drivers/mtd/maps/gpio-addr-flash.c
>> > -       state->gpio_addrs     = (void *)gpios->start;
>> > +       state->gpio_addrs     = (void *)(unsigned long)gpios->start;
>> > -       state->map.virt       = (void __iomem *)memory->start;
>> > +       state->map.virt       = (void __iomem *)(unsigned long)memory->start;
>>
>> on what system do you see warnings ?  start should be resource_size_t
>> which should be phys_addr_t which should cast fine to a void*.
>
> 32bit x86 shows it up because resource_size_t is a physaddr_t which is
> 64bit (because of PAE). Now given you don't seem likely to be using the
> top bits for any real world example that looks ok but really casting a
> physical address to a pointer type needs care and the compiler can't
> deduce your intent there.

i dont think the casts will work in either case here and it's a good
thing that the code doesnt compile warning free

the gpio_addrs member is an array of gpio ids which are 32bit unsigned
values. overlaying an array of 64bit values isnt going to produce
correct behavior.

as for the memory->start, i do need to access that directly, so that
should probably be going through ioremap_nocache() instead.

i.e. i'll have to write/test/post a patch rather than adding casts to
pretend there isnt a problem
-mike