"Narnakaje, Snehaprabha" <[EMAIL PROTECTED]> writes: > Kevin, > > Looks like you have made good progress on getting the GIT more > active. Thanks a lot for pushing the patches into the staging area. > > Regarding the base address definitions across DaVinci series of >devices, I am trying to summarize the changes and raise concern for >some of them:
Thank you for reviewing these changes. To summarize my overarcing goal here: the main purpose is to move all the board/chip specific settings (like base addresses) into board/chip specific code. Drivers should NEVER be using base addresses directly (like davinci_nand currently does.) Rather, board/chip code should be filling out platform_data structs and passing them into the driver. This allows the same driver to work across multiple platforms. > 1. Currently AEMIF Data CE base addresses are defined in the > arch/arm/mach-davinci/include/mach/hardware.h file. These addresses > are the same for DM644x, DM355 and DM365. However they are different > for DM6467. How do we then share and use these base addresses across > DaVinci devices? OK, this is what I was worried about. I wasn't sure about those and that's why were left in hardware.h. I now have updated docs for 6446, 6447 and 355, but not yet for 365 so I will go back over those. Basically, the more devices there are, the less likely that any of the base addresses will be common. In the end, I think hardware.h may be very sparse, and most things defined in <device>.h. What remains is whether to come up with a strategy of how to name things that are common between some devices but not others, or just duplicate them them across each <device>.h with a different <DEVICE>_ prefix. It makes a little more init code because there will more 'if cpu_is_foo()' statements, but that doesn't bother me much. In fact, it will probably be more readable. > 2. PLL base address is defined and used in > arch/arm/mach-davinci/clock.c > > 3. DMA base address is defined and used in arch/arm/mach-davinci/dma.c > > 4. GPIO base address is defined in > arch/arm/mach-davinci/include/mach/gpio.h Why do I see the same > header file in include/asm-arm/arch/davinci/ (others like serial.h > and system.h are also in both the locations) Hmm, include/asm-arm/arch-davinci is the old location and should be empty. I will delete those. This is a remnant of the 2.6.27 merge which look like I didn't quite finish cleaning up. > 5. System module and ATA base addresses are defined in > arch/arm/mach-davinci/include/mach/hardware.h. This should be OK, as > System module base address is the same across all DaVinci > devices. ATA base address is also common across DM644x and DM646x. OK, than ATA base should be moved from hardware.h into each <device>.h and the init code updated. > 6. INTC base address in arch/arm/mach-davinci/include/mach/irqs.h; > DDR and IRAM base addresses in > arch/arm/mach-davinci/include/mach/memory.h; PSC base address in > arch/arm/mach-davinci/psc.c; Timer base addresses in > arch/arm/mach-davinci/time.c and USB base address in > arch/arm/mach-davinci/usb.c. This should OK, as they are same across > all devices. > 7. UART base addresses for DaVinci are defined in > arch/arm/mach-davinci/include/mach/serial.h. But we also have a > DM355 specific definition (UART2) in > arch/arm/mach-davinci/serial.c. Should we be generalizing this and > keeping them in one single header file? I originally wanted to move them all into serial.c, but the early boot and uncompress code uses UART0 and so needs that header. I think these can stay in serial.h for now, but the DM355 one should probably be moved from serial.c into dm355.h. > 8. McBSP base addresses for DaVinci are defined > arch/arm/mach-davinci/include/mach/mcbsp.h. These definitions are > different for these EVMs. So should we be defining mcbsp_dm644x.h > mcbsp_dm355.h and so on? Actually, these should just go into <device>.h instead of mcbsp_<device>.h The current location of this to mcbsp.h was just until someone fixes the ASoC driver. There is lots of board/chip specific code in sound/soc/davinci and it should be moved into board-*.c files. Once that is done, then the board-* files will include <device>.h files, fill out the platform_data and pass it into ASoC. > 9. All other peripheral (or device driver) specific base addresses > are defined in arch/arm/mach-davinci/devices.c. Some of the base > addresses are same across DaVinci devices, but some are different > (e.g. EMAC base address is same for DM644x and DM646x, but different > for DM365). It is true for other peripherals such as MMC/SD and SPI. OK, then the thing to do here is move these defines into <device>.h files. Then devices.c will have to include each <device>.h and then use cpu_is_* at runtime and figure out which base address to use. Thanks again for your review, I'll work more on this rework next week. Kevin >> -----Original Message----- >> From: davinci-linux-open-source- >> [EMAIL PROTECTED] [mailto:davinci-linux- >> [EMAIL PROTECTED] On Behalf Of >> Kevin Hilman >> Sent: Thursday, November 06, 2008 9:06 PM >> To: Rajashekhara, Sudhakar; Paulraj, Sandeep; Subrahmanya, Chaithrika >> Cc: [email protected] >> Subject: staging tree for dm6467 and dm355 support >> >> Hello, >> >> While I review the dm6467 and dm355 patches, and prepare for an update >> to newer kernels, I've created a temporary staging branch[1] where >> I've applied the dm646x and dm355 patches. >> >> On top of that, I've added a small series of rework patches where I've >> reworked how and where base addresses are defined, init functions are >> declared. I'm very interested in your comments on this layout. >> >> Basically, what I've done is got rid of most of the base address >> definitions as global defines. The remaining ones that need to global >> and that are common to ALL chips in the family will live in >> hardware.h. Ones that need to be global and are chip specific should >> live in <chipname>.h >> >> Note that I said "need to be global". Most base address defines do >> not need to be in a global header. They are only ever used in >> chip/board specific init code to fill in platform_data which is then >> passed to the driver. Drivers should _never_ be using base address >> defines directly. >> >> I've compile and boot tested this kernel on dm6446, dm6467 and dm355. >> >> Kevin >> >> [1] See the 'tmp/staging branch in DaVinci git. >> >> http://source.mvista.com/git/?p=linux-davinci- >> 2.6.git;a=shortlog;h=refs/heads/tmp/ti-staging >> >> _______________________________________________ >> Davinci-linux-open-source mailing list >> [email protected] >> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source _______________________________________________ Davinci-linux-open-source mailing list [email protected] http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
