Re: [PATCH 1/3] media: i2c: Add ADV761X support
On 09/26/2013 12:57 AM, Laurent Pinchart wrote: On Wednesday 25 September 2013 18:31:51 Guennadi Liakhovetski wrote: On Wed, 25 Sep 2013, Valentine wrote: On 09/25/2013 06:08 PM, Guennadi Liakhovetski wrote: [snip] +/* I2C I/O operations */ +static s32 adv_smbus_read_byte_data(struct i2c_client *client, u8 command) +{ + s32 ret, i; + + for (i = 0; i 3; i++) { + ret = i2c_smbus_read_byte_data(client, command); Uhm, why do you need to do this 3 times?... I see adv7842 does that too... but e.g. adv7604 dies this only for writing and not for reading... Just thought it would be safe to retry in case of a failure. Other drivers seem to retry I2C I/O as well. This is probably related to the possible cable issues. Not sure. Anyways, retrying doesn't seem to hurt, does it? It does, because it means there's something we don't understand. IMHO it should either work from the first time, or there should be an error, that we understand with a following error processing, that _might_ include re-trying. Re-trying just in case isn't good. Especially if no error has been observed. I have observed very rare errors when reading HDMI status. Just a couple of times during this week. I'm not sure of the nature of those errors. Just thought it would be OK to retry since other decoder drivers do so as well. Ok, understand. As I said, I personally don't like that, but, I think, Laurent will have a final word on this. I don't like the idea of blindly retrying, especially for write operations. If a read fails due to a flaky cable, there's equal chances that a write would fail as well, which could result in writing random values to random registers. Given the side effects that this could have, I'd much rather not retry I/O operations and have the users fix electrical issues. Hiding something this serious could be dangerous. For a ubiquitous and relatively slow-speed bus the i2c bus is surprisingly flaky in my experience. I've seen surprisingly many cases where a retry was useful or even necessary. There can be many causes: electrical issues is one, and while that shouldn't happen, in practice it does. Interference by other devices on the bus is another. And sometimes the device itself is flaky: in the case of the adv7511 trying to enable/disable an interrupt just fails every so often. I have yet to see a product bringup where the i2c bringup 'just works'. There are always problems there. 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 1/3] media: i2c: Add ADV761X support
On 09/25/2013 10:36 PM, Valentine wrote: On 09/25/2013 10:33 PM, Guennadi Liakhovetski wrote: Hi Valentine, On Wed, 25 Sep 2013, Valentine wrote: On 09/25/2013 08:31 PM, Guennadi Liakhovetski wrote: On Wed, 25 Sep 2013, Valentine wrote: On 09/25/2013 06:08 PM, Guennadi Liakhovetski wrote: [snip] Using module parameters has a disadvantage, that all instances of this driver will get the same values, and it is quite possible to have several HDMI receivers in a system, I believe? Wouldn't it be better to pass these addresses from the platform data / DT? Yes, that doesn't look quite right, but I couldn't find a better solution ATM. The problem is that this subdevice is going to be used by a soc_camera driver, and the soc_camera interface uses its own platform data for all I2C subdevices. Thus, if I pass the I2C addresses in client-dev.platform_data here (as in adv7604), it's doing to be overwritten by the soc_camera_i2c_init() function with a soc_camera_subdev_desc data. Not sure what the correct solution should be. Any suggestions are greatly appreciated. You don't think, that soc-camera makes it impossible to pass device-specific platform data to subdevices, right? For an example have a look e.g. at mt9v022 and arch/arm/mach-pxa/pcm990-baseboard.c or at mt9t111 and arch/arm/mach-shmobile/board-armadillo800eva.c. Yes, but in this case adv761x.c has to be a soc_camera i2c subdevice driver. Which means it will get its platform data from ssdd-drv_priv like mt9v022 AOT client-dev.platform_data, like adv7604 does. What if a non-soc_camera needs to use the adv7612 decoder? IMHO, Looks like there's a conflict in the soc/non-soc camera driver subdevice usage. I don't think, that just using soc-camera platform data struct will make it impossible for non-soc-camera bridge / video input drivers to use this subdevice driver. This is the only problem I can see at the moment. Do you see any other issues? As I've said earlier the driver is based much on the adv7604 which is used for non-soc cameras. I guess, implementing dv_timings and ISR for adv7612 will make it look even closer. Other subdevice drivers (like 7180) seem to work with both soc/non-soc cameras. Are you proposing to make it soc-cam-specific, move it to i2c/soc_camera/ and deal with a non-soc variant later if needed? I wouldn't call it soc-camera specific. It would just include an soc-camera header and use one soc-camera struct. It wouldn't even require loading the soc-camera core module, let alone using soc-camera driver binding schemes. So, it would be a very mild dependency. Guennadi, I still think this really, really sucks. And it would be great if soc-camera would just be able to use regular subdev drivers that no longer need to use anything from soc-camera. The dependencies have weakened a lot in recent times, so if a final push could be made to remove them completely, then that would be very helpful. As for where you put it - I don't care that much either. OK, thanks. Please put it in media/i2c. There where all the other receivers/transmitters live. In fact, looking a bit more at the driver, I've got a couple more questions. 1. Hotplug handling: you've got comments like Pull down the hotplug pin, but I don't see any code that would actually pull the pin? Am I missing it or is it permanently low? It is supposed to be done by the camera driver that receives the hotplug notification. Hm, seems strange to me - that pin belongs to the HDMI interface AFAICS and the camera host / bridge driver knows nothing about HDMI... E.g. adv7842_delayed_work_enable_hotplug(), IIUC, does enable the hotplug detection pin itself. The adv7604 handling looks suspicious to me... BTW, the free ADV7612 datasheet with no register information and just a general description does mention hotplug control pins, so, I really think it should be a task of the HDMI decoder driver to control those. IIUC, these pins are supposed to be controlled by the camera or SOC-GPIO. The adv7604 does not control the hotplug pin, so that's why the notify is there: only the bridge driver knows how it is hooked up. The adv7842 on the other hand controls the hotplug pin directly, so that one has no hotplug notify. So it depends on the adv761x hardware whether or not a notify is needed. How can you use this with soc-camera, BTW? soc-camera has no notifier, so cannot control the hotplug pin for you. And if you cannot control the hotplug pin, then you cannot set the EDID. And if you cannot set the EDID you cannot receive HDMI. Which makes it pretty pointless. 2. You're using an own work queue just to queue a work to notify users about the event. Wouldn't it suffice to just use the scheduler work queue? It probably would. I just didn't want to deviate much from the code that is already there (adv7604/adv7842/ad9389b/adv7511/cx25840). diff --git a/include/media/adv761x.h
Re: [PATCH 1/3] media: i2c: Add ADV761X support
On 09/25/2013 12:21 PM, Laurent Pinchart wrote: Hi Valentine, Thank you for the patch. Please see below for a couple of comments (in addition to Hans' and Guennadi's comments). On Tuesday 24 September 2013 17:38:34 Valentine Barshak wrote: This adds ADV7611/ADV7612 Dual Port Xpressview 225 MHz HDMI Receiver support. The code is based on the ADV7604 driver, and ADV7612 patch by Shinobu Uehara shinobu.uehara...@renesas.com Signed-off-by: Valentine Barshak valentine.bars...@cogentembedded.com --- drivers/media/i2c/Kconfig | 11 + drivers/media/i2c/Makefile |1 + drivers/media/i2c/adv761x.c | 1060 include/media/adv761x.h | 28 ++ 4 files changed, 1100 insertions(+) create mode 100644 drivers/media/i2c/adv761x.c create mode 100644 include/media/adv761x.h [snip] diff --git a/drivers/media/i2c/adv761x.c b/drivers/media/i2c/adv761x.c new file mode 100644 index 000..bc3dfc3 --- /dev/null +++ b/drivers/media/i2c/adv761x.c [snip] +/* Supported color format list */ +static const struct adv761x_color_format adv761x_cfmts[] = { +{ +.code = V4L2_MBUS_FMT_RGB888_1X24, +.colorspace = V4L2_COLORSPACE_SRGB, +}, +}; Do you plan to add support for more formats later ? [snip] +static inline int edid_write_block(struct v4l2_subdev *sd, +unsigned len, const u8 *val) I would pass a pointer to adv761x_state to the internal functions instead of getting it from the subdev pointer each time, but that's up to you. +{ +struct i2c_client *client = v4l2_get_subdevdata(sd); +struct adv761x_state *state = to_state(sd); +int ret = 0; +int i; i is used as an unsigned number, please make it unsigned int. Same comment for all the locations below where i is used as unsigned. + +v4l2_dbg(2, debug, sd, writing EDID block (%d bytes)\n, len); + +v4l2_subdev_notify(sd, ADV761X_HOTPLUG, (void *)0); This means that the master V4L2 driver will need to handle ADV761x-specific events, which doesn't sound very good. What do you use the event for ? Could you create a standard interface for this ? +/* Disable I2C access to internal EDID ram from DDC port */ +rep_write(sd, 0x74, 0x0); Could you #define constants for register addresses and values instead of hardcoding them ? These days I would probably use the regmap API instead. That said, I've always hated using defines for register addresses since all the datasheets are always organized around addresses, not names. Using defines means I need to do a double-lookup: from define to address, from address to the right database page that documents it. I'd rather see a useful comment. +for (i = 0; !ret i len; i += I2C_SMBUS_BLOCK_MAX) +ret = adv_smbus_write_i2c_block_data(state-i2c_edid, i, +I2C_SMBUS_BLOCK_MAX, val + i); +if (ret) +return ret; + +/* adv761x calculates the checksums and enables I2C access + * to internal EDID ram from DDC port. + */ +rep_write(sd, 0x74, 0x01); + +for (i = 0; i 1000; i++) { +if (rep_read(sd, 0x76) 0x1) { +/* enable hotplug after 100 ms */ +queue_delayed_work(state-work_queue, +state-enable_hotplug, HZ / 10); +return 0; +} +schedule(); I haven't checked the datasheet, but can't the chip generate an interrupt that could replace the busy-loop ? There isn't one in the adv7604, so I assume it's the same for this chip. +} + +v4l_err(client, error enabling edid\n); +return -EIO; +} 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] [media] marvell-ccic: simplify and fix clk handling (a bit)
Hi Libin, On Wed, Sep 25, 2013 at 07:47:19PM -0700, Libin Yang wrote: In the clk enable and prepare function, we will check the NULL pointer. So it should be no problem. I'm not sure what you mean here and unfortunately your quoting style makes your statement appear without context. if (... !IS_ERR(cam-mipi_clk)) { if (cam-mipi_clk) devm_clk_put(mcam-dev, cam-mipi_clk); cam-mipi_clk = NULL; } might work in your setup, but it's wrong usage of the clk API. There is no reason NULL couldn't be a valid clk pointer. Moreover I cannot find a place in that driver that calls prepare and/or enable for the mipi_clk. (BTW, calling clk_get_rate on a disabled clk is another thing you shouldn't do.) For the mipi_clk, it is shared between other components, so we put the clk it we don't use it. There should be no problem if 1 driver holds a reference to the clock. It's clk_disable (+ maybe clk_unprepare) you should call when you don't need it any more. (But even having 1 driver enabling a clk isn't a problem ...) This patch fixes all but the last issue in this list. This patch doesn't introduce new reasons for not building, but there are already several bulid problems. Care to report those? Sure: CC drivers/media/platform/marvell-ccic/mmp-driver.o drivers/media/platform/marvell-ccic/mmp-driver.c: In function 'mmpcam_calc_dphy': drivers/media/platform/marvell-ccic/mmp-driver.c:264:15: error: 'struct mmp_camera_platform_data' has no member named 'dphy3_algo' drivers/media/platform/marvell-ccic/mmp-driver.c:265:7: error: 'DPHY3_ALGO_PXA910' undeclared (first use in this function) drivers/media/platform/marvell-ccic/mmp-driver.c:265:7: note: each undeclared identifier is reported only once for each function it appears in drivers/media/platform/marvell-ccic/mmp-driver.c:269:8: error: 'struct mmp_camera_platform_data' has no member named 'dphy' drivers/media/platform/marvell-ccic/mmp-driver.c:270:17: error: 'struct mmp_camera_platform_data' has no member named 'lane_clk' drivers/media/platform/marvell-ccic/mmp-driver.c:271:16: error: 'struct mmp_camera_platform_data' has no member named 'lane_clk' drivers/media/platform/marvell-ccic/mmp-driver.c:273:7: error: 'DPHY3_ALGO_PXA2128' undeclared (first use in this function) drivers/media/platform/marvell-ccic/mmp-driver.c:277:8: error: 'struct mmp_camera_platform_data' has no member named 'dphy' drivers/media/platform/marvell-ccic/mmp-driver.c:278:17: error: 'struct mmp_camera_platform_data' has no member named 'lane_clk' drivers/media/platform/marvell-ccic/mmp-driver.c:279:16: error: 'struct mmp_camera_platform_data' has no member named 'lane_clk' drivers/media/platform/marvell-ccic/mmp-driver.c:308:7: error: 'struct mmp_camera_platform_data' has no member named 'dphy' drivers/media/platform/marvell-ccic/mmp-driver.c:312:104: error: 'struct mmp_camera_platform_data' has no member named 'dphy' drivers/media/platform/marvell-ccic/mmp-driver.c:312:120: error: 'struct mmp_camera_platform_data' has no member named 'dphy' drivers/media/platform/marvell-ccic/mmp-driver.c:312:136: error: 'struct mmp_camera_platform_data' has no member named 'dphy' drivers/media/platform/marvell-ccic/mmp-driver.c: In function 'mmpcam_probe': drivers/media/platform/marvell-ccic/mmp-driver.c:385:24: error: 'struct mmp_camera_platform_data' has no member named 'mclk_min' drivers/media/platform/marvell-ccic/mmp-driver.c:386:24: error: 'struct mmp_camera_platform_data' has no member named 'mclk_src' drivers/media/platform/marvell-ccic/mmp-driver.c:387:24: error: 'struct mmp_camera_platform_data' has no member named 'mclk_div' drivers/media/platform/marvell-ccic/mmp-driver.c:388:24: error: 'struct mmp_camera_platform_data' has no member named 'bus_type' drivers/media/platform/marvell-ccic/mmp-driver.c:389:20: error: 'struct mmp_camera_platform_data' has no member named 'dphy' drivers/media/platform/marvell-ccic/mmp-driver.c:391:20: error: 'struct mmp_camera_platform_data' has no member named 'lane' Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | http://www.pengutronix.de/ | -- 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] marvell-ccic: simplify and fix clk handling (a bit)
On Thu, Sep 26, 2013 at 10:13:56AM +0200, Uwe Kleine-König wrote: Hi Libin, On Wed, Sep 25, 2013 at 07:47:19PM -0700, Libin Yang wrote: In the clk enable and prepare function, we will check the NULL pointer. So it should be no problem. I'm not sure what you mean here and unfortunately your quoting style makes your statement appear without context. if (... !IS_ERR(cam-mipi_clk)) { if (cam-mipi_clk) devm_clk_put(mcam-dev, cam-mipi_clk); cam-mipi_clk = NULL; } might work in your setup, but it's wrong usage of the clk API. There is no reason NULL couldn't be a valid clk pointer. Moreover I cannot find a place in that driver that calls prepare and/or enable for the mipi_clk. (BTW, calling clk_get_rate on a disabled clk is another thing you shouldn't do.) It's a bug for another reason. Consider this: clk = devm_clk_get(...); Now, as the CLK API defines only IS_ERR(clk) to be errors, if clk is NULL then the devm API will allocate a tracking structure for the allocated clock. If you then do: if (!IS_ERR(clk)) { if (clk) devm_clk_put(clk); clk = NULL; } Then this structure won't get freed. Next time you call devm_clk_get(), you'll allocate another tracking structure. If the driver does this a lot, it will spawn lots of these tracking structures which will only get cleaned up when the device is unbound (possibly never.) So, what this driver is doing with its NULL checks against clocks is buggy, no two ways about it. -- 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 1/3] media: i2c: Add ADV761X support
Hi Hans, On Thursday 26 September 2013 08:57:43 Hans Verkuil wrote: On 09/25/2013 12:21 PM, Laurent Pinchart wrote: Hi Valentine, Thank you for the patch. Please see below for a couple of comments (in addition to Hans' and Guennadi's comments). On Tuesday 24 September 2013 17:38:34 Valentine Barshak wrote: This adds ADV7611/ADV7612 Dual Port Xpressview 225 MHz HDMI Receiver support. The code is based on the ADV7604 driver, and ADV7612 patch by Shinobu Uehara shinobu.uehara...@renesas.com Signed-off-by: Valentine Barshak valentine.bars...@cogentembedded.com --- drivers/media/i2c/Kconfig | 11 + drivers/media/i2c/Makefile |1 + drivers/media/i2c/adv761x.c | 1060 include/media/adv761x.h | 28 ++ 4 files changed, 1100 insertions(+) create mode 100644 drivers/media/i2c/adv761x.c create mode 100644 include/media/adv761x.h [snip] diff --git a/drivers/media/i2c/adv761x.c b/drivers/media/i2c/adv761x.c new file mode 100644 index 000..bc3dfc3 --- /dev/null +++ b/drivers/media/i2c/adv761x.c [snip] +/* Supported color format list */ +static const struct adv761x_color_format adv761x_cfmts[] = { + { + .code = V4L2_MBUS_FMT_RGB888_1X24, + .colorspace = V4L2_COLORSPACE_SRGB, + }, +}; Do you plan to add support for more formats later ? [snip] +static inline int edid_write_block(struct v4l2_subdev *sd, + unsigned len, const u8 *val) I would pass a pointer to adv761x_state to the internal functions instead of getting it from the subdev pointer each time, but that's up to you. +{ + struct i2c_client *client = v4l2_get_subdevdata(sd); + struct adv761x_state *state = to_state(sd); + int ret = 0; + int i; i is used as an unsigned number, please make it unsigned int. Same comment for all the locations below where i is used as unsigned. + + v4l2_dbg(2, debug, sd, writing EDID block (%d bytes)\n, len); + + v4l2_subdev_notify(sd, ADV761X_HOTPLUG, (void *)0); This means that the master V4L2 driver will need to handle ADV761x-specific events, which doesn't sound very good. What do you use the event for ? Could you create a standard interface for this ? + /* Disable I2C access to internal EDID ram from DDC port */ + rep_write(sd, 0x74, 0x0); Could you #define constants for register addresses and values instead of hardcoding them ? These days I would probably use the regmap API instead. That's a good idea. That said, I've always hated using defines for register addresses since all the datasheets are always organized around addresses, not names. Using defines means I need to do a double-lookup: from define to address, from address to the right database page that documents it. Using #defines usually saves you from looking up the register completely in the datasheet, and is especially important when the datasheet is not publicly available. When reasonably familiar with the chip, proper #defines will tell you what register is modified and how. Hardcoded values are never readable. I'd rather see a useful comment. + for (i = 0; !ret i len; i += I2C_SMBUS_BLOCK_MAX) + ret = adv_smbus_write_i2c_block_data(state-i2c_edid, i, + I2C_SMBUS_BLOCK_MAX, val + i); + if (ret) + return ret; + + /* adv761x calculates the checksums and enables I2C access + * to internal EDID ram from DDC port. + */ + rep_write(sd, 0x74, 0x01); + + for (i = 0; i 1000; i++) { + if (rep_read(sd, 0x76) 0x1) { + /* enable hotplug after 100 ms */ + queue_delayed_work(state-work_queue, + state-enable_hotplug, HZ / 10); + return 0; + } + schedule(); I haven't checked the datasheet, but can't the chip generate an interrupt that could replace the busy-loop ? There isn't one in the adv7604, so I assume it's the same for this chip. + } + + v4l_err(client, error enabling edid\n); + return -EIO; +} -- 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: [PATCH 1/3] media: i2c: Add ADV761X support
Hi Hans, On Thursday 26 September 2013 08:31:58 Hans Verkuil wrote: On 09/26/2013 12:57 AM, Laurent Pinchart wrote: On Wednesday 25 September 2013 18:31:51 Guennadi Liakhovetski wrote: On Wed, 25 Sep 2013, Valentine wrote: On 09/25/2013 06:08 PM, Guennadi Liakhovetski wrote: [snip] +/* I2C I/O operations */ +static s32 adv_smbus_read_byte_data(struct i2c_client *client, u8 command) +{ + s32 ret, i; + + for (i = 0; i 3; i++) { + ret = i2c_smbus_read_byte_data(client, command); Uhm, why do you need to do this 3 times?... I see adv7842 does that too... but e.g. adv7604 dies this only for writing and not for reading... Just thought it would be safe to retry in case of a failure. Other drivers seem to retry I2C I/O as well. This is probably related to the possible cable issues. Not sure. Anyways, retrying doesn't seem to hurt, does it? It does, because it means there's something we don't understand. IMHO it should either work from the first time, or there should be an error, that we understand with a following error processing, that _might_ include re-trying. Re-trying just in case isn't good. Especially if no error has been observed. I have observed very rare errors when reading HDMI status. Just a couple of times during this week. I'm not sure of the nature of those errors. Just thought it would be OK to retry since other decoder drivers do so as well. Ok, understand. As I said, I personally don't like that, but, I think, Laurent will have a final word on this. I don't like the idea of blindly retrying, especially for write operations. If a read fails due to a flaky cable, there's equal chances that a write would fail as well, which could result in writing random values to random registers. Given the side effects that this could have, I'd much rather not retry I/O operations and have the users fix electrical issues. Hiding something this serious could be dangerous. For a ubiquitous and relatively slow-speed bus the i2c bus is surprisingly flaky in my experience. I've seen surprisingly many cases where a retry was useful or even necessary. There can be many causes: electrical issues is one, and while that shouldn't happen, in practice it does. Interference by other devices on the bus is another. And sometimes the device itself is flaky: in the case of the adv7511 trying to enable/disable an interrupt just fails every so often. If we enable retries by default, what could happen is that new boards subject to electrical issues on their I2C bus will not be seen as broken until much later in the development, and possibly too late, after going to production. I don't want to blindly hide the problem, if an electrical issue is present it should be seen. Now, if we need to support an existing broken board, I'm fine with enabling retries, but it shouldn't be enabled by default. I have yet to see a product bringup where the i2c bringup 'just works'. There are always problems there. -- 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: [PATCH v2] [media] marvell-ccic: simplify and fix clk handling (a bit)
Hi Uwe, Thanks for your reviewing. Please see the comments below. On Wed, Sep 25, 2013 at 07:47:19PM -0700, Libin Yang wrote: In the clk enable and prepare function, we will check the NULL pointer. So it should be no problem. I'm not sure what you mean here and unfortunately your quoting style makes your statement appear without context. if (... !IS_ERR(cam-mipi_clk)) { if (cam-mipi_clk) devm_clk_put(mcam-dev, cam-mipi_clk); cam-mipi_clk = NULL; } might work in your setup, but it's wrong usage of the clk API. There is no reason NULL couldn't be a valid clk pointer. Moreover I cannot find a place in that driver that calls prepare and/or enable for the mipi_clk. [Libin] Right. NULL could be a valid clk pointer. In the code, the clk will not be released if mipi_clk is NULL. Is below OK? + if (... !IS_ERR(cam-mipi_clk)) { + devm_clk_put(mcam-dev, cam-mipi_clk); + cam-mipi_clk = NULL; + } Set cam-mipi_clk = NULL is let cam-mipi_clk to be the initial state just like after cam is allocated. (BTW, calling clk_get_rate on a disabled clk is another thing you shouldn't do.) [Libin] Thanks for pointing it out. We enable the clk in other components. Yes, you are right. We should enable the clk explicitly here. For the mipi_clk, it is shared between other components, so we put the clk it we don't use it. There should be no problem if 1 driver holds a reference to the clock. It's clk_disable (+ maybe clk_unprepare) you should call when you don't need it any more. (But even having 1 driver enabling a clk isn't a problem ...) [Libin] So you mean we need not release the clk here and wait for devm to release it later? I will check it with my colleagues to see whether they are OK with this. This patch fixes all but the last issue in this list. This patch doesn't introduce new reasons for not building, but there are already several bulid problems. Care to report those? Sure: CC drivers/media/platform/marvell-ccic/mmp-driver.o drivers/media/platform/marvell-ccic/mmp-driver.c: In function 'mmpcam_calc_dphy': drivers/media/platform/marvell-ccic/mmp-driver.c:264:15: error: 'struct mmp_camera_platform_data' has no member named 'dphy3_algo' drivers/media/platform/marvell-ccic/mmp-driver.c:265:7: error: 'DPHY3_ALGO_PXA910' undeclared (first use in this function) drivers/media/platform/marvell-ccic/mmp-driver.c:265:7: note: each undeclared identifier is reported only once for each function it appears in drivers/media/platform/marvell-ccic/mmp-driver.c:269:8: error: 'struct mmp_camera_platform_data' has no member named 'dphy' drivers/media/platform/marvell-ccic/mmp-driver.c:270:17: error: 'struct mmp_camera_platform_data' has no member named 'lane_clk' drivers/media/platform/marvell-ccic/mmp-driver.c:271:16: error: 'struct mmp_camera_platform_data' has no member named 'lane_clk' drivers/media/platform/marvell-ccic/mmp-driver.c:273:7: error: 'DPHY3_ALGO_PXA2128' undeclared (first use in this function) drivers/media/platform/marvell-ccic/mmp-driver.c:277:8: error: 'struct mmp_camera_platform_data' has no member named 'dphy' drivers/media/platform/marvell-ccic/mmp-driver.c:278:17: error: 'struct mmp_camera_platform_data' has no member named 'lane_clk' drivers/media/platform/marvell-ccic/mmp-driver.c:279:16: error: 'struct mmp_camera_platform_data' has no member named 'lane_clk' drivers/media/platform/marvell-ccic/mmp-driver.c:308:7: error: 'struct mmp_camera_platform_data' has no member named 'dphy' drivers/media/platform/marvell-ccic/mmp-driver.c:312:104: error: 'struct mmp_camera_platform_data' has no member named 'dphy' drivers/media/platform/marvell-ccic/mmp-driver.c:312:120: error: 'struct mmp_camera_platform_data' has no member named 'dphy' drivers/media/platform/marvell-ccic/mmp-driver.c:312:136: error: 'struct mmp_camera_platform_data' has no member named 'dphy' drivers/media/platform/marvell-ccic/mmp-driver.c: In function 'mmpcam_probe': drivers/media/platform/marvell-ccic/mmp-driver.c:385:24: error: 'struct mmp_camera_platform_data' has no member named 'mclk_min' drivers/media/platform/marvell-ccic/mmp-driver.c:386:24: error: 'struct mmp_camera_platform_data' has no member named 'mclk_src' drivers/media/platform/marvell-ccic/mmp-driver.c:387:24: error: 'struct mmp_camera_platform_data' has no member named 'mclk_div' drivers/media/platform/marvell-ccic/mmp-driver.c:388:24: error: 'struct mmp_camera_platform_data' has no member named 'bus_type' drivers/media/platform/marvell-ccic/mmp-driver.c:389:20: error: 'struct mmp_camera_platform_data' has no member named 'dphy' drivers/media/platform/marvell-ccic/mmp-driver.c:391:20: error: 'struct mmp_camera_platform_data' has no member named 'lane' Best regards Uwe --
RE: [PATCH v2] [media] marvell-ccic: simplify and fix clk handling (a bit)
Hi Russell, -Original Message- From: Russell King - ARM Linux [mailto:li...@arm.linux.org.uk] Sent: Thursday, September 26, 2013 4:24 PM To: Uwe Kleine-König Cc: Libin Yang; Jonathan Corbet; Mauro Carvalho Chehab; linux-media@vger.kernel.org; linux-arm-ker...@lists.infradead.org; ker...@pengutronix.de Subject: Re: [PATCH v2] [media] marvell-ccic: simplify and fix clk handling (a bit) On Thu, Sep 26, 2013 at 10:13:56AM +0200, Uwe Kleine-König wrote: Hi Libin, On Wed, Sep 25, 2013 at 07:47:19PM -0700, Libin Yang wrote: In the clk enable and prepare function, we will check the NULL pointer. So it should be no problem. I'm not sure what you mean here and unfortunately your quoting style makes your statement appear without context. if (... !IS_ERR(cam-mipi_clk)) { if (cam-mipi_clk) devm_clk_put(mcam-dev, cam-mipi_clk); cam-mipi_clk = NULL; } might work in your setup, but it's wrong usage of the clk API. There is no reason NULL couldn't be a valid clk pointer. Moreover I cannot find a place in that driver that calls prepare and/or enable for the mipi_clk. (BTW, calling clk_get_rate on a disabled clk is another thing you shouldn't do.) It's a bug for another reason. Consider this: clk = devm_clk_get(...); Now, as the CLK API defines only IS_ERR(clk) to be errors, if clk is NULL then the devm API will allocate a tracking structure for the allocated clock. If you then do: if (!IS_ERR(clk)) { if (clk) devm_clk_put(clk); clk = NULL; } Then this structure won't get freed. Next time you call devm_clk_get(), you'll allocate another tracking structure. If the driver does this a lot, it will spawn lots of these tracking structures which will only get cleaned up when the device is unbound (possibly never.) So, what this driver is doing with its NULL checks against clocks is buggy, no two ways about it. [Libin] Yes, you are right. it will not release the clk tracking structure if it is NULL and may allocate again later. It is a bug. Regards, Libin -- 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 39/51] DMA-API: others: use dma_set_coherent_mask()
Hi, On Friday 20 September 2013 04:41 AM, Russell King wrote: The correct way for a driver to specify the coherent DMA mask is not to directly access the field in the struct device, but to use dma_set_coherent_mask(). Only arch and bus code should access this member directly. Convert all direct write accesses to using the correct API. Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk --- drivers/ata/pata_ixp4xx_cf.c |5 - drivers/gpu/drm/exynos/exynos_drm_drv.c |6 +- drivers/gpu/drm/omapdrm/omap_dmm_tiler.c |5 +++-- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/ata/pata_ixp4xx_cf.c b/drivers/ata/pata_ixp4xx_cf.c index 1ec53f8..ddf470c 100644 --- a/drivers/ata/pata_ixp4xx_cf.c +++ b/drivers/ata/pata_ixp4xx_cf.c @@ -144,6 +144,7 @@ static int ixp4xx_pata_probe(struct platform_device *pdev) struct ata_host *host; struct ata_port *ap; struct ixp4xx_pata_data *data = dev_get_platdata(pdev-dev); + int ret; cs0 = platform_get_resource(pdev, IORESOURCE_MEM, 0); cs1 = platform_get_resource(pdev, IORESOURCE_MEM, 1); @@ -157,7 +158,9 @@ static int ixp4xx_pata_probe(struct platform_device *pdev) return -ENOMEM; /* acquire resources and fill host */ - pdev-dev.coherent_dma_mask = DMA_BIT_MASK(32); + ret = dma_set_coherent_mask(pdev-dev, DMA_BIT_MASK(32)); + if (ret) + return ret; data-cs0 = devm_ioremap(pdev-dev, cs0-start, 0x1000); data-cs1 = devm_ioremap(pdev-dev, cs1-start, 0x1000); diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c index bb82ef7..81192d0 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c @@ -286,7 +286,11 @@ static struct drm_driver exynos_drm_driver = { static int exynos_drm_platform_probe(struct platform_device *pdev) { - pdev-dev.coherent_dma_mask = DMA_BIT_MASK(32); + int ret; + + ret = dma_set_coherent_mask(pdev-dev, DMA_BIT_MASK(32)); + if (ret) + return ret; return drm_platform_init(exynos_drm_driver, pdev); } diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c index acf6678..701c4c1 100644 --- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c +++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c @@ -664,8 +664,9 @@ static int omap_dmm_probe(struct platform_device *dev) } /* set dma mask for device */ - /* NOTE: this is a workaround for the hwmod not initializing properly */ - dev-dev.coherent_dma_mask = DMA_BIT_MASK(32); + ret = dma_set_coherent_mask(dev-dev, DMA_BIT_MASK(32)); + if (ret) + goto fail; Tested with omapdrm on omap4 panda es board. Thanks, Archit -- 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
What are Other video capture settings.
Hi All, Occasionally I see emails suggesting that a video capture card has not been configured properly on Linux and that the problems the questioner is experiencing is down to this. I have no idea what this means. I usually use a gstreamer command like: gst-launch v4l2src device=/dev/video0 ! videorate ! video/x-raw-yuv, framerate=30 ! ffmpegcolorspace ! xvimagesink I also tend to specify the source like this: v4l2-ctrl -d /dev/video0 --set-input=1 To specify S-video or composite if appropriate. Sometime I set gamma, brightness or contrast in the same way. What other settings should I be selecting? Are there any BIOS settings I should know about? Regards Steve -- 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: What are Other video capture settings.
On Thu 26 September 2013 13:04:00 Steve Cookson wrote: Hi All, Occasionally I see emails suggesting that a video capture card has not been configured properly on Linux and that the problems the questioner is experiencing is down to this. I have no idea what this means. Without knowing the exact error message and hardware they used it's hard to say what might be going on. I usually use a gstreamer command like: gst-launch v4l2src device=/dev/video0 ! videorate ! video/x-raw-yuv, framerate=30 ! ffmpegcolorspace ! xvimagesink I also tend to specify the source like this: v4l2-ctrl -d /dev/video0 --set-input=1 To specify S-video or composite if appropriate. Sometime I set gamma, brightness or contrast in the same way. What other settings should I be selecting? Typically the video standard needs to be correct (e.g. PAL vs NTSC), and the video format produced by the video capture card must be supported by the application. Ask them to run 'v4l2-ctl -d /dev/video0 --all', that gives a good overview of the video settings/capabilities. Are there any BIOS settings I should know about? No, the BIOS is no involved in this. 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: What are Other video capture settings.
On 26/09/2013 08:32, Hans Verkuil wrote: Typically the video standard needs to be correct (e.g. PAL vs NTSC), and the video format produced by the video capture card must be supported by the application. Ask them to run 'v4l2-ctl -d /dev/video0 --all', that gives a good overview of the video settings/capabilities. Hi Hans, Thanks for your quick response. Video Standard I understand, in fact I usually do it too, but what is format, are you saying YUV or whatever? How should I specify this? Regards Steve -- 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: What are Other video capture settings.
On Thu 26 September 2013 13:47:26 Steve Cookson wrote: On 26/09/2013 08:32, Hans Verkuil wrote: Typically the video standard needs to be correct (e.g. PAL vs NTSC), and the video format produced by the video capture card must be supported by the application. Ask them to run 'v4l2-ctl -d /dev/video0 --all', that gives a good overview of the video settings/capabilities. Hi Hans, Thanks for your quick response. Video Standard I understand, in fact I usually do it too, but what is format, are you saying YUV or whatever? It's how the actual pixels of the image are laid out in memory. YUV 4:4:4, YUV 4:2:0, YUV 4:1:0, planar, interleaved, RGB, etc. Run 'v4l2-ctl --help-vidcap' (if you have a sufficiently recent v4l2-ctl version) to see all the format-related options. v4l2-ctl -w --list-formats lists the supported video formats. The '-w' option makes v4l2-ctl use the libv4l2 library, which can convert obscure formats to more familiar ones. I assume gstreamer uses libv4l2 as well. See also the documentation on formats here: http://hverkuil.home.xs4all.nl/spec/media.html#pixfmt 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 v8] s5k5baf: add camera sensor driver
Hi Mark, Thanks for the review, sorry for late response. On 09/20/2013 07:06 PM, Mark Rutland wrote: On Fri, Sep 06, 2013 at 11:31:06AM +0100, Andrzej Hajda wrote: Driver for Samsung S5K5BAF UXGA 1/5 2M CMOS Image Sensor with embedded SoC ISP. The driver exposes the sensor as two V4L2 subdevices: - S5K5BAF-CIS - pure CMOS Image Sensor, fixed 1600x1200 format, no controls. - S5K5BAF-ISP - Image Signal Processor, formats up to 1600x1200, pre/post ISP cropping, downscaling via selection API, controls. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Andrzej Hajda a.ha...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- Hi, This is the 8th iteration of the patch. I have applied suggestions from Laurent, Sylwester and Mark, thanks. One exeception, I have left static struct v4l2_rect s5k5baf_cis_rect not const due to fact its address is passed to function which could modify its arguments, of course it never modifies s5k5baf_cis_rect. Regards Andrzej v8 - improved description of data-lanes binding, - added algorithm caching, - added comments to functions, - video bus type checking moved to probe, - clk_get/put moved to probe, - moved streaming checking under mutex, - use proper functions for endian conversion, - probe returns -EPROBE_DEFER in case it cannot get clock, - v4l2_async_unregister_subdev is called on remove, - cosmetic changes v7 - changed description of 'clock-frequency' DT property v6 - endpoint node presence is now optional, - added asynchronous subdev registration support and clock handling, - use named gpios in DT bindings v5 - removed hflip/vflip device tree properties v4 - GPL changed to GPLv2, - bitfields replaced by u8, - cosmetic changes, - corrected s_stream flow, - gpio pins are no longer exported, - added I2C addresses to subdev names, - CIS subdev registration postponed after succesfull HW initialization, - added enums for pads, - selections are initialized only during probe, - default resolution changed to 1600x1200, - state-error pattern removed from few other functions, - entity link creation moved to registered callback. v3: - narrowed state-error usage to i2c and power errors, - private gain controls replaced by red/blue balance user controls, - added checks to devicetree gpio node parsing v2: - lower-cased driver name, - removed underscore from regulator names, - removed platform data code, - v4l controls grouped in anonymous structs, - added s5k5baf_clear_error function, - private controls definitions moved to uapi header file, - added v4l2-controls.h reservation for private controls, - corrected subdev registered/unregistered code, - .log_status sudbev op set to v4l2 helper, - moved entity link creation to probe routines, - added cleanup on error to probe function. --- .../devicetree/bindings/media/samsung-s5k5baf.txt | 58 + MAINTAINERS|7 + drivers/media/i2c/Kconfig |7 + drivers/media/i2c/Makefile |1 + drivers/media/i2c/s5k5baf.c| 2050 5 files changed, 2123 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/samsung-s5k5baf.txt create mode 100644 drivers/media/i2c/s5k5baf.c diff --git a/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt b/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt new file mode 100644 index 000..7704a1e --- /dev/null +++ b/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt @@ -0,0 +1,58 @@ +Samsung S5K5BAF UXGA 1/5 2M CMOS Image Sensor with embedded SoC ISP + + +Required properties: + +- compatible : samsung,s5k5baf; +- reg: I2C slave address of the sensor; +- vdda-supply: analog power supply 2.8V (2.6V to 3.0V); +- vddreg-supply : regulator input power supply 1.8V (1.7V to 1.9V) + or 2.8V (2.6V to 3.0); +- vddio-supply : I/O power supply 1.8V (1.65V to 1.95V) + or 2.8V (2.5V to 3.1V); +- stbyn-gpios: GPIO connected to STDBYN pin; +- rstn-gpios : GPIO connected to RSTN pin; +- clocks : the sensor's master clock specifier (from the common + clock bindings); +- clock-names: must be mclk; I'd reword this slightly: - clocks: clock-specifiers (per the common clock bindings) for the clocks described in clock-names OK - clock-names: should include mclk for the sensor's master clock IMHO it suggests there could be more than one clock, is it OK? + +Optional properties: + +- clock-frequency : the frequency at which the mclk clock should be + configured to operate, in Hz; if this property is not + specified default 24 MHz value will be used. + +The device node should
register ov3640 camera on i2c bus
Hello community, I tried to use my ov3640 camera along with my gumstix overo board and the linux kernel 3.10. So for that I added the appropriate lines into the board-overo.c file, but when I use i2cdetect after startup I get the wrong output: root@overo2:~# i2cdetect -r 3 WARNING! This program can confuse your I2C bus, cause data loss and worse! I will probe file /dev/i2c-3 using read byte commands. I will probe address range 0x03-0x77. Continue? [Y/n] y 0 1 2 3 4 5 6 7 8 9 a b c d e f 00: -- -- -- -- -- -- -- -- -- -- -- -- -- 10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 50: -- 51 -- -- -- -- -- -- -- -- -- -- -- -- -- -- 60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 70: -- -- -- -- -- -- -- -- root@overo2:~# DMESG tells also that the driver couldn't be registered of the ISP. Can someone help me? Did I forget something to register the ov3640 on the ic2 bus? Regrads, Tom DMESG: root@overo2:~# dmesg [0.00] Booting Linux on physical CPU 0x0 [0.00] Linux version 3.10.0 (linuxentwickler@linuxentwickler-OEM) (gcc version 4.3.3 (GCC) ) #15 Thu Sep 26 15:32:18 CEST 2013 [0.00] CPU: ARMv7 Processor [413fc082] revision 2 (ARMv7), cr=10c53c7d [0.00] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache [0.00] Machine: Gumstix Overo [0.00] cma: CMA: reserved 16 MiB at 8ec0 [0.00] Memory policy: ECC disabled, Data cache writeback [0.00] On node 0 totalpages: 56320 [0.00] free_area_init_node: node 0, pgdat c0763f94, node_mem_map c0cc6000 [0.00] Normal zone: 512 pages used for memmap [0.00] Normal zone: 0 pages reserved [0.00] Normal zone: 56320 pages, LIFO batch:15 [0.00] CPU: All CPU(s) started in SVC mode. [0.00] OMAP3630 ES1.2 (l2cache iva sgx neon isp 192mhz_clk ) [0.00] pcpu-alloc: s0 r0 d32768 u32768 alloc=1*32768 [0.00] pcpu-alloc: [0] 0 [0.00] Built 1 zonelists in Zone order, mobility grouping on. Total pages: 55808 [0.00] Kernel command line: mem=93M@0x8000 mem=128M@0x8800 console=ttyO2,115200n8 vram=12M omapfb.mode=dvi:1024x768MR-16@60 omapfb.debug=y omapdss.def_disp=dvi root=/dev/mmcblk0p2 rw rootfstype=ext3 rootwait [0.00] PID hash table entries: 1024 (order: 0, 4096 bytes) [0.00] Dentry cache hash table entries: 32768 (order: 5, 131072 bytes) [0.00] Inode-cache hash table entries: 16384 (order: 4, 65536 bytes) [0.00] Memory: 93MB 127MB = 220MB total [0.00] Memory: 193736k/193736k available, 32568k reserved, 0K highmem [0.00] Virtual kernel memory layout: [0.00] vector : 0x - 0x1000 ( 4 kB) [0.00] fixmap : 0xfff0 - 0xfffe ( 896 kB) [0.00] vmalloc : 0xd080 - 0xff00 ( 744 MB) [0.00] lowmem : 0xc000 - 0xd000 ( 256 MB) [0.00] pkmap : 0xbfe0 - 0xc000 ( 2 MB) [0.00] modules : 0xbf00 - 0xbfe0 ( 14 MB) [0.00] .text : 0xc0008000 - 0xc06bef04 (6876 kB) [0.00] .init : 0xc06bf000 - 0xc07093f4 ( 297 kB) [0.00] .data : 0xc070a000 - 0xc07681e8 ( 377 kB) [0.00].bss : 0xc07681e8 - 0xc0cc53b0 (5493 kB) [0.00] NR_IRQS:16 nr_irqs:16 16 [0.00] IRQ: Found an INTC at 0xfa20 (revision 4.0) with 96 interrupts [0.00] Total of 96 interrupts on 1 active controller [0.00] Clocking rate (Crystal/Core/MPU): 26.0/332/600 MHz [0.00] OMAP clockevent source: timer1 at 32768 Hz [0.00] sched_clock: 32 bits at 32kHz, resolution 30517ns, wraps every 131071999ms [0.00] OMAP clocksource: 32k_counter at 32768 Hz [0.00] Console: colour dummy device 80x30 [0.00] Lock dependency validator: Copyright (c) 2006 Red Hat, Inc., Ingo Molnar [0.00] ... MAX_LOCKDEP_SUBCLASSES: 8 [0.00] ... MAX_LOCK_DEPTH: 48 [0.00] ... MAX_LOCKDEP_KEYS:8191 [0.00] ... CLASSHASH_SIZE: 4096 [0.00] ... MAX_LOCKDEP_ENTRIES: 16384 [0.00] ... MAX_LOCKDEP_CHAINS: 32768 [0.00] ... CHAINHASH_SIZE: 16384 [0.00] memory used by lock dependency info: 3695 kB [0.00] per task-struct memory footprint: 1152 bytes [0.000946] Calibrating delay loop... 398.13 BogoMIPS (lpj=1990656) [0.119720] pid_max: default: 32768 minimum: 301 [0.120025] Security Framework initialized [0.120147] Mount-cache hash table entries: 512 [0.123596] CPU: Testing write buffer coherency: ok [0.125030] Setting up static identity map for 0xc04ddf70 - 0xc04ddfc8 [0.129821] devtmpfs: initialized [0.170440] pinctrl core: initialized
Re: [PATCH v4] media: st-rc: Add ST remote control driver
Hi Stephen, On 24/09/13 20:49, Stephen Warren wrote: Should those property names be prefixed with st,; I assume they're specific to this binding rather than something generic that applies to all IR controller bindings? If you expect them to be generic, it's fine. Officially these bindings are not specified in ePAPR specs Well, there are plenty of properties we now consider generic that aren't in ePAPR... but I see no reason for not having these properties as generic ones. Are you ok with that? I suppose that infrared-vs-uhf is a concept that's probably common enough across any similar HW device, so it may make sense for these properties to be generic. If we do intend them to be generic, I'd suggest they be defined in some generic binding document though; perhaps something like bindings/media/ir.txt or bindings/media/remote-control.txt? That way, a HW-specific binding isn't the only place where a supposedly generic property is defined. For now I will send a v5 for this driver with these generic properties. And, I will send an separate RFC for the generic binding document (bindings/media/remote-control.txt) so that we can get more inputs from others as well. Thanks, srini -- 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] omap3isp : fix image corruption after underrun when using resizer
Hi Laurent, I was able to reliably get corrupted image when placing the pipeline in underrun condition. The pipeline looks like this : YUYV sensor - CCDC - Resizer - V4L output It seems that triggering 'frame sync event' before last line leads to possible corrupted images when using the resizer. With current code, ISP resizer is always configured in oneshot, and must be restarted after each frame. However, has stated by a comment in ispresizer.c, restarting the resizer while a frame is sent to the ccdc leads to corrupted images. The current resizer code takes care of this restart in two places : - in normal situation, when the 'resizer done' IRQ is triggered, a buffer is available and the resizer is restarted in the resizer_isr_buffer function - in underrun situation, no buffer is available when the resizer done irq triggers. After a buffer has eventually been queued, the resizer is restarted on the following frame sync. However, the frame sync event is not generated by the hardware frame sync, but by the VD0 interrupt of the CCDC. But VD0 event is triggered a bit early, since it is configured to trigger after height - 1 lines. It is therefore possible to restart the resizer while a frame's last line is being sent. The following patch configures VD0 to trigger after the last line, and solves the image corruption issue. However, the previous value does not look like an off by one error. What are the reasons to configure VD0 before the last line ? What are the possible issues triggered by a change like this ? Thank you for your comments, Jean-Philippe François Index: b/drivers/media/platform/omap3isp/ispccdc.c === --- a/drivers/media/platform/omap3isp/ispccdc.c +++ b/drivers/media/platform/omap3isp/ispccdc.c @@ -1196,7 +1196,7 @@ /* Generate VD0 on the last line of the image and VD1 on the * 2/3 height line. */ - isp_reg_writel(isp, ((format-height - 2) ISPCCDC_VDINT_0_SHIFT) | + isp_reg_writel(isp, ((format-height - 1) ISPCCDC_VDINT_0_SHIFT) | ((format-height * 2 / 3) ISPCCDC_VDINT_1_SHIFT), OMAP3_ISP_IOMEM_CCDC, ISPCCDC_VDINT); -- 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: Hauppauge HVR-900 HD and HVR 930C-HD with si2165
On 25.09.2013 07:50, Matthias Schwarzott wrote: On 17.08.2013 13:30, Ulf wrote: Hi, I know the topic Hauppauge HVR-900 HD and HVR 930C-HD with si2165 demodulator was already discussed http://permalink.gmane.org/gmane.linux.drivers.video-input-infrastructure/40982 and http://permalink.gmane.org/gmane.linux.drivers.video-input-infrastructure/46266. Just for me as a confirmation nobody plans to work on a driver for si2165. Is there any chance how to push the development? Ulf Hi! I also bought one of these to find out it is not supported. But my plan is to try to write a driver for this. I want to get DVB-C working, but I also have DVB-T and analog reception available. My current status is I got it working in windows in qemu and did a usb snoop. I also have a second system to test it in windows vista directly on the hardware. Current status is documented here. http://www.linuxtv.org/wiki/index.php/Hauppauge_WinTV-HVR-930C-HD Until now I only have a component list summarized from this list. * Conexant http://www.linuxtv.org/wiki/index.php/Conexant CX231xx http://www.linuxtv.org/wiki/index.php/Conexant_CX2310x * Silicon Labs http://www.linuxtv.org/wiki/index.php?title=Silicon_Labsaction=editredlink=1 si2165 http://www.linuxtv.org/wiki/index.php/Silicon_Labs_si2165 (Multi-Standard DVB-T and DVB-C Demodulator) * NXP TDA18271 http://www.linuxtv.org/wiki/index.php/NXP/Philips_TDA182xx (silicon tuner IC, most likely i2c-addr: 0x60) * eeprom (windows driver reads 1kb, i2c-addr: 0x50) Is this correct? Did anyone open his device and can show pictures? I now need to know which component is at which i2c address. Windows driver does upload file hcw10mlD.rom of 16kb to device 0x44. I have opened it. There was similar sandwich PCB than used by rev1 too. So you cannot see all the chip unless you use metal saw to separate PCBs. PCB side A: TDA18271HDC2 16.000 MHz Si2165-GM 16.000 MHz PCB side B: 24C02H regards Antti -- http://palosaari.fi/ -- 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
Capture driver implementation issue/questions
Hi All, I'm working on a video capture driver (my first) for a custom board, and I have a few questions about handling 'overflow' conditions (when the application doesn't get back in time to de-queue every frame). I know that one way to avoid this is to allocate additional frame buffers, but I'm thinking about conditions where even this doesn't provide enough of a FIFO. It looks to me (from studying the videobuf2 code), that if the buffers all fill up (they all end up on the 'done' list), and then the application 'comes back' and starts de-queuing buffers, it will get the OLDEST one first, and then, the newer buffers will be returned, in the order they were originally captured. For some (most?) applications, this is probably what is best, as frames only get dropped when they have to, i.e., when the FIFO overflows, and the app sees the maximum number of frames. But what about applications that always want to see the 'newest' buffer, even if some frames are dropped? What I would like to do is write my driver such that if a new frame is captured before the app has de-queued an earlier frame, the older capture buffer would be removed from the done list and re-queued to the h/w (it's already still on the queued list, I think). The done list would then always contain only 1 frame, and it would be the newest frame captured (and the capture hardware would never run out of capture buffers to use). I think this would be OK as far as the API is concerned - the app shouldn't expect that the buffers will necessarily be returned in the order they were queued, right? So here are the questions: 1. Does this make sense, or am I wanting to do something that isn't reasonable (or do I not understand the framework)? 2. Is there any way to do this within the current videobuf2 framework? 3. If not, do you have any suggestions on changes to make this possible? I'm thinking that we would need a new function that would be called (probably from an ISR, just before calling vb2_buffer_done on the new buffer) that would remove the older buffer from the done queue, re-increment the 'queued_count', and call the 'buf_queue' function provided by the driver to re-queue the older buffer to the h/w. Am I missing anything? Thanks, Rick -- 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] [media] stk1135: fix two warnings added by changeset 76e0598
drivers/media/usb/gspca/stk1135.c:615:6: warning: no previous prototype for 'stk1135_try_fmt' [-Wmissing-prototypes] void stk1135_try_fmt(struct gspca_dev *gspca_dev, struct v4l2_format *fmt) ^ drivers/media/usb/gspca/stk1135.c:627:5: warning: no previous prototype for 'stk1135_enum_framesizes' [-Wmissing-prototypes] int stk1135_enum_framesizes(struct gspca_dev *gspca_dev, Signed-off-by: Mauro Carvalho Chehab m.che...@samsung.com --- drivers/media/usb/gspca/stk1135.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/usb/gspca/stk1135.c b/drivers/media/usb/gspca/stk1135.c index 8add2f7..1fc80af 100644 --- a/drivers/media/usb/gspca/stk1135.c +++ b/drivers/media/usb/gspca/stk1135.c @@ -612,7 +612,7 @@ static int sd_init_controls(struct gspca_dev *gspca_dev) return 0; } -void stk1135_try_fmt(struct gspca_dev *gspca_dev, struct v4l2_format *fmt) +static void stk1135_try_fmt(struct gspca_dev *gspca_dev, struct v4l2_format *fmt) { fmt-fmt.pix.width = clamp(fmt-fmt.pix.width, 32U, 1280U); fmt-fmt.pix.height = clamp(fmt-fmt.pix.height, 32U, 1024U); @@ -624,7 +624,7 @@ void stk1135_try_fmt(struct gspca_dev *gspca_dev, struct v4l2_format *fmt) fmt-fmt.pix.sizeimage = fmt-fmt.pix.width * fmt-fmt.pix.height; } -int stk1135_enum_framesizes(struct gspca_dev *gspca_dev, +static int stk1135_enum_framesizes(struct gspca_dev *gspca_dev, struct v4l2_frmsizeenum *fsize) { if (fsize-index != 0 || fsize-pixel_format != V4L2_PIX_FMT_SBGGR8) -- 1.8.3.1 -- 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 1/1] cx23885-dvb: fix ds3000 ts2020 split for TEVII S471
Hi Christian, Em Wed, 14 Aug 2013 22:58:47 +0200 Christian Volkmann c...@cv-sv.de escreveu: A split for ds3000/ts2020 code forgot to change the TEVII_S471 code. Change the TEVII_S471 according the changes to TEVII_S470. Signed-off-by: Christian Volkmann c...@cv-sv.de --- drivers/media/pci/cx23885/cx23885-dvb.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/media/pci/cx23885/cx23885-dvb.c b/drivers/media/pci/cx23885/cx23885-dvb.c index 9c5ed10..be98c49 100644 --- a/drivers/media/pci/cx23885/cx23885-dvb.c +++ b/drivers/media/pci/cx23885/cx23885-dvb.c @@ -1038,7 +1038,6 @@ static int dvb_register(struct cx23885_tsport *port) tevii_ts2020_config, i2c_bus-i2c_adap); fe0-dvb.frontend-ops.set_voltage = f300_set_voltage; } - break; case CX23885_BOARD_DVBWORLD_2005: i2c_bus = dev-i2c_bus[1]; @@ -1249,6 +1248,11 @@ static int dvb_register(struct cx23885_tsport *port) fe0-dvb.frontend = dvb_attach(ds3000_attach, tevii_ds3000_config, i2c_bus-i2c_adap); + if (fe0-dvb.frontend != NULL) { + dvb_attach(ts2020_attach, fe0-dvb.frontend, + tevii_ts2020_config, i2c_bus-i2c_adap); + fe0-dvb.frontend-ops.set_voltage = f300_set_voltage; + } break; case CX23885_BOARD_PROF_8000: i2c_bus = dev-i2c_bus[0]; A similar patch got applied already, sent by Johannes: commit b43ea8068d2090cb1e44632c8a938ab40d2c7419 Author: Johannes Koch johan...@ortsraum.de Date: Wed Jul 17 14:28:16 2013 -0300 [media] cx23885: Fix TeVii S471 regression since introduction of ts2020 Patch to make TeVii S471 cards use the ts2020 tuner, since ds3000 driver no longer contains tuning code. Signed-off-by: Johannes Koch johan...@ortsraum.de Signed-off-by: Mauro Carvalho Chehab m.che...@samsung.com The difference between your patch and the applied one is: diff --git a/drivers/media/pci/cx23885/cx23885-dvb.c b/drivers/media/pci/cx23885/cx23885-dvb.c index 971e4ff..8ed7b94 100644 --- a/drivers/media/pci/cx23885/cx23885-dvb.c +++ b/drivers/media/pci/cx23885/cx23885-dvb.c @@ -1055,7 +1055,6 @@ static int dvb_register(struct cx23885_tsport *port) tevii_ts2020_config, i2c_bus-i2c_adap); fe0-dvb.frontend-ops.set_voltage = f300_set_voltage; } - break; case CX23885_BOARD_DVBWORLD_2005: i2c_bus = dev-i2c_bus[1]; @@ -1285,6 +1284,7 @@ static int dvb_register(struct cx23885_tsport *port) if (fe0-dvb.frontend != NULL) { dvb_attach(ts2020_attach, fe0-dvb.frontend, tevii_ts2020_config, i2c_bus-i2c_adap); + fe0-dvb.frontend-ops.set_voltage = f300_set_voltage; } break; case CX23885_BOARD_PROF_8000: So, basically, on our patch, you're also filling ops.set_voltage. As I don't know the board details, I can't tell if this is required or not. Christian/Johannes, Could you please double-check it? If this is needed, please send me a new patch, rebased on the top of linux-media git tree. Thanks! 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 v11 2/8] usb: phy: omap-usb2: use the new generic PHY framework
On Wed, Aug 21, 2013 at 11:16:07AM +0530, Kishon Vijay Abraham I wrote: Used the generic PHY framework API to create the PHY. Now the power off and power on are done in omap_usb_power_off and omap_usb_power_on respectively. The omap-usb2 driver is also moved to driver/phy. However using the old USB PHY library cannot be completely removed because OTG is intertwined with PHY and moving to the new framework will break OTG. Once we have a separate OTG state machine, we can get rid of the USB PHY library. Signed-off-by: Kishon Vijay Abraham I kis...@ti.com Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com Acked-by: Felipe Balbi ba...@ti.com --- drivers/phy/Kconfig | 12 + drivers/phy/Makefile |1 + drivers/{usb = }/phy/phy-omap-usb2.c | 45 ++--- drivers/usb/phy/Kconfig | 10 drivers/usb/phy/Makefile |1 - 5 files changed, 54 insertions(+), 15 deletions(-) rename drivers/{usb = }/phy/phy-omap-usb2.c (88%) I tried to apply this to my USB branch, but it fails. Kishon, you were going to refresh this patch series, right? Please do, because as-is, I can't take it. thanks, greg k-h -- 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 00/51] DMA mask changes
2013/9/19 Russell King - ARM Linux li...@arm.linux.org.uk: This email is only being sent to the mailing lists in question, not to anyone personally. The list of individuals is far to great to do that. I'm hoping no mailing lists reject the patches based on the number of recipients. Huh, I think it was enough to send only 3 patches to the b43-dev@. Like: [PATCH 01/51] DMA-API: provide a helper to set both DMA and coherent DMA masks [PATCH 14/51] DMA-API: net: b43: (...) [PATCH 15/51] DMA-API: net: b43legacy: (...) ;) I believe Joe has some nice script for doing it that way. When fixing some coding style / formatting, he sends only related patches to the given ML. -- Rafał -- 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: avermedia A306 / PCIe-minicard (laptop)
Hello The firmware got fixed, the module option, needs a file-name only, no path , lol for once Well, the driver says Firmware OK , I have seen the message of Mauro, he must be right a hundred percent, 'cause we seem to have the initialisations ok, but , for me at lease no data coming out from the tuner ... and it has GPIOs , that have to be used accordinly ... No data meaning no reaction, and no tuning . Otherwise I think we have video from the tuner ( snow ! ) and composite ( parasites ) I will as soon as i can, take macro photos heads/tails , and draw a schematic With the datasheets i have (all ;) ) I will find out the GPIOs where they are going ... :) will keep you all informed of course . Best regards Rémi ### cx25840 2-0044: loaded v4l-cx23885-avcore-01.fw firmware (16382 bytes) [ 4.392762] tuner 1-0061: Tuner -1 found with type(s) Radio TV. [ 4.395040] xc2028 1-0061: creating new instance [ 4.395043] xc2028 1-0061: type set to XCeive xc2028/xc3028 tuner [ 4.395214] cx23885[0]: registered device video1 [v4l2] [ 4.395333] cx23885[0]: registered device vbi0 [ 4.395519] cx23885[0]: registered ALSA audio device [ 4.395870] xc2028 1-0061: Loading 80 firmware images from xc3028-v27.fw, type: xc2028 firmware, ver 2.7 [ 4.598038] xc2028 1-0061: Loading firmware for type=BASE (1), id . [ 5.762199] xc2028 1-0061: Loading firmware for type=(0), id b700. [ 5.777471] SCODE (2000), id b700: [ 5.777474] xc2028 1-0061: Loading SCODE for type=MONO SCODE HAS_IF_4320 (60008000), id 8000. [ 5.923492] cx23885_dev_checkrevision() Hardware revision = 0xb0 [ 5.923499] cx23885[0]/0: found at :05:00.0, rev: 2, irq: 18, latency: 0, mmio: 0xd300 Le 18 septembre 2013 à 16:44, Anca Emanuel anca.eman...@gmail.com a écrit : On Tue, Aug 20, 2013 at 5:44 PM, remi r...@remis.cc wrote: Hello FYI I digged into the firmware problem a little, xc3028L-v36.fw gets loaded by default , and the errors are as you saw earlier forcing the /lib/firmware/xc3028-v27.fw : [ 3569.941404] xc2028 2-0061: Could not load firmware /lib/firmware/xc3028-v27.fw So i searched the original dell/windows driver : I have these files in there : root@medeb:/home/gpunk/.wine/drive_c/dell/drivers/R169070# ls -lR .: total 5468 drwxr-xr-x 2 gpunk gpunk4096 août 20 13:24 Driver_X86 -rwxr-xr-x 1 gpunk gpunk 5589827 sept. 12 2007 Setup.exe -rw-r--r-- 1 gpunk gpunk 197 oct. 9 2007 setup.iss ./Driver_X86: total 1448 -rw-r--r-- 1 gpunk gpunk 114338 sept. 7 2007 A885VCap_ASUS_DELL_2.inf -rw-r--r-- 1 gpunk gpunk 15850 sept. 11 2007 a885vcap.cat -rw-r--r-- 1 gpunk gpunk 733824 sept. 7 2007 A885VCap.sys -rw-r--r-- 1 gpunk gpunk 147870 avril 20 2007 cpnotify.ax -rw-r--r-- 1 gpunk gpunk 376836 avril 20 2007 cx416enc.rom -rw-r--r-- 1 gpunk gpunk 65536 avril 20 2007 cxtvrate.dll -rw-r--r-- 1 gpunk gpunk 16382 avril 20 2007 merlinC.rom I think merlinC.rom is your xc3028-v27.fw Compare it to http://www.linuxtv.org/wiki/index.php/Xceive_XC3028/XC2028 the file extracted there. root@medeb:/home/gpunk/.wine/drive_c/dell/drivers/R169070# root@medeb:/home/gpunk/.wine/drive_c/dell/drivers/R169070/Driver_X86# grep firmware * Fichier binaire A885VCap.sys concordant root@medeb:/home/gpunk/.wine/drive_c/dell/drivers/R169070/Driver_X86# I'll try to find a way to extract maybe the right firmware for what this card , I'd love some help :) Mauro replied this http://www.spinics.net/lists/linux-media/msg25746.html to me in 2010. Then I removed the card from my PC. Some years later I tried again. This time I found this patch to give me some hints: http://www.spinics.net/lists/linux-media/msg43069.html After compiling several versions of the patch for the upstream kernel (try and hope type) I posted what works for me. Tutorial to make kernel patches: http://www.youtube.com/watch?v=LLBrBBImJt4 Tutorial to set git send-email correctly for git: https://coderwall.com/p/dp-gka Tip for first kernel patch: send to your address first to spot any errors. Tip for linux-media patchwork to automatically get yours: use labels (search for discussion about this). I hope this helps. -- 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 31/51] DMA-API: media: omap3isp: use dma_coerce_mask_and_coherent()
Hi Russell, Thank you for the patch. On Thursday 19 September 2013 22:56:02 Russell King wrote: The code sequence: isp-raw_dmamask = DMA_BIT_MASK(32); isp-dev-dma_mask = isp-raw_dmamask; isp-dev-coherent_dma_mask = DMA_BIT_MASK(32); bypasses the architectures check on the DMA mask. It can be replaced with dma_coerce_mask_and_coherent(), avoiding the direct initialization of this mask. Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- drivers/media/platform/omap3isp/isp.c |6 +++--- drivers/media/platform/omap3isp/isp.h |3 --- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c index df3a0ec..1c36080 100644 --- a/drivers/media/platform/omap3isp/isp.c +++ b/drivers/media/platform/omap3isp/isp.c @@ -2182,9 +2182,9 @@ static int isp_probe(struct platform_device *pdev) isp-pdata = pdata; isp-ref_count = 0; - isp-raw_dmamask = DMA_BIT_MASK(32); - isp-dev-dma_mask = isp-raw_dmamask; - isp-dev-coherent_dma_mask = DMA_BIT_MASK(32); + ret = dma_coerce_mask_and_coherent(isp-dev, DMA_BIT_MASK(32)); + if (ret) + return ret; platform_set_drvdata(pdev, isp); diff --git a/drivers/media/platform/omap3isp/isp.h b/drivers/media/platform/omap3isp/isp.h index cd3eff4..ce65d3a 100644 --- a/drivers/media/platform/omap3isp/isp.h +++ b/drivers/media/platform/omap3isp/isp.h @@ -152,7 +152,6 @@ struct isp_xclk { * @mmio_base_phys: Array with physical L4 bus addresses for ISP register * regions. * @mmio_size: Array with ISP register regions size in bytes. - * @raw_dmamask: Raw DMA mask * @stat_lock: Spinlock for handling statistics * @isp_mutex: Mutex for serializing requests to ISP. * @crashed: Bitmask of crashed entities (indexed by entity ID) @@ -190,8 +189,6 @@ struct isp_device { unsigned long mmio_base_phys[OMAP3_ISP_IOMEM_LAST]; resource_size_t mmio_size[OMAP3_ISP_IOMEM_LAST]; - u64 raw_dmamask; - /* ISP Obj */ spinlock_t stat_lock; /* common lock for statistic drivers */ struct mutex isp_mutex; /* For handling ref_count field */ -- 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
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: Fri Sep 27 04:00:13 CEST 2013 git branch: test git hash: ffee921033e64edf8579a3b21c7f15d1a6c3ef71 gcc version:i686-linux-gcc (GCC) 4.8.1 sparse version: 0.4.5-rc1 host hardware: x86_64 host os:3.10.1 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-exynos: OK linux-git-arm-mx: OK linux-git-arm-omap: OK linux-git-arm-omap1: OK linux-git-arm-pxa: OK linux-git-blackfin: OK 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: OK linux-2.6.32.27-i686: OK linux-2.6.33.7-i686: OK linux-2.6.34.7-i686: OK linux-2.6.35.9-i686: OK linux-2.6.36.4-i686: OK linux-2.6.37.6-i686: OK linux-2.6.38.8-i686: OK linux-2.6.39.4-i686: OK linux-3.0.60-i686: OK linux-3.10.1-i686: OK linux-3.1.10-i686: OK linux-3.11.1-i686: OK linux-3.12-rc1-i686: OK linux-3.2.37-i686: OK linux-3.3.8-i686: OK linux-3.4.27-i686: OK linux-3.5.7-i686: OK linux-3.6.11-i686: OK linux-3.7.4-i686: OK linux-3.8-i686: OK linux-3.9.2-i686: OK linux-2.6.31.14-x86_64: OK linux-2.6.32.27-x86_64: OK linux-2.6.33.7-x86_64: OK linux-2.6.34.7-x86_64: OK linux-2.6.35.9-x86_64: OK linux-2.6.36.4-x86_64: OK linux-2.6.37.6-x86_64: OK linux-2.6.38.8-x86_64: OK linux-2.6.39.4-x86_64: OK linux-3.0.60-x86_64: OK linux-3.10.1-x86_64: OK linux-3.1.10-x86_64: OK linux-3.11.1-x86_64: OK linux-3.12-rc1-x86_64: OK linux-3.2.37-x86_64: OK linux-3.3.8-x86_64: OK linux-3.4.27-x86_64: OK linux-3.5.7-x86_64: OK linux-3.6.11-x86_64: OK linux-3.7.4-x86_64: OK linux-3.8-x86_64: OK linux-3.9.2-x86_64: OK apps: WARNINGS spec-git: OK ABI WARNING: change for arm-at91 ABI WARNING: change for arm-davinci ABI WARNING: change for arm-exynos ABI WARNING: change for arm-mx ABI WARNING: change for arm-omap ABI WARNING: change for arm-omap1 ABI WARNING: change for arm-pxa ABI WARNING: change for blackfin ABI WARNING: change for i686 ABI WARNING: change for m32r ABI WARNING: change for mips ABI WARNING: change for powerpc64 ABI WARNING: change for sh ABI WARNING: change for x86_64 sparse version: 0.4.5-rc1 sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Friday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Friday.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
[PATCH] [media] uvcvideo: fix data type for pan/tilt control
The pan/tilt absolute control value is signed value. If minimum value is minus, It will be changed to plus by clamp_t() as commit 64ae9958a62. ([media] uvcvideo: Fix control value clamping for unsigned integer controls). It leads to wrong setting of the control values. For example, when min and max are -36000 and 36000, the setting value between of this range is always 36000. So, Its data type should be changed to signed. Signed-off-by: Chanho Min chanho@lge.com --- drivers/media/usb/uvc/uvc_ctrl.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c index a2f4501..0eb82106 100644 --- a/drivers/media/usb/uvc/uvc_ctrl.c +++ b/drivers/media/usb/uvc/uvc_ctrl.c @@ -664,7 +664,7 @@ static struct uvc_control_mapping uvc_ctrl_mappings[] = { .size = 32, .offset = 0, .v4l2_type = V4L2_CTRL_TYPE_INTEGER, - .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED, + .data_type = UVC_CTRL_DATA_TYPE_SIGNED, }, { .id = V4L2_CID_TILT_ABSOLUTE, @@ -674,7 +674,7 @@ static struct uvc_control_mapping uvc_ctrl_mappings[] = { .size = 32, .offset = 32, .v4l2_type = V4L2_CTRL_TYPE_INTEGER, - .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED, + .data_type = UVC_CTRL_DATA_TYPE_SIGNED, }, { .id = V4L2_CID_PRIVACY, -- 1.7.9.5 -- 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