On Fri, Nov 06, 2009 at 04:02:17PM +0100, Marek Szyprowski wrote: > Hello, > > On Friday, November 06, 2009 4:19 AM, Harald Welte wrote: > > > Hi Marek, > > > > On Tue, Oct 13, 2009 at 10:11:07AM +0200, Marek Szyprowski wrote: > > > > > S5PC110 and S5PC100 register maps differs in many places, rename all > > > defined registers to be S5PC100 specific. System map has been also updated > > > to cover more integrated peripherals. > > > > The general idea of this patch is fine. However, I have some questions: > > > > > /* System */ > > > -#define S5PC100_PA_SYS (0xE0100000) > > > -#define S5PC100_PA_CLK (S5PC100_PA_SYS + 0x0) > > > -#define S5PC100_PA_PWR (S5PC100_PA_SYS + 0x8000) > > > +#define S5PC100_PA_CLK (0xE0100000) > > > +#define S5PC100_PA_CLK_OTHER (0xE0200000) > > > +#define S5PC100_PA_PWR (0xE0108000) > > > > this is more like a rename. Why was this done? It would be good to explain in > > the commitlog > > I renamed these registers to better match the chip specification. The 'SYS' > register name was borrowed from S3C64XX series and is a bit inadequate in C100. > > > > +/* GPIO */ > > > +#define S5PC100_PA_GPIO (0xE0300000) > > > +#define S5PC1XX_PA_GPIO S5PC100_PA_GPIO > > > +#define S5PC1XX_VA_GPIO S3C_ADDR(0x00500000) > > > > If the address is different for c100 and c110: why do we need a S5CP1XX_* > > definition? In my personal opinion, all those compile-time defines are a > > kludge and we should not introduce more of them. They will bite us in the back > > if we ever in the future want to build a kernel that can boot on both c100 and > > c110. > > These C1XX defines were the first step to prepare the code for C110 support. > > S5PC110 register map differs completely from the S5PC100 one. These two SOCs > cannot be easily handled by the same kernel binary image without some hacks and > runtime fixups if we place them in the one kernel platform. Creating yet another > kernel platform just because of these differences would unnecessarily duplicate > a lot of code and would not use the potential of the current kernel > infrastructure (separate include directories, resource&device driver model, etc). can people line-wrap their emails a bit more please.. > My idea was to introduce a sub-platforms in plat-s5pc1xx. Such sub-platforms > would be exclusive - one for C100 and one for C110. Each of the sub-platforms would > have its own include files (in mach-s5pc100 and mach-s5pc110 directories > respectively) and most of the chip differences can be handled in compile time by > proper macros. Macros for the common resources would use 'C1XX' names. In this > approach common resources (GPIO, DMA, io space and so on) can be easily defined > with C1XX defines. This also perfectly matches the current convention of common > S3C_XXX defines (i.e. S3C_PA_UART, S3C_PA_FB, S3C_VA_VICn, S3C_PA_HSMMCn, and so > on), so most of the code from arch/arm/plat-s3c/ can be reused without any > modifications. I assume that the S3C prefix would be renamed to SAMSUNG sometime > later. I will be working on this and hopefully have some stuff soon to try and remove a lot of these problems. Harald and I spent some time on this subject when we where last together. -- Ben Q: What's a light-year? A: One-third less calories than a regular year. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
 |