Re: [U-Boot] [PATCH] video: Add new driver for Silicon Motion SM501/SM502 Part 2/2

Hello Stefan,

Stefan Althoefer wrote:

[PATCH] video: Add new driver for Silicon Motion SM501/SM502

This patch adds a new driver for SM501/SM502. Compared to the existing driver it allows dynamic selection of resolution (environment: videomode).

The drive is based on Vincent Sanders and Ben Dooks' linux kernel driver.

Use CONFIG_VIDEO_SM501NEW to enable the driver.

This has been tested on Janz emPC-A400. On this platform the SM501 is connected via PCI.

The patch is against "latest" u-boot git-repository

Please (still) be patient if style of submission or patches are offending.

Signed-off-by: Stefan Althoefer stefan.althoefer@web.de

see patch header comments in previous email, same applies here.

<snip>

diff -uprN u-boot-orig//drivers/video/sm501new.c u-boot/drivers/video/sm501new.c --- u-boot-orig//drivers/video/sm501new.c 1970-01-01 01:00:00.000000000 +0100 +++ u-boot/drivers/video/sm501new.c 2008-12-03 11:47:22.000000000 +0100 @@ -0,0 +1,1533 @@ +/* Large parts of this have been taken from the linux kernel source code

• • with the following licencse:
• • Copyright (c) 2006 Simtec Electronics

• •  Vincent Sanders <vince@simtec.co.uk>
    

• •  Ben Dooks <ben@simtec.co.uk>
    

• • This program is free software; you can redistribute it and/or modify
• • it under the terms of the GNU General Public License version 2 as
• • published by the Free Software Foundation.
• • Framebuffer driver for the Silicon Motion SM501
• */

+/* Ported to u-boot:

• • Copyright (c) 2008 StefanAlthoefer (as@janz.de)
• */

+#include <common.h> +#include <exports.h>

is exports.h needed here?

+#include <config.h> +#include <watchdog.h>

is watchdog.h really needed?

+#include <command.h> +#include <image.h>

image.h ?

+#include <asm/byteorder.h> +#include <pci.h> +#include <video_fb.h> +#include <video.h> +#include "videomodes.h" +#include "sm501new-regs.h" +#include "sm501new.h"

+#ifdef CONFIG_VIDEO_SM501NEW

please, remove above #ifdef and corresponding #endif. Conditional compiling is already handled by Makefile.

+#undef DEBUG

+/* this should be in pci_ids.h */ +#define PCI_DEVICE_ID_SMI_501 0x0501

please, remove above comment and move PCI_DEVICE_ID_SMI_501 definition to include/pci_ids.h

+#define BIG_ENDIAN_HOST +#define VIDEO_MEM_SIZE (4*1024*1024)

+#define SM501_FRAMEBUFFER_ADDR 0

+#define HEAD_CRT 0 +#define HEAD_PANEL 1

+#if defined(BIG_ENDIAN_HOST) +static inline unsigned int LONGSWAP(unsigned int x) +{

• return (

• ((x<<24) & 0xff000000) |
  

• ((x<< 8) & 0x00ff0000) |
  

• ((x>> 8) & 0x0000ff00) |
  

• ((x>>24) & 0x000000ff) );
  

+} +static inline unsigned int readl(void *addr) +{

• return LONGSWAP((*(volatile unsigned int *)(addr)));

+} +static inline void writel(unsigned int data, void *addr) +{

• /*printf("%p <- %x\n", addr, data); */
• *(volatile unsigned int *)(addr) = LONGSWAP(data);

+} +#else +#define readl(addr) (*(volatile unsigned int*)(addr)) +#define writel(b,addr) ((*(volatile unsigned int *) (addr)) = (b)) +#endif

please, use existing code for readl, writel and byte swapping.

<snip>

+struct sm501_devdata {

• struct sm501_platdata *platdata;
• unsigned long pm_misc;
• int unit_power[20];
• unsigned int pdev_id;
• unsigned int irq;
• void *regs;
• void *dc;
• void *vmem;

pm_misc, pdev_id and irq are not used in the code, so remove them.

• GraphicDevice *gd;

gd is asigned but not dereferenced in the code, another candidate to remove?

• struct ctfb_res_modes *mode;
• int bits_per_pixel;
• int xres_virtual;
• int yres_virtual;

+};

