[PATCH 1/3 v3] ARM Realview PCIX map include file changes

Russell King - ARM Linux
2011-09-08T09:09:31Z

Colin,

Please can you answer Arnd's concerns. He's right that these points
have been brought up before, they've also been discussed on the list
a number of times with regards to other platforms too.

Note that Arnd sent a follow-up email to the one below correcting a
small mistake in his comments.

Thanks.

On Mon, Sep 05, 2011 at 04:35:59PM +0200, Arnd Bergmann wrote:
> On Monday 22 August 2011, Colin Tuckley wrote:
> > This patch adds the memory map PCI support to the include
> > files for the Realview Northbridge based boards.
> >
> > Signed-off-by: Colin Tuckley <colin.tuckley@arm.com>
> > Acked-by: Catalin Marinas <catalin.marinas@arm.com>
>
> Hi Colin,
>
> I think we've discussed this before. I think it's good to
> have PCI support enabled in realview, but please do the
> PIO window properly.
>
> > diff --git a/arch/arm/mach-realview/include/mach/hardware.h b/arch/arm/mach-realview/include/mach/hardware.h
> > index 8a638d1..dcc64f2 100644
> > --- a/arch/arm/mach-realview/include/mach/hardware.h
> > +++ b/arch/arm/mach-realview/include/mach/hardware.h
> > +/*
> > + * PCI space physical addresses and sizes
> > + */
> > +#define REALVIEW_PB_PCI_BASE 0x90040000 /* PCI-X Unit base */
> > +#define REALVIEW_PB_PCI_BASE_SIZE 0x00010000 /* 4 Kb + 60Kb reserved */
> > +#define REALVIEW_PB_PCI_IO_BASE 0x90050000 /* IO Region on AHB */
> > +#define REALVIEW_PB_PCI_IO_SIZE 0x00010000 /* 64 Kb */
> > +#define REALVIEW_PB_PCI_IO_LIMIT (REALVIEW_PB_PCI_IO_BASE + REALVIEW_PB_PCI_IO_SIZE - 1)
> > +#define REALVIEW_PB_PCI_MEM_BASE 0xA0000000 /* MEM Region on AHB */
> > +#define REALVIEW_PB_PCI_MEM_SIZE 0x20000000 /* 512 MB */
> > +
> > +#define REALVIEW_ISSP_REG_BASE 0x100E3000
>
> As you write here, the I/O window size is 64KB, which is totally normal
> for PCI buses.
>
> > +#ifdef CONFIG_PCI
> > +#if !defined(__ASSEMBLY__)
> > +static inline unsigned int pcibios_min_io(void)
> > +{
> > + if (machine_is_realview_pb11mp() || machine_is_realview_pba8() ||
> > + machine_is_realview_pbx())
> > + return REALVIEW_PB_PCI_IO_BASE;
> > + else
> > + return 0;
> > +}
>
> pcibios_min_io should be 0x1000 as a constant, in order to get the
> ISA addresses out of the way, but there is no need to make it
> board dependent.
>
> > +static inline unsigned int pcibios_min_mem(void)
> > +{
> > + if (machine_is_realview_pb11mp() || machine_is_realview_pba8() ||
> > + machine_is_realview_pbx())
> > + return REALVIEW_PB_PCI_MEM_BASE;
> > + else
> > + return 0;
> > +}
> > +#endif
>
> Just hardcode this to REALVIEW_PB_PCI_MEM_BASE
>
> > diff --git a/arch/arm/mach-realview/include/mach/io.h b/arch/arm/mach-realview/include/mach/io.h
> > index f05bcdf..aa97d18 100644
> > --- a/arch/arm/mach-realview/include/mach/io.h
> > +++ b/arch/arm/mach-realview/include/mach/io.h
> > @@ -20,9 +20,25 @@
> > #ifndef __ASM_ARM_ARCH_IO_H
> > #define __ASM_ARM_ARCH_IO_H
> >
> > +#include <asm/mach-types.h>
> > +#include <mach/hardware.h>
> > +
> > #define IO_SPACE_LIMIT 0xffffffff
>
> The IO_SPACE_LIMIT should match the REALVIEW_PB_PCI_IO_BASE above.
>
> > -#define __io(a) __typesafe_io(a)
> > +static inline void __iomem *__io(unsigned long addr)
> > +{
> > +#ifdef CONFIG_PCI
> > + /* check for PCI I/O space */
> > + if (addr >= REALVIEW_PB_PCI_IO_BASE && addr <= REALVIEW_PB_PCI_IO_LIMIT)
> > + return (void __iomem *)((addr - REALVIEW_PB_PCI_IO_BASE) + REALVIEW_PCI_IO_VBASE);
> > + else
> > + return (void __iomem *)addr;
> > +#else
> > + return (void __iomem *)addr;
> > +#endif
> > +}
>
> And there is no need to do these tricks when you only have a single
> PCI bus. Just define the PIO window to be based on zero and 64K
> in size, and hardcode that, to end up with
>
> #define __io(a) (REALVIEW_PCI_IO_VBASE + (a & IO_SPACE_LIMIT))
>
> Arnd
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel