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

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

[Stefan Althoefer]

[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>
----

diff -uprN u-boot-orig//drivers/video/sm501new.h u-boot/drivers/video/sm501new.h
--- u-boot-orig//drivers/video/sm501new.h	1970-01-01 01:00:00.000000000 +0100
+++ u-boot/drivers/video/sm501new.h	2008-12-02 18:28:42.000000000 +0100
@@ -0,0 +1,125 @@
+/* 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 at janz.de)
+ */
+
+/*
+ * Platform data definitions 
+ */
+
+#define SM501FB_FLAG_USE_INIT_MODE	(1<<0)
+#define SM501FB_FLAG_DISABLE_AT_EXIT	(1<<1)
+#define SM501FB_FLAG_USE_HWCURSOR	(1<<2)
+#define SM501FB_FLAG_USE_HWACCEL	(1<<3)
+
+struct sm501_platdata_fbsub {
+	struct fb_videomode	*def_mode;
+	unsigned int		 def_bpp;
+	unsigned long		 max_mem;
+	unsigned int		 flags;
+};
+
+enum sm501_fb_routing {
+	SM501_FB_OWN		= 0,	/* CRT=>CRT, Panel=>Panel */
+	SM501_FB_CRT_PANEL	= 1,	/* Panel=>CRT, Panel=>Panel */
+};
+
+/* sm501_platdata_fb flag field bit definitions */
+
+#define SM501_FBPD_SWAP_FB_ENDIAN	(1<<0)	/* need to endian swap */
+
+/* sm501_platdata_fb
+ *
+ * configuration data for the framebuffer driver
+*/
+
+struct sm501_platdata_fb {
+	enum sm501_fb_routing		 fb_route;
+	unsigned int			 flags;
+	struct sm501_platdata_fbsub	*fb_crt;
+	struct sm501_platdata_fbsub	*fb_pnl;
+};
+
+/* gpio i2c */
+
+struct sm501_platdata_gpio_i2c {
+	unsigned int		pin_sda;
+	unsigned int		pin_scl;
+};
+
+/* sm501_initdata
+ *
+ * use for initialising values that may not have been setup
+ * before the driver is loaded.
+*/
+
+struct sm501_reg_init {
+	unsigned long		set;
+	unsigned long		mask;
+};
+
+#define SM501_USE_USB_HOST	(1<<0)
+#define SM501_USE_USB_SLAVE	(1<<1)
+#define SM501_USE_SSP0		(1<<2)
+#define SM501_USE_SSP1		(1<<3)
+#define SM501_USE_UART0		(1<<4)
+#define SM501_USE_UART1		(1<<5)
+#define SM501_USE_FBACCEL	(1<<6)
+#define SM501_USE_AC97		(1<<7)
+#define SM501_USE_I2S		(1<<8)
+
+#define SM501_USE_ALL		(0xffffffff)
+
+struct sm501_initdata {
+	struct sm501_reg_init	gpio_low;
+	struct sm501_reg_init	gpio_high;
+	struct sm501_reg_init	misc_timing;
+	struct sm501_reg_init	misc_control;
+
+	unsigned long		devices;
+	unsigned long		mclk;		/* non-zero to modify */
+	unsigned long		m1xclk;		/* non-zero to modify */
+};
+
+/* sm501_init_gpio
+ *
+ * default gpio settings
+*/
+
+struct sm501_init_gpio {
+	struct sm501_reg_init	gpio_data_low;
+	struct sm501_reg_init	gpio_data_high;
+	struct sm501_reg_init	gpio_ddr_low;
+	struct sm501_reg_init	gpio_ddr_high;
+};
+
+/* sm501_platdata
+ *
+ * This is passed with the platform device to allow the board
+ * to control the behaviour of the SM501 driver(s) which attach
+ * to the device.
+ *
+*/
+
+struct sm501_platdata {
+	struct sm501_initdata		*init;
+	struct sm501_init_gpio		*init_gpiop;
+	struct sm501_platdata_fb	*fb;
+
+	struct sm501_platdata_gpio_i2c	*gpio_i2c;
+	unsigned int			 gpio_i2c_nr;
+};
diff -uprN u-boot-orig//drivers/video/sm501new-regs.h u-boot/drivers/video/sm501new-regs.h
--- u-boot-orig//drivers/video/sm501new-regs.h	1970-01-01 01:00:00.000000000 +0100
+++ u-boot/drivers/video/sm501new-regs.h	2008-12-02 18:28:42.000000000 +0100
@@ -0,0 +1,365 @@
+/* sm501-regs.h
+ *
+ * Copyright 2006 Simtec Electronics
+ *
+ * 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.
+ *
+ * Silicon Motion SM501 register definitions
+*/
+
+/* System Configuration area */
+/* System config base */
+#define SM501_SYS_CONFIG		(0x000000)
+
+/* config 1 */
+#define SM501_SYSTEM_CONTROL 		(0x000000)
+#define SM501_MISC_CONTROL		(0x000004)
+
+#define SM501_MISC_BUS_SH		(0x0)
+#define SM501_MISC_BUS_PCI		(0x1)
+#define SM501_MISC_BUS_XSCALE		(0x2)
+#define SM501_MISC_BUS_NEC		(0x6)
+#define SM501_MISC_BUS_MASK		(0x7)
+
+#define SM501_MISC_VR_62MB		(1<<3)
+#define SM501_MISC_CDR_RESET		(1<<7)
+#define SM501_MISC_USB_LB		(1<<8)
+#define SM501_MISC_USB_SLAVE		(1<<9)
+#define SM501_MISC_BL_1			(1<<10)
+#define SM501_MISC_MC			(1<<11)
+#define SM501_MISC_DAC_POWER		(1<<12)
+#define SM501_MISC_IRQ_INVERT		(1<<16)
+#define SM501_MISC_SH			(1<<17)
+
+#define SM501_MISC_HOLD_EMPTY		(0<<18)
+#define SM501_MISC_HOLD_8		(1<<18)
+#define SM501_MISC_HOLD_16		(2<<18)
+#define SM501_MISC_HOLD_24		(3<<18)
+#define SM501_MISC_HOLD_32		(4<<18)
+#define SM501_MISC_HOLD_MASK		(7<<18)
+
+#define SM501_MISC_FREQ_12		(1<<24)
+#define SM501_MISC_PNL_24BIT		(1<<25)
+#define SM501_MISC_8051_LE		(1<<26)
+
+
+
+#define SM501_GPIO31_0_CONTROL		(0x000008)
+#define SM501_GPIO63_32_CONTROL		(0x00000C)
+#define SM501_DRAM_CONTROL		(0x000010)
+
+/* command list */
+#define SM501_ARBTRTN_CONTROL		(0x000014)
+
+/* command list */
+#define SM501_COMMAND_LIST_STATUS	(0x000024)
+
+/* interrupt debug */
+#define SM501_RAW_IRQ_STATUS		(0x000028)
+#define SM501_RAW_IRQ_CLEAR		(0x000028)
+#define SM501_IRQ_STATUS		(0x00002C)
+#define SM501_IRQ_MASK			(0x000030)
+#define SM501_DEBUG_CONTROL		(0x000034)
+
+/* power management */
+#define SM501_POWERMODE_P2X_SRC		(1<<29)
+#define SM501_POWERMODE_V2X_SRC		(1<<20)
+#define SM501_POWERMODE_M_SRC		(1<<12)
+#define SM501_POWERMODE_M1_SRC		(1<<4)
+
+#define SM501_CURRENT_GATE		(0x000038)
+#define SM501_CURRENT_CLOCK		(0x00003C)
+#define SM501_POWER_MODE_0_GATE		(0x000040)
+#define SM501_POWER_MODE_0_CLOCK	(0x000044)
+#define SM501_POWER_MODE_1_GATE		(0x000048)
+#define SM501_POWER_MODE_1_CLOCK	(0x00004C)
+#define SM501_SLEEP_MODE_GATE		(0x000050)
+#define SM501_POWER_MODE_CONTROL	(0x000054)
+
+/* power gates for units within the 501 */
+#define SM501_GATE_HOST			(0)
+#define SM501_GATE_MEMORY		(1)
+#define SM501_GATE_DISPLAY		(2)
+#define SM501_GATE_2D_ENGINE		(3)
+#define SM501_GATE_CSC			(4)
+#define SM501_GATE_ZVPORT		(5)
+#define SM501_GATE_GPIO			(6)
+#define SM501_GATE_UART0		(7)
+#define SM501_GATE_UART1		(8)
+#define SM501_GATE_SSP			(10)
+#define SM501_GATE_USB_HOST		(11)
+#define SM501_GATE_USB_GADGET		(12)
+#define SM501_GATE_UCONTROLLER		(17)
+#define SM501_GATE_AC97			(18)
+
+/* panel clock */
+#define SM501_CLOCK_P2XCLK		(24)
+/* crt clock */
+#define SM501_CLOCK_V2XCLK		(16)
+/* main clock */
+#define SM501_CLOCK_MCLK		(8)
+/* SDRAM controller clock */
+#define SM501_CLOCK_M1XCLK		(0)
+
+/* config 2 */
+#define SM501_PCI_MASTER_BASE		(0x000058)
+#define SM501_ENDIAN_CONTROL		(0x00005C)
+#define SM501_DEVICEID			(0x000060)
+/* 0x050100A0 */
+
+#define SM501_DEVICEID_SM501		(0x05010000)
+#define SM501_DEVICEID_IDMASK		(0xffff0000)
+
+#define SM501_PLLCLOCK_COUNT		(0x000064)
+#define SM501_MISC_TIMING		(0x000068)
+#define SM501_CURRENT_SDRAM_CLOCK	(0x00006C)
+
+/* GPIO base */
+#define SM501_GPIO			(0x010000)
+#define SM501_GPIO_DATA_LOW		(0x00)
+#define SM501_GPIO_DATA_HIGH		(0x04)
+#define SM501_GPIO_DDR_LOW		(0x08)
+#define SM501_GPIO_DDR_HIGH		(0x0C)
+#define SM501_GPIO_IRQ_SETUP		(0x10)
+#define SM501_GPIO_IRQ_STATUS		(0x14)
+#define SM501_GPIO_IRQ_RESET		(0x14)
+
+/* I2C controller base */
+#define SM501_I2C			(0x010040)
+#define SM501_I2C_BYTE_COUNT		(0x00)
+#define SM501_I2C_CONTROL		(0x01)
+#define SM501_I2C_STATUS		(0x02)
+#define SM501_I2C_RESET			(0x02)
+#define SM501_I2C_SLAVE_ADDRESS		(0x03)
+#define SM501_I2C_DATA			(0x04)
+
+/* SSP base */
+#define SM501_SSP			(0x020000)
+
+/* Uart 0 base */
+#define SM501_UART0			(0x030000)
+
+/* Uart 1 base */
+#define SM501_UART1			(0x030020)
+
+/* USB host port base */
+#define SM501_USB_HOST			(0x040000)
+
+/* USB slave/gadget base */
+#define SM501_USB_GADGET		(0x060000)
+
+/* USB slave/gadget data port base */
+#define SM501_USB_GADGET_DATA		(0x070000)
+
+/* Display contoller/video engine base */
+#define SM501_DC			(0x080000)
+
+/* common defines for the SM501 address registers */
+#define SM501_ADDR_FLIP			(1<<31)
+#define SM501_ADDR_EXT			(1<<27)
+#define SM501_ADDR_CS1			(1<<26)
+#define SM501_ADDR_MASK			(0x3f << 26)
+
+#define SM501_FIFO_MASK			(0x3 << 16)
+#define SM501_FIFO_1			(0x0 << 16)
+#define SM501_FIFO_3			(0x1 << 16)
+#define SM501_FIFO_7			(0x2 << 16)
+#define SM501_FIFO_11			(0x3 << 16)
+
+/* common registers for panel and the crt */
+#define SM501_OFF_DC_H_TOT		(0x000)
+#define SM501_OFF_DC_V_TOT		(0x008)
+#define SM501_OFF_DC_H_SYNC		(0x004)
+#define SM501_OFF_DC_V_SYNC		(0x00C)
+
+#define SM501_DC_PANEL_CONTROL		(0x000)
+
+#define SM501_DC_PANEL_CONTROL_FPEN	(1<<27)
+#define SM501_DC_PANEL_CONTROL_BIAS	(1<<26)
+#define SM501_DC_PANEL_CONTROL_DATA	(1<<25)
+#define SM501_DC_PANEL_CONTROL_VDD	(1<<24)
+#define SM501_DC_PANEL_CONTROL_DP	(1<<23)
+
+#define SM501_DC_PANEL_CONTROL_TFT_888	(0<<21)
+#define SM501_DC_PANEL_CONTROL_TFT_333	(1<<21)
+#define SM501_DC_PANEL_CONTROL_TFT_444	(2<<21)
+
+#define SM501_DC_PANEL_CONTROL_DE	(1<<20)
+
+#define SM501_DC_PANEL_CONTROL_LCD_TFT	(0<<18)
+#define SM501_DC_PANEL_CONTROL_LCD_STN8	(1<<18)
+#define SM501_DC_PANEL_CONTROL_LCD_STN12 (2<<18)
+
+#define SM501_DC_PANEL_CONTROL_CP	(1<<14)
+#define SM501_DC_PANEL_CONTROL_VSP	(1<<13)
+#define SM501_DC_PANEL_CONTROL_HSP	(1<<12)
+#define SM501_DC_PANEL_CONTROL_CK	(1<<9)
+#define SM501_DC_PANEL_CONTROL_TE	(1<<8)
+#define SM501_DC_PANEL_CONTROL_VPD	(1<<7)
+#define SM501_DC_PANEL_CONTROL_VP	(1<<6)
+#define SM501_DC_PANEL_CONTROL_HPD	(1<<5)
+#define SM501_DC_PANEL_CONTROL_HP	(1<<4)
+#define SM501_DC_PANEL_CONTROL_GAMMA	(1<<3)
+#define SM501_DC_PANEL_CONTROL_EN	(1<<2)
+
+#define SM501_DC_PANEL_CONTROL_8BPP	(0<<0)
+#define SM501_DC_PANEL_CONTROL_16BPP	(1<<0)
+#define SM501_DC_PANEL_CONTROL_32BPP	(2<<0)
+
+
+#define SM501_DC_PANEL_PANNING_CONTROL	(0x004)
+#define SM501_DC_PANEL_COLOR_KEY	(0x008)
+#define SM501_DC_PANEL_FB_ADDR		(0x00C)
+#define SM501_DC_PANEL_FB_OFFSET	(0x010)
+#define SM501_DC_PANEL_FB_WIDTH		(0x014)
+#define SM501_DC_PANEL_FB_HEIGHT	(0x018)
+#define SM501_DC_PANEL_TL_LOC		(0x01C)
+#define SM501_DC_PANEL_BR_LOC		(0x020)
+#define SM501_DC_PANEL_H_TOT		(0x024)
+#define SM501_DC_PANEL_H_SYNC		(0x028)
+#define SM501_DC_PANEL_V_TOT		(0x02C)
+#define SM501_DC_PANEL_V_SYNC		(0x030)
+#define SM501_DC_PANEL_CUR_LINE		(0x034)
+
+#define SM501_DC_VIDEO_CONTROL		(0x040)
+#define SM501_DC_VIDEO_FB0_ADDR		(0x044)
+#define SM501_DC_VIDEO_FB_WIDTH		(0x048)
+#define SM501_DC_VIDEO_FB0_LAST_ADDR	(0x04C)
+#define SM501_DC_VIDEO_TL_LOC		(0x050)
+#define SM501_DC_VIDEO_BR_LOC		(0x054)
+#define SM501_DC_VIDEO_SCALE		(0x058)
+#define SM501_DC_VIDEO_INIT_SCALE	(0x05C)
+#define SM501_DC_VIDEO_YUV_CONSTANTS	(0x060)
+#define SM501_DC_VIDEO_FB1_ADDR		(0x064)
+#define SM501_DC_VIDEO_FB1_LAST_ADDR	(0x068)
+
+#define SM501_DC_VIDEO_ALPHA_CONTROL	(0x080)
+#define SM501_DC_VIDEO_ALPHA_FB_ADDR	(0x084)
+#define SM501_DC_VIDEO_ALPHA_FB_OFFSET	(0x088)
+#define SM501_DC_VIDEO_ALPHA_FB_LAST_ADDR	(0x08C)
+#define SM501_DC_VIDEO_ALPHA_TL_LOC	(0x090)
+#define SM501_DC_VIDEO_ALPHA_BR_LOC	(0x094)
+#define SM501_DC_VIDEO_ALPHA_SCALE	(0x098)
+#define SM501_DC_VIDEO_ALPHA_INIT_SCALE	(0x09C)
+#define SM501_DC_VIDEO_ALPHA_CHROMA_KEY	(0x0A0)
+#define SM501_DC_VIDEO_ALPHA_COLOR_LOOKUP	(0x0A4)
+
+#define SM501_DC_PANEL_HWC_BASE		(0x0F0)
+#define SM501_DC_PANEL_HWC_ADDR		(0x0F0)
+#define SM501_DC_PANEL_HWC_LOC		(0x0F4)
+#define SM501_DC_PANEL_HWC_COLOR_1_2	(0x0F8)
+#define SM501_DC_PANEL_HWC_COLOR_3	(0x0FC)
+
+#define SM501_HWC_EN			(1<<31)
+
+#define SM501_OFF_HWC_ADDR		(0x00)
+#define SM501_OFF_HWC_LOC		(0x04)
+#define SM501_OFF_HWC_COLOR_1_2		(0x08)
+#define SM501_OFF_HWC_COLOR_3		(0x0C)
+
+#define SM501_DC_ALPHA_CONTROL		(0x100)
+#define SM501_DC_ALPHA_FB_ADDR		(0x104)
+#define SM501_DC_ALPHA_FB_OFFSET	(0x108)
+#define SM501_DC_ALPHA_TL_LOC		(0x10C)
+#define SM501_DC_ALPHA_BR_LOC		(0x110)
+#define SM501_DC_ALPHA_CHROMA_KEY	(0x114)
+#define SM501_DC_ALPHA_COLOR_LOOKUP	(0x118)
+
+#define SM501_DC_CRT_CONTROL		(0x200)
+
+#define SM501_DC_CRT_CONTROL_TVP	(1<<15)
+#define SM501_DC_CRT_CONTROL_CP		(1<<14)
+#define SM501_DC_CRT_CONTROL_VSP	(1<<13)
+#define SM501_DC_CRT_CONTROL_HSP	(1<<12)
+#define SM501_DC_CRT_CONTROL_VS		(1<<11)
+#define SM501_DC_CRT_CONTROL_BLANK	(1<<10)
+#define SM501_DC_CRT_CONTROL_SEL	(1<<9)
+#define SM501_DC_CRT_CONTROL_TE		(1<<8)
+#define SM501_DC_CRT_CONTROL_PIXEL_MASK (0xF << 4)
+#define SM501_DC_CRT_CONTROL_GAMMA	(1<<3)
+#define SM501_DC_CRT_CONTROL_ENABLE	(1<<2)
+
+#define SM501_DC_CRT_CONTROL_8BPP	(0<<0)
+#define SM501_DC_CRT_CONTROL_16BPP	(1<<0)
+#define SM501_DC_CRT_CONTROL_32BPP	(2<<0)
+
+#define SM501_DC_CRT_FB_ADDR		(0x204)
+#define SM501_DC_CRT_FB_OFFSET		(0x208)
+#define SM501_DC_CRT_H_TOT		(0x20C)
+#define SM501_DC_CRT_H_SYNC		(0x210)
+#define SM501_DC_CRT_V_TOT		(0x214)
+#define SM501_DC_CRT_V_SYNC		(0x218)
+#define SM501_DC_CRT_SIGNATURE_ANALYZER	(0x21C)
+#define SM501_DC_CRT_CUR_LINE		(0x220)
+#define SM501_DC_CRT_MONITOR_DETECT	(0x224)
+
+#define SM501_DC_CRT_HWC_BASE		(0x230)
+#define SM501_DC_CRT_HWC_ADDR		(0x230)
+#define SM501_DC_CRT_HWC_LOC		(0x234)
+#define SM501_DC_CRT_HWC_COLOR_1_2	(0x238)
+#define SM501_DC_CRT_HWC_COLOR_3	(0x23C)
+
+#define SM501_DC_PANEL_PALETTE		(0x400)
+
+#define SM501_DC_VIDEO_PALETTE		(0x800)
+
+#define SM501_DC_CRT_PALETTE		(0xC00)
+
+/* Zoom Video port base */
+#define SM501_ZVPORT			(0x090000)
+
+/* AC97/I2S base */
+#define SM501_AC97			(0x0A0000)
+
+/* 8051 micro controller base */
+#define SM501_UCONTROLLER		(0x0B0000)
+
+/* 8051 micro controller SRAM base */
+#define SM501_UCONTROLLER_SRAM		(0x0C0000)
+
+/* DMA base */
+#define SM501_DMA			(0x0D0000)
+
+/* 2d engine base */
+#define SM501_2D_ENGINE			(0x100000)
+#define SM501_2D_SOURCE			(0x00)
+#define SM501_2D_DESTINATION		(0x04)
+#define SM501_2D_DIMENSION		(0x08)
+#define SM501_2D_CONTROL		(0x0C)
+#define SM501_2D_PITCH			(0x10)
+#define SM501_2D_FOREGROUND		(0x14)
+#define SM501_2D_BACKGROUND		(0x18)
+#define SM501_2D_STRETCH		(0x1C)
+#define SM501_2D_COLOR_COMPARE		(0x20)
+#define SM501_2D_COLOR_COMPARE_MASK 	(0x24)
+#define SM501_2D_MASK			(0x28)
+#define SM501_2D_CLIP_TL		(0x2C)
+#define SM501_2D_CLIP_BR		(0x30)
+#define SM501_2D_MONO_PATTERN_LOW	(0x34)
+#define SM501_2D_MONO_PATTERN_HIGH	(0x38)
+#define SM501_2D_WINDOW_WIDTH		(0x3C)
+#define SM501_2D_SOURCE_BASE		(0x40)
+#define SM501_2D_DESTINATION_BASE	(0x44)
+#define SM501_2D_ALPHA			(0x48)
+#define SM501_2D_WRAP			(0x4C)
+#define SM501_2D_STATUS			(0x50)
+
+#define SM501_CSC_Y_SOURCE_BASE		(0xC8)
+#define SM501_CSC_CONSTANTS		(0xCC)
+#define SM501_CSC_Y_SOURCE_X		(0xD0)
+#define SM501_CSC_Y_SOURCE_Y		(0xD4)
+#define SM501_CSC_U_SOURCE_BASE		(0xD8)
+#define SM501_CSC_V_SOURCE_BASE		(0xDC)
+#define SM501_CSC_SOURCE_DIMENSION	(0xE0)
+#define SM501_CSC_SOURCE_PITCH		(0xE4)
+#define SM501_CSC_DESTINATION		(0xE8)
+#define SM501_CSC_DESTINATION_DIMENSION	(0xEC)
+#define SM501_CSC_DESTINATION_PITCH	(0xF0)
+#define SM501_CSC_SCALE_FACTOR		(0xF4)
+#define SM501_CSC_DESTINATION_BASE	(0xF8)
+#define SM501_CSC_CONTROL		(0xFC)
+
+/* 2d engine data port base */
+#define SM501_2D_ENGINE_DATA		(0x110000)

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

[Anatolij Gustschin]

Hello Stefan,

please see comments below:

Stefan Althoefer wrote:
> [PATCH] video: Add new driver for Silicon Motion SM501/SM502

this line duplicates the subject, so simply remove it.

> 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.

s/drive/driver/

> 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.

<snip>

> The patch is against "latest" u-boot git-repository
> 
> Please (still) be patient if style of submission or patches are
> offending.

These lines are patch comments which should not appear in
the commit message. The common practice is to place such
comments below "---" line.

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

So, here is the place for patch comments which should not
appear in the commit message. Also changes between patch
revisions are often summarised here.

> diff -uprN u-boot-orig//drivers/video/sm501new.h u-boot/drivers/video/sm501new.h
> --- u-boot-orig//drivers/video/sm501new.h	1970-01-01 01:00:00.000000000 +0100
> +++ u-boot/drivers/video/sm501new.h	2008-12-02 18:28:42.000000000 +0100
> @@ -0,0 +1,125 @@
> +/* Large parts of this have been taken from the linux kernel source code
> + * with the following licencse:

Multi-line comment style is as follows:
/*
 * This is a
 * multi-line
 * comment
 */

> +/* Ported to u-boot:
> + * 
> + * Copyright (c) 2008 StefanAlthoefer (as at janz.de)
> + */

same here, fix multi-line comment here and everywhere in
the code.

<snip>

> +#define SM501FB_FLAG_USE_INIT_MODE	(1<<0)
> +#define SM501FB_FLAG_DISABLE_AT_EXIT	(1<<1)
> +#define SM501FB_FLAG_USE_HWCURSOR	(1<<2)
> +#define SM501FB_FLAG_USE_HWACCEL	(1<<3)
> +
> +struct sm501_platdata_fbsub {
> +	struct fb_videomode	*def_mode;
> +	unsigned int		 def_bpp;
> +	unsigned long		 max_mem;
> +	unsigned int		 flags;
> +};
> +
> +enum sm501_fb_routing {
> +	SM501_FB_OWN		= 0,	/* CRT=>CRT, Panel=>Panel */
> +	SM501_FB_CRT_PANEL	= 1,	/* Panel=>CRT, Panel=>Panel */
> +};
> +
> +/* sm501_platdata_fb flag field bit definitions */
> +
> +#define SM501_FBPD_SWAP_FB_ENDIAN	(1<<0)	/* need to endian swap */
> +
> +/* sm501_platdata_fb
> + *
> + * configuration data for the framebuffer driver
> +*/
> +
> +struct sm501_platdata_fb {
> +	enum sm501_fb_routing		 fb_route;
> +	unsigned int			 flags;
> +	struct sm501_platdata_fbsub	*fb_crt;
> +	struct sm501_platdata_fbsub	*fb_pnl;
> +};
> +
> +/* gpio i2c */
> +
> +struct sm501_platdata_gpio_i2c {
> +	unsigned int		pin_sda;
> +	unsigned int		pin_scl;
> +};

all this stuff above starting from
"#define SM501FB_FLAG_USE_INIT_MODE"
can be removed if you remove "init_gpiop", "fb", "gpio_i2c" and
"gpio_i2c_nr" from the "struct sm501_platdata" definition below.
Also remove ".fb = &sm501_fb_pdata" from "sm501_pci_platdata"
declaration in drivers/video/sm501new.c. These structure members
aren't referenced in this U-Boot driver, so they aren't needed
(unused and nearly dead code and data).

<snip>
> +#define SM501_USE_USB_HOST	(1<<0)
> +#define SM501_USE_USB_SLAVE	(1<<1)
> +#define SM501_USE_SSP0		(1<<2)
> +#define SM501_USE_SSP1		(1<<3)
> +#define SM501_USE_UART0		(1<<4)
> +#define SM501_USE_UART1		(1<<5)
> +#define SM501_USE_FBACCEL	(1<<6)
> +#define SM501_USE_AC97		(1<<7)
> +#define SM501_USE_I2S		(1<<8)

These macros aren't used, remove them too.

> +
> +#define SM501_USE_ALL		(0xffffffff)
> +
> +struct sm501_initdata {
> +	struct sm501_reg_init	gpio_low;
> +	struct sm501_reg_init	gpio_high;
> +	struct sm501_reg_init	misc_timing;
> +	struct sm501_reg_init	misc_control;
> +
> +	unsigned long		devices;

"devices" member is only initialized in "sm501_pci_initdata"
declaration and isn't referenced anywhere in the code, so
another candidate to remove.

> +/* sm501_platdata
> + *
> + * This is passed with the platform device to allow the board
> + * to control the behaviour of the SM501 driver(s) which attach
> + * to the device.
> + *
> +*/
> +
> +struct sm501_platdata {
> +	struct sm501_initdata		*init;
> +	struct sm501_init_gpio		*init_gpiop;
> +	struct sm501_platdata_fb	*fb;
> +
> +	struct sm501_platdata_gpio_i2c	*gpio_i2c;
> +	unsigned int			 gpio_i2c_nr;

see above suggestion to remove init_gpiop, fb, gpio_i2c and
gpio_i2c_nr.


> diff -uprN u-boot-orig//drivers/video/sm501new-regs.h u-boot/drivers/video/sm501new-regs.h
> --- u-boot-orig//drivers/video/sm501new-regs.h	1970-01-01 01:00:00.000000000 +0100
> +++ u-boot/drivers/video/sm501new-regs.h	2008-12-02 18:28:42.000000000 +0100
> @@ -0,0 +1,365 @@
> +/* sm501-regs.h
> + *
> + * Copyright 2006 Simtec Electronics
> + *
> + * 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.
> + *
> + * Silicon Motion SM501 register definitions
> +*/

please, fix multi-line comment style.

Also check all below defined macros in this file. If some
macro isn't used in the driver code, then remove it.

> +/* System Configuration area */
> +/* System config base */
> +#define SM501_SYS_CONFIG		(0x000000)
> +
> +/* config 1 */
> +#define SM501_SYSTEM_CONTROL 		(0x000000)
> +#define SM501_MISC_CONTROL		(0x000004)
> +
> +#define SM501_MISC_BUS_SH		(0x0)
> +#define SM501_MISC_BUS_PCI		(0x1)
> +#define SM501_MISC_BUS_XSCALE		(0x2)
> +#define SM501_MISC_BUS_NEC		(0x6)
> +#define SM501_MISC_BUS_MASK		(0x7)
<snip>

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

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

[Stefan Althoefer]

Hi Anatolij,

<snip>

>> 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.

The CONFIG_VIDEO_SM501 driver is completely differently configured
(boards supply tables with register settings). This will be very
hard to merge. SM50x might be a better name, but is it good style
to mix lowercase and uppercase letters in macro name?

<snip>

>> +#define SM501FB_FLAG_USE_INIT_MODE	(1<<0)
>> +#define SM501FB_FLAG_DISABLE_AT_EXIT	(1<<1)
>> +#define SM501FB_FLAG_USE_HWCURSOR	(1<<2)
>> +#define SM501FB_FLAG_USE_HWACCEL	(1<<3)
>> +
>> +struct sm501_platdata_fbsub {
>> +	struct fb_videomode	*def_mode;
>> +	unsigned int		 def_bpp;
>> +	unsigned long		 max_mem;
>> +	unsigned int		 flags;
>> +};
>> +
>> +enum sm501_fb_routing {
>> +	SM501_FB_OWN		= 0,	/* CRT=>CRT, Panel=>Panel */
>> +	SM501_FB_CRT_PANEL	= 1,	/* Panel=>CRT, Panel=>Panel */
>> +};
>> +
>> +/* sm501_platdata_fb flag field bit definitions */
>> +
>> +#define SM501_FBPD_SWAP_FB_ENDIAN	(1<<0)	/* need to endian swap */
>> +
>> +/* sm501_platdata_fb
>> + *
>> + * configuration data for the framebuffer driver
>> +*/
>> +
>> +struct sm501_platdata_fb {
>> +	enum sm501_fb_routing		 fb_route;
>> +	unsigned int			 flags;
>> +	struct sm501_platdata_fbsub	*fb_crt;
>> +	struct sm501_platdata_fbsub	*fb_pnl;
>> +};
>> +
>> +/* gpio i2c */
>> +
>> +struct sm501_platdata_gpio_i2c {
>> +	unsigned int		pin_sda;
>> +	unsigned int		pin_scl;
>> +};
> 
> all this stuff above starting from
> "#define SM501FB_FLAG_USE_INIT_MODE"
> can be removed if you remove "init_gpiop", "fb", "gpio_i2c" and
> "gpio_i2c_nr" from the "struct sm501_platdata" definition below.
> Also remove ".fb = &sm501_fb_pdata" from "sm501_pci_platdata"
> declaration in drivers/video/sm501new.c. These structure members
> aren't referenced in this U-Boot driver, so they aren't needed
> (unused and nearly dead code and data).
> 
> <snip>
>> +#define SM501_USE_USB_HOST	(1<<0)
>> +#define SM501_USE_USB_SLAVE	(1<<1)
>> +#define SM501_USE_SSP0		(1<<2)
>> +#define SM501_USE_SSP1		(1<<3)
>> +#define SM501_USE_UART0		(1<<4)
>> +#define SM501_USE_UART1		(1<<5)
>> +#define SM501_USE_FBACCEL	(1<<6)
>> +#define SM501_USE_AC97		(1<<7)
>> +#define SM501_USE_I2S		(1<<8)
> 
> These macros aren't used, remove them too.
> 
>> +
>> +#define SM501_USE_ALL		(0xffffffff)
>> +
>> +struct sm501_initdata {
>> +	struct sm501_reg_init	gpio_low;
>> +	struct sm501_reg_init	gpio_high;
>> +	struct sm501_reg_init	misc_timing;
>> +	struct sm501_reg_init	misc_control;
>> +
>> +	unsigned long		devices;
> 
> "devices" member is only initialized in "sm501_pci_initdata"
> declaration and isn't referenced anywhere in the code, so
> another candidate to remove.
> 
>> +/* sm501_platdata
>> + *
>> + * This is passed with the platform device to allow the board
>> + * to control the behaviour of the SM501 driver(s) which attach
>> + * to the device.
>> + *
>> +*/
>> +
>> +struct sm501_platdata {
>> +	struct sm501_initdata		*init;
>> +	struct sm501_init_gpio		*init_gpiop;
>> +	struct sm501_platdata_fb	*fb;
>> +
>> +	struct sm501_platdata_gpio_i2c	*gpio_i2c;
>> +	unsigned int			 gpio_i2c_nr;
> 
> see above suggestion to remove init_gpiop, fb, gpio_i2c and
> gpio_i2c_nr.
> 
> 
>> diff -uprN u-boot-orig//drivers/video/sm501new-regs.h u-boot/drivers/video/sm501new-regs.h
>> --- u-boot-orig//drivers/video/sm501new-regs.h	1970-01-01 01:00:00.000000000 +0100
>> +++ u-boot/drivers/video/sm501new-regs.h	2008-12-02 18:28:42.000000000 +0100
>> @@ -0,0 +1,365 @@
>> +/* sm501-regs.h
>> + *
>> + * Copyright 2006 Simtec Electronics
>> + *
>> + * 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.
>> + *
>> + * Silicon Motion SM501 register definitions
>> +*/
> 
> please, fix multi-line comment style.
> 
> Also check all below defined macros in this file. If some
> macro isn't used in the driver code, then remove it.
> 
>> +/* System Configuration area */
>> +/* System config base */
>> +#define SM501_SYS_CONFIG		(0x000000)
>> +
>> +/* config 1 */
>> +#define SM501_SYSTEM_CONTROL 		(0x000000)
>> +#define SM501_MISC_CONTROL		(0x000004)
>> +
>> +#define SM501_MISC_BUS_SH		(0x0)
>> +#define SM501_MISC_BUS_PCI		(0x1)
>> +#define SM501_MISC_BUS_XSCALE		(0x2)
>> +#define SM501_MISC_BUS_NEC		(0x6)
>> +#define SM501_MISC_BUS_MASK		(0x7)
> <snip>

I agree in removing dead code and structure components that will
never make sense in u-boot environment. However, should I really
delete definitions form -regs.h? This is well developed (linux)
and things might be needed for future development. I did not
touch this file compared to kernel source.

- Stefan

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

[Wolfgang Denk]

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.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Any sufficiently advanced  technology  is  indistinguishable  from  a
rigged demo.                              - Andy Finkel, computer guy

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

[ksi at koi8.net]

On Sun, 7 Dec 2008, Wolfgang Denk wrote:

> 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.

We are about to build our first prototype for MPC8548E based motherboard and
I also have SM502 on it. It is very good that there is somebody else working
on it and I want to take part in the process.

As for the SM502, it is a multi-function chip that has more than just video.
I'm writing an I2C driver for it right now and I want to ask you guys for
your opinion.

MPC8548 has _TWO_ I2C controllers and it is taken care of in fsl_i2c.c by
defining a "bus number." There is a function that one uses to set a bus for
consecutive operations with i2c_read() and friends (they do NOT take a bus
number as a parameter.) That means one sets a bus and then all the
operations are done on that bus until it is changed again.

This might be OK but it is definitely not portable and SOC-specific. In my
case I have a _THIRD_ I2C bus off of SM502... What do you think would be a
best approach to address this:

a.) Rewrite _ALL_ I2C code to make those i2c_read() etc. functions to take
     an additional parameter, bus number

b.) Make a global var i2c_bus or something and add a global function kinda
     i2c_set_bus() so all i2c_read()/i2c_write() functions use that variable

c.) Add a third bus for SM502 I2C adapter to fsl_i2c.c (horrible hack)

d.) Make SM502 I2C a totally separate entity with its own set of functions
     (like sm502_i2c_read() etc.)

Please tell what you think. I personally lean towards option b.) but I might
be wrong :)

