[PATCH 3/9] em28xx: rename em28xx_hint_sensor() to em28xx_detect_sensor()
Now that the board hints and the sensor initialization/configuration have been separated, em28xx_detect_sensor() is the better name for this function. Signed-off-by: Frank Schäfer fschaefer@googlemail.com --- drivers/media/usb/em28xx/em28xx-cards.c |7 +++ 1 Datei geändert, 3 Zeilen hinzugefügt(+), 4 Zeilen entfernt(-) diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c index c8ad7e5..7bb760e 100644 --- a/drivers/media/usb/em28xx/em28xx-cards.c +++ b/drivers/media/usb/em28xx/em28xx-cards.c @@ -2309,11 +2309,10 @@ static int em28xx_initialize_mt9m001(struct em28xx *dev) return 0; } -/* HINT method: webcam I2C chips - * +/* * This method works for webcams with Micron sensors */ -static int em28xx_hint_sensor(struct em28xx *dev) +static int em28xx_detect_sensor(struct em28xx *dev) { int rc; char *sensor_name; @@ -2746,7 +2745,7 @@ static void em28xx_card_setup(struct em28xx *dev) * If sensor is not found, then it isn't a webcam. */ if (dev-board.is_webcam) { - if (em28xx_hint_sensor(dev) 0) + if (em28xx_detect_sensor(dev) 0) dev-board.is_webcam = 0; else dev-progressive = 1; -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/9] em28xx: move sensor code to a separate source code file em28xx-camera.c
em28xx-cards.c is very large and the sensor/camera related code is growing, so move this code to a separate source code file em28xx-camera.c. Signed-off-by: Frank Schäfer fschaefer@googlemail.com --- drivers/media/usb/em28xx/Makefile|2 +- drivers/media/usb/em28xx/em28xx-camera.c | 189 ++ drivers/media/usb/em28xx/em28xx-cards.c | 160 - drivers/media/usb/em28xx/em28xx.h|4 + 4 Dateien geändert, 194 Zeilen hinzugefügt(+), 161 Zeilen entfernt(-) create mode 100644 drivers/media/usb/em28xx/em28xx-camera.c diff --git a/drivers/media/usb/em28xx/Makefile b/drivers/media/usb/em28xx/Makefile index 634fb92..ad6d485 100644 --- a/drivers/media/usb/em28xx/Makefile +++ b/drivers/media/usb/em28xx/Makefile @@ -1,5 +1,5 @@ em28xx-y +=em28xx-video.o em28xx-i2c.o em28xx-cards.o -em28xx-y +=em28xx-core.o em28xx-vbi.o +em28xx-y +=em28xx-core.o em28xx-vbi.o em28xx-camera.o em28xx-alsa-objs := em28xx-audio.o em28xx-rc-objs := em28xx-input.o diff --git a/drivers/media/usb/em28xx/em28xx-camera.c b/drivers/media/usb/em28xx/em28xx-camera.c new file mode 100644 index 000..28dd848 --- /dev/null +++ b/drivers/media/usb/em28xx/em28xx-camera.c @@ -0,0 +1,189 @@ +/* + em28xx-camera.c - driver for Empia EM25xx/27xx/28xx USB video capture devices + + Copyright (C) 2009 Mauro Carvalho Chehab mche...@infradead.org + Copyright (C) 2013 Frank Schäfer fschaefer@googlemail.com + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. +*/ + +#include linux/i2c.h +#include media/mt9v011.h +#include media/v4l2-common.h + +#include em28xx.h + + +/* FIXME: Should be replaced by a proper mt9m111 driver */ +static int em28xx_initialize_mt9m111(struct em28xx *dev) +{ + int i; + unsigned char regs[][3] = { + { 0x0d, 0x00, 0x01, }, /* reset and use defaults */ + { 0x0d, 0x00, 0x00, }, + { 0x0a, 0x00, 0x21, }, + { 0x21, 0x04, 0x00, }, /* full readout speed, no row/col skipping */ + }; + + for (i = 0; i ARRAY_SIZE(regs); i++) + i2c_master_send(dev-i2c_client[dev-def_i2c_bus], + regs[i][0], 3); + + return 0; +} + + +/* FIXME: Should be replaced by a proper mt9m001 driver */ +static int em28xx_initialize_mt9m001(struct em28xx *dev) +{ + int i; + unsigned char regs[][3] = { + { 0x0d, 0x00, 0x01, }, + { 0x0d, 0x00, 0x00, }, + { 0x04, 0x05, 0x00, }, /* hres = 1280 */ + { 0x03, 0x04, 0x00, }, /* vres = 1024 */ + { 0x20, 0x11, 0x00, }, + { 0x06, 0x00, 0x10, }, + { 0x2b, 0x00, 0x24, }, + { 0x2e, 0x00, 0x24, }, + { 0x35, 0x00, 0x24, }, + { 0x2d, 0x00, 0x20, }, + { 0x2c, 0x00, 0x20, }, + { 0x09, 0x0a, 0xd4, }, + { 0x35, 0x00, 0x57, }, + }; + + for (i = 0; i ARRAY_SIZE(regs); i++) + i2c_master_send(dev-i2c_client[dev-def_i2c_bus], + regs[i][0], 3); + + return 0; +} + + +/* + * This method works for webcams with Micron sensors + */ +int em28xx_detect_sensor(struct em28xx *dev) +{ + int ret; + char *name; + u8 reg; + __be16 id_be; + u16 id; + + /* Micron sensor detection */ + dev-i2c_client[dev-def_i2c_bus].addr = 0xba 1; + reg = 0; + i2c_master_send(dev-i2c_client[dev-def_i2c_bus], reg, 1); + ret = i2c_master_recv(dev-i2c_client[dev-def_i2c_bus], + (char *)id_be, 2); + if (ret != 2) + return -EINVAL; + + id = be16_to_cpu(id_be); + switch (id) { + case 0x8232:/* mt9v011 640x480 1.3 Mpix sensor */ + case 0x8243:/* mt9v011 rev B 640x480 1.3 Mpix sensor */ + name = mt9v011; + dev-em28xx_sensor = EM28XX_MT9V011; + break; + case 0x143a:/* MT9M111 as found in the ECS G200 */ + name = mt9m111; + dev-em28xx_sensor = EM28XX_MT9M111; + break; + case 0x8431: + name = mt9m001; + dev-em28xx_sensor = EM28XX_MT9M001; + break; + default: +
[PATCH 5/9] em28xx: detect further Micron sensors
Add further Micron chip IDs to be able to identify all Micron sensors listed by Empiatech. Also probe the two alternate i2c addresses used by Micron sensors with 8 bit address and 16 bit register width. Signed-off-by: Frank Schäfer fschaefer@googlemail.com --- drivers/media/usb/em28xx/em28xx-camera.c | 126 ++ 1 Datei geändert, 95 Zeilen hinzugefügt(+), 31 Zeilen entfernt(-) diff --git a/drivers/media/usb/em28xx/em28xx-camera.c b/drivers/media/usb/em28xx/em28xx-camera.c index 28dd848..2e4856a 100644 --- a/drivers/media/usb/em28xx/em28xx-camera.c +++ b/drivers/media/usb/em28xx/em28xx-camera.c @@ -26,6 +26,15 @@ #include em28xx.h +/* Possible i2c addresses of Micron sensors */ +static unsigned short micron_sensor_addrs[] = { + 0xb8 1, /* MT9V111, MT9V403 */ + 0xba 1, /* MT9M001/011/111/112, MT9V011/012/112, MT9D011 */ + 0x90 1, /* MT9V012/112, MT9D011 (alternative address) */ + I2C_CLIENT_END +}; + + /* FIXME: Should be replaced by a proper mt9m111 driver */ static int em28xx_initialize_mt9m111(struct em28xx *dev) { @@ -78,44 +87,99 @@ static int em28xx_initialize_mt9m001(struct em28xx *dev) */ int em28xx_detect_sensor(struct em28xx *dev) { - int ret; + int ret, i; char *name; u8 reg; __be16 id_be; u16 id; - /* Micron sensor detection */ - dev-i2c_client[dev-def_i2c_bus].addr = 0xba 1; - reg = 0; - i2c_master_send(dev-i2c_client[dev-def_i2c_bus], reg, 1); - ret = i2c_master_recv(dev-i2c_client[dev-def_i2c_bus], - (char *)id_be, 2); - if (ret != 2) - return -EINVAL; - - id = be16_to_cpu(id_be); - switch (id) { - case 0x8232:/* mt9v011 640x480 1.3 Mpix sensor */ - case 0x8243:/* mt9v011 rev B 640x480 1.3 Mpix sensor */ - name = mt9v011; - dev-em28xx_sensor = EM28XX_MT9V011; - break; - case 0x143a:/* MT9M111 as found in the ECS G200 */ - name = mt9m111; - dev-em28xx_sensor = EM28XX_MT9M111; - break; - case 0x8431: - name = mt9m001; - dev-em28xx_sensor = EM28XX_MT9M001; - break; - default: - em28xx_info(unknown Micron sensor detected: 0x%04x\n, id); - return -EINVAL; + struct i2c_client client = dev-i2c_client[dev-def_i2c_bus]; + + dev-em28xx_sensor = EM28XX_NOSENSOR; + /* Probe Micron sensors with 8 bit address and 16 bit register width */ + for (i = 0; micron_sensor_addrs[i] != I2C_CLIENT_END; i++) { + client.addr = micron_sensor_addrs[i]; + /* NOTE: i2c_smbus_read_word_data() doesn't work with BE data */ + /* Read chip ID from register 0x00 */ + reg = 0x00; + ret = i2c_master_send(client, reg, 1); + if (ret 0) { + if (ret != -ENODEV) + em28xx_errdev(couldn't read from i2c device 0x%02x: error %i\n, + client.addr 1, ret); + continue; + } + ret = i2c_master_recv(client, (u8 *)id_be, 2); + if (ret 0) { + em28xx_errdev(couldn't read from i2c device 0x%02x: error %i\n, + client.addr 1, ret); + continue; + } + id = be16_to_cpu(id_be); + /* Read chip ID from register 0xff */ + reg = 0xff; + ret = i2c_master_send(client, reg, 1); + if (ret 0) { + em28xx_errdev(couldn't read from i2c device 0x%02x: error %i\n, + client.addr 1, ret); + continue; + } + ret = i2c_master_recv(client, (u8 *)id_be, 2); + if (ret 0) { + em28xx_errdev(couldn't read from i2c device 0x%02x: error %i\n, + client.addr 1, ret); + continue; + } + /* Validate chip ID to be sure we have a Micron device */ + if (id != be16_to_cpu(id_be)) + continue; + /* Check chip ID */ + id = be16_to_cpu(id_be); + switch (id) { + case 0x1222: + name = MT9V012; /* MI370 */ /* 640x480 */ + break; + case 0x1229: + name = MT9V112; /* 640x480 */ + break; + case 0x1433: + name = MT9M011; /* 1280x1024 */ + break; + case 0x143a:/* found in the ECS G200 */ + name = MT9M111; /* MI1310 */ /* 1280x1024 */ +
[PATCH 6/9] em28xx: move the probing of Micron sensors to a separate function
Other sensors like the ones from OmniVision need a different probing procedure, so it makes sense have separate functions for each manufacturer/sensor type. Signed-off-by: Frank Schäfer fschaefer@googlemail.com --- drivers/media/usb/em28xx/em28xx-camera.c | 23 +++ 1 Datei geändert, 19 Zeilen hinzugefügt(+), 4 Zeilen entfernt(-) diff --git a/drivers/media/usb/em28xx/em28xx-camera.c b/drivers/media/usb/em28xx/em28xx-camera.c index 2e4856a..d744af6 100644 --- a/drivers/media/usb/em28xx/em28xx-camera.c +++ b/drivers/media/usb/em28xx/em28xx-camera.c @@ -83,9 +83,9 @@ static int em28xx_initialize_mt9m001(struct em28xx *dev) /* - * This method works for webcams with Micron sensors + * Probes Micron sensors with 8 bit address and 16 bit register width */ -int em28xx_detect_sensor(struct em28xx *dev) +static int em28xx_probe_sensor_micron(struct em28xx *dev) { int ret, i; char *name; @@ -96,7 +96,6 @@ int em28xx_detect_sensor(struct em28xx *dev) struct i2c_client client = dev-i2c_client[dev-def_i2c_bus]; dev-em28xx_sensor = EM28XX_NOSENSOR; - /* Probe Micron sensors with 8 bit address and 16 bit register width */ for (i = 0; micron_sensor_addrs[i] != I2C_CLIENT_END; i++) { client.addr = micron_sensor_addrs[i]; /* NOTE: i2c_smbus_read_word_data() doesn't work with BE data */ @@ -167,7 +166,7 @@ int em28xx_detect_sensor(struct em28xx *dev) default: em28xx_info(unknown Micron sensor detected: 0x%04x\n, id); - return -EINVAL; + return 0; } if (dev-em28xx_sensor == EM28XX_NOSENSOR) @@ -182,6 +181,22 @@ int em28xx_detect_sensor(struct em28xx *dev) return -ENODEV; } +/* + * This method works for webcams with Micron sensors + */ +int em28xx_detect_sensor(struct em28xx *dev) +{ + int ret; + + ret = em28xx_probe_sensor_micron(dev); + if (dev-em28xx_sensor == EM28XX_NOSENSOR ret 0) { + em28xx_info(No sensor detected\n); + return -ENODEV; + } + + return 0; +} + int em28xx_init_camera(struct em28xx *dev) { switch (dev-em28xx_sensor) { -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/9] em28xx: add probing procedure for OmniVision sensors
OmniVision sensors are used as well in Empiatech based cameras such as the SpeedLink Vicious And Devine Laplace webcam (EM2765 + Omnivision OV2640). With this patch applied, OminiVision sensors with 8 bit address and register width are detected (recent models have a 16 bit address width and use different client addresses). The most commonly used sensors (including the ones listed by Empiatech) are detected properly, although there is no support for them yet. Signed-off-by: Frank Schäfer fschaefer@googlemail.com --- drivers/media/usb/em28xx/em28xx-camera.c | 114 +- 1 Datei geändert, 113 Zeilen hinzugefügt(+), 1 Zeile entfernt(-) diff --git a/drivers/media/usb/em28xx/em28xx-camera.c b/drivers/media/usb/em28xx/em28xx-camera.c index d744af6..e8b3322 100644 --- a/drivers/media/usb/em28xx/em28xx-camera.c +++ b/drivers/media/usb/em28xx/em28xx-camera.c @@ -34,6 +34,13 @@ static unsigned short micron_sensor_addrs[] = { I2C_CLIENT_END }; +/* Possible i2c addresses of Omnivision sensors */ +static unsigned short omnivision_sensor_addrs[] = { + 0x42 1, /* OV7725, OV7670/60/48 */ + 0x60 1, /* OV2640, OV9650/53/55 */ + I2C_CLIENT_END +}; + /* FIXME: Should be replaced by a proper mt9m111 driver */ static int em28xx_initialize_mt9m111(struct em28xx *dev) @@ -182,13 +189,118 @@ static int em28xx_probe_sensor_micron(struct em28xx *dev) } /* - * This method works for webcams with Micron sensors + * Probes Omnivision sensors with 8 bit address and register width */ +static int em28xx_probe_sensor_omnivision(struct em28xx *dev) +{ + int ret, i; + char *name; + u8 reg; + u16 id; + struct i2c_client client = dev-i2c_client[dev-def_i2c_bus]; + + dev-em28xx_sensor = EM28XX_NOSENSOR; + /* NOTE: these devices have the register auto incrementation disabled +* by default, so we have to use single byte reads ! */ + for (i = 0; omnivision_sensor_addrs[i] != I2C_CLIENT_END; i++) { + client.addr = omnivision_sensor_addrs[i]; + /* Read manufacturer ID from registers 0x1c-0x1d (BE) */ + reg = 0x1c; + ret = i2c_smbus_read_byte_data(client, reg); + if (ret 0) { + if (ret != -ENODEV) + em28xx_errdev(couldn't read from i2c device 0x%02x: error %i\n, + client.addr 1, ret); + continue; + } + id = ret 8; + reg = 0x1d; + ret = i2c_smbus_read_byte_data(client, reg); + if (ret 0) { + em28xx_errdev(couldn't read from i2c device 0x%02x: error %i\n, + client.addr 1, ret); + continue; + } + id += ret; + /* Check manufacturer ID */ + if (id != 0x7fa2) + continue; + /* Read product ID from registers 0x0a-0x0b (BE) */ + reg = 0x0a; + ret = i2c_smbus_read_byte_data(client, reg); + if (ret 0) { + em28xx_errdev(couldn't read from i2c device 0x%02x: error %i\n, + client.addr 1, ret); + continue; + } + id = ret 8; + reg = 0x0b; + ret = i2c_smbus_read_byte_data(client, reg); + if (ret 0) { + em28xx_errdev(couldn't read from i2c device 0x%02x: error %i\n, + client.addr 1, ret); + continue; + } + id += ret; + /* Check product ID */ + switch (id) { + case 0x2642: + name = OV2640; + break; + case 0x7648: + name = OV7648; + break; + case 0x7660: + name = OV7660; + break; + case 0x7673: + name = OV7670; + break; + case 0x7720: + name = OV7720; + break; + case 0x7721: + name = OV7725; + break; + case 0x9648: /* Rev 2 */ + case 0x9649: /* Rev 3 */ + name = OV9640; + break; + case 0x9650: + case 0x9652: /* OV9653 */ + name = OV9650; + break; + case 0x9656: /* Rev 4 */ + case 0x9657: /* Rev 5 */ + name = OV9655; + break; + default: + em28xx_info(unknown OmniVision sensor
[PATCH 8/9] em28xx: add comment about Samsung and Kodak sensor probing addresses
The Windows driver also probes at least two further i2c addresses (0x22 1 and 0x66 1). I've got some hints that they are very likely used by Samsung and Kodak sensors, which are known to be used in Empia devices, too. We havn't seen any devices using these sensors yet and don't know how to probe them properly, so leave a comment. Signed-off-by: Frank Schäfer fschaefer@googlemail.com --- drivers/media/usb/em28xx/em28xx-camera.c |5 + 1 Datei geändert, 5 Zeilen hinzugefügt(+) diff --git a/drivers/media/usb/em28xx/em28xx-camera.c b/drivers/media/usb/em28xx/em28xx-camera.c index e8b3322..64b70d4 100644 --- a/drivers/media/usb/em28xx/em28xx-camera.c +++ b/drivers/media/usb/em28xx/em28xx-camera.c @@ -301,6 +301,11 @@ int em28xx_detect_sensor(struct em28xx *dev) if (dev-em28xx_sensor == EM28XX_NOSENSOR ret 0) ret = em28xx_probe_sensor_omnivision(dev); + /* +* NOTE: the Windows driver also probes i2c addresses +* 0x22 (Samsung ?) and 0x66 (Kodak ?) +*/ + if (dev-em28xx_sensor == EM28XX_NOSENSOR ret 0) { em28xx_info(No sensor detected\n); return -ENODEV; -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 9/9] em28xx: add basic support for OmniVision OV2640 sensors
This sensor is used by the SpeedLink Vicious And Devine Laplace webcam and others. It supports resolutions up to 1600x1200 (at 7-8 fps), but for resolutions higher than 640x480, further driver changes will be necessary, such as sensor output resolution switching (including further configuration changes), bridge xclk adjustment and disabling of 16 bit (12 bit) output formats at high resolutions. Image quality should also needs to be improved. Signed-off-by: Frank Schäfer fschaefer@googlemail.com --- drivers/media/usb/em28xx/em28xx-camera.c | 49 ++ drivers/media/usb/em28xx/em28xx.h|1 + 2 Dateien geändert, 50 Zeilen hinzugefügt(+) diff --git a/drivers/media/usb/em28xx/em28xx-camera.c b/drivers/media/usb/em28xx/em28xx-camera.c index 64b70d4..73cc50a 100644 --- a/drivers/media/usb/em28xx/em28xx-camera.c +++ b/drivers/media/usb/em28xx/em28xx-camera.c @@ -20,6 +20,7 @@ */ #include linux/i2c.h +#include media/soc_camera.h #include media/mt9v011.h #include media/v4l2-common.h @@ -42,6 +43,13 @@ static unsigned short omnivision_sensor_addrs[] = { }; +static struct soc_camera_link camlink = { + .bus_id = 0, + .flags = 0, + .module_name = em28xx, +}; + + /* FIXME: Should be replaced by a proper mt9m111 driver */ static int em28xx_initialize_mt9m111(struct em28xx *dev) { @@ -246,6 +254,7 @@ static int em28xx_probe_sensor_omnivision(struct em28xx *dev) switch (id) { case 0x2642: name = OV2640; + dev-em28xx_sensor = EM28XX_OV2640; break; case 0x7648: name = OV7648; @@ -376,6 +385,46 @@ int em28xx_init_camera(struct em28xx *dev) dev-vinctl = 0x00; break; + case EM28XX_OV2640: + { + struct v4l2_subdev *subdev; + struct i2c_board_info ov2640_info = { + .type = ov2640, + .flags = I2C_CLIENT_SCCB, + .addr = dev-i2c_client[dev-def_i2c_bus].addr, + .platform_data = camlink, + }; + struct v4l2_mbus_framefmt fmt; + + /* +* FIXME: sensor supports resolutions up to 1600x1200, but +* resolution setting/switching needs to be modified to +* - switch sensor output resolution (including further +* configuration changes) +* - adjust bridge xclk +* - disable 16 bit (12 bit) output formats on high resolutions +*/ + dev-sensor_xres = 640; + dev-sensor_yres = 480; + + subdev = +v4l2_i2c_new_subdev_board(dev-v4l2_dev, + dev-i2c_adap[dev-def_i2c_bus], + ov2640_info, NULL); + + fmt.code = V4L2_MBUS_FMT_YUYV8_2X8; + fmt.width = 640; + fmt.height = 480; + v4l2_subdev_call(subdev, video, s_mbus_fmt, fmt); + + /* NOTE: for UXGA=1600x1200 switch to 12MHz */ + dev-board.xclk = EM28XX_XCLK_FREQUENCY_24MHZ; + em28xx_write_reg(dev, EM28XX_R0F_XCLK, dev-board.xclk); + dev-vinmode = 0x08; + dev-vinctl = 0x00; + + break; + } case EM28XX_NOSENSOR: default: return -EINVAL; diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h index a14492f..a9323b6 100644 --- a/drivers/media/usb/em28xx/em28xx.h +++ b/drivers/media/usb/em28xx/em28xx.h @@ -364,6 +364,7 @@ enum em28xx_sensor { EM28XX_MT9V011, EM28XX_MT9M001, EM28XX_MT9M111, + EM28XX_OV2640, }; enum em28xx_adecoder { -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: VIDIOC_DBG_G_CHIP_NAME improvements
Hi Hans, On Wednesday 27 March 2013 11:54:34 Hans Verkuil wrote: Now that the VIDIOC_DBG_G_CHIP_NAME ioctl has been added to the v4l2 API I started work on removing the VIDIOC_DBG_G_CHIP_IDENT support in existing drivers. Based on that effort I realized that there are a few things that could be improved. One thing that Laurent pointed out is that this ioctl should be available only if CONFIG_VIDEO_ADV_DEBUG is set to prevent abuse by either userspace or kernelspace. I agree with that, especially since g_chip_ident is being abused today by some bridge drivers. That should be avoided in the future. I am also unhappy with the name. G_CHIP_INFO would certainly be more descriptive, but perhaps we should move a bit more into the direction of the Media Controller and call it G_ENTITY_INFO. Opinions are welcome. We need such an ioctl to retrieve extended informations about entities (it's been on my to-do list for too long), but I'd like to see it on the media device node. What surprised me when digging into the existing uses of G_CHIP_IDENT was that there are more devices than expected that have multiple register blocks. I.e. rather than a single set of registers they have multiple blocks of registers, say one block at address 0x1000, another at 0x2000, etc. Usually such register blocks represent IP blocks inside the chip, each doing a specific task. In other cases (adv7604) each block corresponds to an i2c address, each again representing an IP block inside the chip. In the case of adv7604 it has been implementing by mapping register offsets to specific i2c addresses, in the case of the cx231xx it has been implemented by exposing different bridge chips, unfortunately that's done in such a way that it can't be enumerated. The existing debug API has no support for discovering such ranges, but having worked with such a chip I think that having support for this is very desirable. As this is really a debug API, and applications (and users) need to know what they're doing, do we really need to make the ranges discoverable ? If you don't know what ranges a device supports you probably won't know enough to poke its registers directly anyway. Since we added a new ioctl anyway, I thought that this is a good time to extend it a bit and allow range discovery to be implemented: /** * struct v4l2_dbg_chip_name - VIDIOC_DBG_G_CHIP_NAME argument * @match: which chip to match * @flags: flags that tell whether this range is readable/writable * @name: unique name of the chip * @range_name: name of the register range * @range_min: minimum register of the register range * @range_max: maximum register of the register range * @reserved: future extensions */ struct v4l2_dbg_chip_name { struct v4l2_dbg_match match; __u32 range; __u32 flags; char name[32]; char range_name[32]; __u64 range_start; __u64 range_size; __u32 reserved[8]; } __attribute__ ((packed)); range is the range index, range_name describes the purpose of the register range, range_start and size are the start register address and the size of this register range. This extension allows you to enumerate the available register ranges for each device. If there is only one range, then range_size may be 0. This is mostly for backwards compatibility as otherwise I would have to modify all existing drivers for this, and also because this is not really necessary for simple devices with just one range. These are mostly i2c devices with start address 0 and a size of 256 bytes at most. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: omap3isp: possible circular locking dependency
Hi Enric, On Wednesday 27 March 2013 20:32:45 Enric Balletbo Serra wrote: Hi all, I've a problem running OMAP3 ISP with current 3.9-rc4. I tried to run the Laurent's live application to capture data from my mt9v034 sensor but kernel shows a possible circular locking dependency. Also the captured images are wrong and I see garbage. The same environment worked for me with kernel 3.7. Anyone knows any issue related to this ? Anyone experimented something similar with other sensors ? I tried to find something in ML and Laurent's git repository but I don't see anything. Thanks in advance. Here is the log: ~# live No compatible input device found, disabling digital zoom 32 bpp Device /dev/video7 opened: omap_vout (). /dev/video7: 3 buffers requested. /dev/video7: buffer 0 mapped at address 0xb6df1000. /dev/video7: buffer 1 mapped at address 0xb6c71000. /dev/video7: buffer 2 mapped at address 0xb6af1000. Device /dev/video6 opened: OMAP3 ISP resizer output (media). viewfinder configured for 2011 1024x768 /dev/video6: 3 buffers requested. /dev/video6: buffer 0 valid. /dev/video6: buffer 1 valid. /dev/video6: buffer 2 valid. Device /dev/video6 opened: OMAP3 ISP resizer output (media). /dev/video6: 2 buffers requested. /dev/video6: buffer 0 mapped at address 0xb64f1000. /dev/video6: buffer 1 mapped at address 0xb5ef1000. snapshot configured for 2011 2048x1536 Device /[ 63.557861] [ 63.560577] == [ 63.567077] [ INFO: possible circular locking dependency detected ] [ 63.573669] 3.9.0-rc4-00152-gba9ce12 #4 Not tainted [ 63.578796] --- [ 63.585388] live/1273 is trying to acquire lock: [ 63.590209] (mm-mmap_sem){++}, at: [bf06fb24] omap3isp_video_queue_qbuf+0x280/0x7a4 [omap3_isp] [ 63.600280] [ 63.600280] but task is already holding lock: [ 63.606414] (queue-lock){+.+.+.}, at: [bf06f8d4] omap3isp_video_queue_qbuf+0x30/0x7a4 [omap3_isp] [ 63.616271] [ 63.616271] which lock already depends on the new lock. [ 63.616271] [ 63.624877] [ 63.624877] the existing dependency chain (in reverse order) is: [ 63.632751] - #1 (queue-lock){+.+.+.}: [ 63.637207][c0081948] lock_acquire+0x94/0x100 [ 63.642700][c04f02b0] mutex_lock_nested+0x40/0x298 [ 63.648681][bf0703a0] omap3isp_video_queue_mmap+0x1c/0xe8 [omap3_isp] [ 63.656372][bf02117c] v4l2_mmap+0x54/0x88 [videodev] [ 63.662597][c00dc740] mmap_region+0x2e0/0x520 [ 63.668090][c00dcc38] do_mmap_pgoff+0x2b8/0x340 [ 63.673767][c00cdd1c] vm_mmap_pgoff+0x84/0xac [ 63.679260][c00db4a8] sys_mmap_pgoff+0x54/0xb0 [ 63.684875][c0013660] ret_fast_syscall+0x0/0x3c [ 63.690551] - #0 (mm-mmap_sem){++}: [ 63.695068][c0080e28] __lock_acquire+0x14bc/0x1ae8 [ 63.701019][c0081948] lock_acquire+0x94/0x100 [ 63.706512][c04f095c] down_read+0x30/0x40 [ 63.711639][bf06fb24] omap3isp_video_queue_qbuf+0x280/0x7a4 [omap3_isp] [ 63.719543][bf025ca4] v4l_qbuf+0x3c/0x40 [videodev] [ 63.725646][bf024ba8] __video_do_ioctl+0x240/0x33c [videodev] [ 63.732635][bf025668] video_usercopy+0x114/0x40c [videodev] [ 63.739440][bf0215c0] v4l2_ioctl+0xfc/0x144 [videodev] [ 63.745758][c00fe954] do_vfs_ioctl+0x7c/0x5ac [ 63.751281][c00feee8] sys_ioctl+0x64/0x84 [ 63.756408][c0013660] ret_fast_syscall+0x0/0x3c [ 63.762084] [ 63.762084] other info that might help us debug this: [ 63.762084] [ 63.770507] Possible unsafe locking scenario: [ 63.770507] [ 63.776702]CPU0CPU1 [ 63.781463] [ 63.786224] lock(queue-lock); [ 63.789733]lock(mm-mmap_sem); [ 63.795959]lock(queue-lock); [ 63.802124] lock(mm-mmap_sem); [ 63.805694] [ 63.805694] *** DEADLOCK *** That's normally a false positive. The two code paths are taken on different queues, with different queue-lock. It's pretty annoying nonetheless. And it doesn't explain why you get garbage on the screen. Could you try to bisect the problem ? [ 63.805694] [ 63.811950] 1 lock held by live/1273: [ 63.815795] #0: (queue-lock){+.+.+.}, at: [bf06f8d4] omap3isp_video_queue_qbuf+0x30/0x7a4 [omap3_isp] [ 63.826141] [ 63.826141] stack backtrace: [ 63.830749] [c00196d4] (unwind_backtrace+0x0/0xf0) from [c04eb478] (print_circular_bug+0x25c/0x2a8) [ 63.840637] [c04eb478] (print_circular_bug+0x25c/0x2a8) from [c0080e28] (__lock_acquire+0x14bc/0x1ae8) [ 63.850769] [c0080e28] (__lock_acquire+0x14bc/0x1ae8) from [c0081948] (lock_acquire+0x94/0x100) [ 63.860290] [c0081948] (lock_acquire+0x94/0x100) from [c04f095c] (down_read+0x30/0x40) [ 63.869018] [c04f095c]
[PATCH v2] net: add ETH_P_802_3_MIN
Add a new constant ETH_P_802_3_MIN, the minimum ethernet type for an 802.3 frame. Frames with a lower value in the ethernet type field are Ethernet II. Also update all the users of this value that David Miller and I could find to use the new constant. Also correct a bug in util.c. The comparison with ETH_P_802_3_MIN should be = not . As suggested by Jesse Gross. Compile tested only. Cc: David Miller da...@davemloft.net Cc: Jesse Gross je...@nicira.com Cc: Karsten Keil i...@linux-pingi.de Cc: John W. Linville linvi...@tuxdriver.com Cc: Johannes Berg johan...@sipsolutions.net Cc: Bart De Schuymer bart.de.schuy...@pandora.be Cc: Stephen Hemminger step...@networkplumber.org Cc: Patrick McHardy ka...@trash.net Cc: Marcel Holtmann mar...@holtmann.org Cc: Gustavo Padovan gust...@padovan.org Cc: Johan Hedberg johan.hedb...@gmail.com Cc: linux-blueto...@vger.kernel.org Cc: netfilter-de...@vger.kernel.org Cc: bri...@lists.linux-foundation.org Cc: linux-wirel...@vger.kernel.org Cc: linux1394-de...@lists.sourceforge.net Cc: linux-media@vger.kernel.org Cc: net...@vger.kernel.org Cc: d...@openvswitch.org Acked-by: Mauro Carvalho Chehab mche...@redhat.com Acked-by: Stefan Richter stef...@s5r6.in-berlin.de Signed-off-by: Simon Horman ho...@verge.net.au --- v2 * Make updates to the following files as suggested by David Miller - drivers/media/dvb-core/dvb_net.c - drivers/net/wireless/ray_cs.c - net/bridge/netfilter/ebtables.c - include/linux/if_vlan.h - net/bluetooth/bnep/netdev.c - net/openvswitch/flow.c - net/mac80211/tx.c - net/wireless/util.c --- drivers/firewire/net.c |2 +- drivers/isdn/i4l/isdn_net.c |2 +- drivers/media/dvb-core/dvb_net.c | 10 +- drivers/net/ethernet/sun/niu.c |2 +- drivers/net/plip/plip.c |2 +- drivers/net/wireless/ray_cs.c|2 +- include/linux/if_vlan.h |2 +- include/uapi/linux/if_ether.h|3 +++ net/atm/lec.h|2 +- net/bluetooth/bnep/netdev.c |2 +- net/bridge/netfilter/ebtables.c |2 +- net/ethernet/eth.c |2 +- net/mac80211/tx.c|2 +- net/openvswitch/datapath.c |2 +- net/openvswitch/flow.c |6 +++--- net/wireless/util.c |2 +- 16 files changed, 24 insertions(+), 21 deletions(-) diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c index 5679633..4d56536 100644 --- a/drivers/firewire/net.c +++ b/drivers/firewire/net.c @@ -547,7 +547,7 @@ static int fwnet_finish_incoming_packet(struct net_device *net, if (memcmp(eth-h_dest, net-dev_addr, net-addr_len)) skb-pkt_type = PACKET_OTHERHOST; } - if (ntohs(eth-h_proto) = 1536) { + if (ntohs(eth-h_proto) = ETH_P_802_3_MIN) { protocol = eth-h_proto; } else { rawp = (u16 *)skb-data; diff --git a/drivers/isdn/i4l/isdn_net.c b/drivers/isdn/i4l/isdn_net.c index babc621..88d657d 100644 --- a/drivers/isdn/i4l/isdn_net.c +++ b/drivers/isdn/i4l/isdn_net.c @@ -1385,7 +1385,7 @@ isdn_net_type_trans(struct sk_buff *skb, struct net_device *dev) if (memcmp(eth-h_dest, dev-dev_addr, ETH_ALEN)) skb-pkt_type = PACKET_OTHERHOST; } - if (ntohs(eth-h_proto) = 1536) + if (ntohs(eth-h_proto) = ETH_P_802_3_MIN) return eth-h_proto; rawp = skb-data; diff --git a/drivers/media/dvb-core/dvb_net.c b/drivers/media/dvb-core/dvb_net.c index 44225b1..83a23af 100644 --- a/drivers/media/dvb-core/dvb_net.c +++ b/drivers/media/dvb-core/dvb_net.c @@ -185,7 +185,7 @@ static __be16 dvb_net_eth_type_trans(struct sk_buff *skb, skb-pkt_type=PACKET_MULTICAST; } - if (ntohs(eth-h_proto) = 1536) + if (ntohs(eth-h_proto) = ETH_P_802_3_MIN) return eth-h_proto; rawp = skb-data; @@ -228,9 +228,9 @@ static int ule_test_sndu( struct dvb_net_priv *p ) static int ule_bridged_sndu( struct dvb_net_priv *p ) { struct ethhdr *hdr = (struct ethhdr*) p-ule_next_hdr; - if(ntohs(hdr-h_proto) 1536) { + if(ntohs(hdr-h_proto) ETH_P_802_3_MIN) { int framelen = p-ule_sndu_len - ((p-ule_next_hdr+sizeof(struct ethhdr)) - p-ule_skb-data); - /* A frame Type 1536 for a bridged frame, introduces a LLC Length field. */ + /* A frame Type ETH_P_802_3_MIN for a bridged frame, introduces a LLC Length field. */ if(framelen != ntohs(hdr-h_proto)) { return -1; } @@ -320,7 +320,7 @@ static int handle_ule_extensions( struct dvb_net_priv *p ) (int) p-ule_sndu_type, l, total_ext_len); #endif - } while (p-ule_sndu_type 1536); + } while (p-ule_sndu_type ETH_P_802_3_MIN); return total_ext_len; } @@ -712,7 +712,7 @@
Re: [RFC 01/12] exynos-fimc-is: Adding device tree nodes
On Wed, Mar 27, 2013 at 7:17 PM, Sylwester Nawrocki s.nawro...@samsung.com wrote: On 03/27/2013 05:31 AM, Arun Kumar K wrote: On Wed, Mar 27, 2013 at 4:21 AM, Sylwester Nawrocki sylvester.nawro...@gmail.com wrote: On 03/26/2013 01:17 PM, Arun Kumar K wrote: [...] Only issue is with the context sharing. Right now you can see that the fimc-is context is shared between all the subdevs. As all of them are part of the same driver, it is possible. If sensor is made as an independent i2c device, a separate probe will be called for it. For ISP sensor subdev to work independently, it needs to call the fimc_is_pipeline_* calls for FW initialization and other configurations for which it needs the fimc-is main context. Now there is a workaround here by calling a get_context() macro in sensor's probe to get the fimc-is context. This will cause the same context to be shared and updated by 2 drivers though both are part of fimc-is. Is this acceptable? Or am I missing some other simple solution of implementing it in a better way? OK, thanks for the explanation. I can think of at least one possible way to get hold of the fimc-is context in the subdev. For instance, in subdev's .registered callback you get a pointer to struct v4l2_device, which is normally embedded in a top level driver's private data. Then with container_of() you could get hold of required data at the fimc-is driver. But as per current implementation, it is not the fimc-is driver that is registering the ISP subdevs. It will be registered from the media controller driver. So fimc-is context cannot be obtained by just using container_of(). But... to make the subdev drivers reuse possible subdevs should normally not be required to know the internals of a host driver they are registered to. And it looks a bit unusual to have fimc_pipeline_* calls in the sensor's operation handlers. fimc_pipeline_* I mentioned is not the media controller pipeline. In the fimc-is driver, all the subdevs just implement the interface part. All the core functionalities happen in fimc-is-pipeline.c and fimc-is-interface.c. Since these ISP subdevs (sensor, isp, scc, scp) are not independent devices, all are controlled by the ISP firmware whose configuration and interface is done from the fimc_is_pipeline_* and fimc_is_itf_* functions. So all the ISP subdevs including sensor need to call these functions. I thought that the subdevs could provide basic methods and it would be the top level media driver that would resolve any dependencies in calling required subdev ops, according to media graph configuration done by the user on /dev/media?. In case of ISP subdevs (isp, scc and scp), there is not much configuration that the media device can do. Only control possible is to turn on/off specific scaler DMA outputs which can be done via the video node ioctls. The role of media device here is mostly to convey the pipeline structure to the user. For eg. it is not possible to directly connect isp (sd) -- scp (sd). In the media controller pipeline1 implementation, we were planning to put immutable links between these subdevs. Is that acceptable? The media driver has a list of media entities (subdevices and video nodes) and I though it could coordinate any requests involving whole video/image processing pipeline originating from /dev/video ioctls/fops. So for instance if /dev/video in this pipeline is opened sensor (sd) - mipi-csis (sd) - fimc-lite (sd) - memory (/dev/video) it would call s_power operation on the above subdevs and additionally on e.g. the isp subdev (or any other we choose as a main subdev implementing the FIMC-IS slave interface). Then couldn't it be done that video node ioctls invoke pipeline operations, and the media device resolves any dependencies/calls order, as in case of the exynos4 driver ? On Exynos4 subdevs, it is well and good since all the subdevs are independent IPs. Here in ISP since the same IP can take one input and provide multiple outputs, we designed them as separate subdevs. So here we cannot make the subdevs independent of each other where only the sequence / dependencies is controlled from the media device. Regards Arun -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] net: add ETH_P_802_3_MIN
From: Simon Horman ho...@verge.net.au Date: Thu, 28 Mar 2013 13:38:25 +0900 Add a new constant ETH_P_802_3_MIN, the minimum ethernet type for an 802.3 frame. Frames with a lower value in the ethernet type field are Ethernet II. Also update all the users of this value that David Miller and I could find to use the new constant. Also correct a bug in util.c. The comparison with ETH_P_802_3_MIN should be = not . As suggested by Jesse Gross. Compile tested only. Acked-by: Mauro Carvalho Chehab mche...@redhat.com Acked-by: Stefan Richter stef...@s5r6.in-berlin.de Signed-off-by: Simon Horman ho...@verge.net.au Looks great, applied, thanks Simon. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] net: add ETH_P_802_3_MIN
On Thu, Mar 28, 2013 at 01:21:08AM -0400, David Miller wrote: From: Simon Horman ho...@verge.net.au Date: Thu, 28 Mar 2013 13:38:25 +0900 Add a new constant ETH_P_802_3_MIN, the minimum ethernet type for an 802.3 frame. Frames with a lower value in the ethernet type field are Ethernet II. Also update all the users of this value that David Miller and I could find to use the new constant. Also correct a bug in util.c. The comparison with ETH_P_802_3_MIN should be = not . As suggested by Jesse Gross. Compile tested only. Acked-by: Mauro Carvalho Chehab mche...@redhat.com Acked-by: Stefan Richter stef...@s5r6.in-berlin.de Signed-off-by: Simon Horman ho...@verge.net.au Looks great, applied, thanks Simon. Thanks! -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] dma-buf: Add debugfs support
Hi Laurent! Thanks for the review: On 27 March 2013 05:38, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: Hi Sumit, Thanks for the patch. On Monday 25 March 2013 16:50:46 Sumit Semwal wrote: Add debugfs support to make it easier to print debug information about the dma-buf buffers. Cc: Dave Airlie airl...@redhat.com [minor fixes on init and warning fix] Signed-off-by: Sumit Semwal sumit.sem...@linaro.org --- changes since v1: - fixes on init and warnings as reported and corrected by Dave Airlie. - add locking while walking attachment list - reported by Daniel Vetter. drivers/base/dma-buf.c | 162 include/linux/dma-buf.h |5 +- 2 files changed, 166 insertions(+), 1 deletion(-) diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c index d89102a..7d867ed 100644 --- a/drivers/base/dma-buf.c +++ b/drivers/base/dma-buf.c @@ -27,9 +27,18 @@ #include linux/dma-buf.h #include linux/anon_inodes.h #include linux/export.h +#include linux/debugfs.h +#include linux/seq_file.h static inline int is_dma_buf_file(struct file *); +struct dma_buf_list { + struct list_head head; + struct mutex lock; +}; + +static struct dma_buf_list db_list; + static int dma_buf_release(struct inode *inode, struct file *file) { struct dma_buf *dmabuf; @@ -42,6 +51,11 @@ static int dma_buf_release(struct inode *inode, struct file *file) BUG_ON(dmabuf-vmapping_counter); dmabuf-ops-release(dmabuf); + + mutex_lock(db_list.lock); + list_del(dmabuf-list_node); + mutex_unlock(db_list.lock); + kfree(dmabuf); return 0; } @@ -125,6 +139,10 @@ struct dma_buf *dma_buf_export_named(void *priv, const struct dma_buf_ops *ops, mutex_init(dmabuf-lock); INIT_LIST_HEAD(dmabuf-attachments); + mutex_lock(db_list.lock); + list_add(dmabuf-list_node, db_list.head); + mutex_unlock(db_list.lock); + return dmabuf; } EXPORT_SYMBOL_GPL(dma_buf_export_named); @@ -551,3 +569,147 @@ void dma_buf_vunmap(struct dma_buf *dmabuf, void *vaddr) mutex_unlock(dmabuf-lock); } EXPORT_SYMBOL_GPL(dma_buf_vunmap); + +static int dma_buf_init_debugfs(void); +static void dma_buf_uninit_debugfs(void); + +static int __init dma_buf_init(void) +{ + mutex_init(db_list.lock); + INIT_LIST_HEAD(db_list.head); + dma_buf_init_debugfs(); + return 0; +} + +subsys_initcall(dma_buf_init); + +static void __exit dma_buf_deinit(void) This function is never called. You're right; it's missing an __exitcall() - I will add it! +{ + dma_buf_uninit_debugfs(); +} If you moved those two functions at the end of the file you could get rid of the forward declarations above. Sure - will do that. + +#ifdef CONFIG_DEBUG_FS +static int dma_buf_describe(struct seq_file *s) +{ + int ret; + struct dma_buf *buf_obj; + struct dma_buf_attachment *attach_obj; + int count = 0, attach_count; + size_t size = 0; + + ret = mutex_lock_interruptible(db_list.lock); + + if (ret) + return ret; + + seq_printf(s, \nDma-buf Objects:\n); + seq_printf(s, \texp_name\tsize\tflags\tmode\tcount\n); + + list_for_each_entry(buf_obj, db_list.head, list_node) { + ret = mutex_lock_interruptible(buf_obj-lock); + + if (ret) { + seq_printf(s, + \tERROR locking buffer object: skipping\n); + goto skip_buffer; + } + + seq_printf(s, \t); + + seq_printf(s, \t%s\t%08zu\t%08x\t%08x\t%08d\n, + buf_obj-exp_name, buf_obj-size, + buf_obj-file-f_flags, buf_obj-file-f_mode, + buf_obj-file-f_count.counter); + + seq_printf(s, \t\tAttached Devices:\n); + attach_count = 0; + + list_for_each_entry(attach_obj, buf_obj-attachments, node) { + seq_printf(s, \t\t); + + seq_printf(s, %s\n, attach_obj-dev-init_name); + attach_count++; + } + + seq_printf(s, \n\t\tTotal %d devices attached\n, + attach_count); + + count++; + size += buf_obj-size; +skip_buffer: + mutex_unlock(buf_obj-lock); + } + + seq_printf(s, \nTotal %d objects, %zu bytes\n, count, size); + + mutex_unlock(db_list.lock); + return 0; +} + +static int dma_buf_show(struct seq_file *s, void *unused) +{ + void (*func)(struct seq_file *) = s-private; + func(s); + return 0; +} + +static int dma_buf_debug_open(struct inode *inode, struct file *file) +{ + return single_open(file, dma_buf_show, inode-i_private); +} + +static const struct file_operations dma_buf_debug_fops = { + .open =
Re: [RFC PATCH 2/6] v4l2: add new VIDIOC_DBG_G_CHIP_NAME ioctl.
On Wed March 27 2013 02:11:23 Laurent Pinchart wrote: Hi Hans, On Monday 18 March 2013 17:38:16 Hans Verkuil wrote: From: Hans Verkuil hans.verk...@cisco.com Simplify the debugging ioctls by creating the VIDIOC_DBG_G_CHIP_NAME ioctl. This will eventually replace VIDIOC_DBG_G_CHIP_IDENT. Chip matching is done by the name or index of subdevices or an index to a bridge chip. Most of this can all be done automatically, so most drivers just need to provide get/set register ops. In particular, it is now possible to get/set subdev registers without requiring assistance of the bridge driver. My biggest question is why don't we use the media controller API to get the information provided by this new ioctl ? Because the media controller is implemented by only a handful of drivers, and this debug API is used by many more drivers. So I don't really see how this would be feasible today. Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/v4l2-core/v4l2-common.c |5 +- drivers/media/v4l2-core/v4l2-dev.c|5 +- drivers/media/v4l2-core/v4l2-ioctl.c | 115 -- include/media/v4l2-ioctl.h|3 + include/uapi/linux/videodev2.h| 34 +++--- 5 files changed, 146 insertions(+), 16 deletions(-) [snip] diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c index de1e9ab..c0c651d 100644 --- a/drivers/media/v4l2-core/v4l2-dev.c +++ b/drivers/media/v4l2-core/v4l2-dev.c @@ -591,9 +591,10 @@ static void determine_valid_ioctls(struct video_device [snip] +static int v4l_dbg_g_chip_name(const struct v4l2_ioctl_ops *ops, + struct file *file, void *fh, void *arg) As this is a debug ioctl that should never be used in application, I would like to guard the whole implementation with #ifdef CONFIG_VIDEO_ADV_DEBUG. This will make sure that no applications will abuse it, as it won't be available in distro kernels. Agreed. I'll make that change. Actually, it's not so much userspace abuse I am worried about, but kernel space abuse. I've found several drivers where the bridge driver calls g_chip_ident to get information about subdevice drivers. That was never intended and it complicates my work of removing g_chip_ident. By putting chip_name under ADV_DEBUG we can avoid similar abuse in the future. Regards, Hans +{ + struct video_device *vfd = video_devdata(file); + struct v4l2_dbg_chip_name *p = arg; + struct v4l2_subdev *sd; + int idx = 0; + + switch (p-match.type) { + case V4L2_CHIP_MATCH_BRIDGE: +#ifdef CONFIG_VIDEO_ADV_DEBUG + if (ops-vidioc_s_register) + p-flags |= V4L2_CHIP_FL_WRITABLE; + if (ops-vidioc_g_register) + p-flags |= V4L2_CHIP_FL_READABLE; +#endif + if (ops-vidioc_g_chip_name) + return ops-vidioc_g_chip_name(file, fh, arg); + if (p-match.addr) + return -EINVAL; + if (vfd-v4l2_dev) + strlcpy(p-name, vfd-v4l2_dev-name, sizeof(p-name)); + else if (vfd-parent) + strlcpy(p-name, vfd-parent-driver-name, sizeof(p-name)); + else + strlcpy(p-name, bridge, sizeof(p-name)); + return 0; + + case V4L2_CHIP_MATCH_SUBDEV_IDX: + case V4L2_CHIP_MATCH_SUBDEV_NAME: + if (vfd-v4l2_dev == NULL) + break; + v4l2_device_for_each_subdev(sd, vfd-v4l2_dev) { + if (v4l_dbg_found_match(p-match, sd, idx++)) { +#ifdef CONFIG_VIDEO_ADV_DEBUG + if (sd-ops-core sd-ops-core-s_register) + p-flags |= V4L2_CHIP_FL_WRITABLE; + if (sd-ops-core sd-ops-core-g_register) + p-flags |= V4L2_CHIP_FL_READABLE; +#endif + strlcpy(p-name, sd-name, sizeof(p-name)); + return 0; + } + } + break; + } + return -EINVAL; +} -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/6] v4l2: add new VIDIOC_DBG_G_CHIP_NAME ioctl.
Hi Hans, On Wednesday 27 March 2013 09:41:33 Hans Verkuil wrote: On Wed March 27 2013 02:11:23 Laurent Pinchart wrote: On Monday 18 March 2013 17:38:16 Hans Verkuil wrote: From: Hans Verkuil hans.verk...@cisco.com Simplify the debugging ioctls by creating the VIDIOC_DBG_G_CHIP_NAME ioctl. This will eventually replace VIDIOC_DBG_G_CHIP_IDENT. Chip matching is done by the name or index of subdevices or an index to a bridge chip. Most of this can all be done automatically, so most drivers just need to provide get/set register ops. In particular, it is now possible to get/set subdev registers without requiring assistance of the bridge driver. My biggest question is why don't we use the media controller API to get the information provided by this new ioctl ? Because the media controller is implemented by only a handful of drivers, and this debug API is used by many more drivers. Shouldn't we then fix that instead of adding a new ioctl ? So I don't really see how this would be feasible today. Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/v4l2-core/v4l2-common.c |5 +- drivers/media/v4l2-core/v4l2-dev.c|5 +- drivers/media/v4l2-core/v4l2-ioctl.c | 115 -- include/media/v4l2-ioctl.h|3 + include/uapi/linux/videodev2.h| 34 +++--- 5 files changed, 146 insertions(+), 16 deletions(-) [snip] diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c index de1e9ab..c0c651d 100644 --- a/drivers/media/v4l2-core/v4l2-dev.c +++ b/drivers/media/v4l2-core/v4l2-dev.c @@ -591,9 +591,10 @@ static void determine_valid_ioctls(struct video_device [snip] +static int v4l_dbg_g_chip_name(const struct v4l2_ioctl_ops *ops, + struct file *file, void *fh, void *arg) As this is a debug ioctl that should never be used in application, I would like to guard the whole implementation with #ifdef CONFIG_VIDEO_ADV_DEBUG. This will make sure that no applications will abuse it, as it won't be available in distro kernels. Agreed. I'll make that change. Actually, it's not so much userspace abuse I am worried about, but kernel space abuse. I've found several drivers where the bridge driver calls g_chip_ident to get information about subdevice drivers. That was never intended and it complicates my work of removing g_chip_ident. By putting chip_name under ADV_DEBUG we can avoid similar abuse in the future. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fw: [patch 02/03 v2] usb hid quirks for Masterkit MA901 usb radio
On Tue, 19 Mar 2013, Alexey Klimov wrote: Yes, i just checked how hid_ignore() works and prepared dirty fix to test in almost the same way like it's done for Keene usb driver. I will send correct fix in next few days. Any news on this, please? -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/6] v4l2: add new VIDIOC_DBG_G_CHIP_NAME ioctl.
On Wed March 27 2013 11:11:53 Laurent Pinchart wrote: Hi Hans, On Wednesday 27 March 2013 09:41:33 Hans Verkuil wrote: On Wed March 27 2013 02:11:23 Laurent Pinchart wrote: On Monday 18 March 2013 17:38:16 Hans Verkuil wrote: From: Hans Verkuil hans.verk...@cisco.com Simplify the debugging ioctls by creating the VIDIOC_DBG_G_CHIP_NAME ioctl. This will eventually replace VIDIOC_DBG_G_CHIP_IDENT. Chip matching is done by the name or index of subdevices or an index to a bridge chip. Most of this can all be done automatically, so most drivers just need to provide get/set register ops. In particular, it is now possible to get/set subdev registers without requiring assistance of the bridge driver. My biggest question is why don't we use the media controller API to get the information provided by this new ioctl ? Because the media controller is implemented by only a handful of drivers, and this debug API is used by many more drivers. Shouldn't we then fix that instead of adding a new ioctl ? Mauro was opposed to making the MC available for all drivers. So besides the technical issues which would take a lot of time (which I don't have), there is also a whole discussion about whether or not the MC should be there at all for 'simple' drivers. My main goal at the moment is to make this API more powerful and simplify the drivers. It is also my intention to get rid of G_CHIP_IDENT as soon as possible. Should we get the MC available for all drivers in the future, then it should be quite easy to adapt the code to the MC once G_CHIP_IDENT has been removed. Consider this a first step into the right direction. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] drm/exynos: enable FIMD clocks
2013/3/20 Vikas Sajjan vikas.saj...@linaro.org: While migrating to common clock framework (CCF), found that the FIMD clocks were pulled down by the CCF. If CCF finds any clock(s) which has NOT been claimed by any of the drivers, then such clock(s) are PULLed low by CCF. By calling clk_prepare_enable() for FIMD clocks fixes the issue. this patch also replaces clk_disable() with clk_disable_unprepare() during exit. Signed-off-by: Vikas Sajjan vikas.saj...@linaro.org --- Changes since v1: - added error checking for clk_prepare_enable() and also replaced clk_disable() with clk_disable_unprepare() during exit. --- drivers/gpu/drm/exynos/exynos_drm_fimd.c | 17 +++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 9537761..014d750 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -934,6 +934,19 @@ static int fimd_probe(struct platform_device *pdev) return ret; } + ret = clk_prepare_enable(ctx-lcd_clk); + if (ret) { + dev_err(dev, failed to enable 'sclk_fimd' clock\n); + return ret; + } + + ret = clk_prepare_enable(ctx-bus_clk); + if (ret) { + clk_disable_unprepare(ctx-lcd_clk); + dev_err(dev, failed to enable 'fimd' clock\n); + return ret; + } + Please remove the above two clk_prepare_enable function calls and use them in fimd_clock() instead of clk_enable/disable(). When probed, fimd clock will be enabled by runtime pm. Thanks, Inki Dae ctx-vidcon0 = pdata-vidcon0; ctx-vidcon1 = pdata-vidcon1; ctx-default_win = pdata-default_win; @@ -981,8 +994,8 @@ static int fimd_remove(struct platform_device *pdev) if (ctx-suspended) goto out; - clk_disable(ctx-lcd_clk); - clk_disable(ctx-bus_clk); + clk_disable_unprepare(ctx-lcd_clk); + clk_disable_unprepare(ctx-bus_clk); pm_runtime_set_suspended(dev); pm_runtime_put_sync(dev); -- 1.7.9.5 ___ dri-devel mailing list dri-de...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] documentation: DocBook/media : Fix typo in dvbproperty.xml
On Sun, 24 Mar 2013, Masanari Iida wrote: Correct spelling typos. Signed-off-by: Masanari Iida standby2...@gmail.com I can take this one. Thanks. -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] drm/exynos: enable FIMD clocks
Hi, On 27 March 2013 15:53, Inki Dae inki@samsung.com wrote: 2013/3/20 Vikas Sajjan vikas.saj...@linaro.org: While migrating to common clock framework (CCF), found that the FIMD clocks were pulled down by the CCF. If CCF finds any clock(s) which has NOT been claimed by any of the drivers, then such clock(s) are PULLed low by CCF. By calling clk_prepare_enable() for FIMD clocks fixes the issue. this patch also replaces clk_disable() with clk_disable_unprepare() during exit. Signed-off-by: Vikas Sajjan vikas.saj...@linaro.org --- Changes since v1: - added error checking for clk_prepare_enable() and also replaced clk_disable() with clk_disable_unprepare() during exit. --- drivers/gpu/drm/exynos/exynos_drm_fimd.c | 17 +++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 9537761..014d750 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -934,6 +934,19 @@ static int fimd_probe(struct platform_device *pdev) return ret; } + ret = clk_prepare_enable(ctx-lcd_clk); + if (ret) { + dev_err(dev, failed to enable 'sclk_fimd' clock\n); + return ret; + } + + ret = clk_prepare_enable(ctx-bus_clk); + if (ret) { + clk_disable_unprepare(ctx-lcd_clk); + dev_err(dev, failed to enable 'fimd' clock\n); + return ret; + } + Please remove the above two clk_prepare_enable function calls and use them in fimd_clock() instead of clk_enable/disable(). When probed, fimd clock will be enabled by runtime pm. Sure, will modify and resend. Thanks, Inki Dae ctx-vidcon0 = pdata-vidcon0; ctx-vidcon1 = pdata-vidcon1; ctx-default_win = pdata-default_win; @@ -981,8 +994,8 @@ static int fimd_remove(struct platform_device *pdev) if (ctx-suspended) goto out; - clk_disable(ctx-lcd_clk); - clk_disable(ctx-bus_clk); + clk_disable_unprepare(ctx-lcd_clk); + clk_disable_unprepare(ctx-bus_clk); pm_runtime_set_suspended(dev); pm_runtime_put_sync(dev); -- 1.7.9.5 ___ dri-devel mailing list dri-de...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Thanks and Regards Vikas Sajjan -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RFC: VIDIOC_DBG_G_CHIP_NAME improvements
Now that the VIDIOC_DBG_G_CHIP_NAME ioctl has been added to the v4l2 API I started work on removing the VIDIOC_DBG_G_CHIP_IDENT support in existing drivers. Based on that effort I realized that there are a few things that could be improved. One thing that Laurent pointed out is that this ioctl should be available only if CONFIG_VIDEO_ADV_DEBUG is set to prevent abuse by either userspace or kernelspace. I agree with that, especially since g_chip_ident is being abused today by some bridge drivers. That should be avoided in the future. I am also unhappy with the name. G_CHIP_INFO would certainly be more descriptive, but perhaps we should move a bit more into the direction of the Media Controller and call it G_ENTITY_INFO. Opinions are welcome. What surprised me when digging into the existing uses of G_CHIP_IDENT was that there are more devices than expected that have multiple register blocks. I.e. rather than a single set of registers they have multiple blocks of registers, say one block at address 0x1000, another at 0x2000, etc. Usually such register blocks represent IP blocks inside the chip, each doing a specific task. In other cases (adv7604) each block corresponds to an i2c address, each again representing an IP block inside the chip. In the case of adv7604 it has been implementing by mapping register offsets to specific i2c addresses, in the case of the cx231xx it has been implemented by exposing different bridge chips, unfortunately that's done in such a way that it can't be enumerated. The existing debug API has no support for discovering such ranges, but having worked with such a chip I think that having support for this is very desirable. Since we added a new ioctl anyway, I thought that this is a good time to extend it a bit and allow range discovery to be implemented: /** * struct v4l2_dbg_chip_name - VIDIOC_DBG_G_CHIP_NAME argument * @match: which chip to match * @flags: flags that tell whether this range is readable/writable * @name: unique name of the chip * @range_name: name of the register range * @range_min: minimum register of the register range * @range_max: maximum register of the register range * @reserved: future extensions */ struct v4l2_dbg_chip_name { struct v4l2_dbg_match match; __u32 range; __u32 flags; char name[32]; char range_name[32]; __u64 range_start; __u64 range_size; __u32 reserved[8]; } __attribute__ ((packed)); range is the range index, range_name describes the purpose of the register range, range_start and size are the start register address and the size of this register range. This extension allows you to enumerate the available register ranges for each device. If there is only one range, then range_size may be 0. This is mostly for backwards compatibility as otherwise I would have to modify all existing drivers for this, and also because this is not really necessary for simple devices with just one range. These are mostly i2c devices with start address 0 and a size of 256 bytes at most. Comments? Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] documentation: DocBook/media : Fix typo in dvbproperty.xml
Em Wed, 27 Mar 2013 11:39:27 +0100 (CET) Jiri Kosina jkos...@suse.cz escreveu: On Sun, 24 Mar 2013, Masanari Iida wrote: Correct spelling typos. Signed-off-by: Masanari Iida standby2...@gmail.com I can take this one. Thanks. Hi Jiri, I merged this already on my tree: commit 842059aa4796d7c59bc3801d48896ba06b1a1287 Author: Masanari Iida standby2...@gmail.com AuthorDate: Sun Mar 24 02:25:56 2013 -0300 Commit: Mauro Carvalho Chehab mche...@redhat.com CommitDate: Sun Mar 24 08:51:33 2013 -0300 [media] documentation: DocBook/media : Fix typo in dvbproperty.xml Correct spelling typos. Signed-off-by: Masanari Iida standby2...@gmail.com Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com -- Cheers, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2] [media] s5p-mfc: Modify encoder buffer alloc sequence
Hi, From: Arun Kumar K [mailto:arun...@samsung.com] Sent: Tuesday, March 26, 2013 8:28 AM MFC v6 needs minimum number of output buffers to be queued for encoder depending on the stream type and profile. For achieving this the sequence for allocating buffers at the encoder is modified similar to that of decoder. The new sequence is as follows: 1) Set format on CAPTURE plane 2) REQBUF on CAPTURE 3) QBUFS and STREAMON on CAPTURE 4) G_CTRL to get minimum buffers for OUTPUT plane 5) REQBUF on OUTPUT with the minimum buffers given by driver I don't like the idea of changing the encoding sequence. What if the old applications rely on a particular sequence? I see the problem you are addressing, but let's explore other options. MFC v6 sets the minimum number of buffers needed after the header is generated, v5 did not provide such information at all. Also using the variables dpb_count and state HEAD_PARSED seems odd. I guess that you did reuse the existing variable and state. But the naming used now is definitely misleading. DPB stands for decoded picture buffer. I suggest adding a new variable epb_count, or changing dpb_count to pb_count everywhere would be a good idea. Actually I like the latter more. In case of HEAD_PARSED I suggest adding a HEAD_PRODUCED state. This also fixes the crash happeninig during multi instance encoder- decoder simultaneous run due to memory allocation happening from interrupt context. Could you explain this problem more? What was the reason and how did you fix it? Also a small inline comment/question below. Signed-off-by: Arun Kumar K arun...@samsung.com --- Changes from v1 - Corrected the commit message as pointed out by John Sheu. http://www.spinics.net/lists/linux-media/msg61477.html --- drivers/media/platform/s5p-mfc/s5p_mfc_enc.c| 98 +++ drivers/media/platform/s5p-mfc/s5p_mfc_opr.h|1 + drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c |7 ++ drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 95 +-- --- 4 files changed, 147 insertions(+), 54 deletions(-) diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c index 4f6b553..46ca986 100644 --- a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c @@ -557,6 +557,16 @@ static struct mfc_control controls[] = { .step = 1, .default_value = 0, }, + { + .id = V4L2_CID_MIN_BUFFERS_FOR_OUTPUT, + .type = V4L2_CTRL_TYPE_INTEGER, + .name = Minimum number of output bufs, + .minimum = 1, + .maximum = 32, + .step = 1, + .default_value = 1, + .is_volatile = 1, + }, }; #define NUM_CTRLS ARRAY_SIZE(controls) @@ -661,18 +671,17 @@ static int enc_post_seq_start(struct s5p_mfc_ctx *ctx) vb2_buffer_done(dst_mb-b, VB2_BUF_STATE_DONE); spin_unlock_irqrestore(dev-irqlock, flags); } - if (IS_MFCV6(dev)) { - ctx-state = MFCINST_HEAD_PARSED; /* for INIT_BUFFER cmd */ - } else { + + if (!IS_MFCV6(dev)) { ctx-state = MFCINST_RUNNING; if (s5p_mfc_ctx_ready(ctx)) set_work_bit_irqsave(ctx); s5p_mfc_hw_call(dev-mfc_ops, try_run, dev); - } - - if (IS_MFCV6(dev)) + } else { ctx-dpb_count = s5p_mfc_hw_call(dev-mfc_ops, get_enc_dpb_count, dev); + ctx-state = MFCINST_HEAD_PARSED; + } return 0; } @@ -1055,15 +1064,13 @@ static int vidioc_reqbufs(struct file *file, void *priv, } ctx-capture_state = QUEUE_BUFS_REQUESTED; - if (!IS_MFCV6(dev)) { - ret = s5p_mfc_hw_call(ctx-dev-mfc_ops, - alloc_codec_buffers, ctx); - if (ret) { - mfc_err(Failed to allocate encoding buffers\n); - reqbufs-count = 0; - ret = vb2_reqbufs(ctx-vq_dst, reqbufs); - return -ENOMEM; - } + ret = s5p_mfc_hw_call(ctx-dev-mfc_ops, + alloc_codec_buffers, ctx); + if (ret) { + mfc_err(Failed to allocate encoding buffers\n); + reqbufs-count = 0; + ret = vb2_reqbufs(ctx-vq_dst, reqbufs); + return -ENOMEM; } } else if (reqbufs-type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) { if (ctx-output_state != QUEUE_FREE) { @@ -1071,12 +1078,35 @@ static int vidioc_reqbufs(struct file *file, void *priv, ctx-output_state); return -EINVAL;
Re: [PATCH v2] [media] s5p-mfc: Modify encoder buffer alloc sequence
Hi Kamil, Thank you for the review. Please find my response inline. On Wed, Mar 27, 2013 at 4:54 PM, Kamil Debski k.deb...@samsung.com wrote: Hi, From: Arun Kumar K [mailto:arun...@samsung.com] Sent: Tuesday, March 26, 2013 8:28 AM MFC v6 needs minimum number of output buffers to be queued for encoder depending on the stream type and profile. For achieving this the sequence for allocating buffers at the encoder is modified similar to that of decoder. The new sequence is as follows: 1) Set format on CAPTURE plane 2) REQBUF on CAPTURE 3) QBUFS and STREAMON on CAPTURE 4) G_CTRL to get minimum buffers for OUTPUT plane 5) REQBUF on OUTPUT with the minimum buffers given by driver I don't like the idea of changing the encoding sequence. What if the old applications rely on a particular sequence? I see the problem you are addressing, but let's explore other options. MFC v6 sets the minimum number of buffers needed after the header is generated, v5 did not provide such information at all. Yes. The only other way I can see without making this sequence change is to always keep the maximum number of buffers needed in worst case scenario. But this is not the optimal solution. You have any other suggestion? Also using the variables dpb_count and state HEAD_PARSED seems odd. I guess that you did reuse the existing variable and state. But the naming used now is definitely misleading. DPB stands for decoded picture buffer. I suggest adding a new variable epb_count, or changing dpb_count to pb_count everywhere would be a good idea. Actually I like the latter more. In case of HEAD_PARSED I suggest adding a HEAD_PRODUCED state. Ok I will make these changes. But still the encoding sequence will remain modified. This also fixes the crash happeninig during multi instance encoder- decoder simultaneous run due to memory allocation happening from interrupt context. Could you explain this problem more? What was the reason and how did you fix it? Earlier case, the function s5p_mfc_run_init_enc_buffers() which allocates encoder scratch buffer was called in the try_run during state HEAD_PARSED. If only one instance encoder is running, the allocation happens in reqbufs which calls try_run. But if multi-instance encoder/decoder is running, the try_run called from reqbuf can return due to HW lock. This will cause the function which allocates memory to be called from interrupt context which generated crash. Now it is modified such as the memory allocation part happens from reqbuf directly and then it calls try_run which only sends the command to firmware. Also a small inline comment/question below. Signed-off-by: Arun Kumar K arun...@samsung.com --- Changes from v1 - Corrected the commit message as pointed out by John Sheu. http://www.spinics.net/lists/linux-media/msg61477.html --- drivers/media/platform/s5p-mfc/s5p_mfc_enc.c| 98 +++ drivers/media/platform/s5p-mfc/s5p_mfc_opr.h|1 + drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c |7 ++ drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 95 +-- --- 4 files changed, 147 insertions(+), 54 deletions(-) diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c index 4f6b553..46ca986 100644 --- a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c @@ -557,6 +557,16 @@ static struct mfc_control controls[] = { .step = 1, .default_value = 0, }, + { + .id = V4L2_CID_MIN_BUFFERS_FOR_OUTPUT, + .type = V4L2_CTRL_TYPE_INTEGER, + .name = Minimum number of output bufs, + .minimum = 1, + .maximum = 32, + .step = 1, + .default_value = 1, + .is_volatile = 1, + }, }; #define NUM_CTRLS ARRAY_SIZE(controls) @@ -661,18 +671,17 @@ static int enc_post_seq_start(struct s5p_mfc_ctx *ctx) vb2_buffer_done(dst_mb-b, VB2_BUF_STATE_DONE); spin_unlock_irqrestore(dev-irqlock, flags); } - if (IS_MFCV6(dev)) { - ctx-state = MFCINST_HEAD_PARSED; /* for INIT_BUFFER cmd */ - } else { + + if (!IS_MFCV6(dev)) { ctx-state = MFCINST_RUNNING; if (s5p_mfc_ctx_ready(ctx)) set_work_bit_irqsave(ctx); s5p_mfc_hw_call(dev-mfc_ops, try_run, dev); - } - - if (IS_MFCV6(dev)) + } else { ctx-dpb_count = s5p_mfc_hw_call(dev-mfc_ops, get_enc_dpb_count, dev); + ctx-state = MFCINST_HEAD_PARSED; + } return 0; } @@ -1055,15 +1064,13 @@ static int vidioc_reqbufs(struct file *file, void *priv, } ctx-capture_state = QUEUE_BUFS_REQUESTED; - if (!IS_MFCV6(dev)) { - ret =
Re: [RFC 01/12] exynos-fimc-is: Adding device tree nodes
On 03/27/2013 05:31 AM, Arun Kumar K wrote: On Wed, Mar 27, 2013 at 4:21 AM, Sylwester Nawrocki sylvester.nawro...@gmail.com wrote: On 03/26/2013 01:17 PM, Arun Kumar K wrote: [...] Only issue is with the context sharing. Right now you can see that the fimc-is context is shared between all the subdevs. As all of them are part of the same driver, it is possible. If sensor is made as an independent i2c device, a separate probe will be called for it. For ISP sensor subdev to work independently, it needs to call the fimc_is_pipeline_* calls for FW initialization and other configurations for which it needs the fimc-is main context. Now there is a workaround here by calling a get_context() macro in sensor's probe to get the fimc-is context. This will cause the same context to be shared and updated by 2 drivers though both are part of fimc-is. Is this acceptable? Or am I missing some other simple solution of implementing it in a better way? OK, thanks for the explanation. I can think of at least one possible way to get hold of the fimc-is context in the subdev. For instance, in subdev's .registered callback you get a pointer to struct v4l2_device, which is normally embedded in a top level driver's private data. Then with container_of() you could get hold of required data at the fimc-is driver. But... to make the subdev drivers reuse possible subdevs should normally not be required to know the internals of a host driver they are registered to. And it looks a bit unusual to have fimc_pipeline_* calls in the sensor's operation handlers. I thought that the subdevs could provide basic methods and it would be the top level media driver that would resolve any dependencies in calling required subdev ops, according to media graph configuration done by the user on /dev/media?. The media driver has a list of media entities (subdevices and video nodes) and I though it could coordinate any requests involving whole video/image processing pipeline originating from /dev/video ioctls/fops. So for instance if /dev/video in this pipeline is opened sensor (sd) - mipi-csis (sd) - fimc-lite (sd) - memory (/dev/video) it would call s_power operation on the above subdevs and additionally on e.g. the isp subdev (or any other we choose as a main subdev implementing the FIMC-IS slave interface). Then couldn't it be done that video node ioctls invoke pipeline operations, and the media device resolves any dependencies/calls order, as in case of the exynos4 driver ? As a side note, I'm working on adding a generic method to get any v4l2_subdev/video_device from a struct media_entity instance, so it is easier to handle link_notify events, power/streaming enable/ disable sequences, etc. Currently I have a data structure like: struct exynos_iss_entity { struct video_device vdev; struct v4l2_subdev subdev; struct exynos_iss_pipeline pipe; }; Regards, Sylwester -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Terratec Grabby hwrev 2
On Tue, 26 Mar 2013 10:20:56 +0200 Timo Teras timo.te...@iki.fi wrote: I did manage to get decent traces with USBlyzer evaluation version. Nothing _that_ exciting there. Though, there's quite a bit of differences on certain register writes. I tried copying the changed parts, but did not really help. Turning on saa7115 debug gave: saa7115 1-0025: chip found @ 0x4a (ID 000) does not match a known saa711x chip. Which does not look good. i2c_scan=1 on modprobe gives: em2860 #0: found i2c device @ 0x4a [saa7113h] em2860 #0: found i2c device @ 0xa0 [eeprom] em2860 #0: found i2c device @ 0xa2 [???] em2860 #0: found i2c device @ 0xa4 [???] em2860 #0: found i2c device @ 0xa6 [???] em2860 #0: found i2c device @ 0xa8 [???] em2860 #0: found i2c device @ 0xaa [???] em2860 #0: found i2c device @ 0xac [???] em2860 #0: found i2c device @ 0xae [???] - Timo -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Terratec Grabby hwrev 2
Am 25.03.2013 18:08, schrieb Timo Teras: I just bought a Terratec Grabby hardware revision 2 in hopes that it would work on my linux box. But alas, I got only sound working. It seems that analog video picture grabbing does not work. I tried kernels 3.4.34-grsec, 3.7.1 (vanilla), 3.8.2-grsec and 3.9.0-rc4 (vanilla). And all fail the same way - no video data received. The USB ID is same as on the revision 1 board: Bus 005 Device 002: ID 0ccd:0096 TerraTec Electronic GmbH And it is properly detected as Grabby. It seems that the videobuf2 changes for 3.9.0-rc4 resulted in better debug logging, and it implies that the application (ffmpeg 1.1.4) is behaving well: all buffers are allocated, mmapped, queued, streamon called. But no data is received from the dongle. I also tested mencoder and it fails in similar manner. Dmesg (on 3.9.0-rc4) tells after module load the following: [ 1249.600246] em28xx: New device TerraTec Electronic GmbH TerraTec Grabby @ 480 Mbps (0ccd:0096, inte rface 0, class 0) [ 1249.600258] em28xx: Video interface 0 found: isoc [ 1249.600264] em28xx: DVB interface 0 found: isoc Hmm... yet another device where we detect a DVB endpoint (which is obviously wrong)... Could you please post the output of lsusb -v -d 0ccd:0096 ? Regards, Frank [ 1249.600443] em28xx: chip ID is em2860 [ 1249.715053] em2860 #0: i2c eeprom 00: 1a eb 67 95 cd 0c 96 00 50 00 11 03 9c 20 6a 32 [ 1249.715084] em2860 #0: i2c eeprom 10: 00 00 06 57 0e 02 00 00 00 00 00 00 00 00 00 00 [ 1249.715110] em2860 #0: i2c eeprom 20: 02 00 01 00 f0 10 01 00 00 00 00 00 5b 00 00 00 [ 1249.715136] em2860 #0: i2c eeprom 30: 00 00 20 40 20 80 02 20 01 01 00 00 00 00 00 00 [ 1249.715161] em2860 #0: i2c eeprom 40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 1249.715186] em2860 #0: i2c eeprom 50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 1249.715211] em2860 #0: i2c eeprom 60: 00 00 00 00 00 00 00 00 00 00 32 03 54 00 65 00 [ 1249.715235] em2860 #0: i2c eeprom 70: 72 00 72 00 61 00 54 00 65 00 63 00 20 00 45 00 [ 1249.715261] em2860 #0: i2c eeprom 80: 6c 00 65 00 63 00 74 00 72 00 6f 00 6e 00 69 00 [ 1249.715286] em2860 #0: i2c eeprom 90: 63 00 20 00 47 00 6d 00 62 00 48 00 20 03 54 00 [ 1249.715311] em2860 #0: i2c eeprom a0: 65 00 72 00 72 00 61 00 54 00 65 00 63 00 20 00 [ 1249.715336] em2860 #0: i2c eeprom b0: 47 00 72 00 61 00 62 00 62 00 79 00 48 00 00 00 [ 1249.715361] em2860 #0: i2c eeprom c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 1249.715385] em2860 #0: i2c eeprom d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 1249.715410] em2860 #0: i2c eeprom e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 1249.715435] em2860 #0: i2c eeprom f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 1249.715464] em2860 #0: EEPROM ID= 0x9567eb1a, EEPROM hash = 0xd3498090 [ 1249.715470] em2860 #0: EEPROM info: [ 1249.715475] em2860 #0: AC97 audio (5 sample rates) [ 1249.715480] em2860 #0: 500mA max power [ 1249.715487] em2860 #0: Table at 0x06, strings=0x209c, 0x326a, 0x [ 1249.715495] em2860 #0: Identified as Terratec Grabby (card=67) [ 1250.058076] em2860 #0: Config register raw data: 0x50 [ 1250.076845] em2860 #0: AC97 vendor ID = 0x60f160f1 [ 1250.086814] em2860 #0: AC97 features = 0x60f1 [ 1250.086822] em2860 #0: Unknown AC97 audio processor detected! [ 1251.116646] em2860 #0: v4l2 driver version 0.1.3 [ 1251.891145] em2860 #0: V4L2 video device registered as video0 [ 1251.891155] em2860 #0: V4L2 VBI device registered as vbi0 [ 1251.891161] em2860 #0: analog set to isoc mode. [ 1251.891167] em2860 #0: dvb set to isoc mode. [ 1251.910649] usbcore: registered new interface driver em28xx Any suggestions how to debug/fix this? Thanks, Timo -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] s5p-mfc: Change MFC clock reference w.r.t Common Clock Framework
On 03/27/2013 03:01 AM, Mike Turquette wrote: Quoting Prasanna Kumar (2013-03-25 22:20:51) From: Prasanna Kumar prasanna...@samsung.com According to Common Clock framework , modified the method of getting Huh ? Could you explain in detail what exactly in this patch is related to the Common Clock Framework ? I guess you mean the new clocks driver ? clock for MFC Block. Signed-off-by: Prasanna Kumar prasanna...@samsung.com --- drivers/media/platform/s5p-mfc/s5p_mfc_pm.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c b/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c index 6aa38a5..b8ac8f6 100644 --- a/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_pm.c @@ -50,7 +50,7 @@ int s5p_mfc_init_pm(struct s5p_mfc_dev *dev) goto err_p_ip_clk; } - pm-clock = clk_get(dev-plat_dev-dev, dev-variant-mclk_name); + pm-clock = clk_get_parent(pm-clock_gate); Ok, I'll bite. Why make this change? Was there an issue using clkdev/clk_get to get the clock you needed? Yes, this appears a pretty mysterious change to me... I wonder, have you altered anything at the clock tree definition to make it work ? What platforms has this change been tested on ? For Exynos4, I've changed the sclk_mfc clock definition, so it is possible to assign it to the MFC IP by listing it in the codec DT node clocks property. I suppose something similar is needed for Exynos5, if this is the platform you needed that change for. BTW, I think the mclk_name field should be removed from struct s5p_mfc_variant, an the driver should use fixed name for the second clock. Now there is mfc, sclk_mfc used for Exynos4 and mfc, aclk_133 for Exynos5. Drivers are not really supposed to care about differences like this. Such differences could be easily handled in the device tree. The DT binding documentation just needs to specify the meaning of each clock name. [1] http://www.spinics.net/lists/arm-kernel/msg233521.html Regards, -- Sylwester Nawrocki Samsung Poland RD Center -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Terratec Grabby hwrev 2
On Wed, 27 Mar 2013 18:37:26 +0100 Frank Schäfer fschaefer@googlemail.com wrote: Am 25.03.2013 18:08, schrieb Timo Teras: I just bought a Terratec Grabby hardware revision 2 in hopes that it would work on my linux box. But alas, I got only sound working. It seems that analog video picture grabbing does not work. I tried kernels 3.4.34-grsec, 3.7.1 (vanilla), 3.8.2-grsec and 3.9.0-rc4 (vanilla). And all fail the same way - no video data received. The USB ID is same as on the revision 1 board: Bus 005 Device 002: ID 0ccd:0096 TerraTec Electronic GmbH And it is properly detected as Grabby. It seems that the videobuf2 changes for 3.9.0-rc4 resulted in better debug logging, and it implies that the application (ffmpeg 1.1.4) is behaving well: all buffers are allocated, mmapped, queued, streamon called. But no data is received from the dongle. I also tested mencoder and it fails in similar manner. Dmesg (on 3.9.0-rc4) tells after module load the following: [ 1249.600246] em28xx: New device TerraTec Electronic GmbH TerraTec Grabby @ 480 Mbps (0ccd:0096, inte rface 0, class 0) [ 1249.600258] em28xx: Video interface 0 found: isoc [ 1249.600264] em28xx: DVB interface 0 found: isoc Hmm... yet another device where we detect a DVB endpoint (which is obviously wrong)... Could you please post the output of lsusb -v -d 0ccd:0096 ? # lsusb -vvv -d 0ccd:0096 Bus 005 Device 028: ID 0ccd:0096 TerraTec Electronic GmbH Couldn't open device, some information will be missing Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 2.00 bDeviceClass0 (Defined at Interface level) bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize064 idVendor 0x0ccd TerraTec Electronic GmbH idProduct 0x0096 bcdDevice1.00 iManufacturer 2 iProduct1 iSerial 0 bNumConfigurations 1 Couldn't get configuration descriptor 0, some information will be missing Couldn't get configuration descriptor 0, some information will be missing The errors are weird. strace gives: open(/dev/bus/usb/005/028, O_RDONLY) = -1 ENOENT (No such file or directory) open(/dev/bus/usb/005/028, O_RDONLY) = -1 ENOENT (No such file or directory) # ls /dev/bus/usb/005/ 001 003 013 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Terratec Grabby hwrev 2
On Wed, 27 Mar 2013 19:57:49 +0200 Timo Teras timo.te...@iki.fi wrote: The errors are weird. strace gives: open(/dev/bus/usb/005/028, O_RDONLY) = -1 ENOENT (No such file or directory) open(/dev/bus/usb/005/028, O_RDONLY) = -1 ENOENT (No such file or directory) # ls /dev/bus/usb/005/ 001 003 013 Seems something fishy in my mdev setup. Here's the full output: # lsusb -vvv -d 0ccd:0096 Bus 005 Device 029: ID 0ccd:0096 TerraTec Electronic GmbH Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 2.00 bDeviceClass0 (Defined at Interface level) bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize064 idVendor 0x0ccd TerraTec Electronic GmbH idProduct 0x0096 bcdDevice1.00 iManufacturer 2 TerraTec Electronic GmbH iProduct1 TerraTec Grabby iSerial 0 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 555 bNumInterfaces 3 bConfigurationValue 1 iConfiguration 0 bmAttributes 0x80 (Bus Powered) MaxPower 500mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 0 bNumEndpoints 3 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol255 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes3 Transfer TypeInterrupt Synch Type None Usage Type Data wMaxPacketSize 0x0001 1x 1 bytes bInterval 11 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes1 Transfer TypeIsochronous Synch Type None Usage Type Data wMaxPacketSize 0x 1x 0 bytes bInterval 1 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84 EP 4 IN bmAttributes1 Transfer TypeIsochronous Synch Type None Usage Type Data wMaxPacketSize 0x 1x 0 bytes bInterval 1 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 1 bNumEndpoints 3 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol255 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes3 Transfer TypeInterrupt Synch Type None Usage Type Data wMaxPacketSize 0x0001 1x 1 bytes bInterval 11 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes1 Transfer TypeIsochronous Synch Type None Usage Type Data wMaxPacketSize 0x 1x 0 bytes bInterval 1 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84 EP 4 IN bmAttributes1 Transfer TypeIsochronous Synch Type None Usage Type Data wMaxPacketSize 0x 1x 0 bytes bInterval 1 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 2 bNumEndpoints 3 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol255 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes3 Transfer TypeInterrupt Synch Type None Usage Type Data wMaxPacketSize 0x0001 1x 1 bytes bInterval 11 Endpoint Descriptor: bLength 7
cron job: media_tree daily build: WARNINGS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Wed Mar 27 19:00:21 CET 2013 git branch: test git hash: 004e45d736bfe62159bd4dc1549eff414bd27496 gcc version:i686-linux-gcc (GCC) 4.7.2 host hardware: x86_64 host os:3.8-3.slh.2-amd64 linux-git-arm-davinci: OK linux-git-arm-exynos: WARNINGS linux-git-arm-omap: WARNINGS linux-git-blackfin: WARNINGS linux-git-i686: OK linux-git-m32r: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK linux-2.6.31.14-i686: WARNINGS linux-2.6.32.27-i686: WARNINGS linux-2.6.33.7-i686: WARNINGS linux-2.6.34.7-i686: WARNINGS linux-2.6.35.9-i686: WARNINGS linux-2.6.36.4-i686: WARNINGS linux-2.6.37.6-i686: WARNINGS linux-2.6.38.8-i686: WARNINGS linux-2.6.39.4-i686: WARNINGS linux-3.0.60-i686: WARNINGS linux-3.1.10-i686: WARNINGS linux-3.2.37-i686: WARNINGS linux-3.3.8-i686: WARNINGS linux-3.4.27-i686: WARNINGS linux-3.5.7-i686: WARNINGS linux-3.6.11-i686: WARNINGS linux-3.7.4-i686: WARNINGS linux-3.8-i686: OK linux-3.9-rc1-i686: OK linux-2.6.31.14-x86_64: WARNINGS linux-2.6.32.27-x86_64: WARNINGS linux-2.6.33.7-x86_64: WARNINGS linux-2.6.34.7-x86_64: WARNINGS linux-2.6.35.9-x86_64: WARNINGS linux-2.6.36.4-x86_64: WARNINGS linux-2.6.37.6-x86_64: WARNINGS linux-2.6.38.8-x86_64: WARNINGS linux-2.6.39.4-x86_64: WARNINGS linux-3.0.60-x86_64: WARNINGS linux-3.1.10-x86_64: WARNINGS linux-3.2.37-x86_64: WARNINGS linux-3.3.8-x86_64: WARNINGS linux-3.4.27-x86_64: WARNINGS linux-3.5.7-x86_64: WARNINGS linux-3.6.11-x86_64: WARNINGS linux-3.7.4-x86_64: WARNINGS linux-3.8-x86_64: WARNINGS linux-3.9-rc1-x86_64: WARNINGS apps: WARNINGS spec-git: OK sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
omap3isp: possible circular locking dependency
Hi all, I've a problem running OMAP3 ISP with current 3.9-rc4. I tried to run the Laurent's live application to capture data from my mt9v034 sensor but kernel shows a possible circular locking dependency. Also the captured images are wrong and I see garbage. The same environment worked for me with kernel 3.7. Anyone knows any issue related to this ? Anyone experimented something similar with other sensors ? I tried to find something in ML and Laurent's git repository but I don't see anything. Thanks in advance. Here is the log: ~# live No compatible input device found, disabling digital zoom 32 bpp Device /dev/video7 opened: omap_vout (). /dev/video7: 3 buffers requested. /dev/video7: buffer 0 mapped at address 0xb6df1000. /dev/video7: buffer 1 mapped at address 0xb6c71000. /dev/video7: buffer 2 mapped at address 0xb6af1000. Device /dev/video6 opened: OMAP3 ISP resizer output (media). viewfinder configured for 2011 1024x768 /dev/video6: 3 buffers requested. /dev/video6: buffer 0 valid. /dev/video6: buffer 1 valid. /dev/video6: buffer 2 valid. Device /dev/video6 opened: OMAP3 ISP resizer output (media). /dev/video6: 2 buffers requested. /dev/video6: buffer 0 mapped at address 0xb64f1000. /dev/video6: buffer 1 mapped at address 0xb5ef1000. snapshot configured for 2011 2048x1536 Device /[ 63.557861] [ 63.560577] == [ 63.567077] [ INFO: possible circular locking dependency detected ] [ 63.573669] 3.9.0-rc4-00152-gba9ce12 #4 Not tainted [ 63.578796] --- [ 63.585388] live/1273 is trying to acquire lock: [ 63.590209] (mm-mmap_sem){++}, at: [bf06fb24] omap3isp_video_queue_qbuf+0x280/0x7a4 [omap3_isp] [ 63.600280] [ 63.600280] but task is already holding lock: [ 63.606414] (queue-lock){+.+.+.}, at: [bf06f8d4] omap3isp_video_queue_qbuf+0x30/0x7a4 [omap3_isp] [ 63.616271] [ 63.616271] which lock already depends on the new lock. [ 63.616271] [ 63.624877] [ 63.624877] the existing dependency chain (in reverse order) is: [ 63.632751] - #1 (queue-lock){+.+.+.}: [ 63.637207][c0081948] lock_acquire+0x94/0x100 [ 63.642700][c04f02b0] mutex_lock_nested+0x40/0x298 [ 63.648681][bf0703a0] omap3isp_video_queue_mmap+0x1c/0xe8 [omap3_isp] [ 63.656372][bf02117c] v4l2_mmap+0x54/0x88 [videodev] [ 63.662597][c00dc740] mmap_region+0x2e0/0x520 [ 63.668090][c00dcc38] do_mmap_pgoff+0x2b8/0x340 [ 63.673767][c00cdd1c] vm_mmap_pgoff+0x84/0xac [ 63.679260][c00db4a8] sys_mmap_pgoff+0x54/0xb0 [ 63.684875][c0013660] ret_fast_syscall+0x0/0x3c [ 63.690551] - #0 (mm-mmap_sem){++}: [ 63.695068][c0080e28] __lock_acquire+0x14bc/0x1ae8 [ 63.701019][c0081948] lock_acquire+0x94/0x100 [ 63.706512][c04f095c] down_read+0x30/0x40 [ 63.711639][bf06fb24] omap3isp_video_queue_qbuf+0x280/0x7a4 [omap3_isp] [ 63.719543][bf025ca4] v4l_qbuf+0x3c/0x40 [videodev] [ 63.725646][bf024ba8] __video_do_ioctl+0x240/0x33c [videodev] [ 63.732635][bf025668] video_usercopy+0x114/0x40c [videodev] [ 63.739440][bf0215c0] v4l2_ioctl+0xfc/0x144 [videodev] [ 63.745758][c00fe954] do_vfs_ioctl+0x7c/0x5ac [ 63.751281][c00feee8] sys_ioctl+0x64/0x84 [ 63.756408][c0013660] ret_fast_syscall+0x0/0x3c [ 63.762084] [ 63.762084] other info that might help us debug this: [ 63.762084] [ 63.770507] Possible unsafe locking scenario: [ 63.770507] [ 63.776702]CPU0CPU1 [ 63.781463] [ 63.786224] lock(queue-lock); [ 63.789733]lock(mm-mmap_sem); [ 63.795959]lock(queue-lock); [ 63.802124] lock(mm-mmap_sem); [ 63.805694] [ 63.805694] *** DEADLOCK *** [ 63.805694] [ 63.811950] 1 lock held by live/1273: [ 63.815795] #0: (queue-lock){+.+.+.}, at: [bf06f8d4] omap3isp_video_queue_qbuf+0x30/0x7a4 [omap3_isp] [ 63.826141] [ 63.826141] stack backtrace: [ 63.830749] [c00196d4] (unwind_backtrace+0x0/0xf0) from [c04eb478] (print_circular_bug+0x25c/0x2a8) [ 63.840637] [c04eb478] (print_circular_bug+0x25c/0x2a8) from [c0080e28] (__lock_acquire+0x14bc/0x1ae8) [ 63.850769] [c0080e28] (__lock_acquire+0x14bc/0x1ae8) from [c0081948] (lock_acquire+0x94/0x100) [ 63.860290] [c0081948] (lock_acquire+0x94/0x100) from [c04f095c] (down_read+0x30/0x40) [ 63.869018] [c04f095c] (down_read+0x30/0x40) from [bf06fb24] (omap3isp_video_queue_qbuf+0x280/0x7a4 [omap3_isp]) [ 63.880126] [bf06fb24] (omap3isp_video_queue_qbuf+0x280/0x7a4 [omap3_isp]) from [bf025ca4] (v4l_qbuf+0x3c/0x40 [videodev]) [ 63.892181] [bf025ca4] (v4l_qbuf+0x3c/0x40 [videodev]) from [bf024ba8] (__video_do_ioctl+0x240/0x33c [videodev]) [ 63.903289] [bf024ba8] (__video_do_ioctl+0x240/0x33c [videodev]) from [bf025668]
[PATCH] em28xx: ignore isoc DVB USB endpoints with wMaxPacketSize = 0 bytes for all alt settings
Some devices without DVB support (such as the Terratec Grabby and Easycap DC-60) provide isochronous DVB USB endpoints with wMaxPacketSize set to 0 bytes for all alt settings. Ignore these endpoints and avoid registering a DVB device node and loading the DVB driver extension. Signed-off-by: Frank Schäfer fschaefer@googlemail.com Cc: sta...@kernel.org --- drivers/media/usb/em28xx/em28xx-cards.c |9 - 1 Datei geändert, 8 Zeilen hinzugefügt(+), 1 Zeile entfernt(-) diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c index 54e0362..94536ee 100644 --- a/drivers/media/usb/em28xx/em28xx-cards.c +++ b/drivers/media/usb/em28xx/em28xx-cards.c @@ -3357,14 +3357,15 @@ static int em28xx_usb_probe(struct usb_interface *interface, dev-analog_ep_bulk = e-bEndpointAddress; } else { - has_dvb = true; if (usb_endpoint_xfer_isoc(e)) { dev-dvb_ep_isoc = e-bEndpointAddress; if (size dev-dvb_max_pkt_size_isoc) { + has_dvb = true; /* see NOTE (~) */ dev-dvb_max_pkt_size_isoc = size; dev-dvb_alt_isoc = i; } } else { + has_dvb = true; dev-dvb_ep_bulk = e-bEndpointAddress; } } @@ -3391,6 +3392,12 @@ static int em28xx_usb_probe(struct usb_interface *interface, * so far. But there might be devices for which this * logic is not sufficient... */ + /* +* NOTE (~): some manufacturers (e.g. Terratec) disable +* endpoints by setting wMaxPacketSize to 0 bytes for +* all alt settings. So far, we've seen this for +* DVB isoc endpoints only. +*/ } } -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 1/2] hid: fix Masterkit MA901 hid quirks
This patch reverts commit 0322bd3980b3ebf7dde8474e22614cb443d6479a and adds checks in hid_ignore() for Masterkit MA901 usb radio device. This usb radio device shares USB ID with many Atmel V-USB (and probably other) devices so patch sorts things out by checking name, vendor, product of hid device. Signed-off-by: Alexey Klimov klimov.li...@gmail.com diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 512b01c..aa341d1 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -2077,7 +2077,6 @@ static const struct hid_device_id hid_ignore_list[] = { { HID_USB_DEVICE(USB_VENDOR_ID_LD, USB_DEVICE_ID_LD_HYBRID) }, { HID_USB_DEVICE(USB_VENDOR_ID_LD, USB_DEVICE_ID_LD_HEATCONTROL) }, { HID_USB_DEVICE(USB_VENDOR_ID_MADCATZ, USB_DEVICE_ID_MADCATZ_BEATPAD) }, - { HID_USB_DEVICE(USB_VENDOR_ID_MASTERKIT, USB_DEVICE_ID_MASTERKIT_MA901RADIO) }, { HID_USB_DEVICE(USB_VENDOR_ID_MCC, USB_DEVICE_ID_MCC_PMD1024LS) }, { HID_USB_DEVICE(USB_VENDOR_ID_MCC, USB_DEVICE_ID_MCC_PMD1208LS) }, { HID_USB_DEVICE(USB_VENDOR_ID_MICROCHIP, USB_DEVICE_ID_PICKIT1) }, @@ -2244,6 +2243,18 @@ bool hid_ignore(struct hid_device *hdev) hdev-product = USB_DEVICE_ID_VELLEMAN_K8061_LAST)) return true; break; + case USB_VENDOR_ID_ATMEL_V_USB: + /* Masterkit MA901 usb radio based on Atmel tiny85 chip and +* it has the same USB ID as many Atmel V-USB devices. This +* usb radio is handled by radio-ma901.c driver so we want +* ignore the hid. Check the name, bus, product and ignore +* if we have MA901 usb radio. +*/ + if (hdev-product == USB_DEVICE_ID_ATMEL_V_USB + hdev-bus == BUS_USB + strncmp(hdev-name, www.masterkit.ru MA901, 22) == 0) + return true; + break; } if (hdev-type == HID_TYPE_USBMOUSE diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 92e47e5..57d9f3a 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -158,6 +158,8 @@ #define USB_VENDOR_ID_ATMEL0x03eb #define USB_DEVICE_ID_ATMEL_MULTITOUCH 0x211c #define USB_DEVICE_ID_ATMEL_MXT_DIGITIZER 0x2118 +#define USB_VENDOR_ID_ATMEL_V_USB 0x16c0 +#define USB_DEVICE_ID_ATMEL_V_USB 0x05df #define USB_VENDOR_ID_AUREAL 0x0755 #define USB_DEVICE_ID_AUREAL_W01RN 0x2626 @@ -557,9 +559,6 @@ #define USB_VENDOR_ID_MADCATZ 0x0738 #define USB_DEVICE_ID_MADCATZ_BEATPAD 0x4540 -#define USB_VENDOR_ID_MASTERKIT0x16c0 -#define USB_DEVICE_ID_MASTERKIT_MA901RADIO 0x05df - #define USB_VENDOR_ID_MCC 0x09db #define USB_DEVICE_ID_MCC_PMD1024LS0x0076 #define USB_DEVICE_ID_MCC_PMD1208LS0x007a -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 2/2] media: radio-ma901: return ENODEV in probe if usb_device doesn't match
Masterkit MA901 usb radio device shares USB ID with Atmel V-USB devices. This patch adds additional checks in usb_ma901radio_probe() and if product or manufacturer doesn't match we return -ENODEV and don't continue. This allows hid drivers to handle not MA901 device. Signed-off-by: Alexey Klimov klimov.li...@gmail.com diff --git a/drivers/media/radio/radio-ma901.c b/drivers/media/radio/radio-ma901.c index c61f590..348dafc 100644 --- a/drivers/media/radio/radio-ma901.c +++ b/drivers/media/radio/radio-ma901.c @@ -347,9 +347,20 @@ static void usb_ma901radio_release(struct v4l2_device *v4l2_dev) static int usb_ma901radio_probe(struct usb_interface *intf, const struct usb_device_id *id) { + struct usb_device *dev = interface_to_usbdev(intf); struct ma901radio_device *radio; int retval = 0; + /* Masterkit MA901 usb radio has the same USB ID as many others +* Atmel V-USB devices. Let's make additional checks to be sure +* that this is our device. +*/ + + if (dev-product dev-manufacturer + (strncmp(dev-product, MA901, 5) != 0 + || strncmp(dev-manufacturer, www.masterkit.ru, 16) != 0)) + return -ENODEV; + radio = kzalloc(sizeof(struct ma901radio_device), GFP_KERNEL); if (!radio) { dev_err(intf-dev, kzalloc for ma901radio_device failed\n); -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Terratec Grabby hwrev 2
Am 27.03.2013 19:04, schrieb Timo Teras: On Wed, 27 Mar 2013 19:57:49 +0200 Timo Teras timo.te...@iki.fi wrote: The errors are weird. strace gives: open(/dev/bus/usb/005/028, O_RDONLY) = -1 ENOENT (No such file or directory) open(/dev/bus/usb/005/028, O_RDONLY) = -1 ENOENT (No such file or directory) # ls /dev/bus/usb/005/ 001 003 013 Seems something fishy in my mdev setup. Here's the full output: # lsusb -vvv -d 0ccd:0096 Bus 005 Device 029: ID 0ccd:0096 TerraTec Electronic GmbH Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 2.00 bDeviceClass0 (Defined at Interface level) bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize064 idVendor 0x0ccd TerraTec Electronic GmbH idProduct 0x0096 bcdDevice1.00 iManufacturer 2 TerraTec Electronic GmbH iProduct1 TerraTec Grabby iSerial 0 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 555 bNumInterfaces 3 bConfigurationValue 1 iConfiguration 0 bmAttributes 0x80 (Bus Powered) MaxPower 500mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 0 bNumEndpoints 3 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol255 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes3 Transfer TypeInterrupt Synch Type None Usage Type Data wMaxPacketSize 0x0001 1x 1 bytes bInterval 11 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes1 Transfer TypeIsochronous Synch Type None Usage Type Data wMaxPacketSize 0x 1x 0 bytes bInterval 1 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84 EP 4 IN bmAttributes1 Transfer TypeIsochronous Synch Type None Usage Type Data wMaxPacketSize 0x 1x 0 bytes bInterval 1 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 1 bNumEndpoints 3 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol255 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes3 Transfer TypeInterrupt Synch Type None Usage Type Data wMaxPacketSize 0x0001 1x 1 bytes bInterval 11 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes1 Transfer TypeIsochronous Synch Type None Usage Type Data wMaxPacketSize 0x 1x 0 bytes bInterval 1 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84 EP 4 IN bmAttributes1 Transfer TypeIsochronous Synch Type None Usage Type Data wMaxPacketSize 0x 1x 0 bytes bInterval 1 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 2 bNumEndpoints 3 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol255 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes3 Transfer TypeInterrupt Synch Type None Usage Type
Re: Fw: [patch 02/03 v2] usb hid quirks for Masterkit MA901 usb radio
On Wed, Mar 27, 2013 at 2:18 PM, Jiri Kosina jkos...@suse.cz wrote: On Tue, 19 Mar 2013, Alexey Klimov wrote: Yes, i just checked how hid_ignore() works and prepared dirty fix to test in almost the same way like it's done for Keene usb driver. I will send correct fix in next few days. Any news on this, please? Hi Jiri, I'm very very sorry (was busy because of life). I just sent two patches to you, Mauro and two mail lists: [patch 1/2] hid: fix Masterkit MA901 hid quirks [patch 2/2] media: radio-ma901: return ENODEV in probe if usb_device doesn't match Please check. First one for hid layer, so maybe you can take it directly through your tree. I hope it's not too late. I think Mauro will take second patch. I spend some time testing them trying to figure out right scenarios and i hope i did correct checks. It will be nice if someone can test patches because i don't have any devices with same USB IDs as radio-ma901. Thanks and best regards, Alexey. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/9] em28xx: improve the sensor device code
This patch series improves the sensor device support/code. The changes include - splitting sensor detection, board hints and sensor initialization/configuration (patches 1 and 2) - moving the sensor specific code to a separate source code file (patch 4) - improving/extending sensor probing and identification (patches 5, 7 and 8) - adding (basic) support for the OmniVision OV2640 sensor (patch 9) Frank Schäfer (9): em28xx: fix and separate the board hints for sensor devices em28xx: separate sensor detection and initialization/configuration em28xx: rename em28xx_hint_sensor() to em28xx_detect_sensor() em28xx: move sensor code to a separate source code file em28xx-camera.c em28xx: detect further Micron sensors em28xx: move the probing of Micron sensors to a separate function em28xx: add probing procedure for OmniVision sensors em28xx: add comment about Samsung and Kodak sensor probing addresses em28xx: add basic support for OmniVision OV2640 sensors drivers/media/usb/em28xx/Makefile|2 +- drivers/media/usb/em28xx/em28xx-camera.c | 424 ++ drivers/media/usb/em28xx/em28xx-cards.c | 183 ++--- drivers/media/usb/em28xx/em28xx.h|5 + 4 Dateien geändert, 451 Zeilen hinzugefügt(+), 163 Zeilen entfernt(-) create mode 100644 drivers/media/usb/em28xx/em28xx-camera.c -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/9] em28xx: fix and separate the board hints for sensor devices
The current board hint code is mixed together with the sensor detection and initialization code. It actually selects a board depending on the detected sensor type only, with the result that 3 of the 6 webcam boards are currently dead. Separate it and move it to em28xx_hint_board() which already contains the board hints for analog capturing+TV and DVB devices. This way, we have all board hints at a common place which makes it easier to extend the code and reduces the risk of regressions. It also makes it possible again to use the boards EM2750_BOARD_DLCW_130, EM2820_BOARD_VIDEOLOGY_20K14XUSB and EM2860_BOARD_NETGMBH_CAM (using the module parameter card). NOTE: the current board hint logic for webcams is preserved. Not more not less. Signed-off-by: Frank Schäfer fschaefer@googlemail.com --- drivers/media/usb/em28xx/em28xx-cards.c | 40 +++ 1 Datei geändert, 19 Zeilen hinzugefügt(+), 21 Zeilen entfernt(-) diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c index 54e0362..e41ecb5 100644 --- a/drivers/media/usb/em28xx/em28xx-cards.c +++ b/drivers/media/usb/em28xx/em28xx-cards.c @@ -2333,9 +2333,6 @@ static int em28xx_hint_sensor(struct em28xx *dev) switch (version) { case 0x8232:/* mt9v011 640x480 1.3 Mpix sensor */ case 0x8243:/* mt9v011 rev B 640x480 1.3 Mpix sensor */ - dev-model = EM2820_BOARD_SILVERCREST_WEBCAM; - em28xx_set_model(dev); - sensor_name = mt9v011; dev-em28xx_sensor = EM28XX_MT9V011; dev-sensor_xres = 640; @@ -2359,9 +2356,6 @@ static int em28xx_hint_sensor(struct em28xx *dev) break; case 0x143a:/* MT9M111 as found in the ECS G200 */ - dev-model = EM2750_BOARD_UNKNOWN; - em28xx_set_model(dev); - sensor_name = mt9m111; dev-board.xclk = EM28XX_XCLK_FREQUENCY_48MHZ; dev-em28xx_sensor = EM28XX_MT9M111; @@ -2375,9 +2369,6 @@ static int em28xx_hint_sensor(struct em28xx *dev) break; case 0x8431: - dev-model = EM2750_BOARD_UNKNOWN; - em28xx_set_model(dev); - sensor_name = mt9m001; dev-em28xx_sensor = EM28XX_MT9M001; em28xx_initialize_mt9m001(dev); @@ -2394,11 +2385,7 @@ static int em28xx_hint_sensor(struct em28xx *dev) return -EINVAL; } - /* Setup webcam defaults */ - em28xx_pre_card_setup(dev); - - em28xx_errdev(Sensor is %s, using model %s entry.\n, - sensor_name, em28xx_boards[dev-model].name); + em28xx_info(sensor %s detected\n, sensor_name); return 0; } @@ -2628,6 +2615,18 @@ static int em28xx_hint_board(struct em28xx *dev) { int i; + if (dev-board.is_webcam) { + if (dev-em28xx_sensor == EM28XX_MT9V011) { + dev-model = EM2820_BOARD_SILVERCREST_WEBCAM; + } else if (dev-em28xx_sensor == EM28XX_MT9M001 || + dev-em28xx_sensor == EM28XX_MT9M111) { + dev-model = EM2750_BOARD_UNKNOWN; + } + /* FIXME: IMPROVE ! */ + + return 0; + } + /* HINT method: EEPROM * * This method works only for boards with eeprom. @@ -2719,10 +2718,10 @@ static void em28xx_card_setup(struct em28xx *dev) dev-progressive = 1; } - if (!dev-board.is_webcam) { - switch (dev-model) { - case EM2820_BOARD_UNKNOWN: - case EM2800_BOARD_UNKNOWN: + switch (dev-model) { + case EM2750_BOARD_UNKNOWN: + case EM2820_BOARD_UNKNOWN: + case EM2800_BOARD_UNKNOWN: /* * The K-WORLD DVB-T 310U is detected as an MSI Digivox AD. * @@ -2743,9 +2742,8 @@ static void em28xx_card_setup(struct em28xx *dev) em28xx_pre_card_setup(dev); } break; - default: - em28xx_set_model(dev); - } + default: + em28xx_set_model(dev); } em28xx_info(Identified as %s (card=%d)\n, -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/9] em28xx: separate sensor detection and initialization/configuration
Sensor detection and initialization/configuration are currently mixed together. This works as long as all devices with a particular sensor are working with the same board configuration. In the long run, this will be not sufficient, so separate these both steps to make the code more flexible and future proof. This also makes the code more consistent, because the initialization of the MT9V011 sensor subdevice is already separated. Signed-off-by: Frank Schäfer fschaefer@googlemail.com --- drivers/media/usb/em28xx/em28xx-cards.c | 122 ++- 1 Datei geändert, 72 Zeilen hinzugefügt(+), 50 Zeilen entfernt(-) diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c index e41ecb5..c8ad7e5 100644 --- a/drivers/media/usb/em28xx/em28xx-cards.c +++ b/drivers/media/usb/em28xx/em28xx-cards.c @@ -2335,50 +2335,14 @@ static int em28xx_hint_sensor(struct em28xx *dev) case 0x8243:/* mt9v011 rev B 640x480 1.3 Mpix sensor */ sensor_name = mt9v011; dev-em28xx_sensor = EM28XX_MT9V011; - dev-sensor_xres = 640; - dev-sensor_yres = 480; - /* -* FIXME: mt9v011 uses I2S speed as xtal clk - at least with -* the Silvercrest cam I have here for testing - for higher -* resolutions, a high clock cause horizontal artifacts, so we -* need to use a lower xclk frequency. -* Yet, it would be possible to adjust xclk depending on the -* desired resolution, since this affects directly the -* frame rate. -*/ - dev-board.xclk = EM28XX_XCLK_FREQUENCY_4_3MHZ; - dev-sensor_xtal = 430; - - /* probably means GRGB 16 bit bayer */ - dev-vinmode = 0x0d; - dev-vinctl = 0x00; - break; - case 0x143a:/* MT9M111 as found in the ECS G200 */ sensor_name = mt9m111; - dev-board.xclk = EM28XX_XCLK_FREQUENCY_48MHZ; dev-em28xx_sensor = EM28XX_MT9M111; - em28xx_initialize_mt9m111(dev); - dev-sensor_xres = 640; - dev-sensor_yres = 512; - - dev-vinmode = 0x0a; - dev-vinctl = 0x00; - break; - case 0x8431: sensor_name = mt9m001; dev-em28xx_sensor = EM28XX_MT9M001; - em28xx_initialize_mt9m001(dev); - dev-sensor_xres = 1280; - dev-sensor_yres = 1024; - - /* probably means BGGR 16 bit bayer */ - dev-vinmode = 0x0c; - dev-vinctl = 0x00; - break; default: printk(Unknown Micron Sensor 0x%04x\n, version); @@ -2611,6 +2575,76 @@ static void em28xx_tuner_setup(struct em28xx *dev) v4l2_device_call_all(dev-v4l2_dev, 0, tuner, s_frequency, f); } +static int em28xx_init_camera(struct em28xx *dev) +{ + switch (dev-em28xx_sensor) { + case EM28XX_MT9V011: + { + struct mt9v011_platform_data pdata; + struct i2c_board_info mt9v011_info = { + .type = mt9v011, + .addr = dev-i2c_client[dev-def_i2c_bus].addr, + .platform_data = pdata, + }; + + dev-sensor_xres = 640; + dev-sensor_yres = 480; + + /* +* FIXME: mt9v011 uses I2S speed as xtal clk - at least with +* the Silvercrest cam I have here for testing - for higher +* resolutions, a high clock cause horizontal artifacts, so we +* need to use a lower xclk frequency. +* Yet, it would be possible to adjust xclk depending on the +* desired resolution, since this affects directly the +* frame rate. +*/ + dev-board.xclk = EM28XX_XCLK_FREQUENCY_4_3MHZ; + em28xx_write_reg(dev, EM28XX_R0F_XCLK, dev-board.xclk); + dev-sensor_xtal = 430; + pdata.xtal = dev-sensor_xtal; + if (NULL == + v4l2_i2c_new_subdev_board(dev-v4l2_dev, + dev-i2c_adap[dev-def_i2c_bus], + mt9v011_info, NULL)) + return -ENODEV; + /* probably means GRGB 16 bit bayer */ + dev-vinmode = 0x0d; + dev-vinctl = 0x00; + + break; + } + case EM28XX_MT9M001: + dev-sensor_xres = 1280; + dev-sensor_yres = 1024; + + em28xx_initialize_mt9m001(dev); + + /* probably means BGGR 16 bit bayer */ + dev-vinmode = 0x0c; + dev-vinctl = 0x00; + + break; + case