From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anatolij Gustschin Date: Mon, 08 Dec 2008 14:03:14 +0100 Subject: [U-Boot] [PATCH] video: Add new driver for Silicon Motion SM501/SM502 Part 1/2 In-Reply-To: References: <4938472f.bsulKM1wIgSbATIm%stefan.althoefer@web.de> <493AD29C.80409@denx.de> <20081207004728.1FB58834B020@gemini.denx.de> Message-ID: <493D1B12.8050506@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hello Stefan, Stefan Althoefer wrote: > Dear All, > >> Dear Anatolij & Stefan, >> >> In message <493AD29C.80409@denx.de> you wrote: >>>> Use CONFIG_VIDEO_SM501NEW to enable the driver. >>> not sure if CONFIG_VIDEO_SM501NEW is a good chose here. Maybe >>> we should use s.th. like CONFIG_VIDEO_SM50x. This applies to >>> the file names too: sm50x.h, sm50x.c, etc. Even better would >>> be a merge with the existing driver. >> I agree with Anatolij here. A merge would definitely be best. > > Before I start to implement the good suggestions, I'd like > to know how such a merge would look like in your opinion. see inlined patch below how it could be done without removing old sm501 code. Defining CONFIG_VIDEO_SM501_PCI enables using new sm501 driver code. > Over the long term, I see the old driver vanishing, as it is too > complicated to use (you need to be a video and SMI register > expert). It is not much more than a jump into the board specific > setup routines. However, removing the old driver would break half > a dozen of boards (inacceptable). we do not have to remove old code. Everyone who doesn't additionally define CONFIG_VIDEO_SM501_PCI will be able to use it. The advantage of the old code is that it is small. This is fundamental design principle #1 of U-Boot: http://www.denx.de/wiki/view/U-Boot/DesignPrinciples#1_Keep_it_Small I agree that the old sm501 code is suboptimal and could be improved. diff --git a/drivers/video/sm501.c b/drivers/video/sm501.c index 283d2d9..20a5335 100644 --- a/drivers/video/sm501.c +++ b/drivers/video/sm501.c @@ -33,30 +33,35 @@ #include #include - -#define read8(ptrReg) \ - *(volatile unsigned char *)(sm501.isaBase + ptrReg) - -#define write8(ptrReg,value) \ - *(volatile unsigned char *)(sm501.isaBase + ptrReg) = value - -#define read16(ptrReg) \ - (*(volatile unsigned short *)(sm501.isaBase + ptrReg)) - -#define write16(ptrReg,value) \ - (*(volatile unsigned short *)(sm501.isaBase + ptrReg) = value) - -#define read32(ptrReg) \ - (*(volatile unsigned int *)(sm501.isaBase + ptrReg)) - -#define write32(ptrReg, value) \ - (*(volatile unsigned int *)(sm501.isaBase + ptrReg) = value) +#ifdef CONFIG_VIDEO_SM501_PCI +#include +#include "videomodes.h" +#include "sm50x-regs.h" +#include "sm50x.h" +#endif GraphicDevice sm501; -/*----------------------------------------------------------------------------- - * SmiSetRegs -- - *----------------------------------------------------------------------------- +#ifdef CONFIG_VIDEO_SM501_PCI +/* + * code used in sm501_pci_init() + * comes here. + */ + +static int sm501_pci_init(void) +{ + GraphicDevice *pGD = (GraphicDevice *)&sm501; + /* + * Code to find the pci devide, setup video mode and + * init sm501 struct comes here. + * return 0 on success, non-zero otherwise. + * This is mainly the code you currently have + * in video_hw_init() in sm501new.c file. + */ +} +#else +/* + * SmiSetRegs */ static void SmiSetRegs (void) { @@ -66,7 +71,7 @@ static void SmiSetRegs (void) */ const SMI_REGS *preg = board_get_regs (); while (preg->Index) { - write32 (preg->Index, preg->Value); + writel (preg->Value, sm501.isaBase + preg->Index); /* * Insert a delay between */ @@ -74,10 +79,10 @@ static void SmiSetRegs (void) preg ++; } } +#endif -/*----------------------------------------------------------------------------- - * video_hw_init -- - *----------------------------------------------------------------------------- +/* + * video_hw_init */ void *video_hw_init (void) { @@ -85,6 +90,7 @@ void *video_hw_init (void) memset (&sm501, 0, sizeof (GraphicDevice)); +#ifndef CONFIG_VIDEO_SM501_PCI /* * Initialization of the access to the graphic chipset Retreive base * address of the chipset (see board/RPXClassic/eccx.c) @@ -122,6 +128,10 @@ void *video_hw_init (void) /* (see board/RPXClassic/RPXClassic.c) */ board_validate_screen (sm501.isaBase); +#else + if (!sm501_pci_init()) + return 0; +#endif /* CONFIG_VIDEO_SM501_PCI */ /* Clear video memory */ i = sm501.memSize/4; @@ -132,9 +142,8 @@ void *video_hw_init (void) return (&sm501); } -/*----------------------------------------------------------------------------- - * video_set_lut -- - *----------------------------------------------------------------------------- +/* + * video_set_lut */ void video_set_lut ( unsigned int index, /* color number */ -- Better suggestions are always welcome. Best regards, Anatolij DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de