And there is another useful part of SM502 that's begging for implementation
-- USB controller with legacy KBD/Mouse support...

---
******************************************************************
*  KSI at home    KOI8 Net  < >  The impossible we do immediately.  *
*  Las Vegas   NV, USA   < >  Miracles require 24-hour notice.   *
******************************************************************

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

[Wolfgang Denk]

Dear ksi at koi8.net,

In message <Pine.LNX.4.64ksi.0812062213500.4343@home-gw.koi8.net> you wrote:
>
> As for the SM502, it is a multi-function chip that has more than just video.
> I'm writing an I2C driver for it right now and I want to ask you guys for
> your opinion.

That's not new - the SM501 is pretty similar.

> MPC8548 has _TWO_ I2C controllers and it is taken care of in fsl_i2c.c by
> defining a "bus number." There is a function that one uses to set a bus for
> consecutive operations with i2c_read() and friends (they do NOT take a bus
> number as a parameter.) That means one sets a bus and then all the
> operations are done on that bus until it is changed again.

That's the general idea, also for example for device handling: we set
a current device (or bus) with one command, and then continue to use
that. 

> This might be OK but it is definitely not portable and SOC-specific. In my

I think the "this" here refers to the specific implementation, not the
general approach, right?

> case I have a _THIRD_ I2C bus off of SM502... What do you think would be a
> best approach to address this:
> 
> a.) Rewrite _ALL_ I2C code to make those i2c_read() etc. functions to take
>      an additional parameter, bus number
> 
> b.) Make a global var i2c_bus or something and add a global function kinda
>      i2c_set_bus() so all i2c_read()/i2c_write() functions use that variable
> 
> c.) Add a third bus for SM502 I2C adapter to fsl_i2c.c (horrible hack)
> 
> d.) Make SM502 I2C a totally separate entity with its own set of functions
>      (like sm502_i2c_read() etc.)
> 
> Please tell what you think. I personally lean towards option b.) but I might
> be wrong :)

