| Message ID | Pine.LNX.4.64.0904241832120.8309@axis700.grange (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
On Mon, 27 Apr 2009, Eric Miao wrote: > It looks to me the change to the platform code is to move the I2C board > info registration into 'struct soc_camera_link'. Are there any specific > reason to do so? I'm assuming the original code works equally well, > and lists all the i2c devices in a central place is straight forward. Yes, there are reasons. The first one is, that many i2c camera sensor chips switch on their I2C interface only when the camera interface master clock is running. Therefore you cannot even probe the chip before the host video part has been initialised. So if you load the sensor driver before the host camera driver you cannot probe. The current mainline framework makes the "first-stage" sensor probe a NOP, in it the sensor driver just registers itself with the soc-camera framework and returns success. And only when the soc-camera finds a match of a sensor and a host driver it requests the host to activate the interface (switch on the master clock) and then the second stage sensor probing is called, which now can actually read chip version registers etc. The second reason is that this is also how v4l2-subdev framework works - there your camera host driver (in our case soc-camera core) uses its internal knowledge about present I2C devices (in our case it uses platform devices with referenced there i2c board info) to register new I2C devices dynamically and load their drivers, at which point we have already switched the master clock on so the sensor driver can just probe the chip normally. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Apr 27, 2009 at 5:55 PM, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > On Mon, 27 Apr 2009, Eric Miao wrote: > >> It looks to me the change to the platform code is to move the I2C board >> info registration into 'struct soc_camera_link'. Are there any specific >> reason to do so? I'm assuming the original code works equally well, >> and lists all the i2c devices in a central place is straight forward. > > Yes, there are reasons. The first one is, that many i2c camera sensor > chips switch on their I2C interface only when the camera interface master > clock is running. Therefore you cannot even probe the chip before the host > video part has been initialised. So if you load the sensor driver before > the host camera driver you cannot probe. The current mainline framework > makes the "first-stage" sensor probe a NOP, in it the sensor driver just > registers itself with the soc-camera framework and returns success. Yes, indeed. The driver has to be carefully written so that no actual I2C access is made during probe, but this makes no sense. One workaround to this is to separate the logic of clock enabling from the camera controller and make a 'struct clk' for it. However, this sounds to be tricky as well. > And > only when the soc-camera finds a match of a sensor and a host driver it > requests the host to activate the interface (switch on the master clock) > and then the second stage sensor probing is called, which now can actually > read chip version registers etc. The second reason is that this is also > how v4l2-subdev framework works - there your camera host driver (in our > case soc-camera core) uses its internal knowledge about present I2C > devices (in our case it uses platform devices with referenced there i2c > board info) to register new I2C devices dynamically and load their > drivers, at which point we have already switched the master clock on so > the sensor driver can just probe the chip normally. > Sounds reasonable. I'm then OK with these 3 patches. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/mach-pxa/pcm990-baseboard.c b/arch/arm/mach-pxa/pcm990-baseboard.c index 9ce1ef2..a1ae436 100644 --- a/arch/arm/mach-pxa/pcm990-baseboard.c +++ b/arch/arm/mach-pxa/pcm990-baseboard.c @@ -427,25 +427,56 @@ static void pcm990_camera_free_bus(struct soc_camera_link *link) gpio_bus_switch = -EINVAL; } -static struct soc_camera_link iclink = { - .bus_id = 0, /* Must match with the camera ID above */ - .query_bus_param = pcm990_camera_query_bus_param, - .set_bus_param = pcm990_camera_set_bus_param, - .free_bus = pcm990_camera_free_bus, -}; - /* Board I2C devices. */ static struct i2c_board_info __initdata pcm990_i2c_devices[] = { { /* Must initialize before the camera(s) */ I2C_BOARD_INFO("pca9536", 0x41), .platform_data = &pca9536_data, - }, { + }, +}; + +static struct i2c_board_info __initdata pcm990_camera_i2c[] = { + { I2C_BOARD_INFO("mt9v022", 0x48), - .platform_data = &iclink, /* With extender */ }, { I2C_BOARD_INFO("mt9m001", 0x5d), - .platform_data = &iclink, /* With extender */ + }, +}; + +static struct soc_camera_link iclink[] = { + { + .bus_id = 0, /* Must match with the camera ID */ + .board_info = &pcm990_camera_i2c[0], + .i2c_adapter_id = 0, + .query_bus_param = pcm990_camera_query_bus_param, + .set_bus_param = pcm990_camera_set_bus_param, + .free_bus = pcm990_camera_free_bus, + .module_name = "mt9v022", + }, { + .bus_id = 0, /* Must match with the camera ID */ + .board_info = &pcm990_camera_i2c[1], + .i2c_adapter_id = 0, + .query_bus_param = pcm990_camera_query_bus_param, + .set_bus_param = pcm990_camera_set_bus_param, + .free_bus = pcm990_camera_free_bus, + .module_name = "mt9m001", + }, +}; + +static struct platform_device pcm990_camera[] = { + { + .name = "soc-camera-pdrv", + .id = 0, + .dev = { + .platform_data = &iclink[0], + }, + }, { + .name = "soc-camera-pdrv", + .id = 1, + .dev = { + .platform_data = &iclink[1], + }, }, }; #endif /* CONFIG_VIDEO_PXA27x ||CONFIG_VIDEO_PXA27x_MODULE */ @@ -501,6 +532,9 @@ void __init pcm990_baseboard_init(void) pxa_set_camera_info(&pcm990_pxacamera_platform_data); i2c_register_board_info(0, ARRAY_AND_SIZE(pcm990_i2c_devices)); + + platform_device_register(&pcm990_camera[0]); + platform_device_register(&pcm990_camera[1]); #endif printk(KERN_INFO "PCM-990 Evaluation baseboard initialized\n");
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de> --- For review __ONLY__ for now - will re-submit after I have pushed 1/8 arch/arm/mach-pxa/pcm990-baseboard.c | 54 +++++++++++++++++++++++++++------ 1 files changed, 44 insertions(+), 10 deletions(-)