2008-10-01 08:02:35

by Steven Noonan

[permalink] [raw]
Subject: drivers/pci/probe.c compile warnings on -tip

I was hunting down some warnings I got when compiling -tip, and with
one of them, I'm not sure what would be a proper way to handle it.

If CONFIG_PHYS_ADDR_T_64BIT is not enabled, these warnings show up:

drivers/pci/probe.c: In function '__pci_read_base':
drivers/pci/probe.c:308: warning: format '%llx' expects type 'long
long unsigned int', but argument 4 has type 'resource_size_t'
drivers/pci/probe.c:308: warning: format '%llx' expects type 'long
long unsigned int', but argument 5 has type 'resource_size_t'
drivers/pci/probe.c:320: warning: format '%llx' expects type 'long
long unsigned int', but argument 5 has type 'resource_size_t'
drivers/pci/probe.c:320: warning: format '%llx' expects type 'long
long unsigned int', but argument 6 has type 'resource_size_t'
drivers/pci/probe.c: In function 'pci_read_bridge_bases':
drivers/pci/probe.c:392: warning: format '%llx' expects type 'long
long unsigned int', but argument 3 has type 'resource_size_t'
drivers/pci/probe.c:392: warning: format '%llx' expects type 'long
long unsigned int', but argument 4 has type 'resource_size_t'
drivers/pci/probe.c:405: warning: format '%llx' expects type 'long
long unsigned int', but argument 3 has type 'resource_size_t'
drivers/pci/probe.c:405: warning: format '%llx' expects type 'long
long unsigned int', but argument 4 has type 'resource_size_t'
drivers/pci/probe.c:443: warning: format '%llx' expects type 'long
long unsigned int', but argument 4 has type 'resource_size_t'
drivers/pci/probe.c:443: warning: format '%llx' expects type 'long
long unsigned int', but argument 5 has type 'resource_size_t'


Each of the lines is something like this:

printk(KERN_DEBUG "PCI: %s reg %x 64bit mmio: [%llx, %llx]\n",
pci_name(dev), pos, res->start, res->end);

res->start and res->end are resource_size_t (which is phys_addr_t),
and are sized either 32-bit or 64-bit, based on whether
CONFIG_PHYS_ADDR_T_64BIT is set. So, it seems to me that something
like these would be horrendously bad solutions (for obvious reasons,
like readability):

#ifdef CONFIG_PHYS_ADDR_T_64BIT
printk(KERN_DEBUG "PCI: %s reg %x 64bit mmio: [%llx, %llx]\n",
pci_name(dev), pos, res->start, res->end);
#else
printk(KERN_DEBUG "PCI: %s reg %x 64bit mmio: [%lx, %lx]\n",
pci_name(dev), pos, res->start, res->end);
#endif

or

#ifdef CONFIG_PHYS_ADDR_T_64BIT
printk(KERN_DEBUG "PCI: %s reg %x 64bit mmio: [%llx, %llx]\n",
pci_name(dev), pos, res->start, res->end);
#endif

What's the appropriate way to handle it? Moreover, are those printk's
even truly necessary?

- Steven


2008-10-01 11:03:54

by Mikael Pettersson

[permalink] [raw]
Subject: Re: drivers/pci/probe.c compile warnings on -tip

