Re: [PATCH 10/11] viafb: rework the I2C support in the VIA framebufferdriver
From: Florian Tobias Schandinat
Date: Sun Apr 25 2010 - 11:56:25 EST
Hi Jon,
Jonathan Corbet schrieb:
On Sat, 24 Apr 2010 15:53:16 +0200
Harald Welte <laforge@xxxxxxxxxxxx> 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@xxxxxx>
Signed-off-by: Jonathan Corbet <corbet@xxxxxxx>
---
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 {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/