+struct sm501_devdata smi_devd;

+static void mdelay(unsigned int delay) +{

• while( delay-- > 0 ){

• udelay(1000);
  

• }

+}

+#define MHZ (1000 * 1000)

+#ifdef DEBUG +#define dev_dbg(XXX,...) printf(__VA_ARGS__) +#define dev_info(XXX,...) printf(__VA_ARGS__) +#define dev_err(XXX,...) printf(__VA_ARGS__)

please, use existing debug() macro for debugging purposes, it's in include/common.h.

Debug is debug, info is info, error is error. It is a good idea to report errors unconditionally, I mean independent of DEBUG definition. Errors should always be reported, so I suggest using printf() for error messages. Many dev_info() calls in this driver should be changed to debug() calls, see appropriate comments below. For some short info messages (chip version info, video memory size) printf() is enough, I think. Also, first argument isn't used, so simply drop it.

<snip>

+static void sm501_dump_regs(struct sm501_devdata *sm) +{

sm501_dump_regs() is defined but not used. Please, use it or drop it entirely.

• void *regs = sm->regs;
• dev_info(sm->dev, "System Control %08x\n",

• 	readl(regs + SM501_SYSTEM_CONTROL));
  

Actually, this is debug output, so please use debug() macro. This comment applies if sm501_dump_regs() is going to be used.

<snip>

+static void sm501_dump_gate(struct sm501_devdata *sm) +{

• dev_info(sm->dev, "CurrentGate %08x\n",

• 	readl(sm->regs + SM501_CURRENT_GATE));
  

• dev_info(sm->dev, "CurrentClock %08x\n",

• 	readl(sm->regs + SM501_CURRENT_CLOCK));
  

• dev_info(sm->dev, "PowerModeControl %08x\n",

• 	readl(sm->regs + SM501_POWER_MODE_CONTROL));
  

please, use debug() instead of dev_info() here.

<snip>

+static inline void sm501_mdelay(struct sm501_devdata *sm, unsigned int delay) +{

• mdelay(delay);

+}

please, drop sm501_mdelay() and simply use mdelay().

<snip>

+unsigned long sm501_gpio_get(struct sm501_devdata *sm,

• 	     unsigned long gpio)
  

+{

unused code, please remove it.

<snip>

+void sm501_gpio_set(struct sm501_devdata *sm,

•     unsigned long gpio,
  

•     unsigned int to,
  

•     unsigned int dir)
  

+{

ditto.

<snip>

+/* sm501_init_dev

• • Common init code for an SM501

+*/

+static int sm501_init_dev(struct sm501_devdata *sm) +{

• unsigned long mem_avail;
• unsigned long dramctrl;
• unsigned long devid;
• int ret;
• devid = readl(sm->regs + SM501_DEVICEID);
• if ((devid & SM501_DEVICEID_IDMASK) != SM501_DEVICEID_SM501) {

• dev_err(sm->dev, "incorrect device id %08lx\n", devid);
  

printf() for error messages is ok.

• return -1;
  

• }
• dramctrl = readl(sm->regs + SM501_DRAM_CONTROL);
• mem_avail = sm501_mem_local[(dramctrl >> 13) & 0x7];
• dev_info(sm->dev, "SM501 At %p: Version %08lx, %ld Mb, IRQ %d\n",

•  sm->regs, devid, (unsigned long)mem_avail >> 20, sm->irq);
  

IRQ doesn't make sense here, so I suggest to drop it. Also use debug() instead of dev_info().

• sm501_dump_gate(sm);
• sm501_dump_clk(sm);
• /* check to see if we have some device initialisation */
• if (sm->platdata) {

• struct sm501_platdata *pdata = sm->platdata;
  

• if (pdata->init) {
  

• 	sm501_init_regs(sm, sm->platdata->init);
  

• }
  

• }
• ret = sm501_check_clocks(sm);
• if (ret) {

• dev_err(sm->dev, "M1X and M clocks sourced from different "
  

• 			"PLLs\n");
  

• return -1;
  

• }
• return 0;

+}

please remove empty lines, max. 2 empty lines allowed.

+/* Initialisation data for PCI devices */

+static struct sm501_initdata sm501_pci_initdata = {

• .gpio_high = {

• .set	= 0x3F000000,		/* 24bit panel */
  

• .mask	= 0x0,
  

• },
• .misc_timing = {

• .set	= 0x010100,		/* SDRAM timing */
  

• .mask	= 0x1F1F00,
  

• },
• .misc_control = {

• .set	= SM501_MISC_PNL_24BIT,
  

• .mask	= 0,
  

• },
• .devices = SM501_USE_ALL,

"devices" isn't used in the code, a candidate to remove?

• /* Errata AB-3 says that 72MHz is the fastest available

• * for 33MHZ PCI with proper bus-mastering operation */
  

• .mclk = 72 * MHZ,
• .m1xclk = 144 * MHZ,

+};

+static struct sm501_platdata_fbsub sm501_pdata_fbsub = {

• .flags = (SM501FB_FLAG_USE_INIT_MODE |

• 	   SM501FB_FLAG_USE_HWCURSOR |
  

• 	   SM501FB_FLAG_USE_HWACCEL |
  

• 	   SM501FB_FLAG_DISABLE_AT_EXIT),
  

+};

+static struct sm501_platdata_fb sm501_fb_pdata = {

• .fb_route = SM501_FB_OWN,
• .fb_crt = &sm501_pdata_fbsub,
• .fb_pnl = &sm501_pdata_fbsub,

+};

+static struct sm501_platdata sm501_pci_platdata = {

• .init = &sm501_pci_initdata,
• .fb = &sm501_fb_pdata,

+};

sm501_fb_pdata isn't referenced in the code, so please remove ".fb = &sm501_fb_pdata," in the sm501_pci_platdata declaration. Also drop sm501_fb_pdata and sm501_pdata_fbsub declarations above.

max. 2 empty lines.

<snip>

+static int sm501fb_set_par_common(struct sm501_devdata *sm, int head) +{

• struct ctfb_res_modes *var = sm->mode;
• unsigned long pixclock; /* pixelclock in Hz */
• unsigned long sm501pixclock; /* pixelclock the 501 can achive in Hz */
• /*unsigned int mem_type;*/
• unsigned int clock_type;
• unsigned int head_addr;
• dev_dbg(fbi->dev, "%s: %dx%d, bpp = %d, virtual %dx%d\n",

• __func__, var->xres, var->yres, sm->bits_per_pixel,
  

• sm->xres_virtual, sm->yres_virtual);
  

• switch (head) {
• case HEAD_CRT:

• /*mem_type = SM501_MEMF_CRT;*/
  

• clock_type = SM501_CLOCK_V2XCLK;
  

• head_addr = SM501_DC_CRT_FB_ADDR;
  

• break;
  

• case HEAD_PANEL:

• /*mem_type = SM501_MEMF_PANEL;*/
  

• clock_type = SM501_CLOCK_P2XCLK;
  

• head_addr = SM501_DC_PANEL_FB_ADDR;
  

• break;
  

• default:

• /*mem_type = 0;*/		/* stop compiler warnings */
  

• head_addr = 0;
  

• clock_type = 0;
  

• }

+#if 0

• switch (sm->bits_per_pixel) {
• case 8:

• info->fix.visual = FB_VISUAL_PSEUDOCOLOR;
  

• break;
  

• case 16:

• info->fix.visual = FB_VISUAL_DIRECTCOLOR;
  

• break;
  

• case 32:

• info->fix.visual = FB_VISUAL_TRUECOLOR;
  

• break;
  

• }
• /* allocate fb memory within 501 */
• info->fix.line_length = (var->xres_virtual * var->bits_per_pixel)/8;
• info->fix.smem_len = info->fix.line_length * var->yres_virtual;
• dev_dbg(fbi->dev, "%s: line length = %u\n", __func__,

• info->fix.line_length);
  

• if (sm501_alloc_mem(fbi, &par->screen, mem_type,

• 	    info->fix.smem_len)) {
  

• dev_err(fbi->dev, "no memory available\n");
  

• return -ENOMEM;
  

• }
• info->fix.smem_start = fbi->fbmem_res->start + par->screen.sm_addr;
• info->screen_base = fbi->fbmem + par->screen.sm_addr;
• info->screen_size = info->fix.smem_len;

+#endif

please remove unused code (between #if 0 and #endif).

<snip>

+/* sm501fb_pan_crt

• • pan the CRT display output within an virtual framebuffer

+*/ +#if 0 +static int sm501fb_pan_crt(struct sm501_devdata *sm) +{

• struct ctfb_res_modes *var = sm->mode;
• unsigned int bytes_pixel = sm->bits_per_pixel / 8;
• unsigned long reg;
• unsigned long xoffs;
• xoffs = /*var->xoffset*/0 * bytes_pixel;
• reg = readl(sm->dc + SM501_DC_CRT_CONTROL);
• reg &= ~SM501_DC_CRT_CONTROL_PIXEL_MASK;
• reg |= ((xoffs & 15) / bytes_pixel) << 4;
• writel(reg, sm->dc + SM501_DC_CRT_CONTROL);
• reg = (par->screen.sm_addr + xoffs +

•       /*var->yoffset*/0 * info->fix.line_length);
  

• writel(reg | SM501_ADDR_FLIP, fbi->dc + SM501_DC_CRT_FB_ADDR);
• sm501fb_sync_regs(sm);
• return 0;

+} +#endif

unused code, please remove.

+/* sm501fb_pan_pnl

• • pan the panel display output within an virtual framebuffer

+*/ +static int sm501fb_pan_pnl(struct sm501_devdata *sm) +{

• /*struct ctfb_res_modes *var = sm->mode;*/

drop above unused code.

• unsigned long reg;
• reg = /*var->xoffset*/0 | (sm->xres_virtual << 16);

remove /*var->xoffset*/.

• writel(reg, sm->dc + SM501_DC_PANEL_FB_WIDTH);
• reg = /*var->yoffset*/0 | (sm->yres_virtual << 16);

Ditto.

• writel(reg, sm->dc + SM501_DC_PANEL_FB_HEIGHT);
• sm501fb_sync_regs(sm);
• return 0;

+}

+/* sm501fb_set_par_crt

• • Set the CRT video mode from the fb_info structure

+*/

+static int sm501fb_set_par_crt(struct sm501_devdata *sm) +{

• struct ctfb_res_modes *var = sm->mode;
• unsigned long control; /* control register */
• int ret;
• /* activate new configuration */
• dev_dbg(fbi->dev, "%s(%p)\n", __func__, sm);
• /* enable CRT DAC - note 0 is on!*/
• sm501_misc_control(sm, 0, SM501_MISC_DAC_POWER);
• control = readl(sm->dc + SM501_DC_CRT_CONTROL);
• dev_dbg(fbi->dev, "old control is %08lx\n", control);
• control &= (SM501_DC_CRT_CONTROL_PIXEL_MASK |

•     SM501_DC_CRT_CONTROL_GAMMA |
  

•     SM501_DC_CRT_CONTROL_BLANK |
  

•     SM501_DC_CRT_CONTROL_SEL |
  

•     SM501_DC_CRT_CONTROL_CP |
  

•     SM501_DC_CRT_CONTROL_TVP);
  

• /* set the sync polarities before we check data source */
• if ((var->sync & FB_SYNC_HOR_HIGH_ACT) == 0)

• control |= SM501_DC_CRT_CONTROL_HSP;
  

• if ((var->sync & FB_SYNC_VERT_HIGH_ACT) == 0)

• control |= SM501_DC_CRT_CONTROL_VSP;
  

• if ((control & SM501_DC_CRT_CONTROL_SEL) == 0) {

• /* the head is displaying panel data... */
  

• dev_dbg(fbi->dev, "%s CRT display panel data\n", __func__);
  

• /*sm501_alloc_mem(fbi, &par->screen, SM501_MEMF_CRT, 0);*/
  

drop commented out sm501_alloc_mem code (elsewhere in the file too).

<snip>

+/* sm501fb_set_par_pnl

• • Set the panel video mode from the fb_info structure

+*/

+static int sm501fb_set_par_pnl(struct sm501_devdata *sm) +{

• struct ctfb_res_modes *var = sm->mode;
• unsigned long control;
• unsigned long reg;
• int ret;
• dev_dbg(fbi->dev, "%s(%p)\n", __func__, sm);
• /* activate this new configuration */
• ret = sm501fb_set_par_common(sm, HEAD_PANEL);
• if (ret)

• return ret;
  

• sm501fb_pan_pnl(sm);
• sm501fb_set_par_geometry(sm, HEAD_PANEL);
• /* update control register */
• control = readl(sm->dc + SM501_DC_PANEL_CONTROL);
• control &= (SM501_DC_PANEL_CONTROL_GAMMA |

•     SM501_DC_PANEL_CONTROL_VDD  |
  

•     SM501_DC_PANEL_CONTROL_DATA |
  

•     SM501_DC_PANEL_CONTROL_BIAS |
  

•     SM501_DC_PANEL_CONTROL_FPEN |
  

•     SM501_DC_PANEL_CONTROL_CP |
  

•     SM501_DC_PANEL_CONTROL_CK |
  

•     SM501_DC_PANEL_CONTROL_HP |
  

•     SM501_DC_PANEL_CONTROL_VP |
  

•     SM501_DC_PANEL_CONTROL_HPD |
  

•     SM501_DC_PANEL_CONTROL_VPD);
  

• control |= SM501_FIFO_3; /* fill if >3 free slots */
• switch(sm->bits_per_pixel) {
• case 8:

• control |= SM501_DC_PANEL_CONTROL_8BPP;
  

• break;
  

• case 16:

• control |= SM501_DC_PANEL_CONTROL_16BPP;
  

• break;
  

• case 32:

• control |= SM501_DC_PANEL_CONTROL_32BPP;
  

• /*sm501fb_setup_gamma(fbi, SM501_DC_PANEL_PALETTE);*/
  

drop commented out sm501fb_setup_gamma, also elsewhere in the file.

• break;
  

• default:

• dev_dbg(fbi->dev, "unkown pixel format\n");
  

• }
• writel(0x0, sm->dc + SM501_DC_PANEL_PANNING_CONTROL);
• /* panel plane top left and bottom right location */
• writel(0x00, sm->dc + SM501_DC_PANEL_TL_LOC);
• reg = var->xres - 1;
• reg |= (var->yres - 1) << 16;
• writel(reg, sm->dc + SM501_DC_PANEL_BR_LOC);
• /* program panel control register */
• control |= SM501_DC_PANEL_CONTROL_TE; /* enable PANEL timing */
• control |= SM501_DC_PANEL_CONTROL_EN; /* enable PANEL gfx plane */
• if ((var->sync & FB_SYNC_HOR_HIGH_ACT) == 0)

• control |= SM501_DC_PANEL_CONTROL_HSP;
  

• if ((var->sync & FB_SYNC_VERT_HIGH_ACT) == 0)

• control |= SM501_DC_PANEL_CONTROL_VSP;
  

• writel(control, sm->dc + SM501_DC_PANEL_CONTROL);
• sm501fb_sync_regs(sm);
• /* power the panel up */
• sm501fb_panel_power(sm, 1);
• return 0;

+}

+void video_set_lut(

• unsigned int index, /* color number */
• unsigned char r, /* red */
• unsigned char g, /* green */
• unsigned char b /* blue */
• )

+{

• /* FIXME: to be done */

+}

+/*******************************************************************************

• • Init video chip with common Linux graphic modes (lilo)
• */

+void *video_hw_init(void) +{

• GraphicDevice *pGD = (GraphicDevice *)&smi;
• unsigned short device_id;
• pci_dev_t devbusfn;
• int videomode;
• unsigned long t1, hsynch, vsynch;
• unsigned int pci_mem_base, *vm;
• unsigned int pci_reg_base;
• char *penv;
• int tmp, i, bits_per_pixel;
• int vmem_size;
• struct ctfb_res_modes *res_mode;
• struct ctfb_res_modes var_mode;
• /* Search for video chip */
• printf("Video: ");
• if ((devbusfn = pci_find_devices(supported, 0)) < 0)
• {

• printf ("Controller not found !\n");
  

• return (NULL);
  

• }
• /* PCI setup */
• pci_write_config_dword (devbusfn, PCI_COMMAND, (PCI_COMMAND_MEMORY | PCI_COMMAND_IO));
• pci_read_config_word (devbusfn, PCI_DEVICE_ID, &device_id);
• pci_read_config_dword (devbusfn, PCI_BASE_ADDRESS_0, &pci_mem_base);
• /*pci_mem_base = pci_mem_to_phys (devbusfn, pci_mem_base);*/
• pci_read_config_dword (devbusfn, PCI_BASE_ADDRESS_1, &pci_reg_base);
• /*pci_reg_base = pci_mem_to_phys (devbusfn, pci_reg_base);*/
• dev_dbg(xxx, "mem_base=0x%x reg_base=0x%x\n", pci_mem_base, pci_reg_base);
• tmp = 0;
• videomode = CONFIG_SYS_DEFAULT_VIDEO_MODE;
• /* get video mode via environment */
• if ((penv = getenv ("videomode")) != NULL) {

• /* deceide if it is a string */
  

• if (penv[0] <= '9') {
  

• 	videomode = (int) simple_strtoul (penv, NULL, 16);
  

• 	tmp = 1;
  

• }
  

• } else {

• tmp = 1;
  

• }
• if (tmp) {

• /* parameter are vesa modes */
  

• /* search params */
  

• for (i = 0; i < VESA_MODES_COUNT; i++) {
  

• 	if (vesa_modes[i].vesanr == videomode)
  

• 		break;
  

• }
  

• if (i == VESA_MODES_COUNT) {
  

• 	printf ("no VESA Mode found, switching to mode 0x%x ", CONFIG_SYS_DEFAULT_VIDEO_MODE);
  

above line too long, max. 80 characters please.

• 	i = 0;
  

• }
  

• res_mode =
  

• 	(struct ctfb_res_modes *) &res_mode_init[vesa_modes[i].
  

• 						 resindex];
  

• bits_per_pixel = vesa_modes[i].bits_per_pixel;
  

• } else {

• res_mode = (struct ctfb_res_modes *) &var_mode;
  

• bits_per_pixel = video_get_params (res_mode, penv);
  

• }
• /* calculate hsynch and vsynch freq (info only) */
• t1 = (res_mode->left_margin + res_mode->xres +

•      res_mode->right_margin + res_mode->hsync_len) / 8;
  

• t1 *= 8;
• t1 *= res_mode->pixclock;
• t1 /= 1000;
• hsynch = 1000000000L / t1;
• t1 *=

• (res_mode->upper_margin + res_mode->yres +
  

•  res_mode->lower_margin + res_mode->vsync_len);
  

• t1 /= 1000;
• vsynch = 1000000000L / t1;
• /* fill in Graphic device struct */
• sprintf (pGD->modeIdent, "%dx%dx%d %ldkHz %ldHz", res_mode->xres,

•  res_mode->yres, bits_per_pixel, (hsynch / 1000),
  

•  (vsynch / 1000));
  

• printf ("%s\n", pGD->modeIdent);
• pGD->winSizeX = res_mode->xres;
• pGD->winSizeY = res_mode->yres;
• pGD->plnSizeX = res_mode->xres;
• pGD->plnSizeY = res_mode->yres;
• switch (bits_per_pixel) {
• case 8:

• pGD->gdfBytesPP = 1;
  

• pGD->gdfIndex = GDF__8BIT_INDEX;
  

• break;
  

• case 15:

• pGD->gdfBytesPP = 2;
  

• pGD->gdfIndex = GDF_15BIT_555RGB;
  

• break;
  

• case 16:

• pGD->gdfBytesPP = 2;
  

• pGD->gdfIndex = GDF_16BIT_565RGB;
  

• break;
  

• case 24:

• pGD->gdfBytesPP = 4;
  

• pGD->gdfIndex = GDF_32BIT_X888RGB;
  

• break;
  

• case 32:

• pGD->gdfBytesPP = 4;
  

• pGD->gdfIndex = GDF_32BIT_X888RGB;
  

• break;
  

• }
• pGD->isaBase = 0;
• pGD->pciBase = pci_mem_base;
• pGD->dprBase = pci_reg_base;
• pGD->vprBase = 0;
• pGD->cprBase = 0;
• pGD->frameAdrs = pci_mem_base;
• pGD->memSize = VIDEO_MEM_SIZE;
• smi_devd.platdata = &sm501_pci_platdata;
• smi_devd.regs = (void *)pci_reg_base;
• smi_devd.dc = (void *)pci_reg_base + 0x80000;
• smi_devd.vmem = (void *)pci_mem_base;
• smi_devd.gd = pGD;

"smi_devd.gd" isn't referenced elsewhere in the code, a candidate to remove?

• smi_devd.mode = res_mode;
• switch (bits_per_pixel) {
• case 8: smi_devd.bits_per_pixel = 8; break;
• case 16: smi_devd.bits_per_pixel = 16; break;
• case 24: smi_devd.bits_per_pixel = 32; break;
• default: smi_devd.bits_per_pixel = 32; break;
• }
• smi_devd.xres_virtual = pGD->plnSizeX;
• smi_devd.yres_virtual = pGD->plnSizeY;
• switch( (readl(smi_devd.regs + SM501_DRAM_CONTROL) >> 13) & 0x7 ){
• case 0: vmem_size = 4*1024*1024; break;
• case 1: vmem_size = 8*1024*1024; break;
• case 2: vmem_size = 16*1024*1024; break;
• case 3: vmem_size = 32*1024*1024; break;
• case 4: vmem_size = 64*1024*1024; break;
• case 5: vmem_size = 2*1024*1024; break;
• default: vmem_size = 2*1024*1024; break;
• }
• printf("SM501: %d MB Video memory\n", vmem_size/(1024*1024));
• /* Blank video memory */
• vm = (unsigned int *)pGD->frameAdrs;
• for(i=0; i < vmem_size/sizeof(int); i++){

• *vm++ = 0;
  

• }
• sm501_init_dev(&smi_devd);
• /* enable display controller */
• sm501_unit_power(&smi_devd, SM501_GATE_DISPLAY, 1);

+#if 0

• /* setup cursors */
• sm501_init_cursor(info->fb[HEAD_CRT], SM501_DC_CRT_HWC_ADDR);
• sm501_init_cursor(info->fb[HEAD_PANEL], SM501_DC_PANEL_HWC_ADDR);

+#endif

unused code, please remove it.

• /*

• * Panel setup
  

• */
  

• sm501fb_set_par_pnl(&smi_devd);
• /*

• * CRT setup (we want CRT display panel data)
  

• */
  

• sm501fb_set_par_crt(&smi_devd);
• return ((void*)&smi);

+}

+#if 0 +int t_smi (int argc, char *argv[]) +{

• int i;
• ulong ret;
• ulong t,t1;
• app_startup(argv);
• pci_init();
• drv_video_init();

+#if 0

• video_puts("loading ...\n");
• video_puts("be patient\n");

+#endif

+#if 0

• if (argc == 1) {

• /* Print the ABI version */
  

• printf("Video hw Init\n");
  

• pci_init();
  

• video_hw_init();
  

• return 0;
  

• } else {

• printf("Video Init\n");
  

• pci_init();
  

• drv_video_init();
  

• }

+#endif +} +#endif

unused code, please drop it.

Also check for proper multi-line comment style, please.

Best regards, Anatolij