Steven Noonan writes:
> I was hunting down some warnings I got when compiling -tip, and with
> one of them, I'm not sure what would be a proper way to handle it.
>
> If CONFIG_PHYS_ADDR_T_64BIT is not enabled, these warnings show up:
>
> drivers/pci/probe.c: In function '__pci_read_base':
> drivers/pci/probe.c:308: warning: format '%llx' expects type 'long
> long unsigned int', but argument 4 has type 'resource_size_t'
> drivers/pci/probe.c:308: warning: format '%llx' expects type 'long
> long unsigned int', but argument 5 has type 'resource_size_t'
> drivers/pci/probe.c:320: warning: format '%llx' expects type 'long
> long unsigned int', but argument 5 has type 'resource_size_t'
> drivers/pci/probe.c:320: warning: format '%llx' expects type 'long
> long unsigned int', but argument 6 has type 'resource_size_t'
> drivers/pci/probe.c: In function 'pci_read_bridge_bases':
> drivers/pci/probe.c:392: warning: format '%llx' expects type 'long
> long unsigned int', but argument 3 has type 'resource_size_t'
> drivers/pci/probe.c:392: warning: format '%llx' expects type 'long
> long unsigned int', but argument 4 has type 'resource_size_t'
> drivers/pci/probe.c:405: warning: format '%llx' expects type 'long
> long unsigned int', but argument 3 has type 'resource_size_t'
> drivers/pci/probe.c:405: warning: format '%llx' expects type 'long
> long unsigned int', but argument 4 has type 'resource_size_t'
> drivers/pci/probe.c:443: warning: format '%llx' expects type 'long
> long unsigned int', but argument 4 has type 'resource_size_t'
> drivers/pci/probe.c:443: warning: format '%llx' expects type 'long
> long unsigned int', but argument 5 has type 'resource_size_t'
>
>
> Each of the lines is something like this:
>
> printk(KERN_DEBUG "PCI: %s reg %x 64bit mmio: [%llx, %llx]\n",
> pci_name(dev), pos, res->start, res->end);
>
> res->start and res->end are resource_size_t (which is phys_addr_t),
> and are sized either 32-bit or 64-bit, based on whether
> CONFIG_PHYS_ADDR_T_64BIT is set.

Oh not those again. Similar warnings in 27-rc's drivers/pci
were fixed recently, but apparently someone out there likes
to create new ones.

The fix is to cast the resource_size_t arguments to unsigned long long.
With "%llx" that's the /only/ valid argument type.

Also note that these warnings aren't harmless. On 32-bit machines
with parameters passed on the stack the mismatch in size between
32-bit resource_size_t and the 64-bit %llx causes undefined
behaviour as printk mis-enumerates its parameters.

/Mikael

2008-10-01 15:51:45

by Jesse Barnes

[permalink] [raw]
Subject: Re: drivers/pci/probe.c compile warnings on -tip

On Wednesday, October 1, 2008 3:47 am Mikael Pettersson wrote:
> Steven Noonan writes:
> > I was hunting down some warnings I got when compiling -tip, and with
> > one of them, I'm not sure what would be a proper way to handle it.
> >
> > If CONFIG_PHYS_ADDR_T_64BIT is not enabled, these warnings show up:
> >
> > drivers/pci/probe.c: In function '__pci_read_base':
> > drivers/pci/probe.c:308: warning: format '%llx' expects type 'long
> > long unsigned int', but argument 4 has type 'resource_size_t'
> > drivers/pci/probe.c:308: warning: format '%llx' expects type 'long
> > long unsigned int', but argument 5 has type 'resource_size_t'
> > drivers/pci/probe.c:320: warning: format '%llx' expects type 'long
> > long unsigned int', but argument 5 has type 'resource_size_t'
> > drivers/pci/probe.c:320: warning: format '%llx' expects type 'long
> > long unsigned int', but argument 6 has type 'resource_size_t'
> > drivers/pci/probe.c: In function 'pci_read_bridge_bases':
> > drivers/pci/probe.c:392: warning: format '%llx' expects type 'long
> > long unsigned int', but argument 3 has type 'resource_size_t'
> > drivers/pci/probe.c:392: warning: format '%llx' expects type 'long
> > long unsigned int', but argument 4 has type 'resource_size_t'
> > drivers/pci/probe.c:405: warning: format '%llx' expects type 'long
> > long unsigned int', but argument 3 has type 'resource_size_t'
> > drivers/pci/probe.c:405: warning: format '%llx' expects type 'long
> > long unsigned int', but argument 4 has type 'resource_size_t'
> > drivers/pci/probe.c:443: warning: format '%llx' expects type 'long
> > long unsigned int', but argument 4 has type 'resource_size_t'
> > drivers/pci/probe.c:443: warning: format '%llx' expects type 'long
> > long unsigned int', but argument 5 has type 'resource_size_t'
> >
> >
> > Each of the lines is something like this:
> >
> > printk(KERN_DEBUG "PCI: %s reg %x 64bit mmio: [%llx, %llx]\n",
> > pci_name(dev), pos, res->start, res->end);
> >
> > res->start and res->end are resource_size_t (which is phys_addr_t),
> > and are sized either 32-bit or 64-bit, based on whether
> > CONFIG_PHYS_ADDR_T_64BIT is set.
>
> Oh not those again. Similar warnings in 27-rc's drivers/pci
> were fixed recently, but apparently someone out there likes
> to create new ones.
>
> The fix is to cast the resource_size_t arguments to unsigned long long.
> With "%llx" that's the /only/ valid argument type.
>
> Also note that these warnings aren't harmless. On 32-bit machines
> with parameters passed on the stack the mismatch in size between
> 32-bit resource_size_t and the 64-bit %llx causes undefined
> behaviour as printk mis-enumerates its parameters.

