> -----Original Message----- > From: Kevin Hilman [mailto:khil...@deeprootsystems.com] > Sent: Friday, May 22, 2009 4:48 AM > To: Pedanekar, Hemant > Cc: davinci-linux-open-source@linux.davincidsp.com > Subject: Re: [PATCH 1/2] ARM: DaVinci: DM646x PCIHOST: Core files > modifications for PCI PINMUX and I/O > > Hemant Pedanekar <hema...@ti.com> writes: > > > The changes mainly target two areas: 1) Provide clock structure, setting > up PCI > > PINMUX and perform board specific fix-ups for PCI Host core to begin > > configuration/enumeraion and 2) Provide PCI IO access through in/outx() > and > > ioread/iowritex(). > > > > The arm-kernel configuration is updated to support PCI Bus for DM646x > builds. > > > > In addition, I/O mapping macros are moved inside hardware.h to avoid the > need > > for inclusion of io.h in assembly files which results in assembler > errors for > > PCI I/O functions, as well as serial.h, which results in unresolved > symbols for > > PCI I/O functions (in uncompress code). > > > > Signed-off-by: Hemant Pedanekar <hema...@ti.com> > > --- > > arch/arm/Kconfig | 2 +- > > arch/arm/mach-davinci/Kconfig | 1 + > > arch/arm/mach-davinci/Makefile | 6 + > > arch/arm/mach-davinci/board-dm646x-evm.c | 82 ++++++++ > > arch/arm/mach-davinci/dm646x.c | 22 ++- > > arch/arm/mach-davinci/include/mach/dm646x.h | 33 +++- > > arch/arm/mach-davinci/include/mach/entry-macro.S | 2 +- > > arch/arm/mach-davinci/include/mach/hardware.h | 21 ++ > > arch/arm/mach-davinci/include/mach/io.h | 237 > ++++++++++++++++++++-- > > arch/arm/mach-davinci/include/mach/mux.h | 10 +- > > arch/arm/mach-davinci/include/mach/serial.h | 2 +- > > arch/arm/mach-davinci/include/mach/vmalloc.h | 2 +- > > 12 files changed, 390 insertions(+), 30 deletions(-) > > > > Just some general comments without digging too much into the PCI > specific parts... > > You're doing lots of things in this patch, it should be split break it > up into smaller pieces with better descriptions. > > You could break out some cleanup/fixup patches, and the clock/mux > defines into separate patches, the PCI accessor functions etc. etc. > That seems better approach. I will do that.
> The header rework should be a separate patch with a clearer > description of the problem. If the problem is with incluing from > assembly files, then there's probably a better way to do this like > using #ifdef __ASSEMBLER__. > Its not just assembler errors, I also get undefined references errors from uncompress code as io.h gets included there (through serial.h, I guess) and the PIC I/O functions are not built. Another approach to get around these errors was to add EXTRA_FLAGS in arch/arm/boot/compressed/Makefile and then use that flag along with __ASSEMBLER__ to skip PCI I/O functions in io.h. I also observed that most of the other arm/mach-xxx had IO_ADDRESS etc. defined in hardware.h. So they would not get such errors as io.h was not included. Which option do you suggest? > Also, is PCI going to be specific to the dm646x forever? Something > tells me no. Some of the PCI stuff you put into dm646x.h should > probably be in a more generic header like <mach/pci.h> or something. > Are you referring to "pcibios_assign_all_busses"? If so, you are correct that it is not really DM6467 specific. But the other #defines (such as addresses) are still DM6467 specific aren't they? > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > > index bf935b0..3324c02 100644 > > --- a/arch/arm/Kconfig > > +++ b/arch/arm/Kconfig > > @@ -776,7 +776,7 @@ config ISA_DMA_API > > bool > > > > config PCI > > - bool "PCI support" if ARCH_INTEGRATOR_AP || ARCH_VERSATILE_PB || > ARCH_IXP4XX || ARCH_KS8695 || MACH_ARMCORE > > + bool "PCI support" if ARCH_INTEGRATOR_AP || ARCH_VERSATILE_PB || > ARCH_IXP4XX || ARCH_KS8695 || MACH_ARMCORE || ARCH_DAVINCI_DM646x > > help > > Find out whether you have a PCI motherboard. PCI is the name of a > > bus system, i.e. the way the CPU talks to the other stuff inside > > diff --git a/arch/arm/mach-davinci/Kconfig b/arch/arm/mach- > davinci/Kconfig > > index c460615..2ce0ffd 100644 > > --- a/arch/arm/mach-davinci/Kconfig > > +++ b/arch/arm/mach-davinci/Kconfig > > @@ -17,6 +17,7 @@ config ARCH_DAVINCI_DM644x > > config ARCH_DAVINCI_DM646x > > bool "DaVinci 646x based system" > > select AINTC > > + select DAVINCI_MUX if PCI > > > > config ARCH_DAVINCI_DM355 > > bool "DaVinci 355 based system" > > diff --git a/arch/arm/mach-davinci/Makefile b/arch/arm/mach- > davinci/Makefile > > old mode 100644 > > new mode 100755 > > index 6da7b63..7323c83 > > --- a/arch/arm/mach-davinci/Makefile > > +++ b/arch/arm/mach-davinci/Makefile > > @@ -9,12 +9,18 @@ obj-y := time.o clock.o serial.o io.o > psc.o \ > > sram.o > > > > obj-$(CONFIG_DAVINCI_MUX) += mux.o > > +obj-$(CONFIG_PCI) += pci-generic.o > > You add files to the Makefile which are not added in this patch. > This will break bisecting. > > > # Chip specific > > obj-$(CONFIG_ARCH_DAVINCI_DM644x) += dm644x.o > > obj-$(CONFIG_ARCH_DAVINCI_DM646x) += dm646x.o > > obj-$(CONFIG_ARCH_DAVINCI_DM355) += dm355.o > > > > +# PCI - Chip specific > > +ifeq ($(CONFIG_PCI), y) > > +obj-$(CONFIG_ARCH_DAVINCI_DM646x) += pci-dm6467.o > > +endif > > + > > Ditto. > I will move Kconfig and Makefile patches to PCI driver patch set. > [...] > > Kevin > Thanks for your comments! Hemant _______________________________________________ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
