drivers/pci/probe.c compile warnings on -tip

drivers/pci/probe.c compile warnings on -tip

[Steven Noonan]

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

Re: drivers/pci/probe.c compile warnings on -tip

[Mikael Pettersson]

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

Re: drivers/pci/probe.c compile warnings on -tip

[Jesse Barnes]

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

Re: drivers/pci/probe.c compile warnings on -tip

[Steven Noonan]

On Wed, Oct 1, 2008 at 8:51 AM, Jesse Barnes <jbarnes@virtuousgeek.org> 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

Re: drivers/pci/probe.c compile warnings on -tip

[Ingo Molnar]

* Steven Noonan <steven@uplinklabs.net> 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 <yhlu.kernel@gmail.com>
| 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

Re: drivers/pci/probe.c compile warnings on -tip

[Yinghai Lu]

On Sun, Oct 5, 2008 at 3:18 AM, Ingo Molnar <mingo@elte.hu> wrote:
>
> * Steven Noonan <steven@uplinklabs.net> 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 <yhlu.kernel@gmail.com>
> | 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

Re: drivers/pci/probe.c compile warnings on -tip

[Ingo Molnar]

* Yinghai Lu <yinghai@kernel.org> wrote:

> On Sun, Oct 5, 2008 at 3:18 AM, Ingo Molnar <mingo@elte.hu> wrote:
> >
> > * Steven Noonan <steven@uplinklabs.net> 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 <yhlu.kernel@gmail.com>
> > | 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