From mboxrd@z Thu Jan 1 00:00:00 1970 From: Khasim Syed Mohammed Date: Sat, 9 Jan 2010 08:30:38 +0530 Subject: [U-Boot] [beagleboard] TI:OMAP: [PATCH 4/4] Minimal Display driver for OMAP3 In-Reply-To: <782515bb1001081201j56cfa78dyd136e0daab3adcb9@mail.gmail.com> References: <782515bb1001081201j56cfa78dyd136e0daab3adcb9@mail.gmail.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Sat, Jan 9, 2010 at 1:31 AM, Nishanth Menon wrote: > On Fri, Jan 8, 2010 at 9:40 AM, Khasim Syed Mohammed > wrote: >> From 239c47a4180fb4d5b5217f892955524d476916cf Mon Sep 17 00:00:00 2001 >> From: Syed Mohammed Khasim >> 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 >> --- >> ?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, >> + * Syed Mohammed Khasim >> + * >> + * 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