I agree.

> And there is another useful part of SM502 that's begging for implementation
> -- USB controller with legacy KBD/Mouse support...

Feel free to submit patches :-)

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
You're too beautiful to ignore.  Too much woman.
	-- Kirk to Yeoman Rand, "The Enemy Within", stardate unknown

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

[Stefan Althoefer]

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.

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).

BTW the driver will probably also work with the new SM107,
which as no USB, Keyboard and all this stuff.

-- Stefan

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

[ksi at koi8.net]

On Sun, 7 Dec 2008, Wolfgang Denk wrote:

> Dear ksi at koi8.net,
>
> In message <Pine.LNX.4.64ksi.0812062213500.4343@home-gw.koi8.net> you
> wrote:
>>
>> As for the SM502, it is a multi-function chip that has more than just
> video.
>> I'm writing an I2C driver for it right now and I want to ask you guys
> for
>> your opinion.
>
> That's not new - the SM501 is pretty similar.

As a matter of fact, SM501 is an older, now nonexistent version of SM502.
Its support in U-Boot is rudimentary. I'd rather removed that old sm501.c
altogether and replaced it with a new sm502.c. That should work for old
SM501 chips if there are still any in existence.

Support is much better in Linux kernel -- it is almost fully supported out
of the box, video, USB, sound etc.