AFAICT, these are fixed in Linus' tree, maybe the x86 tree needs an update?

--
Jesse Barnes, Intel Open Source Technology Center

2008-10-05 03:55:31

by Steven Noonan

[permalink] [raw]
Subject: Re: drivers/pci/probe.c compile warnings on -tip

On Wed, Oct 1, 2008 at 8:51 AM, Jesse Barnes <[email protected]> wrote:
> On Wednesday, October 1, 2008 3:47 am Mikael Pettersson wrote:
>> Steven Noonan writes:
>> > I was hunting down some warnings I got when compiling -tip, and with
>> > one of them, I'm not sure what would be a proper way to handle it.
>> >
>> > If CONFIG_PHYS_ADDR_T_64BIT is not enabled, these warnings show up:
>> >
>> > drivers/pci/probe.c: In function '__pci_read_base':
>> > drivers/pci/probe.c:308: warning: format '%llx' expects type 'long
>> > long unsigned int', but argument 4 has type 'resource_size_t'
>> > drivers/pci/probe.c:308: warning: format '%llx' expects type 'long
>> > long unsigned int', but argument 5 has type 'resource_size_t'
>> > drivers/pci/probe.c:320: warning: format '%llx' expects type 'long
>> > long unsigned int', but argument 5 has type 'resource_size_t'
>> > drivers/pci/probe.c:320: warning: format '%llx' expects type 'long
>> > long unsigned int', but argument 6 has type 'resource_size_t'
>> > drivers/pci/probe.c: In function 'pci_read_bridge_bases':
>> > drivers/pci/probe.c:392: warning: format '%llx' expects type 'long
>> > long unsigned int', but argument 3 has type 'resource_size_t'
>> > drivers/pci/probe.c:392: warning: format '%llx' expects type 'long
>> > long unsigned int', but argument 4 has type 'resource_size_t'
>> > drivers/pci/probe.c:405: warning: format '%llx' expects type 'long
>> > long unsigned int', but argument 3 has type 'resource_size_t'
>> > drivers/pci/probe.c:405: warning: format '%llx' expects type 'long
>> > long unsigned int', but argument 4 has type 'resource_size_t'
>> > drivers/pci/probe.c:443: warning: format '%llx' expects type 'long
>> > long unsigned int', but argument 4 has type 'resource_size_t'
>> > drivers/pci/probe.c:443: warning: format '%llx' expects type 'long
>> > long unsigned int', but argument 5 has type 'resource_size_t'
>> >
>> >
>> > Each of the lines is something like this:
>> >
>> > printk(KERN_DEBUG "PCI: %s reg %x 64bit mmio: [%llx, %llx]\n",
>> > pci_name(dev), pos, res->start, res->end);
>> >
>> > res->start and res->end are resource_size_t (which is phys_addr_t),
>> > and are sized either 32-bit or 64-bit, based on whether
>> > CONFIG_PHYS_ADDR_T_64BIT is set.
>>
>> Oh not those again. Similar warnings in 27-rc's drivers/pci
>> were fixed recently, but apparently someone out there likes
>> to create new ones.
>>
>> The fix is to cast the resource_size_t arguments to unsigned long long.
>> With "%llx" that's the /only/ valid argument type.
>>
>> Also note that these warnings aren't harmless. On 32-bit machines
>> with parameters passed on the stack the mismatch in size between
>> 32-bit resource_size_t and the 64-bit %llx causes undefined
>> behaviour as printk mis-enumerates its parameters.
>
> AFAICT, these are fixed in Linus' tree, maybe the x86 tree needs an update?
>

Hey Ingo, these warnings are still showing up on the latest -tip. Any idea why?

- Steven

2008-10-05 10:18:31

by Ingo Molnar

[permalink] [raw]
Subject: Re: drivers/pci/probe.c compile warnings on -tip


* Steven Noonan <[email protected]> wrote:

