Subject: Re: [PATCH] [RFC][POWERPC] Merge 32 and 64 bit pci_process_bridge_OF_ranges() instances

From: Arnd Bergmann <[email-masked]>
Date: Wed, 12 Sep 2007 00:57:17 +0200

On Wednesday 12 September 2007, Vitaly Bordug wrote:
> 
> We are having 2 different instances of pci_process_bridge_OF_ranges(),
> which makes describing 64-bit physical addresses in non PPC64 case
> impossible.
> 
> This approach inherits pci space parsing, but has a new way to behave
> equally good in both 32bit and 64bit environments. This approach uses
> of_translate_address(), so implies proper ranges <> definition in
> devicetree, where PCI node has its reg on soc bus, and its ranges
> effectively belong to LAW addresses.
> 
> Signed-off-by: Vitaly Bordug <[email-masked]>
> Signed-off-by: Stefan Roese <[email-masked]>

The patch looks really good, but it's hard to review when you move the
code around at the same as you change it.

Could you perhaps split the patch into two separate changesets, one
that makes both functions identical in place, and one that merges
them to live in a common location?

> diff --git a/include/asm-powerpc/ppc-pci.h b/include/asm-powerpc/ppc-pci.h
> index b847aa1..882b8bc 100644
> --- a/include/asm-powerpc/ppc-pci.h
> +++ b/include/asm-powerpc/ppc-pci.h
> @@ -15,6 +15,13 @@
>  #include <linux/pci.h>
>  #include <asm/pci-bridge.h>
>  
> +struct ranges_pci {
> +	unsigned int pci_space;
> +	u64 pci_addr;
> +	phys_addr_t phys_addr;
> +	u64 size;
> +} __attribute__((packed));
> +

This structure definition uses unaligned members because of the 'packed'
attribute. Is that really what you intended?

	Arnd <><