>> MPC8548 has _TWO_ I2C controllers and it is taken care of in fsl_i2c.c
> by
>> defining a "bus number." There is a function that one uses to set a
> bus for
>> consecutive operations with i2c_read() and friends (they do NOT take a
> bus
>> number as a parameter.) That means one sets a bus and then all the
>> operations are done on that bus until it is changed again.
>
> That's the general idea, also for example for device handling: we set
> a current device (or bus) with one command, and then continue to use
> that.
>
>> This might be OK but it is definitely not portable and SOC-specific.
> In my
>
> I think the "this" here refers to the specific implementation, not the
> general approach, right?

Yes.

>> case I have a _THIRD_ I2C bus off of SM502... What do you think would
> be a
>> best approach to address this:
>>
>> a.) Rewrite _ALL_ I2C code to make those i2c_read() etc. functions to
> take
>>      an additional parameter, bus number
>>
>> b.) Make a global var i2c_bus or something and add a global function
> kinda
>>      i2c_set_bus() so all i2c_read()/i2c_write() functions use that
> variable
>>
>> c.) Add a third bus for SM502 I2C adapter to fsl_i2c.c (horrible hack)
>>
>> d.) Make SM502 I2C a totally separate entity with its own set of
> functions
>>      (like sm502_i2c_read() etc.)
>>
>> Please tell what you think. I personally lean towards option b.) but I
> might
>> be wrong :)
>
> I agree.

