Patchwork [01/15] ppc: fix return type of BUID_{HI,LO} macros

BUID_HI and BUID_LO are used to pass data to call_rtas, which expects
ints or u32s. But the macro doesn't cast the return, so the result is
still u64. Use the upper_32_bits and lower_32_bits macros that have been
added to kernel.h.

Found by getting printf format errors trying to debug print the args, no
actual code change for 64 bit kernels where the macros are actually
used.

Signed-off-by: Milton Miller <miltonm@bga.com>
Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
---
 arch/powerpc/include/asm/ppc-pci.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Acked-by: Linas Vepstas <linasvepstas@gmail.com>

I'm guessing this worked up til now because the rtas_call function prototype
was telling compiler to cast these to 32-bit before passing them as args.
(and since these would still get passed as one arg per 64-bit reg, it
still wouldn't go wrong.)

What I'm wondering about is why there was no compiler warning about an
implicit cast of a 64-bit int to a 32-bit int?  Surely, this is something that
should be warned about!

-- Linas

On 15 September 2010 13:13, Nishanth Aravamudan <nacc@us.ibm.com> wrote:
> BUID_HI and BUID_LO are used to pass data to call_rtas, which expects
> ints or u32s. But the macro doesn't cast the return, so the result is
> still u64. Use the upper_32_bits and lower_32_bits macros that have been
> added to kernel.h.
>
> Found by getting printf format errors trying to debug print the args, no
> actual code change for 64 bit kernels where the macros are actually
> used.
>
> Signed-off-by: Milton Miller <miltonm@bga.com>
> Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
> ---
>  arch/powerpc/include/asm/ppc-pci.h |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/ppc-pci.h b/arch/powerpc/include/asm/ppc-pci.h
> index 42fdff0..43268f1 100644
> --- a/arch/powerpc/include/asm/ppc-pci.h
> +++ b/arch/powerpc/include/asm/ppc-pci.h
> @@ -28,8 +28,8 @@ extern void find_and_init_phbs(void);
>  extern struct pci_dev *isa_bridge_pcidev;      /* may be NULL if no ISA bus */
>
>  /** Bus Unit ID macros; get low and hi 32-bits of the 64-bit BUID */
> -#define BUID_HI(buid) ((buid) >> 32)
> -#define BUID_LO(buid) ((buid) & 0xffffffff)
> +#define BUID_HI(buid) upper_32_bits(buid)
> +#define BUID_LO(buid) lower_32_bits(buid)
>
>  /* PCI device_node operations */
>  struct device_node;
> --
> 1.7.0.4
>
>

On Thu, 16 Sep 2010 17:54:46 -0500
Linas Vepstas <linasvepstas@gmail.com> wrote:

> Acked-by: Linas Vepstas <linasvepstas@gmail.com>
> 
> I'm guessing this worked up til now because the rtas_call function prototype
> was telling compiler to cast these to 32-bit before passing them as args.
> (and since these would still get passed as one arg per 64-bit reg, it
> still wouldn't go wrong.)
> 
> What I'm wondering about is why there was no compiler warning about an
> implicit cast of a 64-bit int to a 32-bit int?  Surely, this is something that
> should be warned about!

-Wconversion enables this.

There'd be a lot of false positives to squash with that enabled --
and it might not be worth the resulting explosion in casts.

-Scott