On Sat, Jan 9, 2010 at 1:31 AM, Nishanth Menon menon.nishanth@gmail.com wrote:
On Fri, Jan 8, 2010 at 9:40 AM, Khasim Syed Mohammed khasim@beagleboard.org wrote:
From 239c47a4180fb4d5b5217f892955524d476916cf Mon Sep 17 00:00:00 2001 From: Syed Mohammed Khasim khasim@ti.com Date: Fri, 8 Jan 2010 21:01:44 +0530 Subject: [PATCH] Minimal Display driver for OMAP3
Supports dynamic configuration of Panel and Video Encoder Supports Background color on DVID Supports Color bar on S-Video
We are getting there.. thanks a bunch. if you can split this series into two sets: a) introducing DSS layer b) introduce beagle support for the same it will be better.
Can you complete your review review comments here, I will take up this when in-corporate all review comments together.
Kindly remember these patches will be tested on B4, C2, C3, C4, EVM before releasing.
but NAK to this patch.
Signed-off-by: Syed Mohammed Khasim khasim@ti.com
board/ti/beagle/beagle.c | 13 +++ board/ti/beagle/beagle.h | 73 ++++++++++++++ drivers/video/Makefile | 1 + drivers/video/omap3_dss.c | 128 +++++++++++++++++++++++++ include/asm-arm/arch-omap3/dss.h | 193 ++++++++++++++++++++++++++++++++++++++ include/configs/omap3_beagle.h | 1 + 6 files changed, 409 insertions(+), 0 deletions(-) create mode 100644 drivers/video/omap3_dss.c create mode 100644 include/asm-arm/arch-omap3/dss.h
[...]
diff --git a/include/asm-arm/arch-omap3/dss.h b/include/asm-arm/arch-omap3/dss.h new file mode 100644 index 0000000..08c7d8d --- /dev/null +++ b/include/asm-arm/arch-omap3/dss.h @@ -0,0 +1,193 @@ +/*
- (C) Copyright 2010
- Texas Instruments, <www.ti.com>
- Syed Mohammed Khasim khasim@ti.com
- Referred to Linux DSS driver files for OMAP3
- See file CREDITS for list of people who contributed to this
- project.
- This program is free software; you can redistribute it and/or
- modify it under the terms of the GNU General Public License as
- published by the Free Software Foundation's version 2 of
- the License.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- You should have received a copy of the GNU General Public License
- along with this program; if not, write to the Free Software
- Foundation, Inc., 59 Temple Place, Suite 330, Boston,
- MA 02111-1307 USA
- */
+#ifndef DSS_H +#define DSS_H
+/* VENC Register address */ +#define VENC_REV_ID 0x48050C00
NAK. why do you need this if you have a struct?
You need to read patch more carefully before giving NAK.
Structure is used to give multiple instance of Panels or different VENC settings like NTSC or PAL
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.
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.
Give more reasons for NAK.
Regards, Khasim