OK, so I'll take that route.

>> And there is another useful part of SM502 that's begging for
> implementation
>> -- USB controller with legacy KBD/Mouse support...
>
> Feel free to submit patches :-)

Sure, but there are other things that must me done before that USB. I need
Silicon Image Sil3124A SATA driver first :)

---
******************************************************************
*  KSI at home    KOI8 Net  < >  The impossible we do immediately.  *
*  Las Vegas   NV, USA   < >  Miracles require 24-hour notice.   *
******************************************************************

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

[Wolfgang Denk]

Dear ksi at koi8.net,

In message <Pine.LNX.4.64ksi.0812071006250.8651@home-gw.koi8.net> you wrote:
> 
> As a matter of fact, SM501 is an older, now nonexistent version of SM502.

Yeah. Tell that to some hardware companies who  are  shipping  boards
with the SM501 on it. :-)

> Its support in U-Boot is rudimentary. I'd rather removed that old sm501.c
> altogether and replaced it with a new sm502.c. That should work for old
> SM501 chips if there are still any in existence.

They are, and will be in existence at least for a couple of years (my
expectation for couple is a two-digit number).

> Support is much better in Linux kernel -- it is almost fully supported out
> of the box, video, USB, sound etc.

Linux is an OS - U-Boot is a mere boot loader ;-)

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Defaults are wonderful, just like fire.
                  - Larry Wall in <1996Mar6.004121.27890@netlabs.com>

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

