[U-Boot] [beagleboard] TI:OMAP: [PATCH 4/4] Minimal Display driver for OMAP3

[Nishanth Menon]

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.nishanth at 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.nishanth at gmail.com>
>>> wrote:
>>>
>>>> On Fri, Jan 8, 2010 at 9:40 AM, Khasim Syed Mohammed
>>>> <khasim at beagleboard.org> wrote:
>>>>
>>>>> From 239c47a4180fb4d5b5217f892955524d476916cf Mon Sep 17 00:00:00 2001
>>>>> From: Syed Mohammed Khasim <khasim at 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