[RFC] Initial OLPC Viafb merge (V2)

[RFC] Initial OLPC Viafb merge (V2)

[Jonathan Corbet]

The following patches are the beginning of an effort to move work done for
the OLPC XO 1.5 machine into the mainline.  What's here is basic support
for the VX855 chipset, I2C, and the OLPC panel display.  What's coming in
the future is a reworking of the viafb driver into something resembling a
proper multifunction device, GPIO support, and camera support.  But I'd
like to start here.

CHANGES FROM THE FIRST POSTING include fixes in response to a number of
Florian's comments and the removal of the suspend/resume patches.  The
patch series is also now based on 2.6.34-rc3.

If there's no objections, I'll start a tree and get it into linux-next in
the near future, with an eye toward a 2.6.35 merge.

Thanks,

jon

Chris Ball (2):
      viafb: Add 1200x900 DCON/LCD panel modes for OLPC XO-1.5
      viafb: Do not probe for LVDS/TMDS on OLPC XO-1.5

Harald Welte (4):
      [FB] viafb: Fix various resource leaks during module_init()
      viafb: use proper pci config API
      viafb: Determine type of 2D engine and store it in chip_info
      viafb: rework the I2C support in the VIA framebuffer driver

Jonathan Corbet (4):
      viafb: Unmap the frame buffer on initialization error
      viafb: Retain GEMODE reserved bits
      viafb: Unify duplicated set_bpp() code
      viafb: complete support for VX800/VX855 accelerated framebuffer

Paul Fox (1):
      suppress verbose debug messages:  change printk() to DEBUG_MSG()

 drivers/video/via/accel.c    |  111 +++++++++++++++++----------
 drivers/video/via/accel.h    |   40 ++++++++++
 drivers/video/via/chip.h     |    8 ++
 drivers/video/via/dvi.c      |   35 +++-----
 drivers/video/via/hw.c       |   85 +++++++++++++++------
 drivers/video/via/hw.h       |    4 -
 drivers/video/via/ioctl.h    |    2 
 drivers/video/via/lcd.c      |   35 ++++++--
 drivers/video/via/lcd.h      |    2 
 drivers/video/via/share.h    |    7 +
 drivers/video/via/via_i2c.c  |  171 ++++++++++++++++++++++++++-----------------
 drivers/video/via/via_i2c.h  |   43 +++++++---
 drivers/video/via/viafbdev.c |   60 +++++++++++----
 drivers/video/via/viafbdev.h |    4 -
 drivers/video/via/viamode.c  |   14 +++
 drivers/video/via/vt1636.c   |   36 ++++-----
 drivers/video/via/vt1636.h   |    2 
 17 files changed, 446 insertions(+), 213 deletions(-)

[PATCH 01/11] [FB] viafb: Fix various resource leaks during module_init()

[Jonathan Corbet]

From: Harald Welte <laforge@gnumonks.org>

The current code executed from module_init() in viafb does not have
proper error checking and [partial] resoure release paths in case
an error happens half way through driver initialization.

This patch adresses the most obvious of those issues, such as a
leftover i2c bus if module_init (and thus module load) fails.

[jc: fixed merge conflicts]
[jc: also restored -ENOMEM return on ioremap() fail]

Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc: ScottFang@viatech.com.cn
Cc: JosephChan@via.com.tw
Signed-off-by: Harald Welte <HaraldWelte@viatech.com>
---
 drivers/video/via/viafbdev.c |   52 +++++++++++++++++++++++++++++++----------
 1 files changed, 39 insertions(+), 13 deletions(-)

diff --git a/drivers/video/via/viafbdev.c b/drivers/video/via/viafbdev.c
index ce7783b..b7018ef 100644
--- a/drivers/video/via/viafbdev.c
+++ b/drivers/video/via/viafbdev.c
@@ -1,5 +1,5 @@
 /*
- * Copyright 1998-2008 VIA Technologies, Inc. All Rights Reserved.
+ * Copyright 1998-2009 VIA Technologies, Inc. All Rights Reserved.
  * Copyright 2001-2008 S3 Graphics, Inc. All Rights Reserved.
 
  * This program is free software; you can redistribute it and/or
@@ -1737,6 +1737,7 @@ static int __devinit via_pci_probe(struct pci_dev *pdev,
 	u32 default_xres, default_yres;
 	struct VideoModeTable *vmode_entry;
 	struct fb_var_screeninfo default_var;
+	int rc;
 	u32 viafb_par_length;
 
 	DEBUG_MSG(KERN_INFO "VIAFB PCI Probe!!\n");
@@ -1751,7 +1752,7 @@ static int __devinit via_pci_probe(struct pci_dev *pdev,
 		&pdev->dev);
 	if (!viafbinfo) {
 		printk(KERN_ERR"Could not allocate memory for viafb_info.\n");
-		return -ENODEV;
+		return -ENOMEM;
 	}
 
 	viaparinfo = (struct viafb_par *)viafbinfo->par;
@@ -1774,7 +1775,9 @@ static int __devinit via_pci_probe(struct pci_dev *pdev,
 		viafb_dual_fb = 0;
 
 	/* Set up I2C bus stuff */
-	viafb_create_i2c_bus(viaparinfo);
+	rc = viafb_create_i2c_bus(viaparinfo);
+	if (rc)
+		goto out_fb_release;
 
 	viafb_init_chip_info(pdev, ent);
 	viaparinfo->fbmem = pci_resource_start(pdev, 0);
@@ -1785,7 +1788,8 @@ static int __devinit via_pci_probe(struct pci_dev *pdev,
 		viaparinfo->memsize);
 	if (!viafbinfo->screen_base) {
 		printk(KERN_INFO "ioremap failed\n");
-		return -ENOMEM;
+		rc = -ENOMEM;
+		goto out_delete_i2c;
 	}
 
 	viafbinfo->fix.mmio_start = pci_resource_start(pdev, 1);