[ksi at koi8.net]

On Sun, 7 Dec 2008, Wolfgang Denk wrote:

> Dear ksi at koi8.net,
>
> In message <Pine.LNX.4.64ksi.0812071006250.8651@home-gw.koi8.net> you
> wrote:
>>
>> As a matter of fact, SM501 is an older, now nonexistent version of
> SM502.
>
> Yeah. Tell that to some hardware companies who  are  shipping  boards
> with the SM501 on it. :-)

They must be well stocked up with those :) Yeah, sure there are still such
boards and they will still exist in foreseable future. But I personally
think it is not a reason for that old basic driver to hang out forever if
the new one can also work with old boards with minimal changes.

>> Its support in U-Boot is rudimentary. I'd rather removed that old
> sm501.c
>> altogether and replaced it with a new sm502.c. That should work for
> old
>> SM501 chips if there are still any in existence.
>
> They are, and will be in existence at least for a couple of years (my
> expectation for couple is a two-digit number).

They must have loads of them in their inventories... Yes, I agree they will
be used until inventories run dry but it is not something that has future.

I would rather suggest having 2 separate drivers--sm501.c and sm502.c--for a
transitional period. Then we could rework all boards using old sm501.c to
use the new driver, one by one, and when the last one is converted we'll be
able to get rid of the old code.

Just thoughts, ya know...

>> Support is much better in Linux kernel -- it is almost fully supported
> out
>> of the box, video, USB, sound etc.
>
> Linux is an OS - U-Boot is a mere boot loader ;-)

Eh, for some U-Boot is also an OS...

---
******************************************************************
*  KSI at home    KOI8 Net  < >  The impossible we do immediately.  *
*  Las Vegas   NV, USA   < >  Miracles require 24-hour notice.   *
******************************************************************

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

[Anatolij Gustschin]

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 <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: office at denx.de