[U-Boot] [PATCH] 6/12 Multiadapter/multibus I2C, drivers part 3

Signed-off-by: Sergey Kubushyn ksi@koi8.net --- diff -purN u-boot-i2c.orig/drivers/i2c/sm502_i2c.c u-boot-i2c/drivers/i2c/sm502_i2c.c --- u-boot-i2c.orig/drivers/i2c/sm502_i2c.c 1969-12-31 16:00:00.000000000 -0800 +++ u-boot-i2c/drivers/i2c/sm502_i2c.c 2009-02-12 10:46:00.000000000 -0800 @@ -0,0 +1,340 @@ +/* + * Copyright (C) 2009 Sergey Kubushyn ksi@koi8.net + * + * Driver for Silicon Motion SM501/SM502 I2C interface. + */ + +#include <common.h> + +#ifdef CONFIG_SM502_I2C + +#include <command.h> +#include <i2c.h> + +#include <sm501-regs.h> +#include <asm/io.h> + +/* I2C registers field definitions */ +/* --------------------------------*/ +/* I2C_CONTROL (R/W) */ +#define SM501_I2C_ENABLE (1<<0) +#define SM501_I2C_SPEED (1<<1) +#define SM501_I2C_START (1<<2) +#define SM501_I2C_IRQ_ENA (1<<4) +#define SM501_I2C_IRQ_ACK (1<<5) +#define SM501_I2C_RPT_START_ENA (1<<6) +/* I2C_STATUS (R/O) */ +#define SM501_I2C_BUS_BUSY (1<<0) +#define SM501_I2C_NACK (1<<1) +#define SM501_I2C_BUS_ERROR (1<<2) +#define SM501_I2C_XFER_DONE (1<<3) +/* I2C_RESET (W/O) */ +#define SM501_I2C_CLEAR (1<<2) + +#define SM501_I2C_MAX_COUNT 16 + +#define SM501_I2C_READ 1 +#define SM501_I2C_WRITE 0 + +#define SM501_I2C_TIMEOUT (1<<7) + +#define SM501_CHECK_NACK() \ + do {\ + if (tmp & (SM501_I2C_NACK | SM501_I2C_BUS_ERROR)) {\ + tmp = read_i2c_reg(SM501_I2C_CONTROL) & SM501_I2C_SPEED;\ + write_i2c_reg(SM501_I2C_CONTROL, tmp);\ + return(1);\ + }\ + } while (0) + + +i2c_adap_t sm501_i2c_adap; + +DECLARE_GLOBAL_DATA_PTR; + + +static __inline__ u_int8_t read_i2c_reg(unsigned long offset) +{ + return(readb(sm501_iomem_base + SM501_I2C + offset)); +} + +static __inline__ void write_i2c_reg(unsigned long offset, u_int8_t data) +{ + writeb(data, sm501_iomem_base + SM501_I2C + offset); + __asm__("sync;isync;msync"); +#ifndef CONFIG_SYS_SM501_BASEADDR + /* Dummy read to push it through PCI */ + readb(sm501_iomem_base + SM501_I2C + offset); +#endif +} + +static int poll_i2c_status(u_int8_t mask, int timeout) +{ + u_int8_t stat; + int i; + + i = 0; + + do { + stat = read_i2c_reg(SM501_I2C_STATUS); + if (stat & mask) { + return(stat); + } + udelay(1000); + + } while (i++ < timeout); + + return(stat | SM501_I2C_TIMEOUT); +} + +static int wait_for_bus(void) +{ + int timeout = 10; + + while ((read_i2c_reg(SM501_I2C_STATUS) & SM501_I2C_BUS_BUSY) && timeout--) { + udelay (1000); + } + + if (timeout <= 0) { + printf ("SM501_I2C timed out in wait_for_bb: I2C_STAT=%x\n", + read_i2c_reg(SM501_I2C_STATUS)); + return(1); + } + + return(0); +} + +static void sm501_i2c_init(int speed, int slaveadd) +{ + unsigned long tmpl; + + /* Set GPIO pins for hardware I2C */ + tmpl = readl(sm501_iomem_base + SM501_GPIO + SM501_GPIO_DATA_HIGH); + tmpl |= 0x0000c000; + writel(tmpl, sm501_iomem_base + SM501_GPIO + SM501_GPIO_DATA_HIGH); + __asm__("sync;isync;msync"); +#ifndef CONFIG_SYS_SM501_BASEADDR + /* Dummy read to push is through PCI */ + readl(sm501_iomem_base + SM501_GPIO + SM501_GPIO_DATA_HIGH); +#endif + + /* SM501/502 only allows 100 or 400 KHz speed (standard or fast) */ + if (speed <= 100000) + write_i2c_reg(SM501_I2C_CONTROL, SM501_I2C_ENABLE); + else + write_i2c_reg(SM501_I2C_CONTROL, SM501_I2C_ENABLE | SM501_I2C_SPEED); + + if (gd->flags & GD_FLG_RELOC) { + sm501_i2c_adap.speed = speed <= 100000 ? 100000 : 400000; + sm501_i2c_adap.init_done = 1; + } +} + + +static int sm501_i2c_probe(u_int8_t chip) +{ + u_int8_t tmp; + + /* Fail if I2C address is invalid */ + if (chip > 0x7f) {return(1);} + + tmp = read_i2c_reg(SM501_I2C_CONTROL); + write_i2c_reg(SM501_I2C_CONTROL, tmp & ~(SM501_I2C_START | SM501_I2C_ENABLE)); + if (wait_for_bus()) {return(1);} + + /* try to read one byte from current (or only) address */ + write_i2c_reg(SM501_I2C_CONTROL, tmp | SM501_I2C_ENABLE); + write_i2c_reg(SM501_I2C_BYTE_COUNT, 0); + write_i2c_reg(SM501_I2C_SLAVE_ADDRESS, (chip << 1) | SM501_I2C_READ); + tmp = read_i2c_reg(SM501_I2C_CONTROL); + write_i2c_reg(SM501_I2C_CONTROL, tmp | SM501_I2C_START); + udelay (1000); + + if (!(read_i2c_reg(SM501_I2C_STATUS) & (SM501_I2C_NACK | SM501_I2C_BUS_ERROR))) { + tmp = read_i2c_reg(SM501_I2C_CONTROL); + write_i2c_reg(SM501_I2C_CONTROL, tmp & ~(SM501_I2C_START | SM501_I2C_ENABLE)); + if (wait_for_bus()) {return(1);} + return(0); + } + + return(1); +} + + +static int sm501_i2c_read(u_int8_t chip, u_int32_t addr, int alen, u_int8_t *buf, int len) +{ + u_int8_t tmp; + u_int8_t *ptr = buf; + int i, bytes_read = 0; + + if ((alen < 0) || (alen > 2)) { + printf("%s(): bogus address length %x\n", __FUNCTION__, alen); + return(1); + } + + if (len < 0) { + printf("%s(): bogus length %x\n", __FUNCTION__, len); + return(1); + } + + if (wait_for_bus()) {return(1);} + + if (alen != 0) { + /* Start address phase */ + if (alen == 2) { + /* MSB goes first... */ + write_i2c_reg(SM501_I2C_DATA, (addr >> 8) & 0xff); + write_i2c_reg(SM501_I2C_DATA + 1, addr & 0xff); + } else { + write_i2c_reg(SM501_I2C_DATA, addr & 0xff); + } + + /* Now write it out without STOP condition */ + tmp = read_i2c_reg(SM501_I2C_CONTROL); + tmp |= SM501_I2C_ENABLE | SM501_I2C_START | SM501_I2C_RPT_START_ENA; + write_i2c_reg(SM501_I2C_SLAVE_ADDRESS, (chip << 1) | SM501_I2C_WRITE); + write_i2c_reg(SM501_I2C_BYTE_COUNT, (u_int8_t)(alen - 1)); + write_i2c_reg(SM501_I2C_CONTROL, tmp); + + tmp = poll_i2c_status(SM501_I2C_NACK | SM501_I2C_BUS_ERROR | SM501_I2C_XFER_DONE, 2); + + SM501_CHECK_NACK(); + } + + /* + * Address phase is over, now read 'len' bytes. + * SM502 can read up to 16 bytes per transaction so + * we will be reading in 16-bytes batches if len > 16. + */ + do { + i = (len - bytes_read) > 16 ? 16 : len - bytes_read; + tmp = read_i2c_reg(SM501_I2C_CONTROL); + tmp |= SM501_I2C_ENABLE | SM501_I2C_START; + write_i2c_reg(SM501_I2C_SLAVE_ADDRESS, (chip << 1) | SM501_I2C_READ); + write_i2c_reg(SM501_I2C_BYTE_COUNT, (u_int8_t)(i - 1)); + write_i2c_reg(SM501_I2C_CONTROL, tmp); + + tmp = poll_i2c_status(SM501_I2C_NACK | SM501_I2C_BUS_ERROR | SM501_I2C_XFER_DONE, 100); + + SM501_CHECK_NACK(); + + bytes_read += i; + + for (; i > 0; i--) { + *ptr++ = read_i2c_reg(SM501_I2C_DATA + 16 - i); + } + + } while (len > bytes_read); + + tmp = read_i2c_reg(SM501_I2C_CONTROL); + write_i2c_reg(SM501_I2C_CONTROL, tmp & ~(SM501_I2C_START | SM501_I2C_ENABLE)); + + return(0); +} + + +static int sm501_i2c_write(u_int8_t chip, u_int32_t addr, int alen, u_int8_t *buf, int len) +{ + u_int8_t tmp; + u_int8_t *ptr = buf; + int i, bytes_written = 0; + + if ((alen < 0) || (alen > 2)) { + printf("%s(): bogus address length %x\n", __FUNCTION__, alen); + return(1); + } + + if (len < 0) { + printf("%s(): bogus length %x\n", __FUNCTION__, len); + return(1); + } + + if (wait_for_bus()) {return(1);} + + /* + * SM502 can write up to 16 bytes per transaction so we will be + * writing in 16-bytes batches if len > (16 - alen). Address is + * sent first, then data hence (16 - alen)... + */ + do { + i = (len - bytes_written) > (16 - alen) ? 16 - alen : len - bytes_written; + + /* Prepend address */ + if (alen == 2) { + /* MSB goes first... */ + write_i2c_reg(SM501_I2C_DATA, ((addr + bytes_written) >> 8) & 0xff); + write_i2c_reg(SM501_I2C_DATA + 1, (addr + bytes_written) & 0xff); + } + + if (alen == 1) { + write_i2c_reg(SM501_I2C_DATA, (addr + bytes_written) & 0xff); + } + + + for (; i > 0; i--) { + write_i2c_reg(SM501_I2C_DATA + 16 + alen - i, *ptr++); + } + + /* Push it down the bus */ + tmp = read_i2c_reg(SM501_I2C_CONTROL); + tmp |= SM501_I2C_ENABLE | SM501_I2C_START; + write_i2c_reg(SM501_I2C_SLAVE_ADDRESS, (chip << 1) | SM501_I2C_WRITE); + write_i2c_reg(SM501_I2C_BYTE_COUNT, (u_int8_t)(i + alen - 1)); + write_i2c_reg(SM501_I2C_CONTROL, tmp); + + tmp = poll_i2c_status(SM501_I2C_NACK | SM501_I2C_BUS_ERROR | SM501_I2C_XFER_DONE, 100); + + SM501_CHECK_NACK(); + + bytes_written += i; + + } while (len > bytes_written); + + tmp = read_i2c_reg(SM501_I2C_CONTROL); + write_i2c_reg(SM501_I2C_CONTROL, tmp & ~(SM501_I2C_START | SM501_I2C_ENABLE)); + + return(0); +} + + +static unsigned int sm501_i2c_set_bus_speed(unsigned int speed) +{ + u_int8_t tmp; + + tmp = read_i2c_reg(SM501_I2C_CONTROL); + + /* SM501/502 only allows 100 or 400 KHz speed (standard or fast) */ + if (speed <= 100000) + tmp &= ~SM501_I2C_SPEED; + else + tmp |= SM501_I2C_SPEED; + + write_i2c_reg(SM501_I2C_CONTROL, tmp); + + if (gd->flags & GD_FLG_RELOC) { + sm501_i2c_adap.speed = speed <= 100000 ? 100000 : 400000; + } + + return(speed <= 100000 ? 100000 : 400000); +} + + +static unsigned int sm501_i2c_get_bus_speed(void) +{ + return(sm501_i2c_adap.speed); +} + + +i2c_adap_t sm501_i2c_adap = { + .init = sm501_i2c_init, + .probe = sm501_i2c_probe, + .read = sm501_i2c_read, + .write = sm501_i2c_write, + .set_bus_speed = sm501_i2c_set_bus_speed, + .get_bus_speed = sm501_i2c_get_bus_speed, + .speed = CONFIG_SYS_SM501_I2C_SPEED, + .slaveaddr = CONFIG_SYS_SM501_I2C_SLAVE, + .init_done = 0, + .name = "sm501_i2c" +}; +#endif /* CONFIG_SM502_I2C */ diff -purN u-boot-i2c.orig/include/sm501-regs.h u-boot-i2c/include/sm501-regs.h --- u-boot-i2c.orig/include/sm501-regs.h 1969-12-31 16:00:00.000000000 -0800 +++ u-boot-i2c/include/sm501-regs.h 2009-02-12 10:46:00.000000000 -0800 @@ -0,0 +1,394 @@ +/* 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 +*/ + +#ifndef CONFIG_SYS_SM501_BASEADDR +/* SM501/502 on PCI bus, resides and initialized in $(BOARD).c */ +extern unsigned long sm501_iomem_base; +#else +/* SM501/502 on processor bus, preassigned address */ +#define sm501_iomem_base CONFIG_SYS_SM501_BASEADDR +#endif + +/* System Configuration area */ +/* System config base */ +#define SM501_SYS_CONFIG (0x000000) + +/* config 1 */ +#define SM501_SYSTEM_CONTROL (0x000000) + +#define SM501_SYSCTRL_PANEL_TRISTATE (1<<0) +#define SM501_SYSCTRL_MEM_TRISTATE (1<<1) +#define SM501_SYSCTRL_CRT_TRISTATE (1<<2) + +#define SM501_SYSCTRL_PCI_SLAVE_BURST_MASK (3<<4) +#define SM501_SYSCTRL_PCI_SLAVE_BURST_1 (0<<4) +#define SM501_SYSCTRL_PCI_SLAVE_BURST_2 (1<<4) +#define SM501_SYSCTRL_PCI_SLAVE_BURST_4 (2<<4) +#define SM501_SYSCTRL_PCI_SLAVE_BURST_8 (3<<4) + +#define SM501_SYSCTRL_PCI_CLOCK_RUN_EN (1<<6) +#define SM501_SYSCTRL_PCI_RETRY_DISABLE (1<<7) +#define SM501_SYSCTRL_PCI_SUBSYS_LOCK (1<<11) +#define SM501_SYSCTRL_PCI_BURST_READ_EN (1<<15) + +/* miscellaneous control */ + +#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_DEVICEID_REVMASK (0x000000ff) + +#define SM501_PLLCLOCK_COUNT (0x000064) +#define SM501_MISC_TIMING (0x000068) +#define SM501_CURRENT_SDRAM_CLOCK (0x00006C) + +#define SM501_PROGRAMMABLE_PLL_CONTROL (0x000074) + +/* 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 controller/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)

Hello ksi,
ksi@koi8.net wrote:
Signed-off-by: Sergey Kubushyn ksi@koi8.net
Patch apply with warnings:
Applying: 6/12 Multiadapter/multibus I2C, drivers part 3 /home/hs/i2c/u-boot-i2c/.git/rebase-apply/patch:108: trailing whitespace.
/home/hs/i2c/u-boot-i2c/.git/rebase-apply/patch:125: trailing whitespace.
/home/hs/i2c/u-boot-i2c/.git/rebase-apply/patch:209: trailing whitespace. /* /home/hs/i2c/u-boot-i2c/.git/rebase-apply/patch:227: trailing whitespace.
/home/hs/i2c/u-boot-i2c/.git/rebase-apply/patch:259: trailing whitespace. /* warning: squelched 3 whitespace errors warning: 8 lines add whitespace errors.
please fix
bye Heiko

Dear ksi@koi8.net,
In message Pine.LNX.4.64ksi.0902121420120.21067@home-gw.koi8.net you wrote: ...
+/* I2C registers field definitions */ +/* --------------------------------*/ +/* I2C_CONTROL (R/W) */
Incorrect multi-line comment style. Please fix.
+#define SM501_CHECK_NACK() \
- do {\
if (tmp & (SM501_I2C_NACK | SM501_I2C_BUS_ERROR)) {\
tmp = read_i2c_reg(SM501_I2C_CONTROL) & SM501_I2C_SPEED;\
write_i2c_reg(SM501_I2C_CONTROL, tmp);\
return(1);\
}\
- } while (0)
Macros with magic side effects (here on the variable "tmp" are stronly deprecated. Please fix this.
+static __inline__ void write_i2c_reg(unsigned long offset, u_int8_t data) +{
- writeb(data, sm501_iomem_base + SM501_I2C + offset);
- __asm__("sync;isync;msync");
Are you sure the "sync;isync;msync" is needed? The accessor functions should make sure this is not necessary.
Also, instead of register offsets ("...base + SM501_I2C") please use a proper data structure.
- do {
stat = read_i2c_reg(SM501_I2C_STATUS);
if (stat & mask) {
return(stat);
}
No curly braces for single line statements. Same goes everywhere esle, too.
And "return" is not a function - please omit the parens (here and elsewhere).
- while ((read_i2c_reg(SM501_I2C_STATUS) & SM501_I2C_BUS_BUSY) && timeout--) {
...
diff -purN u-boot-i2c.orig/include/sm501-regs.h u-boot-i2c/include/sm501-regs.h --- u-boot-i2c.orig/include/sm501-regs.h 1969-12-31 16:00:00.000000000 -0800 +++ u-boot-i2c/include/sm501-regs.h 2009-02-12 10:46:00.000000000 -0800 @@ -0,0 +1,394 @@ +/* sm501-regs.h
Incorrect multiline comment style.
...
+#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)
Please do not use register offsets, define a C structure instead.
Best regards,
Wolfgang Denk

On Mon, 16 Feb 2009, Wolfgang Denk wrote:
Dear ksi@koi8.net,
In message Pine.LNX.4.64ksi.0902121420120.21067@home-gw.koi8.net you wrote: ...
+/* I2C registers field definitions */ +/* --------------------------------*/ +/* I2C_CONTROL (R/W) */
Incorrect multi-line comment style. Please fix.
OK.
+#define SM501_CHECK_NACK() \
- do {\
if (tmp & (SM501_I2C_NACK | SM501_I2C_BUS_ERROR)) {\
tmp = read_i2c_reg(SM501_I2C_CONTROL) & SM501_I2C_SPEED;\
write_i2c_reg(SM501_I2C_CONTROL, tmp);\
return(1);\
}\
- } while (0)
Macros with magic side effects (here on the variable "tmp" are stronly deprecated. Please fix this.
There is exactly the same code in Davinci I2C driver that had been accepted and made it in the main tree. The reason behind this macro is to make source text smaller and easier to read. It is not used anywhere outside the driver file. Sure I can remove it and put that text as is anywhere where it is used (3 places in sm502_i2c.c) but will it make it any better or more readable? I seriously doubt it.
And that side effect on tmp is accounted for so there is no magic. Nothing in the code relies on tmp content after that code fragment is executed. This is just a text fragment that gets inserted literally at appropriate places.
Please let me know if you still want that changed. I will remove that standard "do {...} while(0)" wrapper and paste the "if{...}" block at appropriate places.
Are we going to do the same for Davinci I2C driver for consistency?
+static __inline__ void write_i2c_reg(unsigned long offset, u_int8_t data) +{
- writeb(data, sm501_iomem_base + SM501_I2C + offset);
- __asm__("sync;isync;msync");
Are you sure the "sync;isync;msync" is needed? The accessor functions should make sure this is not necessary.
OK. It would do any harm anyways but I will remove that.
Also, instead of register offsets ("...base + SM501_I2C") please use a proper data structure.
Are we dropping Linux compatibility or what? That sm501-regs.h file with those offsets has been directly stolen from Linux kernel source and that's the way they do it in the kernel. I don't have any problems with making a data structure for SM501/502 I2C controller but aren't we supposed to keep parts stolen from Linux kernel as close to the source as possible?
Please advise.
- do {
stat = read_i2c_reg(SM501_I2C_STATUS);
if (stat & mask) {
return(stat);
}
No curly braces for single line statements. Same goes everywhere esle, too.
What's wrong with bracing a single line statement? I personally think it makes code more readable and less prone for errors. But sure, I can change this, no problems...
And "return" is not a function - please omit the parens (here and elsewhere).
There is a lot of places where parens are used with "return" in U-Boot (and elsewhere.) C does not REQUIRE parens but also does not forbid or discourage them. I personally think it is better to use excessive parens than not to use them when they are required.
But sure I can change it if it really matters...
- while ((read_i2c_reg(SM501_I2C_STATUS) & SM501_I2C_BUS_BUSY) && timeout--) {
...
diff -purN u-boot-i2c.orig/include/sm501-regs.h u-boot-i2c/include/sm501-regs.h --- u-boot-i2c.orig/include/sm501-regs.h 1969-12-31 16:00:00.000000000 -0800 +++ u-boot-i2c/include/sm501-regs.h 2009-02-12 10:46:00.000000000 -0800 @@ -0,0 +1,394 @@ +/* sm501-regs.h
Incorrect multiline comment style.
This again comes from Linux kernel source. Should I fix such borrowed code?
...
+#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)
Please do not use register offsets, define a C structure instead.
Again, this came from Linux kernel. Should I throw it away and make my own header file? Please advise.
--- ****************************************************************** * KSI@home KOI8 Net < > The impossible we do immediately. * * Las Vegas NV, USA < > Miracles require 24-hour notice. * ******************************************************************

Dear ksi@koi8.net,
In message Pine.LNX.4.64ksi.0902171117560.30222@home-gw.koi8.net you wrote:
Macros with magic side effects (here on the variable "tmp" are stronly deprecated. Please fix this.
There is exactly the same code in Davinci I2C driver that had been accepted and made it in the main tree. The reason behind this macro is to make source
Sorry that the code slipped through the review then.
text smaller and easier to read. It is not used anywhere outside the driver
That's not easier to read. Macros with side effects are dangerous. Quoting the CodingStyle: "...might look like a good thing, but it's confusing as hell when one reads the code and it's prone to breakage from seemingly innocent changes."
file. Sure I can remove it and put that text as is anywhere where it is used (3 places in sm502_i2c.c) but will it make it any better or more readable? I seriously doubt it.
I am sure about it.
And while you are at it, I would appreciate if you could also post a (separate) patch that fixes the same thing in the Davinci I2C driver you were referring to above. Thanks.
Are we going to do the same for Davinci I2C driver for consistency?
Yes, please.
Also, instead of register offsets ("...base + SM501_I2C") please use a proper data structure.
Are we dropping Linux compatibility or what? That sm501-regs.h file with those offsets has been directly stolen from Linux kernel source and that's the way they do it in the kernel. I don't have any problems with making a
I am aware that there are areas in the Linux kernel that could need a thourough rework - like in any bigger project.
Borrowing good code from Linux is excellent. But there is no reason not to improve poor code when we run into it. Who knows, maybe one day Linux might copy code from U-Boot - now would that be a bad thing?
data structure for SM501/502 I2C controller but aren't we supposed to keep parts stolen from Linux kernel as close to the source as possible?
Yes, we are - when it makes sense.
If you have free cycles you could try and post such a cleanup patch for Linux too, so both keep in sync.
if (stat & mask) {
return(stat);
}
No curly braces for single line statements. Same goes everywhere esle, too.
What's wrong with bracing a single line statement? I personally think it makes code more readable and less prone for errors. But sure, I can change this, no problems...
I agree with you - my personal preference is quite often to use braces there, too. But we try all to stick to a common coding style, so everybody has to swallow his own set of bitter pills.
And "return" is not a function - please omit the parens (here and elsewhere).
There is a lot of places where parens are used with "return" in U-Boot (and elsewhere.) C does not REQUIRE parens but also does not forbid or discourage them. I personally think it is better to use excessive parens than not to use them when they are required.
But sure I can change it if it really matters...
We try to be good citicens and follow the CodingStyle, that's all...
- while ((read_i2c_reg(SM501_I2C_STATUS) & SM501_I2C_BUS_BUSY) && timeout--) {
...
diff -purN u-boot-i2c.orig/include/sm501-regs.h u-boot-i2c/include/sm501-regs.h --- u-boot-i2c.orig/include/sm501-regs.h 1969-12-31 16:00:00.000000000 -0800 +++ u-boot-i2c/include/sm501-regs.h 2009-02-12 10:46:00.000000000 -0800 @@ -0,0 +1,394 @@ +/* sm501-regs.h
Incorrect multiline comment style.
This again comes from Linux kernel source. Should I fix such borrowed code?
Since more changes are required to that file anyway: yes, please.
Please do not use register offsets, define a C structure instead.
Again, this came from Linux kernel. Should I throw it away and make my own header file? Please advise.
Yes, please.
Best regards,
Wolfgang Denk

On Thu, 19 Feb 2009, Wolfgang Denk wrote:
Dear ksi@koi8.net,
In message Pine.LNX.4.64ksi.0902171117560.30222@home-gw.koi8.net you wrote:
Macros with magic side effects (here on the variable "tmp" are stronly deprecated. Please fix this.
There is exactly the same code in Davinci I2C driver that had been accepted and made it in the main tree. The reason behind this macro is to make source
Sorry that the code slipped through the review then.
text smaller and easier to read. It is not used anywhere outside the driver
That's not easier to read. Macros with side effects are dangerous. Quoting the CodingStyle: "...might look like a good thing, but it's confusing as hell when one reads the code and it's prone to breakage from seemingly innocent changes."
file. Sure I can remove it and put that text as is anywhere where it is used (3 places in sm502_i2c.c) but will it make it any better or more readable? I seriously doubt it.
I am sure about it.
And while you are at it, I would appreciate if you could also post a (separate) patch that fixes the same thing in the Davinci I2C driver you were referring to above. Thanks.
Are we going to do the same for Davinci I2C driver for consistency?
Yes, please.
OK, posted a patch.
Also, instead of register offsets ("...base + SM501_I2C") please use a proper data structure.
Are we dropping Linux compatibility or what? That sm501-regs.h file with those offsets has been directly stolen from Linux kernel source and that's the way they do it in the kernel. I don't have any problems with making a
I am aware that there are areas in the Linux kernel that could need a thourough rework - like in any bigger project.
Borrowing good code from Linux is excellent. But there is no reason not to improve poor code when we run into it. Who knows, maybe one day Linux might copy code from U-Boot - now would that be a bad thing?
OK, will replace it with a structure, no problems. I do personally prefer a structure too; that code was against my nature but I tried to keep Linux kernel files intact.
[dd]
What's wrong with bracing a single line statement? I personally think it makes code more readable and less prone for errors. But sure, I can change this, no problems...
I agree with you - my personal preference is quite often to use braces there, too. But we try all to stick to a common coding style, so everybody has to swallow his own set of bitter pills.
OK, no problems.
And "return" is not a function - please omit the parens (here and elsewhere).
There is a lot of places where parens are used with "return" in U-Boot (and elsewhere.) C does not REQUIRE parens but also does not forbid or discourage them. I personally think it is better to use excessive parens than not to use them when they are required.
But sure I can change it if it really matters...
We try to be good citicens and follow the CodingStyle, that's all...
OK.
- while ((read_i2c_reg(SM501_I2C_STATUS) & SM501_I2C_BUS_BUSY) && timeout--) {
...
diff -purN u-boot-i2c.orig/include/sm501-regs.h u-boot-i2c/include/sm501-regs.h --- u-boot-i2c.orig/include/sm501-regs.h 1969-12-31 16:00:00.000000000 -0800 +++ u-boot-i2c/include/sm501-regs.h 2009-02-12 10:46:00.000000000 -0800 @@ -0,0 +1,394 @@ +/* sm501-regs.h
Incorrect multiline comment style.
This again comes from Linux kernel source. Should I fix such borrowed code?
Since more changes are required to that file anyway: yes, please.
OK.
Please do not use register offsets, define a C structure instead.
Again, this came from Linux kernel. Should I throw it away and make my own header file? Please advise.
Yes, please.
OK, not a big deal, will do.
--- ****************************************************************** * KSI@home KOI8 Net < > The impossible we do immediately. * * Las Vegas NV, USA < > Miracles require 24-hour notice. * ******************************************************************
participants (3)
-
Heiko Schocher
-
ksi@koi8.net
-
Wolfgang Denk