@@ -1861,8 +1865,8 @@ static int __devinit via_pci_probe(struct pci_dev *pdev,
 		if (!viafbinfo1) {
 			printk(KERN_ERR
 			"allocate the second framebuffer struct error\n");
-			framebuffer_release(viafbinfo);
-			return -ENOMEM;
+			rc = -ENOMEM;
+			goto out_delete_i2c;
 		}
 		viaparinfo1 = viafbinfo1->par;
 		memcpy(viaparinfo1, viaparinfo, viafb_par_length);
@@ -1913,21 +1917,26 @@ static int __devinit via_pci_probe(struct pci_dev *pdev,
 	viaparinfo->depth = fb_get_color_depth(&viafbinfo->var,
 		&viafbinfo->fix);
 	default_var.activate = FB_ACTIVATE_NOW;
-	fb_alloc_cmap(&viafbinfo->cmap, 256, 0);
+	rc = fb_alloc_cmap(&viafbinfo->cmap, 256, 0);
+	if (rc)
+		goto out_fb1_release;
 
 	if (viafb_dual_fb && (viafb_primary_dev == LCD_Device)
 	    && (viaparinfo->chip_info->gfx_chip_name == UNICHROME_CLE266)) {
-		if (register_framebuffer(viafbinfo1) < 0)
-			return -EINVAL;
+		rc = register_framebuffer(viafbinfo1);
+		if (rc)
+			goto out_dealloc_cmap;
 	}
-	if (register_framebuffer(viafbinfo) < 0)
-		return -EINVAL;
+	rc = register_framebuffer(viafbinfo);
+	if (rc)
+		goto out_fb1_unreg_lcd_cle266;
 
 	if (viafb_dual_fb && ((viafb_primary_dev != LCD_Device)
 			|| (viaparinfo->chip_info->gfx_chip_name !=
 			UNICHROME_CLE266))) {
-		if (register_framebuffer(viafbinfo1) < 0)
-			return -EINVAL;
+		rc = register_framebuffer(viafbinfo1);
+		if (rc)
+			goto out_fb_unreg;
 	}
 	DEBUG_MSG(KERN_INFO "fb%d: %s frame buffer device %dx%d-%dbpp\n",
 		  viafbinfo->node, viafbinfo->fix.id, default_var.xres,
@@ -1936,6 +1945,23 @@ static int __devinit via_pci_probe(struct pci_dev *pdev,
 	viafb_init_proc(&viaparinfo->shared->proc_entry);
 	viafb_init_dac(IGA2);
 	return 0;
+
+out_fb_unreg:
+	unregister_framebuffer(viafbinfo);
+out_fb1_unreg_lcd_cle266:
+	if (viafb_dual_fb && (viafb_primary_dev == LCD_Device)
+	    && (viaparinfo->chip_info->gfx_chip_name == UNICHROME_CLE266))
+		unregister_framebuffer(viafbinfo1);
+out_dealloc_cmap:
+	fb_dealloc_cmap(&viafbinfo->cmap);
+out_fb1_release:
+	if (viafbinfo1)
+		framebuffer_release(viafbinfo1);
+out_delete_i2c:
+	viafb_delete_i2c_buss(viaparinfo);
+out_fb_release:
+	framebuffer_release(viafbinfo);
+	return rc;
 }
 
 static void __devexit via_pci_remove(struct pci_dev *pdev)
-- 
1.7.0.1

[PATCH 02/11] viafb: use proper pci config API

[Jonathan Corbet]

From: Harald Welte <laforge@gnumonks.org>

This patch alters viafb to use the proper Linux in-kernel API to access
PCI configuration space, rather than poking at I/O ports by itself.

Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc: ScottFang@viatech.com.cn
Cc: JosephChan@via.com.tw
Signed-off-by: Harald Welte <HaraldWelte@viatech.com>
---
 drivers/video/via/hw.c       |   62 +++++++++++++++++++++++++----------------
 drivers/video/via/hw.h       |    4 +-
 drivers/video/via/viafbdev.c |    4 +++
 3 files changed, 44 insertions(+), 26 deletions(-)

diff --git a/drivers/video/via/hw.c b/drivers/video/via/hw.c
index b1e6dc9..69acae1 100644
--- a/drivers/video/via/hw.c
+++ b/drivers/video/via/hw.c
@@ -2474,24 +2474,37 @@ static void disable_second_display_channel(void)
 	viafb_write_reg_mask(CR6A, VIACR, BIT6, BIT6);
 }
 
+static u_int16_t via_function3[] = {
+	CLE266_FUNCTION3, KM400_FUNCTION3, CN400_FUNCTION3, CN700_FUNCTION3,
+	CX700_FUNCTION3, KM800_FUNCTION3, KM890_FUNCTION3, P4M890_FUNCTION3,
+	P4M900_FUNCTION3, VX800_FUNCTION3, VX855_FUNCTION3,
+};
+
+/* Get the BIOS-configured framebuffer size from PCI configuration space
+ * of function 3 in the respective chipset */
 int viafb_get_fb_size_from_pci(void)
 {
-	unsigned long configid, deviceid, FBSize = 0;
-	int VideoMemSize;
-	int DeviceFound = false;
-
-	for (configid = 0x80000000; configid < 0x80010800; configid += 0x100) {
-		outl(configid, (unsigned long)0xCF8);
-		deviceid = (inl((unsigned long)0xCFC) >> 16) & 0xffff;
-
-		switch (deviceid) {
-		case CLE266:
-		case KM400:
-			outl(configid + 0xE0, (unsigned long)0xCF8);
-			FBSize = inl((unsigned long)0xCFC);
-			DeviceFound = true;	/* Found device id */
-			break;
+	int i;
+	u_int8_t offset = 0;
+	u_int32_t FBSize;
+	u_int32_t VideoMemSize;
+
+	/* search for the "FUNCTION3" device in this chipset */
+	for (i = 0; i < ARRAY_SIZE(via_function3); i++) {
+		struct pci_dev *pdev;
+
+		pdev = pci_get_device(PCI_VENDOR_ID_VIA, via_function3[i],
+				      NULL);
+		if (!pdev)
+			continue;
+
+		DEBUG_MSG(KERN_INFO "Device ID = %x\n", pdev->device);
 
+		switch (pdev->device) {
+		case CLE266_FUNCTION3:
+		case KM400_FUNCTION3:
+			offset = 0xE0;
+			break;
 		case CN400_FUNCTION3:
 		case CN700_FUNCTION3:
 		case CX700_FUNCTION3:
@@ -2501,21 +2514,22 @@ int viafb_get_fb_size_from_pci(void)
 		case P4M900_FUNCTION3:
 		case VX800_FUNCTION3:
 		case VX855_FUNCTION3:
-			/*case CN750_FUNCTION3: */
-			outl(configid + 0xA0, (unsigned long)0xCF8);
-			FBSize = inl((unsigned long)0xCFC);
-			DeviceFound = true;	/* Found device id */
-			break;
-
-		default:
+		/*case CN750_FUNCTION3: */
+			offset = 0xA0;
 			break;
 		}
 
-		if (DeviceFound)
+		if (!offset)
 			break;
+
+		pci_read_config_dword(pdev, offset, &FBSize);
+		pci_dev_put(pdev);
 	}
 
-	DEBUG_MSG(KERN_INFO "Device ID = %lx\n", deviceid);
+	if (!offset) {
+		printk(KERN_ERR "cannot determine framebuffer size\n");
+		return -EIO;
+	}
 
 	FBSize = FBSize & 0x00007000;
 	DEBUG_MSG(KERN_INFO "FB Size = %x\n", FBSize);
diff --git a/drivers/video/via/hw.h b/drivers/video/via/hw.h
index 12ef32d..d6b25ac 100644
--- a/drivers/video/via/hw.h
+++ b/drivers/video/via/hw.h
@@ -823,8 +823,8 @@ struct iga2_crtc_timing {
 };
 
 /* device ID */
-#define CLE266              0x3123
-#define KM400               0x3205
+#define CLE266_FUNCTION3    0x3123
+#define KM400_FUNCTION3     0x3205
 #define CN400_FUNCTION2     0x2259
 #define CN400_FUNCTION3     0x3259
 /* support VT3314 chipset */
diff --git a/drivers/video/via/viafbdev.c b/drivers/video/via/viafbdev.c
index b7018ef..8af405b 100644
--- a/drivers/video/via/viafbdev.c
+++ b/drivers/video/via/viafbdev.c
@@ -1782,6 +1782,10 @@ static int __devinit via_pci_probe(struct pci_dev *pdev,
 	viafb_init_chip_info(pdev, ent);
 	viaparinfo->fbmem = pci_resource_start(pdev, 0);
 	viaparinfo->memsize = viafb_get_fb_size_from_pci();
+	if (viaparinfo->memsize < 0) {
+		rc = viaparinfo->memsize;
+		goto out_delete_i2c;
+	}
 	viaparinfo->fbmem_free = viaparinfo->memsize;
 	viaparinfo->fbmem_used = 0;
 	viafbinfo->screen_base = ioremap_nocache(viaparinfo->fbmem,
-- 
1.7.0.1

[PATCH 03/11] viafb: Unmap the frame buffer on initialization error

[Jonathan Corbet]

This was part of Harald's "make viafb a first-class citizen using
pci_driver" patch, but somehow got dropped when that patch went into
mainline.

Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc: ScottFang@viatech.com.cn
Cc: JosephChan@via.com.tw
Signed-off-by: Jonathan Corbet <corbet@lwn.net>
---
 drivers/video/via/viafbdev.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/video/via/viafbdev.c b/drivers/video/via/viafbdev.c
index 8af405b..8955ab4 100644
--- a/drivers/video/via/viafbdev.c
+++ b/drivers/video/via/viafbdev.c
@@ -1870,7 +1870,7 @@ static int __devinit via_pci_probe(struct pci_dev *pdev,
 			printk(KERN_ERR
 			"allocate the second framebuffer struct error\n");
 			rc = -ENOMEM;
-			goto out_delete_i2c;
+			goto out_unmap_screen;
 		}
 		viaparinfo1 = viafbinfo1->par;
 		memcpy(viaparinfo1, viaparinfo, viafb_par_length);
@@ -1961,6 +1961,8 @@ out_dealloc_cmap:
 out_fb1_release:
 	if (viafbinfo1)
 		framebuffer_release(viafbinfo1);
+out_unmap_screen:
+	iounmap(viafbinfo->screen_base);
 out_delete_i2c:
 	viafb_delete_i2c_buss(viaparinfo);
 out_fb_release:
-- 
1.7.0.1

[PATCH 04/11] viafb: Retain GEMODE reserved bits

[Jonathan Corbet]

Commit c3e25673843153ea75fda79a47cf12f10a25ca37 (viafb: 2D engine rewrite)
changed the setting of the GEMODE register so that the reserved bits are no
longer preserved.  Fix that; at the same time, move this code to its own
function and restore the use of symbolic constants.

Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc: ScottFang@viatech.com.cn
Cc: JosephChan@via.com.tw
Signed-off-by: Jonathan Corbet <corbet@lwn.net>
---
 drivers/video/via/accel.c |   49 ++++++++++++++++++++++++++++++--------------
 1 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/drivers/video/via/accel.c b/drivers/video/via/accel.c
index d5077df..a52147c 100644
--- a/drivers/video/via/accel.c
+++ b/drivers/video/via/accel.c
@@ -165,12 +165,42 @@ static int hw_bitblt_1(void __iomem *engine, u8 op, u32 width, u32 height,
 	return 0;
 }
 
+/*
+ * Figure out an appropriate bytes-per-pixel setting.
+ */
+static int viafb_set_bpp(void __iomem *engine, u8 bpp)
+{
+	u32 gemode;
+
+	/* Preserve the reserved bits */
+	/* Lowest 2 bits to zero gives us no rotation */
+	gemode = readl(engine + VIA_REG_GEMODE) & 0xfffffcfc;
+	switch (bpp) {
+	case 8:
+		gemode |= VIA_GEM_8bpp;
+		break;
+	case 16:
+		gemode |= VIA_GEM_16bpp;
+		break;
+	case 32:
+		gemode |= VIA_GEM_32bpp;
+		break;
+	default:
+		printk(KERN_WARNING "hw_bitblt_2: Unsupported bpp %d\n", bpp);
+		return -EINVAL;
+	}
+	writel(gemode, engine + VIA_REG_GEMODE);
+	return 0;
+}
+
+
 static int hw_bitblt_2(void __iomem *engine, u8 op, u32 width, u32 height,
 	u8 dst_bpp, u32 dst_addr, u32 dst_pitch, u32 dst_x, u32 dst_y,
 	u32 *src_mem, u32 src_addr, u32 src_pitch, u32 src_x, u32 src_y,
 	u32 fg_color, u32 bg_color, u8 fill_rop)
 {
 	u32 ge_cmd = 0, tmp, i;
+	int ret;
 
 	if (!op || op > 3) {
 		printk(KERN_WARNING "hw_bitblt_2: Invalid operation: %d\n", op);
@@ -204,22 +234,9 @@ static int hw_bitblt_2(void __iomem *engine, u8 op, u32 width, u32 height,
 		}
 	}
 
-	switch (dst_bpp) {
-	case 8:
-		tmp = 0x00000000;
-		break;
-	case 16:
-		tmp = 0x00000100;
-		break;
-	case 32:
-		tmp = 0x00000300;
-		break;
-	default:
-		printk(KERN_WARNING "hw_bitblt_2: Unsupported bpp %d\n",
-			dst_bpp);
-		return -EINVAL;
-	}
-	writel(tmp, engine + 0x04);
+	ret = viafb_set_bpp(engine, dst_bpp);
+	if (ret)
+		return ret;
 
 	if (op == VIA_BITBLT_FILL)
 		tmp = 0;
-- 
1.7.0.1

[PATCH 05/11] viafb: Unify duplicated set_bpp() code

[Jonathan Corbet]

As suggested by Florian: make both mode-setting paths use the same code.

Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc: ScottFang@viatech.com.cn
Cc: JosephChan@via.com.tw
Signed-off-by: Jonathan Corbet <corbet@lwn.net>
---
 drivers/video/via/accel.c |   78 +++++++++++++++++++--------------------------
 1 files changed, 33 insertions(+), 45 deletions(-)

diff --git a/drivers/video/via/accel.c b/drivers/video/via/accel.c
index a52147c..7c1d9c4 100644
--- a/drivers/video/via/accel.c
+++ b/drivers/video/via/accel.c
@@ -20,12 +20,42 @@
  */
 #include "global.h"
 
+/*
+ * Figure out an appropriate bytes-per-pixel setting.
+ */
+static int viafb_set_bpp(void __iomem *engine, u8 bpp)
+{
+	u32 gemode;
+
+	/* Preserve the reserved bits */
+	/* Lowest 2 bits to zero gives us no rotation */
+	gemode = readl(engine + VIA_REG_GEMODE) & 0xfffffcfc;
+	switch (bpp) {
+	case 8:
+		gemode |= VIA_GEM_8bpp;
+		break;
+	case 16:
+		gemode |= VIA_GEM_16bpp;
+		break;
+	case 32:
+		gemode |= VIA_GEM_32bpp;
+		break;
+	default:
+		printk(KERN_WARNING "viafb_set_bpp: Unsupported bpp %d\n", bpp);
+		return -EINVAL;
+	}
+	writel(gemode, engine + VIA_REG_GEMODE);
+	return 0;
+}
+
+
 static int hw_bitblt_1(void __iomem *engine, u8 op, u32 width, u32 height,
 	u8 dst_bpp, u32 dst_addr, u32 dst_pitch, u32 dst_x, u32 dst_y,
 	u32 *src_mem, u32 src_addr, u32 src_pitch, u32 src_x, u32 src_y,
 	u32 fg_color, u32 bg_color, u8 fill_rop)
 {
 	u32 ge_cmd = 0, tmp, i;
+	int ret;
 
 	if (!op || op > 3) {
 		printk(KERN_WARNING "hw_bitblt_1: Invalid operation: %d\n", op);
@@ -59,22 +89,9 @@ static int hw_bitblt_1(void __iomem *engine, u8 op, u32 width, u32 height,
 		}
 	}
 
-	switch (dst_bpp) {
-	case 8:
-		tmp = 0x00000000;
-		break;
-	case 16:
-		tmp = 0x00000100;
-		break;
-	case 32:
-		tmp = 0x00000300;
-		break;
-	default:
-		printk(KERN_WARNING "hw_bitblt_1: Unsupported bpp %d\n",
-			dst_bpp);
-		return -EINVAL;
-	}
-	writel(tmp, engine + 0x04);
+	ret = viafb_set_bpp(engine, dst_bpp);
+	if (ret)
+		return ret;
 
 	if (op != VIA_BITBLT_FILL) {
 		if (src_x & (op == VIA_BITBLT_MONO ? 0xFFFF8000 : 0xFFFFF000)
@@ -165,35 +182,6 @@ static int hw_bitblt_1(void __iomem *engine, u8 op, u32 width, u32 height,
 	return 0;
 }
 
-/*
- * Figure out an appropriate bytes-per-pixel setting.
- */
-static int viafb_set_bpp(void __iomem *engine, u8 bpp)
-{
-	u32 gemode;
-
-	/* Preserve the reserved bits */
-	/* Lowest 2 bits to zero gives us no rotation */
-	gemode = readl(engine + VIA_REG_GEMODE) & 0xfffffcfc;
-	switch (bpp) {
-	case 8:
-		gemode |= VIA_GEM_8bpp;
-		break;
-	case 16:
-		gemode |= VIA_GEM_16bpp;
-		break;
-	case 32:
-		gemode |= VIA_GEM_32bpp;
-		break;
-	default:
-		printk(KERN_WARNING "hw_bitblt_2: Unsupported bpp %d\n", bpp);
-		return -EINVAL;
-	}
-	writel(gemode, engine + VIA_REG_GEMODE);
-	return 0;
-}
-
-
 static int hw_bitblt_2(void __iomem *engine, u8 op, u32 width, u32 height,
 	u8 dst_bpp, u32 dst_addr, u32 dst_pitch, u32 dst_x, u32 dst_y,
 	u32 *src_mem, u32 src_addr, u32 src_pitch, u32 src_x, u32 src_y,
-- 
1.7.0.1

[PATCH 06/11] viafb: Determine type of 2D engine and store it in chip_info

[Jonathan Corbet]

From: Harald Welte <laforge@gnumonks.org>

This will help us for the upcoming support for 2D acceleration using
the M1 engine.

[jc: fixed merge conflicts]
Signed-off-by: Harald Welte <HaraldWelte@viatech.com>
---
 drivers/video/via/chip.h |    8 ++++++++
 drivers/video/via/hw.c   |   15 +++++++++++++++
 2 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/drivers/video/via/chip.h b/drivers/video/via/chip.h
index 8c06bd3..d9b6e06 100644
--- a/drivers/video/via/chip.h
+++ b/drivers/video/via/chip.h
@@ -121,9 +121,17 @@ struct lvds_chip_information {
 	int i2c_port;
 };
 
+/* The type of 2D engine */
+enum via_2d_engine {
+	VIA_2D_ENG_H2,
+	VIA_2D_ENG_H5,
+	VIA_2D_ENG_M1,
+};
+
 struct chip_information {
 	int gfx_chip_name;
 	int gfx_chip_revision;
+	enum via_2d_engine twod_engine;
 	struct tmds_chip_information tmds_chip_info;
 	struct lvds_chip_information lvds_chip_info;
 	struct lvds_chip_information lvds_chip_info2;
diff --git a/drivers/video/via/hw.c b/drivers/video/via/hw.c
index 69acae1..d893695 100644
--- a/drivers/video/via/hw.c
+++ b/drivers/video/via/hw.c
@@ -2017,6 +2017,21 @@ static void init_gfx_chip_info(struct pci_dev *pdev,
 				CX700_REVISION_700;
 		}
 	}
+
+	/* Determine which 2D engine we have */
+	switch (viaparinfo->chip_info->gfx_chip_name) {
+	case UNICHROME_VX800:
+	case UNICHROME_VX855:
+		viaparinfo->chip_info->twod_engine = VIA_2D_ENG_M1;
+		break;
+	case UNICHROME_K8M890:
+	case UNICHROME_P4M900:
+		viaparinfo->chip_info->twod_engine = VIA_2D_ENG_H5;
+		break;
+	default:
+		viaparinfo->chip_info->twod_engine = VIA_2D_ENG_H2;
+		break;
+	}
 }
 
 static void init_tmds_chip_info(void)
-- 
1.7.0.1

[PATCH 07/11] viafb: complete support for VX800/VX855 accelerated framebuffer

[Jonathan Corbet]

This patch is a painful merge of change
a90bab567ece3e915d0ccd55ab00c9bb333fa8c0 (viafb: Add support for 2D
accelerated framebuffer on VX800/VX855) in the OLPC tree, originally by
Harald Welte.  Harald's changelog read:

	The VX800/VX820 and the VX855/VX875 chipsets have a different 2D
    	acceleration engine called "M1".  The M1 engine has some subtle
    	(and some not-so-subtle) differences to the previous engines, so
    	support for accelerated framebuffer on those chipsets was disabled
    	so far.

This merge tries to preserve Harald's changes in the framework of the
much-changed 2.6.34 viafb code.

Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc: ScottFang@viatech.com.cn
Cc: JosephChan@via.com.tw
Signed-off-by: Jonathan Corbet <corbet@lwn.net>
---
 drivers/video/via/accel.c |   42 +++++++++++++++++++++++++++++++++---------
 drivers/video/via/accel.h |   40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 9 deletions(-)

diff --git a/drivers/video/via/accel.c b/drivers/video/via/accel.c
index 7c1d9c4..0d90c85 100644
--- a/drivers/video/via/accel.c
+++ b/drivers/video/via/accel.c
@@ -317,6 +317,7 @@ int viafb_init_engine(struct fb_info *info)
 {
 	struct viafb_par *viapar = info->par;
 	void __iomem *engine;
+	int highest_reg, i;
 	u32 vq_start_addr, vq_end_addr, vq_start_low, vq_end_low, vq_high,
 		vq_len, chip_name = viapar->shared->chip_info.gfx_chip_name;
 
@@ -328,6 +329,18 @@ int viafb_init_engine(struct fb_info *info)
 		return -ENOMEM;
 	}
 
+	/* Initialize registers to reset the 2D engine */
+	switch (viapar->shared->chip_info.twod_engine) {
+	case VIA_2D_ENG_M1:
+		highest_reg = 0x5c;
+		break;
+	default:
+		highest_reg = 0x40;
+		break;
+	}
+	for (i = 0; i <= highest_reg; i += 4)
+		writel(0x0, engine + i);
+
 	switch (chip_name) {
 	case UNICHROME_CLE266:
 	case UNICHROME_K400:
@@ -357,13 +370,12 @@ int viafb_init_engine(struct fb_info *info)
 	viapar->shared->vq_vram_addr = viapar->fbmem_free;
 	viapar->fbmem_used += VQ_SIZE;
 
-	/* Init 2D engine reg to reset 2D engine */
-	writel(0x0, engine + VIA_REG_KEYCONTROL);
-
 	/* Init AGP and VQ regs */
 	switch (chip_name) {
 	case UNICHROME_K8M890:
 	case UNICHROME_P4M900:
+	case UNICHROME_VX800:
+	case UNICHROME_VX855:
 		writel(0x00100000, engine + VIA_REG_CR_TRANSET);
 		writel(0x680A0000, engine + VIA_REG_CR_TRANSPACE);
 		writel(0x02000000, engine + VIA_REG_CR_TRANSPACE);
@@ -398,6 +410,8 @@ int viafb_init_engine(struct fb_info *info)
 	switch (chip_name) {
 	case UNICHROME_K8M890:
 	case UNICHROME_P4M900:
+	case UNICHROME_VX800:
+	case UNICHROME_VX855:
 		vq_start_low |= 0x20000000;
 		vq_end_low |= 0x20000000;
 		vq_high |= 0x20000000;
@@ -475,15 +489,25 @@ void viafb_wait_engine_idle(struct fb_info *info)
 {
 	struct viafb_par *viapar = info->par;
 	int loop = 0;
+	u32 mask;
 
-	while (!(readl(viapar->shared->engine_mmio + VIA_REG_STATUS) &
-			VIA_VR_QUEUE_BUSY) && (loop < MAXLOOP)) {
-		loop++;
-		cpu_relax();
+	switch (viapar->shared->chip_info.twod_engine) {
+	case VIA_2D_ENG_H5:
+	case VIA_2D_ENG_M1:
+		mask = VIA_CMD_RGTR_BUSY_M1 | VIA_2D_ENG_BUSY_M1 |
+			      VIA_3D_ENG_BUSY_M1;
+		break;
+	default:
+		while (!(readl(viapar->shared->engine_mmio + VIA_REG_STATUS) &
+				VIA_VR_QUEUE_BUSY) && (loop < MAXLOOP)) {
+			loop++;
+			cpu_relax();
+		}
+		mask = VIA_CMD_RGTR_BUSY | VIA_2D_ENG_BUSY | VIA_3D_ENG_BUSY;
+		break;
 	}
 
-	while ((readl(viapar->shared->engine_mmio + VIA_REG_STATUS) &
-		    (VIA_CMD_RGTR_BUSY | VIA_2D_ENG_BUSY | VIA_3D_ENG_BUSY)) &&
+	while ((readl(viapar->shared->engine_mmio + VIA_REG_STATUS) & mask) &&
 		    (loop < MAXLOOP)) {
 		loop++;
 		cpu_relax();
diff --git a/drivers/video/via/accel.h b/drivers/video/via/accel.h
index 615c84a..2c122d2 100644
--- a/drivers/video/via/accel.h
+++ b/drivers/video/via/accel.h
@@ -67,6 +67,34 @@
 /* from 0x100 to 0x1ff */
 #define VIA_REG_COLORPAT        0x100
 
+/* defines for VIA 2D registers for vt3353/3409 (M1 engine)*/
+#define VIA_REG_GECMD_M1        0x000
+#define VIA_REG_GEMODE_M1       0x004
+#define VIA_REG_GESTATUS_M1     0x004       /* as same as VIA_REG_GEMODE */
+#define VIA_REG_PITCH_M1        0x008       /* pitch of src and dst */
+#define VIA_REG_DIMENSION_M1    0x00C       /* width and height */
+#define VIA_REG_DSTPOS_M1       0x010
+#define VIA_REG_LINE_XY_M1      0x010
+#define VIA_REG_DSTBASE_M1      0x014
+#define VIA_REG_SRCPOS_M1       0x018
+#define VIA_REG_LINE_K1K2_M1    0x018
+#define VIA_REG_SRCBASE_M1      0x01C
+#define VIA_REG_PATADDR_M1      0x020
+#define VIA_REG_MONOPAT0_M1     0x024
+#define VIA_REG_MONOPAT1_M1     0x028
+#define VIA_REG_OFFSET_M1       0x02C
+#define VIA_REG_LINE_ERROR_M1   0x02C
+#define VIA_REG_CLIPTL_M1       0x040       /* top and left of clipping */
+#define VIA_REG_CLIPBR_M1       0x044       /* bottom and right of clipping */
+#define VIA_REG_KEYCONTROL_M1   0x048       /* color key control */
+#define VIA_REG_FGCOLOR_M1      0x04C
+#define VIA_REG_DSTCOLORKEY_M1  0x04C       /* as same as VIA_REG_FG */
+#define VIA_REG_BGCOLOR_M1      0x050
+#define VIA_REG_SRCCOLORKEY_M1  0x050       /* as same as VIA_REG_BG */
+#define VIA_REG_MONOPATFGC_M1   0x058       /* Add BG color of Pattern. */
+#define VIA_REG_MONOPATBGC_M1   0x05C       /* Add FG color of Pattern. */
+#define VIA_REG_COLORPAT_M1     0x100       /* from 0x100 to 0x1ff */
+
 /* VIA_REG_PITCH(0x38): Pitch Setting */
 #define VIA_PITCH_ENABLE        0x80000000
 
@@ -157,6 +185,18 @@
 /* Virtual Queue is busy */
 #define VIA_VR_QUEUE_BUSY       0x00020000
 
+/* VIA_REG_STATUS(0x400): Engine Status for H5 */
+#define VIA_CMD_RGTR_BUSY_H5   0x00000010  /* Command Regulator is busy */
+#define VIA_2D_ENG_BUSY_H5     0x00000002  /* 2D Engine is busy */
+#define VIA_3D_ENG_BUSY_H5     0x00001FE1  /* 3D Engine is busy */
+#define VIA_VR_QUEUE_BUSY_H5   0x00000004  /* Virtual Queue is busy */
+
+/* VIA_REG_STATUS(0x400): Engine Status for VT3353/3409 */
+#define VIA_CMD_RGTR_BUSY_M1   0x00000010  /* Command Regulator is busy */
+#define VIA_2D_ENG_BUSY_M1     0x00000002  /* 2D Engine is busy */
+#define VIA_3D_ENG_BUSY_M1     0x00001FE1  /* 3D Engine is busy */
+#define VIA_VR_QUEUE_BUSY_M1   0x00000004  /* Virtual Queue is busy */
+
 #define MAXLOOP                 0xFFFFFF
 
 #define VIA_BITBLT_COLOR	1
-- 
1.7.0.1

[PATCH 08/11] viafb: Add 1200x900 DCON/LCD panel modes for OLPC XO-1.5

[Jonathan Corbet]

From: Chris Ball <cjb@laptop.org>

[jc: extensive merge conflict fixes]
Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc: ScottFang@viatech.com.cn
Cc: JosephChan@via.com.tw
Signed-off-by: Chris Ball <cjb@laptop.org>
---
 drivers/video/via/hw.c      |    1 +
 drivers/video/via/ioctl.h   |    2 +-
 drivers/video/via/lcd.c     |    9 +++++++++
 drivers/video/via/lcd.h     |    2 ++
 drivers/video/via/share.h   |    7 +++++++
 drivers/video/via/viamode.c |   14 ++++++++++++++
 6 files changed, 34 insertions(+), 1 deletions(-)

diff --git a/drivers/video/via/hw.c b/drivers/video/via/hw.c
index d893695..4aff0f7 100644
--- a/drivers/video/via/hw.c
+++ b/drivers/video/via/hw.c
@@ -63,6 +63,7 @@ static struct pll_map pll_value[] = {
 	 CX700_52_977M,	VX855_52_977M},
 	{CLK_56_250M, CLE266_PLL_56_250M, K800_PLL_56_250M,
 	 CX700_56_250M, VX855_56_250M},
+	{CLK_57_275M, 0, 0, 0, VX855_57_275M},
 	{CLK_60_466M, CLE266_PLL_60_466M, K800_PLL_60_466M,
 	 CX700_60_466M, VX855_60_466M},
 	{CLK_61_500M, CLE266_PLL_61_500M, K800_PLL_61_500M,
diff --git a/drivers/video/via/ioctl.h b/drivers/video/via/ioctl.h
index de89980..c430fa2 100644
--- a/drivers/video/via/ioctl.h
+++ b/drivers/video/via/ioctl.h
@@ -75,7 +75,7 @@
 /*SAMM operation flag*/
 #define OP_SAMM            0x80
 
-#define LCD_PANEL_ID_MAXIMUM	22
+#define LCD_PANEL_ID_MAXIMUM	23
 
 #define STATE_ON            0x1
 #define STATE_OFF           0x0
diff --git a/drivers/video/via/lcd.c b/drivers/video/via/lcd.c
index 79c8c9d..b656e59 100644
--- a/drivers/video/via/lcd.c
+++ b/drivers/video/via/lcd.c
@@ -399,6 +399,15 @@ static void fp_id_to_vindex(int panel_id)
 		viaparinfo->lvds_setting_info->device_lcd_dualedge = 0;
 		viaparinfo->lvds_setting_info->LCDDithering = 1;
 		break;
+	case 0x17:
+		/* OLPC XO-1.5 panel */
+		viaparinfo->lvds_setting_info->lcd_panel_hres = 1200;
+		viaparinfo->lvds_setting_info->lcd_panel_vres = 900;
+		viaparinfo->lvds_setting_info->lcd_panel_id =
+			LCD_PANEL_IDD_1200X900;
+		viaparinfo->lvds_setting_info->device_lcd_dualedge = 0;
+		viaparinfo->lvds_setting_info->LCDDithering = 0;
+		break;
 	default:
 		viaparinfo->lvds_setting_info->lcd_panel_hres = 800;
 		viaparinfo->lvds_setting_info->lcd_panel_vres = 600;
diff --git a/drivers/video/via/lcd.h b/drivers/video/via/lcd.h
index 071f47c..9762ec6 100644
--- a/drivers/video/via/lcd.h
+++ b/drivers/video/via/lcd.h
@@ -60,6 +60,8 @@
 #define     LCD_PANEL_IDB_1360X768     0x0B
 /* Resolution: 480x640,  Channel: single, Dithering: Enable */
 #define     LCD_PANEL_IDC_480X640      0x0C
+/* Resolution: 1200x900,  Channel: single, Dithering: Disable */
+#define     LCD_PANEL_IDD_1200X900      0x0D
 
 
 extern int viafb_LCD2_ON;
diff --git a/drivers/video/via/share.h b/drivers/video/via/share.h
index d55aaa7..f974c73 100644
--- a/drivers/video/via/share.h
+++ b/drivers/video/via/share.h
@@ -570,6 +570,10 @@
 #define M1200X720_R60_HSP       NEGATIVE
 #define M1200X720_R60_VSP       POSITIVE
 
+/* 1200x900@60 Sync Polarity (DCON) */
+#define M1200X900_R60_HSP       NEGATIVE
+#define M1200X900_R60_VSP       NEGATIVE
+
 /* 1280x600@60 Sync Polarity (GTF Mode) */
 #define M1280x600_R60_HSP       NEGATIVE
 #define M1280x600_R60_VSP       POSITIVE
@@ -651,6 +655,7 @@
 #define CLK_52_406M     52406000
 #define CLK_52_977M     52977000
 #define CLK_56_250M     56250000
+#define CLK_57_275M     57275000
 #define CLK_60_466M     60466000
 #define CLK_61_500M     61500000
 #define CLK_65_000M     65000000
@@ -939,6 +944,7 @@
 #define VX855_52_406M     0x00580C03
 #define VX855_52_977M     0x00940C05
 #define VX855_56_250M     0x009D0C05
+#define VX855_57_275M     0x009D8C85    /* Used by XO panel */
 #define VX855_60_466M     0x00A90C05
 #define VX855_61_500M     0x00AC0C05
 #define VX855_65_000M     0x006D0C03
@@ -1065,6 +1071,7 @@
 #define RES_1600X1200_60HZ_PIXCLOCK  6172
 #define RES_1600X1200_75HZ_PIXCLOCK  4938
 #define RES_1280X720_60HZ_PIXCLOCK   13426
+#define RES_1200X900_60HZ_PIXCLOCK   17459
 #define RES_1920X1080_60HZ_PIXCLOCK  5787
 #define RES_1400X1050_60HZ_PIXCLOCK  8214
 #define RES_1400X1050_75HZ_PIXCLOCK  6410
diff --git a/drivers/video/via/viamode.c b/drivers/video/via/viamode.c
index af50e24..6f3bcda 100644
--- a/drivers/video/via/viamode.c
+++ b/drivers/video/via/viamode.c
@@ -66,6 +66,7 @@ struct res_map_refresh res_map_refresh_tbl[] = {
 	{1088, 612, RES_1088X612_60HZ_PIXCLOCK, 60},
 	{1152, 720, RES_1152X720_60HZ_PIXCLOCK, 60},
 	{1200, 720, RES_1200X720_60HZ_PIXCLOCK, 60},
+	{1200, 900, RES_1200X900_60HZ_PIXCLOCK, 60},
 	{1280, 600, RES_1280X600_60HZ_PIXCLOCK, 60},
 	{1280, 720, RES_1280X720_50HZ_PIXCLOCK, 50},
 	{1280, 768, RES_1280X768_50HZ_PIXCLOCK, 50},
@@ -759,6 +760,16 @@ struct crt_mode_table CRTM1200x720[] = {
 	 {1568, 1200, 1200, 368, 1256, 128, 746, 720, 720, 26, 721, 3} }
 };
 
+/* 1200x900 (DCON) */
+struct crt_mode_table DCON1200x900[] = {
+	/* r_rate,          vclk,               hsp,               vsp   */
+	{REFRESH_60, CLK_57_275M, M1200X900_R60_HSP, M1200X900_R60_VSP,
+	/* The correct htotal is 1240, but this doesn't raster on VX855. */
+	/* Via suggested changing to a multiple of 16, hence 1264.       */
+	/*  HT,   HA,  HBS, HBE,  HSS, HSE,  VT,  VA, VBS, VBE, VSS, VSE */
+	 {1264, 1200, 1200,  64, 1211,  32, 912, 900, 900,  12, 901, 10} }
+};
+
 /* 1280x600 (GTF) */
 struct crt_mode_table CRTM1280x600[] = {
 	/* r_rate,          vclk,              hsp,             vsp   */
@@ -937,6 +948,9 @@ struct VideoModeTable viafb_modes[] = {
 	/* Display : 1200x720 (GTF) */
 	{CRTM1200x720, ARRAY_SIZE(CRTM1200x720)},
 
+	/* Display : 1200x900 (DCON) */
+	{DCON1200x900, ARRAY_SIZE(DCON1200x900)},
+
 	/* Display : 1280x600 (GTF) */
 	{CRTM1280x600, ARRAY_SIZE(CRTM1280x600)},
 
-- 
1.7.0.1

[PATCH 09/11] viafb: Do not probe for LVDS/TMDS on OLPC XO-1.5

[Jonathan Corbet]

From: Chris Ball <cjb@laptop.org>

The i2c transactions involved in detecting LVDS (9 seconds) and TMDS
(16 seconds) add an extra 25 seconds to viafb load time on the XO-1.5.

[jc: minor merge conflict fixed, removed ifdefs]
Signed-off-by: Chris Ball <cjb@laptop.org>
---
 drivers/video/via/hw.c  |    7 +++++++
 drivers/video/via/lcd.c |    7 +++++++
 2 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/drivers/video/via/hw.c b/drivers/video/via/hw.c
index 4aff0f7..bd5d5ce 100644
--- a/drivers/video/via/hw.c
+++ b/drivers/video/via/hw.c
@@ -2037,6 +2037,13 @@ static void init_gfx_chip_info(struct pci_dev *pdev,
 
 static void init_tmds_chip_info(void)
 {
+	/*
+	 * OLPC XO 1.5 systems are wired differently, so there is
+	 * no point in them here.
+	 */
+	if (machine_is_olpc())
+		return;
+
 	viafb_tmds_trasmitter_identify();
 
 	if (INTERFACE_NONE == viaparinfo->chip_info->tmds_chip_info.
diff --git a/drivers/video/via/lcd.c b/drivers/video/via/lcd.c
index b656e59..ce3f4b6 100644
--- a/drivers/video/via/lcd.c
+++ b/drivers/video/via/lcd.c
@@ -173,6 +173,13 @@ static bool lvds_identify_integratedlvds(void)
 
 int viafb_lvds_trasmitter_identify(void)
 {
+	/*
+	 * OLPC XO 1.5 systems are wired differently, so there is
+	 * no point in them here.
+	 */
+	if (machine_is_olpc())
+		return FAIL;
+
 	viaparinfo->shared->i2c_stuff.i2c_port = I2CPORTINDEX;
 	if (viafb_lvds_identify_vt1636()) {
 		viaparinfo->chip_info->lvds_chip_info.i2c_port = I2CPORTINDEX;
-- 
1.7.0.1

Re: [PATCH 09/11] viafb: Do not probe for LVDS/TMDS on OLPC XO-1.5

[Florian Tobias Schandinat]

Hi Jon,

I fear your analysis that the #ifdef's are not needed is wrong. At least 
with this patch applied I get:

drivers/video/via/hw.c: In function 'init_tmds_chip_info':
drivers/video/via/hw.c:2043: error: implicit declaration of function 
'machine_is_olpc'

drivers/video/via/lcd.c: In function 'viafb_lvds_trasmitter_identify':
drivers/video/via/lcd.c:179: error: implicit declaration of function 
'machine_is_olpc'

So I vote for taking the original patch but in the series that also adds 
the relevant config option.


Thanks,

Florian Tobias Schandinat

Jonathan Corbet schrieb:
> From: Chris Ball <cjb@laptop.org>
> 
> The i2c transactions involved in detecting LVDS (9 seconds) and TMDS
> (16 seconds) add an extra 25 seconds to viafb load time on the XO-1.5.
> 
> [jc: minor merge conflict fixed, removed ifdefs]
> Signed-off-by: Chris Ball <cjb@laptop.org>
> ---
>  drivers/video/via/hw.c  |    7 +++++++
>  drivers/video/via/lcd.c |    7 +++++++
>  2 files changed, 14 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/video/via/hw.c b/drivers/video/via/hw.c
> index 4aff0f7..bd5d5ce 100644
> --- a/drivers/video/via/hw.c
> +++ b/drivers/video/via/hw.c
> @@ -2037,6 +2037,13 @@ static void init_gfx_chip_info(struct pci_dev *pdev,
>  
>  static void init_tmds_chip_info(void)
>  {
> +	/*
> +	 * OLPC XO 1.5 systems are wired differently, so there is
> +	 * no point in them here.
> +	 */
> +	if (machine_is_olpc())
> +		return;
> +
>  	viafb_tmds_trasmitter_identify();
>  
>  	if (INTERFACE_NONE == viaparinfo->chip_info->tmds_chip_info.
> diff --git a/drivers/video/via/lcd.c b/drivers/video/via/lcd.c
> index b656e59..ce3f4b6 100644
> --- a/drivers/video/via/lcd.c
> +++ b/drivers/video/via/lcd.c
> @@ -173,6 +173,13 @@ static bool lvds_identify_integratedlvds(void)
>  
>  int viafb_lvds_trasmitter_identify(void)
>  {
> +	/*
> +	 * OLPC XO 1.5 systems are wired differently, so there is
> +	 * no point in them here.
> +	 */
> +	if (machine_is_olpc())
> +		return FAIL;
> +
>  	viaparinfo->shared->i2c_stuff.i2c_port = I2CPORTINDEX;
>  	if (viafb_lvds_identify_vt1636()) {
>  		viaparinfo->chip_info->lvds_chip_info.i2c_port = I2CPORTINDEX;

Re: [PATCH 09/11] viafb: Do not probe for LVDS/TMDS on OLPC XO-1.5

[Jonathan Corbet]

On Fri, 23 Apr 2010 22:56:46 +0200
Florian Tobias Schandinat <FlorianSchandinat@gmx.de> wrote:

> I fear your analysis that the #ifdef's are not needed is wrong. At least 
> with this patch applied I get:
> 
> drivers/video/via/hw.c: In function 'init_tmds_chip_info':
> drivers/video/via/hw.c:2043: error: implicit declaration of function 
> 'machine_is_olpc'

No, the problem is the "embarrassing compilation error" I mentioned in
my other mail; it needs "#include <asm/olpc.h>" at the top of the file.

Sorry,

jon

[PATCH 10/11] viafb: rework the I2C support in the VIA framebuffer driver

[Jonathan Corbet]

From: Harald Welte <laforge@gnumonks.org>

This patch changes the way how the various I2C busses are used internally
inside the viafb driver:  Previosuly, only a single i2c_adapter was created,
even though two different hardware I2C busses are accessed: A structure member
in a global variable was modified to indicate the bus to be used.

Now, all existing hardware busses are registered with the i2c core, and the
viafb_i2c_{read,write}byte[s]() function take the adapter number as function
call parameter, rather than referring to the global structure member.

[jc: even more painful merge with mainline changes ->2.6.34]
[jc: painful merge with OLPC changes]

Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc: ScottFang@viatech.com.cn
Cc: JosephChan@via.com.tw
Signed-off-by: Harald Welte <HaraldWelte@viatech.com>
Signed-off-by: Jonathan Corbet <corbet@lwn.net>
---
 drivers/video/via/dvi.c      |   35 ++++-----
 drivers/video/via/lcd.c      |   19 ++---
 drivers/video/via/via_i2c.c  |  171 ++++++++++++++++++++++++++----------------
 drivers/video/via/via_i2c.h  |   43 +++++++----
 drivers/video/via/viafbdev.c |    6 +-
 drivers/video/via/viafbdev.h |    4 +-
 drivers/video/via/vt1636.c   |   36 ++++-----
 drivers/video/via/vt1636.h   |    2 +-
 8 files changed, 182 insertions(+), 134 deletions(-)

diff --git a/drivers/video/via/dvi.c b/drivers/video/via/dvi.c
index abe59b8..be51370 100644
--- a/drivers/video/via/dvi.c
+++ b/drivers/video/via/dvi.c
@@ -96,7 +96,7 @@ int viafb_tmds_trasmitter_identify(void)
 	viaparinfo->chip_info->tmds_chip_info.tmds_chip_name = VT1632_TMDS;
 	viaparinfo->chip_info->
 		tmds_chip_info.tmds_chip_slave_addr = VT1632_TMDS_I2C_ADDR;
-	viaparinfo->chip_info->tmds_chip_info.i2c_port = I2CPORTINDEX;
+	viaparinfo->chip_info->tmds_chip_info.i2c_port = VIA_I2C_ADAP_31;
 	if (check_tmds_chip(VT1632_DEVICE_ID_REG, VT1632_DEVICE_ID) != FAIL) {
 		/*
 		 * Currently only support 12bits,dual edge,add 24bits mode later
@@ -110,7 +110,7 @@ int viafb_tmds_trasmitter_identify(void)
 			  viaparinfo->chip_info->tmds_chip_info.i2c_port);
 		return OK;
 	} else {
-		viaparinfo->chip_info->tmds_chip_info.i2c_port = GPIOPORTINDEX;
+		viaparinfo->chip_info->tmds_chip_info.i2c_port = VIA_I2C_ADAP_2C;
 		if (check_tmds_chip(VT1632_DEVICE_ID_REG, VT1632_DEVICE_ID)
 		    != FAIL) {
 			tmds_register_write(0x08, 0x3b);
@@ -160,32 +160,26 @@ int viafb_tmds_trasmitter_identify(void)
 
 static void tmds_register_write(int index, u8 data)
 {
-	viaparinfo->shared->i2c_stuff.i2c_port =
-		viaparinfo->chip_info->tmds_chip_info.i2c_port;
-
-	viafb_i2c_writebyte(viaparinfo->chip_info->tmds_chip_info.
-		tmds_chip_slave_addr, index,
-		     data);
+	viafb_i2c_writebyte(viaparinfo->chip_info->tmds_chip_info.i2c_port,
+			    viaparinfo->chip_info->tmds_chip_info.tmds_chip_slave_addr,
+			    index, data);
 }
 
 static int tmds_register_read(int index)
 {
 	u8 data;
 
-	viaparinfo->shared->i2c_stuff.i2c_port =
-		viaparinfo->chip_info->tmds_chip_info.i2c_port;
-	viafb_i2c_readbyte((u8) viaparinfo->chip_info->
-	    tmds_chip_info.tmds_chip_slave_addr,
-			(u8) index, &data);
+	viafb_i2c_readbyte(viaparinfo->chip_info->tmds_chip_info.i2c_port,
+			   (u8) viaparinfo->chip_info->tmds_chip_info.tmds_chip_slave_addr,
+			   (u8) index, &data);
 	return data;
 }
 
 static int tmds_register_read_bytes(int index, u8 *buff, int buff_len)
 {
-	viaparinfo->shared->i2c_stuff.i2c_port =
-		viaparinfo->chip_info->tmds_chip_info.i2c_port;
-	viafb_i2c_readbytes((u8) viaparinfo->chip_info->tmds_chip_info.
-			 tmds_chip_slave_addr, (u8) index, buff, buff_len);
+	viafb_i2c_readbytes(viaparinfo->chip_info->tmds_chip_info.i2c_port,
+			    (u8) viaparinfo->chip_info->tmds_chip_info.tmds_chip_slave_addr,
+			    (u8) index, buff, buff_len);
 	return 0;
 }
 
@@ -541,9 +535,10 @@ void viafb_dvi_enable(void)
 				else
 					data = 0x37;
 				viafb_i2c_writebyte(viaparinfo->chip_info->
-					     tmds_chip_info.
-					     tmds_chip_slave_addr,
-					     0x08, data);
+						       tmds_chip_info.i2c_port,
+						    viaparinfo->chip_info->
+						       tmds_chip_info.tmds_chip_slave_addr,
+						    0x08, data);
 			}
 		}
 	}
diff --git a/drivers/video/via/lcd.c b/drivers/video/via/lcd.c
index ce3f4b6..98ae1c7 100644
--- a/drivers/video/via/lcd.c
+++ b/drivers/video/via/lcd.c
@@ -180,18 +180,16 @@ int viafb_lvds_trasmitter_identify(void)
 	if (machine_is_olpc())
 		return FAIL;
 
-	viaparinfo->shared->i2c_stuff.i2c_port = I2CPORTINDEX;
-	if (viafb_lvds_identify_vt1636()) {
-		viaparinfo->chip_info->lvds_chip_info.i2c_port = I2CPORTINDEX;
+	if (viafb_lvds_identify_vt1636(VIA_I2C_ADAP_31)) {
+		viaparinfo->chip_info->lvds_chip_info.i2c_port = VIA_I2C_ADAP_31;
 		DEBUG_MSG(KERN_INFO
-			  "Found VIA VT1636 LVDS on port i2c 0x31 \n");
+			  "Found VIA VT1636 LVDS on port i2c 0x31\n");
 	} else {
-		viaparinfo->shared->i2c_stuff.i2c_port = GPIOPORTINDEX;
-		if (viafb_lvds_identify_vt1636()) {
+		if (viafb_lvds_identify_vt1636(VIA_I2C_ADAP_26)) {
 			viaparinfo->chip_info->lvds_chip_info.i2c_port =
-				GPIOPORTINDEX;
+				VIA_I2C_ADAP_2C;
 			DEBUG_MSG(KERN_INFO
-				  "Found VIA VT1636 LVDS on port gpio 0x2c \n");
+				  "Found VIA VT1636 LVDS on port gpio 0x2c\n");
 		}
 	}
 
@@ -429,9 +427,8 @@ static int lvds_register_read(int index)
 {
 	u8 data;
 
-	viaparinfo->shared->i2c_stuff.i2c_port = GPIOPORTINDEX;
-	viafb_i2c_readbyte((u8) viaparinfo->chip_info->
-	    lvds_chip_info.lvds_chip_slave_addr,
+	viafb_i2c_readbyte(VIA_I2C_ADAP_2C,
+			(u8) viaparinfo->chip_info->lvds_chip_info.lvds_chip_slave_addr,
 			(u8) index, &data);
 	return data;
 }
diff --git a/drivers/video/via/via_i2c.c b/drivers/video/via/via_i2c.c
index 15543e9..8f8e0bf 100644
--- a/drivers/video/via/via_i2c.c
+++ b/drivers/video/via/via_i2c.c
@@ -1,5 +1,5 @@
 /*
- * Copyright 1998-2008 VIA Technologies, Inc. All Rights Reserved.
+ * Copyright 1998-2009 VIA Technologies, Inc. All Rights Reserved.
  * Copyright 2001-2008 S3 Graphics, Inc. All Rights Reserved.
 
  * This program is free software; you can redistribute it and/or
@@ -24,40 +24,44 @@
 static void via_i2c_setscl(void *data, int state)
 {
 	u8 val;
-	struct via_i2c_stuff *via_i2c_chan = (struct via_i2c_stuff *)data;
+	struct via_i2c_adap_cfg *adap_data = data;
 
-	val = viafb_read_reg(VIASR, via_i2c_chan->i2c_port) & 0xF0;
+	printk(KERN_DEBUG "reading index 0x%02x from IO 0x%x\n",
+		adap_data->ioport_index, adap_data->io_port);
+	val = viafb_read_reg(adap_data->io_port,
+			     adap_data->ioport_index) & 0xF0;
 	if (state)
 		val |= 0x20;
 	else
 		val &= ~0x20;
-	switch (via_i2c_chan->i2c_port) {
-	case I2CPORTINDEX:
+	switch (adap_data->type) {
+	case VIA_I2C_I2C:
 		val |= 0x01;
 		break;
-	case GPIOPORTINDEX:
+	case VIA_I2C_GPIO:
 		val |= 0x80;
 		break;
 	default:
-		DEBUG_MSG("via_i2c: specify wrong i2c port.\n");
+		DEBUG_MSG("viafb_i2c: specify wrong i2c type.\n");
 	}
-	viafb_write_reg(via_i2c_chan->i2c_port, VIASR, val);
+	viafb_write_reg(adap_data->ioport_index,
+			adap_data->io_port, val);
 }
 
 static int via_i2c_getscl(void *data)
 {
-	struct via_i2c_stuff *via_i2c_chan = (struct via_i2c_stuff *)data;
+	struct via_i2c_adap_cfg *adap_data = data;
 
-	if (viafb_read_reg(VIASR, via_i2c_chan->i2c_port) & 0x08)
+	if (viafb_read_reg(adap_data->io_port, adap_data->ioport_index) & 0x08)
 		return 1;
 	return 0;
 }
 
 static int via_i2c_getsda(void *data)
 {
-	struct via_i2c_stuff *via_i2c_chan = (struct via_i2c_stuff *)data;
+	struct via_i2c_adap_cfg *adap_data = data;
 
-	if (viafb_read_reg(VIASR, via_i2c_chan->i2c_port) & 0x04)
+	if (viafb_read_reg(adap_data->io_port, adap_data->ioport_index) & 0x04)
 		return 1;
 	return 0;
 }
@@ -65,27 +69,29 @@ static int via_i2c_getsda(void *data)
 static void via_i2c_setsda(void *data, int state)
 {
 	u8 val;
-	struct via_i2c_stuff *via_i2c_chan = (struct via_i2c_stuff *)data;
+	struct via_i2c_adap_cfg *adap_data = data;
 
-	val = viafb_read_reg(VIASR, via_i2c_chan->i2c_port) & 0xF0;
+	val = viafb_read_reg(adap_data->io_port,
+			     adap_data->ioport_index) & 0xF0;
 	if (state)
 		val |= 0x10;
 	else
 		val &= ~0x10;
-	switch (via_i2c_chan->i2c_port) {
-	case I2CPORTINDEX:
+	switch (adap_data->type) {
+	case VIA_I2C_I2C:
 		val |= 0x01;
 		break;
-	case GPIOPORTINDEX:
+	case VIA_I2C_GPIO:
 		val |= 0x40;
 		break;
 	default:
-		DEBUG_MSG("via_i2c: specify wrong i2c port.\n");
+		DEBUG_MSG("viafb_i2c: specify wrong i2c type.\n");
 	}
-	viafb_write_reg(via_i2c_chan->i2c_port, VIASR, val);
+	viafb_write_reg(adap_data->ioport_index,
+			adap_data->io_port, val);
 }
 
-int viafb_i2c_readbyte(u8 slave_addr, u8 index, u8 *pdata)
+int viafb_i2c_readbyte(u8 adap, u8 slave_addr, u8 index, u8 *pdata)
 {
 	u8 mm1[] = {0x00};
 	struct i2c_msg msgs[2];
@@ -97,12 +103,11 @@ int viafb_i2c_readbyte(u8 slave_addr, u8 index, u8 *pdata)
 	mm1[0] = index;
 	msgs[0].len = 1; msgs[1].len = 1;
 	msgs[0].buf = mm1; msgs[1].buf = pdata;
-	i2c_transfer(&viaparinfo->shared->i2c_stuff.adapter, msgs, 2);
-
-	return 0;
+	return i2c_transfer(&viaparinfo->shared->i2c_stuff[adap].adapter,
+			msgs, 2);
 }
 
-int viafb_i2c_writebyte(u8 slave_addr, u8 index, u8 data)
+int viafb_i2c_writebyte(u8 adap, u8 slave_addr, u8 index, u8 data)
 {
 	u8 msg[2] = { index, data };
 	struct i2c_msg msgs;
@@ -111,10 +116,11 @@ int viafb_i2c_writebyte(u8 slave_addr, u8 index, u8 data)
 	msgs.addr = slave_addr / 2;
 	msgs.len = 2;
 	msgs.buf = msg;
-	return i2c_transfer(&viaparinfo->shared->i2c_stuff.adapter, &msgs, 1);
+	return i2c_transfer(&viaparinfo->shared->i2c_stuff[adap].adapter,
+			&msgs, 1);
 }
 
-int viafb_i2c_readbytes(u8 slave_addr, u8 index, u8 *buff, int buff_len)
+int viafb_i2c_readbytes(u8 adap, u8 slave_addr, u8 index, u8 *buff, int buff_len)
 {
 	u8 mm1[] = {0x00};
 	struct i2c_msg msgs[2];
@@ -125,53 +131,88 @@ int viafb_i2c_readbytes(u8 slave_addr, u8 index, u8 *buff, int buff_len)
 	mm1[0] = index;
 	msgs[0].len = 1; msgs[1].len = buff_len;
 	msgs[0].buf = mm1; msgs[1].buf = buff;
-	i2c_transfer(&viaparinfo->shared->i2c_stuff.adapter, msgs, 2);
-	return 0;
+	return i2c_transfer(&viaparinfo->shared->i2c_stuff[adap].adapter,
+			msgs, 2);
 }
 
-int viafb_create_i2c_bus(void *viapar)
+static int create_i2c_bus(struct i2c_adapter *adapter,
+			  struct i2c_algo_bit_data *algo,
+			  struct via_i2c_adap_cfg *adap_cfg,
+			  struct pci_dev *pdev)
 {
-	int ret;
-	struct via_i2c_stuff *i2c_stuff =
-		&((struct viafb_par *)viapar)->shared->i2c_stuff;
-
-	strcpy(i2c_stuff->adapter.name, "via_i2c");
-	i2c_stuff->i2c_port = 0x0;
-	i2c_stuff->adapter.owner = THIS_MODULE;
-	i2c_stuff->adapter.id = 0x01FFFF;
-	i2c_stuff->adapter.class = 0;
-	i2c_stuff->adapter.algo_data = &i2c_stuff->algo;
-	i2c_stuff->adapter.dev.parent = NULL;
-	i2c_stuff->algo.setsda = via_i2c_setsda;
-	i2c_stuff->algo.setscl = via_i2c_setscl;
-	i2c_stuff->algo.getsda = via_i2c_getsda;
-	i2c_stuff->algo.getscl = via_i2c_getscl;
-	i2c_stuff->algo.udelay = 40;
-	i2c_stuff->algo.timeout = 20;
-	i2c_stuff->algo.data = i2c_stuff;
-
-	i2c_set_adapdata(&i2c_stuff->adapter, i2c_stuff);
+	printk(KERN_DEBUG "viafb: creating bus adap=0x%p, algo_bit_data=0x%p, adap_cfg=0x%p\n", adapter, algo, adap_cfg);
+
+	algo->setsda = via_i2c_setsda;
+	algo->setscl = via_i2c_setscl;
+	algo->getsda = via_i2c_getsda;
+	algo->getscl = via_i2c_getscl;
+	algo->udelay = 40;
+	algo->timeout = 20;
+	algo->data = adap_cfg;
+
+	sprintf(adapter->name, "viafb i2c io_port idx 0x%02x",
+		adap_cfg->ioport_index);
+	adapter->owner = THIS_MODULE;
+	adapter->id = 0x01FFFF;
+	adapter->class = I2C_CLASS_DDC;
+	adapter->algo_data = algo;
+	if (pdev)
+		adapter->dev.parent = &pdev->dev;
+	else
+		adapter->dev.parent = NULL;
+	/* i2c_set_adapdata(adapter, adap_cfg); */
 
 	/* Raise SCL and SDA */
-	i2c_stuff->i2c_port = I2CPORTINDEX;
-	via_i2c_setsda(i2c_stuff, 1);
-	via_i2c_setscl(i2c_stuff, 1);
-
-	i2c_stuff->i2c_port = GPIOPORTINDEX;
-	via_i2c_setsda(i2c_stuff, 1);
-	via_i2c_setscl(i2c_stuff, 1);
+	via_i2c_setsda(adap_cfg, 1);
+	via_i2c_setscl(adap_cfg, 1);
 	udelay(20);
 
-	ret = i2c_bit_add_bus(&i2c_stuff->adapter);
-	if (ret == 0)
-		DEBUG_MSG("I2C bus %s registered.\n", i2c_stuff->adapter.name);
-	else
-		DEBUG_MSG("Failed to register I2C bus %s.\n",
-			i2c_stuff->adapter.name);
-	return ret;
+	return i2c_bit_add_bus(adapter);
+}
+
+static struct via_i2c_adap_cfg adap_configs[] = {
+	[VIA_I2C_ADAP_26]	= { VIA_I2C_I2C,  VIASR, 0x26 },
+	[VIA_I2C_ADAP_31]	= { VIA_I2C_I2C,  VIASR, 0x31 },
+	[VIA_I2C_ADAP_25]	= { VIA_I2C_GPIO, VIASR, 0x25 },
+	[VIA_I2C_ADAP_2C]	= { VIA_I2C_GPIO, VIASR, 0x2c },
+	[VIA_I2C_ADAP_3D]	= { VIA_I2C_GPIO, VIASR, 0x3d },
+	{ 0, 0, 0 }
+};
+
+int viafb_create_i2c_busses(struct viafb_par *viapar)
+{
+	int i, ret;
+
+	for (i = 0; i < VIAFB_NUM_I2C; i++) {
+		struct via_i2c_adap_cfg *adap_cfg = &adap_configs[i];
+		struct via_i2c_stuff *i2c_stuff = &viapar->shared->i2c_stuff[i];
+
+		if (adap_cfg->type == 0)
+			break;
+
+		ret = create_i2c_bus(&i2c_stuff->adapter,
+				     &i2c_stuff->algo, adap_cfg,
+				NULL); /* FIXME: PCIDEV */
+		if (ret < 0) {
+			printk(KERN_ERR "viafb: cannot create i2c bus %u:%d\n",
+				i, ret);
+			/* FIXME: properly release previous busses */
+			return ret;
+		}
+	}
+
+	return 0;
 }
 
-void viafb_delete_i2c_buss(void *par)
+void viafb_delete_i2c_busses(struct viafb_par *par)
 {
-	i2c_del_adapter(&((struct viafb_par *)par)->shared->i2c_stuff.adapter);
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(par->shared->i2c_stuff); i++) {
+		struct via_i2c_stuff *i2c_stuff = &par->shared->i2c_stuff[i];
+		/* only remove those entries in the array that we've
+		 * actually used (and thus initialized algo_data) */
+		if (i2c_stuff->adapter.algo_data == &i2c_stuff->algo)
+			i2c_del_adapter(&i2c_stuff->adapter);
+	}
 }
diff --git a/drivers/video/via/via_i2c.h b/drivers/video/via/via_i2c.h
index 3a13242..00ed978 100644
--- a/drivers/video/via/via_i2c.h
+++ b/drivers/video/via/via_i2c.h
@@ -1,5 +1,5 @@
 /*
- * Copyright 1998-2008 VIA Technologies, Inc. All Rights Reserved.
+ * Copyright 1998-2009 VIA Technologies, Inc. All Rights Reserved.
  * Copyright 2001-2008 S3 Graphics, Inc. All Rights Reserved.
 
  * This program is free software; you can redistribute it and/or
@@ -24,23 +24,38 @@
 #include <linux/i2c.h>
 #include <linux/i2c-algo-bit.h>
 
+enum via_i2c_type {
+	VIA_I2C_NONE,
+	VIA_I2C_I2C,
+	VIA_I2C_GPIO,
+};
+
+/* private data for each adapter */
+struct via_i2c_adap_cfg {
+	enum via_i2c_type	type;
+	u_int16_t		io_port;
+	u_int8_t		ioport_index;
+};
+
 struct via_i2c_stuff {
 	u16 i2c_port;			/* GPIO or I2C port */
 	struct i2c_adapter adapter;
 	struct i2c_algo_bit_data algo;
 };
 
-#define I2CPORT           0x3c4
-#define I2CPORTINDEX      0x31
-#define GPIOPORT          0x3C4
-#define GPIOPORTINDEX     0x2C
-#define I2C_BUS             1
-#define GPIO_BUS            2
-#define DELAYPORT           0x3C3
-
-int viafb_i2c_readbyte(u8 slave_addr, u8 index, u8 *pdata);
-int viafb_i2c_writebyte(u8 slave_addr, u8 index, u8 data);
-int viafb_i2c_readbytes(u8 slave_addr, u8 index, u8 *buff, int buff_len);
-int viafb_create_i2c_bus(void *par);
-void viafb_delete_i2c_buss(void *par);
+enum viafb_i2c_adap {
+	VIA_I2C_ADAP_26,
+	VIA_I2C_ADAP_31,
+	VIA_I2C_ADAP_25,
+	VIA_I2C_ADAP_2C,
+	VIA_I2C_ADAP_3D,
+};
+
+int viafb_i2c_readbyte(u8 adap, u8 slave_addr, u8 index, u8 *pdata);
+int viafb_i2c_writebyte(u8 adap, u8 slave_addr, u8 index, u8 data);
+int viafb_i2c_readbytes(u8 adap, u8 slave_addr, u8 index, u8 *buff, int buff_len);
+
+struct viafb_par;
+int viafb_create_i2c_busses(struct viafb_par *par);
+void viafb_delete_i2c_busses(struct viafb_par *par);
 #endif /* __VIA_I2C_H__ */
diff --git a/drivers/video/via/viafbdev.c b/drivers/video/via/viafbdev.c
index 8955ab4..fa10049 100644
--- a/drivers/video/via/viafbdev.c
+++ b/drivers/video/via/viafbdev.c
@@ -1775,7 +1775,7 @@ static int __devinit via_pci_probe(struct pci_dev *pdev,
 		viafb_dual_fb = 0;
 
 	/* Set up I2C bus stuff */
-	rc = viafb_create_i2c_bus(viaparinfo);
+	rc = viafb_create_i2c_busses(viaparinfo);
 	if (rc)
 		goto out_fb_release;
 
@@ -1964,7 +1964,7 @@ out_fb1_release:
 out_unmap_screen:
 	iounmap(viafbinfo->screen_base);
 out_delete_i2c:
-	viafb_delete_i2c_buss(viaparinfo);
+	viafb_delete_i2c_busses(viaparinfo);
 out_fb_release:
 	framebuffer_release(viafbinfo);
 	return rc;
@@ -1980,7 +1980,7 @@ static void __devexit via_pci_remove(struct pci_dev *pdev)
 	iounmap((void *)viafbinfo->screen_base);
 	iounmap(viaparinfo->shared->engine_mmio);
 
-	viafb_delete_i2c_buss(viaparinfo);
+	viafb_delete_i2c_busses(viaparinfo);
 
 	framebuffer_release(viafbinfo);
 	if (viafb_dual_fb)
diff --git a/drivers/video/via/viafbdev.h b/drivers/video/via/viafbdev.h
index 61b5953..4bc00ec 100644
--- a/drivers/video/via/viafbdev.h
+++ b/drivers/video/via/viafbdev.h
@@ -37,11 +37,13 @@
 #define VERSION_OS          0	/* 0: for 32 bits OS, 1: for 64 bits OS */
 #define VERSION_MINOR       4
 
+#define VIAFB_NUM_I2C		5
+
 struct viafb_shared {
 	struct proc_dir_entry *proc_entry;	/*viafb proc entry */
 
 	/* I2C stuff */
-	struct via_i2c_stuff i2c_stuff;
+	struct via_i2c_stuff i2c_stuff[VIAFB_NUM_I2C];
 
 	/* All the information will be needed to set engine */
 	struct tmds_setting_information tmds_setting_info;
diff --git a/drivers/video/via/vt1636.c b/drivers/video/via/vt1636.c
index a6b3749..4589c6e 100644
--- a/drivers/video/via/vt1636.c
+++ b/drivers/video/via/vt1636.c
@@ -27,9 +27,8 @@ u8 viafb_gpio_i2c_read_lvds(struct lvds_setting_information
 {
 	u8 data;
 
-	viaparinfo->shared->i2c_stuff.i2c_port = plvds_chip_info->i2c_port;
-	viafb_i2c_readbyte(plvds_chip_info->lvds_chip_slave_addr, index, &data);
-
+	viafb_i2c_readbyte(plvds_chip_info->i2c_port,
+			   plvds_chip_info->lvds_chip_slave_addr, index, &data);
 	return data;
 }
 
@@ -39,14 +38,13 @@ void viafb_gpio_i2c_write_mask_lvds(struct lvds_setting_information
 {
 	int index, data;
 
-	viaparinfo->shared->i2c_stuff.i2c_port = plvds_chip_info->i2c_port;
-
 	index = io_data.Index;
 	data = viafb_gpio_i2c_read_lvds(plvds_setting_info, plvds_chip_info,
 		index);
 	data = (data & (~io_data.Mask)) | io_data.Data;
 
-	viafb_i2c_writebyte(plvds_chip_info->lvds_chip_slave_addr, index, data);
+	viafb_i2c_writebyte(plvds_chip_info->i2c_port,
+			    plvds_chip_info->lvds_chip_slave_addr, index, data);
 }
 
 void viafb_init_lvds_vt1636(struct lvds_setting_information
@@ -159,7 +157,7 @@ void viafb_disable_lvds_vt1636(struct lvds_setting_information
 	}
 }
 
-bool viafb_lvds_identify_vt1636(void)
+bool viafb_lvds_identify_vt1636(u8 i2c_adapter)
 {
 	u8 Buffer[2];
 
@@ -170,23 +168,23 @@ bool viafb_lvds_identify_vt1636(void)
 	VT1636_LVDS_I2C_ADDR;
 
 	/* Check vendor ID first: */
-	viafb_i2c_readbyte((u8) viaparinfo->chip_info->lvds_chip_info.
-	lvds_chip_slave_addr,
-		    0x00, &Buffer[0]);
-	viafb_i2c_readbyte((u8) viaparinfo->chip_info->lvds_chip_info.
-		lvds_chip_slave_addr,
-		    0x01, &Buffer[1]);
+	viafb_i2c_readbyte(i2c_adapter,
+			   (u8) viaparinfo->chip_info->lvds_chip_info.lvds_chip_slave_addr,
+			   0x00, &Buffer[0]);
+	viafb_i2c_readbyte(i2c_adapter,
+			   (u8) viaparinfo->chip_info->lvds_chip_info.lvds_chip_slave_addr,
+			   0x01, &Buffer[1]);
 
 	if (!((Buffer[0] == 0x06) && (Buffer[1] == 0x11)))
 		return false;
 
 	/* Check Chip ID: */
-	viafb_i2c_readbyte((u8) viaparinfo->chip_info->lvds_chip_info.
-	lvds_chip_slave_addr,
-		    0x02, &Buffer[0]);
-	viafb_i2c_readbyte((u8) viaparinfo->chip_info->lvds_chip_info.
-		lvds_chip_slave_addr,
-		    0x03, &Buffer[1]);
+	viafb_i2c_readbyte(i2c_adapter,
+			   (u8) viaparinfo->chip_info->lvds_chip_info.lvds_chip_slave_addr,
+			   0x02, &Buffer[0]);
+	viafb_i2c_readbyte(i2c_adapter,
+			   (u8) viaparinfo->chip_info->lvds_chip_info.lvds_chip_slave_addr,
+			   0x03, &Buffer[1]);
 	if ((Buffer[0] == 0x45) && (Buffer[1] == 0x33)) {
 		viaparinfo->chip_info->lvds_chip_info.lvds_chip_name =
 			VT1636_LVDS;
diff --git a/drivers/video/via/vt1636.h b/drivers/video/via/vt1636.h
index 2a150c5..4c1314e 100644
--- a/drivers/video/via/vt1636.h
+++ b/drivers/video/via/vt1636.h
@@ -22,7 +22,7 @@
 #ifndef _VT1636_H_
 #define _VT1636_H_
 #include "chip.h"
-bool viafb_lvds_identify_vt1636(void);
+bool viafb_lvds_identify_vt1636(u8 i2c_adapter);
 void viafb_init_lvds_vt1636(struct lvds_setting_information
 	*plvds_setting_info, struct lvds_chip_information *plvds_chip_info);
 void viafb_enable_lvds_vt1636(struct lvds_setting_information
-- 
1.7.0.1

Re: [PATCH 10/11] viafb: rework the I2C support in the VIA framebuffer driver

[Florian Tobias Schandinat]

Hi,

Jonathan Corbet schrieb:
> From: Harald Welte <laforge@gnumonks.org>
> 
> This patch changes the way how the various I2C busses are used internally
> inside the viafb driver:  Previosuly, only a single i2c_adapter was created,
> even though two different hardware I2C busses are accessed: A structure member
> in a global variable was modified to indicate the bus to be used.
> 
> Now, all existing hardware busses are registered with the i2c core, and the
> viafb_i2c_{read,write}byte[s]() function take the adapter number as function
> call parameter, rather than referring to the global structure member.

as I did some runtime testing (VX800 only yet) I ran in 2 issues:

1. while the module loads the screen shows some strange colours. The 
colours itself do not worry me but it probably at least means that the 
module load time was increased significantly (2-3 seconds, I guess)

2. most annoyingly after this patch I can no longer get a working VGA 
console after unbinding the module. Normally I just do:

vbetool post
vbetool vbemode set 3
echo 0 >/sys/class/vtconsole/vtcon1/bind
clear
rmmod viafb

and have a working VGA console and can use another viafb. However after 
the patch I just get white out and strange colours. Reordering the 
commands does not help.

As the last one is especially annoying as it basically forces me to 
reboot for every viafb test and therefore makes development hell I tend 
to reject this patch, sorry. (and I don't have any idea what the cause 
is as I do not know I2C very well)


Thanks,

Florian Tobias Schandinat

> [jc: even more painful merge with mainline changes ->2.6.34]
> [jc: painful merge with OLPC changes]
> 
> Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
> Cc: ScottFang@viatech.com.cn
> Cc: JosephChan@via.com.tw
> Signed-off-by: Harald Welte <HaraldWelte@viatech.com>
> Signed-off-by: Jonathan Corbet <corbet@lwn.net>
> ---
>  drivers/video/via/dvi.c      |   35 ++++-----
>  drivers/video/via/lcd.c      |   19 ++---
>  drivers/video/via/via_i2c.c  |  171 ++++++++++++++++++++++++++----------------
>  drivers/video/via/via_i2c.h  |   43 +++++++----
>  drivers/video/via/viafbdev.c |    6 +-
>  drivers/video/via/viafbdev.h |    4 +-
>  drivers/video/via/vt1636.c   |   36 ++++-----
>  drivers/video/via/vt1636.h   |    2 +-
>  8 files changed, 182 insertions(+), 134 deletions(-)
> 
> diff --git a/drivers/video/via/dvi.c b/drivers/video/via/dvi.c
> index abe59b8..be51370 100644
> --- a/drivers/video/via/dvi.c
> +++ b/drivers/video/via/dvi.c
> @@ -96,7 +96,7 @@ int viafb_tmds_trasmitter_identify(void)
>  	viaparinfo->chip_info->tmds_chip_info.tmds_chip_name = VT1632_TMDS;
>  	viaparinfo->chip_info->
>  		tmds_chip_info.tmds_chip_slave_addr = VT1632_TMDS_I2C_ADDR;
> -	viaparinfo->chip_info->tmds_chip_info.i2c_port = I2CPORTINDEX;
> +	viaparinfo->chip_info->tmds_chip_info.i2c_port = VIA_I2C_ADAP_31;
>  	if (check_tmds_chip(VT1632_DEVICE_ID_REG, VT1632_DEVICE_ID) != FAIL) {
>  		/*
>  		 * Currently only support 12bits,dual edge,add 24bits mode later
> @@ -110,7 +110,7 @@ int viafb_tmds_trasmitter_identify(void)
>  			  viaparinfo->chip_info->tmds_chip_info.i2c_port);
>  		return OK;
>  	} else {
> -		viaparinfo->chip_info->tmds_chip_info.i2c_port = GPIOPORTINDEX;
> +		viaparinfo->chip_info->tmds_chip_info.i2c_port = VIA_I2C_ADAP_2C;
>  		if (check_tmds_chip(VT1632_DEVICE_ID_REG, VT1632_DEVICE_ID)
>  		    != FAIL) {
>  			tmds_register_write(0x08, 0x3b);
> @@ -160,32 +160,26 @@ int viafb_tmds_trasmitter_identify(void)
>  
>  static void tmds_register_write(int index, u8 data)
>  {
> -	viaparinfo->shared->i2c_stuff.i2c_port =
> -		viaparinfo->chip_info->tmds_chip_info.i2c_port;
> -
> -	viafb_i2c_writebyte(viaparinfo->chip_info->tmds_chip_info.
> -		tmds_chip_slave_addr, index,
> -		     data);
> +	viafb_i2c_writebyte(viaparinfo->chip_info->tmds_chip_info.i2c_port,
> +			    viaparinfo->chip_info->tmds_chip_info.tmds_chip_slave_addr,
> +			    index, data);
>  }
>  
>  static int tmds_register_read(int index)
>  {
>  	u8 data;
>  
> -	viaparinfo->shared->i2c_stuff.i2c_port =
> -		viaparinfo->chip_info->tmds_chip_info.i2c_port;
> -	viafb_i2c_readbyte((u8) viaparinfo->chip_info->
> -	    tmds_chip_info.tmds_chip_slave_addr,
> -			(u8) index, &data);
> +	viafb_i2c_readbyte(viaparinfo->chip_info->tmds_chip_info.i2c_port,
> +			   (u8) viaparinfo->chip_info->tmds_chip_info.tmds_chip_slave_addr,
> +			   (u8) index, &data);
>  	return data;
>  }
>  
>  static int tmds_register_read_bytes(int index, u8 *buff, int buff_len)
>  {
> -	viaparinfo->shared->i2c_stuff.i2c_port =
> -		viaparinfo->chip_info->tmds_chip_info.i2c_port;
> -	viafb_i2c_readbytes((u8) viaparinfo->chip_info->tmds_chip_info.
> -			 tmds_chip_slave_addr, (u8) index, buff, buff_len);
> +	viafb_i2c_readbytes(viaparinfo->chip_info->tmds_chip_info.i2c_port,
> +			    (u8) viaparinfo->chip_info->tmds_chip_info.tmds_chip_slave_addr,
> +			    (u8) index, buff, buff_len);
>  	return 0;
>  }
>  
> @@ -541,9 +535,10 @@ void viafb_dvi_enable(void)
>  				else
>  					data = 0x37;
>  				viafb_i2c_writebyte(viaparinfo->chip_info->
> -					     tmds_chip_info.
> -					     tmds_chip_slave_addr,
> -					     0x08, data);
> +						       tmds_chip_info.i2c_port,
> +						    viaparinfo->chip_info->
> +						       tmds_chip_info.tmds_chip_slave_addr,
> +						    0x08, data);
>  			}
>  		}
>  	}
> diff --git a/drivers/video/via/lcd.c b/drivers/video/via/lcd.c
> index ce3f4b6..98ae1c7 100644
> --- a/drivers/video/via/lcd.c
> +++ b/drivers/video/via/lcd.c
> @@ -180,18 +180,16 @@ int viafb_lvds_trasmitter_identify(void)
>  	if (machine_is_olpc())
>  		return FAIL;
>  
> -	viaparinfo->shared->i2c_stuff.i2c_port = I2CPORTINDEX;
> -	if (viafb_lvds_identify_vt1636()) {
> -		viaparinfo->chip_info->lvds_chip_info.i2c_port = I2CPORTINDEX;
> +	if (viafb_lvds_identify_vt1636(VIA_I2C_ADAP_31)) {
> +		viaparinfo->chip_info->lvds_chip_info.i2c_port = VIA_I2C_ADAP_31;
>  		DEBUG_MSG(KERN_INFO
> -			  "Found VIA VT1636 LVDS on port i2c 0x31 \n");
> +			  "Found VIA VT1636 LVDS on port i2c 0x31\n");
>  	} else {
> -		viaparinfo->shared->i2c_stuff.i2c_port = GPIOPORTINDEX;
> -		if (viafb_lvds_identify_vt1636()) {
> +		if (viafb_lvds_identify_vt1636(VIA_I2C_ADAP_26)) {
>  			viaparinfo->chip_info->lvds_chip_info.i2c_port =
> -				GPIOPORTINDEX;
> +				VIA_I2C_ADAP_2C;
>  			DEBUG_MSG(KERN_INFO
> -				  "Found VIA VT1636 LVDS on port gpio 0x2c \n");
> +				  "Found VIA VT1636 LVDS on port gpio 0x2c\n");
>  		}
>  	}
>  
> @@ -429,9 +427,8 @@ static int lvds_register_read(int index)
>  {
>  	u8 data;
>  
> -	viaparinfo->shared->i2c_stuff.i2c_port = GPIOPORTINDEX;
> -	viafb_i2c_readbyte((u8) viaparinfo->chip_info->
> -	    lvds_chip_info.lvds_chip_slave_addr,
> +	viafb_i2c_readbyte(VIA_I2C_ADAP_2C,
> +			(u8) viaparinfo->chip_info->lvds_chip_info.lvds_chip_slave_addr,
>  			(u8) index, &data);
>  	return data;
>  }
> diff --git a/drivers/video/via/via_i2c.c b/drivers/video/via/via_i2c.c
> index 15543e9..8f8e0bf 100644
> --- a/drivers/video/via/via_i2c.c
> +++ b/drivers/video/via/via_i2c.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright 1998-2008 VIA Technologies, Inc. All Rights Reserved.
> + * Copyright 1998-2009 VIA Technologies, Inc. All Rights Reserved.
>   * Copyright 2001-2008 S3 Graphics, Inc. All Rights Reserved.
>  
>   * This program is free software; you can redistribute it and/or
> @@ -24,40 +24,44 @@
>  static void via_i2c_setscl(void *data, int state)
>  {
>  	u8 val;
> -	struct via_i2c_stuff *via_i2c_chan = (struct via_i2c_stuff *)data;
> +	struct via_i2c_adap_cfg *adap_data = data;
>  
> -	val = viafb_read_reg(VIASR, via_i2c_chan->i2c_port) & 0xF0;
> +	printk(KERN_DEBUG "reading index 0x%02x from IO 0x%x\n",
> +		adap_data->ioport_index, adap_data->io_port);
> +	val = viafb_read_reg(adap_data->io_port,
> +			     adap_data->ioport_index) & 0xF0;
>  	if (state)
>  		val |= 0x20;
>  	else
>  		val &= ~0x20;
> -	switch (via_i2c_chan->i2c_port) {
> -	case I2CPORTINDEX:
> +	switch (adap_data->type) {
> +	case VIA_I2C_I2C:
>  		val |= 0x01;
>  		break;
> -	case GPIOPORTINDEX:
> +	case VIA_I2C_GPIO:
>  		val |= 0x80;
>  		break;
>  	default:
> -		DEBUG_MSG("via_i2c: specify wrong i2c port.\n");
> +		DEBUG_MSG("viafb_i2c: specify wrong i2c type.\n");
>  	}
> -	viafb_write_reg(via_i2c_chan->i2c_port, VIASR, val);
> +	viafb_write_reg(adap_data->ioport_index,
> +			adap_data->io_port, val);
>  }
>  
>  static int via_i2c_getscl(void *data)
>  {
> -	struct via_i2c_stuff *via_i2c_chan = (struct via_i2c_stuff *)data;
> +	struct via_i2c_adap_cfg *adap_data = data;
>  
> -	if (viafb_read_reg(VIASR, via_i2c_chan->i2c_port) & 0x08)
> +	if (viafb_read_reg(adap_data->io_port, adap_data->ioport_index) & 0x08)
>  		return 1;
>  	return 0;
>  }
>  
>  static int via_i2c_getsda(void *data)
>  {
> -	struct via_i2c_stuff *via_i2c_chan = (struct via_i2c_stuff *)data;
> +	struct via_i2c_adap_cfg *adap_data = data;
>  
> -	if (viafb_read_reg(VIASR, via_i2c_chan->i2c_port) & 0x04)
> +	if (viafb_read_reg(adap_data->io_port, adap_data->ioport_index) & 0x04)
>  		return 1;
>  	return 0;
>  }
> @@ -65,27 +69,29 @@ static int via_i2c_getsda(void *data)
>  static void via_i2c_setsda(void *data, int state)
>  {
>  	u8 val;
> -	struct via_i2c_stuff *via_i2c_chan = (struct via_i2c_stuff *)data;
> +	struct via_i2c_adap_cfg *adap_data = data;
>  
> -	val = viafb_read_reg(VIASR, via_i2c_chan->i2c_port) & 0xF0;
> +	val = viafb_read_reg(adap_data->io_port,
> +			     adap_data->ioport_index) & 0xF0;
>  	if (state)
>  		val |= 0x10;
>  	else
>  		val &= ~0x10;
> -	switch (via_i2c_chan->i2c_port) {
> -	case I2CPORTINDEX:
> +	switch (adap_data->type) {
> +	case VIA_I2C_I2C:
>  		val |= 0x01;
>  		break;
> -	case GPIOPORTINDEX:
> +	case VIA_I2C_GPIO:
>  		val |= 0x40;
>  		break;
>  	default:
> -		DEBUG_MSG("via_i2c: specify wrong i2c port.\n");
> +		DEBUG_MSG("viafb_i2c: specify wrong i2c type.\n");
>  	}
> -	viafb_write_reg(via_i2c_chan->i2c_port, VIASR, val);
> +	viafb_write_reg(adap_data->ioport_index,
> +			adap_data->io_port, val);
>  }
>  
> -int viafb_i2c_readbyte(u8 slave_addr, u8 index, u8 *pdata)
> +int viafb_i2c_readbyte(u8 adap, u8 slave_addr, u8 index, u8 *pdata)
>  {
>  	u8 mm1[] = {0x00};
>  	struct i2c_msg msgs[2];
> @@ -97,12 +103,11 @@ int viafb_i2c_readbyte(u8 slave_addr, u8 index, u8 *pdata)
>  	mm1[0] = index;
>  	msgs[0].len = 1; msgs[1].len = 1;
>  	msgs[0].buf = mm1; msgs[1].buf = pdata;
> -	i2c_transfer(&viaparinfo->shared->i2c_stuff.adapter, msgs, 2);
> -
> -	return 0;
> +	return i2c_transfer(&viaparinfo->shared->i2c_stuff[adap].adapter,
> +			msgs, 2);
>  }
>  
> -int viafb_i2c_writebyte(u8 slave_addr, u8 index, u8 data)
> +int viafb_i2c_writebyte(u8 adap, u8 slave_addr, u8 index, u8 data)
>  {
>  	u8 msg[2] = { index, data };
>  	struct i2c_msg msgs;
> @@ -111,10 +116,11 @@ int viafb_i2c_writebyte(u8 slave_addr, u8 index, u8 data)
>  	msgs.addr = slave_addr / 2;
>  	msgs.len = 2;
>  	msgs.buf = msg;
> -	return i2c_transfer(&viaparinfo->shared->i2c_stuff.adapter, &msgs, 1);
> +	return i2c_transfer(&viaparinfo->shared->i2c_stuff[adap].adapter,
> +			&msgs, 1);
>  }
>  
> -int viafb_i2c_readbytes(u8 slave_addr, u8 index, u8 *buff, int buff_len)
> +int viafb_i2c_readbytes(u8 adap, u8 slave_addr, u8 index, u8 *buff, int buff_len)
>  {
>  	u8 mm1[] = {0x00};
>  	struct i2c_msg msgs[2];
> @@ -125,53 +131,88 @@ int viafb_i2c_readbytes(u8 slave_addr, u8 index, u8 *buff, int buff_len)
>  	mm1[0] = index;
>  	msgs[0].len = 1; msgs[1].len = buff_len;
>  	msgs[0].buf = mm1; msgs[1].buf = buff;
> -	i2c_transfer(&viaparinfo->shared->i2c_stuff.adapter, msgs, 2);
> -	return 0;
> +	return i2c_transfer(&viaparinfo->shared->i2c_stuff[adap].adapter,
> +			msgs, 2);
>  }
>  
> -int viafb_create_i2c_bus(void *viapar)
> +static int create_i2c_bus(struct i2c_adapter *adapter,
> +			  struct i2c_algo_bit_data *algo,
> +			  struct via_i2c_adap_cfg *adap_cfg,
> +			  struct pci_dev *pdev)
>  {
> -	int ret;
> -	struct via_i2c_stuff *i2c_stuff =
> -		&((struct viafb_par *)viapar)->shared->i2c_stuff;
> -
> -	strcpy(i2c_stuff->adapter.name, "via_i2c");
> -	i2c_stuff->i2c_port = 0x0;
> -	i2c_stuff->adapter.owner = THIS_MODULE;
> -	i2c_stuff->adapter.id = 0x01FFFF;
> -	i2c_stuff->adapter.class = 0;
> -	i2c_stuff->adapter.algo_data = &i2c_stuff->algo;
> -	i2c_stuff->adapter.dev.parent = NULL;
> -	i2c_stuff->algo.setsda = via_i2c_setsda;
> -	i2c_stuff->algo.setscl = via_i2c_setscl;
> -	i2c_stuff->algo.getsda = via_i2c_getsda;
> -	i2c_stuff->algo.getscl = via_i2c_getscl;
> -	i2c_stuff->algo.udelay = 40;
> -	i2c_stuff->algo.timeout = 20;
> -	i2c_stuff->algo.data = i2c_stuff;
> -
> -	i2c_set_adapdata(&i2c_stuff->adapter, i2c_stuff);
> +	printk(KERN_DEBUG "viafb: creating bus adap=0x%p, algo_bit_data=0x%p, adap_cfg=0x%p\n", adapter, algo, adap_cfg);
> +
> +	algo->setsda = via_i2c_setsda;
> +	algo->setscl = via_i2c_setscl;
> +	algo->getsda = via_i2c_getsda;
> +	algo->getscl = via_i2c_getscl;
> +	algo->udelay = 40;
> +	algo->timeout = 20;
> +	algo->data = adap_cfg;
> +
> +	sprintf(adapter->name, "viafb i2c io_port idx 0x%02x",
> +		adap_cfg->ioport_index);
> +	adapter->owner = THIS_MODULE;
> +	adapter->id = 0x01FFFF;
> +	adapter->class = I2C_CLASS_DDC;
> +	adapter->algo_data = algo;
> +	if (pdev)
> +		adapter->dev.parent = &pdev->dev;
> +	else
> +		adapter->dev.parent = NULL;
> +	/* i2c_set_adapdata(adapter, adap_cfg); */
>  
>  	/* Raise SCL and SDA */
> -	i2c_stuff->i2c_port = I2CPORTINDEX;
> -	via_i2c_setsda(i2c_stuff, 1);
> -	via_i2c_setscl(i2c_stuff, 1);
> -
> -	i2c_stuff->i2c_port = GPIOPORTINDEX;
> -	via_i2c_setsda(i2c_stuff, 1);
> -	via_i2c_setscl(i2c_stuff, 1);
> +	via_i2c_setsda(adap_cfg, 1);
> +	via_i2c_setscl(adap_cfg, 1);
>  	udelay(20);
>  
> -	ret = i2c_bit_add_bus(&i2c_stuff->adapter);
> -	if (ret == 0)
> -		DEBUG_MSG("I2C bus %s registered.\n", i2c_stuff->adapter.name);
> -	else
> -		DEBUG_MSG("Failed to register I2C bus %s.\n",
> -			i2c_stuff->adapter.name);
> -	return ret;
> +	return i2c_bit_add_bus(adapter);
> +}
> +
> +static struct via_i2c_adap_cfg adap_configs[] = {
> +	[VIA_I2C_ADAP_26]	= { VIA_I2C_I2C,  VIASR, 0x26 },
> +	[VIA_I2C_ADAP_31]	= { VIA_I2C_I2C,  VIASR, 0x31 },
> +	[VIA_I2C_ADAP_25]	= { VIA_I2C_GPIO, VIASR, 0x25 },
> +	[VIA_I2C_ADAP_2C]	= { VIA_I2C_GPIO, VIASR, 0x2c },
> +	[VIA_I2C_ADAP_3D]	= { VIA_I2C_GPIO, VIASR, 0x3d },
> +	{ 0, 0, 0 }
> +};
> +
> +int viafb_create_i2c_busses(struct viafb_par *viapar)
> +{
> +	int i, ret;
> +
> +	for (i = 0; i < VIAFB_NUM_I2C; i++) {
> +		struct via_i2c_adap_cfg *adap_cfg = &adap_configs[i];
> +		struct via_i2c_stuff *i2c_stuff = &viapar->shared->i2c_stuff[i];
> +
> +		if (adap_cfg->type == 0)
> +			break;
> +
> +		ret = create_i2c_bus(&i2c_stuff->adapter,
> +				     &i2c_stuff->algo, adap_cfg,
> +				NULL); /* FIXME: PCIDEV */
> +		if (ret < 0) {
> +			printk(KERN_ERR "viafb: cannot create i2c bus %u:%d\n",
> +				i, ret);
> +			/* FIXME: properly release previous busses */
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
>  }
>  
> -void viafb_delete_i2c_buss(void *par)
> +void viafb_delete_i2c_busses(struct viafb_par *par)
>  {
> -	i2c_del_adapter(&((struct viafb_par *)par)->shared->i2c_stuff.adapter);
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(par->shared->i2c_stuff); i++) {
> +		struct via_i2c_stuff *i2c_stuff = &par->shared->i2c_stuff[i];
> +		/* only remove those entries in the array that we've
> +		 * actually used (and thus initialized algo_data) */
> +		if (i2c_stuff->adapter.algo_data == &i2c_stuff->algo)
> +			i2c_del_adapter(&i2c_stuff->adapter);
> +	}
>  }
> diff --git a/drivers/video/via/via_i2c.h b/drivers/video/via/via_i2c.h
> index 3a13242..00ed978 100644
> --- a/drivers/video/via/via_i2c.h
> +++ b/drivers/video/via/via_i2c.h
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright 1998-2008 VIA Technologies, Inc. All Rights Reserved.
> + * Copyright 1998-2009 VIA Technologies, Inc. All Rights Reserved.
>   * Copyright 2001-2008 S3 Graphics, Inc. All Rights Reserved.
>  
>   * This program is free software; you can redistribute it and/or
> @@ -24,23 +24,38 @@
>  #include <linux/i2c.h>
>  #include <linux/i2c-algo-bit.h>
>  
> +enum via_i2c_type {
> +	VIA_I2C_NONE,
> +	VIA_I2C_I2C,
> +	VIA_I2C_GPIO,
> +};
> +
> +/* private data for each adapter */
> +struct via_i2c_adap_cfg {
> +	enum via_i2c_type	type;
> +	u_int16_t		io_port;
> +	u_int8_t		ioport_index;
> +};
> +
>  struct via_i2c_stuff {
>  	u16 i2c_port;			/* GPIO or I2C port */
>  	struct i2c_adapter adapter;
>  	struct i2c_algo_bit_data algo;
>  };
>  
> -#define I2CPORT           0x3c4
> -#define I2CPORTINDEX      0x31
> -#define GPIOPORT          0x3C4
> -#define GPIOPORTINDEX     0x2C
> -#define I2C_BUS             1
> -#define GPIO_BUS            2
> -#define DELAYPORT           0x3C3
> -
> -int viafb_i2c_readbyte(u8 slave_addr, u8 index, u8 *pdata);
> -int viafb_i2c_writebyte(u8 slave_addr, u8 index, u8 data);
> -int viafb_i2c_readbytes(u8 slave_addr, u8 index, u8 *buff, int buff_len);
> -int viafb_create_i2c_bus(void *par);
> -void viafb_delete_i2c_buss(void *par);
> +enum viafb_i2c_adap {
> +	VIA_I2C_ADAP_26,
> +	VIA_I2C_ADAP_31,
> +	VIA_I2C_ADAP_25,
> +	VIA_I2C_ADAP_2C,
> +	VIA_I2C_ADAP_3D,
> +};
> +
> +int viafb_i2c_readbyte(u8 adap, u8 slave_addr, u8 index, u8 *pdata);
> +int viafb_i2c_writebyte(u8 adap, u8 slave_addr, u8 index, u8 data);
> +int viafb_i2c_readbytes(u8 adap, u8 slave_addr, u8 index, u8 *buff, int buff_len);
> +
> +struct viafb_par;
> +int viafb_create_i2c_busses(struct viafb_par *par);
> +void viafb_delete_i2c_busses(struct viafb_par *par);
>  #endif /* __VIA_I2C_H__ */
> diff --git a/drivers/video/via/viafbdev.c b/drivers/video/via/viafbdev.c
> index 8955ab4..fa10049 100644
> --- a/drivers/video/via/viafbdev.c
> +++ b/drivers/video/via/viafbdev.c
> @@ -1775,7 +1775,7 @@ static int __devinit via_pci_probe(struct pci_dev *pdev,
>  		viafb_dual_fb = 0;
>  
>  	/* Set up I2C bus stuff */
> -	rc = viafb_create_i2c_bus(viaparinfo);
> +	rc = viafb_create_i2c_busses(viaparinfo);
>  	if (rc)
>  		goto out_fb_release;
>  
> @@ -1964,7 +1964,7 @@ out_fb1_release:
>  out_unmap_screen:
>  	iounmap(viafbinfo->screen_base);
>  out_delete_i2c:
> -	viafb_delete_i2c_buss(viaparinfo);
> +	viafb_delete_i2c_busses(viaparinfo);
>  out_fb_release:
>  	framebuffer_release(viafbinfo);
>  	return rc;
> @@ -1980,7 +1980,7 @@ static void __devexit via_pci_remove(struct pci_dev *pdev)
>  	iounmap((void *)viafbinfo->screen_base);
>  	iounmap(viaparinfo->shared->engine_mmio);
>  
> -	viafb_delete_i2c_buss(viaparinfo);
> +	viafb_delete_i2c_busses(viaparinfo);
>  
>  	framebuffer_release(viafbinfo);
>  	if (viafb_dual_fb)
> diff --git a/drivers/video/via/viafbdev.h b/drivers/video/via/viafbdev.h
> index 61b5953..4bc00ec 100644
> --- a/drivers/video/via/viafbdev.h
> +++ b/drivers/video/via/viafbdev.h
> @@ -37,11 +37,13 @@
>  #define VERSION_OS          0	/* 0: for 32 bits OS, 1: for 64 bits OS */
>  #define VERSION_MINOR       4
>  
> +#define VIAFB_NUM_I2C		5
> +
>  struct viafb_shared {
>  	struct proc_dir_entry *proc_entry;	/*viafb proc entry */
>  
>  	/* I2C stuff */
> -	struct via_i2c_stuff i2c_stuff;
> +	struct via_i2c_stuff i2c_stuff[VIAFB_NUM_I2C];
>  
>  	/* All the information will be needed to set engine */
>  	struct tmds_setting_information tmds_setting_info;
> diff --git a/drivers/video/via/vt1636.c b/drivers/video/via/vt1636.c
> index a6b3749..4589c6e 100644
> --- a/drivers/video/via/vt1636.c
> +++ b/drivers/video/via/vt1636.c
> @@ -27,9 +27,8 @@ u8 viafb_gpio_i2c_read_lvds(struct lvds_setting_information
>  {
>  	u8 data;
>  
> -	viaparinfo->shared->i2c_stuff.i2c_port = plvds_chip_info->i2c_port;
> -	viafb_i2c_readbyte(plvds_chip_info->lvds_chip_slave_addr, index, &data);
> -
> +	viafb_i2c_readbyte(plvds_chip_info->i2c_port,
> +			   plvds_chip_info->lvds_chip_slave_addr, index, &data);
>  	return data;
>  }
>  
> @@ -39,14 +38,13 @@ void viafb_gpio_i2c_write_mask_lvds(struct lvds_setting_information
>  {
>  	int index, data;
>  
> -	viaparinfo->shared->i2c_stuff.i2c_port = plvds_chip_info->i2c_port;
> -
>  	index = io_data.Index;
>  	data = viafb_gpio_i2c_read_lvds(plvds_setting_info, plvds_chip_info,
>  		index);
>  	data = (data & (~io_data.Mask)) | io_data.Data;
>  
> -	viafb_i2c_writebyte(plvds_chip_info->lvds_chip_slave_addr, index, data);
> +	viafb_i2c_writebyte(plvds_chip_info->i2c_port,
> +			    plvds_chip_info->lvds_chip_slave_addr, index, data);
>  }
>  
>  void viafb_init_lvds_vt1636(struct lvds_setting_information
> @@ -159,7 +157,7 @@ void viafb_disable_lvds_vt1636(struct lvds_setting_information
>  	}
>  }
>  
> -bool viafb_lvds_identify_vt1636(void)
> +bool viafb_lvds_identify_vt1636(u8 i2c_adapter)
>  {
>  	u8 Buffer[2];
>  
> @@ -170,23 +168,23 @@ bool viafb_lvds_identify_vt1636(void)
>  	VT1636_LVDS_I2C_ADDR;
>  
>  	/* Check vendor ID first: */
> -	viafb_i2c_readbyte((u8) viaparinfo->chip_info->lvds_chip_info.
> -	lvds_chip_slave_addr,
> -		    0x00, &Buffer[0]);
> -	viafb_i2c_readbyte((u8) viaparinfo->chip_info->lvds_chip_info.
> -		lvds_chip_slave_addr,
> -		    0x01, &Buffer[1]);
> +	viafb_i2c_readbyte(i2c_adapter,
> +			   (u8) viaparinfo->chip_info->lvds_chip_info.lvds_chip_slave_addr,
> +			   0x00, &Buffer[0]);
> +	viafb_i2c_readbyte(i2c_adapter,
> +			   (u8) viaparinfo->chip_info->lvds_chip_info.lvds_chip_slave_addr,
> +			   0x01, &Buffer[1]);
>  
>  	if (!((Buffer[0] == 0x06) && (Buffer[1] == 0x11)))
>  		return false;
>  
>  	/* Check Chip ID: */
> -	viafb_i2c_readbyte((u8) viaparinfo->chip_info->lvds_chip_info.
> -	lvds_chip_slave_addr,
> -		    0x02, &Buffer[0]);
> -	viafb_i2c_readbyte((u8) viaparinfo->chip_info->lvds_chip_info.
> -		lvds_chip_slave_addr,
> -		    0x03, &Buffer[1]);
> +	viafb_i2c_readbyte(i2c_adapter,
> +			   (u8) viaparinfo->chip_info->lvds_chip_info.lvds_chip_slave_addr,
> +			   0x02, &Buffer[0]);
> +	viafb_i2c_readbyte(i2c_adapter,
> +			   (u8) viaparinfo->chip_info->lvds_chip_info.lvds_chip_slave_addr,
> +			   0x03, &Buffer[1]);
>  	if ((Buffer[0] == 0x45) && (Buffer[1] == 0x33)) {
>  		viaparinfo->chip_info->lvds_chip_info.lvds_chip_name =
>  			VT1636_LVDS;
> diff --git a/drivers/video/via/vt1636.h b/drivers/video/via/vt1636.h
> index 2a150c5..4c1314e 100644
> --- a/drivers/video/via/vt1636.h
> +++ b/drivers/video/via/vt1636.h
> @@ -22,7 +22,7 @@
>  #ifndef _VT1636_H_
>  #define _VT1636_H_
>  #include "chip.h"
> -bool viafb_lvds_identify_vt1636(void);
> +bool viafb_lvds_identify_vt1636(u8 i2c_adapter);
>  void viafb_init_lvds_vt1636(struct lvds_setting_information
>  	*plvds_setting_info, struct lvds_chip_information *plvds_chip_info);
>  void viafb_enable_lvds_vt1636(struct lvds_setting_information

Re: [PATCH 10/11] viafb: rework the I2C support in the VIA framebuffer driver

[Jonathan Corbet]

On Fri, 23 Apr 2010 23:12:54 +0200
Florian Tobias Schandinat <FlorianSchandinat@gmx.de> wrote:

> 1. while the module loads the screen shows some strange colours. The 
> colours itself do not worry me but it probably at least means that the 
> module load time was increased significantly (2-3 seconds, I guess)
> 
> 2. most annoyingly after this patch I can no longer get a working VGA 
> console after unbinding the module. Normally I just do:

Hmm...compiling the driver as a module is not really something that
makes sense on the OLPC platform, so I've never done it.

#1 is weird, there should be nothing there that slows initialization
down.  Putting in some prints and looking at the timing would be most
helpful here.

As for #2, I can certainly say that I've never unloaded this code, so
that's an untested path.  I'll have a look and see if I can see
anything obvious.

jon

Re: [PATCH 10/11] viafb: rework the I2C support in the VIA framebuffer driver

[Florian Tobias Schandinat]

Jonathan Corbet schrieb:
> On Fri, 23 Apr 2010 23:12:54 +0200
> Florian Tobias Schandinat <FlorianSchandinat@gmx.de> wrote:
> 
>> 1. while the module loads the screen shows some strange colours. The 
>> colours itself do not worry me but it probably at least means that the 
>> module load time was increased significantly (2-3 seconds, I guess)
>>
>> 2. most annoyingly after this patch I can no longer get a working VGA 
>> console after unbinding the module. Normally I just do:
> 
> Hmm...compiling the driver as a module is not really something that
> makes sense on the OLPC platform, so I've never done it.
> 
> #1 is weird, there should be nothing there that slows initialization
> down.  Putting in some prints and looking at the timing would be most
> helpful here.

Actually that is probably a mistake on my side. I had the impression 
that it was much longer but didn't take into account that the old 
behaviour allowed the VGA console to work until viafb was completly 
loaded and fbcon took over while the new one immediately destroys the 
screen and shows random things until it is completely loaded. However 
the time between hitting enter for the module load command and getting a 
working fbcon really is at least nearly the same.

> As for #2, I can certainly say that I've never unloaded this code, so
> that's an untested path.  I'll have a look and see if I can see
> anything obvious.

Well as for the behaviour change described above I think the problem 
might be in the load path. At least when I faked an exit as when memory 
size detection or ioremapping fails (which is a very common issue that 
really needs a workaround) I get the same unusable VGA console. This 
needs to be fixed.
This whole I2C stuff seems incredibly unstable I even have indicators 
that the current code might be to blame for freezing the machine on some 
configurations with P4M900 IGP. I just try to prevent it to get even 
worse...


Thanks,

Florian Tobias Schandinat

Re: [PATCH 10/11] viafb: rework the I2C support in the VIA framebuffer driver

[Jonathan Corbet]

On Sat, 24 Apr 2010 00:40:39 +0200
Florian Tobias Schandinat <FlorianSchandinat@gmx.de> wrote:

> Actually that is probably a mistake on my side. I had the impression 
> that it was much longer but didn't take into account that the old 
> behaviour allowed the VGA console to work until viafb was completly 
> loaded and fbcon took over while the new one immediately destroys the 
> screen and shows random things until it is completely loaded. 

"New one" being new relative to what?  Is that change the result of the
patches I've posted, or something else?

> > As for #2, I can certainly say that I've never unloaded this code, so
> > that's an untested path.  I'll have a look and see if I can see
> > anything obvious.  
> 
> Well as for the behaviour change described above I think the problem 
> might be in the load path. At least when I faked an exit as when memory 
> size detection or ioremapping fails (which is a very common issue that 
> really needs a workaround) I get the same unusable VGA console. This 
> needs to be fixed.

Interesting.  In the environment I've been working in the whole box is
a brick if the framebuffer doesn't come up right.  But things are
pretty solid on that front here.

> This whole I2C stuff seems incredibly unstable I even have indicators 
> that the current code might be to blame for freezing the machine on some 
> configurations with P4M900 IGP. I just try to prevent it to get even 
> worse...

I have to say that i2c has often been the bane of my existence.  It
seems like something that just barely works most of the time.

That said, it's been a long time since I've seen any trouble I could
blame on i2c in the viafb driver.  It's *really* hard to imagine how
it could free machines.  Unless, maybe, you're hitting some sort of
race condition in all of those indexed I/O port operations.  But that
looks like something which would be hard to do on most machines that
would have these chipsets in them.

The second series adds some locking around i2c port operations, but has
not yet pushed that locking into the framebuffer side of the driver;
that would be a good thing to do.

Meanwhile, I'm a little unsure now...is there an action item for me
with regard to the i2c code?  I've been staring at it since your last
note, but I couldn't find any obvious problems.  I do have to say that
Harald's rework is far cleaner than what came before...

Thanks,

jon

Re: [PATCH 10/11] viafb: rework the I2C support in the VIA framebuffer driver

[Florian Tobias Schandinat]

Jonathan Corbet schrieb:
> On Sat, 24 Apr 2010 00:40:39 +0200
> Florian Tobias Schandinat <FlorianSchandinat@gmx.de> wrote:
> 
>> Actually that is probably a mistake on my side. I had the impression 
>> that it was much longer but didn't take into account that the old 
>> behaviour allowed the VGA console to work until viafb was completly 
>> loaded and fbcon took over while the new one immediately destroys the 
>> screen and shows random things until it is completely loaded. 
> 
> "New one" being new relative to what?  Is that change the result of the
> patches I've posted, or something else?

Relative to the patch we are talking about. With patches 01-09 applied 
it just works fine as it always did (no screen "destruction" on load, a 
working VGA console if unloaded as described earlier). With additionally 
patch 10 applied I get the weird behaviour I'm talking about.

>>> As for #2, I can certainly say that I've never unloaded this code, so
>>> that's an untested path.  I'll have a look and see if I can see
>>> anything obvious.  
>> Well as for the behaviour change described above I think the problem 
>> might be in the load path. At least when I faked an exit as when memory 
>> size detection or ioremapping fails (which is a very common issue that 
>> really needs a workaround) I get the same unusable VGA console. This 
>> needs to be fixed.
> 
> Interesting.  In the environment I've been working in the whole box is
> a brick if the framebuffer doesn't come up right.  But things are
> pretty solid on that front here.

Well the ioremapping fails when huge amount of video memory is available 
(128 MB and up) if no extra space is allocated the "vmalloc=" kernel 
option. To not require it we (a) should only remap the needed memory or 
(b) reduce the memory to be remapped until we succeed (or hit a too low 
value).

>> This whole I2C stuff seems incredibly unstable I even have indicators 
>> that the current code might be to blame for freezing the machine on some 
>> configurations with P4M900 IGP. I just try to prevent it to get even 
>> worse...
> 
> I have to say that i2c has often been the bane of my existence.  It
> seems like something that just barely works most of the time.
> 
> That said, it's been a long time since I've seen any trouble I could
> blame on i2c in the viafb driver.  It's *really* hard to imagine how
> it could free machines.  Unless, maybe, you're hitting some sort of
> race condition in all of those indexed I/O port operations.  But that
> looks like something which would be hard to do on most machines that
> would have these chipsets in them.

I don't know but a freeze reported due to viafb with viafb itself 
exiting due to ioremap failure can't have so much causes. Either we do 
something terribly wrong on the error out side (this might be possible 
regarding the current condition of error handling) or in the I2C setup 
which is done prior to remapping. The full story can be read on
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/539020

> The second series adds some locking around i2c port operations, but has
> not yet pushed that locking into the framebuffer side of the driver;
> that would be a good thing to do.
> 
> Meanwhile, I'm a little unsure now...is there an action item for me
> with regard to the i2c code?  I've been staring at it since your last
> note, but I couldn't find any obvious problems.  I do have to say that
> Harald's rework is far cleaner than what came before...

Well the main question is probably:
How does it change the behaviour towards the hardware?
I tend to think that OLPC might not be the only ones who did something 
weird with it....and we already know that we shouldn't trust the 
documentation too much.


Thanks,

Florian Tobias Schandinat

Re: [PATCH 10/11] viafb: rework the I2C support in the VIA framebuffer driver

[Florian Tobias Schandinat]

Florian Tobias Schandinat schrieb:
> Jonathan Corbet schrieb:
>> On Sat, 24 Apr 2010 00:40:39 +0200
>> Florian Tobias Schandinat <FlorianSchandinat@gmx.de> wrote:
>> Meanwhile, I'm a little unsure now...is there an action item for me
>> with regard to the i2c code?  I've been staring at it since your last
>> note, but I couldn't find any obvious problems.  I do have to say that
>> Harald's rework is far cleaner than what came before...
> 
> Well the main question is probably:
> How does it change the behaviour towards the hardware?
> I tend to think that OLPC might not be the only ones who did something 
> weird with it....and we already know that we shouldn't trust the 
> documentation too much.

I was able to narrow this issue down to the fourth bus. So with
if (i == 4)
	continue;
in the bus creation loop I'm able to get a working framebuffer which 
does not have the issues mentioned. Any ideas what should be done now?


Thanks,

Florian Tobias Schandinat

Re: [PATCH 10/11] viafb: rework the I2C support in the VIA framebuffer driver

[Jonathan Corbet]

On Sat, 24 Apr 2010 12:47:34 +0200
Florian Tobias Schandinat <FlorianSchandinat@gmx.de> wrote:

> I was able to narrow this issue down to the fourth bus. So with
> if (i == 4)
> 	continue;
> in the bus creation loop I'm able to get a working framebuffer which 
> does not have the issues mentioned. Any ideas what should be done now?

That is useful information indeed, thanks.

This patch creates an i2c bus on every port which could conceivably
have one.  That's rather different from the original code, which
created a single bus (in the kernel) then swapped it between ports 31
and 2c in weird ways.  In particular, it takes over ports which,
conceivably, somebody else could be using for GPIO.  So, perhaps, there
is some contention going on there.

If my hypothesis is correct, this problem will go away with the
application of the second series, which doesn't create i2c buses on
ports used for GPIO.  But the problem should be fixed here so we don't
have things broken in the middle.

Harald, does this reasoning make sense to you?  If so, what I should do
is bring forward just enough of the via-core code to be able to
configure which ports actually get i2c buses created on them.  Easily
done.

Thanks,

jon

Re: [PATCH 10/11] viafb: rework the I2C support in the VIA framebuffer driver

[Harald Welte]

Hi Jon,

On Sat, Apr 24, 2010 at 07:33:59AM -0600, Jonathan Corbet wrote:

> This patch creates an i2c bus on every port which could conceivably
> have one.  That's rather different from the original code, which
> created a single bus (in the kernel) then swapped it between ports 31
> and 2c in weird ways.  In particular, it takes over ports which,
> conceivably, somebody else could be using for GPIO.  So, perhaps, there
> is some contention going on there.
> 
> If my hypothesis is correct, this problem will go away with the
> application of the second series, which doesn't create i2c buses on
> ports used for GPIO.  But the problem should be fixed here so we don't
> have things broken in the middle.
> 
> Harald, does this reasoning make sense to you?  If so, what I should do
> is bring forward just enough of the via-core code to be able to
> configure which ports actually get i2c buses created on them.  Easily
> done.

It makes a lot of sense.  When I added all possible I2C busses, this was merely
the result of "let's try to make sure every possible configuration is
supported" rather than some kind of neccessity.

Simply converting the original two busses to the new code is definitely
the way to go.  Maybe we'll keep the code for the other two around, but
somehow keep them inactive and don't advertise them unless somebody explicitly
does so?

-- 
- Harald Welte <laforge@gnumonks.org>           http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)

Re: [PATCH 10/11] viafb: rework the I2C support in the VIA framebuffer driver

[Jonathan Corbet]

On Sat, 24 Apr 2010 15:53:16 +0200
Harald Welte <laforge@gnumonks.org> wrote:

> Simply converting the original two busses to the new code is definitely
> the way to go.  Maybe we'll keep the code for the other two around, but
> somehow keep them inactive and don't advertise them unless somebody explicitly
> does so?

OK, my proposal would be to add the following patch into the early part
of the series; that will help to avoid the creation of confusion in the
middle until the full i2c/gpio configuration code is in.

Look good?

Thanks,

jon

viafb: Only establish i2c busses on ports that always had them

...otherwise it seems we run into conflicts with shadowy other users which
don't expect to see i2c taking control of ports it never used to do
anything with.

Reported-by: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Signed-off-by: Jonathan Corbet <corbet@lwn.net>
---
 drivers/video/via/via_i2c.c |   19 +++++++++++++------
 drivers/video/via/via_i2c.h |    1 +
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/video/via/via_i2c.c b/drivers/video/via/via_i2c.c
index fe5535c..b5253e3 100644
--- a/drivers/video/via/via_i2c.c
+++ b/drivers/video/via/via_i2c.c
@@ -170,13 +170,18 @@ static int create_i2c_bus(struct i2c_adapter *adapter,
 	return i2c_bit_add_bus(adapter);
 }
 
+/*
+ * By default, we only activate busses on ports 2c and 31 to avoid
+ * conflicts with other possible users; that default can be changed
+ * below.
+ */
 static struct via_i2c_adap_cfg adap_configs[] = {
-	[VIA_I2C_ADAP_26]	= { VIA_I2C_I2C,  VIASR, 0x26 },
-	[VIA_I2C_ADAP_31]	= { VIA_I2C_I2C,  VIASR, 0x31 },
-	[VIA_I2C_ADAP_25]	= { VIA_I2C_GPIO, VIASR, 0x25 },
-	[VIA_I2C_ADAP_2C]	= { VIA_I2C_GPIO, VIASR, 0x2c },
-	[VIA_I2C_ADAP_3D]	= { VIA_I2C_GPIO, VIASR, 0x3d },
-	{ 0, 0, 0 }
+	[VIA_I2C_ADAP_26]	= { VIA_I2C_I2C,  VIASR, 0x26, 0 },
+	[VIA_I2C_ADAP_31]	= { VIA_I2C_I2C,  VIASR, 0x31, 1 },
+	[VIA_I2C_ADAP_25]	= { VIA_I2C_GPIO, VIASR, 0x25, 0 },
+	[VIA_I2C_ADAP_2C]	= { VIA_I2C_GPIO, VIASR, 0x2c, 1 },
+	[VIA_I2C_ADAP_3D]	= { VIA_I2C_GPIO, VIASR, 0x3d, 0 },
+	{ 0, 0, 0, 0 }
 };
 
 int viafb_create_i2c_busses(struct viafb_par *viapar)
@@ -189,6 +194,8 @@ int viafb_create_i2c_busses(struct viafb_par *viapar)
 
 		if (adap_cfg->type == 0)
 			break;
+		if (!adap_cfg->is_active)
+			continue;
 
 		ret = create_i2c_bus(&i2c_stuff->adapter,
 				     &i2c_stuff->algo, adap_cfg,
diff --git a/drivers/video/via/via_i2c.h b/drivers/video/via/via_i2c.h
index 00ed978..73d682f 100644
--- a/drivers/video/via/via_i2c.h
+++ b/drivers/video/via/via_i2c.h
@@ -35,6 +35,7 @@ struct via_i2c_adap_cfg {
 	enum via_i2c_type	type;
 	u_int16_t		io_port;
 	u_int8_t		ioport_index;
+	u8			is_active;
 };
 
 struct via_i2c_stuff {
-- 
1.7.0.1

Re: [PATCH 10/11] viafb: rework the I2C support in the VIA framebuffer driver

[Florian Tobias Schandinat]

Hi Jon,

Jonathan Corbet schrieb:
> On Sat, 24 Apr 2010 15:53:16 +0200
> Harald Welte <laforge@gnumonks.org> wrote:
> 
>> Simply converting the original two busses to the new code is definitely
>> the way to go.  Maybe we'll keep the code for the other two around, but
>> somehow keep them inactive and don't advertise them unless somebody explicitly
>> does so?
> 
> OK, my proposal would be to add the following patch into the early part
> of the series; that will help to avoid the creation of confusion in the
> middle until the full i2c/gpio configuration code is in.
> 
> Look good?

Yes, it does. But it highlighted a bug in the original patch:

BUG: unable to handle kernel NULL pointer dereference at (null)
IP: [<c11df2db>] i2c_transfer+0x19/0xb0
*pdpt = 000000000af77001 *pde = 0000000000000000
Oops: 0000 [#1] PREEMPT
last sysfs file: /sys/devices/virtual/vtconsole/vtcon1/bind
Modules linked in: viafb(+) fbcon font bitblit softcursor fb 
i2c_algo_bit cfbcopyarea cfbimgblt cfbfillrect snd_hda_codec_realtek 
snd_hda_intel snd_hda_codec snd_hwdep snd_pcm snd_timer mmc_block 
rtl8187 snd eeprom_93cx6 soundcore via_sdmmc snd_page_alloc i2c_viapro 
ehci_hcd uhci_hcd mmc_core video output [last unloaded: viafb]

Pid: 3561, comm: insmod Tainted: G        W  2.6.34-rc5 #1 IL1/
EIP: 0060:[<c11df2db>] EFLAGS: 00010296 CPU: 0
EIP is at i2c_transfer+0x19/0xb0
EAX: 00000000 EBX: ffffffa1 ECX: 00000002 EDX: c74f2d68
ESI: db034294 EDI: c74f2d96 EBP: c74f2d60 ESP: c74f2d48
  DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 0068
Process insmod (pid: 3561, ti=c74f2000 task=db022000 task.ti=c74f2000)
Stack:
  00000002 c74f2d68 c74f2d97 c74f2d96 c74f2d97 c74f2d96 c74f2d88 dc7308f0
<0> 00000040 c74f0001 c74f2d83 00010040 c74f0001 c74f2d96 00000001 00000000
<0> c74f2da4 dc733a7f c74f2d96 00002db0 dc738a28 db0349a8 c74f2e84 c74f2db0
Call Trace:
  [<dc7308f0>] ? viafb_i2c_readbyte+0x60/0x68 [viafb]
  [<dc733a7f>] ? viafb_lvds_identify_vt1636+0x38/0xbc [viafb]
  [<dc732a7f>] ? viafb_lvds_trasmitter_identify+0x2c/0x10d [viafb]
  [<dc7304cc>] ? viafb_init_chip_info+0x15b/0x23f [viafb]
  [<dc7340c3>] ? via_pci_probe+0x155/0x81c [viafb]
  [<c111f1c0>] ? idr_get_empty_slot+0x152/0x226
  [<c112e1ee>] ? pci_match_device+0xbf/0xca
  [<c112e09c>] ? local_pci_probe+0xe/0x10
  [<c112e71a>] ? pci_device_probe+0x43/0x66
  [<c1186374>] ? driver_probe_device+0x78/0x103
  [<c1186442>] ? __driver_attach+0x43/0x5f
  [<c1185d79>] ? bus_for_each_dev+0x3d/0x67
  [<c118624e>] ? driver_attach+0x14/0x16
  [<c11863ff>] ? __driver_attach+0x0/0x5f
  [<c11857f3>] ? bus_add_driver+0xa2/0x1d4
  [<c1186687>] ? driver_register+0x8b/0xeb
  [<c112e8fe>] ? __pci_register_driver+0x31/0x8a
  [<dc6f7000>] ? viafb_init+0x0/0x297 [viafb]
  [<dc6f7288>] ? viafb_init+0x288/0x297 [viafb]
  [<c1001133>] ? do_one_initcall+0x4b/0x130
  [<c1041ae9>] ? sys_init_module+0xa7/0x1de
  [<c1002750>] ? sysenter_do_call+0x12/0x26
Code: 8d 55 f8 89 4d fc b9 af f0 1d c1 e8 47 4c fa ff c9 c3 55 89 e5 57 
56 89 c6 53 bb a1 ff ff ff 83 ec 0c 89 55 ec 89 4d e8 8b 40 0c <83> 38 
00 0f 84 84 00 00 00 89 e0 25 00 f0 ff ff 8b 0d 68 5c 3a
EIP: [<c11df2db>] i2c_transfer+0x19/0xb0 SS:ESP 0068:c74f2d48
CR2: 0000000000000000

as you can see it is caused in viafb_lvds_trasmitter_identify by
-		viaparinfo->shared->i2c_stuff.i2c_port = GPIOPORTINDEX;
-		if (viafb_lvds_identify_vt1636()) {
+		if (viafb_lvds_identify_vt1636(VIA_I2C_ADAP_26)) {
which should rather be
-		viaparinfo->shared->i2c_stuff.i2c_port = GPIOPORTINDEX;
-		if (viafb_lvds_identify_vt1636()) {
+		if (viafb_lvds_identify_vt1636(VIA_I2C_ADAP_2C)) {
after which your patches including this work fine (on VX800 and CLE266).


Thanks,

Florian Tobias Schandinat

> viafb: Only establish i2c busses on ports that always had them
> 
> ...otherwise it seems we run into conflicts with shadowy other users which
> don't expect to see i2c taking control of ports it never used to do
> anything with.
> 
> Reported-by: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
> Signed-off-by: Jonathan Corbet <corbet@lwn.net>
> ---
>  drivers/video/via/via_i2c.c |   19 +++++++++++++------
>  drivers/video/via/via_i2c.h |    1 +
>  2 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/video/via/via_i2c.c b/drivers/video/via/via_i2c.c
> index fe5535c..b5253e3 100644
> --- a/drivers/video/via/via_i2c.c
> +++ b/drivers/video/via/via_i2c.c
> @@ -170,13 +170,18 @@ static int create_i2c_bus(struct i2c_adapter *adapter,
>  	return i2c_bit_add_bus(adapter);
>  }
>  
> +/*
> + * By default, we only activate busses on ports 2c and 31 to avoid
> + * conflicts with other possible users; that default can be changed
> + * below.
> + */
>  static struct via_i2c_adap_cfg adap_configs[] = {
> -	[VIA_I2C_ADAP_26]	= { VIA_I2C_I2C,  VIASR, 0x26 },
> -	[VIA_I2C_ADAP_31]	= { VIA_I2C_I2C,  VIASR, 0x31 },
> -	[VIA_I2C_ADAP_25]	= { VIA_I2C_GPIO, VIASR, 0x25 },
> -	[VIA_I2C_ADAP_2C]	= { VIA_I2C_GPIO, VIASR, 0x2c },
> -	[VIA_I2C_ADAP_3D]	= { VIA_I2C_GPIO, VIASR, 0x3d },
> -	{ 0, 0, 0 }
> +	[VIA_I2C_ADAP_26]	= { VIA_I2C_I2C,  VIASR, 0x26, 0 },
> +	[VIA_I2C_ADAP_31]	= { VIA_I2C_I2C,  VIASR, 0x31, 1 },
> +	[VIA_I2C_ADAP_25]	= { VIA_I2C_GPIO, VIASR, 0x25, 0 },
> +	[VIA_I2C_ADAP_2C]	= { VIA_I2C_GPIO, VIASR, 0x2c, 1 },
> +	[VIA_I2C_ADAP_3D]	= { VIA_I2C_GPIO, VIASR, 0x3d, 0 },
> +	{ 0, 0, 0, 0 }
>  };
>  
>  int viafb_create_i2c_busses(struct viafb_par *viapar)
> @@ -189,6 +194,8 @@ int viafb_create_i2c_busses(struct viafb_par *viapar)
>  
>  		if (adap_cfg->type == 0)
>  			break;
> +		if (!adap_cfg->is_active)
> +			continue;
>  
>  		ret = create_i2c_bus(&i2c_stuff->adapter,
>  				     &i2c_stuff->algo, adap_cfg,
> diff --git a/drivers/video/via/via_i2c.h b/drivers/video/via/via_i2c.h
> index 00ed978..73d682f 100644
> --- a/drivers/video/via/via_i2c.h
> +++ b/drivers/video/via/via_i2c.h
> @@ -35,6 +35,7 @@ struct via_i2c_adap_cfg {
>  	enum via_i2c_type	type;
>  	u_int16_t		io_port;
>  	u_int8_t		ioport_index;
> +	u8			is_active;
>  };
>  
>  struct via_i2c_stuff {

Re: [PATCH 10/11] viafb: rework the I2C support in the VIA framebuffer driver

[Jonathan Corbet]

On Sun, 25 Apr 2010 17:56:09 +0200
Florian Tobias Schandinat <FlorianSchandinat@gmx.de> wrote:

> +		if (viafb_lvds_identify_vt1636(VIA_I2C_ADAP_26)) {
> which should rather be
> -		viaparinfo->shared->i2c_stuff.i2c_port = GPIOPORTINDEX;
> -		if (viafb_lvds_identify_vt1636()) {
> +		if (viafb_lvds_identify_vt1636(VIA_I2C_ADAP_2C)) {
> after which your patches including this work fine (on VX800 and CLE266).

Two bugs, actually...I wouldn't expect things to work with the wrong
port, but neither should it crash.  The crash is fixed in the second
series (where I added checks to ensure that the adapter was active) so
I'll probably leave it as-is in between.  The port number needs to be
fixed, though, thanks for catching that.

jon

[PATCH 11/11] suppress verbose debug messages: change printk() to DEBUG_MSG()

[Jonathan Corbet]

From: Paul Fox <pgf@laptop.org>

[jc: no signoff, added my own]
Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc: ScottFang@viatech.com.cn
Cc: JosephChan@via.com.tw
Signed-off-by: Jonathan Corbet <corbet@lwn.net>
---
 drivers/video/via/via_i2c.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/via/via_i2c.c b/drivers/video/via/via_i2c.c
index 8f8e0bf..fe5535c 100644
--- a/drivers/video/via/via_i2c.c
+++ b/drivers/video/via/via_i2c.c
@@ -26,7 +26,7 @@ static void via_i2c_setscl(void *data, int state)
 	u8 val;
 	struct via_i2c_adap_cfg *adap_data = data;
 
-	printk(KERN_DEBUG "reading index 0x%02x from IO 0x%x\n",
+	DEBUG_MSG(KERN_DEBUG "reading index 0x%02x from IO 0x%x\n",
 		adap_data->ioport_index, adap_data->io_port);
 	val = viafb_read_reg(adap_data->io_port,
 			     adap_data->ioport_index) & 0xF0;
@@ -140,7 +140,7 @@ static int create_i2c_bus(struct i2c_adapter *adapter,
 			  struct via_i2c_adap_cfg *adap_cfg,
 			  struct pci_dev *pdev)
 {
-	printk(KERN_DEBUG "viafb: creating bus adap=0x%p, algo_bit_data=0x%p, adap_cfg=0x%p\n", adapter, algo, adap_cfg);
+	DEBUG_MSG(KERN_DEBUG "viafb: creating bus adap=0x%p, algo_bit_data=0x%p, adap_cfg=0x%p\n", adapter, algo, adap_cfg);
 
 	algo->setsda = via_i2c_setsda;
 	algo->setscl = via_i2c_setscl;
-- 
1.7.0.1