> > AFAICT, these are fixed in Linus' tree, maybe the x86 tree needs an
> > update?
> >
>
> Hey Ingo, these warnings are still showing up on the latest -tip. Any
> idea why?

yes, they got reintroduced via:

| From ca81beb4a340ee690803fcb1498f863d0e68843f Mon Sep 17 00:00:00 2001
| From: Yinghai Lu <[email protected]>
| Date: Thu, 4 Sep 2008 20:57:15 +0200
| Subject: [PATCH] pci: fix BAR print out

which spread into a few topic branches:

# irq/sparseirq: ca81beb: pci: fix BAR print out
# irq/sparseirq: f430d990: Merge branch 'linus' into irq/sparseirq
# timers/hpet-percpu: ca81beb: pci: fix BAR print out
# x86/uv: ca81beb: pci: fix BAR print out
# x86/uv: f430d990: Merge branch 'linus' into irq/sparseirq

i suspect f430d990 is where the in-linus fix collided with the
in-irq/sparseirq fix. I'm inclined to revert it. Yinghai?

Ingo

2008-10-05 18:06:42

by Yinghai Lu

[permalink] [raw]
Subject: Re: drivers/pci/probe.c compile warnings on -tip

On Sun, Oct 5, 2008 at 3:18 AM, Ingo Molnar <[email protected]> wrote:
>
> * Steven Noonan <[email protected]> wrote:
>
>> > AFAICT, these are fixed in Linus' tree, maybe the x86 tree needs an
>> > update?
>> >
>>
>> Hey Ingo, these warnings are still showing up on the latest -tip. Any
>> idea why?
>
> yes, they got reintroduced via:
>
> | From ca81beb4a340ee690803fcb1498f863d0e68843f Mon Sep 17 00:00:00 2001
> | From: Yinghai Lu <[email protected]>
> | Date: Thu, 4 Sep 2008 20:57:15 +0200
> | Subject: [PATCH] pci: fix BAR print out
>
> which spread into a few topic branches:
>
> # irq/sparseirq: ca81beb: pci: fix BAR print out
> # irq/sparseirq: f430d990: Merge branch 'linus' into irq/sparseirq
> # timers/hpet-percpu: ca81beb: pci: fix BAR print out
> # x86/uv: ca81beb: pci: fix BAR print out
> # x86/uv: f430d990: Merge branch 'linus' into irq/sparseirq
>
> i suspect f430d990 is where the in-linus fix collided with the
> in-irq/sparseirq fix. I'm inclined to revert it. Yinghai?
>

please. it should be reverted from irq/sparseirq etc branch...because
updated one is already in linus tree.

YH

2008-10-06 06:21:46

by Ingo Molnar

[permalink] [raw]
Subject: Re: drivers/pci/probe.c compile warnings on -tip


* Yinghai Lu <[email protected]> wrote:

> On Sun, Oct 5, 2008 at 3:18 AM, Ingo Molnar <[email protected]> wrote:
> >
> > * Steven Noonan <[email protected]> wrote:
> >
> >> > AFAICT, these are fixed in Linus' tree, maybe the x86 tree needs an
> >> > update?
> >> >
> >>
> >> Hey Ingo, these warnings are still showing up on the latest -tip. Any
> >> idea why?
> >
> > yes, they got reintroduced via:
> >
> > | From ca81beb4a340ee690803fcb1498f863d0e68843f Mon Sep 17 00:00:00 2001
> > | From: Yinghai Lu <[email protected]>
> > | Date: Thu, 4 Sep 2008 20:57:15 +0200
> > | Subject: [PATCH] pci: fix BAR print out
> >
> > which spread into a few topic branches:
> >
> > # irq/sparseirq: ca81beb: pci: fix BAR print out
> > # irq/sparseirq: f430d990: Merge branch 'linus' into irq/sparseirq
> > # timers/hpet-percpu: ca81beb: pci: fix BAR print out
> > # x86/uv: ca81beb: pci: fix BAR print out
> > # x86/uv: f430d990: Merge branch 'linus' into irq/sparseirq
> >
> > i suspect f430d990 is where the in-linus fix collided with the
> > in-irq/sparseirq fix. I'm inclined to revert it. Yinghai?
> >
>
> please. it should be reverted from irq/sparseirq etc branch...because
> updated one is already in linus tree.

ok, reverted it in irq/sparseirq and propagated that to the
sparseirq-derived topic trees as well.

Ingo