Re: [RFC] Timestamps and V4L2
On Sat September 22 2012 14:38:07 Sakari Ailus wrote:
Hi Hans,
Thanks for the comments.
Hans Verkuil wrote:
On Thu September 20 2012 22:21:22 Sakari Ailus wrote:
Hi all,
This RFC intends to summarise and further the recent discussion on
linux-media regarding the proposed changes of timestamping V4L2 buffers.
The problem
===
The V4L2 has long used realtime timestamps (such as
clock_gettime(CLOCK_REALTIME, ...)) to stamp the video buffers before
handing them over to the user. This has been found problematic in
associating the video buffers with data from other sources: realtime clock
may jump around due to daylight saving time, for example, and ALSA
(audio-video synchronisation is a common use case) user space API does not
provide the user with realtime timestamps, but instead uses monotonic time
(i.e. clock_gettime(CLOCK_MONOTONIC, ...)).
This is especially an issue in embedded systems where video recording is a
common use case. Drivers typically used in such systems have silently
switched to use monotonic timestamps. While against the spec, this is
necessary for those systems to operate properly.
In general, realtime timestamps are seen of little use in other than
debugging purposes, but monotonic timestamps are fine for that as well.
It's
still possible that an application I'm not aware of uses them in a peculiar
way that would be adversely affected by changing to monotonic timestamps.
Nevertheless, we're not supposed to break the API (or ABI). It'd be also
very important for the application to know what kind of timestamps are
provided by the device.
Requirements, wishes and constraints
Now that it seems to be about the time to fix these issues, it's worth
looking a little bit to the future to anticipate the coming changes to be
able to accommodate them better later on.
- The new default should be monotonic. As the monotonic timestamps are seen
to be the most useful, they should be made the default.
- timeval vs. timespec. The two structs can be used to store timestamp
information. They are not compatible with each other. It's a little bit
uncertain what's the case with all the architectures but it looks like the
timespec fits into the space of timeval in all cases. If timespec is
considered to be used somewhere the compatibility must be ensured. Timespec
is better than timeval since timespec has more precision and it's the same
struct that's used everywhere else in the V4L2 API: timespec does not need
conversion to timespec in the user space.
struct timespec {
__kernel_time_t tv_sec; /* seconds */
longtv_nsec;/* nanoseconds */
};
struct timeval {
__kernel_time_t tv_sec; /* seconds */
__kernel_suseconds_ttv_usec;/* microseconds */
};
To be able to use timespec, the user would have to most likely explicitly
choose to do that.
- Users should know what kind of timestamps the device produces. This
includes existing and future kernels. What should be considered are
uninformed porting drivers back and forth across kernel versions and
out-of-date kernel header files.
- Device-dependent timestamps. Some devices such as the uvcvideo ones
produce device-dependent timestamps for synchronising video and audio, both
produced by the same physical hardware device. For uvcvideo these
timestamps
are unsigned 32-bit integers.
- There's also another clock, Linux-specific raw monotonic clock (as in
clock_gettime(CLOCK_RAW_MONOTONIC, ...)) that could be better in some use
cases than the regular monotonic clock. The difference is that the raw
monotonic clock is free from the NTP adjustments. It would be nice for the
user to be able to choose the clock used for timestamps. This is especially
important for device-dependent timestamps: not all applications can be
expected to be able to use them.
- The field adjacent to timestamp, timecode, is 128 bits wide, and not used
by a single driver. This field could be re-used.
Possible solutions
==
Not all of the solutions below that have been proposed are mutually
exclusive. That's also what's making the choice difficult: the ultimate
solution to the issue of timestamping may involve several of these --- or
possibly something better that's not on the list.
Use of timespec
---
If we can conclude timespec will always fit into the size of timeval (or
timecode) we could use timespec instead. The solution should still make
the use of timespec explicit to the user space. This seems to conflict with
the idea of making monotonic timestamps the default: the default can't be
anything incompatible with timeval, and at the same time it's the most
important that the monotonic timestamps are timespec.
Re: tda8290 regression fix
Em 22-09-2012 11:32, Anders Eriksson escreveu:
Not to my knowledge. It's a standard antenna cable to my cabletv box. I watch
tv over hdmi to get HD. I only use analogue (and this htpc card) to record
stuff.
(please, don't top-post - it makes harder to preserve the history of the
discussions)
Then, maybe that's the reason why you're having troubles with this board.
The tda8290-based devices have two components:
1) a tda8275 tuner, at address 0x61 at the 7-bit I2C address notation
(or 0xc2, at the 8-bit notation);
2) a tda8290 analog demod at address 0x4b (7-bit notation).
Some devices provide a way to send power to a low noise amplifier located at the
antenna or at the device itself (called LNA). The way to activate the LNA is
board-dependent.
On some devices the tda8290 can also be used to enable/disable a linear
amplifier
(LNA). Enabling/disabling the LNA and its gain affects the quality of the
signal.
In the case of tda8275/tda8290 based devices, the LNA setup type is stored at
priv-cfg-config, where:
0 - means no LNA control at all - device won't use it;
1, 2 - LNA is via a pin at tda8290 (GPIO 0):
When config is 1, LNA high gain happens writing a 0;
When config is 2, LNA high gain happens writing a 1;
3 - The LNA gain control is via a pin at saa713x.
For modes 1 and 2, the switch_addr should be equal to 0x4b, as the commands
sent to the device are for the tda8290 chip; sending them to tda8275 will
likely produce no results or would affect something else there.
I suspect that, in the case of your board, the LNA is at the antenna bundled
together with the device. If I'm right, by enabling LNA, your board is sending
some voltage through the cabling (you could easily check it with a voltmeter).
What I think that your patch is actually doing is to disable LNA. As such, it
should be equivalent to:
diff --git a/drivers/media/pci/saa7134/saa7134-cards.c
b/drivers/media/pci/saa7134/saa7134-cards.c
index bc08f1d..98b482e 100644
--- a/drivers/media/pci/saa7134/saa7134-cards.c
+++ b/drivers/media/pci/saa7134/saa7134-cards.c
@@ -3288,13 +3288,13 @@ struct saa7134_board saa7134_boards[] = {
.name = Pinnacle PCTV 310i,
.audio_clock= 0x00187de7,
.tuner_type = TUNER_PHILIPS_TDA8290,
.radio_type = UNSET,
.tuner_addr = ADDR_UNSET,
.radio_addr = ADDR_UNSET,
- .tuner_config = 1,
+ .tuner_config = 0,
.mpeg = SAA7134_MPEG_DVB,
.gpiomask = 0x00020,
.inputs = {{
.name = name_tv,
.vmux = 4,
.amux = TV,
Please test if the above patch fixes the issue you're suffering[1]. If so, then
we'll need to add a modprobe parameter to allow disabling LNA for saa7134
devices
with LNA.
[1] Note: the above is not the fix, as some users of this board may be using the
original antenna, and changing tuner_config will break things for them; the
right
fix is likely to allow controlling the LNA via userspace.
Regards,
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: [RFC] Timestamps and V4L2
Hi Sylwester, On Sat, Sep 22, 2012 at 07:12:52PM +0200, Sylwester Nawrocki wrote: On 09/22/2012 02:38 PM, Sakari Ailus wrote: You are missing one other option: Using v4l2_buffer flags to report the clock --- By defining flags like this: V4L2_BUF_FLAG_CLOCK_MASK 0x7000 /* Possible Clocks */ V4L2_BUF_FLAG_CLOCK_UNKNOWN 0x /* system or monotonic, we don't know */ V4L2_BUF_FLAG_CLOCK_MONOTONIC 0x1000 you could tell the application which clock is used. This does allow for more clocks to be added in the future and clock selection would then be done by a control or possibly an ioctl. For now there are no plans to do such things, so this flag should be sufficient. And it can be implemented very efficiently. It works with existing drivers as well, since they will report CLOCK_UNKNOWN. I am very much in favor of this approach. +1 I think I like this idea best, it's relatively simple (even with adding support for reporting flags in VIDIOC_QUERYBUF) for the purpose. If we ever need the clock selection API I would vote for an IOCTL. The controls API is a bad choice for something such fundamental as type of clock for buffer timestamping IMHO. Let's stop making the controls API a dumping ground for almost everything in V4L2! ;) Why would the control API be worse than an IOCTL for choosing the type of the timestamp? The control API after all has functionality for exactly for this: this is an obvious menu control. What comes to the nature of things that can be configured using controls and what can be done using IOCTLs I see no difference. It's just a mechanism. That's what traditional Unix APIs do in general: provide mechanism, not a policy. Thanks for adding this. I knew I was forgetting something but didn't remember what --- I swear it was unintentional! :-) If we'd add more clocks without providing an ability to choose the clock from the user space, how would the clock be selected? It certainly isn't the driver's job, nor I think it should be system-specific either (platform data on embedded systems). It's up to the application and its needs. That would suggest we should always provide monotonic timestamps to applications (besides a potential driver-specific timestamp), and for that purpose the capability flag --- I admit I disliked the idea at first --- is enough. What comes to buffer flags, the application would also have to receive the first buffer from the device to even know what kind of timestamps the device uses, or at least call QUERYBUF. And in principle the flag should be checked on every buffer, unless we also specify the flag is the same for all buffers. And at certain point this will stop to make any sense... Good point. Perhaps VIDIOC_QUERYBUF and VIDIOC_DQBUF should be reporting timestamps type only for the time they are being called. Not per buffer, per device. And applications would be checking the flags any time they want to find out what is the buffer timestamp type. Or every time if it don't have full control over the device (S/G_PRIORITY). I think we should have valid timestamp flags for every buffer. What I meant to say was that we should say in the definition that the flags will be the same for every buffer --- that will hold until (or if) we provide a mechanism for timestamp source selecton. In that canse the flags will reflect the user-selected timestamp. A capability flag is cleaner solution from this perspective, and it can be amended by a control (or an ioctl) later on: the flag can be disregarded by applications whenever the control is present. If the application doesn't know about the control it can still rely on the flag. (I think this would be less clean than to go for the control right from the beginning, but better IMO.) But with the capability flag we would only be able to report one type of clock, right ? That's true but doesn't that apply to any other non-application-selectable timestamp source, apart from the device dependent timestamps? Device-dependent timestamp -- Should we agree on selectable timestamps, the existing timestamp field (or a union with another field of different type) could be used for the device-dependent timestamps. No. Device timestamps should get their own field. You want to be able to relate device timestamps with the monotonic timestamps, so you need both. Alternatively we can choose to re-use the existing timecode field. At the moment there's no known use case for passing device-dependent timestamps at the same time with monotonic timestamps. Well, the use case is there, but there is no driver support. The device timestamps should be 64 bits to accomodate things like PTS and DTS from MPEG streams. Since timecode is 128 bits we might want to use two u64 fields or perhaps 4 u32 fields.
Re: [RFC] Timestamps and V4L2
Hi Hans, Hans Verkuil wrote: On Sat September 22 2012 14:38:07 Sakari Ailus wrote: Hi Hans, Thanks for the comments. Hans Verkuil wrote: On Thu September 20 2012 22:21:22 Sakari Ailus wrote: ... Capability flag for monotonic timestamps A capability flag can be used to tell whether the timestamp is monotonic. However, it's not extensible cleanly to provide selectable timestamps. These are not features that are needed right now, though. The upside of this option is ease of implementation and use, but it's not extensible. Also we're left with a flag that's set for all drivers: in the end it provides no information to the user and is only noise in the spec. Control for timestamp type -- Using a control to tell the type of the timestamp is extensible but not as easy to implement than the capability flag: each and every device would get an additional control. The value should likely be also file handle specific, and we do not have file handle specific controls yet. Yes, we do. You can make per-file handle controls. M2M devices need that. Thanks for correcting me. I'm not sure why this would be filehandle specific, BTW. Good point. I thought that as other properties of the buffers are specific to file handles, including format when using CREATE_BUFS, it'd make sense to make the timestamp source file-handle specific as well. What do you think? I don't think it makes sense to have different streams from the same device use different clocks. In the meantime the control could be read-only, and later made read-write when the timestamp type can be made selectable. Much of he work of timestamping can be done by the framework: drivers can use a single helper function and need to create one extra standard control. Should the control also have an effect on the types of the timestamps in V4L2 events? Likely yes. You are missing one other option: Using v4l2_buffer flags to report the clock --- By defining flags like this: V4L2_BUF_FLAG_CLOCK_MASK0x7000 /* Possible Clocks */ V4L2_BUF_FLAG_CLOCK_UNKNOWN 0x /* system or monotonic, we don't know */ V4L2_BUF_FLAG_CLOCK_MONOTONIC 0x1000 you could tell the application which clock is used. This does allow for more clocks to be added in the future and clock selection would then be done by a control or possibly an ioctl. For now there are no plans to do such things, so this flag should be sufficient. And it can be implemented very efficiently. It works with existing drivers as well, since they will report CLOCK_UNKNOWN. I am very much in favor of this approach. Thanks for adding this. I knew I was forgetting something but didn't remember what --- I swear it was unintentional! :-) If we'd add more clocks without providing an ability to choose the clock from the user space, how would the clock be selected? It certainly isn't the driver's job, nor I think it should be system-specific either (platform data on embedded systems). IF a driver supports more than one clock (which I really don't see happening anytime soon), then we either need a control to select the clock or an ioctl. The timestamps can be taken from any standard clock using a helper function. Right now drivers call do_gettimeofday() or ktime_get_ts(), it's the same whether the function was called e.g. v4l2_get_ts(). The driver doesn't even need to know which timer is being used to make that timestamp. It used to be the realtime clock and soon it's the monotonic clock. So I really don't see why the timestamp source should depend on the driver. It rather depends on what the user wants. That said, I also don't see use for other clocks _right now_ than the monotonic one. And something as well to enumerate the available clocks. I'm leaning towards ioctls, but I think this should be decided if we ever get an actual use-case for this. I think it should be a control --- I see no reason to add new IOCTLs when controls are equally, or even better, fit for the same job. But let's decide this only when the functionality is needed. It's up to the application and its needs. That would suggest we should always provide monotonic timestamps to applications (besides a potential driver-specific timestamp), and for that purpose the capability flag --- I admit I disliked the idea at first --- is enough. What comes to buffer flags, the application would also have to receive the first buffer from the device to even know what kind of timestamps the device uses, or at least call QUERYBUF. And in principle the flag should be checked on every buffer, unless we also specify the flag is the same for all buffers. And at certain point this will stop to make any sense... It should definitely be the same for all buffers. And since apps will typically call querybuf anyway I don't see this as a problem. These clocks are also specific to the streaming I/O API, so reporting
[PATCH 1/3] ov2640: select sensor register bank before applying h/v-flip settings
We currently don't select the register bank in ov2640_s_ctrl, so we can end up
writing to DSP register 0x04 instead of sensor register 0x04.
This happens for example when calling ov2640_s_ctrl after ov2640_s_fmt.
Signed-off-by: Frank Schäfer fschaefer@googlemail.com
Cc: sta...@kernel.org
---
drivers/media/i2c/soc_camera/ov2640.c |8
1 Datei geändert, 8 Zeilen hinzugefügt(+)
diff --git a/drivers/media/i2c/soc_camera/ov2640.c
b/drivers/media/i2c/soc_camera/ov2640.c
index 78ac574..e4fc79e 100644
--- a/drivers/media/i2c/soc_camera/ov2640.c
+++ b/drivers/media/i2c/soc_camera/ov2640.c
@@ -683,8 +683,16 @@ static int ov2640_s_ctrl(struct v4l2_ctrl *ctrl)
struct v4l2_subdev *sd =
container_of(ctrl-handler, struct ov2640_priv, hdl)-subdev;
struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct regval_list regval;
+ int ret;
u8 val;
+ regval.reg_num = BANK_SEL;
+ regval.value = BANK_SEL_SENS;
+ ret = ov2640_write_array(client, regval);
+ if (ret 0)
+ return ret;
+
switch (ctrl-id) {
case V4L2_CID_VFLIP:
val = ctrl-val ? REG04_VFLIP_IMG : 0x00;
--
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/3] ov2640: add support for V4L2_MBUS_FMT_YUYV8_2X8, V4L2_MBUS_FMT_RGB565_2X8_BE
This is the result of experimenting with the SpeedLink VAD Laplace webcam.
The register sequence for V4L2_MBUS_FMT_YUYV8_2X8 has been identified by
analyzing USB-logs of this device running on MS Windows.
Signed-off-by: Frank Schäfer fschaefer@googlemail.com
---
drivers/media/i2c/soc_camera/ov2640.c | 49 -
1 Datei geändert, 42 Zeilen hinzugefügt(+), 7 Zeilen entfernt(-)
diff --git a/drivers/media/i2c/soc_camera/ov2640.c
b/drivers/media/i2c/soc_camera/ov2640.c
index e4fc79e..182d5a1 100644
--- a/drivers/media/i2c/soc_camera/ov2640.c
+++ b/drivers/media/i2c/soc_camera/ov2640.c
@@ -586,9 +586,20 @@ static const struct regval_list
ov2640_format_change_preamble_regs[] = {
ENDMARKER,
};
-static const struct regval_list ov2640_yuv422_regs[] = {
+static const struct regval_list ov2640_yuyv_regs[] = {
+ { IMAGE_MODE, IMAGE_MODE_YUV422 },
+ { 0xd7, 0x03 },
+ { 0x33, 0xa0 },
+ { 0xe5, 0x1f },
+ { 0xe1, 0x67 },
+ { RESET, 0x00 },
+ { R_BYPASS, R_BYPASS_USE_DSP },
+ ENDMARKER,
+};
+
+static const struct regval_list ov2640_uyvy_regs[] = {
{ IMAGE_MODE, IMAGE_MODE_LBYTE_FIRST | IMAGE_MODE_YUV422 },
- { 0xD7, 0x01 },
+ { 0xd7, 0x01 },
{ 0x33, 0xa0 },
{ 0xe1, 0x67 },
{ RESET, 0x00 },
@@ -596,7 +607,15 @@ static const struct regval_list ov2640_yuv422_regs[] = {
ENDMARKER,
};
-static const struct regval_list ov2640_rgb565_regs[] = {
+static const struct regval_list ov2640_rgb565_be_regs[] = {
+ { IMAGE_MODE, IMAGE_MODE_RGB565 },
+ { 0xd7, 0x03 },
+ { RESET, 0x00 },
+ { R_BYPASS, R_BYPASS_USE_DSP },
+ ENDMARKER,
+};
+
+static const struct regval_list ov2640_rgb565_le_regs[] = {
{ IMAGE_MODE, IMAGE_MODE_LBYTE_FIRST | IMAGE_MODE_RGB565 },
{ 0xd7, 0x03 },
{ RESET, 0x00 },
@@ -605,7 +624,9 @@ static const struct regval_list ov2640_rgb565_regs[] = {
};
static enum v4l2_mbus_pixelcode ov2640_codes[] = {
+ V4L2_MBUS_FMT_YUYV8_2X8,
V4L2_MBUS_FMT_UYVY8_2X8,
+ V4L2_MBUS_FMT_RGB565_2X8_BE,
V4L2_MBUS_FMT_RGB565_2X8_LE,
};
@@ -790,14 +811,22 @@ static int ov2640_set_params(struct i2c_client *client,
u32 *width, u32 *height,
/* select format */
priv-cfmt_code = 0;
switch (code) {
+ case V4L2_MBUS_FMT_RGB565_2X8_BE:
+ dev_dbg(client-dev, %s: Selected cfmt RGB565 BE, __func__);
+ selected_cfmt_regs = ov2640_rgb565_be_regs;
+ break;
case V4L2_MBUS_FMT_RGB565_2X8_LE:
- dev_dbg(client-dev, %s: Selected cfmt RGB565, __func__);
- selected_cfmt_regs = ov2640_rgb565_regs;
+ dev_dbg(client-dev, %s: Selected cfmt RGB565 LE, __func__);
+ selected_cfmt_regs = ov2640_rgb565_le_regs;
+ break;
+ case V4L2_MBUS_FMT_YUYV8_2X8:
+ dev_dbg(client-dev, %s: Selected cfmt YUYV (YUV422),
__func__);
+ selected_cfmt_regs = ov2640_yuyv_regs;
break;
default:
case V4L2_MBUS_FMT_UYVY8_2X8:
- dev_dbg(client-dev, %s: Selected cfmt YUV422, __func__);
- selected_cfmt_regs = ov2640_yuv422_regs;
+ dev_dbg(client-dev, %s: Selected cfmt UYVY, __func__);
+ selected_cfmt_regs = ov2640_uyvy_regs;
}
/* reset hardware */
@@ -862,10 +891,12 @@ static int ov2640_g_fmt(struct v4l2_subdev *sd,
mf-code= priv-cfmt_code;
switch (mf-code) {
+ case V4L2_MBUS_FMT_RGB565_2X8_BE:
case V4L2_MBUS_FMT_RGB565_2X8_LE:
mf-colorspace = V4L2_COLORSPACE_SRGB;
break;
default:
+ case V4L2_MBUS_FMT_YUYV8_2X8:
case V4L2_MBUS_FMT_UYVY8_2X8:
mf-colorspace = V4L2_COLORSPACE_JPEG;
}
@@ -882,11 +913,13 @@ static int ov2640_s_fmt(struct v4l2_subdev *sd,
switch (mf-code) {
+ case V4L2_MBUS_FMT_RGB565_2X8_BE:
case V4L2_MBUS_FMT_RGB565_2X8_LE:
mf-colorspace = V4L2_COLORSPACE_SRGB;
break;
default:
mf-code = V4L2_MBUS_FMT_UYVY8_2X8;
+ case V4L2_MBUS_FMT_YUYV8_2X8:
case V4L2_MBUS_FMT_UYVY8_2X8:
mf-colorspace = V4L2_COLORSPACE_JPEG;
}
@@ -909,11 +942,13 @@ static int ov2640_try_fmt(struct v4l2_subdev *sd,
mf-field = V4L2_FIELD_NONE;
switch (mf-code) {
+ case V4L2_MBUS_FMT_RGB565_2X8_BE:
case V4L2_MBUS_FMT_RGB565_2X8_LE:
mf-colorspace = V4L2_COLORSPACE_SRGB;
break;
default:
mf-code = V4L2_MBUS_FMT_UYVY8_2X8;
+ case V4L2_MBUS_FMT_YUYV8_2X8:
case V4L2_MBUS_FMT_UYVY8_2X8:
mf-colorspace = V4L2_COLORSPACE_JPEG;
}
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to
[PATCH 3/3] ov2640: simplify single register writes
Signed-off-by: Frank Schäfer fschaefer@googlemail.com
---
drivers/media/i2c/soc_camera/ov2640.c | 21 -
1 Datei geändert, 12 Zeilen hinzugefügt(+), 9 Zeilen entfernt(-)
diff --git a/drivers/media/i2c/soc_camera/ov2640.c
b/drivers/media/i2c/soc_camera/ov2640.c
index 182d5a1..70b222f 100644
--- a/drivers/media/i2c/soc_camera/ov2640.c
+++ b/drivers/media/i2c/soc_camera/ov2640.c
@@ -639,17 +639,23 @@ static struct ov2640_priv *to_ov2640(const struct
i2c_client *client)
subdev);
}
+static int ov2640_write_single(struct i2c_client *client, u8 reg, u8 val)
+{
+ int ret;
+
+ ret = i2c_smbus_write_byte_data(client, reg, val);
+ dev_vdbg(client-dev, write: 0x%02x, 0x%02x, reg, val);
+
+ return ret;
+}
+
static int ov2640_write_array(struct i2c_client *client,
const struct regval_list *vals)
{
int ret;
while ((vals-reg_num != 0xff) || (vals-value != 0xff)) {
- ret = i2c_smbus_write_byte_data(client,
- vals-reg_num, vals-value);
- dev_vdbg(client-dev, array: 0x%02x, 0x%02x,
-vals-reg_num, vals-value);
-
+ ret = ov2640_write_single(client, vals-reg_num, vals-value);
if (ret 0)
return ret;
vals++;
@@ -704,13 +710,10 @@ static int ov2640_s_ctrl(struct v4l2_ctrl *ctrl)
struct v4l2_subdev *sd =
container_of(ctrl-handler, struct ov2640_priv, hdl)-subdev;
struct i2c_client *client = v4l2_get_subdevdata(sd);
- struct regval_list regval;
int ret;
u8 val;
- regval.reg_num = BANK_SEL;
- regval.value = BANK_SEL_SENS;
- ret = ov2640_write_array(client, regval);
+ ret = ov2640_write_single(client, BANK_SEL, BANK_SEL_SENS);
if (ret 0)
return ret;
--
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: Gain controls in v4l2-ctrl framework
Hi Prabhakar, Prabhakar Lad wrote: Hi All, The CCD/Sensors have the capability to adjust the R/ye, Gr/Cy, Gb/G, B/Mg gain values. Since these control can be re-usable I am planning to add the following gain controls as part of the framework: 1: V4L2_CID_GAIN_RED 2: V4L2_CID_GAIN_GREEN_RED 3: V4L2_CID_GAIN_GREEN_BLUE 4: V4L2_CID_GAIN_BLUE 5: V4L2_CID_GAIN_OFFSET I need your opinion's to get moving to add them. I think these controls can fit under the image processing controls class --- image processing and not image source since these can also have a digital implementation e.g. in an ISP. Kind regards, -- Sakari Ailus sakari.ai...@iki.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
Re: [PATCH 4/4] gspca_pac7302: add support for green balance adjustment
Hi, Am 20.09.2012 13:54, schrieb Frank Schäfer: Hi, Am 20.09.2012 11:08, schrieb Hans de Goede: Hi, Hans, it seems like you didn't pick up these patches up yet... Is there anything wrong with them ? I've somehow completely missed them. Can you resend the entire set please? No problem, but I can't do that before weekend (I'm currently not at home). I've sent these 4 patches on last Sunday (16. Sept) evening. Maybe you can pick them up from patchwork ? http://patchwork.linuxtv.org/patch/14433/ Ah yes, patchwork, that will work. Unfortunately that only solves the me having missed the patches problem. First of all thanks for working on this! I'm afraid you've managed to find one of the weak spots in the v4l2 API, namely how we deal with RGB gains. It seems like this is one of my talents... :( Many webcams have RGB gains, but we don't have standard CID-s for these, instead we've Blue and Red Balance. This has grown historically because of the bttv cards which actually have Blue and Red balance controls in hardware, rather then the usual RGB gain triplet. Various gspca drivers cope with this in different ways. If you look at the pac7302 driver before your latest 4 patches it has a Red and Blue balance control controlling the Red and Blue gain, and a Whitebalance control, which is not White balance at all, but simply controls the green gain... Ok, so if I understand you right, red+green+blue balance = white balance. And because we already have defined red, blue and whitebalance controls for historical reasons, we don't need green balance ? Maybe that matches physics, but I don't think it's a sane approach for a user interface... And as said other drivers have similar (albeit usually different) hacks. At a minimum I would like you to rework your patches to: 1) Not add the new Green balance, and instead modify the existing whitebalance to control the new green gain you've found. Keeping things as broken as they are, but not worse; and I prefer waiting for the results of the discussion you are proposing further down. 2) Try to use both the page 0 reg 0x01 - 0x03 and page 0 reg 0xc5 - 0xc7 at the same time to get a wider ranged control. Maybe 0xc5 - 0xc7 are simply the most significant bits of a wider ranged gain ? I don't think so. The windows driver does not use them. It even doesn't use the full range of registers 0x01-0x03. Of course, I have expermiented with the full range and it works, but it doesn't make much sense to use it. Experimenting with the device to determine the meaing of unknown registerts, you will notice, that there are several registert which affect RGB. But that doesn't mean that they are suitable for a user control... Note that if you cannot control them both from a single control in such a way that you get a smooth control range, then lets just fix 0xc5 - 0xc7 at a value of 2 for all 3 and be done with it, but at least we should try :) There is no need to fix registers 0xc5 and 0xc7. The Windows driver sets them to 1, which is exactly the value we are currently using as default value with the blue and red value controls. As said the above is the minimum, what I would really like is a discussion on linux-media about adding proper RGB gain controls for all the cameras which have these. Note this brings with itself the question should we export such lowlevel controls at all ? In some drivers the per color gains are simply all kept at the same level and controlled as part of the master gain control, giving it a wider and/or more fine grained range, leading to better autogain behavior. Given how our sw autowhitebalance works (and that that works reasonable well), their is not much added value in exporting them separately, while they do tend to improve the standard gain control when used as part of it ... I would say, let the drivers decide how to do things. It also depends on the hardware design. Regards, Frank So what we really need is a plan how to deal with these controls, and then send an RFC for this to the list. Regards, Hans Hans, I don't have the bandwidth to go through a long discussion in which probably nobody is really interested. So please just drop the last two patches which are related to the V4L2_CID_GREEN_BALANCE issue. I documented the registers and behavior of the Windows driver, so if you one day come to the conclusion that such a control would be usefull, it can be added easily at any time later. But I would really like to see the first two patches getting applied for 3.7. The first one is a documentation fix, the second one clearly improves the behavior of the red and blue balance control (as explained above). I will resend both patches. Regards, Frank -- 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/4] gspca_pac7302: correct register documentation
R,G,B balance registers are 0x01-0x03 instead of 0x02-0x04, which lead to the wrong conclusion that values are inverted. Exposure is controlled via page 3 registers and this is already documented. Also fix a whitespace issue. Signed-off-by: Frank Schäfer fschaefer@googlemail.com --- drivers/media/usb/gspca/pac7302.c | 11 +-- 1 files changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/media/usb/gspca/pac7302.c b/drivers/media/usb/gspca/pac7302.c index 2d5c6d83..4894ac1 100644 --- a/drivers/media/usb/gspca/pac7302.c +++ b/drivers/media/usb/gspca/pac7302.c @@ -29,14 +29,13 @@ * Register page 0: * * Address Description - * 0x02Red balance control - * 0x03Green balance control - * 0x04Blue balance control - * Valus are inverted (0=max, 255=min). + * 0x01Red balance control + * 0x02Green balance control + * 0x03Blue balance control * The Windows driver uses a quadratic approach to map * the settable values (0-200) on register values: - * min=0x80, default=0x40, max=0x20 - * 0x0f-0x20 Colors, saturation and exposure control + * min=0x20, default=0x40, max=0x80 + * 0x0f-0x20 Color and saturation control * 0xa2-0xab Brightness, contrast and gamma control * 0xb6Sharpness control (bits 0-4) * -- 1.7.7 -- 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/4] gspca_pac7302: use registers 0x01 and 0x03 for red and blue balance controls
Currently used registers 0xc5 and 0xc7 provide only a very coarse
adjustment possibility within a very small value range (0-3).
With registers 0x01 and 0x03, a fine grained adjustment with
255 steps is possible. This is also what the Windows driver does.
Signed-off-by: Frank Schäfer fschaefer@googlemail.com
---
drivers/media/usb/gspca/pac7302.c | 51 +
1 files changed, 40 insertions(+), 11 deletions(-)
diff --git a/drivers/media/usb/gspca/pac7302.c
b/drivers/media/usb/gspca/pac7302.c
index 4894ac1..8a0f4d6 100644
--- a/drivers/media/usb/gspca/pac7302.c
+++ b/drivers/media/usb/gspca/pac7302.c
@@ -77,12 +77,12 @@
*
* Page | Register | Function
* -++---
+ * 0 | 0x01 | setredbalance()
+ * 0 | 0x03 | setbluebalance()
* 0 | 0x0f..0x20 | setcolors()
* 0 | 0xa2..0xab | setbrightcont()
* 0 | 0xb6 | setsharpness()
- * 0 | 0xc5 | setredbalance()
* 0 | 0xc6 | setwhitebalance()
- * 0 | 0xc7 | setbluebalance()
* 0 | 0xdc | setbrightcont(), setcolors()
* 3 | 0x02 | setexposure()
* 3 | 0x10, 0x12 | setgain()
@@ -98,10 +98,13 @@
/* Include pac common sof detection functions */
#include pac_common.h
-#define PAC7302_GAIN_DEFAULT 15
-#define PAC7302_GAIN_KNEE 42
-#define PAC7302_EXPOSURE_DEFAULT 66 /* 33 ms / 30 fps */
-#define PAC7302_EXPOSURE_KNEE133 /* 66 ms / 15 fps */
+#define PAC7302_RGB_BALANCE_MIN 0
+#define PAC7302_RGB_BALANCE_MAX200
+#define PAC7302_RGB_BALANCE_DEFAULT100
+#define PAC7302_GAIN_DEFAULT15
+#define PAC7302_GAIN_KNEE 42
+#define PAC7302_EXPOSURE_DEFAULT66 /* 33 ms / 30 fps */
+#define PAC7302_EXPOSURE_KNEE 133 /* 66 ms / 15 fps */
MODULE_AUTHOR(Jean-Francois Moine http://moinejf.free.fr,
Thomas Kaiser tho...@kaiser-linux.li);
@@ -438,12 +441,31 @@ static void setwhitebalance(struct gspca_dev *gspca_dev)
reg_w(gspca_dev, 0xdc, 0x01);
}
+static u8 rgbbalance_ctrl_to_reg_value(s32 rgb_ctrl_val)
+{
+ const unsigned int k = 1000;/* precision factor */
+ unsigned int norm;
+
+ /* Normed value [0...k] */
+ norm = k * (rgb_ctrl_val - PAC7302_RGB_BALANCE_MIN)
+ / (PAC7302_RGB_BALANCE_MAX - PAC7302_RGB_BALANCE_MIN);
+ /* Qudratic apporach improves control at small (register) values: */
+ return 64 * norm * norm / (k*k) + 32 * norm / k + 32;
+ /* Y = 64*X*X + 32*X + 32
+* = register values 0x20-0x80; Windows driver uses these limits */
+
+ /* NOTE: for full value range (0x00-0xff) use
+* Y = 254*X*X + X
+* = 254 * norm * norm / (k*k) + 1 * norm / k*/
+}
+
static void setredbalance(struct gspca_dev *gspca_dev)
{
struct sd *sd = (struct sd *) gspca_dev;
- reg_w(gspca_dev, 0xff, 0x00); /* page 0 */
- reg_w(gspca_dev, 0xc5, sd-red_balance-val);
+ reg_w(gspca_dev, 0xff, 0x00); /* page 0 */
+ reg_w(gspca_dev, 0x01,
+ rgbbalance_ctrl_to_reg_value(sd-red_balance-val));
reg_w(gspca_dev, 0xdc, 0x01);
}
@@ -453,7 +475,8 @@ static void setbluebalance(struct gspca_dev *gspca_dev)
struct sd *sd = (struct sd *) gspca_dev;
reg_w(gspca_dev, 0xff, 0x00); /* page 0 */
- reg_w(gspca_dev, 0xc7, sd-blue_balance-val);
+ reg_w(gspca_dev, 0x03,
+ rgbbalance_ctrl_to_reg_value(sd-blue_balance-val));
reg_w(gspca_dev, 0xdc, 0x01);
}
@@ -642,9 +665,15 @@ static int sd_init_controls(struct gspca_dev *gspca_dev)
V4L2_CID_WHITE_BALANCE_TEMPERATURE,
0, 255, 1, 55);
sd-red_balance = v4l2_ctrl_new_std(hdl, sd_ctrl_ops,
- V4L2_CID_RED_BALANCE, 0, 3, 1, 1);
+ V4L2_CID_RED_BALANCE,
+ PAC7302_RGB_BALANCE_MIN,
+ PAC7302_RGB_BALANCE_MAX,
+ 1, PAC7302_RGB_BALANCE_DEFAULT);
sd-blue_balance = v4l2_ctrl_new_std(hdl, sd_ctrl_ops,
- V4L2_CID_BLUE_BALANCE, 0, 3, 1, 1);
+ V4L2_CID_BLUE_BALANCE,
+ PAC7302_RGB_BALANCE_MIN,
+ PAC7302_RGB_BALANCE_MAX,
+ 1, PAC7302_RGB_BALANCE_DEFAULT);
gspca_dev-autogain = v4l2_ctrl_new_std(hdl, sd_ctrl_ops,
V4L2_CID_AUTOGAIN, 0, 1, 1, 1);
--
1.7.7
--
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
Re: How to add support for the em2765 webcam Speedlink VAD Laplace to the kernel ?
Ping !
Am 26.08.2012 20:53, schrieb Frank Schäfer:
Sorry for the delayed reply, I got distracted by something with higher
prority.
Am 22.08.2012 20:15, schrieb Mauro Carvalho Chehab:
Em 22-08-2012 04:53, Frank Schäfer escreveu:
Am 21.08.2012 19:29, schrieb Mauro Carvalho Chehab:
Hmm... before reading the rest of this email... I found some doc saying
that
em2765 is UVC compliant. Doesn't the uvcdriver work with this device?
Yeah, I stumbled over that, too. :D
But this device is definitely not UVC compliant. Take a look at the
lsusb output.
Maybe they are using a different firmware or something like that, but I
have no idea why the hell they should make a UVC compliant device
non-UVC-compliant...
Another notable difference to the devices we've seen so far is the
em25xx-style EEPROM. Maybe there is a connection.
Btw, do we know any em25xx devices ???
No, I never heard about em25xx. It seems that there are some new em275xx
chips, but I don't have any technical data.
Maybe they changed the name and there was never a em2580/em2585.
But I assume this is an older chip design.
In the mean time I was told that em2580/em2585 devices really exists.
They are used for example in intraoral cameras for dentists.
The em2765 seems to be a kind of relabled em25xx.
Both chips have two i2c busses and work only with 16 bit address
eeproms, which have to be connected to bus A.
The sensor read/write procedure used for this webcam is very likely the
standard method for accessing i2c bus B of these chips.
It COULD also be vendor specific procedure, but I don't think 3 other
slave addresses would be probed in that case...
snip
You'll see several supported devices using the secondary bus for TV and
demod. As, currently, the TV eeprom is not read on those devices, nobody
cared enough to add a separate I2C bus code for it, as all access used
by the driver happen just on the second bus.
Well, the same applies to this webcam. We do not really need to read the
EEPROM at the moment.
A proper mapping for it to use ov2640 driver is to create two i2c
buses, one used by eeprom access, and another one for sensor.
Sure.
Interestingly, the standard I2C reads are used, too, for reading the
EEPROM. So maybe there is a physical difference.
Proprietary is probably not the best name, but I don't have e better
one at the moment (suggestions ?).
It is just another bus: instead of using req 3/4 for read/write, it uses
req 6 for both reads/writes at the i2c-like sensor bus.
- uses 16bit eeprom
- em25xx-eeprom with different layout
There are other supported chips with 16bit eeproms. Currently,
support for 16bit eeproms is disabled just because this weren't
needed so far, but I'm sure this is a need there.
Yes, I've read the comment in em28xx_i2c_eeprom():
...there is the risk that we could corrupt the eeprom (since a 16-bit
read call is interpreted as a write call by 8-bit eeproms)...
How can we know if a device uses an 8bit or 16bit EEPROM ? Can we derive
that from the uses em27xx/28xx-chip ?
I don't know any other way to check it than to read the chip ID, at
register
0x0a. Those are the chip ID's that we currently know:
enum em28xx_chip_id {
CHIP_ID_EM2800 = 7,
CHIP_ID_EM2710 = 17,
CHIP_ID_EM2820 = 18,/* Also used by some em2710 */
CHIP_ID_EM2840 = 20,
CHIP_ID_EM2750 = 33,
CHIP_ID_EM2860 = 34,
CHIP_ID_EM2870 = 35,
CHIP_ID_EM2883 = 36,
CHIP_ID_EM2874 = 65,
CHIP_ID_EM2884 = 68,
CHIP_ID_EM28174 = 113,
};
Even if we add it as a separate driver, it is likely wise to re-use the
registers description at drivers/media/usb/em28xx/em28xx-reg.h, moving it
to drivers/include/media, as em2765 likely uses the same registers.
It also makes sense to add a chip detection at the existing driver,
for it to bail out if it detects an em2765 (and the reverse on the new
driver).
em2765 has chip-id 0x36 = 54.
Do you want me to send a patch ?
Yes, please send it when you'll be ready for driver submission.
Will do that.
Do you really think the em28xx driver should always bail out when it
detects the em2765 ?
Well, having 2 drivers for the same chipset is a very bad idea. Either
one should use it.
Another option would be to have a generic em28xx dispatcher driver
that would handle all of them, probe the board, and then starting
either one, depending if the driver is webcam or not.
Sounds good.
Btw, this is on my TODO list (with low priority), as there are several
devices that have only DVB. So, it makes sense to split the analog
TV driver, just like we did with the DVB and alsa drivers. This way,
an em28xx core driver will contain only the probe and the core functions
like i2c and the common helper functions, while all the rest would be
on separate drivers.
Yeah, a compact bridge module providing chip info, bridge register r/w
functions and access to the 2 + 1 i2c busses sounds good.
If I understand you right, this module should also do the probing and
Re: [PATCH 02/14] davinci: vpfe: add IPIPE hardware layer support
Hi Prabhakar,
Thanks for the patchset! I've got a few comments below.
Prabhakar Lad wrote:
From: Manjunath Hadli manjunath.ha...@ti.com
add dm365 IPIPE hardware support. IPIPE is the hardware IP which
implements the functionality required for resizer, previewer and
the associated feature support. This is built along with the vpfe
driver, and implements hardware setup including coeffcient
programming for various hardware filters, gamma, cfa and clock
enable.
Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com
Signed-off-by: Lad, Prabhakar prabhakar@ti.com
---
drivers/media/platform/davinci/dm365_ipipe_hw.c | 936 +++
drivers/media/platform/davinci/dm365_ipipe_hw.h | 538 +
2 files changed, 1474 insertions(+), 0 deletions(-)
create mode 100644 drivers/media/platform/davinci/dm365_ipipe_hw.c
create mode 100644 drivers/media/platform/davinci/dm365_ipipe_hw.h
diff --git a/drivers/media/platform/davinci/dm365_ipipe_hw.c
b/drivers/media/platform/davinci/dm365_ipipe_hw.c
new file mode 100644
index 000..4ce6d95
--- /dev/null
+++ b/drivers/media/platform/davinci/dm365_ipipe_hw.c
@@ -0,0 +1,936 @@
+/*
+ * Copyright (C) 2012 Texas Instruments Inc
+ *
+ * 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 version 2.
+ *
+ * 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., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * Contributors:
+ * Manjunath Hadli manjunath.ha...@ti.com
+ * Prabhakar Lad prabhakar@ti.com
+ */
+
+#include linux/errno.h
+#include linux/delay.h
+#include linux/device.h
+#include linux/v4l2-mediabus.h
+
+#include dm365_ipipe.h
+#include dm3xx_ipipeif.h
+#include dm365_ipipe_hw.h
+
+static void ipipe_clock_enable(void)
+{
+ /* enable IPIPE MMR for register write access */
+ regw_ip(IPIPE_GCK_MMR_DEFAULT, IPIPE_GCK_MMR);
+ /* enable the clock wb,cfa,dfc,d2f,pre modules */
+ regw_ip(IPIPE_GCK_PIX_DEFAULT, IPIPE_GCK_PIX);
+ /* enable RSZ MMR for register write access */
+}
+
+/* Set input channel format to either 420 Y or C format */
+void rsz_set_in_pix_format(unsigned char y_c)
+{
+ u32 val;
+
+ val = regr_rsz(RSZ_SRC_FMT1);
+ val |= y_c 1;
+ regw_rsz(val, RSZ_SRC_FMT1);
+}
+
+static void rsz_set_common_params(struct ipipe_params *params)
+{
+ struct rsz_common_params *rsz_common = params-rsz_common;
+ u32 val;
+
+ /* Set mode */
+ regw_rsz(params-ipipe_mode, RSZ_SRC_MODE);
+
+ /* data source selection and bypass */
+ val = (rsz_common-passthrough RSZ_BYPASS_SHIFT) |
+ rsz_common-source;
+
+ regw_rsz(val, RSZ_SRC_FMT0);
+ val = regr_rsz(RSZ_SRC_MODE);
val is assigned but there's no need to.
+ /* src image selection */
+ val = (rsz_common-raw_flip 1) |
+ (rsz_common-src_img_fmt RSZ_SRC_IMG_FMT_SHIFT) |
+ ((rsz_common-y_c 1) RSZ_SRC_Y_C_SEL_SHIFT);
+
+ regw_rsz(val, RSZ_SRC_FMT1);
+ regw_rsz(rsz_common-vps IPIPE_RSZ_VPS_MASK, RSZ_SRC_VPS);
+ regw_rsz(rsz_common-hps IPIPE_RSZ_HPS_MASK, RSZ_SRC_HPS);
+ regw_rsz(rsz_common-vsz IPIPE_RSZ_VSZ_MASK, RSZ_SRC_VSZ);
+ regw_rsz(rsz_common-hsz IPIPE_RSZ_HSZ_MASK, RSZ_SRC_HSZ);
+ regw_rsz(rsz_common-yuv_y_min, RSZ_YUV_Y_MIN);
+ regw_rsz(rsz_common-yuv_y_max, RSZ_YUV_Y_MAX);
+ regw_rsz(rsz_common-yuv_c_min, RSZ_YUV_C_MIN);
+ regw_rsz(rsz_common-yuv_c_max, RSZ_YUV_C_MAX);
+ /* chromatic position */
+ regw_rsz(rsz_common-out_chr_pos, RSZ_YUV_PHS);
+ val = regr_rsz(RSZ_SRC_MODE);
Same here.
+}
+
+static void rsz_set_rsz_regs(unsigned int rsz_id, struct ipipe_params *params)
+{
+ struct ipipe_rsz_rescale_param *rsc_params;
+ struct ipipe_ext_mem_param *ext_mem;
+ struct ipipe_rsz_resize2rgb *rgb;
+ u32 reg_base;
+ u32 val;
+
+ val = regr_rsz(RSZ_SEQ);
And here.
+ rsc_params = params-rsz_rsc_param[rsz_id];
+ rgb = params-rsz2rgb[rsz_id];
+ ext_mem = params-ext_mem_param[rsz_id];
+
+ if (rsz_id == RSZ_A) {
+ val = rsc_params-h_flip RSZA_H_FLIP_SHIFT;
+ val |= rsc_params-v_flip RSZA_V_FLIP_SHIFT;
+ reg_base = RSZ_EN_A;
+ } else {
+ val = rsc_params-h_flip RSZB_H_FLIP_SHIFT;
+ val |= rsc_params-v_flip RSZB_V_FLIP_SHIFT;
+ reg_base = RSZ_EN_B;
+ }
+ /* update flip settings */
+ regw_rsz(val, RSZ_SEQ);
Re: [PATCH 00/14] Media Controller capture driver for DM365
Hi Prabhakar, Prabhakar Lad wrote: From: Lad, Prabhakar prabhakar@ti.com This patch set adds media controller based capture driver for DM365. Thanks for the set. Do you happen to have an updated version of the same documentation you posted to the list a while ago? Kind regards, -- Sakari Ailus sakari.ai...@iki.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
Re: Gain controls in v4l2-ctrl framework
Hi, On Sunday 23 September 2012 16:20:06 Sakari Ailus wrote: Prabhakar Lad wrote: Hi All, The CCD/Sensors have the capability to adjust the R/ye, Gr/Cy, Gb/G, B/Mg gain values. Since these control can be re-usable I am planning to add the following gain controls as part of the framework: 1: V4L2_CID_GAIN_RED 2: V4L2_CID_GAIN_GREEN_RED 3: V4L2_CID_GAIN_GREEN_BLUE 4: V4L2_CID_GAIN_BLUE 5: V4L2_CID_GAIN_OFFSET I need your opinion's to get moving to add them. We already have a V4L2_CID_GAIN control and a V4L2_CID_CHROMA_GAIN control in the user controls class. I'd like to document how those controls and the new proposed gain controls interact. At first glance they don't interact at all, devices should not implement both, the user class gain controls are higher- level than the controls you proposed - this should still be documented though, to make sure driver and application authors will not get confused. A couple of quick questions about the new controls. Do we also need a common gain controls for monochrome sensors ? Is the offset always common for the 4 channels, or could devices implement a per-channel offset ? Is the offset applied before or after the gains ? How does it relate to black level compensation ? I think these controls can fit under the image processing controls class --- image processing and not image source since these can also have a digital implementation e.g. in an ISP. Sounds good to me. -- 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] rtl28xxu: [0413:6680] DigitalNow Quad DVB-T Receiver
On 09/21/2012 12:40 AM, Antti Palosaari wrote:
It is 4 x RTL2832U + 4 x FC0012 in one PCIe board.
Of course there is a PCIe USB host controller too.
Big thanks for Darryl Bond reporting and testing that!
Cc: Darryl Bond darryl.b...@gmail.com
Signed-off-by: Antti Palosaari cr...@iki.fi
Maybe proper tags doesn't hurt here.
Tested-by: Darryl Bond darryl.b...@gmail.com
Reported-by: Darryl Bond darryl.b...@gmail.com
---
drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
index 70c2df1..f62cfba 100644
--- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
+++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
@@ -1342,6 +1342,8 @@ static const struct usb_device_id rtl28xxu_id_table[] = {
rtl2832u_props, Trekstor DVB-T Stick Terres 2.0, NULL) },
{ DVB_USB_DEVICE(USB_VID_DEXATEK, 0x1101,
rtl2832u_props, Dexatek DK DVB-T Dongle, NULL) },
+ { DVB_USB_DEVICE(USB_VID_LEADTEK, 0x6680,
+ rtl2832u_props, DigitalNow Quad DVB-T Receiver, NULL) },
{ }
};
MODULE_DEVICE_TABLE(usb, rtl28xxu_id_table);
--
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
Re: ITE9135 on AMD SB700 - ehci_hcd bug
On 09/16/2012 05:23 PM, Antti Palosaari wrote: On 09/12/2012 09:32 AM, Marx wrote: Hello I'm trying to use dual DVB-T tuner based on ITE9135 tuner. I use Debian kernel 3.5-trunk-686-pae. My motherboard is AsRock E350M1 (no USB3 ports). Tuner is detected ok, see log at the end of post. When I try to scan channels, bug happens: Sep 11 17:16:31 wuwek kernel: [ 209.291329] ehci_hcd :00:13.2: force halt; handshake f821a024 4000 - -110 Sep 11 17:16:31 wuwek kernel: [ 209.291401] ehci_hcd :00:13.2: HC died; cleaning up Sep 11 17:16:31 wuwek kernel: [ 209.291606] usb 2-3: USB disconnect, device number 2 Sep 11 17:16:41 wuwek kernel: [ 219.312848] dvb-usb: error while stopping stream. Sep 11 17:16:41 wuwek kernel: [ 219.320585] dvb-usb: ITE 9135(9006) Generic successfully deinitialized and disconnected. After trying many ways I've read about problems with ehci on SB700 based boards and switched off ehci via command sh -c 'echo -n :00:13.2 unbind' and now ehci bug doesn't happen. Of course I can see only one tuner and in slower USB mode (see log at the end). But now I can scan succesfully without any errors. Of course it isn't acceptable fix for my problem. Drivers for ITE9135 seems ok, but there is a problem with ehci_hcd on my motherboard. I would like to know what can I do to fix my problem. I am quite sure dvb_usb_v2 fixes that. Test latest tree. Antti Test results please? 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
Re: Gain controls in v4l2-ctrl framework
Hi Laurent, Laurent Pinchart wrote: Hi, On Sunday 23 September 2012 16:20:06 Sakari Ailus wrote: Prabhakar Lad wrote: Hi All, The CCD/Sensors have the capability to adjust the R/ye, Gr/Cy, Gb/G, B/Mg gain values. Since these control can be re-usable I am planning to add the following gain controls as part of the framework: 1: V4L2_CID_GAIN_RED 2: V4L2_CID_GAIN_GREEN_RED 3: V4L2_CID_GAIN_GREEN_BLUE 4: V4L2_CID_GAIN_BLUE 5: V4L2_CID_GAIN_OFFSET I need your opinion's to get moving to add them. We already have a V4L2_CID_GAIN control and a V4L2_CID_CHROMA_GAIN control in the user controls class. I'd like to document how those controls and the new proposed gain controls interact. At first glance they don't interact at all, devices should not implement both, the user class gain controls are higher- level than the controls you proposed - this should still be documented though, to make sure driver and application authors will not get confused. A couple of quick questions about the new controls. Do we also need a common gain controls for monochrome sensors ? Is the offset always common for the 4 I think we should have a common gain control for sensors in general, whether monochrome or not. Many sensors support global digital gain, either only or besides the per-channel gains. Kind regards, -- Sakari Ailus sakari.ai...@iki.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
Re: [PATCH] v3 Add support to Avermedia Twinstar double tuner in af9035
Hello Jose,
Could you try to split and resent?
I will get af9035 + fc0012 dual tuner next week and add support for it.
I wish to use your patch for dual mode, but I as there is that
unresolved MXL5007t dependency I cannot user it currently.
regards
Antti
On 09/15/2012 08:45 PM, Antti Palosaari wrote:
Hello
Could you split that patch to 2?
1) mxl5007t changes
2) af9035/af9033 dual mode
3) af9035/af9033 changes needed for mxl5007t
I cannot say much about tuner changes, but I still wonder are those
really needed as this device is already supported. Is it broken currently?
What happens when no_probe = 0 ?
What happens when no_reset = 0 ?
Soft reset means usually resetting chip state machine. It is something
like start operating command. First program registers then issue soft
reset in order to restart state machine.
regards
Antti
On 08/30/2012 02:02 AM, Jose Alberto Reguero wrote:
This patch add support to the Avermedia Twinstar double tuner in the
af9035
driver. Version 3 of the patch.
Signed-off-by: Jose Alberto Reguero jaregu...@telefonica.net
Jose Alberto
diff -upr linux/drivers/media/dvb-frontends/af9033.c
linux.new/drivers/media/dvb-frontends/af9033.c
--- linux/drivers/media/dvb-frontends/af9033.c2012-08-14
05:45:22.0 +0200
+++ linux.new/drivers/media/dvb-frontends/af9033.c2012-08-29
16:00:52.020523899 +0200
@@ -326,6 +326,18 @@ static int af9033_init(struct dvb_fronte
goto err;
}
+if (state-cfg.ts_mode == AF9033_TS_MODE_SERIAL) {
+ret = af9033_wr_reg_mask(state, 0x00d91c, 0x01, 0x01);
+if (ret 0)
+goto err;
+ret = af9033_wr_reg_mask(state, 0x00d917, 0x00, 0x01);
+if (ret 0)
+goto err;
+ret = af9033_wr_reg_mask(state, 0x00d916, 0x00, 0x01);
+if (ret 0)
+goto err;
+}
+
state-bandwidth_hz = 0; /* force to program all parameters */
return 0;
diff -upr linux/drivers/media/tuners/mxl5007t.c
linux.new/drivers/media/tuners/mxl5007t.c
--- linux/drivers/media/tuners/mxl5007t.c2012-08-14
05:45:22.0 +0200
+++ linux.new/drivers/media/tuners/mxl5007t.c2012-08-29
13:07:57.299884405 +0200
@@ -374,7 +374,6 @@ static struct reg_pair_t *mxl5007t_calc_
mxl5007t_set_if_freq_bits(state, cfg-if_freq_hz, cfg-invert_if);
mxl5007t_set_xtal_freq_bits(state, cfg-xtal_freq_hz);
-set_reg_bits(state-tab_init, 0x04, 0x01, cfg-loop_thru_enable);
set_reg_bits(state-tab_init, 0x03, 0x08, cfg-clk_out_enable
3);
set_reg_bits(state-tab_init, 0x03, 0x07, cfg-clk_out_amp);
@@ -531,9 +530,11 @@ static int mxl5007t_tuner_init(struct mx
struct reg_pair_t *init_regs;
int ret;
-ret = mxl5007t_soft_reset(state);
-if (mxl_fail(ret))
-goto fail;
+if (!state-config-no_reset) {
+ret = mxl5007t_soft_reset(state);
+if (mxl_fail(ret))
+goto fail;
+}
/* calculate initialization reg array */
init_regs = mxl5007t_calc_init_regs(state, mode);
@@ -887,7 +888,11 @@ struct dvb_frontend *mxl5007t_attach(str
if (fe-ops.i2c_gate_ctrl)
fe-ops.i2c_gate_ctrl(fe, 1);
-ret = mxl5007t_get_chip_id(state);
+if (!state-config-no_probe)
+ret = mxl5007t_get_chip_id(state);
+
+ret = mxl5007t_write_reg(state, 0x04,
+state-config-loop_thru_enable);
if (fe-ops.i2c_gate_ctrl)
fe-ops.i2c_gate_ctrl(fe, 0);
diff -upr linux/drivers/media/tuners/mxl5007t.h
linux.new/drivers/media/tuners/mxl5007t.h
--- linux/drivers/media/tuners/mxl5007t.h2012-08-14
05:45:22.0 +0200
+++ linux.new/drivers/media/tuners/mxl5007t.h2012-08-25
19:38:19.990920623 +0200
@@ -73,8 +73,10 @@ struct mxl5007t_config {
enum mxl5007t_xtal_freq xtal_freq_hz;
enum mxl5007t_if_freq if_freq_hz;
unsigned int invert_if:1;
-unsigned int loop_thru_enable:1;
+unsigned int loop_thru_enable:2;
unsigned int clk_out_enable:1;
+unsigned int no_probe:1;
+unsigned int no_reset:1;
};
#if defined(CONFIG_MEDIA_TUNER_MXL5007T) ||
(defined(CONFIG_MEDIA_TUNER_MXL5007T_MODULE) defined(MODULE))
diff -upr linux/drivers/media/usb/dvb-usb-v2/af9035.c
linux.new/drivers/media/usb/dvb-usb-v2/af9035.c
--- linux/drivers/media/usb/dvb-usb-v2/af9035.c2012-08-16
05:45:24.0 +0200
+++ linux.new/drivers/media/usb/dvb-usb-v2/af9035.c2012-08-29
19:20:00.862523903 +0200
@@ -209,10 +209,14 @@ static int af9035_i2c_master_xfer(struct
if (msg[0].len 40 || msg[1].len 40) {
/* TODO: correct limits 40 */
ret = -EOPNOTSUPP;
-} else if (msg[0].addr == state-af9033_config[0].i2c_addr) {
+} else if ((msg[0].addr == state-af9033_config[0].i2c_addr) ||
+ (msg[0].addr == state-af9033_config[1].i2c_addr)) {
/* integrated demod */
u32 reg = msg[0].buf[0] 16 | msg[0].buf[1] 8 |
msg[0].buf[2];
+
Re: [PATCH v2 3/4] sta2x11_vip: convert to videobuf2 and control framework
+struct sta2x11_vip_fh {
+ struct v4l2_fh fh;
+};
No need to make a sta2x11_vip_fh struct, just use v4l2_fh directly. It
doesn't add anything. In fact, it's not even used.
Thank you :)
static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
struct v4l2_format *f)
{
- struct video_device *dev = priv;
- struct sta2x11_vip *vip = video_get_drvdata(dev);
+ struct sta2x11_vip *vip = video_drvdata(file);
int interlace_lim;
- if (V4L2_PIX_FMT_UYVY != f-fmt.pix.pixelformat)
- return -EINVAL;
-
if (V4L2_STD_525_60 vip-std)
interlace_lim = 240;
else
@@ -827,6 +522,8 @@ static int vidioc_try_fmt_vid_cap(struct file
*file, void *priv,
return -EINVAL;
No -EINVAL allowed anymore in try_fmt_vid_cap. I generally handle
unknown field values in try_fmt_vid_cap as if FIELD_ANY was
specified.
ok, unknown - any
memcpy(vip-format, f-fmt.pix, sizeof(struct
v4l2_pix_format));
Just use an assignment: vip-format = f-fmt.pix
memcpy(f-fmt.pix, vip-format, sizeof(struct
v4l2_pix_format));
Assignment
Fixed
-
static const struct v4l2_ioctl_ops vip_ioctl_ops = {
.vidioc_querycap = vidioc_querycap,
- .vidioc_s_std = vidioc_s_std,
+ /* FMT handling */
+ .vidioc_enum_fmt_vid_cap = vidioc_enum_fmt_vid_cap,
+ .vidioc_g_fmt_vid_cap = vidioc_g_fmt_vid_cap,
+ .vidioc_s_fmt_vid_cap = vidioc_s_fmt_vid_cap,
+ .vidioc_try_fmt_vid_cap = vidioc_try_fmt_vid_cap,
+ /* Buffer handlers */
+ .vidioc_reqbufs = vb2_ioctl_reqbufs,
+ .vidioc_querybuf = vb2_ioctl_querybuf,
+ .vidioc_qbuf = vb2_ioctl_qbuf,
+ .vidioc_dqbuf = vb2_ioctl_dqbuf,
+ .vidioc_create_bufs = vb2_ioctl_create_bufs,
If you want to use create_bufs, then in queue_setup() you need to
handle the fmt argument. See e.g. vivi.c for an example.
Fixed
I will send a patch v3 tomorrow
--
Federico Vaga
--
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
Fwd: [PATCH 5/5] drivers/media/platform/omap3isp/isp.c: fix error return code
Laurent,
Could you please review this patch?
Peter,
Please, always c/c the driver maintainer/author on patches you submit.
You can check it with scripts/get_maintainer.pl.
Regards,
Mauro
Mensagem original
Assunto: [PATCH 5/5] drivers/media/platform/omap3isp/isp.c: fix error return
code
Data: Tue, 4 Sep 2012 18:14:25 +0200
De: Peter Senna Tschudin peter.se...@gmail.com
Para: Mauro Carvalho Chehab mche...@infradead.org
CC: kernel-janit...@vger.kernel.org, julia.law...@lip6.fr,
linux-media@vger.kernel.org, linux-ker...@vger.kernel.org
From: Peter Senna Tschudin peter.se...@gmail.com
Convert a nonnegative error return code to a negative one, as returned
elsewhere in the function.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// smpl
(
if@p1 (\(ret 0\|ret != 0\))
{ ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != ret
*if(...)
{
... when != ret = e2
when forall
return ret;
}
// /smpl
Signed-off-by: Peter Senna Tschudin peter.se...@gmail.com
---
drivers/media/platform/omap3isp/isp.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/omap3isp/isp.c
b/drivers/media/platform/omap3isp/isp.c
index e0096e0..91fcaef 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2102,8 +2102,10 @@ static int __devinit isp_probe(struct platform_device
*pdev)
if (ret 0)
goto error;
- if (__omap3isp_get(isp, false) == NULL)
+ if (__omap3isp_get(isp, false) == NULL) {
+ ret = -EBUSY; /* Not sure if EBUSY is best for here */
goto error;
+ }
ret = isp_reset(isp);
if (ret 0)
--
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: [media-workshop] [ANNOUNCE] media workshop in November
Em 20-09-2012 18:19, Alain VOLMAT escreveu: Hi Mauro, I'd like to attend this one (couldn't attend the one last month in San-Diego). That would be me and possibly another member (Silvano Vigna) also. Amount various parts we have in our LinuxDVB/V4L2 model that need discussion with you, we would particularly like to discuss about a TSMux (a Mux rather than a demux) device within LinuxTV. Can you confirm the possibility of our attendance? Sure. It will be a please to meet you there. What's Silvano's email? Please ask him to also subscribe to media-workshop ML. Regards, Alain -Original Message- From: media-workshop-boun...@linuxtv.org [mailto:media-workshop-boun...@linuxtv.org] On Behalf Of Mauro Carvalho Chehab Sent: mercredi 19 septembre 2012 10:21 To: media-works...@linuxtv.org; Linux Media Mailing List Subject: Re: [media-workshop] [ANNOUNCE] media workshop in November Em 19-09-2012 05:11, Mauro Carvalho Chehab escreveu: Dear developers, We're feeling the need for one more media workshop this year. As there will be already several developers going to LinuxCon Europe and Embedded Linux Conference Europe, we'll be co-locating the workshop together with those two events. As there will be several developers speaking about the media subsystem at both LinuxCon and ELCE, we decided to take just one day (September, 8th) Sorry, I meant November, 8th. for the media workshop (while we expect that we'll likely have some other discussions during the week). In order to finish the arrangements, I need to know who will be attending, and also we need to receive the theme proposals. Please estimate how long do you think that it would be needed for the proposed theme presentation and discussions. I have a theme proposal already: How to improve the patch submission workflow for media patches - 2 hours. So, please confirm your intention to be there and propose the themes of your interests to media-works...@linuxtv.org mailing list. Thanks! Mauro ___ media-workshop mailing list media-works...@linuxtv.org http://www.linuxtv.org/cgi-bin/mailman/listinfo/media-workshop ___ media-workshop mailing list media-works...@linuxtv.org http://www.linuxtv.org/cgi-bin/mailman/listinfo/media-workshop -- 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 5/5] drivers/media/platform/omap3isp/isp.c: fix error return code
On Sun, Sep 23, 2012 at 7:39 PM, Mauro Carvalho Chehab
mche...@redhat.com wrote:
Laurent,
Could you please review this patch?
Peter,
Please, always c/c the driver maintainer/author on patches you submit.
You can check it with scripts/get_maintainer.pl.
Sorry for that. I'll be more careful next time. Thanks!
Regards,
Mauro
Mensagem original
Assunto: [PATCH 5/5] drivers/media/platform/omap3isp/isp.c: fix error return
code
Data: Tue, 4 Sep 2012 18:14:25 +0200
De: Peter Senna Tschudin peter.se...@gmail.com
Para: Mauro Carvalho Chehab mche...@infradead.org
CC: kernel-janit...@vger.kernel.org, julia.law...@lip6.fr,
linux-media@vger.kernel.org, linux-ker...@vger.kernel.org
From: Peter Senna Tschudin peter.se...@gmail.com
Convert a nonnegative error return code to a negative one, as returned
elsewhere in the function.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// smpl
(
if@p1 (\(ret 0\|ret != 0\))
{ ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != ret
*if(...)
{
... when != ret = e2
when forall
return ret;
}
// /smpl
Signed-off-by: Peter Senna Tschudin peter.se...@gmail.com
---
drivers/media/platform/omap3isp/isp.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/omap3isp/isp.c
b/drivers/media/platform/omap3isp/isp.c
index e0096e0..91fcaef 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2102,8 +2102,10 @@ static int __devinit isp_probe(struct platform_device
*pdev)
if (ret 0)
goto error;
- if (__omap3isp_get(isp, false) == NULL)
+ if (__omap3isp_get(isp, false) == NULL) {
+ ret = -EBUSY; /* Not sure if EBUSY is best for here */
goto error;
+ }
ret = isp_reset(isp);
if (ret 0)
--
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
--
Peter
--
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: tda8290 regression fix
On 2012-09-23 13:36, Mauro Carvalho Chehab wrote:
Em 22-09-2012 11:32, Anders Eriksson escreveu:
Not to my knowledge. It's a standard antenna cable to my cabletv box. I
watch tv over hdmi to get HD. I only use analogue (and this htpc card) to record
stuff.
(please, don't top-post - it makes harder to preserve the history of the
discussions)
Sorry about that. I was using my notsosmartphone.
Then, maybe that's the reason why you're having troubles with this board.
The tda8290-based devices have two components:
1) a tda8275 tuner, at address 0x61 at the 7-bit I2C address notation
(or 0xc2, at the 8-bit notation);
2) a tda8290 analog demod at address 0x4b (7-bit notation).
Some devices provide a way to send power to a low noise amplifier located at the
antenna or at the device itself (called LNA). The way to activate the LNA is
board-dependent.
On some devices the tda8290 can also be used to enable/disable a linear
amplifier
(LNA). Enabling/disabling the LNA and its gain affects the quality of the
signal.
In the case of tda8275/tda8290 based devices, the LNA setup type is stored at
priv-cfg-config, where:
0 - means no LNA control at all - device won't use it;
1, 2 - LNA is via a pin at tda8290 (GPIO 0):
When config is 1, LNA high gain happens writing a 0;
When config is 2, LNA high gain happens writing a 1;
3 - The LNA gain control is via a pin at saa713x.
For modes 1 and 2, the switch_addr should be equal to 0x4b, as the commands
sent to the device are for the tda8290 chip; sending them to tda8275 will
likely produce no results or would affect something else there.
I suspect that, in the case of your board, the LNA is at the antenna bundled
together with the device. If I'm right, by enabling LNA, your board is sending
some voltage through the cabling (you could easily check it with a voltmeter).
I actually have a multimeter somewhere. We're talking about the
antenna-in (unconnected) on the card, right? And what voltages
should I expect?
What I think that your patch is actually doing is to disable LNA. As such, it
should be equivalent to:
diff --git a/drivers/media/pci/saa7134/saa7134-cards.c
b/drivers/media/pci/saa7134/saa7134-cards.c
index bc08f1d..98b482e 100644
--- a/drivers/media/pci/saa7134/saa7134-cards.c
+++ b/drivers/media/pci/saa7134/saa7134-cards.c
@@ -3288,13 +3288,13 @@ struct saa7134_board saa7134_boards[] = {
.name = Pinnacle PCTV 310i,
.audio_clock= 0x00187de7,
.tuner_type = TUNER_PHILIPS_TDA8290,
.radio_type = UNSET,
.tuner_addr = ADDR_UNSET,
.radio_addr = ADDR_UNSET,
- .tuner_config = 1,
+ .tuner_config = 0,
.mpeg = SAA7134_MPEG_DVB,
.gpiomask = 0x00020,
.inputs = {{
.name = name_tv,
.vmux = 4,
.amux = TV,
Please test if the above patch fixes the issue you're suffering[1]. If so, then
we'll need to add a modprobe parameter to allow disabling LNA for saa7134
devices
with LNA.
[1] Note: the above is not the fix, as some users of this board may be using the
original antenna, and changing tuner_config will break things for them; the
right
fix is likely to allow controlling the LNA via userspace.
Tried that patch on 3.5.3. No improvement, unfortunately.
Regards,
/Anders
--
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: tda8290 regression fix
Em 23-09-2012 14:54, Anders Thomson escreveu:
On 2012-09-23 13:36, Mauro Carvalho Chehab wrote:
Em 22-09-2012 11:32, Anders Eriksson escreveu:
I suspect that, in the case of your board, the LNA is at the antenna bundled
together with the device. If I'm right, by enabling LNA, your board is
sending
some voltage through the cabling (you could easily check it with a
voltmeter).
I actually have a multimeter somewhere. We're talking about the
antenna-in (unconnected) on the card, right? And what voltages
should I expect?
zero (or close to zero) if LNA is disabled; some volts when LNA is enabled ;)
According with Wikipedia[1]:
Usually LNA require less operating voltage in the range of 2 .. 10 V. MAX 2640
operate at +2.7 .. +5.5 V.
[1] http://en.wikipedia.org/wiki/Low-noise_amplifier
(Satellites amplifiers are typically 13V-18V - I never actually tried to use
LNA for
terrestrial systems).
What I think that your patch is actually doing is to disable LNA. As such, it
should be equivalent to:
diff --git a/drivers/media/pci/saa7134/saa7134-cards.c
b/drivers/media/pci/saa7134/saa7134-cards.c
index bc08f1d..98b482e 100644
--- a/drivers/media/pci/saa7134/saa7134-cards.c
+++ b/drivers/media/pci/saa7134/saa7134-cards.c
@@ -3288,13 +3288,13 @@ struct saa7134_board saa7134_boards[] = {
.name = Pinnacle PCTV 310i,
.audio_clock= 0x00187de7,
.tuner_type = TUNER_PHILIPS_TDA8290,
.radio_type = UNSET,
.tuner_addr = ADDR_UNSET,
.radio_addr = ADDR_UNSET,
-.tuner_config = 1,
+.tuner_config = 0,
.mpeg = SAA7134_MPEG_DVB,
.gpiomask = 0x00020,
.inputs = {{
.name = name_tv,
.vmux = 4,
.amux = TV,
Please test if the above patch fixes the issue you're suffering[1]. If so,
then
we'll need to add a modprobe parameter to allow disabling LNA for saa7134
devices
with LNA.
[1] Note: the above is not the fix, as some users of this board may be using
the
original antenna, and changing tuner_config will break things for them; the
right
fix is likely to allow controlling the LNA via userspace.
Tried that patch on 3.5.3. No improvement, unfortunately.
That's weird. Well, then we need to read tda827x datasheets and to try get
information
data from Pinnacle about this specific device configuration.
Regards,
/Anders
--
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: tda8290 regression fix
On 2012-09-23 19:54, Anders Thomson wrote:
diff --git a/drivers/media/pci/saa7134/saa7134-cards.c
b/drivers/media/pci/saa7134/saa7134-cards.c
index bc08f1d..98b482e 100644
--- a/drivers/media/pci/saa7134/saa7134-cards.c
+++ b/drivers/media/pci/saa7134/saa7134-cards.c
@@ -3288,13 +3288,13 @@ struct saa7134_board saa7134_boards[] = {
.name = Pinnacle PCTV 310i,
.audio_clock= 0x00187de7,
.tuner_type = TUNER_PHILIPS_TDA8290,
.radio_type = UNSET,
.tuner_addr = ADDR_UNSET,
.radio_addr = ADDR_UNSET,
- .tuner_config = 1,
+ .tuner_config = 0,
.mpeg = SAA7134_MPEG_DVB,
.gpiomask = 0x00020,
.inputs = {{
.name = name_tv,
.vmux = 4,
.amux = TV,
Please test if the above patch fixes the issue you're suffering[1]. If so,
then
we'll need to add a modprobe parameter to allow disabling LNA for saa7134
devices
with LNA.
[1] Note: the above is not the fix, as some users of this board may be using
the
original antenna, and changing tuner_config will break things for them; the
right
fix is likely to allow controlling the LNA via userspace.
Tried that patch on 3.5.3. No improvement, unfortunately.
I have to retract that. It turns out that there is some strange interaction
between the cabletv box and the card. When I rebooted into 'my' patch
I still got the noisy signal. I then power cycled the cabletv box, and
voila,
I got a good signal on my own patch. Wondering what I had actually tested
with your patch, I tested it again, and indeed it works!
So, 1) you're on to something, that's for sure, and 2) there is
_something_ in
the cabletv box which can make all this fall into a bad state too.
Cheers,
/Anders
--
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] Timestamps and V4L2
On 09/22/2012 10:28 PM, Daniel Glöckner wrote: On Sat, Sep 22, 2012 at 07:12:52PM +0200, Sylwester Nawrocki wrote: If we ever need the clock selection API I would vote for an IOCTL. The controls API is a bad choice for something such fundamental as type of clock for buffer timestamping IMHO. Let's stop making the controls API a dumping ground for almost everything in V4L2! ;) Perhaps VIDIOC_QUERYBUF and VIDIOC_DQBUF should be reporting timestamps type only for the time they are being called. Not per buffer, per device. And applications would be checking the flags any time they want to find out what is the buffer timestamp type. Or every time if it don't have full control over the device (S/G_PRIORITY). I'm all for adding an IOCTL, but if we think about adding a VIDIOC_S_TIMESTAMP_TYPE in the future, we might as well add a VIDIOC_G_TIMESTAMP_TYPE right now. Old drivers will return ENOSYS, so the application knows it will have to guess the type (or take own timestamps). Hmm, would it make sense to design a single ioctl that would allow getting and setting the clock type, e.g. VIDIOC_CLOCK/TIMESTAMP_TYPE ? I can't imagine anything useful coming from an app that has to process timestamps that change their source every now and then and I seriously doubt anyone will go to such an extent that they check the timestamp type on every buffer. If they don't set their priority high enough to prevent others from changing the timestamp type, they also run the risk of someone else changing the image format. It should be enough to forbid changing the timestamp type while I/O is in progress, as it is done for VIDIOC_S_FMT. I agree, but mem-to-mem devices can have multiple logically independent, concurrent streams active. If the clock type is per device it might not be that straightforward... 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
cron job: media_tree daily build: ERRORS
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:Sun Sep 23 19:00:25 CEST 2012 git hash:9a888ba273b8bbd82a0b88cfd57c270f6eb8d724 gcc version: i686-linux-gcc (GCC) 4.7.1 host hardware:x86_64 host os: 3.4.07-marune linux-git-arm-eabi-davinci: WARNINGS linux-git-arm-eabi-exynos: OK linux-git-arm-eabi-omap: WARNINGS linux-git-i686: WARNINGS linux-git-m32r: WARNINGS linux-git-mips: WARNINGS linux-git-powerpc64: OK linux-git-sh: ERRORS linux-git-x86_64: WARNINGS linux-2.6.31.12-x86_64: ERRORS linux-2.6.32.6-x86_64: ERRORS linux-2.6.33-x86_64: ERRORS linux-2.6.34-x86_64: ERRORS linux-2.6.35.3-x86_64: ERRORS linux-2.6.36-x86_64: ERRORS linux-2.6.37-x86_64: WARNINGS linux-2.6.38.2-x86_64: WARNINGS linux-2.6.39.1-x86_64: WARNINGS linux-3.0-x86_64: WARNINGS linux-3.1-x86_64: WARNINGS linux-3.2.1-x86_64: WARNINGS linux-3.3-x86_64: WARNINGS linux-3.4-x86_64: WARNINGS linux-3.5-x86_64: WARNINGS linux-3.6-rc2-x86_64: WARNINGS linux-2.6.31.12-i686: ERRORS linux-2.6.32.6-i686: ERRORS linux-2.6.33-i686: ERRORS linux-2.6.34-i686: ERRORS linux-2.6.35.3-i686: ERRORS linux-2.6.36-i686: ERRORS linux-2.6.37-i686: WARNINGS linux-2.6.38.2-i686: WARNINGS linux-2.6.39.1-i686: WARNINGS linux-3.0-i686: WARNINGS linux-3.1-i686: WARNINGS linux-3.2.1-i686: WARNINGS linux-3.3-i686: WARNINGS linux-3.4-i686: WARNINGS linux-3.5-i686: WARNINGS linux-3.6-rc2-i686: WARNINGS apps: WARNINGS spec-git: WARNINGS sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Sunday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Sunday.tar.bz2 The V4L-DVB specification 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
Re: [PATCH 3/3] ov2640: simplify single register writes
Am 23.09.2012 15:07, schrieb Frank Schäfer:
Signed-off-by: Frank Schäfer fschaefer@googlemail.com
---
drivers/media/i2c/soc_camera/ov2640.c | 21 -
1 Datei geändert, 12 Zeilen hinzugefügt(+), 9 Zeilen entfernt(-)
diff --git a/drivers/media/i2c/soc_camera/ov2640.c
b/drivers/media/i2c/soc_camera/ov2640.c
index 182d5a1..70b222f 100644
--- a/drivers/media/i2c/soc_camera/ov2640.c
+++ b/drivers/media/i2c/soc_camera/ov2640.c
@@ -639,17 +639,23 @@ static struct ov2640_priv *to_ov2640(const struct
i2c_client *client)
subdev);
}
+static int ov2640_write_single(struct i2c_client *client, u8 reg, u8 val)
+{
+ int ret;
+
+ ret = i2c_smbus_write_byte_data(client, reg, val);
+ dev_vdbg(client-dev, write: 0x%02x, 0x%02x, reg, val);
+
+ return ret;
Uhm... wait ! Of course we can get rid of int ret.
Will resend in a minute...
Regards,
Frank
+}
+
static int ov2640_write_array(struct i2c_client *client,
const struct regval_list *vals)
{
int ret;
while ((vals-reg_num != 0xff) || (vals-value != 0xff)) {
- ret = i2c_smbus_write_byte_data(client,
- vals-reg_num, vals-value);
- dev_vdbg(client-dev, array: 0x%02x, 0x%02x,
- vals-reg_num, vals-value);
-
+ ret = ov2640_write_single(client, vals-reg_num, vals-value);
if (ret 0)
return ret;
vals++;
@@ -704,13 +710,10 @@ static int ov2640_s_ctrl(struct v4l2_ctrl *ctrl)
struct v4l2_subdev *sd =
container_of(ctrl-handler, struct ov2640_priv, hdl)-subdev;
struct i2c_client *client = v4l2_get_subdevdata(sd);
- struct regval_list regval;
int ret;
u8 val;
- regval.reg_num = BANK_SEL;
- regval.value = BANK_SEL_SENS;
- ret = ov2640_write_array(client, regval);
+ ret = ov2640_write_single(client, BANK_SEL, BANK_SEL_SENS);
if (ret 0)
return ret;
--
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 v2 1/3] ov2640: select sensor register bank before applying h/v-flip settings
We currently don't select the register bank in ov2640_s_ctrl, so we can end up
writing to DSP register 0x04 instead of sensor register 0x04.
This happens for example when calling ov2640_s_ctrl after ov2640_s_fmt.
Signed-off-by: Frank Schäfer fschaefer@googlemail.com
Cc: sta...@kernel.org
---
drivers/media/i2c/soc_camera/ov2640.c |8
1 Datei geändert, 8 Zeilen hinzugefügt(+)
diff --git a/drivers/media/i2c/soc_camera/ov2640.c
b/drivers/media/i2c/soc_camera/ov2640.c
index 78ac574..e4fc79e 100644
--- a/drivers/media/i2c/soc_camera/ov2640.c
+++ b/drivers/media/i2c/soc_camera/ov2640.c
@@ -683,8 +683,16 @@ static int ov2640_s_ctrl(struct v4l2_ctrl *ctrl)
struct v4l2_subdev *sd =
container_of(ctrl-handler, struct ov2640_priv, hdl)-subdev;
struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct regval_list regval;
+ int ret;
u8 val;
+ regval.reg_num = BANK_SEL;
+ regval.value = BANK_SEL_SENS;
+ ret = ov2640_write_array(client, regval);
+ if (ret 0)
+ return ret;
+
switch (ctrl-id) {
case V4L2_CID_VFLIP:
val = ctrl-val ? REG04_VFLIP_IMG : 0x00;
--
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 v2 2/3] ov2640: add support for V4L2_MBUS_FMT_YUYV8_2X8, V4L2_MBUS_FMT_RGB565_2X8_BE
This is the result of experimenting with the SpeedLink VAD Laplace webcam.
The register sequence for V4L2_MBUS_FMT_YUYV8_2X8 has been identified by
analyzing USB-logs of this device running on MS Windows.
Signed-off-by: Frank Schäfer fschaefer@googlemail.com
---
drivers/media/i2c/soc_camera/ov2640.c | 49 -
1 Datei geändert, 42 Zeilen hinzugefügt(+), 7 Zeilen entfernt(-)
diff --git a/drivers/media/i2c/soc_camera/ov2640.c
b/drivers/media/i2c/soc_camera/ov2640.c
index e4fc79e..182d5a1 100644
--- a/drivers/media/i2c/soc_camera/ov2640.c
+++ b/drivers/media/i2c/soc_camera/ov2640.c
@@ -586,9 +586,20 @@ static const struct regval_list
ov2640_format_change_preamble_regs[] = {
ENDMARKER,
};
-static const struct regval_list ov2640_yuv422_regs[] = {
+static const struct regval_list ov2640_yuyv_regs[] = {
+ { IMAGE_MODE, IMAGE_MODE_YUV422 },
+ { 0xd7, 0x03 },
+ { 0x33, 0xa0 },
+ { 0xe5, 0x1f },
+ { 0xe1, 0x67 },
+ { RESET, 0x00 },
+ { R_BYPASS, R_BYPASS_USE_DSP },
+ ENDMARKER,
+};
+
+static const struct regval_list ov2640_uyvy_regs[] = {
{ IMAGE_MODE, IMAGE_MODE_LBYTE_FIRST | IMAGE_MODE_YUV422 },
- { 0xD7, 0x01 },
+ { 0xd7, 0x01 },
{ 0x33, 0xa0 },
{ 0xe1, 0x67 },
{ RESET, 0x00 },
@@ -596,7 +607,15 @@ static const struct regval_list ov2640_yuv422_regs[] = {
ENDMARKER,
};
-static const struct regval_list ov2640_rgb565_regs[] = {
+static const struct regval_list ov2640_rgb565_be_regs[] = {
+ { IMAGE_MODE, IMAGE_MODE_RGB565 },
+ { 0xd7, 0x03 },
+ { RESET, 0x00 },
+ { R_BYPASS, R_BYPASS_USE_DSP },
+ ENDMARKER,
+};
+
+static const struct regval_list ov2640_rgb565_le_regs[] = {
{ IMAGE_MODE, IMAGE_MODE_LBYTE_FIRST | IMAGE_MODE_RGB565 },
{ 0xd7, 0x03 },
{ RESET, 0x00 },
@@ -605,7 +624,9 @@ static const struct regval_list ov2640_rgb565_regs[] = {
};
static enum v4l2_mbus_pixelcode ov2640_codes[] = {
+ V4L2_MBUS_FMT_YUYV8_2X8,
V4L2_MBUS_FMT_UYVY8_2X8,
+ V4L2_MBUS_FMT_RGB565_2X8_BE,
V4L2_MBUS_FMT_RGB565_2X8_LE,
};
@@ -790,14 +811,22 @@ static int ov2640_set_params(struct i2c_client *client,
u32 *width, u32 *height,
/* select format */
priv-cfmt_code = 0;
switch (code) {
+ case V4L2_MBUS_FMT_RGB565_2X8_BE:
+ dev_dbg(client-dev, %s: Selected cfmt RGB565 BE, __func__);
+ selected_cfmt_regs = ov2640_rgb565_be_regs;
+ break;
case V4L2_MBUS_FMT_RGB565_2X8_LE:
- dev_dbg(client-dev, %s: Selected cfmt RGB565, __func__);
- selected_cfmt_regs = ov2640_rgb565_regs;
+ dev_dbg(client-dev, %s: Selected cfmt RGB565 LE, __func__);
+ selected_cfmt_regs = ov2640_rgb565_le_regs;
+ break;
+ case V4L2_MBUS_FMT_YUYV8_2X8:
+ dev_dbg(client-dev, %s: Selected cfmt YUYV (YUV422),
__func__);
+ selected_cfmt_regs = ov2640_yuyv_regs;
break;
default:
case V4L2_MBUS_FMT_UYVY8_2X8:
- dev_dbg(client-dev, %s: Selected cfmt YUV422, __func__);
- selected_cfmt_regs = ov2640_yuv422_regs;
+ dev_dbg(client-dev, %s: Selected cfmt UYVY, __func__);
+ selected_cfmt_regs = ov2640_uyvy_regs;
}
/* reset hardware */
@@ -862,10 +891,12 @@ static int ov2640_g_fmt(struct v4l2_subdev *sd,
mf-code= priv-cfmt_code;
switch (mf-code) {
+ case V4L2_MBUS_FMT_RGB565_2X8_BE:
case V4L2_MBUS_FMT_RGB565_2X8_LE:
mf-colorspace = V4L2_COLORSPACE_SRGB;
break;
default:
+ case V4L2_MBUS_FMT_YUYV8_2X8:
case V4L2_MBUS_FMT_UYVY8_2X8:
mf-colorspace = V4L2_COLORSPACE_JPEG;
}
@@ -882,11 +913,13 @@ static int ov2640_s_fmt(struct v4l2_subdev *sd,
switch (mf-code) {
+ case V4L2_MBUS_FMT_RGB565_2X8_BE:
case V4L2_MBUS_FMT_RGB565_2X8_LE:
mf-colorspace = V4L2_COLORSPACE_SRGB;
break;
default:
mf-code = V4L2_MBUS_FMT_UYVY8_2X8;
+ case V4L2_MBUS_FMT_YUYV8_2X8:
case V4L2_MBUS_FMT_UYVY8_2X8:
mf-colorspace = V4L2_COLORSPACE_JPEG;
}
@@ -909,11 +942,13 @@ static int ov2640_try_fmt(struct v4l2_subdev *sd,
mf-field = V4L2_FIELD_NONE;
switch (mf-code) {
+ case V4L2_MBUS_FMT_RGB565_2X8_BE:
case V4L2_MBUS_FMT_RGB565_2X8_LE:
mf-colorspace = V4L2_COLORSPACE_SRGB;
break;
default:
mf-code = V4L2_MBUS_FMT_UYVY8_2X8;
+ case V4L2_MBUS_FMT_YUYV8_2X8:
case V4L2_MBUS_FMT_UYVY8_2X8:
mf-colorspace = V4L2_COLORSPACE_JPEG;
}
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to
[PATCH v2 3/3] ov2640: simplify single register writes
Signed-off-by: Frank Schäfer fschaefer@googlemail.com
---
drivers/media/i2c/soc_camera/ov2640.c | 17 -
1 Datei geändert, 8 Zeilen hinzugefügt(+), 9 Zeilen entfernt(-)
diff --git a/drivers/media/i2c/soc_camera/ov2640.c
b/drivers/media/i2c/soc_camera/ov2640.c
index 182d5a1..e71bf4c 100644
--- a/drivers/media/i2c/soc_camera/ov2640.c
+++ b/drivers/media/i2c/soc_camera/ov2640.c
@@ -639,17 +639,19 @@ static struct ov2640_priv *to_ov2640(const struct
i2c_client *client)
subdev);
}
+static int ov2640_write_single(struct i2c_client *client, u8 reg, u8 val)
+{
+ dev_vdbg(client-dev, write: 0x%02x, 0x%02x, reg, val);
+ return i2c_smbus_write_byte_data(client, reg, val);
+}
+
static int ov2640_write_array(struct i2c_client *client,
const struct regval_list *vals)
{
int ret;
while ((vals-reg_num != 0xff) || (vals-value != 0xff)) {
- ret = i2c_smbus_write_byte_data(client,
- vals-reg_num, vals-value);
- dev_vdbg(client-dev, array: 0x%02x, 0x%02x,
-vals-reg_num, vals-value);
-
+ ret = ov2640_write_single(client, vals-reg_num, vals-value);
if (ret 0)
return ret;
vals++;
@@ -704,13 +706,10 @@ static int ov2640_s_ctrl(struct v4l2_ctrl *ctrl)
struct v4l2_subdev *sd =
container_of(ctrl-handler, struct ov2640_priv, hdl)-subdev;
struct i2c_client *client = v4l2_get_subdevdata(sd);
- struct regval_list regval;
int ret;
u8 val;
- regval.reg_num = BANK_SEL;
- regval.value = BANK_SEL_SENS;
- ret = ov2640_write_array(client, regval);
+ ret = ov2640_write_single(client, BANK_SEL, BANK_SEL_SENS);
if (ret 0)
return ret;
--
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
Fwd: [PATCH v2] drivers/media/platform/s5p-tv/sdo_drv.c: fix error return code
Sylwester,
Please review.
Regards,
Mauro
Mensagem original
Assunto: [PATCH v2] drivers/media/platform/s5p-tv/sdo_drv.c: fix error return
code
Data: Thu, 6 Sep 2012 10:38:29 +0200
De: Peter Senna Tschudin peter.se...@gmail.com
Para: peter.se...@gmail.com, Mauro Carvalho Chehab mche...@infradead.org
CC: linux-media@vger.kernel.org, linux-ker...@vger.kernel.org
From: Peter Senna Tschudin peter.se...@gmail.com
Convert a nonnegative error return code to a negative one, as returned
elsewhere in the function.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// smpl
(
if@p1 (\(ret 0\|ret != 0\))
{ ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != ret
*if(...)
{
... when != ret = e2
when forall
return ret;
}
// /smpl
Signed-off-by: Peter Senna Tschudin peter.se...@gmail.com
---
drivers/media/platform/s5p-tv/sdo_drv.c |3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/media/platform/s5p-tv/sdo_drv.c
b/drivers/media/platform/s5p-tv/sdo_drv.c
index ad68bbe..58cf56d 100644
--- a/drivers/media/platform/s5p-tv/sdo_drv.c
+++ b/drivers/media/platform/s5p-tv/sdo_drv.c
@@ -369,6 +369,7 @@ static int __devinit sdo_probe(struct platform_device *pdev)
sdev-fout_vpll = clk_get(dev, fout_vpll);
if (IS_ERR_OR_NULL(sdev-fout_vpll)) {
dev_err(dev, failed to get clock 'fout_vpll'\n);
+ ret = -ENXIO;
goto fail_dacphy;
}
dev_info(dev, fout_vpll.rate = %lu\n, clk_get_rate(sclk_vpll));
@@ -377,11 +378,13 @@ static int __devinit sdo_probe(struct platform_device
*pdev)
sdev-vdac = devm_regulator_get(dev, vdd33a_dac);
if (IS_ERR_OR_NULL(sdev-vdac)) {
dev_err(dev, failed to get regulator 'vdac'\n);
+ ret = -ENXIO;
goto fail_fout_vpll;
}
sdev-vdet = devm_regulator_get(dev, vdet);
if (IS_ERR_OR_NULL(sdev-vdet)) {
dev_err(dev, failed to get regulator 'vdet'\n);
+ ret = -ENXIO;
goto fail_fout_vpll;
}
--
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
Fwd: [PATCH 1/14] drivers/media/platform/soc_camera/soc_camera.c: fix error return code
Please review.
Please review.
Regards,
Mauro.
Mensagem original
Assunto: [PATCH 1/14] drivers/media/platform/soc_camera/soc_camera.c: fix error
return code
Data: Thu, 6 Sep 2012 17:24:00 +0200
De: Peter Senna Tschudin peter.se...@gmail.com
Para: Mauro Carvalho Chehab mche...@infradead.org
CC: kernel-janit...@vger.kernel.org, julia.law...@lip6.fr,
linux-media@vger.kernel.org, linux-ker...@vger.kernel.org
From: Peter Senna Tschudin peter.se...@gmail.com
Convert a nonnegative error return code to a negative one, as returned
elsewhere in the function.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// smpl
(
if@p1 (\(ret 0\|ret != 0\))
{ ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != ret
*if(...)
{
... when != ret = e2
when forall
return ret;
}
// /smpl
Signed-off-by: Peter Senna Tschudin peter.se...@gmail.com
---
drivers/media/platform/soc_camera/soc_camera.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/soc_camera/soc_camera.c
b/drivers/media/platform/soc_camera/soc_camera.c
index 10b57f8..a4beb6c 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -1184,7 +1184,8 @@ static int soc_camera_probe(struct soc_camera_device *icd)
sd-grp_id = soc_camera_grp_id(icd);
v4l2_set_subdev_hostdata(sd, icd);
- if (v4l2_ctrl_add_handler(icd-ctrl_handler, sd-ctrl_handler))
+ ret = v4l2_ctrl_add_handler(icd-ctrl_handler, sd-ctrl_handler);
+ if (ret)
goto ectrl;
/* At this point client .probe() should have run already */
--
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
Fwd: [PATCH 2/14] drivers/media/platform/soc_camera/mx2_camera.c: fix error return code
Please review,
Regards,
Mauro.
Mensagem original
Assunto: [PATCH 2/14] drivers/media/platform/soc_camera/mx2_camera.c: fix error
return code
Data: Thu, 6 Sep 2012 17:23:59 +0200
De: Peter Senna Tschudin peter.se...@gmail.com
Para: Mauro Carvalho Chehab mche...@infradead.org
CC: kernel-janit...@vger.kernel.org, julia.law...@lip6.fr,
linux-media@vger.kernel.org, linux-ker...@vger.kernel.org
From: Peter Senna Tschudin peter.se...@gmail.com
Convert a nonnegative error return code to a negative one, as returned
elsewhere in the function.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// smpl
(
if@p1 (\(ret 0\|ret != 0\))
{ ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != ret
*if(...)
{
... when != ret = e2
when forall
return ret;
}
// /smpl
Signed-off-by: Peter Senna Tschudin peter.se...@gmail.com
---
drivers/media/platform/soc_camera/mx2_camera.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/soc_camera/mx2_camera.c
b/drivers/media/platform/soc_camera/mx2_camera.c
index 256187f..f8884a7 100644
--- a/drivers/media/platform/soc_camera/mx2_camera.c
+++ b/drivers/media/platform/soc_camera/mx2_camera.c
@@ -1800,13 +1800,16 @@ static int __devinit mx2_camera_probe(struct
platform_device *pdev)
if (!res_emma || !irq_emma) {
dev_err(pdev-dev, no EMMA resources\n);
+ err = -ENODEV;
goto exit_free_irq;
}
pcdev-res_emma = res_emma;
pcdev-irq_emma = irq_emma;
- if (mx27_camera_emma_init(pcdev))
+ if (mx27_camera_emma_init(pcdev)) {
+ err = -ENODEV;
goto exit_free_irq;
+ }
}
pcdev-soc_host.drv_name= MX2_CAM_DRV_NAME,
--
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] v4 Add support to Avermedia Twinstar double tuner in af9035
This patch add support to the Avermedia Twinstar double tuner in the af9035
driver. Version 4 of the patch. I split the patch as suggested by Antti. I send
the changes to mxl5007 driver in another patch.
Signed-off-by: Jose Alberto Reguero jaregu...@telefonica.net
Jose Alberto
diff -upr linux/drivers/media/dvb-frontends/af9033.c
linux.new/drivers/media/dvb-frontends/af9033.c
--- linux/drivers/media/dvb-frontends/af9033.c 2012-08-14 05:45:22.0
+0200
+++ linux.new/drivers/media/dvb-frontends/af9033.c 2012-09-13
22:22:29.012301231 +0200
@@ -326,6 +326,18 @@ static int af9033_init(struct dvb_fronte
goto err;
}
+ if (state-cfg.ts_mode == AF9033_TS_MODE_SERIAL) {
+ ret = af9033_wr_reg_mask(state, 0x00d91c, 0x01, 0x01);
+ if (ret 0)
+ goto err;
+ ret = af9033_wr_reg_mask(state, 0x00d917, 0x00, 0x01);
+ if (ret 0)
+ goto err;
+ ret = af9033_wr_reg_mask(state, 0x00d916, 0x00, 0x01);
+ if (ret 0)
+ goto err;
+ }
+
state-bandwidth_hz = 0; /* force to program all parameters */
return 0;
diff -upr linux/drivers/media/usb/dvb-usb-v2/af9035.c
linux.new/drivers/media/usb/dvb-usb-v2/af9035.c
--- linux/drivers/media/usb/dvb-usb-v2/af9035.c 2012-08-16 05:45:24.0
+0200
+++ linux.new/drivers/media/usb/dvb-usb-v2/af9035.c 2012-09-23
21:32:10.313657063 +0200
@@ -209,10 +209,14 @@ static int af9035_i2c_master_xfer(struct
if (msg[0].len 40 || msg[1].len 40) {
/* TODO: correct limits 40 */
ret = -EOPNOTSUPP;
- } else if (msg[0].addr == state-af9033_config[0].i2c_addr) {
+ } else if ((msg[0].addr == state-af9033_config[0].i2c_addr) ||
+ (msg[0].addr == state-af9033_config[1].i2c_addr)) {
/* integrated demod */
u32 reg = msg[0].buf[0] 16 | msg[0].buf[1] 8 |
msg[0].buf[2];
+ if (state-af9033_config[1].i2c_addr
+ (msg[0].addr == state-af9033_config[1].i2c_addr))
+ reg |= 0x10;
ret = af9035_rd_regs(d, reg, msg[1].buf[0],
msg[1].len);
} else {
@@ -220,8 +224,9 @@ static int af9035_i2c_master_xfer(struct
u8 buf[5 + msg[0].len];
struct usb_req req = { CMD_I2C_RD, 0, sizeof(buf),
buf, msg[1].len, msg[1].buf };
+ req.mbox |= ((msg[0].addr 0x80)3);
buf[0] = msg[1].len;
- buf[1] = msg[0].addr 1;
+ buf[1] = (u8)(msg[0].addr 1);
buf[2] = 0x00; /* reg addr len */
buf[3] = 0x00; /* reg addr MSB */
buf[4] = 0x00; /* reg addr LSB */
@@ -232,10 +237,14 @@ static int af9035_i2c_master_xfer(struct
if (msg[0].len 40) {
/* TODO: correct limits 40 */
ret = -EOPNOTSUPP;
- } else if (msg[0].addr == state-af9033_config[0].i2c_addr) {
+ } else if ((msg[0].addr == state-af9033_config[0].i2c_addr) ||
+ (msg[0].addr == state-af9033_config[1].i2c_addr)) {
/* integrated demod */
u32 reg = msg[0].buf[0] 16 | msg[0].buf[1] 8 |
msg[0].buf[2];
+ if (state-af9033_config[1].i2c_addr
+ (msg[0].addr == state-af9033_config[1].i2c_addr))
+ reg |= 0x10;
ret = af9035_wr_regs(d, reg, msg[0].buf[3],
msg[0].len - 3);
} else {
@@ -243,8 +252,9 @@ static int af9035_i2c_master_xfer(struct
u8 buf[5 + msg[0].len];
struct usb_req req = { CMD_I2C_WR, 0, sizeof(buf), buf,
0, NULL };
+ req.mbox |= ((msg[0].addr 0x80)3);
buf[0] = msg[0].len;
- buf[1] = msg[0].addr 1;
+ buf[1] = (u8)(msg[0].addr 1);
buf[2] = 0x00; /* reg addr len */
buf[3] = 0x00; /* reg addr MSB */
buf[4] = 0x00; /* reg addr LSB */
@@ -283,9 +293,30 @@ static int af9035_identify_state(struct
int ret;
u8 wbuf[1] = { 1 };
u8 rbuf[4];
+ u8 tmp;
struct usb_req req = { CMD_FW_QUERYINFO, 0, sizeof(wbuf), wbuf,
sizeof(rbuf), rbuf };
+ /* check if there is dual tuners */
+
Re: [PATCH] tlg2300: fix missing check for audio creation
Hi Alan,
Em 04-09-2012 11:43, Alan Cox escreveu:
From: Alan Cox a...@linux.intel.com
If we fail to set up the capture device we go through negative indexes and
badness happens. Add the missing test.
Resolves-bug: https://bugzilla.kernel.org/show_bug.cgi?id=44551
Signed-off-by: Alan Cox a...@linux.intel.com
---
drivers/media/usb/tlg2300/pd-alsa.c |4
1 file changed, 4 insertions(+)
diff --git a/drivers/media/usb/tlg2300/pd-alsa.c
b/drivers/media/usb/tlg2300/pd-alsa.c
index 9f8b7da..0c77869 100644
--- a/drivers/media/usb/tlg2300/pd-alsa.c
+++ b/drivers/media/usb/tlg2300/pd-alsa.c
@@ -305,6 +305,10 @@ int poseidon_audio_init(struct poseidon *p)
return ret;
ret = snd_pcm_new(card, poseidon audio, 0, 0, 1, pcm);
+ if (ret 0) {
+ snd_free_card(card);
That patch broke compilation:
CC drivers/media/usb/tlg2300/pd-alsa.o
drivers/media/usb/tlg2300/pd-alsa.c: In function 'poseidon_audio_init':
drivers/media/usb/tlg2300/pd-alsa.c:309:3: error: implicit declaration of
function 'snd_free_card' [-Werror=implicit-function-declaration]
This change fixed it:
- snd_free_card(card);
+ snd_card_free(card);
Not sure if this is a typo, or if it is due to some function rename
that might be happening at alsa subsystem and you might be noticed
at -next. In any case, I had to apply a patch on my tree fixing it.
PS: I noticed the compilation breakage too late to merge the fix together
with your patch - my background compilation process was supposed to
not only print a warning message in red, but also to bug me at the
speakers - due to Murphy laws, everything got wrong: the screen
was overlapped by another one; my speakers were muted).
Still, I can swap the patch order for the patches I didn't sent
upstream yet, in order to put the fixup patch just after yours.
Regards,
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: go7007 question
Em Fri, 7 Sep 2012 18:18:31 +0400 vol...@telros.ru escreveu: On Thu, Sep 06, 2012 at 11:10:14PM +0400, Volokh Konstantin wrote: On Mon, Sep 03, 2012 at 02:37:16PM -0400, Adam Rosi-Kessel wrote: [469.928881] wis-saa7115: initializing SAA7115 at address 32 on WIS GO7007SB EZ-USB [469.989083] go7007: probing for module i2c:wis_saa7115 failed [470.004785] wis-uda1342: initializing UDA1342 at address 26 on WIS GO7007SB EZ-USB [470.005454] go7007: probing for module i2c:wis_uda1342 failed [470.011659] wis-sony-tuner: initializing tuner at address 96 on WIS GO7007SB EZ-USB Hi, I generated patchs, that u may in your own go7007/ folder It contains go7007 initialization and i2c_subdev fixing It was checked for 3.6 branch (compile only) Sorry, but I don't know what do you intend with this post. I can't merge this patch upstream for a number of reasons: - There's no Signed-off-by: on this patch; - There's no description explaining what is there at the patch; - the patch looks too complex - it is hard to believe that this is a single functional change. Merging lots of stuff into the same patch makes hard for it to be reviewed, so please break it into a proper, well-described patch series; - scripts/checkpatch.pl also didn't like the patch: ERROR: Missing Signed-off-by: line(s) total: 5 errors, 44 warnings, 393 lines checked Please fix it. 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 v2 1/3] ov2640: select sensor register bank before applying h/v-flip settings
Hi Frank
On Sun, 23 Sep 2012, Frank Schäfer wrote:
We currently don't select the register bank in ov2640_s_ctrl, so we can end up
writing to DSP register 0x04 instead of sensor register 0x04.
This happens for example when calling ov2640_s_ctrl after ov2640_s_fmt.
Yes, in principle, I agree, bank switching in the driver is not very...
consistent and also this specific case looks buggy. But, we have to fix
your fix.
Signed-off-by: Frank Schäfer fschaefer@googlemail.com
Cc: sta...@kernel.org
---
drivers/media/i2c/soc_camera/ov2640.c |8
1 Datei geändert, 8 Zeilen hinzugefügt(+)
diff --git a/drivers/media/i2c/soc_camera/ov2640.c
b/drivers/media/i2c/soc_camera/ov2640.c
index 78ac574..e4fc79e 100644
--- a/drivers/media/i2c/soc_camera/ov2640.c
+++ b/drivers/media/i2c/soc_camera/ov2640.c
@@ -683,8 +683,16 @@ static int ov2640_s_ctrl(struct v4l2_ctrl *ctrl)
struct v4l2_subdev *sd =
container_of(ctrl-handler, struct ov2640_priv, hdl)-subdev;
struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct regval_list regval;
+ int ret;
u8 val;
+ regval.reg_num = BANK_SEL;
+ regval.value = BANK_SEL_SENS;
+ ret = ov2640_write_array(client, regval);
This doesn't look right to me. ov2640_write_array() keeps writing register
address - value pairs to the hardware until it encounters an ENDMARKER,
which you don't have here, so, it's hard to say what will be written to
the sensor... Secondly, you only have to write a single register here, for
this the driver is already using i2c_smbus_write_byte_data() directly,
please, do the same.
Thanks
Guennadi
+ if (ret 0)
+ return ret;
+
switch (ctrl-id) {
case V4L2_CID_VFLIP:
val = ctrl-val ? REG04_VFLIP_IMG : 0x00;
--
1.7.10.4
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c: Replace kmemdup for kstrdup
Em Mon, 10 Sep 2012 14:45:54 +0200
Peter Senna Tschudin peter.se...@gmail.com escreveu:
From: Peter Senna Tschudin peter.se...@gmail.com
Replace kmemdup for kstrdup and cleaning up the code.
Signed-off-by: Peter Senna Tschudin peter.se...@gmail.com
Maintainers/interested parties not copied. Also:
Hunk #1 succeeded at 708 (offset 1 line).
Hunk #2 FAILED at 742.
1 out of 2 hunks FAILED -- saving rejects to file
drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c.rej
tmp/cx25821-video-upstream-ch2.c | 27 +--
1 file changed, 9 insertions(+), 18 deletions(-)
---
It depends on the patch http://patchwork.linuxtv.org/patch/14231/
tmp/cx25821-video-upstream-ch2.c | 27 +--
1 file changed, 9 insertions(+), 18 deletions(-)
diff --git a/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
b/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
index 273df94..b663dac 100644
--- a/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
+++ b/tmp/cx25821-video-upstream-ch2.c
@@ -707,7 +707,6 @@ int cx25821_vidupstream_init_ch2(struct cx25821_dev *dev,
int channel_select,
int err = 0;
int data_frame_size = 0;
int risc_buffer_size = 0;
- int str_length = 0;
if (dev-_is_running_ch2) {
pr_info(Video Channel is still running so return!\n);
@@ -743,24 +742,16 @@ int cx25821_vidupstream_init_ch2(struct cx25821_dev
*dev, int channel_select,
risc_buffer_size = dev-_isNTSC_ch2 ?
NTSC_RISC_BUF_SIZE : PAL_RISC_BUF_SIZE;
- if (dev-input_filename_ch2) {
- str_length = strlen(dev-input_filename_ch2);
- dev-_filename_ch2 = kmemdup(dev-input_filename_ch2,
- str_length + 1, GFP_KERNEL);
-
- if (!dev-_filename_ch2) {
- err = -ENOENT;
- goto error;
- }
- } else {
- str_length = strlen(dev-_defaultname_ch2);
- dev-_filename_ch2 = kmemdup(dev-_defaultname_ch2,
- str_length + 1, GFP_KERNEL);
+ if (dev-input_filename_ch2)
+ dev-_filename_ch2 = kstrdup(dev-input_filename_ch2,
+ GFP_KERNEL);
+ else
+ dev-_filename_ch2 = kstrdup(dev-_defaultname_ch2,
+ GFP_KERNEL);
- if (!dev-_filename_ch2) {
- err = -ENOENT;
- goto error;
- }
+ if (!dev-_filename_ch2) {
+ err = -ENOENT;
+ goto error;
}
/* Default if filename is empty string */
--
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
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 2/3] ov2640: add support for V4L2_MBUS_FMT_YUYV8_2X8, V4L2_MBUS_FMT_RGB565_2X8_BE
On Sun, 23 Sep 2012, Frank SchÀfer wrote:
This is the result of experimenting with the SpeedLink VAD Laplace webcam.
The register sequence for V4L2_MBUS_FMT_YUYV8_2X8 has been identified by
analyzing USB-logs of this device running on MS Windows.
Signed-off-by: Frank SchÀfer fschaefer@googlemail.com
Looks good to me, thanks, will queue for 3.7.
Thanks
Guennadi
---
drivers/media/i2c/soc_camera/ov2640.c | 49
-
1 Datei geÀndert, 42 Zeilen hinzugefÌgt(+), 7 Zeilen entfernt(-)
diff --git a/drivers/media/i2c/soc_camera/ov2640.c
b/drivers/media/i2c/soc_camera/ov2640.c
index e4fc79e..182d5a1 100644
--- a/drivers/media/i2c/soc_camera/ov2640.c
+++ b/drivers/media/i2c/soc_camera/ov2640.c
@@ -586,9 +586,20 @@ static const struct regval_list
ov2640_format_change_preamble_regs[] = {
ENDMARKER,
};
-static const struct regval_list ov2640_yuv422_regs[] = {
+static const struct regval_list ov2640_yuyv_regs[] = {
+ { IMAGE_MODE, IMAGE_MODE_YUV422 },
+ { 0xd7, 0x03 },
+ { 0x33, 0xa0 },
+ { 0xe5, 0x1f },
+ { 0xe1, 0x67 },
+ { RESET, 0x00 },
+ { R_BYPASS, R_BYPASS_USE_DSP },
+ ENDMARKER,
+};
+
+static const struct regval_list ov2640_uyvy_regs[] = {
{ IMAGE_MODE, IMAGE_MODE_LBYTE_FIRST | IMAGE_MODE_YUV422 },
- { 0xD7, 0x01 },
+ { 0xd7, 0x01 },
{ 0x33, 0xa0 },
{ 0xe1, 0x67 },
{ RESET, 0x00 },
@@ -596,7 +607,15 @@ static const struct regval_list ov2640_yuv422_regs[] = {
ENDMARKER,
};
-static const struct regval_list ov2640_rgb565_regs[] = {
+static const struct regval_list ov2640_rgb565_be_regs[] = {
+ { IMAGE_MODE, IMAGE_MODE_RGB565 },
+ { 0xd7, 0x03 },
+ { RESET, 0x00 },
+ { R_BYPASS, R_BYPASS_USE_DSP },
+ ENDMARKER,
+};
+
+static const struct regval_list ov2640_rgb565_le_regs[] = {
{ IMAGE_MODE, IMAGE_MODE_LBYTE_FIRST | IMAGE_MODE_RGB565 },
{ 0xd7, 0x03 },
{ RESET, 0x00 },
@@ -605,7 +624,9 @@ static const struct regval_list ov2640_rgb565_regs[] = {
};
static enum v4l2_mbus_pixelcode ov2640_codes[] = {
+ V4L2_MBUS_FMT_YUYV8_2X8,
V4L2_MBUS_FMT_UYVY8_2X8,
+ V4L2_MBUS_FMT_RGB565_2X8_BE,
V4L2_MBUS_FMT_RGB565_2X8_LE,
};
@@ -790,14 +811,22 @@ static int ov2640_set_params(struct i2c_client *client,
u32 *width, u32 *height,
/* select format */
priv-cfmt_code = 0;
switch (code) {
+ case V4L2_MBUS_FMT_RGB565_2X8_BE:
+ dev_dbg(client-dev, %s: Selected cfmt RGB565 BE, __func__);
+ selected_cfmt_regs = ov2640_rgb565_be_regs;
+ break;
case V4L2_MBUS_FMT_RGB565_2X8_LE:
- dev_dbg(client-dev, %s: Selected cfmt RGB565, __func__);
- selected_cfmt_regs = ov2640_rgb565_regs;
+ dev_dbg(client-dev, %s: Selected cfmt RGB565 LE, __func__);
+ selected_cfmt_regs = ov2640_rgb565_le_regs;
+ break;
+ case V4L2_MBUS_FMT_YUYV8_2X8:
+ dev_dbg(client-dev, %s: Selected cfmt YUYV (YUV422),
__func__);
+ selected_cfmt_regs = ov2640_yuyv_regs;
break;
default:
case V4L2_MBUS_FMT_UYVY8_2X8:
- dev_dbg(client-dev, %s: Selected cfmt YUV422, __func__);
- selected_cfmt_regs = ov2640_yuv422_regs;
+ dev_dbg(client-dev, %s: Selected cfmt UYVY, __func__);
+ selected_cfmt_regs = ov2640_uyvy_regs;
}
/* reset hardware */
@@ -862,10 +891,12 @@ static int ov2640_g_fmt(struct v4l2_subdev *sd,
mf-code= priv-cfmt_code;
switch (mf-code) {
+ case V4L2_MBUS_FMT_RGB565_2X8_BE:
case V4L2_MBUS_FMT_RGB565_2X8_LE:
mf-colorspace = V4L2_COLORSPACE_SRGB;
break;
default:
+ case V4L2_MBUS_FMT_YUYV8_2X8:
case V4L2_MBUS_FMT_UYVY8_2X8:
mf-colorspace = V4L2_COLORSPACE_JPEG;
}
@@ -882,11 +913,13 @@ static int ov2640_s_fmt(struct v4l2_subdev *sd,
switch (mf-code) {
+ case V4L2_MBUS_FMT_RGB565_2X8_BE:
case V4L2_MBUS_FMT_RGB565_2X8_LE:
mf-colorspace = V4L2_COLORSPACE_SRGB;
break;
default:
mf-code = V4L2_MBUS_FMT_UYVY8_2X8;
+ case V4L2_MBUS_FMT_YUYV8_2X8:
case V4L2_MBUS_FMT_UYVY8_2X8:
mf-colorspace = V4L2_COLORSPACE_JPEG;
}
@@ -909,11 +942,13 @@ static int ov2640_try_fmt(struct v4l2_subdev *sd,
mf-field = V4L2_FIELD_NONE;
switch (mf-code) {
+ case V4L2_MBUS_FMT_RGB565_2X8_BE:
case V4L2_MBUS_FMT_RGB565_2X8_LE:
mf-colorspace = V4L2_COLORSPACE_SRGB;
break;
default:
mf-code = V4L2_MBUS_FMT_UYVY8_2X8;
+ case V4L2_MBUS_FMT_YUYV8_2X8:
case V4L2_MBUS_FMT_UYVY8_2X8:
mf-colorspace = V4L2_COLORSPACE_JPEG;
}
--
1.7.10.4
---
Guennadi
Re: [PATCH v2 1/3] ov2640: select sensor register bank before applying h/v-flip settings
Am 23.09.2012 23:21, schrieb Guennadi Liakhovetski:
Hi Frank
On Sun, 23 Sep 2012, Frank Schäfer wrote:
We currently don't select the register bank in ov2640_s_ctrl, so we can end
up
writing to DSP register 0x04 instead of sensor register 0x04.
This happens for example when calling ov2640_s_ctrl after ov2640_s_fmt.
Yes, in principle, I agree, bank switching in the driver is not very...
consistent and also this specific case looks buggy. But, we have to fix
your fix.
Signed-off-by: Frank Schäfer fschaefer@googlemail.com
Cc: sta...@kernel.org
---
drivers/media/i2c/soc_camera/ov2640.c |8
1 Datei geändert, 8 Zeilen hinzugefügt(+)
diff --git a/drivers/media/i2c/soc_camera/ov2640.c
b/drivers/media/i2c/soc_camera/ov2640.c
index 78ac574..e4fc79e 100644
--- a/drivers/media/i2c/soc_camera/ov2640.c
+++ b/drivers/media/i2c/soc_camera/ov2640.c
@@ -683,8 +683,16 @@ static int ov2640_s_ctrl(struct v4l2_ctrl *ctrl)
struct v4l2_subdev *sd =
container_of(ctrl-handler, struct ov2640_priv, hdl)-subdev;
struct i2c_client *client = v4l2_get_subdevdata(sd);
+struct regval_list regval;
+int ret;
u8 val;
+regval.reg_num = BANK_SEL;
+regval.value = BANK_SEL_SENS;
+ret = ov2640_write_array(client, regval);
This doesn't look right to me. ov2640_write_array() keeps writing register
address - value pairs to the hardware until it encounters an ENDMARKER,
which you don't have here, so, it's hard to say what will be written to
the sensor... Secondly, you only have to write a single register here, for
this the driver is already using i2c_smbus_write_byte_data() directly,
please, do the same.
Argh, yes, you're right.
The mistake was to split this off from patch 3 to reduce changes for
stable...
I will combine both patches and resend the series.
Regards,
Frank
Thanks
Guennadi
+if (ret 0)
+return ret;
+
switch (ctrl-id) {
case V4L2_CID_VFLIP:
val = ctrl-val ? REG04_VFLIP_IMG : 0x00;
--
1.7.10.4
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] ov2640: simplify single register writes
On Sun, 23 Sep 2012, Frank Schäfer wrote:
Signed-off-by: Frank Schäfer fschaefer@googlemail.com
---
drivers/media/i2c/soc_camera/ov2640.c | 17 -
1 Datei geändert, 8 Zeilen hinzugefügt(+), 9 Zeilen entfernt(-)
diff --git a/drivers/media/i2c/soc_camera/ov2640.c
b/drivers/media/i2c/soc_camera/ov2640.c
index 182d5a1..e71bf4c 100644
--- a/drivers/media/i2c/soc_camera/ov2640.c
+++ b/drivers/media/i2c/soc_camera/ov2640.c
@@ -639,17 +639,19 @@ static struct ov2640_priv *to_ov2640(const struct
i2c_client *client)
subdev);
}
+static int ov2640_write_single(struct i2c_client *client, u8 reg, u8 val)
+{
+ dev_vdbg(client-dev, write: 0x%02x, 0x%02x, reg, val);
+ return i2c_smbus_write_byte_data(client, reg, val);
+}
Well, I'm not convinced. I don't necessarily see it as a simplification.
You replace one perfectly ok function with another one with exactly the
same parameters. Ok, you also hide a debug printk() in your wrapper, but
that's not too useful either, IMHO. Besides, you're missing more calls to
i2c_smbus_write_byte_data() in ov2640_mask_set(), ov2640_s_register() and
ov2640_video_probe(). So, I'd just drop it.
Thanks
Guennadi
+
static int ov2640_write_array(struct i2c_client *client,
const struct regval_list *vals)
{
int ret;
while ((vals-reg_num != 0xff) || (vals-value != 0xff)) {
- ret = i2c_smbus_write_byte_data(client,
- vals-reg_num, vals-value);
- dev_vdbg(client-dev, array: 0x%02x, 0x%02x,
- vals-reg_num, vals-value);
-
+ ret = ov2640_write_single(client, vals-reg_num, vals-value);
if (ret 0)
return ret;
vals++;
@@ -704,13 +706,10 @@ static int ov2640_s_ctrl(struct v4l2_ctrl *ctrl)
struct v4l2_subdev *sd =
container_of(ctrl-handler, struct ov2640_priv, hdl)-subdev;
struct i2c_client *client = v4l2_get_subdevdata(sd);
- struct regval_list regval;
int ret;
u8 val;
- regval.reg_num = BANK_SEL;
- regval.value = BANK_SEL_SENS;
- ret = ov2640_write_array(client, regval);
+ ret = ov2640_write_single(client, BANK_SEL, BANK_SEL_SENS);
if (ret 0)
return ret;
--
1.7.10.4
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] ov2640: select sensor register bank before applying h/v-flip settings
On Sun, 23 Sep 2012, Frank Schäfer wrote:
Am 23.09.2012 23:21, schrieb Guennadi Liakhovetski:
Hi Frank
On Sun, 23 Sep 2012, Frank Schäfer wrote:
We currently don't select the register bank in ov2640_s_ctrl, so we can
end up
writing to DSP register 0x04 instead of sensor register 0x04.
This happens for example when calling ov2640_s_ctrl after ov2640_s_fmt.
Yes, in principle, I agree, bank switching in the driver is not very...
consistent and also this specific case looks buggy. But, we have to fix
your fix.
Signed-off-by: Frank Schäfer fschaefer@googlemail.com
Cc: sta...@kernel.org
---
drivers/media/i2c/soc_camera/ov2640.c |8
1 Datei geändert, 8 Zeilen hinzugefügt(+)
diff --git a/drivers/media/i2c/soc_camera/ov2640.c
b/drivers/media/i2c/soc_camera/ov2640.c
index 78ac574..e4fc79e 100644
--- a/drivers/media/i2c/soc_camera/ov2640.c
+++ b/drivers/media/i2c/soc_camera/ov2640.c
@@ -683,8 +683,16 @@ static int ov2640_s_ctrl(struct v4l2_ctrl *ctrl)
struct v4l2_subdev *sd =
container_of(ctrl-handler, struct ov2640_priv, hdl)-subdev;
struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct regval_list regval;
+ int ret;
u8 val;
+ regval.reg_num = BANK_SEL;
+ regval.value = BANK_SEL_SENS;
+ ret = ov2640_write_array(client, regval);
This doesn't look right to me. ov2640_write_array() keeps writing register
address - value pairs to the hardware until it encounters an ENDMARKER,
which you don't have here, so, it's hard to say what will be written to
the sensor... Secondly, you only have to write a single register here, for
this the driver is already using i2c_smbus_write_byte_data() directly,
please, do the same.
Argh, yes, you're right.
The mistake was to split this off from patch 3 to reduce changes for
stable...
I will combine both patches and resend the series.
No, please, don't. Just use i2c_smbus_write_byte_data() at this one
location for the fix patch.
Thanks
Guennadi
Regards,
Frank
Thanks
Guennadi
+ if (ret 0)
+ return ret;
+
switch (ctrl-id) {
case V4L2_CID_VFLIP:
val = ctrl-val ? REG04_VFLIP_IMG : 0x00;
--
1.7.10.4
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] ov2640: simplify single register writes
Am 23.09.2012 23:43, schrieb Guennadi Liakhovetski:
On Sun, 23 Sep 2012, Frank Schäfer wrote:
Signed-off-by: Frank Schäfer fschaefer@googlemail.com
---
drivers/media/i2c/soc_camera/ov2640.c | 17 -
1 Datei geändert, 8 Zeilen hinzugefügt(+), 9 Zeilen entfernt(-)
diff --git a/drivers/media/i2c/soc_camera/ov2640.c
b/drivers/media/i2c/soc_camera/ov2640.c
index 182d5a1..e71bf4c 100644
--- a/drivers/media/i2c/soc_camera/ov2640.c
+++ b/drivers/media/i2c/soc_camera/ov2640.c
@@ -639,17 +639,19 @@ static struct ov2640_priv *to_ov2640(const struct
i2c_client *client)
subdev);
}
+static int ov2640_write_single(struct i2c_client *client, u8 reg, u8 val)
+{
+dev_vdbg(client-dev, write: 0x%02x, 0x%02x, reg, val);
+return i2c_smbus_write_byte_data(client, reg, val);
+}
Well, I'm not convinced. I don't necessarily see it as a simplification.
You replace one perfectly ok function with another one with exactly the
same parameters. Ok, you also hide a debug printk() in your wrapper, but
that's not too useful either, IMHO.
Sure, at the moment this is not really needed. But that will change in
the future, when we need to do more single writes / can't use static
register sequences.
A good example is the powerline frequency filter control, which I'm
currently experimenting with.
But if you don't want to take it at the moment, it's ok for me.
Besides, you're missing more calls to
i2c_smbus_write_byte_data() in ov2640_mask_set(), ov2640_s_register() and
ov2640_video_probe(). So, I'd just drop it.
I skipped that because of the different debug output (which could of
course be improved).
Regrads,
Frank
Thanks
Guennadi
+
static int ov2640_write_array(struct i2c_client *client,
const struct regval_list *vals)
{
int ret;
while ((vals-reg_num != 0xff) || (vals-value != 0xff)) {
-ret = i2c_smbus_write_byte_data(client,
-vals-reg_num, vals-value);
-dev_vdbg(client-dev, array: 0x%02x, 0x%02x,
- vals-reg_num, vals-value);
-
+ret = ov2640_write_single(client, vals-reg_num, vals-value);
if (ret 0)
return ret;
vals++;
@@ -704,13 +706,10 @@ static int ov2640_s_ctrl(struct v4l2_ctrl *ctrl)
struct v4l2_subdev *sd =
container_of(ctrl-handler, struct ov2640_priv, hdl)-subdev;
struct i2c_client *client = v4l2_get_subdevdata(sd);
-struct regval_list regval;
int ret;
u8 val;
-regval.reg_num = BANK_SEL;
-regval.value = BANK_SEL_SENS;
-ret = ov2640_write_array(client, regval);
+ret = ov2640_write_single(client, BANK_SEL, BANK_SEL_SENS);
if (ret 0)
return ret;
--
1.7.10.4
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to 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: tda8290 regression fix
On 2012-09-23 20:39, Anders Thomson wrote:
On 2012-09-23 19:54, Anders Thomson wrote:
diff --git a/drivers/media/pci/saa7134/saa7134-cards.c
b/drivers/media/pci/saa7134/saa7134-cards.c
index bc08f1d..98b482e 100644
--- a/drivers/media/pci/saa7134/saa7134-cards.c
+++ b/drivers/media/pci/saa7134/saa7134-cards.c
@@ -3288,13 +3288,13 @@ struct saa7134_board saa7134_boards[] = {
.name = Pinnacle PCTV 310i,
.audio_clock= 0x00187de7,
.tuner_type = TUNER_PHILIPS_TDA8290,
.radio_type = UNSET,
.tuner_addr = ADDR_UNSET,
.radio_addr = ADDR_UNSET,
- .tuner_config = 1,
+ .tuner_config = 0,
.mpeg = SAA7134_MPEG_DVB,
.gpiomask = 0x00020,
.inputs = {{
.name = name_tv,
.vmux = 4,
.amux = TV,
Please test if the above patch fixes the issue you're suffering[1]. If
so, then
we'll need to add a modprobe parameter to allow disabling LNA for
saa7134 devices
with LNA.
[1] Note: the above is not the fix, as some users of this board may be
using the
original antenna, and changing tuner_config will break things for them;
the right
fix is likely to allow controlling the LNA via userspace.
Tried that patch on 3.5.3. No improvement, unfortunately.
I have to retract that. It turns out that there is some strange interaction
between the cabletv box and the card. When I rebooted into 'my' patch
I still got the noisy signal. I then power cycled the cabletv box, and
voila,
I got a good signal on my own patch.
Awfully sorry about this. After having had the familty sit in and check
the differences,
I must say that the patch does not fix the issue. This time around I
have x11grabs with
ffmpeg to show if you want.
I'll be away from the card until the end of the coming week. Then, I'll
bring out the multimeter...
Best regards,
/Anders
--
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: Fwd: [PATCH 2/14] drivers/media/platform/soc_camera/mx2_camera.c: fix error return code
Hi Peter
Thanks for the patch, but I think it can be improved:
On Sun, 23 Sep 2012, Mauro Carvalho Chehab wrote:
Please review,
Regards,
Mauro.
Mensagem original
Assunto: [PATCH 2/14] drivers/media/platform/soc_camera/mx2_camera.c: fix
error return code
Data: Thu, 6 Sep 2012 17:23:59 +0200
De: Peter Senna Tschudin peter.se...@gmail.com
Para: Mauro Carvalho Chehab mche...@infradead.org
CC: kernel-janit...@vger.kernel.org, julia.law...@lip6.fr,
linux-media@vger.kernel.org, linux-ker...@vger.kernel.org
From: Peter Senna Tschudin peter.se...@gmail.com
Convert a nonnegative error return code to a negative one, as returned
elsewhere in the function.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// smpl
(
if@p1 (\(ret 0\|ret != 0\))
{ ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != ret
*if(...)
{
... when != ret = e2
when forall
return ret;
}
// /smpl
Signed-off-by: Peter Senna Tschudin peter.se...@gmail.com
---
drivers/media/platform/soc_camera/mx2_camera.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/soc_camera/mx2_camera.c
b/drivers/media/platform/soc_camera/mx2_camera.c
index 256187f..f8884a7 100644
--- a/drivers/media/platform/soc_camera/mx2_camera.c
+++ b/drivers/media/platform/soc_camera/mx2_camera.c
@@ -1800,13 +1800,16 @@ static int __devinit mx2_camera_probe(struct
platform_device *pdev)
if (!res_emma || !irq_emma) {
dev_err(pdev-dev, no EMMA resources\n);
+ err = -ENODEV;
goto exit_free_irq;
}
pcdev-res_emma = res_emma;
pcdev-irq_emma = irq_emma;
- if (mx27_camera_emma_init(pcdev))
+ if (mx27_camera_emma_init(pcdev)) {
+ err = -ENODEV;
I think, propagating the error, returned by mx27_camera_emma_init() to the
caller would be better, than using -ENODEV.
Thanks
Guennadi
goto exit_free_irq;
+ }
}
pcdev-soc_host.drv_name= MX2_CAM_DRV_NAME,
--
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
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] ov2640: select sensor register bank before applying h/v-flip settings
We currently don't select the register bank in ov2640_s_ctrl, so we can end up
writing to DSP register 0x04 instead of sensor register 0x04.
This happens for example when calling ov2640_s_ctrl after ov2640_s_fmt.
Signed-off-by: Frank Schäfer fschaefer@googlemail.com
Cc: sta...@kernel.org
---
drivers/media/i2c/soc_camera/ov2640.c |5 +
1 Datei geändert, 5 Zeilen hinzugefügt(+)
diff --git a/drivers/media/i2c/soc_camera/ov2640.c
b/drivers/media/i2c/soc_camera/ov2640.c
index 78ac574..d2d298b 100644
--- a/drivers/media/i2c/soc_camera/ov2640.c
+++ b/drivers/media/i2c/soc_camera/ov2640.c
@@ -684,6 +684,11 @@ static int ov2640_s_ctrl(struct v4l2_ctrl *ctrl)
container_of(ctrl-handler, struct ov2640_priv, hdl)-subdev;
struct i2c_client *client = v4l2_get_subdevdata(sd);
u8 val;
+ int ret;
+
+ ret = i2c_smbus_write_byte_data(client, BANK_SEL, BANK_SEL_SENS);
+ if (ret 0)
+ return ret;
switch (ctrl-id) {
case V4L2_CID_VFLIP:
--
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: [PATCH v2 3/3] ov2640: simplify single register writes
On Sun, 23 Sep 2012, Frank Schäfer wrote:
Am 23.09.2012 23:43, schrieb Guennadi Liakhovetski:
On Sun, 23 Sep 2012, Frank Schäfer wrote:
Signed-off-by: Frank Schäfer fschaefer@googlemail.com
---
drivers/media/i2c/soc_camera/ov2640.c | 17 -
1 Datei geändert, 8 Zeilen hinzugefügt(+), 9 Zeilen entfernt(-)
diff --git a/drivers/media/i2c/soc_camera/ov2640.c
b/drivers/media/i2c/soc_camera/ov2640.c
index 182d5a1..e71bf4c 100644
--- a/drivers/media/i2c/soc_camera/ov2640.c
+++ b/drivers/media/i2c/soc_camera/ov2640.c
@@ -639,17 +639,19 @@ static struct ov2640_priv *to_ov2640(const struct
i2c_client *client)
subdev);
}
+static int ov2640_write_single(struct i2c_client *client, u8 reg, u8 val)
+{
+ dev_vdbg(client-dev, write: 0x%02x, 0x%02x, reg, val);
+ return i2c_smbus_write_byte_data(client, reg, val);
+}
Well, I'm not convinced. I don't necessarily see it as a simplification.
You replace one perfectly ok function with another one with exactly the
same parameters. Ok, you also hide a debug printk() in your wrapper, but
that's not too useful either, IMHO.
Sure, at the moment this is not really needed. But that will change in
the future, when we need to do more single writes / can't use static
register sequences.
Why won't you be able to just use i2c_smbus_write_byte_data() directly
with those your sequences? Ok, if you just dislike the long name, and if
you have a number of them, I might buy that as a valid reason:-) And yes,
it'd be good to add such a helper function in a separate patch, preceding
the actual functional changes. But then I'd probably suggest to name, that
offers an even greater saving of your monitor real estate and is more
similar to what other drivers use, something like ov2640_reg_write() and
also add an ov2640_reg_read() for symmetry.
Thanks
Guennadi
A good example is the powerline frequency filter control, which I'm
currently experimenting with.
But if you don't want to take it at the moment, it's ok for me.
Besides, you're missing more calls to
i2c_smbus_write_byte_data() in ov2640_mask_set(), ov2640_s_register() and
ov2640_video_probe(). So, I'd just drop it.
I skipped that because of the different debug output (which could of
course be improved).
Regrads,
Frank
Thanks
Guennadi
+
static int ov2640_write_array(struct i2c_client *client,
const struct regval_list *vals)
{
int ret;
while ((vals-reg_num != 0xff) || (vals-value != 0xff)) {
- ret = i2c_smbus_write_byte_data(client,
- vals-reg_num, vals-value);
- dev_vdbg(client-dev, array: 0x%02x, 0x%02x,
- vals-reg_num, vals-value);
-
+ ret = ov2640_write_single(client, vals-reg_num, vals-value);
if (ret 0)
return ret;
vals++;
@@ -704,13 +706,10 @@ static int ov2640_s_ctrl(struct v4l2_ctrl *ctrl)
struct v4l2_subdev *sd =
container_of(ctrl-handler, struct ov2640_priv, hdl)-subdev;
struct i2c_client *client = v4l2_get_subdevdata(sd);
- struct regval_list regval;
int ret;
u8 val;
- regval.reg_num = BANK_SEL;
- regval.value = BANK_SEL_SENS;
- ret = ov2640_write_array(client, regval);
+ ret = ov2640_write_single(client, BANK_SEL, BANK_SEL_SENS);
if (ret 0)
return ret;
--
1.7.10.4
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fwd: [PATCH 1/14] drivers/media/platform/soc_camera/soc_camera.c: fix error return code
Hi Mauro, Peter
On Sun, 23 Sep 2012, Mauro Carvalho Chehab wrote:
Please review.
Please review.
Regards,
Mauro.
Mensagem original
Assunto: [PATCH 1/14] drivers/media/platform/soc_camera/soc_camera.c: fix
error return code
Data: Thu, 6 Sep 2012 17:24:00 +0200
De: Peter Senna Tschudin peter.se...@gmail.com
Para: Mauro Carvalho Chehab mche...@infradead.org
CC: kernel-janit...@vger.kernel.org, julia.law...@lip6.fr,
linux-media@vger.kernel.org, linux-ker...@vger.kernel.org
From: Peter Senna Tschudin peter.se...@gmail.com
Convert a nonnegative error return code to a negative one, as returned
elsewhere in the function.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// smpl
(
if@p1 (\(ret 0\|ret != 0\))
{ ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != ret
*if(...)
{
... when != ret = e2
when forall
return ret;
}
// /smpl
Signed-off-by: Peter Senna Tschudin peter.se...@gmail.com
---
drivers/media/platform/soc_camera/soc_camera.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/soc_camera/soc_camera.c
b/drivers/media/platform/soc_camera/soc_camera.c
index 10b57f8..a4beb6c 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -1184,7 +1184,8 @@ static int soc_camera_probe(struct soc_camera_device
*icd)
sd-grp_id = soc_camera_grp_id(icd);
v4l2_set_subdev_hostdata(sd, icd);
- if (v4l2_ctrl_add_handler(icd-ctrl_handler, sd-ctrl_handler))
+ ret = v4l2_ctrl_add_handler(icd-ctrl_handler, sd-ctrl_handler);
+ if (ret)
goto ectrl;
Yes, this is a correct fix. I'm actually also fixing it in one of my
current experimental patches, I don't think it's occurring too often in
real life, so, I didn't bother sending a separate fix:-) But yes, let's
fix it properly. Please, update the other patch to mx2_camera and I'll
send a fixes pull request with these two and an ov2640 fix.
Thanks
Guennadi
/* At this point client .probe() should have run already */
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ov2640: select sensor register bank before applying h/v-flip settings
On Sun, 23 Sep 2012, Frank SchÀfer wrote:
We currently don't select the register bank in ov2640_s_ctrl, so we can end up
writing to DSP register 0x04 instead of sensor register 0x04.
This happens for example when calling ov2640_s_ctrl after ov2640_s_fmt.
Signed-off-by: Frank SchÀfer fschaefer@googlemail.com
Cc: sta...@kernel.org
Ok, if Linus decides to release 3.6 tomorrow, I anyway don't think it'd be
reasonable to try to convince him to pull this hours before the release:-)
So, I'll wait for those other 2 fixes from Peter Senna / coccinelle and
submit a normal fixes pull request some time tomorrow. Just wondering:
---
drivers/media/i2c/soc_camera/ov2640.c |5 +
1 Datei geÀndert, 5 Zeilen hinzugefÌgt(+)
are we soon going to see this line in all possible languages / alphabets /
logographic systems? ;-)
Thanks
Guennadi
diff --git a/drivers/media/i2c/soc_camera/ov2640.c
b/drivers/media/i2c/soc_camera/ov2640.c
index 78ac574..d2d298b 100644
--- a/drivers/media/i2c/soc_camera/ov2640.c
+++ b/drivers/media/i2c/soc_camera/ov2640.c
@@ -684,6 +684,11 @@ static int ov2640_s_ctrl(struct v4l2_ctrl *ctrl)
container_of(ctrl-handler, struct ov2640_priv, hdl)-subdev;
struct i2c_client *client = v4l2_get_subdevdata(sd);
u8 val;
+ int ret;
+
+ ret = i2c_smbus_write_byte_data(client, BANK_SEL, BANK_SEL_SENS);
+ if (ret 0)
+ return ret;
switch (ctrl-id) {
case V4L2_CID_VFLIP:
--
1.7.10.4
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] ov2640: simplify single register writes
Am 24.09.2012 00:23, schrieb Guennadi Liakhovetski:
On Sun, 23 Sep 2012, Frank Schäfer wrote:
Am 23.09.2012 23:43, schrieb Guennadi Liakhovetski:
On Sun, 23 Sep 2012, Frank Schäfer wrote:
Signed-off-by: Frank Schäfer fschaefer@googlemail.com
---
drivers/media/i2c/soc_camera/ov2640.c | 17 -
1 Datei geändert, 8 Zeilen hinzugefügt(+), 9 Zeilen entfernt(-)
diff --git a/drivers/media/i2c/soc_camera/ov2640.c
b/drivers/media/i2c/soc_camera/ov2640.c
index 182d5a1..e71bf4c 100644
--- a/drivers/media/i2c/soc_camera/ov2640.c
+++ b/drivers/media/i2c/soc_camera/ov2640.c
@@ -639,17 +639,19 @@ static struct ov2640_priv *to_ov2640(const struct
i2c_client *client)
subdev);
}
+static int ov2640_write_single(struct i2c_client *client, u8 reg, u8 val)
+{
+ dev_vdbg(client-dev, write: 0x%02x, 0x%02x, reg, val);
+ return i2c_smbus_write_byte_data(client, reg, val);
+}
Well, I'm not convinced. I don't necessarily see it as a simplification.
You replace one perfectly ok function with another one with exactly the
same parameters. Ok, you also hide a debug printk() in your wrapper, but
that's not too useful either, IMHO.
Sure, at the moment this is not really needed. But that will change in
the future, when we need to do more single writes / can't use static
register sequences.
Why won't you be able to just use i2c_smbus_write_byte_data() directly
with those your sequences? Ok, if you just dislike the long name, and if
you have a number of them, I might buy that as a valid reason:-)
The suggest helper function also prints a debugging message, so we are
talking about two lines for each single write.
And yes,
it'd be good to add such a helper function in a separate patch, preceding
the actual functional changes. But then I'd probably suggest to name, that
offers an even greater saving of your monitor real estate and is more
similar to what other drivers use, something like ov2640_reg_write() and
also add an ov2640_reg_read() for symmetry.
Ok, thats a matter of taste ;). ov2640_write_single seemed to be the
logocal counterpart to ov2640_write_array, but I don't care.
I will come back to this when the next feature patch(es) are ready.
Regards,
Frank
Thanks
Guennadi
A good example is the powerline frequency filter control, which I'm
currently experimenting with.
But if you don't want to take it at the moment, it's ok for me.
Besides, you're missing more calls to
i2c_smbus_write_byte_data() in ov2640_mask_set(), ov2640_s_register() and
ov2640_video_probe(). So, I'd just drop it.
I skipped that because of the different debug output (which could of
course be improved).
Regrads,
Frank
Thanks
Guennadi
+
static int ov2640_write_array(struct i2c_client *client,
const struct regval_list *vals)
{
int ret;
while ((vals-reg_num != 0xff) || (vals-value != 0xff)) {
- ret = i2c_smbus_write_byte_data(client,
- vals-reg_num, vals-value);
- dev_vdbg(client-dev, array: 0x%02x, 0x%02x,
- vals-reg_num, vals-value);
-
+ ret = ov2640_write_single(client, vals-reg_num, vals-value);
if (ret 0)
return ret;
vals++;
@@ -704,13 +706,10 @@ static int ov2640_s_ctrl(struct v4l2_ctrl *ctrl)
struct v4l2_subdev *sd =
container_of(ctrl-handler, struct ov2640_priv, hdl)-subdev;
struct i2c_client *client = v4l2_get_subdevdata(sd);
- struct regval_list regval;
int ret;
u8 val;
- regval.reg_num = BANK_SEL;
- regval.value = BANK_SEL_SENS;
- ret = ov2640_write_array(client, regval);
+ ret = ov2640_write_single(client, BANK_SEL, BANK_SEL_SENS);
if (ret 0)
return ret;
--
1.7.10.4
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to 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] ov2640: select sensor register bank before applying h/v-flip settings
Am 24.09.2012 00:33, schrieb Guennadi Liakhovetski:
On Sun, 23 Sep 2012, Frank SchÀfer wrote:
We currently don't select the register bank in ov2640_s_ctrl, so we can end
up
writing to DSP register 0x04 instead of sensor register 0x04.
This happens for example when calling ov2640_s_ctrl after ov2640_s_fmt.
Signed-off-by: Frank SchÀfer fschaefer@googlemail.com
Cc: sta...@kernel.org
Ok, if Linus decides to release 3.6 tomorrow, I anyway don't think it'd be
reasonable to try to convince him to pull this hours before the release:-)
So, I'll wait for those other 2 fixes from Peter Senna / coccinelle and
submit a normal fixes pull request some time tomorrow. Just wondering:
Sure.
---
drivers/media/i2c/soc_camera/ov2640.c |5 +
1 Datei geÀndert, 5 Zeilen hinzugefÌgt(+)
are we soon going to see this line in all possible languages / alphabets /
logographic systems? ;-)
I don't know, I see this only in your replies, so it seems to be a
problem with your mail client ?
Regards,
Frank
Thanks
Guennadi
diff --git a/drivers/media/i2c/soc_camera/ov2640.c
b/drivers/media/i2c/soc_camera/ov2640.c
index 78ac574..d2d298b 100644
--- a/drivers/media/i2c/soc_camera/ov2640.c
+++ b/drivers/media/i2c/soc_camera/ov2640.c
@@ -684,6 +684,11 @@ static int ov2640_s_ctrl(struct v4l2_ctrl *ctrl)
container_of(ctrl-handler, struct ov2640_priv, hdl)-subdev;
struct i2c_client *client = v4l2_get_subdevdata(sd);
u8 val;
+int ret;
+
+ret = i2c_smbus_write_byte_data(client, BANK_SEL, BANK_SEL_SENS);
+if (ret 0)
+return ret;
switch (ctrl-id) {
case V4L2_CID_VFLIP:
--
1.7.10.4
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to 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] ov2640: select sensor register bank before applying h/v-flip settings
On Sun, 23 Sep 2012, Frank Schäfer wrote:
Am 24.09.2012 00:33, schrieb Guennadi Liakhovetski:
On Sun, 23 Sep 2012, Frank SchÀfer wrote:
We currently don't select the register bank in ov2640_s_ctrl, so we can
end up
writing to DSP register 0x04 instead of sensor register 0x04.
This happens for example when calling ov2640_s_ctrl after ov2640_s_fmt.
Signed-off-by: Frank SchÀfer fschaefer@googlemail.com
Cc: sta...@kernel.org
Ok, if Linus decides to release 3.6 tomorrow, I anyway don't think it'd be
reasonable to try to convince him to pull this hours before the release:-)
So, I'll wait for those other 2 fixes from Peter Senna / coccinelle and
submit a normal fixes pull request some time tomorrow. Just wondering:
Sure.
---
drivers/media/i2c/soc_camera/ov2640.c |5 +
1 Datei geÀndert, 5 Zeilen hinzugefÌgt(+)
are we soon going to see this line in all possible languages / alphabets /
logographic systems? ;-)
I don't know, I see this only in your replies, so it seems to be a
problem with your mail client ?
Sorry, I don't mean those specific Umlaut characters:-) I mean in general
- here it's in German, next time we see it in Russian / cyrillic, then in
Japanese / kanji, then in hebrew... See what I mean? :-) I certainly don't
have anything against those languages as such, I myself switch between a
couple of them all the time, but this specific line carries information,
that should be understood by all on this list, IMHO. But well, maybe we'll
just get used to it just as well as to Am ... schrieb ... etc:-) And I
certainly realise that you (probably:-)) have nothing to do with this,
rather git maintainers, anyway, enough rant for a Sunday evening:-)
Thanks
Guennadi
Regards,
Frank
Thanks
Guennadi
diff --git a/drivers/media/i2c/soc_camera/ov2640.c
b/drivers/media/i2c/soc_camera/ov2640.c
index 78ac574..d2d298b 100644
--- a/drivers/media/i2c/soc_camera/ov2640.c
+++ b/drivers/media/i2c/soc_camera/ov2640.c
@@ -684,6 +684,11 @@ static int ov2640_s_ctrl(struct v4l2_ctrl *ctrl)
container_of(ctrl-handler, struct ov2640_priv, hdl)-subdev;
struct i2c_client *client = v4l2_get_subdevdata(sd);
u8 val;
+ int ret;
+
+ ret = i2c_smbus_write_byte_data(client, BANK_SEL, BANK_SEL_SENS);
+ if (ret 0)
+ return ret;
switch (ctrl-id) {
case V4L2_CID_VFLIP:
--
1.7.10.4
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] stk1160: Make kill/free urb debug message more verbose
Mauro,
On Sun, Aug 19, 2012 at 10:23 PM, Ezequiel Garcia elezegar...@gmail.com wrote:
This is just a cleaning patch to produce more useful
debug messages.
Signed-off-by: Ezequiel Garcia elezegar...@gmail.com
---
drivers/media/usb/stk1160/stk1160-video.c | 14 +++---
1 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/media/usb/stk1160/stk1160-video.c
b/drivers/media/usb/stk1160/stk1160-video.c
index 3785269..022092a 100644
--- a/drivers/media/usb/stk1160/stk1160-video.c
+++ b/drivers/media/usb/stk1160/stk1160-video.c
@@ -342,18 +342,18 @@ static void stk1160_isoc_irq(struct urb *urb)
*/
void stk1160_cancel_isoc(struct stk1160 *dev)
{
- int i;
+ int i, num_bufs = dev-isoc_ctl.num_bufs;
/*
* This check is not necessary, but we add it
* to avoid a spurious debug message
*/
- if (!dev-isoc_ctl.num_bufs)
+ if (!num_bufs)
return;
- stk1160_dbg(killing urbs...\n);
+ stk1160_dbg(killing %d urbs...\n, num_bufs);
- for (i = 0; i dev-isoc_ctl.num_bufs; i++) {
+ for (i = 0; i num_bufs; i++) {
/*
* To kill urbs we can't be in atomic context.
@@ -373,11 +373,11 @@ void stk1160_cancel_isoc(struct stk1160 *dev)
void stk1160_free_isoc(struct stk1160 *dev)
{
struct urb *urb;
- int i;
+ int i, num_bufs = dev-isoc_ctl.num_bufs;
- stk1160_dbg(freeing urb buffers...\n);
+ stk1160_dbg(freeing %d urb buffers...\n, num_bufs);
- for (i = 0; i dev-isoc_ctl.num_bufs; i++) {
+ for (i = 0; i num_bufs; i++) {
urb = dev-isoc_ctl.urb[i];
if (urb) {
--
1.7.8.6
Please don't forget these patches for your v3.7 pull request.
Unless you don't want to add them yet.
Thanks,
Ezequiel.
--
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 07/16] rtl2830: use .get_if_frequency()
Em Thu, 13 Sep 2012 03:23:48 +0300 Antti Palosaari cr...@iki.fi escreveu: Use .get_if_frequency() as all used tuner drivers (mt2060/qt1010/mxl5005s) supports it. Signed-off-by: Antti Palosaari cr...@iki.fi @@ -240,26 +237,6 @@ static int rtl2830_init(struct dvb_frontend *fe) if (ret) goto err; - num = priv-cfg.if_dvbt % priv-cfg.xtal; - num *= 0x40; - num = div_u64(num, priv-cfg.xtal); - num = -num; - if_ctl = num 0x3f; - dev_dbg(priv-i2c-dev, %s: if_ctl=%08x\n, __func__, if_ctl); - - ret = rtl2830_rd_reg_mask(priv, 0x119, tmp, 0xc0); /* b[7:6] */ - if (ret) - goto err; - - buf[0] = tmp 6; - buf[0] |= (if_ctl 16) 0x3f; - buf[1] = (if_ctl 8) 0xff; - buf[2] = (if_ctl 0) 0xff; Patch applied, but there was a context difference above: --- a/drivers/media/dvb-frontends/rtl2830.c +++ b/drivers/media/dvb-frontends/rtl2830.c @@ -182,9 +182,6 @@ static int rtl2830_init(struct dvb_frontend *fe) @@ -28,7 +50,7 @@ index eca1d72..3954760 100644 - goto err; - - buf[0] = tmp 6; -- buf[0] = (if_ctl 16) 0x3f; +- buf[0] |= (if_ctl 16) 0x3f; - buf[1] = (if_ctl 8) 0xff; - buf[2] = (if_ctl 0) 0xff; - (that's the diff between the patch applied and your original one) -- Regards, 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 07/16] rtl2830: use .get_if_frequency()
On 09/24/2012 02:17 AM, Mauro Carvalho Chehab wrote: Em Thu, 13 Sep 2012 03:23:48 +0300 Antti Palosaari cr...@iki.fi escreveu: Use .get_if_frequency() as all used tuner drivers (mt2060/qt1010/mxl5005s) supports it. Signed-off-by: Antti Palosaari cr...@iki.fi @@ -240,26 +237,6 @@ static int rtl2830_init(struct dvb_frontend *fe) if (ret) goto err; - num = priv-cfg.if_dvbt % priv-cfg.xtal; - num *= 0x40; - num = div_u64(num, priv-cfg.xtal); - num = -num; - if_ctl = num 0x3f; - dev_dbg(priv-i2c-dev, %s: if_ctl=%08x\n, __func__, if_ctl); - - ret = rtl2830_rd_reg_mask(priv, 0x119, tmp, 0xc0); /* b[7:6] */ - if (ret) - goto err; - - buf[0] = tmp 6; - buf[0] |= (if_ctl 16) 0x3f; - buf[1] = (if_ctl 8) 0xff; - buf[2] = (if_ctl 0) 0xff; Patch applied, but there was a context difference above: --- a/drivers/media/dvb-frontends/rtl2830.c +++ b/drivers/media/dvb-frontends/rtl2830.c @@ -182,9 +182,6 @@ static int rtl2830_init(struct dvb_frontend *fe) @@ -28,7 +50,7 @@ index eca1d72..3954760 100644 - goto err; - - buf[0] = tmp 6; -- buf[0] = (if_ctl 16) 0x3f; +- buf[0] |= (if_ctl 16) 0x3f; - buf[1] = (if_ctl 8) 0xff; - buf[2] = (if_ctl 0) 0xff; - (that's the diff between the patch applied and your original one) Because of that: http://patchwork.linuxtv.org/patch/14066/ 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
Re: [PATCH 07/16] rtl2830: use .get_if_frequency()
Em Mon, 24 Sep 2012 03:08:17 +0300 Antti Palosaari cr...@iki.fi escreveu: On 09/24/2012 02:17 AM, Mauro Carvalho Chehab wrote: Em Thu, 13 Sep 2012 03:23:48 +0300 Antti Palosaari cr...@iki.fi escreveu: Use .get_if_frequency() as all used tuner drivers (mt2060/qt1010/mxl5005s) supports it. Signed-off-by: Antti Palosaari cr...@iki.fi @@ -240,26 +237,6 @@ static int rtl2830_init(struct dvb_frontend *fe) if (ret) goto err; - num = priv-cfg.if_dvbt % priv-cfg.xtal; - num *= 0x40; - num = div_u64(num, priv-cfg.xtal); - num = -num; - if_ctl = num 0x3f; - dev_dbg(priv-i2c-dev, %s: if_ctl=%08x\n, __func__, if_ctl); - - ret = rtl2830_rd_reg_mask(priv, 0x119, tmp, 0xc0); /* b[7:6] */ - if (ret) - goto err; - - buf[0] = tmp 6; - buf[0] |= (if_ctl 16) 0x3f; - buf[1] = (if_ctl 8) 0xff; - buf[2] = (if_ctl 0) 0xff; Patch applied, but there was a context difference above: --- a/drivers/media/dvb-frontends/rtl2830.c +++ b/drivers/media/dvb-frontends/rtl2830.c @@ -182,9 +182,6 @@ static int rtl2830_init(struct dvb_frontend *fe) @@ -28,7 +50,7 @@ index eca1d72..3954760 100644 - goto err; - - buf[0] = tmp 6; -- buf[0] = (if_ctl 16) 0x3f; +- buf[0] |= (if_ctl 16) 0x3f; - buf[1] = (if_ctl 8) 0xff; - buf[2] = (if_ctl 0) 0xff; - (that's the diff between the patch applied and your original one) Because of that: http://patchwork.linuxtv.org/patch/14066/ That's why I ask driver maintainers to send me pull requests, instead of sending long series of patches at the mailing list, and tagging the patches for review at ML as RFC: it is not warranted that the patches will be merged at the order they're sent to the mailing list. -- Regards, 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 07/16] rtl2830: use .get_if_frequency()
On 09/24/2012 03:23 AM, Mauro Carvalho Chehab wrote: Em Mon, 24 Sep 2012 03:08:17 +0300 Antti Palosaari cr...@iki.fi escreveu: On 09/24/2012 02:17 AM, Mauro Carvalho Chehab wrote: Em Thu, 13 Sep 2012 03:23:48 +0300 Antti Palosaari cr...@iki.fi escreveu: Use .get_if_frequency() as all used tuner drivers (mt2060/qt1010/mxl5005s) supports it. Signed-off-by: Antti Palosaari cr...@iki.fi @@ -240,26 +237,6 @@ static int rtl2830_init(struct dvb_frontend *fe) if (ret) goto err; - num = priv-cfg.if_dvbt % priv-cfg.xtal; - num *= 0x40; - num = div_u64(num, priv-cfg.xtal); - num = -num; - if_ctl = num 0x3f; - dev_dbg(priv-i2c-dev, %s: if_ctl=%08x\n, __func__, if_ctl); - - ret = rtl2830_rd_reg_mask(priv, 0x119, tmp, 0xc0); /* b[7:6] */ - if (ret) - goto err; - - buf[0] = tmp 6; - buf[0] |= (if_ctl 16) 0x3f; - buf[1] = (if_ctl 8) 0xff; - buf[2] = (if_ctl 0) 0xff; Patch applied, but there was a context difference above: --- a/drivers/media/dvb-frontends/rtl2830.c +++ b/drivers/media/dvb-frontends/rtl2830.c @@ -182,9 +182,6 @@ static int rtl2830_init(struct dvb_frontend *fe) @@ -28,7 +50,7 @@ index eca1d72..3954760 100644 -goto err; - -buf[0] = tmp 6; -- buf[0] = (if_ctl 16) 0x3f; +- buf[0] |= (if_ctl 16) 0x3f; -buf[1] = (if_ctl 8) 0xff; -buf[2] = (if_ctl 0) 0xff; - (that's the diff between the patch applied and your original one) Because of that: http://patchwork.linuxtv.org/patch/14066/ That's why I ask driver maintainers to send me pull requests, instead of sending long series of patches at the mailing list, and tagging the patches for review at ML as RFC: it is not warranted that the patches will be merged at the order they're sent to the mailing list. Do you mean I start again review pick those patches myself from the mailing list and pull-request then from git tree? It is fine for me. How about my own patches for my own drivers. Should I sent those to the mailing list and then pull-request via git? If yes, is there some tag which could be used to inform that this patch will be pull-requested via git tree? 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
Re: [PATCH] bt8xx: Add video4linux control V4L2_CID_COLOR_KILLER.
Oi Guilherme,
Em Thu, 13 Sep 2012 14:12:11 -0300
Guilherme Herrmann Destefani linu...@destefani.eng.br escreveu:
Added V4L2_CID_COLOR_KILLER control to the bt8xx driver.
The control V4L2_CID_PRIVATE_CHROMA_AGC was changed too because
with this change the bttv driver must touch two bits in the
SC Loop Control Registers, for controls V4L2_CID_COLOR_KILLER
and V4L2_CID_PRIVATE_CHROMA_AGC.
---
Documentation/video4linux/bttv/Insmod-options | 1 +
drivers/media/video/bt8xx/bttv-driver.c | 36
---
drivers/media/video/bt8xx/bttvp.h | 1 +
3 files changed, 34 insertions(+), 4 deletions(-)
diff --git a/Documentation/video4linux/bttv/Insmod-options
b/Documentation/video4linux/bttv/Insmod-options
index 14c065fa..089e961 100644
--- a/Documentation/video4linux/bttv/Insmod-options
+++ b/Documentation/video4linux/bttv/Insmod-options
@@ -53,6 +53,7 @@ bttv.o
quality which leading to unwanted sound
dropouts.
chroma_agc=0/1 AGC of chroma signal, off by default.
+ color_killer=0/1 Low color detection and removal, off by
default
adc_crush=0/1 Luminance ADC crush, on by default.
i2c_udelay= Allow reduce I2C speed. Default is 5 usecs
(meaning 66,67 Kbps). The default is the
diff --git a/drivers/media/video/bt8xx/bttv-driver.c
b/drivers/media/video/bt8xx/bttv-driver.c
index e581b37..7208d5a 100644
--- a/drivers/media/video/bt8xx/bttv-driver.c
+++ b/drivers/media/video/bt8xx/bttv-driver.c
@@ -93,6 +93,7 @@ static unsigned int combfilter;
static unsigned int lumafilter;
static unsigned int automute= 1;
static unsigned int chroma_agc;
+static unsigned int color_killer;
static unsigned int adc_crush = 1;
static unsigned int whitecrush_upper = 0xCF;
static unsigned int whitecrush_lower = 0x7F;
@@ -125,6 +126,7 @@ module_param(combfilter,int, 0444);
module_param(lumafilter,int, 0444);
module_param(automute, int, 0444);
module_param(chroma_agc,int, 0444);
+module_param(color_killer, int, 0444);
module_param(adc_crush, int, 0444);
module_param(whitecrush_upper, int, 0444);
module_param(whitecrush_lower, int, 0444);
@@ -151,6 +153,7 @@ MODULE_PARM_DESC(reset_crop,reset cropping parameters at
open(), default
is 1 (yes) for compatibility with older applications);
MODULE_PARM_DESC(automute,mute audio on bad/missing video signal, default
is 1 (yes));
MODULE_PARM_DESC(chroma_agc,enables the AGC of chroma signal, default is 0
(no));
+MODULE_PARM_DESC(color_killer,enables the low color detector and removal,
default is 0 (no));
Why to add a parameter here? No other driver uses insmod parameter
to configure controls. Ok, this driver is legacy, so, most of those
stuff are there since nobody cared enough to remove those things,
but let's not add more things here.
MODULE_PARM_DESC(adc_crush,enables the luminance ADC crush, default is 1
(yes));
MODULE_PARM_DESC(whitecrush_upper,sets the white crush upper value, default
is 207);
MODULE_PARM_DESC(whitecrush_lower,sets the white crush lower value, default
is 127);
@@ -674,6 +677,12 @@ static const struct v4l2_queryctrl bttv_ctls[] = {
.default_value = 32768,
.type = V4L2_CTRL_TYPE_INTEGER,
},{
+ .id= V4L2_CID_COLOR_KILLER,
+ .name = Color killer,
+ .minimum = 0,
+ .maximum = 1,
+ .type = V4L2_CTRL_TYPE_BOOLEAN,
+ },{
.id= V4L2_CID_HUE,
.name = Hue,
.minimum = 0,
@@ -1412,6 +1421,8 @@ static void init_bt848(struct bttv *btv)
BT848_GPIO_DMA_CTL);
val = btv-opt_chroma_agc ? BT848_SCLOOP_CAGC : 0;
+ if (btv-opt_color_killer)
+ val |= BT848_SCLOOP_CKILL;
btwrite(val, BT848_E_SCLOOP);
btwrite(val, BT848_O_SCLOOP);
@@ -1475,6 +1486,9 @@ static int bttv_g_ctrl(struct file *file, void *priv,
case V4L2_CID_SATURATION:
c-value = btv-saturation;
break;
+ case V4L2_CID_COLOR_KILLER:
+ c-value = btv-opt_color_killer;
+ break;
case V4L2_CID_AUDIO_MUTE:
case V4L2_CID_AUDIO_VOLUME:
@@ -1527,7 +1541,6 @@ static int bttv_s_ctrl(struct file *file, void *f,
struct v4l2_control *c)
{
int err;
- int val;
struct bttv_fh *fh = f;
struct bttv *btv = fh-btv;
@@ -1548,6 +1561,16 @@ static int bttv_s_ctrl(struct file *file, void *f,
case V4L2_CID_SATURATION:
bt848_sat(btv, c-value);
break;
+ case V4L2_CID_COLOR_KILLER:
+ btv-opt_color_killer = c-value;
+ if
Re: [PATCH 07/16] rtl2830: use .get_if_frequency()
Em Mon, 24 Sep 2012 03:28:35 +0300 Antti Palosaari cr...@iki.fi escreveu: On 09/24/2012 03:23 AM, Mauro Carvalho Chehab wrote: Em Mon, 24 Sep 2012 03:08:17 +0300 Antti Palosaari cr...@iki.fi escreveu: On 09/24/2012 02:17 AM, Mauro Carvalho Chehab wrote: Em Thu, 13 Sep 2012 03:23:48 +0300 Antti Palosaari cr...@iki.fi escreveu: Use .get_if_frequency() as all used tuner drivers (mt2060/qt1010/mxl5005s) supports it. Signed-off-by: Antti Palosaari cr...@iki.fi @@ -240,26 +237,6 @@ static int rtl2830_init(struct dvb_frontend *fe) if (ret) goto err; -num = priv-cfg.if_dvbt % priv-cfg.xtal; -num *= 0x40; -num = div_u64(num, priv-cfg.xtal); -num = -num; -if_ctl = num 0x3f; -dev_dbg(priv-i2c-dev, %s: if_ctl=%08x\n, __func__, if_ctl); - -ret = rtl2830_rd_reg_mask(priv, 0x119, tmp, 0xc0); /* b[7:6] */ -if (ret) -goto err; - -buf[0] = tmp 6; -buf[0] |= (if_ctl 16) 0x3f; -buf[1] = (if_ctl 8) 0xff; -buf[2] = (if_ctl 0) 0xff; Patch applied, but there was a context difference above: --- a/drivers/media/dvb-frontends/rtl2830.c +++ b/drivers/media/dvb-frontends/rtl2830.c @@ -182,9 +182,6 @@ static int rtl2830_init(struct dvb_frontend *fe) @@ -28,7 +50,7 @@ index eca1d72..3954760 100644 - goto err; - - buf[0] = tmp 6; --buf[0] = (if_ctl 16) 0x3f; +-buf[0] |= (if_ctl 16) 0x3f; - buf[1] = (if_ctl 8) 0xff; - buf[2] = (if_ctl 0) 0xff; - (that's the diff between the patch applied and your original one) Because of that: http://patchwork.linuxtv.org/patch/14066/ That's why I ask driver maintainers to send me pull requests, instead of sending long series of patches at the mailing list, and tagging the patches for review at ML as RFC: it is not warranted that the patches will be merged at the order they're sent to the mailing list. Do you mean I start again review pick those patches myself from the mailing list and pull-request then from git tree? It is fine for me. How about my own patches for my own drivers. Should I sent those to the mailing list and then pull-request via git? If yes, is there some tag which could be used to inform that this patch will be pull-requested via git tree? What I mean is that the better is for you to apply the patches for your driver on your tree (your patches and the ones from the others you acked). Then, from time to time[1], send me pull requests from the branch that contains them. That warrants that I'll apply the patches at the right order, and that I won't forget anything. As your patches also need to be reviewed, you can tag them as RFC. [1] Don't accumulate too many patches there... finding time to review a series of 5-10 patches is easier than to review 20-30 patches. 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 00/14] Media Controller capture driver for DM365
Hi Sakari, On Sun, Sep 23, 2012 at 8:46 PM, Sakari Ailus sakari.ai...@iki.fi wrote: Hi Prabhakar, Prabhakar Lad wrote: From: Lad, Prabhakar prabhakar@ti.com This patch set adds media controller based capture driver for DM365. Thanks for the set. Do you happen to have an updated version of the same documentation you posted to the list a while ago? Yes I have included the documentation patch in same series. 'Documentation/video4linux/davinci-vpfe-mc.txt' is the one which contains the documentation. Regards, --Prabhakar Lad Kind regards, -- Sakari Ailus sakari.ai...@iki.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
