Khasim Syed Mohammed said the following on 01/09/2010 09:16 PM: > On Sat, Jan 9, 2010 at 8:18 PM, Nishanth Menon <menon.nisha...@gmail.com> > wrote: >> Khasim Syed Mohammed said the following on 01/08/2010 09:00 PM: >>> On Sat, Jan 9, 2010 at 1:31 AM, Nishanth Menon <menon.nisha...@gmail.com> >>> wrote: >>> >>>> On Fri, Jan 8, 2010 at 9:40 AM, Khasim Syed Mohammed >>>> <kha...@beagleboard.org> wrote: >>>> >>>>> From 239c47a4180fb4d5b5217f892955524d476916cf Mon Sep 17 00:00:00 2001 >>>>> From: Syed Mohammed Khasim <kha...@ti.com> [...]
>> The recomendation here is to move from #defines to struct based register >> usage. I am ok with the rest(except for need to split). > Split is done, posted yesterday. > > Struct based register needs more comments, not that I am lazy to > implement that. I need to know the reason for doing the same when no > multiple instances are used. > >>> You can add a new panel or a new tv standard with these structures >>> easily. Structures are not used for register accesses. >>> >>> >>>> here is what I think: >>>> venc_config { >>>> } >>>> >>>> if it is organized as the register definitions, >>>> >>>> configure_venc(struct venc_config *values) >>>> struct venc_config * d = BASE_ADDRESS_OF_OMAP3_VENC; >>>> writel(values->regx, &d->regx); >>>> >>>> refer: drivers/mtd/nand/omap_gpmc.c >>>> >>>> >>> GPIO, GPMC and other controllers have multiple instances in OMAP, it >>> makes sense to organize such register set in structure mode. I did >>> start with that but found no use for DSS as it is just one instance. >>> Structures don't give any value here. >>> >> there were other reasons mentioned when nand got split -> one of them had to >> do with the compiler or something. Dirk might remember - unfortunately, this >> was more than a year back.. if I recollect right.. > Will try doing a google. May be some one can point me to that > decision. It would help developing drivers which have single instance > of controller being used. the reference I got: http://old.nabble.com/-U-Boot---PATCH-08-13-v4--ARM%3A-OMAP3%3A-Add-NAND-support-tp20039673p20039673.html V5 became: http://old.nabble.com/-U-Boot---PATCH-07-13-v5--ARM%3A-OMAP3%3A-Add-NAND-support-tp20292477p20292477.html similar changes happend for GPMC etc.. Quote: > >Is GPMC_BASE an integer or a pointer? > > Nothing. A macro: > > #define OMAP34XX_GPMC_BASE (0x6E000000) > #define GPMC_BASE (OMAP34XX_GPMC_BASE) So it's an integer. > It's then casted to volatile pointer by ARM's readx/writex. The cast should be done by the driver, or you'll get warnings if readx/writex ever become inline functions (as they are on other arches). might help explain.. > >>> More over I am introducing minimal DSS driver with minimal register >>> set. It doesn't help any to give structure based register access for >>> single instance drivers. >>> >> moving to struct based is easy and done once and improves your chance of >> your driver getting upstreamed :). > DSS in OMAP3 has following register domains. > > DSI Protocol Engine 0x4804 FC00 512 bytes > DSI_PHY 0x4804 FE00 64 bytes > DSI PLL Controller 0x4804 FF00 32 bytes > Display Subsystem 0x4805 0000 512 bytes > Display Controller 0x4805 0400 1K byte > Display Controller VID1 0x4805 0400 1K byte > Display Controller VID2 0x4805 0400 1K byte > RFBI 0x4805 0800 256 bytes > Video Encode 0x4805 0C00 256 bytes > > I am not sure why one would ask me to give struct definitions for > these 500 (approx) registers when only 50 of these are required to > implement background and color bar. As I am saying all the way, DSS is > not multiple instance module like GPMC (NAND) and GPIO it is just one > module. Aren't you extrapolating this a bit out of scope? DSI,RFBI etc.. is not relevant to your patch. you may need DSS, controller and VID1(and VID2 is the same). I think your complaint is about having to define the reg structs when multiple instances dont exist - how about OMAP4? wont these structs get reused there(once we get around to it)? Regards, Nishanth Menon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot