Hello Stefan, Stefan Althoefer wrote: > Dear All, > >> Dear Anatolij & Stefan, >> >> In message <[EMAIL PROTECTED]> 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 <video_fb.h> #include <sm501.h> - -#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 <pci.h> +#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: [EMAIL PROTECTED] _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
